Request SSLInfo when updating a main service worker script

Network requests for byte-for-byte update checking are issued by
ServiceWorkerSingleScriptUpdateChecker. It didn't set an option flag to request
a SSL info to the network service, and the updated script was missing it. The
info is needed to show SSL padlock etc for the navigation request.
This CL adds the flag in the single script update checker, and then remove it in
the updated script loader if it's not necessary to be sent to the renderer.

Bug: 999562
Change-Id: If12a5cbddfc08d0fb50f3a9a2197a5281ab867d3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1778460
Commit-Queue: Makoto Shimazu <shimazu@chromium.org>
Auto-Submit: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#692430}
diff --git a/content/browser/service_worker/service_worker_browsertest.cc b/content/browser/service_worker/service_worker_browsertest.cc
index de2ef7c..57cc541 100644
--- a/content/browser/service_worker/service_worker_browsertest.cc
+++ b/content/browser/service_worker/service_worker_browsertest.cc
@@ -480,6 +480,19 @@
     "NetworkError: The service worker navigation preload request failed with "
     "a network error.";
 
+void CheckPageIsMarkedSecure(
+    Shell* shell,
+    scoped_refptr<net::X509Certificate> expected_certificate) {
+  NavigationEntry* entry =
+      shell->web_contents()->GetController().GetVisibleEntry();
+  EXPECT_TRUE(entry->GetSSL().initialized);
+  EXPECT_FALSE(!!(entry->GetSSL().content_status &
+                  SSLStatus::DISPLAYED_INSECURE_CONTENT));
+  EXPECT_TRUE(expected_certificate->EqualsExcludingChain(
+      entry->GetSSL().certificate.get()));
+  EXPECT_FALSE(net::IsCertStatusError(entry->GetSSL().cert_status));
+}
+
 }  // namespace
 
 class ServiceWorkerBrowserTest : public ContentBrowserTest {
@@ -2834,35 +2847,78 @@
 IN_PROC_BROWSER_TEST_F(ServiceWorkerBrowserTest,
                        ResponseFromHTTPSServiceWorkerIsMarkedAsSecure) {
   StartServerAndNavigateToSetup();
-  const char kPageUrl[] = "/service_worker/fetch_event_blob.html";
-  const char kWorkerUrl[] = "/service_worker/fetch_event_blob.js";
+  const char kPageUrl[] = "/service_worker/in-scope";
+  const char kWorkerUrl[] = "/service_worker/worker_script";
+  const char kWorkerScript[] = R"(
+      self.addEventListener('fetch', e => {
+        e.respondWith(new Response('<title>Title</title>', {
+          headers: {'Content-Type': 'text/html'}
+        }));
+      });
+      // Version: %d)";
+
+  // Register a handler which serves different script on each request. The
+  // service worker returns a page titled by "Title" via Blob.
+  int service_worker_served_count = 0;
   net::EmbeddedTestServer https_server(net::EmbeddedTestServer::TYPE_HTTPS);
-  https_server.ServeFilesFromSourceDirectory(GetTestDataFilePath());
+  https_server.RegisterRequestHandler(base::BindLambdaForTesting(
+      [&](const net::test_server::HttpRequest& request)
+          -> std::unique_ptr<net::test_server::HttpResponse> {
+        if (request.relative_url != kWorkerUrl)
+          return nullptr;
+        auto response = std::make_unique<net::test_server::BasicHttpResponse>();
+        response->set_code(net::HTTP_OK);
+        response->set_content_type("text/javascript");
+        response->set_content(
+            base::StringPrintf(kWorkerScript, ++service_worker_served_count));
+        return response;
+      }));
   ASSERT_TRUE(https_server.Start());
 
-  scoped_refptr<WorkerActivatedObserver> observer =
-      new WorkerActivatedObserver(wrapper());
-  observer->Init();
-  blink::mojom::ServiceWorkerRegistrationOptions options(
-      https_server.GetURL(kPageUrl), blink::mojom::ScriptType::kClassic,
-      blink::mojom::ServiceWorkerUpdateViaCache::kImports);
-  public_context()->RegisterServiceWorker(
-      https_server.GetURL(kWorkerUrl), options,
-      base::BindOnce(&ExpectResultAndRun, true, base::DoNothing()));
-  observer->Wait();
+  // 1st attempt: install a service worker and open the controlled page.
+  {
+    // Register a service worker which controls |kPageUrl|.
+    auto observer = base::MakeRefCounted<WorkerActivatedObserver>(wrapper());
+    observer->Init();
+    blink::mojom::ServiceWorkerRegistrationOptions options(
+        https_server.GetURL(kPageUrl), blink::mojom::ScriptType::kClassic,
+        blink::mojom::ServiceWorkerUpdateViaCache::kImports);
+    public_context()->RegisterServiceWorker(
+        https_server.GetURL(kWorkerUrl), options,
+        base::BindOnce(&ExpectResultAndRun, true, base::DoNothing()));
+    observer->Wait();
+    EXPECT_EQ(1, service_worker_served_count);
 
-  const base::string16 title = base::ASCIIToUTF16("Title");
-  TitleWatcher title_watcher(shell()->web_contents(), title);
-  EXPECT_TRUE(NavigateToURL(shell(), https_server.GetURL(kPageUrl)));
-  EXPECT_EQ(title, title_watcher.WaitAndGetTitle());
-  NavigationEntry* entry =
-      shell()->web_contents()->GetController().GetVisibleEntry();
-  EXPECT_TRUE(entry->GetSSL().initialized);
-  EXPECT_FALSE(!!(entry->GetSSL().content_status &
-                  SSLStatus::DISPLAYED_INSECURE_CONTENT));
-  EXPECT_TRUE(https_server.GetCertificate()->EqualsExcludingChain(
-      entry->GetSSL().certificate.get()));
-  EXPECT_FALSE(net::IsCertStatusError(entry->GetSSL().cert_status));
+    // Wait until the page is appropriately served by the service worker.
+    const base::string16 title = base::ASCIIToUTF16("Title");
+    TitleWatcher title_watcher(shell()->web_contents(), title);
+    EXPECT_TRUE(NavigateToURL(shell(), https_server.GetURL(kPageUrl)));
+    EXPECT_EQ(title, title_watcher.WaitAndGetTitle());
+
+    // The page should be marked as secure.
+    CheckPageIsMarkedSecure(shell(), https_server.GetCertificate());
+  }
+
+  // Navigate away from the page so that the worker has no controllee.
+  EXPECT_TRUE(NavigateToURL(shell(), GURL("about:blank")));
+
+  // 2nd attempt: update the service worker and open the controlled page again.
+  {
+    // Update the service worker.
+    auto observer = base::MakeRefCounted<WorkerActivatedObserver>(wrapper());
+    observer->Init();
+    wrapper()->UpdateRegistration(https_server.GetURL(kPageUrl));
+    observer->Wait();
+
+    // Wait until the page is appropriately served by the service worker.
+    const base::string16 title = base::ASCIIToUTF16("Title");
+    TitleWatcher title_watcher(shell()->web_contents(), title);
+    EXPECT_TRUE(NavigateToURL(shell(), https_server.GetURL(kPageUrl)));
+    EXPECT_EQ(title, title_watcher.WaitAndGetTitle());
+
+    // The page should be marked as secure.
+    CheckPageIsMarkedSecure(shell(), https_server.GetCertificate());
+  }
 
   shell()->Close();
 
diff --git a/content/browser/service_worker/service_worker_single_script_update_checker.cc b/content/browser/service_worker/service_worker_single_script_update_checker.cc
index f9ca75b..abde435 100644
--- a/content/browser/service_worker/service_worker_single_script_update_checker.cc
+++ b/content/browser/service_worker/service_worker_single_script_update_checker.cc
@@ -123,6 +123,7 @@
                          script_url.spec(), "main_script_url",
                          main_script_url.spec());
 
+  uint32_t options = network::mojom::kURLLoadOptionNone;
   network::ResourceRequest resource_request;
   resource_request.url = script_url;
   resource_request.site_for_cookies = main_script_url;
@@ -162,6 +163,11 @@
         static_cast<int>(blink::mojom::RequestContextType::SERVICE_WORKER);
     resource_request.resource_type =
         static_cast<int>(ResourceType::kServiceWorker);
+
+    // Request SSLInfo. It will be persisted in service worker storage and
+    // may be used by ServiceWorkerNavigationLoader for navigations handled
+    // by this service worker.
+    options |= network::mojom::kURLLoadOptionSendSSLInfoWithResponse;
   } else {
     // The "fetch a classic worker-imported script" doesn't have any statement
     // about mode and credentials mode. Use the default value, which is
@@ -219,8 +225,8 @@
   network_loader_ = ServiceWorkerUpdatedScriptLoader::
       ThrottlingURLLoaderCoreWrapper::CreateLoaderAndStart(
           loader_factory->Clone(), browser_context_getter, MSG_ROUTING_NONE,
-          request_id, network::mojom::kURLLoadOptionNone, resource_request,
-          std::move(network_client), kUpdateCheckTrafficAnnotation);
+          request_id, options, resource_request, std::move(network_client),
+          kUpdateCheckTrafficAnnotation);
   DCHECK_EQ(network_loader_state_,
             ServiceWorkerUpdatedScriptLoader::LoaderState::kNotStarted);
   network_loader_state_ =
