First experimental implementation of the Clear-Site-Data header

chrome/browser/chrome_content_browser_client.cc

Index: chrome/browser/chrome_content_browser_client.cc
diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc
index bb2aa36451ff92f62c614f1c5d4a24840ce6a60e..1680243d885b0879a09ad3d8a548d31248d93333 100644
--- a/chrome/browser/chrome_content_browser_client.cc
+++ b/chrome/browser/chrome_content_browser_client.cc
@@ -35,6 +35,8 @@
 #include "chrome/browser/browsing_data/browsing_data_helper.h"
 #include "chrome/browser/browsing_data/browsing_data_remover.h"
 #include "chrome/browser/browsing_data/browsing_data_remover_factory.h"
+#include "chrome/browser/browsing_data/origin_filter_builder.h"
+#include "chrome/browser/browsing_data/registrable_domain_filter_builder.h"
 #include "chrome/browser/character_encoding.h"
 #include "chrome/browser/chrome_content_browser_client_parts.h"
 #include "chrome/browser/chrome_net_benchmarking_message_filter.h"
@@ -164,6 +166,7 @@
 #include "device/usb/public/interfaces/device_manager.mojom.h"
 #include "gin/v8_initializer.h"
 #include "net/base/mime_util.h"
+#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 #include "net/cookies/canonical_cookie.h"
 #include "net/cookies/cookie_options.h"
 #include "net/ssl/ssl_cert_request_info.h"
@@ -2438,6 +2441,71 @@ void ChromeContentBrowserClient::ClearCookies(RenderFrameHost* rfh) {
                   BrowsingDataHelper::UNPROTECTED_WEB);
 }
 
+void ChromeContentBrowserClient::ClearSiteData(
+    content::BrowserContext* browser_context, const url::Origin& origin,
+    bool remove_cookies, bool remove_storage, bool remove_cache) {
+  BrowsingDataRemover* remover =
+      BrowsingDataRemoverFactory::GetForBrowserContext(
+          Profile::FromBrowserContext(browser_context));
+
+  int origin_type_mask =
+      BrowsingDataHelper::UNPROTECTED_WEB |
+      BrowsingDataHelper::PROTECTED_WEB |
+      BrowsingDataHelper::EXTENSION;
+
+  // Cookies and channel IDs are scoped to eTLD+1 and should be deleted
+  // in this scope.
+  if (remove_cookies || remove_storage) {

[Mike West, 2016/06/20 07:57:37]

Nit: I think this might end up simpler to understand if you drop this outer if,
and do something like:

```
std::string domain = ...;
int remove_mask = 0;
if (remove_cookies)
  remove_mask |= ...;
if (remove_storage)
  remove_mask |= ...;
if (remove_mask && !domain.empty()) {
  RegistrableDomainFilterBuilder(...);
  ...
}
```

[msramek, 2016/07/15 16:47:39]

Done.

+    std::string domain = GetDomainAndRegistry(
+        origin.host(),
+        net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES);
+
+    // TODO(msramek): What about origins such as http://localhost:80?
+    if (domain.empty())
+      return;

[Mike West, 2016/06/20 07:57:37]

Returning here is probably wrong. We can still clear the origin data below even
if we can't get a registry here.

[msramek, 2016/07/15 16:47:39]

Good catch :)

Fortunately, this is not needed anymore as we're handling localhost et al.
correctly after https://codereview.chromium.org/2127603002/ .

+
+    int remove_mask = 0;
+    if (remove_cookies)
+      remove_mask |= BrowsingDataRemover::REMOVE_COOKIES;
+    if (remove_storage)
+      remove_mask |= BrowsingDataRemover::REMOVE_CHANNEL_IDS;
+
+    RegistrableDomainFilterBuilder domain_filter_builder(
+        BrowsingDataFilterBuilder::WHITELIST);
+    domain_filter_builder.AddRegisterableDomain(domain);
+
+    remover->RemoveWithFilter(
+        BrowsingDataRemover::Period(
+            BrowsingDataRemover::TimePeriod::EVERYTHING),
+        remove_mask,
+        origin_type_mask,
+        domain_filter_builder);
+  }
+
+  // Delete origin-scoped data.
+  if (remove_storage || remove_cache) {
+    OriginFilterBuilder origin_filter_builder(
+        BrowsingDataFilterBuilder::WHITELIST);
+    origin_filter_builder.AddOrigin(origin);
+
+    int remove_mask = 0;
+    if (remove_storage) {
+      remove_mask |= BrowsingDataRemover::REMOVE_SITE_DATA &
+          ~BrowsingDataRemover::REMOVE_COOKIES &
+          ~BrowsingDataRemover::REMOVE_CHANNEL_IDS;
+    }
+    if (remove_cache)
+      remove_mask |= BrowsingDataRemover::REMOVE_CACHE;
+
+    remover->RemoveWithFilter(

[Mike West, 2016/06/20 07:57:37]

It would be excellent to add some tests which verify that this method is being
called with the expected arguments.

[msramek, 2016/07/15 16:47:39]

I was going to say that two layers of testing should be enough, but given that I
made like 10 mistakes in this method already, that's a fair advice :)

Added unittests. This required making BrowsingDataFilters comparable, and
BrowsingDataRemover mockable.

+        BrowsingDataRemover::Period(
+            BrowsingDataRemover::TimePeriod::EVERYTHING),
+        remove_mask,
+        origin_type_mask,
+        origin_filter_builder);
+  }
+}
+
 base::FilePath ChromeContentBrowserClient::GetDefaultDownloadDirectory() {
   return DownloadPrefs::GetDefaultDownloadDirectory();
 }