Support the Clear-Site-Data header on resource requests

content/browser/browsing_data/clear_site_data_throttle_browsertest.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/browsing_data/clear_site_data_throttle.h"
 
 #include <memory>
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/command_line.h"
+#include "base/strings/stringprintf.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/synchronization/waitable_event.h"
+#include "content/browser/browsing_data/browsing_data_filter_builder_impl.h"
+#include "content/browser/service_worker/service_worker_context_wrapper.h"
+#include "content/public/browser/browser_context.h"
+#include "content/public/browser/browsing_data_remover.h"
 #include "content/public/browser/content_browser_client.h"
+#include "content/public/browser/storage_partition.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/content_switches.h"
+#include "content/public/test/browser_test_utils.h"
+#include "content/public/test/cache_test_util.h"
 #include "content/public/test/content_browser_test.h"
 #include "content/public/test/content_browser_test_utils.h"
+#include "content/public/test/mock_browsing_data_remover_delegate.h"
 #include "content/public/test/test_navigation_observer.h"
 #include "content/shell/browser/shell.h"
 #include "net/base/escape.h"
 #include "net/base/url_util.h"
+#include "net/cookies/cookie_store.h"
 #include "net/dns/mock_host_resolver.h"
 #include "net/test/embedded_test_server/http_request.h"
 #include "net/test/embedded_test_server/http_response.h"