diff --git a/content/browser/service_worker/service_worker_single_script_update_checker_unittest.cc b/content/browser/service_worker/service_worker_single_script_update_checker_unittest.cc
index 76532aa7..e51d6cc 100644
--- a/content/browser/service_worker/service_worker_single_script_update_checker_unittest.cc
+++ b/content/browser/service_worker/service_worker_single_script_update_checker_unittest.cc
@@ -1163,5 +1163,51 @@
             blink::ServiceWorkerStatusCode::kErrorNetwork);
 }
 
+// The main script needs to request a SSL info so that the navigation handled
+// by the service worker can use the SSL info served for the main script.
+TEST_F(ServiceWorkerSingleScriptUpdateCheckerTest, RequestSSLInfo) {
+  auto loader_factory = std::make_unique<network::TestURLLoaderFactory>();
+  base::Optional<CheckResult> check_result;
+
+  // Load the main script. It needs a SSL info.
+  std::unique_ptr<ServiceWorkerSingleScriptUpdateChecker> checker =
+      CreateSingleScriptUpdateChecker(
+          kScriptURL, kScriptURL, GURL(kScope), false /* force_bypass_cache */,
+          blink::mojom::ServiceWorkerUpdateViaCache::kNone, base::TimeDelta(),
+          std::make_unique<MockServiceWorkerResponseReader>(),
+          std::make_unique<MockServiceWorkerResponseReader>(),
+          std::make_unique<MockServiceWorkerResponseWriter>(),
+          loader_factory.get(), &check_result);
+  base::RunLoop().RunUntilIdle();
+
+  {
+    ASSERT_EQ(1u, loader_factory->pending_requests()->size());
+    const network::TestURLLoaderFactory::PendingRequest* pending_request =
+        loader_factory->GetPendingRequest(0);
+    EXPECT_EQ(kScriptURL, pending_request->request.url);
+    EXPECT_EQ(network::mojom::kURLLoadOptionSendSSLInfoWithResponse,
+              pending_request->options);
+  }
+
+  // Load imported script. It doesn't need SSL info.
+  checker = CreateSingleScriptUpdateChecker(
+      kImportedScriptURL, kScriptURL, GURL(kScope),
+      false /* force_bypass_cache */,
+      blink::mojom::ServiceWorkerUpdateViaCache::kNone, base::TimeDelta(),
+      std::make_unique<MockServiceWorkerResponseReader>(),
+      std::make_unique<MockServiceWorkerResponseReader>(),
+      std::make_unique<MockServiceWorkerResponseWriter>(), loader_factory.get(),
+      &check_result);
+  base::RunLoop().RunUntilIdle();
+
+  {
+    ASSERT_EQ(2u, loader_factory->pending_requests()->size());
+    const network::TestURLLoaderFactory::PendingRequest* pending_request =
+        loader_factory->GetPendingRequest(1);
+    EXPECT_EQ(kImportedScriptURL, pending_request->request.url);
+    EXPECT_EQ(network::mojom::kURLLoadOptionNone, pending_request->options);
+  }
+}
+
 }  // namespace
 }  // namespace content
