Refactor ClearSiteData to remove duplicate code

To simplify fixing Clear-Site-Data, I move some of the code that was
duplicated for the network_service into a shared location.

Bug: 898465
Change-Id: I25eac82e7a6f3481b03ac88946a1e7a6828cbfc9
Reviewed-on: https://chromium-review.googlesource.com/c/1350971
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611633}

content/browser/BUILD.gn

@@ -543,6 +543,8 @@ jumbo_source_set("browser") {
     "browsing_data/clear_site_data_handler.h",
     "browsing_data/clear_site_data_throttle.cc",
     "browsing_data/clear_site_data_throttle.h",
+    "browsing_data/clear_site_data_utils.cc",
+    "browsing_data/clear_site_data_utils.h",
     "browsing_data/conditional_cache_deletion_helper.cc",
     "browsing_data/conditional_cache_deletion_helper.h",
     "browsing_data/storage_partition_code_cache_data_remover.cc",

content/browser/browsing_data/clear_site_data_handler.cc

@@ -10,6 +10,7 @@
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
+#include "content/browser/browsing_data/clear_site_data_utils.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browsing_data_filter_builder.h"
 #include "content/public/browser/browsing_data_remover.h"
@@ -47,134 +48,6 @@ int ParametersMask(bool clear_cookies, bool clear_storage, bool clear_cache) {
          static_cast<int>(clear_cache) * (1 << 2);
 }
 
-// Helper class to find the BrowserContext associated with the request and
-// requests the actual clearing of data for |origin|. The data types to be
-// deleted are determined by |clear_cookies|, |clear_storage|, and
-// |clear_cache|. |web_contents_getter| identifies the WebContents from which
-// the request originated.
-// TODO(crbug.com/876931): |SiteDataClearer| could be merged into
-// |ClearSiteDataHandler| to make things cleaner.
-class SiteDataClearer : public BrowsingDataRemover::Observer {
- public:
-  static void Run(
-      const base::RepeatingCallback<WebContents*()>& web_contents_getter,
-      const url::Origin& origin,
-      bool clear_cookies,
-      bool clear_storage,
-      bool clear_cache,
-      base::OnceClosure callback) {
-    WebContents* web_contents = web_contents_getter.Run();
-    // TODO(crbug.com/898465): Fix Clear-Site-Data for requests without
-    // WebContents. (E.g. service worker updates)
-    if (!web_contents) {
-      std::move(callback).Run();
-      return;
-    }
-
-    (new SiteDataClearer(web_contents, origin, clear_cookies, clear_storage,
-                         clear_cache, std::move(callback)))
-        ->RunAndDestroySelfWhenDone();
-  }
-
- private:
-  SiteDataClearer(const WebContents* web_contents,
-                  const url::Origin& origin,
-                  bool clear_cookies,
-                  bool clear_storage,
-                  bool clear_cache,
-                  base::OnceClosure callback)
-      : origin_(origin),
-        clear_cookies_(clear_cookies),
-        clear_storage_(clear_storage),
-        clear_cache_(clear_cache),
-        callback_(std::move(callback)),
-        pending_task_count_(0),
-        remover_(nullptr),
-        scoped_observer_(this) {
-    remover_ = BrowserContext::GetBrowsingDataRemover(
-        web_contents->GetBrowserContext());
-    DCHECK(remover_);
-    scoped_observer_.Add(remover_);
-  }
-
-  ~SiteDataClearer() override {}
-
-  void RunAndDestroySelfWhenDone() {
-    // Cookies and channel IDs are scoped to
-    // a) eTLD+1 of |origin|'s host if |origin|'s host is a registrable domain
-    //    or a subdomain thereof
-    // b) |origin|'s host exactly if it is an IP address or an internal hostname
-    //    (e.g. "localhost" or "fileserver").
-    // TODO(msramek): What about plugin data?
-    if (clear_cookies_) {
-      std::string domain = GetDomainAndRegistry(
-          origin_.host(),
-          net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES);
-
-      if (domain.empty())
-        domain = origin_.host();  // IP address or internal hostname.
-
-      std::unique_ptr<BrowsingDataFilterBuilder> domain_filter_builder(
-          BrowsingDataFilterBuilder::Create(
-              BrowsingDataFilterBuilder::WHITELIST));
-      domain_filter_builder->AddRegisterableDomain(domain);
-
-      pending_task_count_++;
-      remover_->RemoveWithFilterAndReply(
-          base::Time(), base::Time::Max(),
-          BrowsingDataRemover::DATA_TYPE_COOKIES |
-              BrowsingDataRemover::DATA_TYPE_CHANNEL_IDS |
-              BrowsingDataRemover::DATA_TYPE_AVOID_CLOSING_CONNECTIONS,
-          BrowsingDataRemover::ORIGIN_TYPE_UNPROTECTED_WEB |
-              BrowsingDataRemover::ORIGIN_TYPE_PROTECTED_WEB,
-          std::move(domain_filter_builder), this);
-    }
-
-    // Delete origin-scoped data.
-    int remove_mask = 0;
-    if (clear_storage_)
-      remove_mask |= BrowsingDataRemover::DATA_TYPE_DOM_STORAGE;
-    if (clear_cache_)
-      remove_mask |= BrowsingDataRemover::DATA_TYPE_CACHE;
-
-    if (remove_mask) {
-      std::unique_ptr<BrowsingDataFilterBuilder> origin_filter_builder(
-          BrowsingDataFilterBuilder::Create(
-              BrowsingDataFilterBuilder::WHITELIST));
-      origin_filter_builder->AddOrigin(origin_);
-
-      pending_task_count_++;
-      remover_->RemoveWithFilterAndReply(
-          base::Time(), base::Time::Max(), remove_mask,
-          BrowsingDataRemover::ORIGIN_TYPE_UNPROTECTED_WEB |
-              BrowsingDataRemover::ORIGIN_TYPE_PROTECTED_WEB,
-          std::move(origin_filter_builder), this);
-    }
-
-    DCHECK_GT(pending_task_count_, 0);
-  }
-
-  // BrowsingDataRemover::Observer:
-  void OnBrowsingDataRemoverDone() override {
-    DCHECK(pending_task_count_);
-    if (--pending_task_count_)
-      return;
-
-    std::move(callback_).Run();
-    delete this;
-  }
-
-  url::Origin origin_;
-  bool clear_cookies_;
-  bool clear_storage_;
-  bool clear_cache_;
-  base::OnceClosure callback_;
-  int pending_task_count_;
-  BrowsingDataRemover* remover_;
-  ScopedObserver<BrowsingDataRemover, BrowsingDataRemover::Observer>
-      scoped_observer_;
-};
-
 // Outputs a single |formatted_message| on the UI thread.
 void OutputFormattedMessage(WebContents* web_contents,
                             ConsoleMessageLevel level,
@@ -419,8 +292,9 @@ void ClearSiteDataHandler::ExecuteClearingTask(const url::Origin& origin,
                                                bool clear_storage,
                                                bool clear_cache,
                                                base::OnceClosure callback) {
-  SiteDataClearer::Run(web_contents_getter_, origin, clear_cookies,
-                       clear_storage, clear_cache, std::move(callback));
+  clear_site_data_utils::ClearSiteData(web_contents_getter_, origin,
+                                       clear_cookies, clear_storage,
+                                       clear_cache, std::move(callback));
 }
 
 // static

content/browser/browsing_data/clear_site_data_throttle.cc

@@ -13,19 +13,17 @@
 #include "base/strings/stringprintf.h"
 #include "base/task/post_task.h"
 #include "base/values.h"
+#include "content/browser/browsing_data/clear_site_data_utils.h"
 #include "content/browser/service_worker/service_worker_response_info.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
-#include "content/public/browser/browsing_data_filter_builder.h"
-#include "content/public/browser/browsing_data_remover.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/origin_util.h"
 #include "content/public/common/resource_type.h"
 #include "net/base/load_flags.h"
-#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 #include "net/http/http_response_headers.h"
 #include "net/url_request/redirect_info.h"
 #include "services/network/public/cpp/resource_response_info.h"
@@ -77,138 +75,6 @@ void JumpFromUIToIOThread(base::OnceClosure callback) {
   base::PostTaskWithTraits(FROM_HERE, {BrowserThread::IO}, std::move(callback));
 }
 
-// Finds the BrowserContext associated with the request and requests
-// the actual clearing of data for |origin|. The data types 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.
-class UIThreadSiteDataClearer : public BrowsingDataRemover::Observer {
- public:
-  static void Run(
-      const ResourceRequestInfo::WebContentsGetter& web_contents_getter,
-      const url::Origin& origin,
-      bool clear_cookies,
-      bool clear_storage,
-      bool clear_cache,
-      base::OnceClosure callback) {
-    WebContents* web_contents = web_contents_getter.Run();
-    // TODO(crbug.com/898465): Fix Clear-Site-Data for requests without
-    // WebContents. (E.g. service worker updates)
-    if (!web_contents) {
-      JumpFromUIToIOThread(std::move(callback));
-      return;
-    }
-
-    (new UIThreadSiteDataClearer(web_contents, origin, clear_cookies,
-                                 clear_storage, clear_cache,
-                                 std::move(callback)))
-        ->RunAndDestroySelfWhenDone();
-  }
-
- private:
-  UIThreadSiteDataClearer(const WebContents* web_contents,
-                          const url::Origin& origin,
-                          bool clear_cookies,
-                          bool clear_storage,
-                          bool clear_cache,
-                          base::OnceClosure callback)
-      : origin_(origin),
-        clear_cookies_(clear_cookies),
-        clear_storage_(clear_storage),
-        clear_cache_(clear_cache),
-        callback_(std::move(callback)),
-        pending_task_count_(0),
-        remover_(nullptr),
-        scoped_observer_(this) {
-    DCHECK_CURRENTLY_ON(BrowserThread::UI);
-
-    remover_ = BrowserContext::GetBrowsingDataRemover(
-        web_contents->GetBrowserContext());
-    DCHECK(remover_);
-    scoped_observer_.Add(remover_);
-  }
-
-  ~UIThreadSiteDataClearer() override {}
-
-  void RunAndDestroySelfWhenDone() {
-    DCHECK_CURRENTLY_ON(BrowserThread::UI);
-
-    // Cookies and channel IDs are scoped to
-    // a) eTLD+1 of |origin|'s host if |origin|'s host is a registrable domain
-    //    or a subdomain thereof
-    // b) |origin|'s host exactly if it is an IP address or an internal hostname
-    //    (e.g. "localhost" or "fileserver").
-    // TODO(msramek): What about plugin data?
-    if (clear_cookies_) {
-      std::string domain = GetDomainAndRegistry(
-          origin_.host(),
-          net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES);
-
-      if (domain.empty())
-        domain = origin_.host();  // IP address or internal hostname.
-
-      std::unique_ptr<BrowsingDataFilterBuilder> domain_filter_builder(
-          BrowsingDataFilterBuilder::Create(
-              BrowsingDataFilterBuilder::WHITELIST));
-      domain_filter_builder->AddRegisterableDomain(domain);
-
-      pending_task_count_++;
-      remover_->RemoveWithFilterAndReply(
-          base::Time(), base::Time::Max(),
-          BrowsingDataRemover::DATA_TYPE_COOKIES |
-              BrowsingDataRemover::DATA_TYPE_CHANNEL_IDS |
-              BrowsingDataRemover::DATA_TYPE_AVOID_CLOSING_CONNECTIONS,
-          BrowsingDataRemover::ORIGIN_TYPE_UNPROTECTED_WEB |
-              BrowsingDataRemover::ORIGIN_TYPE_PROTECTED_WEB,
-          std::move(domain_filter_builder), this);
-    }
-
-    // Delete origin-scoped data.
-    int remove_mask = 0;
-    if (clear_storage_)
-      remove_mask |= BrowsingDataRemover::DATA_TYPE_DOM_STORAGE;
-    if (clear_cache_)
-      remove_mask |= BrowsingDataRemover::DATA_TYPE_CACHE;
-
-    if (remove_mask) {
-      std::unique_ptr<BrowsingDataFilterBuilder> origin_filter_builder(
-          BrowsingDataFilterBuilder::Create(
-              BrowsingDataFilterBuilder::WHITELIST));
-      origin_filter_builder->AddOrigin(origin_);
-
-      pending_task_count_++;
-      remover_->RemoveWithFilterAndReply(
-          base::Time(), base::Time::Max(), remove_mask,
-          BrowsingDataRemover::ORIGIN_TYPE_UNPROTECTED_WEB |
-              BrowsingDataRemover::ORIGIN_TYPE_PROTECTED_WEB,
-          std::move(origin_filter_builder), this);
-    }
-
-    DCHECK_GT(pending_task_count_, 0);
-  }
-
-  // BrowsingDataRemover::Observer:
-  void OnBrowsingDataRemoverDone() override {
-    DCHECK(pending_task_count_);
-    if (--pending_task_count_)
-      return;
-
-    JumpFromUIToIOThread(std::move(callback_));
-    delete this;
-  }
-
-  url::Origin origin_;
-  bool clear_cookies_;
-  bool clear_storage_;
-  bool clear_cache_;
-  base::OnceClosure callback_;
-  int pending_task_count_;
-  BrowsingDataRemover* remover_;
-  ScopedObserver<BrowsingDataRemover, BrowsingDataRemover::Observer>
-      scoped_observer_;
-};
-
 // Outputs a single |formatted_message| on the UI thread.
 void OutputFormattedMessage2(WebContents* web_contents,
                              ConsoleMessageLevel level,
@@ -531,11 +397,12 @@ void ClearSiteDataThrottle::ExecuteClearingTask(const url::Origin& origin,
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   base::PostTaskWithTraits(
       FROM_HERE, {BrowserThread::UI},
-      base::BindOnce(&UIThreadSiteDataClearer::Run,
-                     ResourceRequestInfo::ForRequest(request_)
-                         ->GetWebContentsGetterForRequest(),
-                     origin, clear_cookies, clear_storage, clear_cache,
-                     std::move(callback)));
+      base::BindOnce(
+          &clear_site_data_utils::ClearSiteData,
+          ResourceRequestInfo::ForRequest(request_)
+              ->GetWebContentsGetterForRequest(),
+          origin, clear_cookies, clear_storage, clear_cache,
+          base::BindOnce(&JumpFromUIToIOThread, std::move(callback))));
 }
 
 void ClearSiteDataThrottle::TaskFinished() {

content/browser/browsing_data/clear_site_data_utils.cc

+// Copyright 2018 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 "content/browser/browsing_data/clear_site_data_throttle.h"
+
+#include "base/scoped_observer.h"
+#include "content/public/browser/browser_context.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/browsing_data_filter_builder.h"
+#include "content/public/browser/browsing_data_remover.h"
+#include "content/public/browser/web_contents.h"
+#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
+
+namespace content {
+namespace clear_site_data_utils {
+
+namespace {
+
+// Finds the BrowserContext associated with the request and requests
+// the actual clearing of data for |origin|. The data types 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.
+class SiteDataClearer : public BrowsingDataRemover::Observer {
+ public:
+  SiteDataClearer(BrowserContext* browser_context,
+                  const url::Origin& origin,
+                  bool clear_cookies,
+                  bool clear_storage,
+                  bool clear_cache,
+                  base::OnceClosure callback)
+      : origin_(origin),
+        clear_cookies_(clear_cookies),
+        clear_storage_(clear_storage),
+        clear_cache_(clear_cache),
+        callback_(std::move(callback)),
+        pending_task_count_(0),
+        remover_(nullptr),
+        scoped_observer_(this) {
+    remover_ = BrowserContext::GetBrowsingDataRemover(browser_context);
+    DCHECK(remover_);
+    scoped_observer_.Add(remover_);
+  }
+
+  ~SiteDataClearer() override {}
+
+  void RunAndDestroySelfWhenDone() {
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    // Cookies and channel IDs are scoped to
+    // a) eTLD+1 of |origin|'s host if |origin|'s host is a registrable domain
+    //    or a subdomain thereof
+    // b) |origin|'s host exactly if it is an IP address or an internal hostname
+    //    (e.g. "localhost" or "fileserver").
+    // TODO(msramek): What about plugin data?
+    if (clear_cookies_) {
+      std::string domain = GetDomainAndRegistry(
+          origin_.host(),
+          net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES);
+
+      if (domain.empty())
+        domain = origin_.host();  // IP address or internal hostname.
+
+      std::unique_ptr<BrowsingDataFilterBuilder> domain_filter_builder(
+          BrowsingDataFilterBuilder::Create(
+              BrowsingDataFilterBuilder::WHITELIST));
+      domain_filter_builder->AddRegisterableDomain(domain);
+
+      pending_task_count_++;
+      remover_->RemoveWithFilterAndReply(
+          base::Time(), base::Time::Max(),
+          BrowsingDataRemover::DATA_TYPE_COOKIES |
+              BrowsingDataRemover::DATA_TYPE_CHANNEL_IDS |
+              BrowsingDataRemover::DATA_TYPE_AVOID_CLOSING_CONNECTIONS,
+          BrowsingDataRemover::ORIGIN_TYPE_UNPROTECTED_WEB |
+              BrowsingDataRemover::ORIGIN_TYPE_PROTECTED_WEB,
+          std::move(domain_filter_builder), this);
+    }
+
+    // Delete origin-scoped data.
+    int remove_mask = 0;
+    if (clear_storage_)
+      remove_mask |= BrowsingDataRemover::DATA_TYPE_DOM_STORAGE;
+    if (clear_cache_)
+      remove_mask |= BrowsingDataRemover::DATA_TYPE_CACHE;
+
+    if (remove_mask) {
+      std::unique_ptr<BrowsingDataFilterBuilder> origin_filter_builder(
+          BrowsingDataFilterBuilder::Create(
+              BrowsingDataFilterBuilder::WHITELIST));
+      origin_filter_builder->AddOrigin(origin_);
+
+      pending_task_count_++;
+      remover_->RemoveWithFilterAndReply(
+          base::Time(), base::Time::Max(), remove_mask,
+          BrowsingDataRemover::ORIGIN_TYPE_UNPROTECTED_WEB |
+              BrowsingDataRemover::ORIGIN_TYPE_PROTECTED_WEB,
+          std::move(origin_filter_builder), this);
+    }
+
+    DCHECK_GT(pending_task_count_, 0);
+  }
+
+ private:
+  // BrowsingDataRemover::Observer:
+  void OnBrowsingDataRemoverDone() override {
+    DCHECK(pending_task_count_);
+    if (--pending_task_count_)
+      return;
+
+    std::move(callback_).Run();
+    delete this;
+  }
+
+  url::Origin origin_;
+  bool clear_cookies_;
+  bool clear_storage_;
+  bool clear_cache_;
+  base::OnceClosure callback_;
+  int pending_task_count_;
+  BrowsingDataRemover* remover_;
+  ScopedObserver<BrowsingDataRemover, BrowsingDataRemover::Observer>
+      scoped_observer_;
+};
+
+}  // namespace
+
+void ClearSiteData(
+    const ResourceRequestInfo::WebContentsGetter& web_contents_getter,
+    const url::Origin& origin,
+    bool clear_cookies,
+    bool clear_storage,
+    bool clear_cache,
+    base::OnceClosure callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  WebContents* web_contents = web_contents_getter.Run();
+  // TODO(crbug.com/898465): Fix Clear-Site-Data for requests without
+  // WebContents. (E.g. service worker updates)
+  if (!web_contents) {
+    std::move(callback).Run();
+    return;
+  }
+  (new SiteDataClearer(web_contents->GetBrowserContext(), origin, clear_cookies,
+                       clear_storage, clear_cache, std::move(callback)))
+      ->RunAndDestroySelfWhenDone();
+}
+
+}  // namespace clear_site_data_utils
+}  // namespace content

content/browser/browsing_data/clear_site_data_utils.h

+// Copyright 2018 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.
+
+#ifndef CONTENT_BROWSER_BROWSING_DATA_CLEAR_SITE_DATA_UTILS_H_
+#define CONTENT_BROWSER_BROWSING_DATA_CLEAR_SITE_DATA_UTILS_H_
+
+#include "base/callback_forward.h"
+#include "content/public/browser/resource_request_info.h"
+
+namespace url {
+class Origin;
+}
+
+namespace content {
+namespace clear_site_data_utils {
+
+// Removes browsing data associated with |origin| when the Clear-Site-Data
+// header is sent.
+// Has to be called on the UI thread and will execute |callback| on the UI
+// thread when done.
+// TODO(dullweber): Consider merging back when network service is shipped.
+void ClearSiteData(
+    const ResourceRequestInfo::WebContentsGetter& web_contents_getter,
+    const url::Origin& origin,
+    bool clear_cookies,
+    bool clear_storage,
+    bool clear_cache,
+    base::OnceClosure callback);
+
+}  // namespace clear_site_data_utils
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_BROWSING_DATA_CLEAR_SITE_DATA_UTILS_H_