Plumb the top frame origin of resource requests to the net stack

In order to experiment with splitting the disk cache by top frame origin, we'll need to plumb the top frame origin up to the network stack for each resource request. We're focused on just document subresource requests and navigation requests for now because that's the majority of request types and this is meant to support an early experiment. We will need to include other types of requests (e.g., from workers, or signed exchange redirects) later. Subresources requests (and subframe navigations) don't change the top frame origin on redirect. Top frame navigations do however. Bug: 898855 Change-Id: I86a1c5a61986d15c445991c4e50482209f051cca Reviewed-on: https://chromium-review.googlesource.com/c/1335869 Reviewed-by: Andrey Kosyakov <caseq@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Reviewed-by: Camille Lamy <clamy@chromium.org> Reviewed-by: Matt Menke <mmenke@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Commit-Queue: Josh Karlin <jkarlin@chromium.org> Cr-Commit-Position: refs/heads/master@{#614520}
marcosholgado · Dec 7, 2018 · be37f91 · be37f91
1 parent ad6a381
commit be37f91
Show file tree

Hide file tree

Showing 42 changed files with 492 additions and 73 deletions.
diff --git a/content/browser/devtools/devtools_url_loader_interceptor.cc b/content/browser/devtools/devtools_url_loader_interceptor.cc
@@ -1071,10 +1071,10 @@ void InterceptionJob::ProcessRedirectByClient(const GURL& redirect_url) {
   response_metadata_->redirect_info = std::make_unique<net::RedirectInfo>(
       net::RedirectInfo::ComputeRedirectInfo(
           request.method, request.url, request.site_for_cookies,
-          first_party_url_policy, request.referrer_policy,
-          request.referrer.spec(), &headers, headers.response_code(),
-          redirect_url, false /* insecure_scheme_was_upgraded */,
-          true /* copy_fragment */));
+          request.top_frame_origin, first_party_url_policy,
+          request.referrer_policy, request.referrer.spec(), &headers,
+          headers.response_code(), redirect_url,
+          false /* insecure_scheme_was_upgraded */, true /* copy_fragment */));
 
   client_->OnReceiveRedirect(*response_metadata_->redirect_info,
                              response_metadata_->head);

diff --git a/content/browser/frame_host/navigation_request.cc b/content/browser/frame_host/navigation_request.cc
@@ -1489,12 +1489,21 @@ void NavigationRequest::OnStartChecksComplete(
       frame_tree_node_, begin_params_.get(), &report_raw_headers);
   devtools_instrumentation::OnNavigationRequestWillBeSent(*this);
 
