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..f155040b99b797c65801243710722f9b5d4a4da6 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;
+
+  BrowserContext* browser_context = web_contents->GetBrowserContext();
 
-  return std::unique_ptr<NavigationThrottle>();
+  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,47 +126,118 @@ 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 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), 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();
+  *defer = HandleHeader();
+  current_url_ = redirect_info.new_url;
+}
 
-  return clearing_in_progress_ ? DEFER : PROCEED;
+void ClearSiteDataThrottle::WillProcessResponse(bool* defer) {
+  *defer = HandleHeader();
 }
 
-ClearSiteDataThrottle::ThrottleCheckResult
-ClearSiteDataThrottle::WillProcessResponse() {
-  HandleHeader();
-  return clearing_in_progress_ ? DEFER : PROCEED;
+const char* ClearSiteDataThrottle::GetNameForLogging() const {
+  return kNameForLogging;
 }
 
-void ClearSiteDataThrottle::HandleHeader() {
-  NavigationHandleImpl* handle =
-      static_cast<NavigationHandleImpl*>(navigation_handle());
-  const net::HttpResponseHeaders* headers = handle->GetResponseHeaders();
+bool ClearSiteDataThrottle::HandleHeader() {
+  const net::HttpResponseHeaders* headers = request_->response_headers();
 
-  if (!headers || !headers->HasHeader(kClearSiteDataHeader))
-    return;
+  std::string header_value;
+  if (!headers ||
+      !headers->GetNormalizedHeader(kClearSiteDataHeader, &header_value)) {

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

Should the kClearSiteDataHeader be added to kCookieResponseHeaders in
http_response_headers?  When we write an entry to the cache, we don't save
set-cookie headers, so we don't set cookies when reading cached responses.  I
assume the same behavior should be done here?  And should probably have a test
for it, which I assume will be trivial, assuming we already test that code for
other headers.

[msramek, 2016/10/31 19:23:35]

Yes, that's a good point. This header should be network-only. Done.

+    return false;
+  }
 
-  // Only accept the header on secure origins.
+  // Only accept the header on secure non-unique origins.
   if (!IsOriginSecure(current_url_)) {

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

There's a lot of logic here which it seems like we should be testing.

[msramek, 2016/10/31 19:23:35]

Added a few tests:

ClearSiteDataThrottleTest.InvalidOrigin - handling insecure origins, unique
origins, and their error messages

ClearSiteDataThrottleTest.LoadDoNotSaveCookies - handling
LOAD_DO_NOT_SAVE_COOKIES

ClearSiteDataThrottleBrowserTest.CredentialsOnRedirect - essentially handling
LOAD_DO_NOT_SAVE_COOKIES when it changes during the request lifetime (although
the browser doesn't directly see that)

The service worker parts are already covered in browsertests.

     ConsoleLog(&messages_, current_url_, "Not supported for insecure origins.",
                CONSOLE_MESSAGE_LEVEL_ERROR);

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

Do we care about console messages enough to test this?

[msramek, 2016/10/31 19:23:35]

We don't care that much, but I tried to test them. This, however, led to a
complication.

The console messages are output from the destructor, and that cannot be
overriden in tests (even if we made it a virtual function and overrode it in
tests, the destructor will not call the overriden version as the subclass is
already destroyed at that time).

So I had to create a delegate class that handles console messages. That
unfortunately added some boilerplate code.

-    return;
+    return false;
   }
 
-  std::string header_value;
-  headers->GetNormalizedHeader(kClearSiteDataHeader, &header_value);
+  url::Origin origin(current_url_);
+  if (origin.unique()) {
+    ConsoleLog(&messages_, current_url_, "Not supported for unique origins.",
+               CONSOLE_MESSAGE_LEVEL_ERROR);
+    return false;
+  }
+
+  // The LOAD_DO_NOT_SAVE_COOKIES flag prohibits the request from doing any
+  // modification to cookies. Clear-Site-Data applies this restriction to other
+  // datatypes as well.
+  if (request_->load_flags() & net::LOAD_DO_NOT_SAVE_COOKIES) {
+    ConsoleLog(&messages_, current_url_,
+               "The request's credentials mode prohibits modifying cookies "
+               "and other local data.",
+               CONSOLE_MESSAGE_LEVEL_ERROR);
+    return false;
+  }
+
+  // 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 false;
+    }
+  }
 
   bool clear_cookies;
   bool clear_storage;
@@ -143,7 +245,7 @@ void ClearSiteDataThrottle::HandleHeader() {
 
   if (!ParseHeader(header_value, &clear_cookies, &clear_storage, &clear_cache,
                    &messages_)) {
-    return;
+    return false;
   }
 
   // Record the call parameters.
@@ -152,22 +254,19 @@ 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_);
+  clearing_started_ = base::TimeTicks::Now();
 
-  if (origin.unique()) {
-    ConsoleLog(&messages_, current_url_, "Not supported for unique origins.",
-               CONSOLE_MESSAGE_LEVEL_ERROR);
-    return;
-  }
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);

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

optional:  May want to put this at the start of the method...Not sure, may be
best here to document where we are before the PostTask, up to you.

[msramek, 2016/10/31 19:23:35]

I extracted everything that knows about threads to separate methods so that I
don't have to deal with it in unittests. Those separate methods now contain the
DCHECK_CURRENTLY_ON.

The rest of the code can run on both UI and IO threads.

+  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())));
 
-  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()));
+  return true;
 }
 
 bool ClearSiteDataThrottle::ParseHeader(const std::string& header,
@@ -268,15 +367,14 @@ bool ClearSiteDataThrottle::ParseHeader(const std::string& header,
 }
 
 void ClearSiteDataThrottle::TaskFinished() {
-  DCHECK(clearing_in_progress_);
-  clearing_in_progress_ = false;
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   UMA_HISTOGRAM_CUSTOM_TIMES("Navigation.ClearSiteData.Duration",
                              base::TimeTicks::Now() - clearing_started_,
                              base::TimeDelta::FromMilliseconds(1),
                              base::TimeDelta::FromSeconds(1), 50);
 
-  navigation_handle()->Resume();
+  controller()->Resume();
 }
 
 }  // namespace content