Give ResourceRequest::site_for_cookies proper type.

(With net::RedirectInfo matching it as well). It previously being a URL was highly misleading since: 1) There was a special meaning to empty URLs 2) The path bits weren't really guaranteed to exist. ... And also having a dedicated type will simplify computations by centralizing the use... in following CLs. Bug: 577565 Change-Id: I4917cd2a8044caa9d526b08c3750c2c127aca84c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1928206 Commit-Queue: Maksim Orlovich <morlovich@chromium.org> Reviewed-by: John Abd-El-Malek <jam@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Reviewed-by: Lily Chen <chlily@chromium.org> Cr-Commit-Position: refs/heads/master@{#724982}
Denger-Network · Dec 15, 2019 · 5d02d9e · 5d02d9e
1 parent f815237
commit 5d02d9e
Show file tree

Hide file tree

Showing 75 changed files with 513 additions and 201 deletions.
diff --git a/chrome/browser/android/customtabs/detached_resource_request.cc b/chrome/browser/android/customtabs/detached_resource_request.cc
@@ -82,7 +82,8 @@ DetachedResourceRequest::DetachedResourceRequest(
   resource_request->referrer = net::URLRequestJob::ComputeReferrerForPolicy(
       referrer_policy, site_for_cookies_, url_);
   resource_request->referrer_policy = referrer_policy;
-  resource_request->site_for_cookies = site_for_cookies_;
+  resource_request->site_for_cookies =
+      net::SiteForCookies::FromUrl(site_for_cookies_);
 
   url::Origin site_for_cookies_origin = url::Origin::Create(site_for_cookies_);
   resource_request->request_initiator = site_for_cookies_origin;

diff --git a/chrome/browser/devtools/devtools_ui_bindings.cc b/chrome/browser/devtools/devtools_ui_bindings.cc
@@ -839,7 +839,7 @@ void DevToolsUIBindings::LoadNetworkResource(const DispatchCallback& callback,
   resource_request.url = gurl;
   // TODO(caseq): this preserves behavior of URLFetcher-based implementation.
   // We really need to pass proper first party origin from the front-end.
-  resource_request.site_for_cookies = gurl;
+  resource_request.site_for_cookies = net::SiteForCookies::FromUrl(gurl);
   resource_request.headers.AddHeadersFromString(headers);
 
   NetworkResourceLoader::URLLoaderFactoryHolder url_loader_factory;

diff --git a/chrome/browser/extensions/extension_protocols_unittest.cc b/chrome/browser/extensions/extension_protocols_unittest.cc
@@ -126,7 +126,8 @@ network::ResourceRequest CreateResourceRequest(const std::string& method,
   network::ResourceRequest request;
   request.method = method;
   request.url = url;
-  request.site_for_cookies = url;  // bypass third-party cookie blocking.
+  request.site_for_cookies =
+      net::SiteForCookies::FromUrl(url);  // bypass third-party cookie blocking.
   request.request_initiator =
       url::Origin::Create(url);  // ensure initiator set.
   request.referrer_policy = content::Referrer::GetDefaultReferrerPolicy();

diff --git a/chrome/browser/net/network_context_configuration_browsertest.cc b/chrome/browser/net/network_context_configuration_browsertest.cc
@@ -547,12 +547,13 @@ class NetworkContextConfigurationBrowserTest
     if (cookie_type == CookieType::kThirdParty)
       cookie_line += ";SameSite=None;Secure";
     request->url = server->GetURL("/set-cookie?" + cookie_line);
+    url::Origin origin;
     if (cookie_type == CookieType::kThirdParty)
-      request->site_for_cookies = GURL("http://example.com");
+      origin = url::Origin::Create(GURL("http://example.com"));
     else
-      request->site_for_cookies = server->base_url();
+      origin = url::Origin::Create(server->base_url());
+    request->site_for_cookies = net::SiteForCookies::FromOrigin(origin);
 
-    url::Origin origin = url::Origin::Create(request->site_for_cookies);
     request->trusted_params = network::ResourceRequest::TrustedParams();
     request->trusted_params->network_isolation_key =
         net::NetworkIsolationKey(origin, origin);
@@ -786,8 +787,8 @@ IN_PROC_BROWSER_TEST_P(NetworkContextConfigurationBrowserTest,
   std::unique_ptr<network::ResourceRequest> request =
       std::make_unique<network::ResourceRequest>();
   request->url = https_server.GetURL("/echoheader?Cookie");
-  request->site_for_cookies = GURL(chrome::kChromeUIPrintURL);
-  url::Origin origin = url::Origin::Create(request->site_for_cookies);
+  url::Origin origin = url::Origin::Create(GURL(chrome::kChromeUIPrintURL));
+  request->site_for_cookies = net::SiteForCookies::FromOrigin(origin);
   request->trusted_params = network::ResourceRequest::TrustedParams();
   request->trusted_params->network_isolation_key =
       net::NetworkIsolationKey(origin, origin);

diff --git a/chrome/browser/offline_pages/offline_page_url_loader.cc b/chrome/browser/offline_pages/offline_page_url_loader.cc
@@ -42,7 +42,8 @@ net::RedirectInfo CreateRedirectInfo(const GURL& redirected_url,
   redirect_info.new_referrer_policy = net::URLRequest::NO_REFERRER;
   redirect_info.new_method = "GET";
   redirect_info.status_code = response_code;
-  redirect_info.new_site_for_cookies = redirect_info.new_url;
+  redirect_info.new_site_for_cookies =
+      net::SiteForCookies::FromUrl(redirect_info.new_url);
   return redirect_info;
 }
 

diff --git a/chrome/renderer/net/net_error_helper.cc b/chrome/renderer/net/net_error_helper.cc
@@ -282,7 +282,8 @@ std::unique_ptr<network::ResourceRequest> NetErrorHelper::CreatePostRequest(
       static_cast<int>(content::ResourceType::kSubResource);
 
   blink::WebLocalFrame* frame = render_frame()->GetWebFrame();
-  resource_request->site_for_cookies = frame->GetDocument().SiteForCookies();
+  resource_request->site_for_cookies =
+      net::SiteForCookies::FromUrl(frame->GetDocument().SiteForCookies());
   // The security origin of the error page should exist and be opaque.
   DCHECK(!frame->GetDocument().GetSecurityOrigin().IsNull());
   DCHECK(frame->GetDocument().GetSecurityOrigin().IsOpaque());

diff --git a/chromecast/browser/cast_extension_url_loader_factory.cc b/chromecast/browser/cast_extension_url_loader_factory.cc
@@ -221,7 +221,7 @@ void CastExtensionURLLoaderFactory::CreateLoaderAndStart(
 
   network::ResourceRequest new_request(request);
   new_request.url = GURL(cast_url);
-  new_request.site_for_cookies = new_request.url;
+  new_request.site_for_cookies = net::SiteForCookies::FromUrl(new_request.url);
 
   // Force a redirect to the new URL but without changing where the webpage
   // thinks it is.

diff --git a/components/download/internal/common/download_utils.cc b/components/download/internal/common/download_utils.cc
@@ -268,7 +268,7 @@ std::unique_ptr<network::ResourceRequest> CreateResourceRequest(
         params->network_isolation_key();
   }
   request->do_not_prompt_for_login = params->do_not_prompt_for_login();
-  request->site_for_cookies = params->url();
+  request->site_for_cookies = net::SiteForCookies::FromUrl(params->url());
   request->referrer = params->referrer();
   request->referrer_policy = params->referrer_policy();
   request->is_main_frame = true;

diff --git a/components/feed/core/feed_networking_host.cc b/components/feed/core/feed_networking_host.cc
@@ -229,7 +229,7 @@ std::unique_ptr<network::SimpleURLLoader> NetworkFetch::MakeLoader() {
   if (host_overridden_) {
     resource_request->credentials_mode =
         network::mojom::CredentialsMode::kInclude;
-    resource_request->site_for_cookies = url;
+    resource_request->site_for_cookies = net::SiteForCookies::FromUrl(url);
   }
 
   SetRequestHeaders(resource_request.get());

diff --git a/components/omnibox/browser/remote_suggestions_service.cc b/components/omnibox/browser/remote_suggestions_service.cc
@@ -120,7 +120,7 @@ void RemoteSuggestionsService::CreateSuggestionsRequest(
   request->load_flags = net::LOAD_DO_NOT_SAVE_COOKIES;
   // Try to attach cookies for signed in user.
   request->attach_same_site_cookies = true;
-  request->site_for_cookies = suggest_url;
+  request->site_for_cookies = net::SiteForCookies::FromUrl(suggest_url);
   AddVariationHeaders(request.get());
   StartDownloadAndTransferLoader(std::move(request), std::string(),
                                  traffic_annotation, std::move(start_callback),

diff --git a/components/omnibox/browser/remote_suggestions_service_unittest.cc b/components/omnibox/browser/remote_suggestions_service_unittest.cc
@@ -66,7 +66,8 @@ TEST_F(RemoteSuggestionsServiceTest, EnsureAttachCookies) {
   RunAndWait();
   EXPECT_TRUE(resource_request.attach_same_site_cookies);
   EXPECT_EQ(net::LOAD_DO_NOT_SAVE_COOKIES, resource_request.load_flags);
-  EXPECT_EQ(resource_request.url, resource_request.site_for_cookies);
+  EXPECT_TRUE(resource_request.site_for_cookies.IsEquivalent(
+      net::SiteForCookies::FromUrl(resource_request.url)));
   const std::string kServiceUri = "https://www.google.com/complete/search";
   EXPECT_EQ(kServiceUri,
             resource_request.url.spec().substr(0, kServiceUri.size()));

diff --git a/content/browser/appcache/appcache_host.cc b/content/browser/appcache/appcache_host.cc
@@ -204,7 +204,8 @@ void AppCacheHost::SelectCache(const GURL& document_url,
     // continue whether it was set or not.
 
     AppCachePolicy* policy = service()->appcache_policy();
-    if (policy && !policy->CanCreateAppCache(manifest_url, first_party_url_)) {
+    if (policy && !policy->CanCreateAppCache(
+                      manifest_url, site_for_cookies_.RepresentativeUrl())) {
       FinishCacheSelection(nullptr, nullptr, mojo::ReportBadMessageCallback());
       frontend()->EventRaised(
           blink::mojom::AppCacheEventID::APPCACHE_CHECKING_EVENT);
@@ -389,8 +390,8 @@ std::unique_ptr<AppCacheRequestHandler> AppCacheHost::CreateRequestHandler(
   if (AppCacheRequestHandler::IsMainResourceType(resource_type)) {
     // Store the first party origin so that it can be used later in SelectCache
     // for checking whether the creation of the appcache is allowed.
-    first_party_url_ = request->GetSiteForCookies();
-    first_party_url_initialized_ = true;
+    site_for_cookies_ = request->GetSiteForCookies();
+    site_for_cookies_initialized_ = true;
     return base::WrapUnique(new AppCacheRequestHandler(
         this, resource_type, should_reset_appcache, std::move(request)));
   }

diff --git a/content/browser/appcache/appcache_host.h b/content/browser/appcache/appcache_host.h
@@ -213,10 +213,13 @@ class CONTENT_EXPORT AppCacheHost : public blink::mojom::AppCacheHost,
            !pending_selected_manifest_url_.is_empty();
   }
 
-  const GURL& first_party_url() const { return first_party_url_; }
-  void SetFirstPartyUrlForTesting(const GURL& url) {
-    first_party_url_ = url;
-    first_party_url_initialized_ = true;
+  const net::SiteForCookies& site_for_cookies() const {
+    return site_for_cookies_;
+  }
+  void SetSiteForCookiesForTesting(
+      const net::SiteForCookies& site_for_cookies) {
+    site_for_cookies_ = site_for_cookies;
+    site_for_cookies_initialized_ = true;
   }
 
   // Returns a weak pointer reference to the host.
@@ -379,9 +382,9 @@ class CONTENT_EXPORT AppCacheHost : public blink::mojom::AppCacheHost,
   // Used to inform the QuotaManager of what origins are currently in use.
   url::Origin origin_in_use_;
 
-  // First party url to be used in policy checks.
-  GURL first_party_url_;
-  bool first_party_url_initialized_ = false;
+  // To be used in policy checks.
+  net::SiteForCookies site_for_cookies_;
+  bool site_for_cookies_initialized_ = false;
 
   FRIEND_TEST_ALL_PREFIXES(content::AppCacheGroupTest, CleanupUnusedGroup);
   FRIEND_TEST_ALL_PREFIXES(content::AppCacheGroupTest, QueueUpdate);

diff --git a/content/browser/appcache/appcache_host_unittest.cc b/content/browser/appcache/appcache_host_unittest.cc
@@ -511,7 +511,8 @@ TEST_F(AppCacheHostTest, SelectCacheAllowed) {
     AppCacheHost host(kHostIdForTest, kProcessIdForTest, kRenderFrameIdForTest,
                       mojo::NullRemote(), &service_);
     host.set_frontend_for_testing(&mock_frontend_);
-    host.SetFirstPartyUrlForTesting(kDocAndOriginUrl);
+    host.SetSiteForCookiesForTesting(
+        net::SiteForCookies::FromUrl(kDocAndOriginUrl));
     host.SelectCache(kDocAndOriginUrl, blink::mojom::kAppCacheNoCacheId,
                      kManifestUrl);
     EXPECT_EQ(1, mock_quota_proxy->GetInUseCount(kOrigin));
@@ -560,7 +561,8 @@ TEST_F(AppCacheHostTest, SelectCacheBlocked) {
     AppCacheHost host(kHostIdForTest, kProcessIdForTest, kRenderFrameIdForTest,
                       mojo::NullRemote(), &service_);
     host.set_frontend_for_testing(&mock_frontend_);
-    host.SetFirstPartyUrlForTesting(kDocAndOriginUrl);
+    host.SetSiteForCookiesForTesting(
+        net::SiteForCookies::FromUrl(kDocAndOriginUrl));
     host.SelectCache(kDocAndOriginUrl, blink::mojom::kAppCacheNoCacheId,
                      kManifestUrl);
     EXPECT_EQ(1, mock_quota_proxy->GetInUseCount(kOrigin));

diff --git a/content/browser/appcache/appcache_request.h b/content/browser/appcache/appcache_request.h
@@ -40,7 +40,9 @@ class CONTENT_EXPORT AppCacheRequest {
   const std::string& GetMethod() const { return request_.method; }
 
   // Used for cookie policy.
-  const GURL& GetSiteForCookies() const { return request_.site_for_cookies; }
+  net::SiteForCookies GetSiteForCookies() const {
+    return request_.site_for_cookies;
+  }
 
   // The referrer for this request.
   const GURL GetReferrer() const { return request_.referrer; }

diff --git a/content/browser/appcache/appcache_request_handler.cc b/content/browser/appcache/appcache_request_handler.cc
@@ -358,8 +358,10 @@ void AppCacheRequestHandler::OnMainResponseFound(
     return;
 
   AppCachePolicy* policy = host_->service()->appcache_policy();
-  bool was_blocked_by_policy = !manifest_url.is_empty() && policy &&
-      !policy->CanLoadAppCache(manifest_url, host_->first_party_url());
+  bool was_blocked_by_policy =
+      !manifest_url.is_empty() && policy &&
+      !policy->CanLoadAppCache(manifest_url,
+                               host_->site_for_cookies().RepresentativeUrl());
 
   if (was_blocked_by_policy) {
     if (IsResourceTypeFrame(resource_type_)) {

diff --git a/content/browser/appcache/appcache_storage_impl_unittest.cc b/content/browser/appcache/appcache_storage_impl_unittest.cc
@@ -1655,7 +1655,8 @@ class AppCacheStorageImplTest : public testing::Test {
                              kMockProcessId, GetBadMessageCallback());
       AppCacheHost* host1 = service_->GetHost(host1_id_);
       const GURL kEmptyPageUrl(GetMockUrl("empty.html"));
-      host1->SetFirstPartyUrlForTesting(kEmptyPageUrl);
+      host1->SetSiteForCookiesForTesting(
+          net::SiteForCookies::FromUrl(kEmptyPageUrl));
       host1->SelectCache(kEmptyPageUrl, blink::mojom::kAppCacheNoCacheId,
                          GetMockUrl("manifest"));
     } else {

diff --git a/content/browser/appcache/appcache_update_job_unittest.cc b/content/browser/appcache/appcache_update_job_unittest.cc
@@ -2426,7 +2426,8 @@ class AppCacheUpdateJobTest : public testing::Test,
 
     MockFrontend* frontend = MakeMockFrontend();
     AppCacheHost* host = MakeHost(frontend);
-    host->SetFirstPartyUrlForTesting(kManifestUrl);
+    host->SetSiteForCookiesForTesting(
+        net::SiteForCookies::FromUrl(kManifestUrl));
     host->SelectCache(MockHttpServer::GetMockUrl("files/empty1"),
                       blink::mojom::kAppCacheNoCacheId, kManifestUrl);
 

diff --git a/content/browser/appcache/appcache_update_url_loader_request.cc b/content/browser/appcache/appcache_update_url_loader_request.cc
@@ -90,7 +90,7 @@ std::string AppCacheUpdateJob::UpdateURLLoaderRequest::GetMimeType() const {
 
 void AppCacheUpdateJob::UpdateURLLoaderRequest::SetSiteForCookies(
     const GURL& site_for_cookies) {
-  request_.site_for_cookies = site_for_cookies;
+  request_.site_for_cookies = net::SiteForCookies::FromUrl(site_for_cookies);
 }
 
 void AppCacheUpdateJob::UpdateURLLoaderRequest::SetInitiator(

diff --git a/content/browser/devtools/devtools_url_loader_interceptor.cc b/content/browser/devtools/devtools_url_loader_interceptor.cc
@@ -1084,13 +1084,12 @@ void InterceptionJob::ProcessSetCookies(const net::HttpResponseHeaders& headers,
       GetContentClient()
           ->browser()
           ->ShouldIgnoreSameSiteCookieRestrictionsWhenTopLevel(
-              create_loader_params_->request.site_for_cookies.scheme_piece(),
+              create_loader_params_->request.site_for_cookies.scheme(),
               create_loader_params_->request.url.SchemeIsCryptographic());
   options.set_same_site_cookie_context(
       net::cookie_util::ComputeSameSiteContextForResponse(
           create_loader_params_->request.url,
-          net::SiteForCookies::FromUrl(
-              create_loader_params_->request.site_for_cookies),
+          create_loader_params_->request.site_for_cookies,
           create_loader_params_->request.request_initiator,
           (create_loader_params_->request.attach_same_site_cookies ||
            should_treat_as_first_party)));
@@ -1233,12 +1232,11 @@ void InterceptionJob::FetchCookies(
       GetContentClient()
           ->browser()
           ->ShouldIgnoreSameSiteCookieRestrictionsWhenTopLevel(
-              request.site_for_cookies.scheme_piece(),
+              request.site_for_cookies.scheme(),
               request.url.SchemeIsCryptographic());
   options.set_same_site_cookie_context(
       net::cookie_util::ComputeSameSiteContextForRequest(
-          request.method, request.url,
-          net::SiteForCookies::FromUrl(request.site_for_cookies),
+          request.method, request.url, request.site_for_cookies,
           request.request_initiator,
           (request.attach_same_site_cookies || should_treat_as_first_party)));