For service worker, add browser tests to check that the top_frame_origin is null

and the request_initiator is the script origin.

Also fixed 2 issues:
- For the main resource request (serviceWorker.register(...)), it goes through
FrameFetchContext::PrepareRequest() that will always set the top_frame_origin
regardless whether it's from service worker. Adding a check for service worker
seems to fix it.
- The static function GetNetworkFactoryCallbackForTest() in
embedded_worker_instance.cc returns an Optional of Callback, but the caller only
checks the nullity of the Optional but not of the Callback. This sometimes is
causing a crash in the test. Removing the Optional seems to fix it.

Bug: 918868
Change-Id: I2a5b83d8b13b2d52310614dfb5a6dfa4d1cd620a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1553948
Commit-Queue: Yao Xiao <yaoxia@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Shivani Sharma <shivanisha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#652483}

content/browser/service_worker/embedded_worker_instance.cc

@@ -50,10 +50,10 @@ namespace content {
 
 namespace {
 
-base::Optional<EmbeddedWorkerInstance::CreateNetworkFactoryCallback>&
+EmbeddedWorkerInstance::CreateNetworkFactoryCallback&
 GetNetworkFactoryCallbackForTest() {
   static base::NoDestructor<
-      base::Optional<EmbeddedWorkerInstance::CreateNetworkFactoryCallback>>
+      EmbeddedWorkerInstance::CreateNetworkFactoryCallback>
       callback;
   return *callback;
 }
@@ -974,16 +974,16 @@ EmbeddedWorkerInstance::CreateFactoryBundleOnUI(RenderProcessHost* rph,
         rph, routing_id, &default_factory_request);
   }
 
-  if (!GetNetworkFactoryCallbackForTest()) {
+  if (GetNetworkFactoryCallbackForTest().is_null()) {
     rph->CreateURLLoaderFactory(origin, std::move(default_header_client),
                                 std::move(default_factory_request));
   } else {
     network::mojom::URLLoaderFactoryPtr original_factory;
     rph->CreateURLLoaderFactory(origin, std::move(default_header_client),
                                 mojo::MakeRequest(&original_factory));
-    GetNetworkFactoryCallbackForTest()->Run(std::move(default_factory_request),
-                                            rph->GetID(),
-                                            original_factory.PassInterface());
+    GetNetworkFactoryCallbackForTest().Run(std::move(default_factory_request),
+                                           rph->GetID(),
+                                           original_factory.PassInterface());
   }
 
   factory_bundle->set_bypass_redirect_checks(bypass_redirect_checks);

content/browser/service_worker/service_worker_browsertest.cc

@@ -72,6 +72,7 @@
 #include "content/public/test/content_browser_test.h"
 #include "content/public/test/content_browser_test_utils.h"
 #include "content/public/test/test_utils.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/test/test_content_browser_client.h"
@@ -1698,6 +1699,64 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest, FetchWithoutSaveData) {
   SetBrowserClientForTesting(old_client);
 }
 
+// Tests the |top_frame_origin| and |request_initiator| on the main resource and
+// subresource requests from service workers, in order to ensure proper handling
+// by the SplitCache. See https://crbug.com/918868.
+IN_PROC_BROWSER_TEST_F(ServiceWorkerBrowserTest, RequestOrigin) {
+  embedded_test_server()->StartAcceptingConnections();
+
+  // To make things tricky about |top_frame_origin|, this test navigates to a
+  // page on |embedded_test_server()| which has a cross-origin iframe that
+  // registers the service worker.
+  net::EmbeddedTestServer cross_origin_server;
+  cross_origin_server.ServeFilesFromSourceDirectory(GetTestDataFilePath());
+  ASSERT_TRUE(cross_origin_server.Start());
+
+  // There are three requests to test:
+  // 1) The request for the worker itself ("request_origin_worker.js").
+  // 2) importScripts("empty.js") from the service worker.
+  // 3) fetch("empty.html") from the service worker.
+  std::set<GURL> expected_request_urls = {
+      cross_origin_server.GetURL("/service_worker/request_origin_worker.js"),
+      cross_origin_server.GetURL("/service_worker/empty.js"),
+      cross_origin_server.GetURL("/service_worker/empty.html")};
+
+  base::RunLoop request_origin_expectation_waiter;
+  URLLoaderInterceptor request_listener(base::BindLambdaForTesting(
+      [&](URLLoaderInterceptor::RequestParams* params) {
+        auto it = expected_request_urls.find(params->url_request.url);
+        if (it != expected_request_urls.end()) {
+          EXPECT_TRUE(params->url_request.originated_from_service_worker);
+          EXPECT_FALSE(params->url_request.top_frame_origin.has_value());
+          EXPECT_TRUE(params->url_request.request_initiator.has_value());
+          EXPECT_EQ(params->url_request.request_initiator->GetURL(),
+                    cross_origin_server.base_url());
+          expected_request_urls.erase(it);
+        }
+        if (expected_request_urls.empty())
+          request_origin_expectation_waiter.Quit();
+        return false;
+      }));
+
+  NavigateToURLBlockUntilNavigationsComplete(
+      shell(),
+      embedded_test_server()->GetURL(
+          "/service_worker/one_subframe.html?subframe_url=" +
+          cross_origin_server
+              .GetURL("/service_worker/create_service_worker.html")
+              .spec()),
+      1);
+  RenderFrameHost* subframe_rfh = FrameMatchingPredicate(
+      shell()->web_contents(),
+      base::BindRepeating(&FrameMatchesName, "subframe_name"));
+  DCHECK(subframe_rfh);
+
+  EXPECT_EQ("DONE",
+            EvalJs(subframe_rfh, "register('request_origin_worker.js');"));
+
+  request_origin_expectation_waiter.Run();
+}
+
 IN_PROC_BROWSER_TEST_F(ServiceWorkerBrowserTest, FetchPageWithSaveData) {
   StartServerAndNavigateToSetup();
   const char kPageUrl[] = "/service_worker/handle_fetch.html";

content/renderer/service_worker/service_worker_fetch_context_impl.cc

@@ -128,8 +128,6 @@ blink::WebURL ServiceWorkerFetchContextImpl::SiteForCookies() const {
 
 base::Optional<blink::WebSecurityOrigin>
 ServiceWorkerFetchContextImpl::TopFrameOrigin() const {
-  // TODO(jkarlin): Determine what the top-frame-origin of a service worker is.
-  // See https://crbug.com/918868.
   return base::nullopt;
 }
 

content/test/data/service_worker/one_subframe.html

+<body></body>
+<title>a page with an iframe</title>
+<!-- This page makes an iframe to |subframe_url|.-->
+<script>
+const url_params = new URLSearchParams(location.search);
+let subframe_url = url_params.get("subframe_url");
+if (subframe_url != null) {
+  let iframe = document.createElement('iframe');
+  iframe.src = subframe_url;
+  iframe.name = "subframe_name";
+  document.body.appendChild(iframe);
+}
+</script>

content/test/data/service_worker/request_origin_worker.js

+// The service worker for the request origin test.
+importScripts('empty.js');
+fetch('empty.html');

services/network/public/mojom/url_loader.mojom

@@ -87,9 +87,22 @@ struct URLRequest {
   url.mojom.Url site_for_cookies;
 
   // A non-null |top_frame_origin| contains the origin of the top frame of the
-  // page making the request. Note that this is experimental and is only set for
-  // navigation and document subresource requests but not other cases such as
-  // workers.
+  // page making the request. |top_frame_origin| depends on the context making
+  // the request.
+  // - If a page is making the request, it is the origin of the top-level
+  //   frame of the page. This is set for the navigation and subresource
+  //   requests.
+  // - If a dedicated worker is making the request, it is the top-frame
+  //   origin of its creator (which may be a page or a dedicated worker).
+  // - If a shared worker or a service worker is making the request, it is null,
+  //   including for both the main script request and subresource requests.
+  //
+  // In some other cases(?) |top_frame_origin| may be null.
+  //
+  // Note that this is experimental.
+  //
+  // TODO(crbug.com/911299): This field will most likely be removed at some
+  // point.
   url.mojom.Origin? top_frame_origin;
 
   // Boolean indicating whether SameSite cookies are allowed to be attached

third_party/blink/public/platform/web_worker_fetch_context.h

@@ -112,7 +112,7 @@ class WebWorkerFetchContext : public base::RefCounted<WebWorkerFetchContext> {
 
   // The top-frame-origin for the worker. For a dedicated worker this is the
   // top-frame origin of the page that created the worker. For a shared worker
-  // this is unset.
+  // or a service worker this is unset.
   virtual base::Optional<WebSecurityOrigin> TopFrameOrigin() const = 0;
 
   // Reports the certificate error to the browser process.

third_party/blink/renderer/core/loader/frame_fetch_context.cc

@@ -396,7 +396,14 @@ void FrameFetchContext::PrepareRequest(
        ResourceRequest::RedirectStatus::kFollowedRedirect);
 
   SetFirstPartyCookie(request);
-  request.SetTopFrameOrigin(GetTopFrameOrigin());
+  if (request.GetRequestContext() ==
+      mojom::RequestContextType::SERVICE_WORKER) {
+    // The top frame origin is defined to be null for service worker main
+    // resource requests.
+    DCHECK(!request.TopFrameOrigin());
+  } else {
+    request.SetTopFrameOrigin(GetTopFrameOrigin());
+  }
 
   String user_agent = GetUserAgent();
   request.SetHTTPUserAgent(AtomicString(user_agent));

third_party/blink/renderer/core/loader/worker_fetch_context.cc

@@ -55,8 +55,7 @@ scoped_refptr<const SecurityOrigin> WorkerFetchContext::GetTopFrameOrigin()
   base::Optional<WebSecurityOrigin> top_frame_origin =
       web_context_->TopFrameOrigin();
 
-  // TODO(crbug.com/918868) The top frame origin of shared and service
-  // workers is unknown.
+  // The top frame origin of shared and service workers is null.
   if (!top_frame_origin) {
     DCHECK(global_scope_->IsSharedWorkerGlobalScope() ||
            global_scope_->IsServiceWorkerGlobalScope());