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..78fe12bdd04a03aa7125ce94ec1ca9c46b2d2afc 100644
--- a/content/browser/browsing_data/clear_site_data_throttle.cc
+++ b/content/browser/browsing_data/clear_site_data_throttle.cc
@@ -12,15 +12,21 @@
 #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/browser/service_worker/service_worker_response_info.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/storage_partition.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 "content/public/common/resource_response_info.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 +34,8 @@ namespace content {
 
 namespace {
 
+static const char* kNameForLogging = "ClearSiteDataThrottle";
+
 static const char* kClearSiteDataHeader = "Clear-Site-Data";
 
 static const char* kTypesKey = "types";
@@ -58,34 +66,57 @@ int ParametersMask(bool clear_cookies, bool clear_storage, bool clear_cache) {
          static_cast<int>(clear_cache) * (1 << 2);
 }
 
-}  // namespace
+// 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);
+}
 
-// static
-std::unique_ptr<NavigationThrottle>
-ClearSiteDataThrottle::CreateThrottleForNavigation(NavigationHandle* handle) {
-  if (AreExperimentalFeaturesEnabled())
-    return base::WrapUnique(new ClearSiteDataThrottle(handle));
+// Finds the BrowserContext associated with the request and requests
+// the actual clearing of data for |origin|. The datatypes to be deleted
+// are determined by |clear_cookies|, |clear_storage|, and |clear_cache|.
+// |web_contents_getter| identifies the WebContents from which the request
+// originated. Must be run on the UI thread. The |callback| will be executed
+// on the IO thread.
+void 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;
 
-  return std::unique_ptr<NavigationThrottle>();
+  BrowserContext* browser_context = web_contents->GetBrowserContext();
+
+  GetContentClient()->browser()->ClearSiteData(
+      browser_context, origin, clear_cookies, clear_storage, clear_cache,
+      base::Bind(&JumpFromUIToIOThread, callback));
 }
 
-ClearSiteDataThrottle::ClearSiteDataThrottle(
-    NavigationHandle* navigation_handle)
-    : NavigationThrottle(navigation_handle),
-      clearing_in_progress_(false),
-      weak_ptr_factory_(this) {}
+// Outputs |messages| to the console of WebContents retrieved from
+// |web_contents_getter|. Must be run on the UI thread.
+void OutputConsoleMessagesOnUIThread(
+    const ResourceRequestInfo::WebContentsGetter& web_contents_getter,
+    const std::vector<ClearSiteDataThrottle::ConsoleMessage>& messages) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  WebContents* web_contents = web_contents_getter.Run();
+  if (!web_contents)
+    return;
 
-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_) {
+  for (const ClearSiteDataThrottle::ConsoleMessage& message : messages) {
     if (message.url == last_seen_url) {
-      navigation_handle()->GetRenderFrameHost()->AddMessageToConsole(
-          message.level, message.text);
+      web_contents->GetMainFrame()->AddMessageToConsole(message.level,
+                                                        message.text);
     } else {
-      navigation_handle()->GetRenderFrameHost()->AddMessageToConsole(
+      web_contents->GetMainFrame()->AddMessageToConsole(
           message.level,
           base::StringPrintf(kConsoleMessagePrefix, message.url.spec().c_str(),
                              message.text.c_str()));
@@ -95,34 +126,86 @@ ClearSiteDataThrottle::~ClearSiteDataThrottle() {
   }
 }
 
-ClearSiteDataThrottle::ThrottleCheckResult
-ClearSiteDataThrottle::WillStartRequest() {
-  current_url_ = navigation_handle()->GetURL();
-  return PROCEED;
+}  // namespace
+
+// static
+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);

[mmenke, 2016/10/20 19:03:10]

This is all just request->load_flags()...May be simplest to just check this only
when we see the new header.  load flags can be updated / modified both between
when the throttle is created and when it's first sent, and during redirects. 
I'm more concerned about when it's set than when it's cleared, though I'm not
sure this paritucular flag is ever modified, once set.

That does leave us in an odd state where in WillRedirectRequest, the flag could
either have been set or cleared between receiving the headers and when |this| is
called, but...erm...Let's ignore that case.

[msramek, 2016/10/21 13:36:00]

Good to know! I moved this to HandleHeader(). That also makes it possible to
output a console message.

Also changed it to load_flags() (somehow I didn't notice that before).

+  if (load_flags & net::LOAD_DO_NOT_SAVE_COOKIES)
+    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>();
+
+  return base::WrapUnique(new ClearSiteDataThrottle(request));
+}
+
+ClearSiteDataThrottle::ClearSiteDataThrottle(net::URLRequest* request)
+    : request_(request),
+      clearing_in_progress_(false),
+      weak_ptr_factory_(this) {}
+
+ClearSiteDataThrottle::~ClearSiteDataThrottle() {
+  // 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;
+
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&OutputConsoleMessagesOnUIThread,
+                 ResourceRequestInfo::ForRequest(request_)
+                     ->GetWebContentsGetterForRequest(),
+                 std::move(messages_)));
+}
+
+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_;

