Port StaticCookiePolicy to net::SiteForCookies

This further centralizes the first-partiness check, and increases type
safety by having the notion of 1st party be more consistently kept inside
net::SiteForCookies (this helped find 2 bugs!)

Bug: 577565
Change-Id: Ie11f8a0fc5a762318c3a0c0b818b488c4775374b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1992264
Commit-Queue: Maksim Orlovich <morlovich@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#741619}

android_webview/browser/aw_cookie_access_policy.cc

@@ -13,7 +13,8 @@
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/websocket_handshake_request_info.h"
 #include "net/base/net_errors.h"
-#include "net/base/static_cookie_policy.h"
+#include "net/cookies/site_for_cookies.h"
+#include "net/cookies/static_cookie_policy.h"
 #include "url/gurl.h"
 
 using base::AutoLock;
@@ -60,20 +61,22 @@ bool AwCookieAccessPolicy::GetShouldAcceptThirdPartyCookies(
   return io_thread_client->ShouldAcceptThirdPartyCookies();
 }
 
-bool AwCookieAccessPolicy::AllowCookies(const GURL& url,
-                                        const GURL& first_party,
-                                        int render_process_id,
-                                        int render_frame_id) {
+bool AwCookieAccessPolicy::AllowCookies(
+    const GURL& url,
+    const net::SiteForCookies& site_for_cookies,
+    int render_process_id,
+    int render_frame_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   bool third_party = GetShouldAcceptThirdPartyCookies(
       render_process_id, render_frame_id,
       content::RenderFrameHost::kNoFrameTreeNodeId);
-  return CanAccessCookies(url, first_party, third_party);
+  return CanAccessCookies(url, site_for_cookies, third_party);
 }
 
-bool AwCookieAccessPolicy::CanAccessCookies(const GURL& url,
-                                            const GURL& site_for_cookies,
-                                            bool accept_third_party_cookies) {
+bool AwCookieAccessPolicy::CanAccessCookies(
+    const GURL& url,
+    const net::SiteForCookies& site_for_cookies,
+    bool accept_third_party_cookies) {
   if (!accept_cookies_)
     return false;
 

android_webview/browser/aw_cookie_access_policy.h

@@ -12,6 +12,7 @@
 class GURL;
 
 namespace net {
+class SiteForCookies;
 class URLRequest;
 }  // namespace net
 
@@ -40,7 +41,7 @@ class AwCookieAccessPolicy {
 
   // Whether or not to allow cookies for requests with these parameters.
   bool AllowCookies(const GURL& url,
-                    const GURL& first_party,
+                    const net::SiteForCookies& site_for_cookies,
                     int render_process_id,
                     int render_frame_id);
 
@@ -52,7 +53,7 @@ class AwCookieAccessPolicy {
   ~AwCookieAccessPolicy();
 
   bool CanAccessCookies(const GURL& url,
-                        const GURL& site_for_cookies,
+                        const net::SiteForCookies& site_for_cookies,
                         bool accept_third_party_cookies);
   bool accept_cookies_;
   base::Lock lock_;

android_webview/browser/network_service/aw_proxying_restricted_cookie_manager.cc

@@ -203,7 +203,7 @@ bool AwProxyingRestrictedCookieManager::AllowCookies(
     return AwCookieAccessPolicy::GetInstance()->GetShouldAcceptCookies();
   } else {
     return AwCookieAccessPolicy::GetInstance()->AllowCookies(
-        url, site_for_cookies.RepresentativeUrl(), process_id_, frame_id_);
+        url, site_for_cookies, process_id_, frame_id_);
   }
 }
 

chrome/browser/content_settings/host_content_settings_map_unittest.cc

@@ -37,7 +37,7 @@
 #include "components/prefs/scoped_user_pref_update.h"
 #include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "content/public/test/browser_task_environment.h"
-#include "net/base/static_cookie_policy.h"
+#include "net/cookies/static_cookie_policy.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"

components/content_settings/core/browser/host_content_settings_map.cc

@@ -42,7 +42,7 @@
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_service.h"
 #include "net/base/net_errors.h"
-#include "net/base/static_cookie_policy.h"
+#include "net/cookies/static_cookie_policy.h"
 #include "url/gurl.h"
 
 using content_settings::WebsiteSettingsInfo;

components/content_settings/core/common/DEPS

@@ -5,6 +5,7 @@ include_rules = [
   "+net/base",
   "+net/cookies/cookie_util.h",
   "+net/cookies/cookie_constants.h",
+  "+net/cookies/static_cookie_policy.h",
   "+testing",
   "+url",
 ]

components/content_settings/core/common/cookie_settings_base.cc

@@ -11,16 +11,17 @@
 #include "build/build_config.h"
 #include "components/content_settings/core/common/features.h"
 #include "net/base/net_errors.h"
-#include "net/base/static_cookie_policy.h"
 #include "net/cookies/cookie_util.h"
+#include "net/cookies/static_cookie_policy.h"
 #include "url/gurl.h"
 
 namespace content_settings {
 namespace {
 bool IsThirdPartyRequest(const GURL& url, const GURL& site_for_cookies) {
   net::StaticCookiePolicy policy(
       net::StaticCookiePolicy::BLOCK_ALL_THIRD_PARTY_COOKIES);
-  return policy.CanAccessCookies(url, site_for_cookies) != net::OK;
+  return policy.CanAccessCookies(
+             url, net::SiteForCookies::FromUrl(site_for_cookies)) != net::OK;
 }
 }  // namespace
 

extensions/browser/api/declarative_webrequest/webrequest_condition_attribute.cc

@@ -28,7 +28,7 @@
 #include "extensions/common/error_utils.h"
 #include "net/base/net_errors.h"
 #include "net/base/registry_controlled_domains/registry_controlled_domain.h"
-#include "net/base/static_cookie_policy.h"
+#include "net/cookies/static_cookie_policy.h"
 #include "net/http/http_request_headers.h"
 #include "net/http/http_util.h"
 

extensions/browser/api/declarative_webrequest/webrequest_condition_attribute_unittest.cc

@@ -172,7 +172,6 @@ TEST(WebRequestConditionAttributeTest, ThirdParty) {
   EXPECT_EQ(std::string(keys::kThirdPartyKey),
             first_party_attribute->GetName());
 
-  const GURL url_empty;
   const GURL url_a("http://a.com");
   const GURL url_b("http://b.com");
 
@@ -182,7 +181,7 @@ TEST(WebRequestConditionAttributeTest, ThirdParty) {
     const RequestStage stage = static_cast<RequestStage>(i);
     WebRequestInfoInitParams empty_params;
     empty_params.url = url_a;
-    empty_params.site_for_cookies = url_empty;
+    empty_params.site_for_cookies = net::SiteForCookies();
     WebRequestInfo request_info1(std::move(empty_params));
     EXPECT_TRUE(third_party_attribute->IsFulfilled(
         WebRequestData(&request_info1, stage)));
@@ -191,7 +190,7 @@ TEST(WebRequestConditionAttributeTest, ThirdParty) {
 
     WebRequestInfoInitParams b_params;
     b_params.url = url_a;
-    b_params.site_for_cookies = url_b;
+    b_params.site_for_cookies = net::SiteForCookies::FromUrl(url_b);
     WebRequestInfo request_info2(std::move(b_params));
     EXPECT_TRUE(third_party_attribute->IsFulfilled(
         WebRequestData(&request_info2, stage)));
@@ -200,7 +199,7 @@ TEST(WebRequestConditionAttributeTest, ThirdParty) {
 
     WebRequestInfoInitParams a_params;
     a_params.url = url_a;
-    a_params.site_for_cookies = url_a;
+    a_params.site_for_cookies = net::SiteForCookies::FromUrl(url_a);
     WebRequestInfo request_info3(std::move(a_params));
     EXPECT_FALSE(third_party_attribute->IsFulfilled(
         WebRequestData(&request_info3, stage)));

extensions/browser/api/declarative_webrequest/webrequest_condition_unittest.cc

@@ -172,7 +172,8 @@ TEST(WebRequestConditionTest, NoUrlAttributes) {
 
   WebRequestInfoInitParams params;
   params.url = GURL("https://www.example.com");
-  params.site_for_cookies = GURL("https://www.example.com");
+  params.site_for_cookies =
+      net::SiteForCookies::FromUrl(GURL("https://www.example.com"));
   WebRequestInfo https_request_info(std::move(params));
 
   // 1. A non-empty condition without UrlFilter attributes is fulfilled iff its

extensions/browser/api/web_request/web_request_info.cc

@@ -166,7 +166,7 @@ WebRequestInfoInitParams::WebRequestInfoInitParams(
     base::Optional<int64_t> navigation_id)
     : id(request_id),
       url(request.url),
-      site_for_cookies(request.site_for_cookies.RepresentativeUrl()),
+      site_for_cookies(request.site_for_cookies),
       render_process_id(render_process_id),
       routing_id(routing_id),
       frame_id(render_frame_id),

extensions/browser/api/web_request/web_request_info.h

@@ -55,7 +55,7 @@ struct WebRequestInfoInitParams {
 
   uint64_t id = 0;
   GURL url;
-  GURL site_for_cookies;
+  net::SiteForCookies site_for_cookies;
   int render_process_id = -1;
   int routing_id = MSG_ROUTING_NONE;
   int frame_id = -1;
@@ -99,7 +99,7 @@ struct WebRequestInfo {
 
   // The URL of the request.
   const GURL url;
-  const GURL site_for_cookies;
+  const net::SiteForCookies site_for_cookies;
 
   // The ID of the render process which initiated the request, or -1 of not
   // applicable (i.e. if initiated by the browser).

net/BUILD.gn

@@ -485,8 +485,6 @@ component("net") {
       "base/scheme_host_port_matcher_result.h",
       "base/scheme_host_port_matcher_rule.cc",
       "base/scheme_host_port_matcher_rule.h",
-      "base/static_cookie_policy.cc",
-      "base/static_cookie_policy.h",
       "base/test_data_stream.cc",
       "base/test_data_stream.h",
       "base/upload_bytes_element_reader.cc",
@@ -570,6 +568,8 @@ component("net") {
       "cookies/parsed_cookie.h",
       "cookies/site_for_cookies.cc",
       "cookies/site_for_cookies.h",
+      "cookies/static_cookie_policy.cc",
+      "cookies/static_cookie_policy.h",
       "disk_cache/backend_cleanup_tracker.cc",
       "disk_cache/backend_cleanup_tracker.h",
       "disk_cache/blockfile/addr.cc",
@@ -4141,7 +4141,6 @@ test("net_unittests") {
     "base/registry_controlled_domains/registry_controlled_domain_unittest.cc",
     "base/scheme_host_port_matcher_rule_unittest.cc",
     "base/scheme_host_port_matcher_unittest.cc",
-    "base/static_cookie_policy_unittest.cc",
     "base/test_completion_callback_unittest.cc",
     "base/test_proxy_delegate.cc",
     "base/test_proxy_delegate.h",
@@ -4209,6 +4208,7 @@ test("net_unittests") {
     "cookies/cookie_util_unittest.cc",
     "cookies/parsed_cookie_unittest.cc",
     "cookies/site_for_cookies_unittest.cc",
+    "cookies/static_cookie_policy_unittest.cc",
     "der/encode_values_unittest.cc",
     "der/input_unittest.cc",
     "der/parse_values_unittest.cc",

net/base/static_cookie_policy.cc → net/cookies/static_cookie_policy.cc

@@ -2,26 +2,23 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "net/base/static_cookie_policy.h"
+#include "net/cookies/static_cookie_policy.h"
 
 #include "base/logging.h"
 #include "net/base/net_errors.h"
-#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
+#include "net/cookies/site_for_cookies.h"
 #include "url/gurl.h"
 
 namespace net {
 
-int StaticCookiePolicy::CanAccessCookies(const GURL& url,
-                                         const GURL& site_for_cookies) const {
+int StaticCookiePolicy::CanAccessCookies(
+    const GURL& url,
+    const net::SiteForCookies& site_for_cookies) const {
   switch (type_) {
     case StaticCookiePolicy::ALLOW_ALL_COOKIES:
       return OK;
     case StaticCookiePolicy::BLOCK_ALL_THIRD_PARTY_COOKIES:
-      return registry_controlled_domains::SameDomainOrHost(
-                 url, site_for_cookies,
-                 registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES)
-                 ? OK
-                 : ERR_ACCESS_DENIED;
+      return site_for_cookies.IsFirstParty(url) ? OK : ERR_ACCESS_DENIED;
     case StaticCookiePolicy::BLOCK_ALL_COOKIES:
       return ERR_ACCESS_DENIED;
     default:

net/base/static_cookie_policy.h → net/cookies/static_cookie_policy.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef NET_BASE_STATIC_COOKIE_POLICY_H_
-#define NET_BASE_STATIC_COOKIE_POLICY_H_
+#ifndef NET_COOKIES_STATIC_COOKIE_POLICY_H_
+#define NET_COOKIES_STATIC_COOKIE_POLICY_H_
 
 #include "base/macros.h"
 #include "net/base/net_export.h"
@@ -12,6 +12,8 @@ class GURL;
 
 namespace net {
 
+class SiteForCookies;
+
 // The StaticCookiePolicy class implements a static cookie policy that supports
 // three modes: allow all, deny all, or block third-party cookies.
 class NET_EXPORT StaticCookiePolicy {
@@ -27,13 +29,9 @@ class NET_EXPORT StaticCookiePolicy {
     BLOCK_ALL_THIRD_PARTY_COOKIES
   };
 
-  StaticCookiePolicy()
-      : type_(StaticCookiePolicy::ALLOW_ALL_COOKIES) {
-  }
+  StaticCookiePolicy() : type_(StaticCookiePolicy::ALLOW_ALL_COOKIES) {}
 
-  explicit StaticCookiePolicy(Type type)
-      : type_(type) {
-  }
+  explicit StaticCookiePolicy(Type type) : type_(type) {}
 
   // Sets the current policy to enforce. This should be called when the user's
   // preferences change.
@@ -42,7 +40,8 @@ class NET_EXPORT StaticCookiePolicy {
 
   // Consults the user's third-party cookie blocking preferences to determine
   // whether the URL's cookies can be accessed (i.e., can be get or set).
-  int CanAccessCookies(const GURL& url, const GURL& site_for_cookies) const;
+  int CanAccessCookies(const GURL& url,
+                       const net::SiteForCookies& site_for_cookies) const;
 
  private:
   Type type_;
@@ -52,4 +51,4 @@ class NET_EXPORT StaticCookiePolicy {
 
 }  // namespace net
 
-#endif  // NET_BASE_STATIC_COOKIE_POLICY_H_
+#endif  // NET_COOKIES_STATIC_COOKIE_POLICY_H_

net/base/static_cookie_policy_unittest.cc → net/cookies/static_cookie_policy_unittest.cc

@@ -2,8 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "net/cookies/static_cookie_policy.h"
 #include "net/base/net_errors.h"
-#include "net/base/static_cookie_policy.h"
+#include "net/cookies/site_for_cookies.h"
 #include "net/test/gtest_util.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -19,14 +20,13 @@ class StaticCookiePolicyTest : public testing::Test {
       : url_google_("http://www.google.izzle"),
         url_google_secure_("https://www.google.izzle"),
         url_google_mail_("http://mail.google.izzle"),
-        url_google_analytics_("http://www.googleanalytics.izzle") {
-  }
-  void SetPolicyType(StaticCookiePolicy::Type type) {
-    policy_.set_type(type);
-  }
+        url_google_analytics_("http://www.googleanalytics.izzle") {}
+  void SetPolicyType(StaticCookiePolicy::Type type) { policy_.set_type(type); }
   int CanAccessCookies(const GURL& url, const GURL& first_party) {
-    return policy_.CanAccessCookies(url, first_party);
+    return policy_.CanAccessCookies(url,
+                                    net::SiteForCookies::FromUrl(first_party));
   }
+
  protected:
   StaticCookiePolicy policy_;
   GURL url_google_;

services/network/cookie_settings.cc

@@ -10,8 +10,8 @@
 #include "base/strings/string_split.h"
 #include "components/content_settings/core/common/content_settings_utils.h"
 #include "net/base/net_errors.h"
-#include "net/base/static_cookie_policy.h"
 #include "net/cookies/cookie_util.h"
+#include "net/cookies/static_cookie_policy.h"
 #include "services/network/public/cpp/features.h"
 
 namespace network {

services/network/network_service_network_delegate.cc

@@ -215,8 +215,7 @@ bool NetworkServiceNetworkDelegate::OnCanGetCookies(
 
   URLLoader* url_loader = URLLoader::ForRequest(request);
   if (url_loader)
-    return url_loader->AllowCookies(
-        request.url(), request.site_for_cookies().RepresentativeUrl());
+    return url_loader->AllowCookies(request.url(), request.site_for_cookies());
 #if !defined(OS_IOS)
   WebSocket* web_socket = WebSocket::ForRequest(request);
   if (web_socket)
@@ -241,8 +240,7 @@ bool NetworkServiceNetworkDelegate::OnCanSetCookie(
     return false;
   URLLoader* url_loader = URLLoader::ForRequest(request);
   if (url_loader)
-    return url_loader->AllowCookies(
-        request.url(), request.site_for_cookies().RepresentativeUrl());
+    return url_loader->AllowCookies(request.url(), request.site_for_cookies());
 #if !defined(OS_IOS)
   WebSocket* web_socket = WebSocket::ForRequest(request);
   if (web_socket)