+#include "net/url_request/url_request_context.h"
+#include "net/url_request/url_request_context_getter.h"
 #include "storage/browser/quota/quota_settings.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "url/origin.h"
 #include "url/url_constants.h"
 
 using testing::_;
 
 namespace content {
 
 namespace {
 
-class MockContentBrowserClient : public ContentBrowserClient {
- public:
-  MOCK_METHOD6(ClearSiteData,
-               void(content::BrowserContext* browser_context,
-                    const url::Origin& origin,
-                    bool remove_cookies,
-                    bool remove_storage,
-                    bool remove_cache,
-                    const base::Closure& callback));
-
-  void GetQuotaSettings(
-      content::BrowserContext* context,
-      content::StoragePartition* partition,
-      const storage::OptionalQuotaSettingsCallback& callback) override {
-    callback.Run(storage::GetHardCodedSettings(100 * 1024 * 1024));
-  }
-};
-
-class TestContentBrowserClient : public MockContentBrowserClient {
- public:
-  void ClearSiteData(content::BrowserContext* browser_context,
-                     const url::Origin& origin,
-                     bool remove_cookies,
-                     bool remove_storage,
-                     bool remove_cache,
-                     const base::Closure& callback) override {
-    // Record the method call and run the |callback|.
-    MockContentBrowserClient::ClearSiteData(browser_context, origin,
-                                            remove_cookies, remove_storage,
-                                            remove_cache, callback);
-    callback.Run();
-  }
-};
-
 // Adds a key=value pair to the url's query.
 void AddQuery(GURL* url, const std::string& key, const std::string& value) {
   *url = GURL(url->spec() + (url->has_query() ? "&" : "?") + key + "=" +
               net::EscapeQueryParamValue(value, false));
 }
 
+// A helper function to synchronize with JS side of the tests. JS can append
+// information to the loaded website's title and C++ will wait until that
+// happens.
+void WaitForTitle(const Shell* shell, const char* expected_title) {
+  base::string16 expected_title_16 = base::ASCIIToUTF16(expected_title);
+  TitleWatcher title_watcher(shell->web_contents(), expected_title_16);
+  ASSERT_EQ(expected_title_16, title_watcher.WaitAndGetTitle());

[mmenke, 2017/05/25 19:23:09]

I don't think we want to depend on the implementation of navigation - i.e.,
creating the watcher after we start navigation is relying on squatting on the UI
thread blocking the title from being updated.  This is probably going to always
be safe to do, but I don't think we want to make that a dependency of this test.

[msramek, 2017/05/30 21:58:45]

Sorry, I don't understand. TitleWatcher constructor doesn't really do anything,
it just stores the expected title in a member variable.

It's WaitAndGetTitle() that does the actual work, and that one obviously must be
called after the navigation is executed.

It checks the title first and then spins a RunLoop waiting for the title update
event. This will always work if the title update happens on the UI thread, which
it should, because that's where WebContents lives.

[mmenke, 2017/05/30 22:58:35]

TitleWatcher starts watching, which is rather important - i.e., if the title
changes before you start watching, you could have a problem.

That having been said, it looks like TitleWatcher::WaitAndGetTitle now checks
the current title of the tab - it didn't always do that, so if the title had
already changed by the time you created the watcher, the test would fail.  But
looks like this has long since been fixed.  Now it's just if the title changes
away from the one you want that you have a problem, which shouldn't happen here.

[msramek, 2017/06/01 22:02:27]

Acknowledged. That's true, but indeed isn't the case in these tests.

+}
+
 // A value of the Clear-Site-Data header that requests cookie deletion. Reused
 // in tests that need a valid header but do not depend on its value.
 static const char* kClearCookiesHeader = "{ \"types\": [ \"cookies\" ] }";
 
+// A helper class to observe BrowsingDataRemover deletion tasks coming from
+// ClearSiteData.
+class TestBrowsingDataRemoverDelegate : public MockBrowsingDataRemoverDelegate {
+ public:
+  // Sets a test expectation that a Clear-Site-Data header call from |origin|,
+  // instructing to delete |cookies|, |storage|, and |cache|, will schedule
+  // the corresponding BrowsingDataRemover deletion tasks.
+  void ExpectClearSiteDataCall(const url::Origin& origin,
+                               bool cookies,
+                               bool storage,
+                               bool cache) {
+    const int origin_type_mask =

[mmenke, 2017/05/25 19:23:09]

kOriginTypeMatch

[msramek, 2017/05/30 21:58:45]

Done.

+        BrowsingDataRemover::ORIGIN_TYPE_UNPROTECTED_WEB |
+        BrowsingDataRemover::ORIGIN_TYPE_PROTECTED_WEB;
+
+    if (cookies) {
+      int data_type_mask = BrowsingDataRemover::DATA_TYPE_COOKIES |
+                           BrowsingDataRemover::DATA_TYPE_CHANNEL_IDS;
+
+      BrowsingDataFilterBuilderImpl filter_builder(
+          BrowsingDataFilterBuilder::WHITELIST);
+      filter_builder.AddRegisterableDomain(origin.host());
+      ExpectCall(base::Time(), base::Time::Max(), data_type_mask,
+                 origin_type_mask, std::move(filter_builder));
+    }
+    if (storage || cache) {
+      int data_type_mask =
+          (storage ? BrowsingDataRemover::DATA_TYPE_DOM_STORAGE : 0) |
+          (cache ? BrowsingDataRemover::DATA_TYPE_CACHE : 0);
+
+      BrowsingDataFilterBuilderImpl filter_builder(
+          BrowsingDataFilterBuilder::WHITELIST);
+      filter_builder.AddOrigin(origin);
+      ExpectCall(base::Time(), base::Time::Max(), data_type_mask,
+                 origin_type_mask, std::move(filter_builder));
+    }
+  }
+
+  // A shortcut for the above method, but with only cookies deleted. This is
+  // useful for most tests that use |kClearCookiesHeader|.
+  void ExpectClearSiteDataCookiesCall(const url::Origin& origin) {
+    ExpectClearSiteDataCall(origin, true, false, false);
+  }
+};
+
 }  // namespace
 
 class ClearSiteDataThrottleBrowserTest : public ContentBrowserTest {
  public:
   void SetUpCommandLine(base::CommandLine* command_line) override {
     ContentBrowserTest::SetUpCommandLine(command_line);
     command_line->AppendSwitch(
         switches::kEnableExperimentalWebPlatformFeatures);
 
     // We're redirecting all hosts to localhost even on HTTPS, so we'll get
     // certificate errors.
     command_line->AppendSwitch(switches::kIgnoreCertificateErrors);
   }
 
   void SetUpOnMainThread() override {
     ContentBrowserTest::SetUpOnMainThread();
 
-    SetBrowserClientForTesting(&test_client_);
+    BrowserContext::GetBrowsingDataRemover(browser_context())
+        ->SetEmbedderDelegate(&embedder_delegate_);
 
     // Set up HTTP and HTTPS test servers that handle all hosts.
     host_resolver()->AddRule("*", "127.0.0.1");
 
     embedded_test_server()->RegisterRequestHandler(
         base::Bind(&ClearSiteDataThrottleBrowserTest::HandleRequest,
                    base::Unretained(this)));
     ASSERT_TRUE(embedded_test_server()->Start());
 
     https_server_.reset(new net::EmbeddedTestServer(
         net::test_server::EmbeddedTestServer::TYPE_HTTPS));
     https_server_->SetSSLConfig(net::EmbeddedTestServer::CERT_OK);
     https_server_->RegisterRequestHandler(
         base::Bind(&ClearSiteDataThrottleBrowserTest::HandleRequest,
                    base::Unretained(this)));
     ASSERT_TRUE(https_server_->Start());
+
+    // Initialize the cookie store pointer on the IO thread.
+    base::WaitableEvent waitable_event(
+        base::WaitableEvent::ResetPolicy::MANUAL,
+        base::WaitableEvent::InitialState::NOT_SIGNALED);
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::BindOnce(
+            &ClearSiteDataThrottleBrowserTest::InitializeCookieStore,
+            base::Unretained(this),
+            base::Unretained(
+                BrowserContext::GetDefaultStoragePartition(browser_context())
+                    ->GetURLRequestContext()),
+            base::Unretained(&waitable_event)));
+    waitable_event.Wait();

[mmenke, 2017/05/25 19:23:10]

optional:  I think RunLoops are a lot more common in tests than events.  Also
seems strange the sometimes events are used in this file, sometimes RunLoops
are, for pretty much the same thing.

[msramek, 2017/05/30 21:58:45]

Done. (Changed to RunLoops)

   }
 
-  TestContentBrowserClient* GetContentBrowserClient() { return &test_client_; }
+  BrowserContext* browser_context() {
+    return shell()->web_contents()->GetBrowserContext();
+  }
+
+  void InitializeCookieStore(

[mmenke, 2017/05/25 19:23:10]

Why is this needed?  Can't AddCookie/GetCookie take the URLRequestContextGetter
themselves?

[msramek, 2017/05/30 21:58:45]

We need to do two things on the IO thread:
1. Call GetURLRequestContext() in order to get to CookieStore.
2. Execute a method on CookieStore
(SetCookieWithOptionsAsync()/GetAllCookiesAsync() in AddCookie()/GetCookie()
respectively).

It seems to me that the cleanest solution is to do 1. separately in a helper
function, then just pass the pointer to CookieStore in both instances of 2.

Alternatively, AddCookie() and GetCookies() could contain two PostTask()s on IO
thread to execute both operations. We would end up with something like:

PostTask(CookieStore::operator=, base::Unretained(cookie_store_), ...)
PostTask(SetCookieWithOptionsAsync, base::Unretained(cookie_store), ...)

Or we could have two helper methods AddCookieOnIOThread()/GetCookiesOnIOThread
that both execute both operations. But the current solution only requires one
helper method. So it seems cleaner.

+      net::URLRequestContextGetter* request_context_getter,
+      base::WaitableEvent* waitable_event) {
+    cookie_store_ =
+        request_context_getter->GetURLRequestContext()->cookie_store();
+    waitable_event->Signal();
+  }
+
+  // Adds a cookie for the |url|. Used in the cookie integration tests.
+  void AddCookie(const GURL& url) {
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    base::WaitableEvent waitable_event(
+        base::WaitableEvent::ResetPolicy::MANUAL,
+        base::WaitableEvent::InitialState::NOT_SIGNALED);
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::BindOnce(
+            &net::CookieStore::SetCookieWithOptionsAsync,
+            base::Unretained(cookie_store_), url, "A=1", net::CookieOptions(),
+            base::Bind(&ClearSiteDataThrottleBrowserTest::AddCookieCallback,
+                       base::Unretained(this),
+                       base::Unretained(&waitable_event))));
+    waitable_event.Wait();
+  }
+
+  // Retrieves the list of all cookies. Used in the cookie integration tests.
+  net::CookieList GetCookies() {
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    base::WaitableEvent waitable_event(
+        base::WaitableEvent::ResetPolicy::MANUAL,
+        base::WaitableEvent::InitialState::NOT_SIGNALED);
+    net::CookieList cookie_list;
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::BindOnce(
+            &net::CookieStore::GetAllCookiesAsync,
+            base::Unretained(cookie_store_),
+            base::Bind(&ClearSiteDataThrottleBrowserTest::GetCookiesCallback,
+                       base::Unretained(this),
+                       base::Unretained(&waitable_event),
+                       base::Unretained(&cookie_list))));
+    waitable_event.Wait();
+    return cookie_list;
+  }
+
+  // Adds a service worker. Used in the storage integration tests.
+  void AddServiceWorker(const std::string& origin) {
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    ServiceWorkerContextWrapper* service_worker_context =
+        static_cast<ServiceWorkerContextWrapper*>(
+            BrowserContext::GetDefaultStoragePartition(browser_context())
+                ->GetServiceWorkerContext());
+
+    GURL html_url = https_server()->GetURL(origin, "/worker_setup.html");
+    GURL js_url = https_server()->GetURL(origin, "/?file=worker.js");
+    base::RunLoop run_loop;
+
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::BindOnce(
+            &ServiceWorkerContextWrapper::RegisterServiceWorker,
+            base::Unretained(service_worker_context), html_url, js_url,
+            base::Bind(
+                &ClearSiteDataThrottleBrowserTest::AddServiceWorkerCallback,
+                base::Unretained(this), base::Unretained(&run_loop))));
+    run_loop.Run();
+  }
+
+  // Retrieves the list of all service workers. Used in the storage integration
+  // tests.
+  std::vector<ServiceWorkerUsageInfo> GetServiceWorkers() {
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    ServiceWorkerContextWrapper* service_worker_context =
+        static_cast<ServiceWorkerContextWrapper*>(
+            BrowserContext::GetDefaultStoragePartition(browser_context())
+                ->GetServiceWorkerContext());
+
+    std::vector<ServiceWorkerUsageInfo> service_workers;
+    base::WaitableEvent waitable_event(
+        base::WaitableEvent::ResetPolicy::MANUAL,
+        base::WaitableEvent::InitialState::NOT_SIGNALED);
+
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::BindOnce(
+            &ServiceWorkerContextWrapper::GetAllOriginsInfo,
+            base::Unretained(service_worker_context),
+            base::Bind(
+                &ClearSiteDataThrottleBrowserTest::GetServiceWorkersCallback,
+                base::Unretained(this), base::Unretained(&waitable_event),
+                base::Unretained(&service_workers))));
+    waitable_event.Wait();
+
+    return service_workers;
+  }
+
+  TestBrowsingDataRemoverDelegate* delegate() { return &embedder_delegate_; }
 
   net::EmbeddedTestServer* https_server() { return https_server_.get(); }
 
  private:
-  // Handles all requests. If the request url query contains a "header" key,
-  // responds with the "Clear-Site-Data" header of the corresponding value.
-  // If the query contains a "redirect" key, responds with a redirect to a url
-  // given by the corresponding value.
+  // Handles all requests.
+  //
+  // Supports the following <key>=<value> query parameters in the url:
+  // <key>="header"       responds with the header "Clear-Site-Data: <value>"
+  // <key>="redirect"     responds with a redirect to the url <value>
+  // <key>="html"         responds with a text/html content <value>
+  // <key>="file"         responds with the content of file <value>
   //
   // Example: "https://localhost/?header={}&redirect=example.com" will respond
   // with headers
   // Clear-Site-Data: {}
   // Location: example.com
+  //
+  // Example: "https://localhost/?html=<html><head></head><body></body></html>"
+  // will respond with the header
+  // Content-Type: text/html
+  // and content
+  // <html><head></head><body></body></html>
+  //
+  // Example: "https://localhost/?file=file.html

[mmenke, 2017/05/25 19:23:10]

+"

[msramek, 2017/05/30 21:58:45]

Done.

+  // will respond with the header
+  // Content-Type: text/html
+  // and content from the file content/test/data/file.html
   std::unique_ptr<net::test_server::HttpResponse> HandleRequest(
       const net::test_server::HttpRequest& request) {
     std::unique_ptr<net::test_server::BasicHttpResponse> response(
         new net::test_server::BasicHttpResponse());
 
     std::string value;
     if (net::GetValueForKeyInQuery(request.GetURL(), "header", &value))
       response->AddCustomHeader("Clear-Site-Data", value);
 
     if (net::GetValueForKeyInQuery(request.GetURL(), "redirect", &value)) {
       response->set_code(net::HTTP_FOUND);
       response->AddCustomHeader("Location", value);
     } else {
       response->set_code(net::HTTP_OK);
     }
 
+    if (net::GetValueForKeyInQuery(request.GetURL(), "html", &value)) {
+      response->set_content_type("text/html");
+      response->set_content(value);
+
+      // The "html" parameter is telling the server what to serve, and the XSS
+      // auditor will complain if its |value| contains JS code. Disable that
+      // protection.
+      response->AddCustomHeader("X-XSS-Protection", "0");
+    }
+
+    if (net::GetValueForKeyInQuery(request.GetURL(), "file", &value)) {
+      base::FilePath path(GetTestFilePath("browsing_data", value.c_str()));
+      base::File file(path, base::File::FLAG_OPEN | base::File::FLAG_READ);
+      EXPECT_TRUE(file.IsValid());
+      int64_t length = file.GetLength();
+      EXPECT_GE(length, 0);
+      std::unique_ptr<char[]> buffer(new char[length + 1]);
+      file.Read(0, buffer.get(), length);
+      buffer[length] = '\0';
+
+      if (path.Extension() == FILE_PATH_LITERAL(".js"))
+        response->set_content_type("application/javascript");
+      else if (path.Extension() == FILE_PATH_LITERAL(".html"))
+        response->set_content_type("text/html");
+      else
+        NOTREACHED();
+
+      response->set_content(buffer.get());
+    }
+
     return std::move(response);
   }
 
