Fix hanging requests from Clear-Site-Data

Some requests, e.g. requests for service worker updates, are not
associated with a WebContents. In these cases Clear-Site-Data returns
early and doesn't mark its task as finished, which leads to hanging
requests.
This CL fixes the hanging requests but does not actually perform the
site data deletion. That will be fixed in a followup CL.

Bug: 898465
Change-Id: I99bbf4339f7b123d4a46552e9f117d4d95b7ec67
Reviewed-on: https://chromium-review.googlesource.com/c/1346468
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611625}

content/browser/browsing_data/clear_site_data_handler.cc

@@ -64,8 +64,12 @@ class SiteDataClearer : public BrowsingDataRemover::Observer {
       bool clear_cache,
       base::OnceClosure callback) {
     WebContents* web_contents = web_contents_getter.Run();
-    if (!web_contents)
+    // TODO(crbug.com/898465): Fix Clear-Site-Data for requests without
+    // WebContents. (E.g. service worker updates)
+    if (!web_contents) {
+      std::move(callback).Run();
       return;
+    }
 
     (new SiteDataClearer(web_contents, origin, clear_cookies, clear_storage,
                          clear_cache, std::move(callback)))

content/browser/browsing_data/clear_site_data_handler_browsertest.cc

@@ -365,6 +365,19 @@ class ClearSiteDataHandlerBrowserTest : public ContentBrowserTest {
 
   net::EmbeddedTestServer* https_server() { return https_server_.get(); }
 
+  // Set a Clear-Site-Data header that |HandleRequest| will use for every
+  // following request.
+  void SetClearSiteDataHeader(const std::string& header) {
+    clear_site_data_header_ = header;
+  }
+
+  bool RunScriptAndGetBool(const std::string& script) {
+    bool data;
+    EXPECT_TRUE(content::ExecuteScriptAndExtractBool(shell()->web_contents(),
+                                                     script, &data));
+    return data;
+  }
+
  private:
   // Handles all requests.
   //
@@ -394,6 +407,9 @@ class ClearSiteDataHandlerBrowserTest : public ContentBrowserTest {
     std::unique_ptr<net::test_server::BasicHttpResponse> response(
         new net::test_server::BasicHttpResponse());
 
+    if (!clear_site_data_header_.empty())
+      response->AddCustomHeader("Clear-Site-Data", clear_site_data_header_);
+
     std::string value;
     if (net::GetValueForKeyInQuery(request.GetURL(), "header", &value))
       response->AddCustomHeader("Clear-Site-Data", value);
@@ -489,6 +505,9 @@ class ClearSiteDataHandlerBrowserTest : public ContentBrowserTest {
   // We can only use |MockCertVerifier| when Network Service was enabled.
   bool is_network_service_enabled_ = false;
 
+  // If this is set, |HandleRequest| will always respond with Clear-Site-Data.
+  std::string clear_site_data_header_;
+
   // Only used when |is_network_service_enabled_| is false.
   std::unique_ptr<CacheTestUtil> cache_test_util_ = nullptr;
 
@@ -978,4 +997,40 @@ IN_PROC_BROWSER_TEST_F(ClearSiteDataHandlerBrowserTest, ClosedTab) {
   shell()->Close();
 }
 
+// Tests that sending Clear-Site-Data during a service worker installation
+// doesn't fail. (see crbug.com/898465)
+IN_PROC_BROWSER_TEST_F(ClearSiteDataHandlerBrowserTest,
+                       ClearSiteDataDuringServiceWorkerInstall) {
+  GURL url = embedded_test_server()->GetURL("127.0.0.1", "/");
+  AddQuery(&url, "file", "worker_test.html");
+  NavigateToURL(shell(), url);
+  // TODO(crbug.com/898465): Fix and enable expectation for storage deletion.
+  // delegate()->ExpectClearSiteDataCall(url::Origin::Create(url), false, true,
+  // false);
+  SetClearSiteDataHeader("\"storage\"");
+  EXPECT_TRUE(RunScriptAndGetBool("installServiceWorker()"));
+  delegate()->VerifyAndClearExpectations();
+  EXPECT_TRUE(RunScriptAndGetBool("hasServiceWorker()"));
+}
+
+// Tests that sending Clear-Site-Data during a service worker update
+// doesn't fail. (see crbug.com/898465)
+IN_PROC_BROWSER_TEST_F(ClearSiteDataHandlerBrowserTest,
+                       ClearSiteDataDuringServiceWorkerUpdate) {
+  GURL url = embedded_test_server()->GetURL("127.0.0.1", "/");
+  AddQuery(&url, "file", "worker_test.html");
+  NavigateToURL(shell(), url);
+  // Install a service worker.
+  EXPECT_TRUE(RunScriptAndGetBool("installServiceWorker()"));
+  delegate()->VerifyAndClearExpectations();
+  // Update the service worker and send C-S-D during update.
+  SetClearSiteDataHeader("\"storage\"");
+  EXPECT_TRUE(RunScriptAndGetBool("updateServiceWorker()"));
+  // TODO(crbug.com/898465): Fix and enable expectation for storage deletion.
+  // delegate()->ExpectClearSiteDataCall(url::Origin::Create(url), false, true,
+  // false);
+  delegate()->VerifyAndClearExpectations();
+  EXPECT_TRUE(RunScriptAndGetBool("hasServiceWorker()"));
+}
+
 }  // namespace content

content/browser/browsing_data/clear_site_data_throttle.cc

@@ -93,8 +93,12 @@ class UIThreadSiteDataClearer : public BrowsingDataRemover::Observer {
       bool clear_cache,
       base::OnceClosure callback) {
     WebContents* web_contents = web_contents_getter.Run();
-    if (!web_contents)
+    // TODO(crbug.com/898465): Fix Clear-Site-Data for requests without
+    // WebContents. (E.g. service worker updates)
+    if (!web_contents) {
+      JumpFromUIToIOThread(std::move(callback));
       return;
+    }
 
     (new UIThreadSiteDataClearer(web_contents, origin, clear_cookies,
                                  clear_storage, clear_cache,

content/test/data/browsing_data/worker_test.html

+<html>
+<script>
+
+  function success_() {
+    domAutomationController.send(true);
+  }
+
+  function failure_() {
+    domAutomationController.send(false);
+  }
+
+  function installServiceWorker() {
+    navigator.serviceWorker.register('/?file=empty_worker.js').then(function() {
+      navigator.serviceWorker.ready.then(success_);
+    }).catch(failure_);
+  }
+
+  function updateServiceWorker() {
+    navigator.serviceWorker.getRegistrations().then(function (registrations) {
+      registrations[0].update().then(success_).catch(failure_);
+    });
+  }
+
+  function hasServiceWorker() {
+    navigator.serviceWorker.getRegistrations().then(function (registrations) {
+      domAutomationController.send(registrations.length > 0);
+    });
+  }
+
+</script>
+<body>
+  This page is used to test creation, update and deletion service workers.
+</body>
+
+</html>
\ No newline at end of file