[mmenke, 2016/10/20 19:03:10]

I think the code is easier to follow if HandleHeader takes / sets defer instead.
 (And I'd suggest getting rid of clearing_in_progress_, as then it's no longer
needed, except for DCHECKs).

[msramek, 2016/10/21 13:36:00]

Done. Since the "success" of HandleHeader() is equivalent to "should defer", I
just made HandleHeader() return a boolean. I think "return false" / "return
true" reads better than setting "*defer=false" at the beginning of the method
and then "*defer=true" at the end. But let me know if you prefer to use bool*
instead of a return value anyway.

It seems like pretty much all implementations of ResourceController::Resume()
have the same DCHECK already, so we can safely remove it.

 }
 
-ClearSiteDataThrottle::ThrottleCheckResult
-ClearSiteDataThrottle::WillProcessResponse() {
+void ClearSiteDataThrottle::WillProcessResponse(bool* defer) {
   HandleHeader();
-  return clearing_in_progress_ ? DEFER : PROCEED;
+  *defer = clearing_in_progress_;

[mmenke, 2016/10/20 19:03:10]

This behavior should probably be documented in the header - pausing the request
long enough for relevant data to be cleared.

[msramek, 2016/10/21 13:36:00]

It is mentioned in the class-level comment. Or do you mean to add the comment to
WillRedirectRequest() and WillProcessResponse() as well?

[mmenke, 2016/10/21 15:16:20]

Sorry, I missed that.

+}
+
+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;
@@ -134,6 +217,24 @@ void ClearSiteDataThrottle::HandleHeader() {
     return;
   }
 
+  // Service workers can handle fetches of third-party resources and inject
+  // arbitrary headers. Ignore responses that came from a service worker,
+  // as supporting Clear-Site-Data would give them the power to delete data from
+  // any website.
+  const ServiceWorkerResponseInfo* response_info =
+      ServiceWorkerResponseInfo::ForRequest(request_);
+  if (response_info) {
+    ResourceResponseInfo extra_response_info;
+    response_info->GetExtraResponseInfo(&extra_response_info);
+
+    if (extra_response_info.was_fetched_via_service_worker) {
+      ConsoleLog(&messages_, current_url_,
+                 "Ignoring, as the response came from a service worker.",
+                 CONSOLE_MESSAGE_LEVEL_ERROR);
+      return;
+    }
+  }
+
   std::string header_value;
   headers->GetNormalizedHeader(kClearSiteDataHeader, &header_value);

[mmenke, 2016/10/20 19:03:11]

Rather than two searches for the header, can just replace HasHeader above with
GetNormalizedHeader.  Not a huge savings, but no need to waste CPU time.

[msramek, 2016/10/21 13:36:00]

Done. Good catch.

 
@@ -152,10 +253,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()) {

[mmenke, 2016/10/20 19:03:10]

Should this go up with the IsOriginSecure check?  Seems to make sense to have
all url validation in one place.

[msramek, 2016/10/21 13:36:00]

Done. Yeah, makes sense.

Now that I'm looking at it, I'm not sure if a unique origin can be secure. But
IsOriginSecure takes a GURL instead of an origin so it's not easy to verify. I'd
keep the check to be sure.

[mmenke, 2016/10/21 15:16:20]

File origins are secure and unique (Though they don't have any data to
clear...Or headers, for that matter, though I guess blobs from file origins do
have headers)

     ConsoleLog(&messages_, current_url_, "Not supported for unique origins.",
                CONSOLE_MESSAGE_LEVEL_ERROR);
@@ -164,10 +262,16 @@ void ClearSiteDataThrottle::HandleHeader() {
 
   clearing_in_progress_ = true;
   clearing_started_ = base::TimeTicks::Now();
-  GetContentClient()->browser()->ClearSiteData(
-      browser_context, origin, clear_cookies, clear_storage, clear_cache,
-      base::Bind(&ClearSiteDataThrottle::TaskFinished,
-                 weak_ptr_factory_.GetWeakPtr()));
+
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&ClearSiteDataOnUIThread,
+                 ResourceRequestInfo::ForRequest(request_)
+                     ->GetWebContentsGetterForRequest(),
+                 origin, clear_cookies, clear_storage, clear_cache,
+                 base::Bind(&ClearSiteDataThrottle::TaskFinished,
+                            weak_ptr_factory_.GetWeakPtr())));
 }
 
 bool ClearSiteDataThrottle::ParseHeader(const std::string& header,
@@ -268,6 +372,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 +381,7 @@ void ClearSiteDataThrottle::TaskFinished() {
                              base::TimeDelta::FromMilliseconds(1),
                              base::TimeDelta::FromSeconds(1), 50);
 
-  navigation_handle()->Resume();
+  controller()->Resume();
 }
 
 }  // namespace content