diff --git a/content/browser/service_worker/service_worker_updated_script_loader.cc b/content/browser/service_worker/service_worker_updated_script_loader.cc
index 1e72f6f..449d74b 100644
--- a/content/browser/service_worker/service_worker_updated_script_loader.cc
+++ b/content/browser/service_worker/service_worker_updated_script_loader.cc
@@ -342,6 +342,13 @@
   auto response = ServiceWorkerUtils::CreateResourceResponseHeadAndMetadata(
       info, options_, request_start_, base::TimeTicks::Now(),
       response_info->response_data_size);
+  // Don't pass SSLInfo to the client when the original request doesn't ask
+  // to send it.
+  if (response.head.ssl_info.has_value() &&
+      !(options_ & network::mojom::kURLLoadOptionSendSSLInfoWithResponse)) {
+    response.head.ssl_info.reset();
+  }
+
   client_->OnReceiveResponse(std::move(response.head));
   if (!response.metadata.empty())
     client_->OnReceiveCachedMetadata(std::move(response.metadata));
diff --git a/services/network/test/test_url_loader_factory.cc b/services/network/test/test_url_loader_factory.cc
index 31ea672c..ad3cca53 100644
--- a/services/network/test/test_url_loader_factory.cc
+++ b/services/network/test/test_url_loader_factory.cc
@@ -128,6 +128,7 @@
   PendingRequest pending_request;
   pending_request.client = std::move(client);
   pending_request.request = url_request;
+  pending_request.options = options;
   pending_requests_.push_back(std::move(pending_request));
 }
 
diff --git a/services/network/test/test_url_loader_factory.h b/services/network/test/test_url_loader_factory.h
index f6087f98..f6e3319 100644
--- a/services/network/test/test_url_loader_factory.h
+++ b/services/network/test/test_url_loader_factory.h
@@ -34,6 +34,7 @@
 
     mojom::URLLoaderClientPtr client;
     ResourceRequest request;
+    uint32_t options;
   };
 
   // Bitfield that is used with |SimulateResponseForPendingRequest()| to