Support the Clear-Site-Data header on resource requests

content/browser/browsing_data/clear_site_data_throttle.h

Index: content/browser/browsing_data/clear_site_data_throttle.h
diff --git a/content/browser/browsing_data/clear_site_data_throttle.h b/content/browser/browsing_data/clear_site_data_throttle.h
index a149900908468fc7110979374a8c2b5796689560..e92dbff5b270b559a3a90fb5bef1973cde83592d 100644
--- a/content/browser/browsing_data/clear_site_data_throttle.h
+++ b/content/browser/browsing_data/clear_site_data_throttle.h
@@ -12,20 +12,27 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/values.h"

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

Nont needed.

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

Done. Removed.

-#include "content/public/browser/navigation_throttle.h"
 #include "content/public/browser/resource_request_info.h"

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

Not needed.

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

True, but after last changes it's needed again because of
ResourceRequestInfo::WebContentsGetter.

+#include "content/public/browser/resource_throttle.h"
 #include "content/public/common/console_message_level.h"
 #include "url/gurl.h"
 
-namespace content {
+namespace net {
+struct RedirectInfo;
+class URLRequest;
+}
+
+namespace url {
+class Origin;
+}

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

Not needed.

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

True, but after last changes it's needed again now because of extracting the
clearing logic to ExecuteClearingTask() :)

 
-class NavigationHandle;
+namespace content {
 
 // This throttle parses the Clear-Site-Data header and executes the clearing
-// of browsing data. The navigation is delayed until the header is parsed and,
-// if valid, until the browsing data are deleted. See the W3C working draft at
-// https://www.w3.org/TR/clear-site-data/.
-class CONTENT_EXPORT ClearSiteDataThrottle : public NavigationThrottle {
+// of browsing data. The resource load is delayed until the header is parsed
+// and, if valid, until the browsing data are deleted. See the W3C working draft
+// at https://www.w3.org/TR/clear-site-data/.
+class CONTENT_EXPORT ClearSiteDataThrottle : public ResourceThrottle {
  public:
   struct ConsoleMessage {
     GURL url;
@@ -33,15 +40,19 @@ class CONTENT_EXPORT ClearSiteDataThrottle : public NavigationThrottle {
     ConsoleMessageLevel level;
   };
 
-  static std::unique_ptr<NavigationThrottle> CreateThrottleForNavigation(
-      NavigationHandle* handle);
+  // Instantiates a throttle for the given |request|. The caller must
+  // guarantee that |request| outlives the throttle.
+  static std::unique_ptr<ResourceThrottle> CreateThrottleForRequest(
+      net::URLRequest* request);
 
   ~ClearSiteDataThrottle() override;
 
-  // NavigationThrottle implementation:
-  ThrottleCheckResult WillStartRequest() override;
-  ThrottleCheckResult WillRedirectRequest() override;
-  ThrottleCheckResult WillProcessResponse() override;
+  // ResourceThrottle implementation:
+  const char* GetNameForLogging() const override;
+  void WillStartRequest(bool* defer) override;
+  void WillRedirectRequest(const net::RedirectInfo& redirect_info,
+                           bool* defer) override;
+  void WillProcessResponse(bool* defer) override;
 
  private:
   friend class ClearSiteDataFuzzerTest;
@@ -49,12 +60,13 @@ class CONTENT_EXPORT ClearSiteDataThrottle : public NavigationThrottle {
   FRIEND_TEST_ALL_PREFIXES(ClearSiteDataThrottleTest, ParseHeader);
   FRIEND_TEST_ALL_PREFIXES(ClearSiteDataThrottleTest, InvalidHeader);

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

Do we really need to friend the test fixture?  Think just making a public
ParseHeaderForTesting method is cleaner, and makes tests stick to using the
public API.

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

I made ParseHeader "public static". I don't think it even needs to be called
ForTesting - there isn't any reason to call this method as standalone outside
tests, or any danger if someone did.

Removed the unittest friendship. Removed the fuzzer friendship and simplified
the fuzzer code.

 
-  explicit ClearSiteDataThrottle(NavigationHandle* navigation_handle);
+  explicit ClearSiteDataThrottle(net::URLRequest* request);
 
   // Scans for the first occurrence of the 'Clear-Site-Data' header, calls
   // ParseHeader() to parse it, and requests the actual data clearing. This is
   // the common logic of WillRedirectRequest() and WillProcessResponse().
-  void HandleHeader();
+  // Returns true if a valid header was found and the clearing was executed.
+  bool HandleHeader();
 
   // Parses the value of the 'Clear-Site-Data' header and outputs whether
   // the header requests to |clear_cookies|, |clear_storage|, and |clear_cache|.
@@ -69,14 +81,13 @@ class CONTENT_EXPORT ClearSiteDataThrottle : public NavigationThrottle {
   // Signals that a parsing and deletion task was finished.
   void TaskFinished();
 
-  // Cached console messages to be output when the RenderFrameHost is ready.
+  // The request this throttle is observing.
+  net::URLRequest* request_;
+
+  // Cached console messages to be output when the resource is loaded.
   std::vector<ConsoleMessage> messages_;
   GURL current_url_;

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

Don't think we need this variable - can just grab it from the URLRequest as
needed.

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

Done. Yes, this is a remnant of the NavigationThrottle - there, we needed it
during redirects, as NavigationHandle already had the new URL while we still
needed to process headers from the old URL.

 
-  // Whether we are currently waiting for a callback that data clearing has
-  // been completed;
-  bool clearing_in_progress_;
-
   // The time when the last clearing operation started. Used when clearing
   // finishes to compute the duration.
   base::TimeTicks clearing_started_;

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

While you're here, should include base/time.h

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

Done.