Support the Clear-Site-Data header on resource requests

content/browser/browsing_data/clear_site_data_throttle.cc

Index: content/browser/browsing_data/clear_site_data_throttle.cc
diff --git a/content/browser/browsing_data/clear_site_data_throttle.cc b/content/browser/browsing_data/clear_site_data_throttle.cc
index c036a988df0c571e099a7db379e0994bf1e725e8..1baacd55311eb3b5c55a4bb52cd51e58f9f27a22 100644
--- a/content/browser/browsing_data/clear_site_data_throttle.cc
+++ b/content/browser/browsing_data/clear_site_data_throttle.cc
@@ -12,15 +12,18 @@
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/values.h"
-#include "content/browser/frame_host/navigation_handle_impl.h"
 #include "content/public/browser/browser_context.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
-#include "content/public/browser/navigation_handle.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/resource_controller.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/content_client.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/origin_util.h"
+#include "net/base/load_flags.h"
 #include "net/http/http_response_headers.h"
+#include "net/url_request/redirect_info.h"
 #include "url/gurl.h"
 #include "url/origin.h"
 
@@ -28,6 +31,8 @@ namespace content {
 
 namespace {
 
+static const char* kNameForLogging = "ClearSiteDataThrottle";
+
 static const char* kClearSiteDataHeader = "Clear-Site-Data";
 
 static const char* kTypesKey = "types";
@@ -58,71 +63,99 @@ int ParametersMask(bool clear_cookies, bool clear_storage, bool clear_cache) {
          static_cast<int>(clear_cache) * (1 << 2);
 }
 
+// A helper function to pass an IO thread callback to a method called on
+// the UI thread.
+void JumpFromUIToIOThread(const base::Closure& callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, callback);
+}
+
 }  // namespace
 
 // static