-  TestContentBrowserClient test_client_;
+  // Callback handler for AddCookie().
+  void AddCookieCallback(base::WaitableEvent* waitable_event, bool success) {

[mmenke, 2017/05/25 19:23:10]

static?

[msramek, 2017/05/30 21:58:45]

Done.

+    DCHECK_CURRENTLY_ON(BrowserThread::IO);
+    ASSERT_TRUE(success);
+    waitable_event->Signal();
+  }
+
+  // Callback handler for GetCookies().
+  void GetCookiesCallback(base::WaitableEvent* waitable_event,

[mmenke, 2017/05/25 19:23:09]

static?

[msramek, 2017/05/30 21:58:45]

Done.

+                          net::CookieList* out_cookie_list,
+                          const net::CookieList& cookie_list) {
+    DCHECK_CURRENTLY_ON(BrowserThread::IO);
+    *out_cookie_list = cookie_list;
+    waitable_event->Signal();
+  }
+
+  // Callback handler for AddServiceWorker().
+  void AddServiceWorkerCallback(base::RunLoop* run_loop, bool success) {
+    ASSERT_TRUE(success);
+    run_loop->Quit();
+  }
+
+  // Callback handler for GetServiceWorkers().
+  void GetServiceWorkersCallback(
+      base::WaitableEvent* waitable_event,
+      std::vector<ServiceWorkerUsageInfo>* out_service_workers,
+      const std::vector<ServiceWorkerUsageInfo>& service_workers) {
+    *out_service_workers = service_workers;
+    waitable_event->Signal();
+  }
+
   std::unique_ptr<net::EmbeddedTestServer> https_server_;
+  TestBrowsingDataRemoverDelegate embedder_delegate_;
+
+  net::CookieStore* cookie_store_;
 };
 
 // Tests that the header is recognized on the beginning, in the middle, and on
