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 c279e1a2163b629b07dd3f3f503df06109cc352d..2aeb4cb7e679ae0e1c39c8aab604e48b709df56d 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"
@@ -2492,6 +2495,66 @@ 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));
+
+  // Cookies and channel IDs are scoped to eTLD+1 and should be deleted
+  // in this scope.
+  if (remove_storage || remove_cache) {

[Mike West, 2016/06/02 07:00:08]

This should be `remove_cookies`, right?

Actually, I don't understand the logic here. Why do you check for cookies and
storage inside this `if`? What do you intend for this block to do, and what do
you intend the next to take care of?

[msramek, 2016/06/14 20:12:07]

Sorry, I've been rewriting this a few times, apparently I messed up the
condition again.

This block is for eTLD+1-scoped deletion. We do that for cookies (which are
included in remove_cookies) and for channel IDs (which are part of storage), so
the condition should be (remove_cookies || remove_storage). But we need to check
again inside the block, because it could be only one of them.

+    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;
+
+    int remove_mask = 0;
+    if (remove_cookies)
+      remove_mask |= BrowsingDataRemover::REMOVE_COOKIES;
+    if (remove_storage)
+      remove_mask |= BrowsingDataRemover::REMOVE_CHANNEL_IDS;

[Mike West, 2016/06/02 07:00:08]

1.  Can you file a bug against the spec (or send a PR? :) ) to note that we
treat channel ID/token binding like cookies?
2.  Why don't you actually treat them like cookies here? Why tie them to storage
and not cookies?

[msramek, 2016/06/14 20:12:07]

BrowsingDataRemover treats them as a part of site data, so I think they fit more
to the storage. If we're going to tie them to cookies, I would prefer to do that
in our backend as well. I'll have to find out if it makes sense at first though.

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

Cookies are part of site data too. :)
I think we need to tie them to cookies, as that's how they're used. For
instance, Google uses channel ID to bind cookies to an origin (see
http://www.browserauth.net/channel-bound-cookies for an old, but relevant
description). If we cleared the ID, we'd also invalidate the cookies. My
understanding is that this is pretty much the entire point of channel ID.
I think that's worth doing.

+
+    RegistrableDomainFilterBuilder domain_filter_builder(
+        BrowsingDataFilterBuilder::WHITELIST);
+    domain_filter_builder.AddRegisterableDomain(domain);
+
+    remover->RemoveWithFilter(

[Mike West, 2016/06/02 07:00:08]

If folks want to clear both cookies and DOM storage, we're going to end up
calling into BDR twice. That's hugely expensive...

[msramek, 2016/06/14 20:12:07]

Not necessarily. BDR is full of if()-s that are only executed if the removal
mask contains given datatypes. So we wont end up doing more deletion tasks, or
more thread-jumping etc., just more overhead with the method calls and
initializing the filters which shouldn't be that bad?

+        BrowsingDataRemover::Period(
+            BrowsingDataRemover::TimePeriod::EVERYTHING),
+        remove_mask,
+        BrowsingDataHelper::UNPROTECTED_WEB,

[Mike West, 2016/06/02 07:00:08]

Hrm. I forget what we actually used PROTECTED_WEB for now; have we killed off
hosted apps? If not, how does this interact with them?

[msramek, 2016/06/14 20:12:07]

Heh, good point. In my mind, I am still mapping this to the checkboxes "cookies
and site data" and "cache". The former deletes only UNPROTECTED_WEB. The "hosted
apps" checkbox then does the same for PROTECTED_WEB.

But the header has no reason to respect this distinction. If a website requests
clearing, then we should simply clear the data, period.

+        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(
+        BrowsingDataRemover::Period(
+            BrowsingDataRemover::TimePeriod::EVERYTHING),
+        remove_mask,
+        BrowsingDataHelper::UNPROTECTED_WEB,
+        origin_filter_builder);
+  }
+}
+
 base::FilePath ChromeContentBrowserClient::GetDefaultDownloadDirectory() {
   return DownloadPrefs::GetDefaultDownloadDirectory();
 }