Clear site data through NetworkContextClient

When Clear-Site-Data header is received, call the OnClearSiteData()
method of NetworkContextClient instead of NetworkServiceClient because
NetworkContextClient always has a BrowserContext associated.

Bug: 952723
Change-Id: I8aff6bbd8eb9486379694d30c731974f24f86d50
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1604469Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Ting Shao <ting.shao@intel.com>
Cr-Commit-Position: refs/heads/master@{#661162}

content/browser/network_service_client.cc

@@ -335,20 +335,6 @@ WebContents* GetWebContents(int process_id, int routing_id) {
   return WebContents::FromFrameTreeNodeId(routing_id);
 }
 
-BrowserContext* GetBrowserContextFromIds(int process_id, int routing_id) {
-  WebContents* web_contents = GetWebContents(process_id, routing_id);
-  if (web_contents)
-    return web_contents->GetBrowserContext();
-  // Some requests such as service worker updates are not associated with
-  // a WebContents so we can't use it to obtain the BrowserContext.
-  // TODO(dullweber): Could we always use RenderProcessHost?
-  RenderProcessHost* process_host = RenderProcessHostImpl::FromID(process_id);
-  if (process_host)
-    return process_host->GetBrowserContext();
-
-  return nullptr;
-}
-
 bool IsMainFrameRequest(int process_id, int routing_id) {
   if (process_id != network::mojom::kBrowserProcessId)
     return false;
@@ -443,7 +429,7 @@ void NetworkServiceClient::OnAuthRequired(
     const net::AuthChallengeInfo& auth_info,
     const base::Optional<network::ResourceResponseHead>& head,
     network::mojom::AuthChallengeResponderPtr auth_challenge_responder) {
-  base::Callback<WebContents*(void)> web_contents_getter =
+  base::RepeatingCallback<WebContents*(void)> web_contents_getter =
       base::BindRepeating(GetWebContents, process_id, routing_id);
 
   if (!web_contents_getter.Run()) {
@@ -491,7 +477,7 @@ void NetworkServiceClient::OnSSLCertificateError(
     OnSSLCertificateErrorCallback response) {
   SSLErrorDelegate* delegate =
       new SSLErrorDelegate(std::move(response));  // deletes self
-  base::Callback<WebContents*(void)> web_contents_getter =
+  base::RepeatingCallback<WebContents*(void)> web_contents_getter =
       base::BindRepeating(GetWebContents, process_id, routing_id);
   bool is_main_frame_request = IsMainFrameRequest(process_id, routing_id);
   SSLManager::OnSSLCertificateError(
@@ -564,21 +550,6 @@ void NetworkServiceClient::OnLoadingStateUpdate(
   std::move(callback).Run();
 }
 
-void NetworkServiceClient::OnClearSiteData(int process_id,
-                                           int routing_id,
-                                           const GURL& url,
-                                           const std::string& header_value,
-                                           int load_flags,
-                                           OnClearSiteDataCallback callback) {
-  auto browser_context_getter =
-      base::BindRepeating(GetBrowserContextFromIds, process_id, routing_id);
-  auto web_contents_getter =
-      base::BindRepeating(GetWebContents, process_id, routing_id);
-  ClearSiteDataHandler::HandleHeader(browser_context_getter,
-                                     web_contents_getter, url, header_value,
-                                     load_flags, std::move(callback));
-}
-
 void NetworkServiceClient::OnCertDBChanged() {
   GetNetworkService()->OnCertDBChanged();
 }

content/browser/network_service_client.h

@@ -80,12 +80,6 @@ class CONTENT_EXPORT NetworkServiceClient
                       bool blocked_by_policy) override;
   void OnLoadingStateUpdate(std::vector<network::mojom::LoadInfoPtr> infos,
                             OnLoadingStateUpdateCallback callback) override;
-  void OnClearSiteData(int process_id,
-                       int routing_id,
-                       const GURL& url,
-                       const std::string& header_value,
-                       int load_flags,
-                       OnClearSiteDataCallback callback) override;
   void OnDataUseUpdate(int32_t network_traffic_annotation_id_hash,
                        int64_t recv_bytes,
                        int64_t sent_bytes) override;

content/browser/storage_partition_impl.cc

@@ -30,6 +30,7 @@
 #include "content/browser/blob_storage/blob_registry_wrapper.h"
 #include "content/browser/blob_storage/chrome_blob_storage_context.h"
 #include "content/browser/browser_main_loop.h"
+#include "content/browser/browsing_data/clear_site_data_handler.h"
 #include "content/browser/browsing_data/storage_partition_code_cache_data_remover.h"
 #include "content/browser/browsing_data/storage_partition_http_cache_data_remover.h"
 #include "content/browser/child_process_security_policy_impl.h"
@@ -41,6 +42,7 @@
 #include "content/browser/loader/prefetch_url_loader_service.h"
 #include "content/browser/native_file_system/native_file_system_manager_impl.h"
 #include "content/browser/notifications/platform_notification_context_impl.h"
+#include "content/browser/web_contents/web_contents_impl.h"
 #include "content/common/dom_storage/dom_storage_types.h"
 #include "content/common/service_worker/service_worker_utils.h"
 #include "content/public/browser/browser_context.h"
@@ -270,6 +272,18 @@ void ClearSessionStorageOnUIThread(
       origin_matcher, perform_storage_cleanup, std::move(callback)));
 }
 
+WebContents* GetWebContents(uint32_t process_id, uint32_t routing_id) {
+  if (process_id != network::mojom::kBrowserProcessId) {
+    return WebContentsImpl::FromRenderFrameHostID(process_id, routing_id);
+  }
+  return WebContents::FromFrameTreeNodeId(routing_id);
+}
+
+BrowserContext* GetBrowserContext(
+    base::WeakPtr<StoragePartitionImpl> weak_partition_ptr) {
+  return weak_partition_ptr ? weak_partition_ptr->browser_context() : nullptr;
+}
+
 }  // namespace
 
 // Class to own the NetworkContext wrapping a storage partitions
@@ -992,6 +1006,21 @@ void StoragePartitionImpl::OnCanSendDomainReliabilityUpload(
       blink::mojom::PermissionStatus::GRANTED);
 }
 
+void StoragePartitionImpl::OnClearSiteData(uint32_t process_id,
+                                           int32_t routing_id,
+                                           const GURL& url,
+                                           const std::string& header_value,
+                                           int load_flags,
+                                           OnClearSiteDataCallback callback) {
+  auto browser_context_getter =
+      base::BindRepeating(GetBrowserContext, weak_factory_.GetWeakPtr());
+  auto web_contents_getter =
+      base::BindRepeating(GetWebContents, process_id, routing_id);
+  ClearSiteDataHandler::HandleHeader(browser_context_getter,
+                                     web_contents_getter, url, header_value,
+                                     load_flags, std::move(callback));
+}
+
 void StoragePartitionImpl::ClearDataImpl(
     uint32_t remove_mask,
     uint32_t quota_storage_remove_mask,

content/browser/storage_partition_impl.h

@@ -171,6 +171,12 @@ class CONTENT_EXPORT StoragePartitionImpl
   void OnCanSendDomainReliabilityUpload(
       const GURL& origin,
       OnCanSendDomainReliabilityUploadCallback callback) override;
+  void OnClearSiteData(uint32_t process_id,
+                       int32_t routing_id,
+                       const GURL& url,
+                       const std::string& header_value,
+                       int load_flags,
+                       OnClearSiteDataCallback callback) override;
 
   scoped_refptr<URLLoaderFactoryGetter> url_loader_factory_getter() {
     return url_loader_factory_getter_;

services/network/network_service_network_delegate.cc

@@ -205,8 +205,7 @@ int NetworkServiceNetworkDelegate::HandleClearSiteDataHeader(
     const net::HttpResponseHeaders* original_response_headers) {
   DCHECK(base::FeatureList::IsEnabled(network::features::kNetworkService));
   DCHECK(request);
-  if (!original_response_headers ||
-      !network_context_->network_service()->client())
+  if (!original_response_headers || !network_context_->client())
     return net::OK;
 
   URLLoader* url_loader = URLLoader::ForRequest(*request);
@@ -218,7 +217,7 @@ int NetworkServiceNetworkDelegate::HandleClearSiteDataHeader(
                                                       &header_value))
     return net::OK;
 
-  network_context_->network_service()->client()->OnClearSiteData(
+  network_context_->client()->OnClearSiteData(
       url_loader->GetProcessId(), url_loader->GetRenderFrameId(),
       request->url(), header_value, request->load_flags(),
       base::BindOnce(&NetworkServiceNetworkDelegate::FinishedClearSiteData,

services/network/network_service_unittest.cc

@@ -43,6 +43,7 @@
 #include "services/network/public/cpp/network_switches.h"
 #include "services/network/public/mojom/net_log.mojom.h"
 #include "services/network/public/mojom/network_change_manager.mojom.h"
+#include "services/network/public/mojom/network_context.mojom.h"
 #include "services/network/public/mojom/network_service.mojom.h"
 #include "services/network/test/test_network_service_client.h"
 #include "services/network/test/test_url_loader_client.h"
@@ -1411,30 +1412,23 @@ class NetworkServiceNetworkDelegateTest : public NetworkServiceTest {
   DISALLOW_COPY_AND_ASSIGN(NetworkServiceNetworkDelegateTest);
 };
 
-class ClearSiteDataNetworkServiceClient : public TestNetworkServiceClient {
+class ClearSiteDataNetworkContextClient : public mojom::NetworkContextClient {
  public:
-  explicit ClearSiteDataNetworkServiceClient(
-      mojom::NetworkServiceClientRequest request)
-      : TestNetworkServiceClient(std::move(request)) {}
-  ~ClearSiteDataNetworkServiceClient() override = default;
-
-  // Needed these two cookie overrides to avoid NOTREACHED().
-  void OnCookiesRead(int process_id,
-                     int routing_id,
-                     const GURL& url,
-                     const GURL& first_party_url,
-                     const net::CookieList& cookie_list,
-                     bool blocked_by_policy) override {}
-
-  void OnCookieChange(int process_id,
-                      int routing_id,
-                      const GURL& url,
-                      const GURL& first_party_url,
-                      const net::CanonicalCookie& cookie,
-                      bool blocked_by_policy) override {}
-
-  void OnClearSiteData(int process_id,
-                       int routing_id,
+  explicit ClearSiteDataNetworkContextClient(
+      mojom::NetworkContextClientRequest request)
+      : binding_(this, std::move(request)) {}
+  ~ClearSiteDataNetworkContextClient() override = default;
+
+  void OnCanSendReportingReports(
+      const std::vector<url::Origin>& origins,
+      OnCanSendReportingReportsCallback callback) override {}
+
+  void OnCanSendDomainReliabilityUpload(
+      const GURL& origin,
+      OnCanSendDomainReliabilityUploadCallback callback) override {}
+
+  void OnClearSiteData(uint32_t process_id,
+                       int32_t routing_id,
                        const GURL& url,
                        const std::string& header_value,
                        int load_flags,
@@ -1445,6 +1439,7 @@ class ClearSiteDataNetworkServiceClient : public TestNetworkServiceClient {
   }
 
   int on_clear_site_data_counter() const { return on_clear_site_data_counter_; }
+
   const std::string& last_on_clear_site_data_header_value() const {
     return last_on_clear_site_data_header_value_;
   }
@@ -1457,29 +1452,28 @@ class ClearSiteDataNetworkServiceClient : public TestNetworkServiceClient {
  private:
   int on_clear_site_data_counter_ = 0;
   std::string last_on_clear_site_data_header_value_;
+  mojo::Binding<mojom::NetworkContextClient> binding_;
 };
 
 // Check that |NetworkServiceNetworkDelegate| handles Clear-Site-Data header
-// w/ and w/o |NetworkServiceCient|.
-TEST_F(NetworkServiceNetworkDelegateTest, ClearSiteDataNetworkServiceCient) {
+// w/ and w/o |NetworkContextCient|.
+TEST_F(NetworkServiceNetworkDelegateTest, ClearSiteDataNetworkContextCient) {
   const char kClearCookiesHeader[] = "Clear-Site-Data: \"cookies\"";
   CreateNetworkContext();
 
-  // Null |NetworkServiceCient|. The request should complete without being
+  // Null |NetworkContextCient|. The request should complete without being
   // deferred.
-  EXPECT_EQ(nullptr, service()->client());
   GURL url = https_server()->GetURL("/foo");
   url = AddQuery(url, "header", kClearCookiesHeader);
   LoadURL(url);
   EXPECT_EQ(net::OK, client()->completion_status().error_code);
 
-  // With |NetworkServiceCient|. The request should go through
-  // |ClearSiteDataNetworkServiceClient| and complete.
-  mojom::NetworkServiceClientPtr client_ptr;
-  auto client_impl = std::make_unique<ClearSiteDataNetworkServiceClient>(
+  // With |NetworkContextCient|. The request should go through
+  // |ClearSiteDataNetworkContextClient| and complete.
+  mojom::NetworkContextClientPtr client_ptr;
+  auto client_impl = std::make_unique<ClearSiteDataNetworkContextClient>(
       mojo::MakeRequest(&client_ptr));
-  service()->SetClient(std::move(client_ptr),
-                       network::mojom::NetworkServiceParams::New());
+  network_context_->SetClient(std::move(client_ptr));
   url = https_server()->GetURL("/bar");
   url = AddQuery(url, "header", kClearCookiesHeader);
   EXPECT_EQ(0, client_impl->on_clear_site_data_counter());
@@ -1494,11 +1488,10 @@ TEST_F(NetworkServiceNetworkDelegateTest, HandleClearSiteDataHeaders) {
   const char kClearCookiesHeader[] = "Clear-Site-Data: \"cookies\"";
   CreateNetworkContext();
 
-  mojom::NetworkServiceClientPtr client_ptr;
-  auto client_impl = std::make_unique<ClearSiteDataNetworkServiceClient>(
+  mojom::NetworkContextClientPtr client_ptr;
+  auto client_impl = std::make_unique<ClearSiteDataNetworkContextClient>(
       mojo::MakeRequest(&client_ptr));
-  service()->SetClient(std::move(client_ptr),
-                       network::mojom::NetworkServiceParams::New());
+  network_context_->SetClient(std::move(client_ptr));
 
   // |passed_header_value| are only checked if |should_call_client| is true.
   const struct TestCase {

services/network/public/mojom/network_context.mojom

@@ -494,6 +494,17 @@ interface NetworkContextClient {
 
   // Checks if a Domain Reliability report can be uploaded for the given origin.
   OnCanSendDomainReliabilityUpload(url.mojom.Url origin) => (bool allowed);
+
+  // Called when the Clear-Site-Data header has been received. The callback
+  // should be run after the data is deleted.
+  // https://www.w3.org/TR/clear-site-data/
+  // TODO(crbug.com/876931): We might want to move header parsing work to
+  // Network Service for security concerns (e.g. |header_value| => booleans).
+  OnClearSiteData(uint32 process_id,
+                  int32 routing_id,
+                  url.mojom.Url url,
+                  string header_value,
+                  int32 load_flags) => ();
 };
 
 // Represents a distinct context for making network requests, with its own

services/network/public/mojom/network_service.mojom

@@ -148,17 +148,6 @@ interface NetworkServiceClient {
   // receive the next update.
   OnLoadingStateUpdate(array<LoadInfo> infos) => ();
 
-  // Called when the Clear-Site-Data header has been received. The callback
-  // should be run after the data is deleted.
-  // https://www.w3.org/TR/clear-site-data/
-  // TODO(crbug.com/876931): We might want to move header parsing work to
-  // Network Service for security concerns (e.g. |header_value| => booleans).
-  OnClearSiteData(int32 process_id,
-                  int32 routing_id,
-                  url.mojom.Url url,
-                  string header_value,
-                  int32 load_flags) => ();
-
   // Called on every request completion to update the network traffic annotation
   // ID, and the total bytes received and sent.
   // |network_traffic_annotation_id_hash| represents the hash of unique tag that

services/network/test/test_network_service_client.cc

@@ -112,16 +112,6 @@ void TestNetworkServiceClient::OnLoadingStateUpdate(
     std::vector<mojom::LoadInfoPtr> infos,
     OnLoadingStateUpdateCallback callback) {}
 
-void TestNetworkServiceClient::OnClearSiteData(
-    int process_id,
-    int routing_id,
-    const GURL& url,
-    const std::string& header_value,
-    int load_flags,
-    OnClearSiteDataCallback callback) {
-  NOTREACHED();
-}
-
 void TestNetworkServiceClient::OnDataUseUpdate(
     int32_t network_traffic_annotation_id_hash,
     int64_t recv_bytes,

services/network/test/test_network_service_client.h

@@ -73,12 +73,6 @@ class TestNetworkServiceClient : public network::mojom::NetworkServiceClient {
                              OnFileUploadRequestedCallback callback) override;
   void OnLoadingStateUpdate(std::vector<mojom::LoadInfoPtr> infos,
                             OnLoadingStateUpdateCallback callback) override;
-  void OnClearSiteData(int process_id,
-                       int routing_id,
-                       const GURL& url,
-                       const std::string& header_value,
-                       int load_flags,
-                       OnClearSiteDataCallback callback) override;
   void OnDataUseUpdate(int32_t network_traffic_annotation_id_hash,
                        int64_t recv_bytes,
                        int64_t sent_bytes) override;