First experimental implementation of the Clear-Site-Data header

chrome/browser/browsing_data/clear_site_data_throttle.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 "chrome/browser/browsing_data/clear_site_data_throttle.h"
+
+#include "base/bind.h"
+#include "base/callback.h"
+#include "base/values.h"
+#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/registrable_domain_filter_builder.h"
+#include "chrome/browser/profiles/profile.h"
+#include "components/safe_json/safe_json_parser.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/web_contents.h"
+#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
+#include "net/url_request/url_request.h"
+#include "url/gurl.h"
+
+namespace {
+
+const char* kNameForLogging = "ClearSiteDataThrottle";
+
+const char* kClearSiteDataHeader = "Clear-Site-Data";
+
+const char* kTypesKey = "types";
+
+}  // namespace
+
+ClearSiteDataThrottle::ClearSiteDataThrottle(const net::URLRequest* request)
+    : request_(request) {}
+
+ClearSiteDataThrottle::~ClearSiteDataThrottle() {}
+
+void ClearSiteDataThrottle::WillProcessResponse(bool* defer) {
+  std::string header;
+  request_->GetResponseHeaderByName(kClearSiteDataHeader, &header);
+  if (header.empty())
+    return;
+
+  std::string domain;
+  if (request_->url().HostIsIPAddress()) {
+    domain = request_->url().host();
+  } else {
+    domain = GetDomainAndRegistry(

[Mike West, 2016/05/31 05:35:51]

This is going to strip `www.example.com` down to `example.com`, isn't it? That
doesn't seem correct, if we want to be able to target subdomains (like
`plus.google.com` for clearing).

[msramek, 2016/06/01 14:20:40]

Done. Sorry, forgot to add a comment about this. I'm of course going to use
OriginFilterBuilder, just wanted to start out with
RegisterableDomainFilterBuilder since it supports more datatypes.

+        request_->url(),
+        net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES);
+  }
+
+  if (domain.empty())

[Mike West, 2016/05/31 05:35:51]

Another implication is that folks on an intranet wouldn't be able to use this
for their internal names; `https://fileserver/` or whatever should be clearable.

[msramek, 2016/06/01 14:20:40]

Yes, or even a simpler example: localhost. I think this is an oversight in the
design of RegisterableDomainFilterBuilder. I'll start a discussion about this in
an email.