-// the end of a redirect chain. Each of the three parts of the chain may or
-// may not send the header, so there are 8 configurations to test.
-IN_PROC_BROWSER_TEST_F(ClearSiteDataThrottleBrowserTest, Redirect) {
+// the end of a navigation redirect chain. Each of the three parts of the chain
+// may or may not send the header, so there are 8 configurations to test.
+IN_PROC_BROWSER_TEST_F(ClearSiteDataThrottleBrowserTest, RedirectNavigation) {
   GURL base_urls[3] = {
       https_server()->GetURL("origin1.com", "/"),
       https_server()->GetURL("origin2.com", "/foo/bar"),
       https_server()->GetURL("origin3.com", "/index.html"),
   };
 
   // Iterate through the configurations. URLs whose index is matched by the mask
   // will send the header, the others won't.
   for (int mask = 0; mask < (1 << 3); ++mask) {
     GURL urls[3];
 
     // Set up the expectations.
     for (int i = 0; i < 3; ++i) {
       urls[i] = base_urls[i];
       if (mask & (1 << i))
         AddQuery(&urls[i], "header", kClearCookiesHeader);
 
-      EXPECT_CALL(*GetContentBrowserClient(),
-                  ClearSiteData(shell()->web_contents()->GetBrowserContext(),
-                                url::Origin(urls[i]), _, _, _, _))
-          .Times((mask & (1 << i)) ? 1 : 0);
+      if (mask & (1 << i))
+        delegate()->ExpectClearSiteDataCookiesCall(url::Origin(urls[i]));
     }
 
     // Set up redirects between urls 0 --> 1 --> 2.
     AddQuery(&urls[1], "redirect", urls[2].spec());
     AddQuery(&urls[0], "redirect", urls[1].spec());
 
     // Navigate to the first url of the redirect chain.
     NavigateToURL(shell(), urls[0]);
 
     // We reached the end of the redirect chain.
     EXPECT_EQ(urls[2], shell()->web_contents()->GetURL());
 
-    testing::Mock::VerifyAndClearExpectations(GetContentBrowserClient());
+    delegate()->VerifyAndClearExpectations();
   }
 }
 
+// Tests that the header is recognized on the beginning, in the middle, and on
+// the end of a resource load redirect chain. Each of the three parts of the
+// chain may or may not send the header, so there are 8 configurations to test.
+IN_PROC_BROWSER_TEST_F(ClearSiteDataThrottleBrowserTest, RedirectResourceLoad) {
+  GURL base_urls[3] = {

[mmenke, 2017/05/25 19:23:09]

These don't mean the same thing as https_server()->base_url(), so perhaps
another name?  Do we even need different paths for these?  Without paths, they
are analogous to base_url, actually.

[msramek, 2017/05/30 21:58:45]

I added the paths here partially to document the meaning of the three URLs
(redirect chain start, middle, and end), and partially to test that they don't
matter (i.e. the header is respected from any path on the origin).

Let me rename them to make it clearer.

I also renamed "base_urls" to "page_urls/resource_urls". Maybe there's a better
name, but I want to convey that these URLs represent pages we navigate to /
resources that we fetch, and then below we add the query parameter to adjust
their behavior.

+      https_server()->GetURL("origin1.com", "/image.png"),
+      https_server()->GetURL("origin2.com", "/redirected-image.png"),
+      https_server()->GetURL("origin3.com", "/actual-image.png"),

[mmenke, 2017/05/25 19:23:09]

Worth noting that actual-image actually just ends up being an HTTP response with
an empty body and no content-type, instead of an actual image?

[msramek, 2017/05/30 21:58:45]

As mentioned in the above comment, I renamed the paths to avoid confusion.

+  };
+
+  // Iterate through the configurations. URLs whose index is matched by the mask
+  // will send the header, the others won't.
+  for (int mask = 0; mask < (1 << 3); ++mask) {
+    GURL urls[3];
+
+    // Set up the expectations.
+    for (int i = 0; i < 3; ++i) {
+      urls[i] = base_urls[i];
+      if (mask & (1 << i))
+        AddQuery(&urls[i], "header", kClearCookiesHeader);
+
+      if (mask & (1 << i))
+        delegate()->ExpectClearSiteDataCookiesCall(url::Origin(urls[i]));
+    }
+
+    // Set up redirects between urls 0 --> 1 --> 2.
+    AddQuery(&urls[1], "redirect", urls[2].spec());
+    AddQuery(&urls[0], "redirect", urls[1].spec());
+
+    // Navigate to a page that embeds "https://origin1.com/image.png"
+    // and observe the loading of that resource.
+    GURL page_with_image = https_server()->GetURL("origin4.com", "/index.html");
+    std::string content_with_image =
+        "<html><head></head><body>"
+        "<img src=\"" +
+        urls[0].spec() +
+        "\" />"
+        "</body></html>";
+    AddQuery(&page_with_image, "html", content_with_image);
+    NavigateToURL(shell(), page_with_image);
+
+    delegate()->VerifyAndClearExpectations();
+  }
+}
+
 // Tests that the Clear-Site-Data header is ignored for insecure origins.
-IN_PROC_BROWSER_TEST_F(ClearSiteDataThrottleBrowserTest, Insecure) {
+IN_PROC_BROWSER_TEST_F(ClearSiteDataThrottleBrowserTest, InsecureNavigation) {
   // ClearSiteData() should not be called on HTTP.
   GURL url = embedded_test_server()->GetURL("example.com", "/");
   AddQuery(&url, "header", kClearCookiesHeader);
   ASSERT_FALSE(url.SchemeIsCryptographic());
 
-  EXPECT_CALL(*GetContentBrowserClient(), ClearSiteData(_, _, _, _, _, _))
-      .Times(0);
-
   NavigateToURL(shell(), url);
+
+  // We do not expect any calls to have been made.
+  delegate()->VerifyAndClearExpectations();
+}
+
+// Tests that the Clear-Site-Data header is honored for secure resource loads
+// and ignored for insecure ones.

[mmenke, 2017/05/25 19:23:10]

This doesn't check the header is ignored for insecure resource loads, does it? 
Just secure resource loads from secure and insecure pages.  Should it (Or
another test) check for insecure resource loads, instead of just checking for
insecure navigations?  Could add an insecure resource to the above test pretty
easily.

[msramek, 2017/05/30 21:58:45]

It does? This test tries all four combinations - insecure/secure resource on
insecure/secure navigation.

[mmenke, 2017/05/30 22:58:35]

Hrm...not sure what I was thinking.  I'll take another look tomorrow, but unless
I comment again, consider it resolved.

+IN_PROC_BROWSER_TEST_F(ClearSiteDataThrottleBrowserTest,
+                       SecureAndInsecureResourceLoad) {
+  GURL insecure_image =
+      embedded_test_server()->GetURL("example.com", "/image.png");
+  GURL secure_image = https_server()->GetURL("example.com", "/image.png");
+
+  ASSERT_TRUE(secure_image.SchemeIsCryptographic());
+  ASSERT_FALSE(insecure_image.SchemeIsCryptographic());
+
+  AddQuery(&secure_image, "header", kClearCookiesHeader);
+  AddQuery(&insecure_image, "header", kClearCookiesHeader);
+
+  std::string content_with_insecure_image =
+      "<html><head></head><body>"
+      "<img src=\"" +
+      insecure_image.spec() +
+      "\" />"
+      "</body></html>";
+
+  std::string content_with_secure_image =
+      "<html><head></head><body>"
+      "<img src=\"" +
+      secure_image.spec() +
+      "\" />"
+      "</body></html>";
+
+  // Test insecure resources.
+  GURL insecure_page = embedded_test_server()->GetURL("example.com", "/");
+  GURL secure_page = https_server()->GetURL("example.com", "/");
+
+  AddQuery(&insecure_page, "html", content_with_insecure_image);
+  AddQuery(&secure_page, "html", content_with_insecure_image);
+
+  // Insecure resource on an insecure page does not execute Clear-Site-Data.
+  NavigateToURL(shell(), insecure_page);
+
+  // Insecure resource on a secure page does not execute Clear-Site-Data.
+  NavigateToURL(shell(), secure_page);
+
+  // We do not expect any calls to have been made.
+  delegate()->VerifyAndClearExpectations();
+
+  // Test secure resources.
+  insecure_page = embedded_test_server()->GetURL("example.com", "/");
+  secure_page = https_server()->GetURL("example.com", "/");
+
+  AddQuery(&insecure_page, "html", content_with_secure_image);
+  AddQuery(&secure_page, "html", content_with_secure_image);
+
+  // Secure resource on an insecure page does execute Clear-Site-Data.
+  delegate()->ExpectClearSiteDataCookiesCall(url::Origin(secure_image));
+
+  NavigateToURL(shell(), secure_page);
+  delegate()->VerifyAndClearExpectations();
+
+  // Secure resource on a secure page does execute Clear-Site-Data.
+  delegate()->ExpectClearSiteDataCookiesCall(url::Origin(secure_image));
+
+  NavigateToURL(shell(), secure_page);
+  delegate()->VerifyAndClearExpectations();
+}
+
+// Tests that the Clear-Site-Data header is ignored for service worker resource
+// loads.
+IN_PROC_BROWSER_TEST_F(ClearSiteDataThrottleBrowserTest, ServiceWorker) {
+  GURL origin1 = https_server()->GetURL("origin1.com", "/");
+  GURL origin2 = https_server()->GetURL("origin2.com", "/");
+  GURL origin3 = https_server()->GetURL("origin3.com", "/");
+  GURL origin4 = https_server()->GetURL("origin4.com", "/");
+
+  // Navigation to worker_setup.html will install a service worker. Since
+  // the installation is asynchronous, the JS side will inform us about it in
+  // the page title.
+  GURL url = origin1;
+  AddQuery(&url, "file", "worker_setup.html");
+  NavigateToURL(shell(), url);
+  WaitForTitle(shell(), "service worker is ready");
+
+  // The service worker will now serve a page containing several images, which
+  // the browser will try to fetch. The service worker will be instructed
+  // to handle some of the fetches itself, while others will be handled by
+  // the testing server. The setup is the following:
+  //
+  // origin1.com/resource         (-> server; should respect header)
+  // origin1.com/resource_from_sw (-> service worker; should not respect header)
+  // origin2.com/resource_from_sw (-> service worker; should not respect header)
+  // origin2.com/resource         (-> server; should respect header)
+  // origin3.com/resource_from_sw (-> service worker; should not respect header)
+  // origin3.com/resource_from_sw (-> service worker; should not respect header)
+  // origin4.com/resource         (-> server; should respect header)
+  // origin4.com/resource         (-> server; should respect header)

[mmenke, 2017/05/25 19:23:09]

Out of paranoia, and since we check ordering, would it make sense to reorder
these as:

origin1.com/resource
origin2.com/resource_from_sw
origin3.com/resource_from_sw
origin4.com/resource
origin1.com/resource_from_sw
origin2.com/resource
origin3.com/resource_from_sw
origin4.com/resource

This may make the test slightly more robust, as we're checking the order in
which ClearSiteDataCookies is called - I can't see how it would happen, but it
would be slightly more robust in the case we somehow managed to flip which of
the two requests to an origin cleared the cookie.

Anyhow, even if we ran into that bug, this test should still catch it, just
being paranoid.

[msramek, 2017/05/30 21:58:44]

Done. That's fine, I like paranoid tests :)

Interestingly, changing the order of the non-SW fetches in worker.js to 1-4-2-4
resulted in the EmbeddedTestServer seeing requests 1-4-4-2.

My assumption was that the last fetch to origin4 was sped up by the fact that
there was already a previous connection to origin4? Preconnect or something like
that?

In any case, this is unrelated to Clear-Site-Data, so I just changed worker.js
to fetch the files sequentially.

[mmenke, 2017/05/31 16:25:33]

I didn't realize it wasn't fetching sequentially...Yea, that seems like a really
good idea.

+  //
+  // |origin1| and |origin2| are used to test that there is no difference
+  // between same-origin and third-party fetches. Clear-Site-Data should be
+  // called once for each of these origins - caused by the "/resource" fetch,
+  // but not by the "/resource_from_sw" fetch. |origin3| and |origin4| prove
+  // that the number of calls is dependent on the number of network responses,
+  // i.e. that it isn't always 1 as in the case of |origin1| and |origin2|.
+  delegate()->ExpectClearSiteDataCookiesCall(url::Origin(origin1));
+  delegate()->ExpectClearSiteDataCookiesCall(url::Origin(origin2));
+  delegate()->ExpectClearSiteDataCookiesCall(url::Origin(origin4));
+  delegate()->ExpectClearSiteDataCookiesCall(url::Origin(origin4));
+
+  url = https_server()->GetURL("origin1.com", "/anything-in-workers-scope");
+  AddQuery(&url, "origin1", origin1.spec());
+  AddQuery(&url, "origin2", origin2.spec());
+  AddQuery(&url, "origin3", origin3.spec());
+  AddQuery(&url, "origin4", origin4.spec());
+  NavigateToURL(shell(), url);
+  WaitForTitle(shell(), "done");
+  delegate()->VerifyAndClearExpectations();
+}
+
+// Tests that Clear-Site-Data is only executed on a resource fetch
+// if credentials are allowed in that fetch.
+IN_PROC_BROWSER_TEST_F(ClearSiteDataThrottleBrowserTest, Credentials) {
+  GURL page_template = https_server()->GetURL("origin1.com", "/");
+  GURL same_origin_resource =
+      https_server()->GetURL("origin1.com", "/resource");
+  GURL different_origin_resource =
+      https_server()->GetURL("origin2.com", "/resource");
+
+  AddQuery(&same_origin_resource, "header", kClearCookiesHeader);
+  AddQuery(&different_origin_resource, "header", kClearCookiesHeader);
+
+  const struct TestCase {
+    bool same_origin;
+    std::string credentials;
+    bool should_run;
+  } kTestCases[] = {
+      {true, "", false},
+      {true, "omit", false},
+      {true, "same-origin", true},
+      {true, "include", true},
+      {false, "", false},
+      {false, "omit", false},
+      {false, "same-origin", false},
+      {false, "include", true},
+  };
+
+  for (const TestCase& test_case : kTestCases) {
+    const GURL& resource = test_case.same_origin ? same_origin_resource
+                                                 : different_origin_resource;
+    std::string credentials =
+        test_case.credentials.empty()
+            ? ""
+            : "credentials: '" + test_case.credentials + "'";
+
+    // Fetch a resource. Note that the script also handles fetch() error which
+    // might be thrown for third-party fetches because of missing
+    // Access-Control-Allow-Origin. However, that only affects the visibility
+    // of the response; the header will still be processed.
+    std::string content = base::StringPrintf(
+        "<html><head></head><body><script>"
+        "fetch('%s', {%s})"
+        ".then(function() { document.title = 'done'; })"
+        ".catch(function() { document.title = 'done'; })"
+        "</script></body></html>",
+        resource.spec().c_str(), credentials.c_str());
+
+    GURL page = page_template;
+    AddQuery(&page, "html", content);
+
+    if (test_case.should_run)
+      delegate()->ExpectClearSiteDataCookiesCall(url::Origin(resource));
+
+    NavigateToURL(shell(), page);
+    WaitForTitle(shell(), "done");
+    delegate()->VerifyAndClearExpectations();
+  }
+}
+
+// Tests that the credentials flag is correctly taken into account when it
+// interpretation changes after redirect.
+IN_PROC_BROWSER_TEST_F(ClearSiteDataThrottleBrowserTest,
+                       CredentialsOnRedirect) {
+  GURL urls[2] = {
+      https_server()->GetURL("origin1.com", "/image.png"),
+      https_server()->GetURL("origin2.com", "/image.png"),
+  };
+
+  AddQuery(&urls[0], "header", kClearCookiesHeader);
+  AddQuery(&urls[1], "header", kClearCookiesHeader);
+
+  AddQuery(&urls[0], "redirect", urls[1].spec());
+
+  // Fetch a resource on origin1.com, which will redirect to origin2.com.
+  // Both URLs will respond with Clear-Site-Data. Since the credentials mode is
+  // 'same-origin', the LOAD_DO_NOT_SAVE_COOKIES flag will be set while
+  // processing the response from origin1.com, but not while processing the
+  // response from origin2.com. Therefore, the deletion will only be executed
+  // for origin1.com.
+  //
+  // Note that the script also handles fetch() error which might be thrown for
+  // third-party fetches because of missing Access-Control-Allow-Origin.
+  // However, that only affects the visibility of the response; the header will
+  // still be processed.
+  std::string content = base::StringPrintf(
+      "<html><head></head><body><script>"
+      "fetch('%s', {'credentials': 'same-origin'})"
+      ".then(function() { document.title = 'done'; })"
+      ".catch(function() { document.title = 'done'; })"
+      "</script></body></html>",
+      urls[0].spec().c_str());
+
+  delegate()->ExpectClearSiteDataCookiesCall(url::Origin(urls[0]));
+
+  GURL page = https_server()->GetURL("origin1.com", "/");
+  AddQuery(&page, "html", content);
+
+  NavigateToURL(shell(), page);
+  WaitForTitle(shell(), "done");
+  delegate()->VerifyAndClearExpectations();
 }
 
 // Tests that ClearSiteData() is called for the correct datatypes.
 IN_PROC_BROWSER_TEST_F(ClearSiteDataThrottleBrowserTest, Types) {
   GURL base_url = https_server()->GetURL("example.com", "/");
 
   struct TestCase {
     const char* value;
     bool remove_cookies;
     bool remove_storage;
     bool remove_cache;
   } test_cases[] = {
       {"{ \"types\": [ \"cookies\" ] }", true, false, false},
       {"{ \"types\": [ \"storage\" ] }", false, true, false},
       {"{ \"types\": [ \"cache\" ] }", false, false, true},
       {"{ \"types\": [ \"cookies\", \"storage\" ] }", true, true, false},
       {"{ \"types\": [ \"cookies\", \"cache\" ] }", true, false, true},
       {"{ \"types\": [ \"storage\", \"cache\" ] }", false, true, true},
       {"{ \"types\": [ \"cookies\", \"storage\", \"cache\" ] }", true, true,
        true},
   };
 
   for (const TestCase& test_case : test_cases) {
     GURL url = base_url;
     AddQuery(&url, "header", test_case.value);
 
-    EXPECT_CALL(
-        *GetContentBrowserClient(),
-        ClearSiteData(shell()->web_contents()->GetBrowserContext(),
-                      url::Origin(url), test_case.remove_cookies,
-                      test_case.remove_storage, test_case.remove_cache, _));
+    delegate()->ExpectClearSiteDataCall(
+        url::Origin(url), test_case.remove_cookies, test_case.remove_storage,
+        test_case.remove_cache);
 
     NavigateToURL(shell(), url);
 
-    testing::Mock::VerifyAndClearExpectations(GetContentBrowserClient());
+    delegate()->VerifyAndClearExpectations();
   }
 }
 
+// Integration test for the deletion of cookies.
+IN_PROC_BROWSER_TEST_F(ClearSiteDataThrottleBrowserTest,
+                       CookiesIntegrationTest) {
+  AddCookie(https_server()->GetURL("origin1.com", "/abc"));
+  AddCookie(https_server()->GetURL("subdomain.origin1.com", "/"));
+  AddCookie(https_server()->GetURL("origin2.com", "/def"));
+  AddCookie(https_server()->GetURL("subdomain.origin2.com", "/"));
+
+  // There are four cookies on two eTLD+1s.
+  net::CookieList cookies = GetCookies();
+  EXPECT_EQ(4u, cookies.size());
+
+  // Let Clear-Site-Data delete the "cookies" of "origin1.com".
+  GURL url = https_server()->GetURL("origin1.com", "/clear-site-data");
+  AddQuery(&url, "header", kClearCookiesHeader);
+  NavigateToURL(shell(), url);
+
+  // Only the "origin2.com" eTLD now has cookies.
+  cookies = GetCookies();
+  EXPECT_EQ(2u, cookies.size());

[mmenke, 2017/05/25 19:23:10]

ASSERT_EQ?

[msramek, 2017/05/30 21:58:45]

Done.

+  EXPECT_EQ(cookies[1].Domain(), "subdomain.origin2.com");
+  EXPECT_EQ(cookies[0].Domain(), "origin2.com");

[mmenke, 2017/05/25 19:23:10]

nit:  Why the flipped order?

[msramek, 2017/05/30 21:58:45]

Done. Flipped, no reason :)

+}
+
+// Integration test for the unregistering of service workers.
+IN_PROC_BROWSER_TEST_F(ClearSiteDataThrottleBrowserTest,
+                       StorageServiceWorkersIntegrationTest) {
+  AddServiceWorker("origin1.com");
+  AddServiceWorker("origin2.com");
+
+  // There are two service workers installed on two origins.
+  std::vector<ServiceWorkerUsageInfo> service_workers = GetServiceWorkers();
+  EXPECT_EQ(2u, service_workers.size());
+
+  // Let Clear-Site-Data delete the "storage" of "origin1.com".
+  GURL url = https_server()->GetURL("origin1.com", "/clear-site-data");

[mmenke, 2017/05/25 19:23:09]

Why doesn't origin1's SW intercept this and return the default response?

[msramek, 2017/05/30 21:58:45]

That's a great question :) Apparently I confused the first parameter to
ServiceWorkerContextWrapper::RegisterServiceWorker(), which was supposed to be
the scope, with the URL that registered it.

Fixed.

I also added an observer to wait until the worker is actually activated, and
expanded the test to verify that navigation inside the scope will not delete it,
but outside will. This is practically already covered by the other test which
checks service worker fetches, but still... a nice addition :)

+  AddQuery(&url, "header", "{ \"types\": [ \"storage\" ] }");
+  NavigateToURL(shell(), url);
+
+  // Only "origin2.com" now has a service worker.
+  service_workers = GetServiceWorkers();
+  EXPECT_EQ(1u, service_workers.size());

[mmenke, 2017/05/25 19:23:10]

ASSERT_EQ?

[msramek, 2017/05/30 21:58:45]

Done.

+  EXPECT_EQ(service_workers[0].origin,
+            https_server()->GetURL("origin2.com", "/"));
+}
+
+// TODO(msramek): Add integration tests for other storage datatypes, such as
+// local storage, indexed DB, etc.
+
+// Integration test for the deletion of cache entries.
+// NOTE: This test might be flaky. disk_cache::Backend calls back before cache
+// entries are actually written to the disk. Other tests using CacheTestUtil
+// show that a timeout of around 1s between cache operations is necessary to
+// avoid flakiness.
+IN_PROC_BROWSER_TEST_F(ClearSiteDataThrottleBrowserTest, CacheIntegrationTest) {
+  const int kTimeoutMs = 1000;
+
+  CacheTestUtil util(
+      BrowserContext::GetDefaultStoragePartition(browser_context()));
+  std::string url1 = https_server()->GetURL("origin1.com", "/foo").spec();
+  std::string url2 = https_server()->GetURL("origin1.com", "/bar").spec();
+  std::string url3 = https_server()->GetURL("origin2.com", "/foo").spec();
+  std::string url4 = https_server()->GetURL("origin2.com", "/bar").spec();
+
+  std::set<std::string> entries_to_create = {url1, url2, url3, url4};
+  util.CreateCacheEntries(entries_to_create);
+  base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(kTimeoutMs));
+
+  // There are four cache entries on two origins.
+  std::vector<std::string> cache_keys = util.GetEntryKeys();
+  EXPECT_EQ(4u, cache_keys.size());
+
+  // Let Clear-Site-Data delete the "cache" of "origin1.com".
+  GURL url = https_server()->GetURL("origin1.com", "/clear-site-data");
+  AddQuery(&url, "header", "{ \"types\": [ \"cache\" ] }");
+  NavigateToURL(shell(), url);
+  base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(kTimeoutMs));
+
+  // Only "origin2.com" now has cache entries.
+  cache_keys = util.GetEntryKeys();
+  EXPECT_EQ(2u, cache_keys.size());
+  std::sort(cache_keys.begin(), cache_keys.end());
+  EXPECT_EQ(url4, cache_keys[0]);
+  EXPECT_EQ(url3, cache_keys[1]);
+}
+
 }  // namespace content