-std::unique_ptr<NavigationThrottle>
-ClearSiteDataThrottle::CreateThrottleForNavigation(NavigationHandle* handle) {
-  if (AreExperimentalFeaturesEnabled())
-    return base::WrapUnique(new ClearSiteDataThrottle(handle));
-
-  return std::unique_ptr<NavigationThrottle>();
+std::unique_ptr<ResourceThrottle>
+ClearSiteDataThrottle::CreateThrottleForRequest(net::URLRequest* request) {
+  // This is an experimental feature.
+  if (!AreExperimentalFeaturesEnabled())
+    return std::unique_ptr<ResourceThrottle>();
+
+  // The LOAD_DO_NOT_SAVE_COOKIES flag prohibits the request from doing any
+  // modification to cookies.
+  std::unique_ptr<base::Value> value = request->GetStateAsValue();
+  const base::DictionaryValue* state;
+  DCHECK(value->GetAsDictionary(&state));
+  int load_flags = 0;
+  bool success =
+      state->GetIntegerWithoutPathExpansion("load_flags", &load_flags);
+  DCHECK(success);
+  if (load_flags & net::LOAD_DO_NOT_SAVE_COOKIES)

[mmenke, 2016/09/26 12:59:16]

Using this load flag isn't reliable - NetworkDelegate has another method to
check if a request can save cookies, in addition to this load flag.  Not sure
the best way to handle this.  If this were only for cookies, I'd say this would
be best in URLRequestHttpJob, but it's not.  :(

[msramek, 2016/10/13 14:26:03]

Hmm... So I'm really not an expert on these things, but I think this check might
be enough. There are several things to consider w.r.t. cookies:

1. Chrome's internal pings to services which don't want to set cookies, and
those use LOAD_DO_NOT_SAVE_COOKIES.

2. As I mentioned in another comment, the 'credentials' parameter in fetch().
That one is actually also reflected as LOAD_DO_NOT_SAVE_COOKIES here.

3. Expiration timestamps. We do not have anything like that here.

4. Content settings - cookies could be blocked on a particular site or blocked
for third party in general. mkwst@ has argued that this should not affect us -
the intention of a user blocking cookies is that they do not want cookies, not
that they want to prevent cookies from being modified, therefore deletion should
be fine.

[mmenke, 2016/10/13 17:16:31]

I'll defer to you guys here.

+    return std::unique_ptr<ResourceThrottle>();
+
+  // The throttle has no purpose if the request has no ResourceRequestInfo,
+  // because we won't be able to determine whose data should be deleted.
+  if (!ResourceRequestInfo::ForRequest(request))
+    return std::unique_ptr<ResourceThrottle>();
+
+  // Service workers can handle fetches of third-party resources and inject
+  // arbitrary headers. Supporting Clear-Site-Data would give them the power
+  // to delete data from any website.
+  if (ResourceRequestInfo::OriginatedFromServiceWorker(request))

[mmenke, 2016/09/26 12:59:17]

This doesn't seem right.  We don't care if the request is from a ServiceWorker,
but if it's going to a service worker.

The way things work:  renderer->URLRequest (With possibly bogus
URL)->ServiceWorker->URLRequest (With real URL, assuming it doesn't go through a
second ServiceWorker).

We want to catch the first URLRequest, not the second, and I think this only
catches the second.

[msramek, 2016/10/13 14:26:02]

Thanks for the explanation.

From the name of the method, I wrongly assumed it means "originated from a URL
in the scope of a service worker".

But in the end, that's not what we want either; mkwst@ pointed out that we don't
want to completely disable the feature for service workers - it should work at
least for the update pings. I filed a bug for the spec here:
https://github.com/w3c/webappsec-clear-site-data/issues/5

It seems that ServiceWorkerUrlRequestJob inserts ServiceWorkerResponseInfo into
the responses it handles. So its existence should be the signal we're looking
for.

+    return std::unique_ptr<ResourceThrottle>();
+
+  return base::WrapUnique(new ClearSiteDataThrottle(request));
 }
 
-ClearSiteDataThrottle::ClearSiteDataThrottle(
-    NavigationHandle* navigation_handle)
-    : NavigationThrottle(navigation_handle),
+ClearSiteDataThrottle::ClearSiteDataThrottle(net::URLRequest* request)
+    : request_(request),
       clearing_in_progress_(false),
       weak_ptr_factory_(this) {}
 
 ClearSiteDataThrottle::~ClearSiteDataThrottle() {
-  // At the end of the navigation we finally have access to the correct
-  // RenderFrameHost. Output the cached console messages. Prefix each sequence
-  // of messages belonging to the same URL with |kConsoleMessagePrefix|.
-  GURL last_seen_url;
-  for (const ConsoleMessage& message : messages_) {
-    if (message.url == last_seen_url) {
-      navigation_handle()->GetRenderFrameHost()->AddMessageToConsole(
-          message.level, message.text);
-    } else {
-      navigation_handle()->GetRenderFrameHost()->AddMessageToConsole(
-          message.level,
-          base::StringPrintf(kConsoleMessagePrefix, message.url.spec().c_str(),
-                             message.text.c_str()));
-    }
+  // Output the cached console messages. Prefix each sequence of messages
+  // belonging to the same URL with |kConsoleMessagePrefix|. We output console
+  // messages when the request is finished rather than in real time, since in
+  // case of navigations swapping RenderFrameHost would cause the outputs to
+  // disappear.
+  if (messages_.empty())
+    return;
 
-    last_seen_url = message.url;
-  }
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&ClearSiteDataThrottle::OutputConsoleMessagesOnUIThread,
+                 ResourceRequestInfo::ForRequest(request_)
+                     ->GetWebContentsGetterForRequest(),
+                 std::move(messages_)));
 }
 