+    return;
+
+  const content::ResourceRequestInfo* info =
+      content::ResourceRequestInfo::ForRequest(request_);
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::Bind(
+          &ClearSiteDataThrottle::ProcessResponseOnUIThread,
+          info->GetWebContentsGetterForRequest(),
+          domain,
+          header));
+}
+
+const char* ClearSiteDataThrottle::GetNameForLogging() const {
+  return kNameForLogging;
+}
+
+// static
+void ClearSiteDataThrottle::ProcessResponseOnUIThread(
+    const content::ResourceRequestInfo::WebContentsGetter& getter,
+    std::string domain,
+    std::string header) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  content::WebContents* web_contents = getter.Run();
+  if (!web_contents)
+    return;
+
+  Profile* profile =
+      Profile::FromBrowserContext(
+          web_contents->GetBrowserContext())->GetOriginalProfile();
+
+  safe_json::SafeJsonParser::Parse(

[Mike West, 2016/05/31 05:35:51]

Why do we need to parse JSON on the UI thread? Can we do this in
`WillProcessResponse` as well?

[msramek, 2016/06/01 14:20:41]

N/A, since after moving to content/ were using the simpler JSONReader.

Anyway, SafeJSONParser has its own thread anyway, so it shouldn't matter where
it's initiated from? I put it here to avoid having two asynchronous calls at the
same time to wait for.

+      header,
+      base::Bind(
+          &ClearSiteDataThrottle::ClearData,
+          base::Unretained(profile),
+          domain),
+      base::Bind(&ClearSiteDataThrottle::OnParseError));
+}
+
+// static
+void ClearSiteDataThrottle::ClearData(
+    Profile* profile,
+    std::string domain,
+    std::unique_ptr<base::Value> header) {
+  if (!header || !header->GetAsDictionary(nullptr)) {
+    DVLOG(1) << "Not a valid dictionary value.";

[Mike West, 2016/05/31 05:35:52]

// TODO(you): Write a console error.

[msramek, 2016/06/01 14:20:40]

Done. (actually implemented the console error)

+    return;
+  }
+
+  const base::ListValue* types;
+  if (!static_cast<base::DictionaryValue*>(header.get())
+          ->GetListWithoutPathExpansion(kTypesKey, &types)) {
+    DVLOG(1) << "No 'types' field present.";

[Mike West, 2016/05/31 05:35:51]

// TODO(you): Write a console error.

[msramek, 2016/06/01 14:20:40]

Done. (actually implemented the console error)

+    return;
+  }
+
+  int remove_mask = 0;
+  for (const base::Value* value : *types) {

[Mike West, 2016/05/31 05:35:51]

Nit: These are `unique_ptr` aren't they?

[msramek, 2016/06/01 14:20:40]

types->begin() is std::vector<base::Value*>::const_iterator, although I have
currently no idea why.

[msramek, 2016/06/01 14:25:30]

Ah. Because this is a recent change and I have an old checkout (with
base::Value*). I was reading the code in code search and could not comprehend
where is the pointer unwrapped.

Will rebase and fix the compilation error shortly...

+    std::string type;
+    if (!value->GetAsString(&type)) {
+      DVLOG(1) << "Invalid type: " << *value;

[Mike West, 2016/05/31 05:35:51]

// TODO(you): Write a console error.

[msramek, 2016/06/01 14:20:40]

Done. (actually implemented the console error)

+      continue;
+    }
+
+    if (type == "cache") {
+      remove_mask |= BrowsingDataRemover::REMOVE_CACHE;
+      DVLOG(2) << "Clearing cache.";
+    } else if (type == "cookies") {
+      remove_mask |= BrowsingDataRemover::REMOVE_COOKIES;
+      DVLOG(2) << "Clearing cookies.";
+    } else if (type == "storage") {
+      remove_mask |= BrowsingDataRemover::REMOVE_SITE_DATA;

[Mike West, 2016/05/31 05:35:51]

This includes cookies; it shouldn't. I think you'll probably need to do
something like `remove_mask |= BrowsingDataRemover::REMOVE_SITE_DATA ^
BrowsingDataRemover::REMOVE_COOKIES` to exclude them (or define a new constant).

[msramek, 2016/06/01 14:20:40]

Done. Thanks for paying attention to this :)

+      DVLOG(2) << "Clearing storage.";
+    }
+  }
+
+  if (!remove_mask) {
+    DVLOG(2) << "No valid types specified for clearing.";

[Mike West, 2016/05/31 05:35:51]

// TODO(you): Write a console error.

[msramek, 2016/06/01 14:20:41]

Done. (actually implemented the console error)

+    return;
+  }
+
+  BrowsingDataRemover* browsing_data_remover =
+      BrowsingDataRemoverFactory::GetForBrowserContext(profile);
+
+  RegistrableDomainFilterBuilder filter_builder(
+      BrowsingDataFilterBuilder::WHITELIST);
+  filter_builder.AddRegisterableDomain(domain);

[Mike West, 2016/05/31 05:35:51]

Again, we should talk about the registerable domain bits.

[msramek, 2016/06/01 14:20:40]

Please see my current implementation. I'm using origin-scoped deletion for every
data type that supports it. I'm using eTLD+1-scoped deletion for cookies and
channel IDs, in both cases it was an explicit request during the code review not
to support origins.

But as I mentioned earlier, I will send out an email to discuss some details.

+
+  browsing_data_remover->RemoveWithFilter(

[Mike West, 2016/05/31 05:35:51]

This eventually hops back to the IO thread, right? Can you add a comment to that
effect?

[msramek, 2016/06/01 14:20:40]

N/A, since we're not doing any more thread hopping after moving to content/.

+      BrowsingDataRemover::Period(BrowsingDataRemover::TimePeriod::EVERYTHING),
+          remove_mask,
+          BrowsingDataHelper::UNPROTECTED_WEB,
+          filter_builder);
+}
+
+// static
+void ClearSiteDataThrottle::OnParseError(const std::string& error) {
+  DVLOG(1) << "The header value is not a valid JSON.";
+  DVLOG(1) << error;

[Mike West, 2016/05/31 05:35:52]

// TODO(you): write a console error.

[msramek, 2016/06/01 14:20:41]

Done. (actually implemented the console error)

+}