Add more comments and description to the logic.

chromium_src/components/content_settings/core/browser/cookie_settings.cc

@@ -8,12 +8,19 @@
 #include "net/base/features.h"
 #include "net/base/url_util.h"
 
+// Determines whether a 3p cookies block should be applied if a requesting URL
+// uses an explicit 1PES setting (CONTENT_SETTING_SESSION_ONLY). By default
+// Chromimum allows all 3p cookies if applied CookieSettingsPatterns for the URL
+// were explicit. We use explicit setting to enable 1PES mode, but in this mode
+// we still want to block 3p frames as usual and not fallback to "allow
+// everything" path.
 #define BRAVE_COOKIE_SETTINGS_GET_COOKIES_SETTINGS_INTERNAL               \
   if (!block && is_third_party_request) {                                 \
     block = ShouldBlockThirdPartyIfSettingIsExplicit(                     \
         ShouldBlockThirdPartyCookies(), setting, IsExplicitSetting(info), \
         first_party_url.SchemeIs(extension_scheme_));                     \
   }                                                                       \
+  /* Store patterns information to determine if Shields are disabled. */  \
   if (auto* setting_with_brave_metadata =                                 \
           cookie_setting_with_brave_metadata()) {                         \
     setting_with_brave_metadata->primary_pattern_matches_all_hosts =      \

chromium_src/components/content_settings/core/common/cookie_settings_base.cc

@@ -111,8 +111,8 @@ bool CookieSettingsBase::ShouldUseEphemeralStorage(
   if (!first_party_url.is_valid())
     return false;
 
-  // SESSION_ONLY cookie setting works as a "1PES enabler" for the website if
-  // a 1PES feature is enabled.
+  // Enable ephemeral storage for a first party URL if SESSION_ONLY cookie
+  // setting is set and the feature is enabled.
   if (base::FeatureList::IsEnabled(
           net::features::kBraveFirstPartyEphemeralStorage) &&
       IsCookieSessionOnly(first_party_url)) {
@@ -188,37 +188,49 @@ bool CookieSettingsBase::IsCookieAccessAllowedImpl(
   const GURL first_party_url = GetFirstPartyURL(
       site_for_cookies, base::OptionalOrNullptr(top_frame_origin));
 
-  // Determine whether a first party frame is ephemeral or shields are down.
-  enum class FirstPartyFrameMode {
+  // Determine whether a main frame is ephemeral or Shields are down.
+  // This is required to properly handle main and nested frames depending on the
+  // main frame mode.
+  enum class MainFrameMode {
+    // Main frame works as usual, nested 3p frames use ephemeral storage if
+    // necessary.
     kDefault,
+    // Main frame is in Ephemeral Storage mode, 1p/3p frames use ephemeral
+    // storage if necessary.
     kEphemeral,
+    // Main frame is in "Shields down" mode, all 1p/3p frames should use
+    // persistend storage, *including* 3p frames with enabled "First party
+    // ephemeral storage" mode.
     kShieldsDown,
   };
-  FirstPartyFrameMode first_party_frame_mode = FirstPartyFrameMode::kDefault;
+  MainFrameMode main_frame_mode = MainFrameMode::kDefault;
   if (is_1p_ephemeral_feature_enabled) {
+    // Get CookieSetting for the main frame and get matched patterns if any.
     CookieSettingWithBraveMetadata setting_with_brave_metadata =
         GetCookieSettingWithBraveMetadata(first_party_url, first_party_url);
 
     if (setting_with_brave_metadata.setting == CONTENT_SETTING_SESSION_ONLY) {
-      first_party_frame_mode = FirstPartyFrameMode::kEphemeral;
-    } else if (setting_with_brave_metadata.setting == CONTENT_SETTING_ALLOW &&
-               setting_with_brave_metadata.primary_pattern_matches_all_hosts &&
-               !setting_with_brave_metadata
-                    .secondary_pattern_matches_all_hosts) {
-      // Disabled shields mode allows everything in nested frames, so we
-      // determine the fact that shields are disabled by expecting primary and
-      // secondary patterns to be in a specific state.
-      first_party_frame_mode = FirstPartyFrameMode::kShieldsDown;
+      main_frame_mode = MainFrameMode::kEphemeral;
+    } else {
+      // Disabled shields mode allows everything in nested frames. To properly
+      // handle this state we need to know if Shields are down in the main
+      // frame. The shields check is done by analyzing the primary and secondary
+      // patterns and expecting them to be in a specific state.
+      if (setting_with_brave_metadata.setting == CONTENT_SETTING_ALLOW &&
+          setting_with_brave_metadata.primary_pattern_matches_all_hosts &&
+          !setting_with_brave_metadata.secondary_pattern_matches_all_hosts) {
+        main_frame_mode = MainFrameMode::kShieldsDown;
+      }
     }
   }
 
   if (allow) {
-    // Block all non ephemeral-supported activities (service workers, etc.) if
-    // 1p is ephemeral.
-    if (first_party_frame_mode == FirstPartyFrameMode::kEphemeral) {
-      allow = false;
+    // When the main frame is in ephemeral mode, we should block all non
+    // ephemeral-supported activities (service workers, etc.).
+    if (main_frame_mode == MainFrameMode::kEphemeral) {
+      return false;
     }
-    return allow;
+    return true;
   }
 
   if (!IsFirstPartyAccessAllowed(first_party_url, this))
@@ -227,10 +239,11 @@ bool CookieSettingsBase::IsCookieAccessAllowedImpl(
   if (BraveIsAllowedThirdParty(url, first_party_url, this))
     return true;
 
+  // This allows Session-only frames to work as usual when Shields are down for
+  // the main frame.
   if (is_1p_ephemeral_feature_enabled &&
-      first_party_frame_mode == FirstPartyFrameMode::kShieldsDown &&
+      main_frame_mode == MainFrameMode::kShieldsDown &&
       IsCookieSessionOnly(url)) {
-    // Allow 3p session-only frames when shields are disabled.
     return true;
   }
 

chromium_src/services/network/cookie_settings.cc

@@ -8,6 +8,12 @@
 #include "net/base/features.h"
 #include "url/origin.h"
 
+// Determines whether a 3p cookies block should be applied if a requesting URL
+// uses an explicit 1PES setting (CONTENT_SETTING_SESSION_ONLY). By default
+// Chromimum allows all 3p cookies if applied CookieSettingsPatterns for the URL
+// were explicit. We use explicit setting to enable 1PES mode, but in this mode
+// we still want to block 3p frames as usual and not fallback to "allow
+// everything" path.
 #define BRAVE_COOKIE_SETTINGS_GET_COOKIE_SETTINGS_INTERNAL                     \
   if (!blocked_by_third_party_setting && is_third_party_request) {             \
     blocked_by_third_party_setting = ShouldBlockThirdPartyIfSettingIsExplicit( \
@@ -16,6 +22,7 @@
         base::Contains(third_party_cookies_allowed_schemes_,                   \
                        first_party_url.scheme()));                             \
   }                                                                            \
+  /* Store patterns information to determine if Shields are disabled. */       \
   if (auto* setting_with_brave_metadata =                                      \
           cookie_setting_with_brave_metadata()) {                              \
     setting_with_brave_metadata->primary_pattern_matches_all_hosts =           \