-ClearSiteDataThrottle::ThrottleCheckResult
-ClearSiteDataThrottle::WillStartRequest() {
-  current_url_ = navigation_handle()->GetURL();
-  return PROCEED;
+void ClearSiteDataThrottle::WillStartRequest(bool* defer) {
+  current_url_ = request_->original_url();
+  *defer = false;
 }
 
-ClearSiteDataThrottle::ThrottleCheckResult
-ClearSiteDataThrottle::WillRedirectRequest() {
+void ClearSiteDataThrottle::WillRedirectRequest(
+    const net::RedirectInfo& redirect_info,
+    bool* defer) {
   // We are processing a redirect from url1 to url2. GetResponseHeaders()
   // contains headers from url1, but GetURL() is already equal to url2. Handle
   // the headers before updating the URL, so that |current_url_| corresponds
   // to the URL that sent the headers.
   HandleHeader();
-  current_url_ = navigation_handle()->GetURL();
-
-  return clearing_in_progress_ ? DEFER : PROCEED;
+  current_url_ = redirect_info.new_url;
+  *defer = clearing_in_progress_;
 }
 
-ClearSiteDataThrottle::ThrottleCheckResult
-ClearSiteDataThrottle::WillProcessResponse() {
+void ClearSiteDataThrottle::WillProcessResponse(bool* defer) {
   HandleHeader();
-  return clearing_in_progress_ ? DEFER : PROCEED;
+  *defer = clearing_in_progress_;
+}
+
+const char* ClearSiteDataThrottle::GetNameForLogging() const {
+  return kNameForLogging;
 }
 
 void ClearSiteDataThrottle::HandleHeader() {
-  NavigationHandleImpl* handle =
-      static_cast<NavigationHandleImpl*>(navigation_handle());
-  const net::HttpResponseHeaders* headers = handle->GetResponseHeaders();
+  const net::HttpResponseHeaders* headers = request_->response_headers();
 
   if (!headers || !headers->HasHeader(kClearSiteDataHeader))
     return;

[mmenke, 2016/09/27 18:24:02]

Do we get here for WebSockets?  I don't think we do.  Should we run this code
for them?

[msramek, 2016/10/13 14:26:03]

I played with some WebSockets demos on the web and the throttle wasn't even
created for them. Not sure why, since I thought that a successful request must
consult each of the throttles?

But in any case, Clear-Site-Data in a WebSockets handshake sounds like a corner
case that we probably don't care much about (for now at least).

[mmenke, 2016/10/13 17:16:31]

Websockets have an entirely different API - they don't even have bodies, for
instance, so they don't go through the ResourceLoader stack.

@@ -152,10 +185,7 @@ void ClearSiteDataThrottle::HandleHeader() {
       ParametersMask(clear_cookies, clear_storage, clear_cache), (1 << 3));
 
   // If the header is valid, clear the data for this browser context and origin.
-  BrowserContext* browser_context =
-      navigation_handle()->GetWebContents()->GetBrowserContext();
   url::Origin origin(current_url_);
-
   if (origin.unique()) {
     ConsoleLog(&messages_, current_url_, "Not supported for unique origins.",
                CONSOLE_MESSAGE_LEVEL_ERROR);
@@ -164,10 +194,37 @@ void ClearSiteDataThrottle::HandleHeader() {
 
   clearing_in_progress_ = true;
   clearing_started_ = base::TimeTicks::Now();
+
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&ClearSiteDataThrottle::ClearSiteDataOnUIThread,
+                 ResourceRequestInfo::ForRequest(request_)
+                     ->GetWebContentsGetterForRequest(),
+                 origin, clear_cookies, clear_storage, clear_cache,
+                 base::Bind(&ClearSiteDataThrottle::TaskFinished,
+                            weak_ptr_factory_.GetWeakPtr())));
+}
+
+// static
+void ClearSiteDataThrottle::ClearSiteDataOnUIThread(
+    const ResourceRequestInfo::WebContentsGetter& web_contents_getter,
+    url::Origin origin,
+    bool clear_cookies,
+    bool clear_storage,
+    bool clear_cache,
+    const base::Closure& callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  WebContents* web_contents = web_contents_getter.Run();
+  if (!web_contents)
+    return;
+
+  BrowserContext* browser_context = web_contents->GetBrowserContext();
+
   GetContentClient()->browser()->ClearSiteData(
       browser_context, origin, clear_cookies, clear_storage, clear_cache,
-      base::Bind(&ClearSiteDataThrottle::TaskFinished,
-                 weak_ptr_factory_.GetWeakPtr()));
+      base::Bind(&JumpFromUIToIOThread, callback));
 }
 
 bool ClearSiteDataThrottle::ParseHeader(const std::string& header,
@@ -268,6 +325,7 @@ bool ClearSiteDataThrottle::ParseHeader(const std::string& header,
 }
 
 void ClearSiteDataThrottle::TaskFinished() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(clearing_in_progress_);
   clearing_in_progress_ = false;
 
@@ -276,7 +334,33 @@ void ClearSiteDataThrottle::TaskFinished() {
                              base::TimeDelta::FromMilliseconds(1),
                              base::TimeDelta::FromSeconds(1), 50);
 
-  navigation_handle()->Resume();
+  controller()->Resume();
+}
+
+// static
+void ClearSiteDataThrottle::OutputConsoleMessagesOnUIThread(
+    const ResourceRequestInfo::WebContentsGetter& web_contents_getter,
+    const std::vector<ConsoleMessage>& messages) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  WebContents* web_contents = web_contents_getter.Run();
+  if (!web_contents)
+    return;
+
+  GURL last_seen_url;
+  for (const ConsoleMessage& message : messages) {
+    if (message.url == last_seen_url) {
+      web_contents->GetMainFrame()->AddMessageToConsole(message.level,
+                                                        message.text);
+    } else {
+      web_contents->GetMainFrame()->AddMessageToConsole(
+          message.level,
+          base::StringPrintf(kConsoleMessagePrefix, message.url.spec().c_str(),
+                             message.text.c_str()));
+    }
+
+    last_seen_url = message.url;
+  }
 }
 
 }  // namespace content