+  // If this is a top-frame navigation, then use the origin of the url (and
+  // update it as redirects happen). If this is a sub-frame navigation, get the
+  // URL from the top frame.
+  GURL top_frame_url =
+      frame_tree_node_->IsMainFrame()
+          ? common_params_.url
+          : frame_tree_node_->frame_tree()->root()->current_url();
+  url::Origin top_frame_origin = url::Origin::Create(top_frame_url);
+
   loader_ = NavigationURLLoader::Create(
       browser_context->GetResourceContext(), partition,
       std::make_unique<NavigationRequestInfo>(
           common_params_, begin_params_.Clone(), site_for_cookies,
-          frame_tree_node_->IsMainFrame(), parent_is_main_frame,
-          IsSecureFrame(frame_tree_node_->parent()),
+          top_frame_origin, frame_tree_node_->IsMainFrame(),
+          parent_is_main_frame, IsSecureFrame(frame_tree_node_->parent()),
           frame_tree_node_->frame_tree_node_id(), is_for_guests_only,
           report_raw_headers,
           navigating_frame_host->GetVisibilityState() ==

diff --git a/content/browser/frame_host/navigation_request_info.cc b/content/browser/frame_host/navigation_request_info.cc
@@ -11,6 +11,7 @@ NavigationRequestInfo::NavigationRequestInfo(
     const CommonNavigationParams& common_params,
     mojom::BeginNavigationParamsPtr begin_params,
     const GURL& site_for_cookies,
+    const base::Optional<url::Origin>& top_frame_origin,
     bool is_main_frame,
     bool parent_is_main_frame,
     bool are_ancestors_secure,
@@ -26,6 +27,7 @@ NavigationRequestInfo::NavigationRequestInfo(
     : common_params(common_params),
       begin_params(std::move(begin_params)),
       site_for_cookies(site_for_cookies),
+      top_frame_origin(top_frame_origin),
       is_main_frame(is_main_frame),
       parent_is_main_frame(parent_is_main_frame),
       are_ancestors_secure(are_ancestors_secure),
@@ -42,6 +44,7 @@ NavigationRequestInfo::NavigationRequestInfo(const NavigationRequestInfo& other)
     : common_params(other.common_params),
       begin_params(other.begin_params.Clone()),
       site_for_cookies(other.site_for_cookies),
+      top_frame_origin(other.top_frame_origin),
       is_main_frame(other.is_main_frame),
       parent_is_main_frame(other.parent_is_main_frame),
       are_ancestors_secure(other.are_ancestors_secure),

diff --git a/content/browser/frame_host/navigation_request_info.h b/content/browser/frame_host/navigation_request_info.h
@@ -26,6 +26,7 @@ struct CONTENT_EXPORT NavigationRequestInfo {
   NavigationRequestInfo(const CommonNavigationParams& common_params,
                         mojom::BeginNavigationParamsPtr begin_params,
                         const GURL& site_for_cookies,
+                        const base::Optional<url::Origin>& top_frame_origin,
                         bool is_main_frame,
                         bool parent_is_main_frame,
                         bool are_ancestors_secure,
@@ -48,6 +49,12 @@ struct CONTENT_EXPORT NavigationRequestInfo {
   // checked by the third-party cookie blocking policy.
   const GURL site_for_cookies;
 
+  // The origin of the navigation if top frame, else the origin of the top
+  // frame.
+  // TODO(crbug.com/910716) Make this required. I believe we just need to add
+  // support for signed exchange redirects.
+  const base::Optional<url::Origin> top_frame_origin;
+
   const bool is_main_frame;
   const bool parent_is_main_frame;
 

diff --git a/content/browser/frame_host/navigator_impl_unittest.cc b/content/browser/frame_host/navigator_impl_unittest.cc
@@ -312,6 +312,8 @@ TEST_F(NavigatorTestWithBrowserSideNavigation, BeginNavigation) {
   EXPECT_EQ(kUrl2, subframe_loader->request_info()->common_params.url);
   // First party for cookies url should be that of the main frame.
   EXPECT_EQ(kUrl1, subframe_loader->request_info()->site_for_cookies);
+  EXPECT_EQ(url::Origin::Create(kUrl1),
+            subframe_loader->request_info()->top_frame_origin);
   EXPECT_FALSE(subframe_loader->request_info()->is_main_frame);
   EXPECT_TRUE(subframe_loader->request_info()->parent_is_main_frame);
   EXPECT_TRUE(subframe_request->browser_initiated());

diff --git a/content/browser/loader/navigation_url_loader_impl.cc b/content/browser/loader/navigation_url_loader_impl.cc
@@ -209,6 +209,7 @@ std::unique_ptr<network::ResourceRequest> CreateResourceRequest(
   new_request->method = request_info->common_params.method;
   new_request->url = request_info->common_params.url;
   new_request->site_for_cookies = request_info->site_for_cookies;
+  new_request->top_frame_origin = request_info->top_frame_origin;
 
   net::RequestPriority net_priority = net::HIGHEST;
   if (!request_info->is_main_frame &&
@@ -299,6 +300,7 @@ std::unique_ptr<NavigationRequestInfo> CreateNavigationRequestInfoForRedirect(
   return std::make_unique<NavigationRequestInfo>(
       std::move(new_common_params), std::move(new_begin_params),
       updated_resource_request.site_for_cookies,
+      updated_resource_request.top_frame_origin,
       previous_request_info.is_main_frame,
       previous_request_info.parent_is_main_frame,
       previous_request_info.are_ancestors_secure,
@@ -1028,7 +1030,6 @@ class NavigationURLLoaderImpl::URLLoaderRequestController
       const base::Optional<net::HttpRequestHeaders>& modified_request_headers) {
     DCHECK_CURRENTLY_ON(BrowserThread::IO);
     DCHECK(!redirect_info_.new_url.is_empty());
-
     if (!base::FeatureList::IsEnabled(network::features::kNetworkService)) {
       auto* common_params =
           const_cast<CommonNavigationParams*>(&request_info_->common_params);
@@ -1065,6 +1066,7 @@ class NavigationURLLoaderImpl::URLLoaderRequestController
     resource_request_->url = redirect_info_.new_url;
     resource_request_->method = redirect_info_.new_method;
     resource_request_->site_for_cookies = redirect_info_.new_site_for_cookies;
+    resource_request_->top_frame_origin = redirect_info_.new_top_frame_origin;
     resource_request_->referrer = GURL(redirect_info_.new_referrer);
     resource_request_->referrer_policy = redirect_info_.new_referrer_policy;
     url_chain_.push_back(redirect_info_.new_url);

diff --git a/content/browser/loader/navigation_url_loader_impl_unittest.cc b/content/browser/loader/navigation_url_loader_impl_unittest.cc
@@ -181,7 +181,8 @@ class NavigationURLLoaderImplTest : public testing::Test {
 
     std::unique_ptr<NavigationRequestInfo> request_info(
         new NavigationRequestInfo(
-            common_params, std::move(begin_params), url, is_main_frame,
+            common_params, std::move(begin_params), url,
+            url::Origin::Create(url), is_main_frame,
             false /* parent_is_main_frame */, false /* are_ancestors_secure */,
             -1 /* frame_tree_node_id */, false /* is_for_guests_only */,
             false /* report_raw_headers */, false /* is_prerenering */,
@@ -306,6 +307,38 @@ TEST_F(NavigationURLLoaderImplTest, RequestPriority) {
             NavigateAndReturnRequestPriority(url, false /* is_main_frame */));
 }
 
+TEST_F(NavigationURLLoaderImplTest, TopFrameOriginOfMainFrameNavigation) {
+  ASSERT_TRUE(http_test_server_.Start());
+
+  const GURL url = http_test_server_.GetURL("/foo");
+
+  TestNavigationURLLoaderDelegate delegate;
+  std::unique_ptr<NavigationURLLoader> loader = CreateTestLoader(
+      url,
+      base::StringPrintf("%s: %s", net::HttpRequestHeaders::kOrigin,
+                         url.GetOrigin().spec().c_str()),
+      "GET", &delegate, NavigationDownloadPolicy::kAllow,
+      true /*is_main_frame*/, false /*upgrade_if_insecure*/);
+  delegate.WaitForRequestStarted();
+
+  ASSERT_TRUE(most_recent_resource_request_);
+  EXPECT_EQ(url::Origin::Create(url),
+            *most_recent_resource_request_->top_frame_origin);
+}
+
+TEST_F(NavigationURLLoaderImplTest,
+       TopFrameOriginOfRedirectedMainFrameNavigation) {
+  ASSERT_TRUE(http_test_server_.Start());
+
+  const GURL url = http_test_server_.GetURL("/redirect301-to-echo");
+  const GURL final_url = http_test_server_.GetURL("/echo");
+
+  HTTPRedirectOriginHeaderTest(url, "GET", "GET", url.GetOrigin().spec());
+
+  EXPECT_EQ(url::Origin::Create(final_url),
+            *most_recent_resource_request_->top_frame_origin);
+}
+
 TEST_F(NavigationURLLoaderImplTest, Redirect301Tests) {
   ASSERT_TRUE(http_test_server_.Start());
 

diff --git a/content/browser/loader/navigation_url_loader_unittest.cc b/content/browser/loader/navigation_url_loader_unittest.cc
@@ -94,8 +94,8 @@ class NavigationURLLoaderTest : public testing::Test {
 
     std::unique_ptr<NavigationRequestInfo> request_info(
         new NavigationRequestInfo(common_params, std::move(begin_params), url,
-                                  true, false, false, -1, false, false, false,
-                                  false, nullptr,
+                                  url::Origin::Create(url), true, false, false,
+                                  -1, false, false, false, false, nullptr,
                                   base::UnguessableToken::Create(),
                                   base::UnguessableToken::Create()));
     return NavigationURLLoader::Create(

diff --git a/content/browser/loader/resource_dispatcher_host_impl.cc b/content/browser/loader/resource_dispatcher_host_impl.cc
@@ -850,6 +850,7 @@ void ResourceDispatcherHostImpl::ContinuePendingBeginRequest(
 
   new_request->set_method(request_data.method);
   new_request->set_site_for_cookies(request_data.site_for_cookies);
+  new_request->set_top_frame_origin(request_data.top_frame_origin);
   new_request->set_attach_same_site_cookies(
       request_data.attach_same_site_cookies);
   new_request->set_upgrade_if_insecure(request_data.upgrade_if_insecure);
@@ -1474,6 +1475,7 @@ void ResourceDispatcherHostImpl::BeginNavigationRequest(
 
   new_request->set_method(info.common_params.method);
   new_request->set_site_for_cookies(info.site_for_cookies);
+  new_request->set_top_frame_origin(info.top_frame_origin);
   new_request->set_initiator(info.begin_params->initiator_origin);
   new_request->set_upgrade_if_insecure(info.upgrade_if_insecure);
   if (info.is_main_frame) {

diff --git a/content/browser/loader/resource_dispatcher_host_unittest.cc b/content/browser/loader/resource_dispatcher_host_unittest.cc
@@ -843,8 +843,8 @@ class ResourceDispatcherHostTest : public testing::TestWithParam<TestMode> {
     common_params.url = url;
     std::unique_ptr<NavigationRequestInfo> request_info(
         new NavigationRequestInfo(common_params, std::move(begin_params), url,
-                                  true, false, false, -1, false, false, false,
-                                  false, nullptr,
+                                  url::Origin::Create(url), true, false, false,
+                                  -1, false, false, false, false, nullptr,
                                   base::UnguessableToken::Create(),
                                   base::UnguessableToken::Create()));
     std::unique_ptr<NavigationURLLoader> test_loader =

diff --git a/content/browser/navigation_browsertest.cc b/content/browser/navigation_browsertest.cc
@@ -43,6 +43,7 @@
 #include "content/public/test/test_navigation_observer.h"
 #include "content/public/test/test_navigation_throttle.h"
 #include "content/public/test/test_navigation_throttle_inserter.h"
+#include "content/public/test/url_loader_interceptor.h"
 #include "content/shell/browser/shell.h"
 #include "content/shell/browser/shell_content_browser_client.h"
 #include "content/shell/browser/shell_download_manager_delegate.h"
@@ -191,6 +192,40 @@ class NavigationBrowserTest : public NavigationBaseBrowserTest {
     NavigationBaseBrowserTest::SetUpOnMainThread();
     ASSERT_TRUE(embedded_test_server()->Start());
   }
+
+  // Navigate to |url| and for each ResourceRequest record its
+  // top_frame_origin. Stop listening after |final_resource| has been
+  // detected. The output is recorded in |top_frame_origins|.
+  void NavigateAndRecordTopFrameOrigins(
+      const GURL& url,
+      const GURL& final_resource,
+      bool from_renderer,
+      std::map<GURL, url::Origin>* top_frame_origins) {
+    if (from_renderer)
+      EXPECT_TRUE(NavigateToURL(shell(), GURL("about:blank")));
+
+    base::RunLoop run_loop;
+    base::OnceClosure quit_closure = run_loop.QuitClosure();
+
+    // Intercept network requests and record them.
+    URLLoaderInterceptor interceptor(base::BindLambdaForTesting(
+        [&](URLLoaderInterceptor::RequestParams* params) -> bool {
+          (*top_frame_origins)[params->url_request.url] =
+              *params->url_request.top_frame_origin;
+
+          if (params->url_request.url == final_resource)
+            std::move(quit_closure).Run();
+          return false;
+        }));
+
+    if (from_renderer)
+      EXPECT_TRUE(NavigateToURLFromRenderer(shell(), url));
+    else
+      EXPECT_TRUE(NavigateToURL(shell(), url));
+
+    // Wait until the last resource we care about has been requested.
+    run_loop.Run();
+  }
 };
 
 // Ensure that browser initiated basic navigations work.
@@ -715,6 +750,48 @@ IN_PROC_BROWSER_TEST_F(NavigationBaseBrowserTest,
   EXPECT_EQ("\"done\"", done);
 }
 
+IN_PROC_BROWSER_TEST_F(NavigationBrowserTest, BrowserNavigationTopFrameOrigin) {
+  std::map<GURL, url::Origin> top_frame_origins;
+  GURL url(embedded_test_server()->GetURL("/title1.html"));
+
+  NavigateAndRecordTopFrameOrigins(url, url /*final_resource*/,
+                                   false /*from_renderer*/, &top_frame_origins);
+  EXPECT_EQ(url::Origin::Create(url), top_frame_origins[url]);
+}
+
+IN_PROC_BROWSER_TEST_F(NavigationBrowserTest, RenderNavigationTopFrameOrigin) {
+  std::map<GURL, url::Origin> top_frame_origins;
+  GURL url(embedded_test_server()->GetURL("/title2.html"));
+
+  NavigateAndRecordTopFrameOrigins(url, url /*final_resource*/,
+                                   true /*from_renderer*/, &top_frame_origins);
+  EXPECT_EQ(url::Origin::Create(url), top_frame_origins[url]);
+}
+
+IN_PROC_BROWSER_TEST_F(NavigationBrowserTest, SubframeTopFrameOrigin) {
+  std::map<GURL, url::Origin> top_frame_origins;
+  GURL url(embedded_test_server()->GetURL("/page_with_iframe.html"));
+  GURL iframe_document = embedded_test_server()->GetURL("/title1.html");
+
+  NavigateAndRecordTopFrameOrigins(url, iframe_document /*final_resource*/,
+                                   false /*from_renderer*/, &top_frame_origins);
+  EXPECT_EQ(url::Origin::Create(url), top_frame_origins[url]);
+  EXPECT_EQ(url::Origin::Create(url), top_frame_origins[iframe_document]);
+}
+
+IN_PROC_BROWSER_TEST_F(NavigationBrowserTest, SubresourceTopFrameOrigin) {
+  std::map<GURL, url::Origin> top_frame_origins;
+  GURL url(embedded_test_server()->GetURL("/page_with_iframe_and_image.html"));
+  GURL blank_image = embedded_test_server()->GetURL("/blank.jpg");
+
+  NavigateAndRecordTopFrameOrigins(url, blank_image /*final_resource*/,
+                                   false /*from_renderer*/, &top_frame_origins);
+  EXPECT_EQ(url::Origin::Create(url), top_frame_origins[url]);
+  EXPECT_EQ(url::Origin::Create(url),
+            top_frame_origins[embedded_test_server()->GetURL("/image.jpg")]);
+  EXPECT_EQ(url::Origin::Create(url), top_frame_origins[blank_image]);
+}
+
 // Navigation are started in the browser process. After the headers are
 // received, the URLLoaderClient is transferred from the browser process to the
 // renderer process. This test ensures that when the the URLLoader is deleted

diff --git a/content/common/service_worker/service_worker_loader_helpers.cc b/content/common/service_worker/service_worker_loader_helpers.cc
@@ -124,8 +124,8 @@ ServiceWorkerLoaderHelpers::ComputeRedirectInfo(
           : net::URLRequest::NEVER_CHANGE_FIRST_PARTY_URL;
   return net::RedirectInfo::ComputeRedirectInfo(
       original_request.method, original_request.url,
-      original_request.site_for_cookies, first_party_url_policy,
-      original_request.referrer_policy,
+      original_request.site_for_cookies, original_request.top_frame_origin,
+      first_party_url_policy, original_request.referrer_policy,
       network::ComputeReferrer(original_request.referrer),
       response_head.headers.get(), response_head.headers->response_code(),
       original_request.url.Resolve(new_location), false);

diff --git a/content/common/throttling_url_loader.cc b/content/common/throttling_url_loader.cc
@@ -350,6 +350,8 @@ void ThrottlingURLLoader::StartNow() {
     redirect_info.new_method = start_info_->url_request.method;
     redirect_info.new_url = throttle_will_start_redirect_url_;
     redirect_info.new_site_for_cookies = throttle_will_start_redirect_url_;
+    redirect_info.new_top_frame_origin =
+        url::Origin::Create(throttle_will_start_redirect_url_);
 
     network::ResourceResponseHead response_head;
     std::string header_string = base::StringPrintf(
@@ -557,6 +559,7 @@ void ThrottlingURLLoader::OnReceiveRedirect(
   request.url = redirect_info.new_url;
   request.method = redirect_info.new_method;
   request.site_for_cookies = redirect_info.new_site_for_cookies;
+  request.top_frame_origin = redirect_info.new_top_frame_origin;
   request.referrer = GURL(redirect_info.new_referrer);
   request.referrer_policy = redirect_info.new_referrer_policy;
 

diff --git a/content/renderer/loader/web_url_loader_impl.cc b/content/renderer/loader/web_url_loader_impl.cc
@@ -641,6 +641,7 @@ void WebURLLoaderImpl::Context::Start(const WebURLRequest& request,
   resource_request->method = method;
   resource_request->url = url_;
   resource_request->site_for_cookies = request.SiteForCookies();
+  resource_request->top_frame_origin = request.TopFrameOrigin();
   resource_request->upgrade_if_insecure = request.UpgradeIfInsecure();
   resource_request->is_revalidating = request.IsRevalidating();
   if (!request.RequestorOrigin().IsNull()) {
@@ -818,6 +819,7 @@ bool WebURLLoaderImpl::Context::OnReceivedRedirect(
   url_ = WebURL(redirect_info.new_url);
   return client_->WillFollowRedirect(
       url_, redirect_info.new_site_for_cookies,
+      redirect_info.new_top_frame_origin,
       WebString::FromUTF8(redirect_info.new_referrer),
       Referrer::NetReferrerPolicyToBlinkReferrerPolicy(
           redirect_info.new_referrer_policy),

diff --git a/content/renderer/loader/web_url_loader_impl_unittest.cc b/content/renderer/loader/web_url_loader_impl_unittest.cc
@@ -185,13 +185,15 @@ class TestWebURLLoaderClient : public blink::WebURLLoaderClient {
   ~TestWebURLLoaderClient() override {}
 
   // blink::WebURLLoaderClient implementation:
-  bool WillFollowRedirect(const blink::WebURL& new_url,
-                          const blink::WebURL& new_site_for_cookies,
-                          const blink::WebString& new_referrer,
-                          network::mojom::ReferrerPolicy new_referrer_policy,
-                          const blink::WebString& new_method,
-                          const blink::WebURLResponse& passed_redirect_response,
-                          bool& report_raw_headers) override {
+  bool WillFollowRedirect(
+      const blink::WebURL& new_url,
+      const blink::WebURL& new_site_for_cookies,
+      const base::Optional<blink::WebSecurityOrigin>& new_top_frame_origin,
+      const blink::WebString& new_referrer,
+      network::mojom::ReferrerPolicy new_referrer_policy,
+      const blink::WebString& new_method,
+      const blink::WebURLResponse& passed_redirect_response,
+      bool& report_raw_headers) override {
     EXPECT_TRUE(loader_);
 
     // No test currently simulates mutiple redirects.

diff --git a/content/test/data/page_with_iframe_and_image.html b/content/test/data/page_with_iframe_and_image.html
@@ -0,0 +1,9 @@
+<html>
+<head></head>
+<body>
+  <img src="image.jpg"/>
+  <p>This page has an iframe. Yay for iframes!
+  <p>
+    <iframe src="page_with_image.html" id="test_iframe"></iframe>
+</body>
+</html>
diff --git a/headless/test/test_network_interceptor.cc b/headless/test/test_network_interceptor.cc
@@ -51,6 +51,7 @@ class RedirectLoader : public network::mojom::URLLoader {
   void NotifyRedirect(const std::string& location) {
     auto redirect_info = net::RedirectInfo::ComputeRedirectInfo(
         url_request_.method, url_request_.url, url_request_.site_for_cookies,
+        url_request_.top_frame_origin,
         net::URLRequest::FirstPartyURLPolicy::
             UPDATE_FIRST_PARTY_URL_ON_REDIRECT,
         url_request_.referrer_policy, url_request_.referrer.spec(),