Make ShouldBlockThirdPartyCookies consistent with 3pc restrictions

This fixes an inconsistency in network::CookieSettings, where
ShouldBlockThirdPartyCookies erroneously returned false when 3PC
restrictions were enabled via flags, but the block_third_party_cookies_
member had not been set to true (yet).

In real Chrome, this condition was not possible to provoke; but it was
possible in tests, which made the tests confusing and misleading.

Note that the other CookieSettings implementation doesn't suffer from
this inconsistency, since it uses a single boolean to track both the
global 3pcb setting as well as various ways of enabling 3pc
restrictions. (Ref: https://crsrc.org/c/components/content_settings/core/browser/cookie_settings.cc;drc=3deaac799dc39f0ba14200463fd31bba49eb72a6;l=384-392)

Change-Id: Ie3b4ccfa179a82a4fd541c143a6d47b5fc5a7afe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6226539
Reviewed-by: Aaron Selya <selya@google.com>
Auto-Submit: Chris Fredrickson <cfredric@chromium.org>
Commit-Queue: Aaron Selya <selya@google.com>
Cr-Commit-Position: refs/heads/main@{#1415044}

chrome/browser/top_level_storage_access_api/request_storage_access_for_browsertest.cc

@@ -1387,9 +1387,12 @@ INSTANTIATE_TEST_SUITE_P(
         TopLevelStorageExemptionReasonTestData{
             .cookie_set_mechanism = CookieSetMechanism::kBrowserInternal,
             .cookie_name_value = {"cross-site=b.test"},
+            // Note: setting the block-third-party-cookies pref is not
+            // sufficient to unblock third-party cookies, since cookies can be
+            // blocked for other reasons (e.g. CLI switch).
             .block_third_party_cookies = false,
             .expected_cookie_string = "cross-site=b.test",
-            .expected_metric_value = {},
+            .expected_metric_value = {1},
         },
         // One cookie access granted through TopLevelStorage.
         TopLevelStorageExemptionReasonTestData{

components/content_settings/core/common/cookie_settings_base.h

@@ -509,7 +509,7 @@ class CookieSettingsBase {
   virtual bool ShouldAlwaysAllowCookies(const GURL& url,
                                         const GURL& first_party_url) const = 0;
 
-  // Returns whether the global 3p cookie blocking setting is enabled.
+  // Returns whether third-party cookies are blocked.
   virtual bool ShouldBlockThirdPartyCookies() const = 0;
 
   // Returns whether Third Party Cookie Deprecation mitigations should take

services/network/cookie_settings.cc

@@ -386,7 +386,9 @@ bool CookieSettings::IsThirdPartyCookiesAllowedScheme(
 }
 
 bool CookieSettings::ShouldBlockThirdPartyCookies() const {
-  return block_third_party_cookies_;
+  return block_third_party_cookies_ ||
+         IsThirdPartyPhaseoutEnabled(std::nullopt,
+                                     net::CookieSettingOverrides());
 }
 
 bool CookieSettings::IsThirdPartyPhaseoutEnabled(

services/network/cookie_settings_unittest.cc

@@ -1375,25 +1375,17 @@ TEST_P(CookieSettingsTestP, IsPrivacyModeEnabled) {
 
 TEST_P(CookieSettingsTestP, IsCookieAccessible_SameSiteNoneCookie) {
   CookieSettings settings;
-  net::CookieInclusionStatus status;
-  settings.set_block_third_party_cookies(false);
-
-  std::unique_ptr<net::CanonicalCookie> cookie =
-      MakeCanonicalSameSiteNoneCookie("name", kURL);
-
-  EXPECT_TRUE(settings.IsCookieAccessible(
-      *cookie, GURL(kURL), net::SiteForCookies(),
-      url::Origin::Create(GURL(kOtherURL)), net::FirstPartySetMetadata(),
-      GetCookieSettingOverrides(), &status));
-
   settings.set_block_third_party_cookies(true);
   if (IsTrackingProtectionEnabledFor3pcd()) {
     settings.set_tracking_protection_enabled_for_3pcd(true);
   }
 
+  std::unique_ptr<net::CanonicalCookie> cookie =
+      MakeCanonicalSameSiteNoneCookie("name", kURL);
+
   // Third-party cookies are blocked, so the cookie should not be accessible by
   // default in a third-party context.
-  status.ResetForTesting();
+  net::CookieInclusionStatus status;
   EXPECT_FALSE(settings.IsCookieAccessible(
       *cookie, GURL(kURL), net::SiteForCookies(),
       url::Origin::Create(GURL(kOtherURL)), net::FirstPartySetMetadata(),
@@ -1457,25 +1449,15 @@ TEST_P(CookieSettingsTestP, IsCookieAccessible_SameSiteNoneCookie) {
 
 TEST_P(CookieSettingsTestP, IsCookieAccessible_SameSiteLaxCookie) {
   CookieSettings settings;
-  net::CookieInclusionStatus status;
-  settings.set_block_third_party_cookies(false);
-
-  std::unique_ptr<net::CanonicalCookie> cookie =
-      MakeCanonicalCookie("name", kURL);
-
-  EXPECT_TRUE(settings.IsCookieAccessible(
-      *cookie, GURL(kURL), net::SiteForCookies(),
-      url::Origin::Create(GURL(kOtherURL)), net::FirstPartySetMetadata(),
-      GetCookieSettingOverrides(), &status));
-  EXPECT_FALSE(status.HasWarningReason(
-      net::CookieInclusionStatus::WarningReason::WARN_THIRD_PARTY_PHASEOUT));
-
   settings.set_block_third_party_cookies(true);
   if (IsTrackingProtectionEnabledFor3pcd()) {
     settings.set_tracking_protection_enabled_for_3pcd(true);
   }
 
-  status.ResetForTesting();
+  std::unique_ptr<net::CanonicalCookie> cookie =
+      MakeCanonicalCookie("name", kURL);
+
+  net::CookieInclusionStatus status;
   EXPECT_FALSE(settings.IsCookieAccessible(
       *cookie, GURL(kURL), net::SiteForCookies(),
       url::Origin::Create(GURL(kOtherURL)), net::FirstPartySetMetadata(),
@@ -2251,8 +2233,10 @@ TEST_P(
     CookieSettingsTestP,
     AnnotateAndMoveUserBlockedCookies_SitesInFirstPartySet_FirstPartyURLBlocked) {
   CookieSettings settings;
-  net::CookieInclusionStatus status;
-  settings.set_block_third_party_cookies(false);
+  settings.set_block_third_party_cookies(true);
+  if (IsTrackingProtectionEnabledFor3pcd()) {
+    settings.set_tracking_protection_enabled_for_3pcd(true);
+  }
   settings.set_content_settings(
       ContentSettingsType::COOKIES,
       {CreateSetting(kRwsOwnerURL, kRwsOwnerURL, CONTENT_SETTING_BLOCK)});
@@ -2266,19 +2250,6 @@ TEST_P(
   net::FirstPartySetEntry frame_entry(primary, net::SiteType::kAssociated, 1u);
   net::FirstPartySetEntry top_frame_entry(primary, net::SiteType::kPrimary,
                                           std::nullopt);
-  // Without third-party-cookie-blocking enabled, the cookie is accessible, even
-  // though cookies are blocked for the top-level URL.
-  ASSERT_TRUE(settings.IsCookieAccessible(
-      *cookie, GURL(kRwsMemberURL), net::SiteForCookies(), top_frame_origin,
-      net::FirstPartySetMetadata(frame_entry, top_frame_entry),
-      GetCookieSettingOverrides(), &status));
-
-  // Now we enable third-party-cookie-blocking, and verify that the right
-  // exclusion reasons are still applied.
-  settings.set_block_third_party_cookies(true);
-  if (IsTrackingProtectionEnabledFor3pcd()) {
-    settings.set_tracking_protection_enabled_for_3pcd(true);
-  }
 
   net::CookieAccessResultList maybe_included_cookies = {{*cookie, {}}};
   net::CookieAccessResultList excluded_cookies = {};