Move ClearSiteData to AuthenticationAndCertificateObserver.

Move another message to the frame specific handler for an URLLoader.

BUG=1173710

Change-Id: Id00ba3fc9d890be2b7bcac5246b7f6fd49a5b44a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2720522
Reviewed-by: Robert Ogden <robertogden@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#859052}

chrome/browser/chromeos/wilco_dtc_supportd/wilco_dtc_supportd_network_context.cc

@@ -109,6 +109,14 @@ void WilcoDtcSupportdNetworkContextImpl::OnAuthRequired(
   auth_challenge_responder_remote->OnAuthCredentials(base::nullopt);
 }
 
+void WilcoDtcSupportdNetworkContextImpl::OnClearSiteData(
+    const GURL& url,
+    const std::string& header_value,
+    int32_t load_flags,
+    OnClearSiteDataCallback callback) {
+  std::move(callback).Run();
+}
+
 void WilcoDtcSupportdNetworkContextImpl::Clone(
     mojo::PendingReceiver<network::mojom::AuthenticationAndCertificateObserver>
         observer) {

chrome/browser/chromeos/wilco_dtc_supportd/wilco_dtc_supportd_network_context.h

@@ -70,6 +70,10 @@ class WilcoDtcSupportdNetworkContextImpl
       const scoped_refptr<net::HttpResponseHeaders>& head_headers,
       mojo::PendingRemote<network::mojom::AuthChallengeResponder>
           auth_challenge_responder) override;
+  void OnClearSiteData(const GURL& url,
+                       const std::string& header_value,
+                       int32_t load_flags,
+                       OnClearSiteDataCallback callback) override;
   void Clone(mojo::PendingReceiver<
              network::mojom::AuthenticationAndCertificateObserver> listener)
       override;

chrome/browser/prefetch/prefetch_proxy/prefetch_proxy_network_context_client.cc

@@ -34,16 +34,6 @@ void PrefetchProxyNetworkContextClient::OnCanSendDomainReliabilityUpload(
   std::move(callback).Run(false);
 }
 
-void PrefetchProxyNetworkContextClient::OnClearSiteData(
-    int32_t process_id,
-    int32_t routing_id,
-    const GURL& url,
-    const std::string& header_value,
-    int load_flags,
-    OnClearSiteDataCallback callback) {
-  std::move(callback).Run();
-}
-
 #if defined(OS_ANDROID)
 void PrefetchProxyNetworkContextClient::OnGenerateHttpNegotiateAuthToken(
     const std::string& server_auth_token,

chrome/browser/prefetch/prefetch_proxy/prefetch_proxy_network_context_client.h

@@ -30,12 +30,6 @@ class PrefetchProxyNetworkContextClient
   void OnCanSendDomainReliabilityUpload(
       const GURL& origin,
       OnCanSendDomainReliabilityUploadCallback callback) override;
-  void OnClearSiteData(int32_t process_id,
-                       int32_t routing_id,
-                       const GURL& url,
-                       const std::string& header_value,
-                       int load_flags,
-                       OnClearSiteDataCallback callback) override;
 #if defined(OS_ANDROID)
   void OnGenerateHttpNegotiateAuthToken(
       const std::string& server_auth_token,

content/browser/network_context_client_base_impl.cc

@@ -103,16 +103,6 @@ void NetworkContextClientBase::OnCanSendDomainReliabilityUpload(
   std::move(callback).Run(false);
 }
 
-void NetworkContextClientBase::OnClearSiteData(
-    int32_t process_id,
-    int32_t routing_id,
-    const GURL& url,
-    const std::string& header_value,
-    int load_flags,
-    OnClearSiteDataCallback callback) {
-  std::move(callback).Run();
-}
-
 #if defined(OS_ANDROID)
 void NetworkContextClientBase::OnGenerateHttpNegotiateAuthToken(
     const std::string& server_auth_token,

content/browser/storage_partition_impl.cc

@@ -1836,13 +1836,13 @@ void StoragePartitionImpl::OnCanSendDomainReliabilityUpload(
       blink::mojom::PermissionStatus::GRANTED);
 }
 
-void StoragePartitionImpl::OnClearSiteData(int32_t process_id,
-                                           int32_t routing_id,
-                                           const GURL& url,
+void StoragePartitionImpl::OnClearSiteData(const GURL& url,
                                            const std::string& header_value,
                                            int load_flags,
                                            OnClearSiteDataCallback callback) {
   DCHECK(initialized_);
+  int process_id = auth_cert_observers_.current_context().process_id;
+  int routing_id = auth_cert_observers_.current_context().routing_id;
   auto browser_context_getter = base::BindRepeating(
       GetBrowserContextFromStoragePartition, weak_factory_.GetWeakPtr());
   auto web_contents_getter = base::BindRepeating(
@@ -2551,6 +2551,7 @@ StoragePartitionImpl::GetURLLoaderFactoryForBrowserProcessInternal(
   // Corb requests are likely made on behalf of untrusted renderers.
   if (!corb_enabled)
     params->is_trusted = true;
+  params->auth_cert_observer = CreateAuthCertObserverForServiceWorker();
   params->disable_web_security =
       base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kDisableWebSecurity);

content/browser/storage_partition_impl.h

@@ -250,12 +250,6 @@ class CONTENT_EXPORT StoragePartitionImpl
   void OnCanSendDomainReliabilityUpload(
       const GURL& origin,
       OnCanSendDomainReliabilityUploadCallback callback) override;
-  void OnClearSiteData(int32_t process_id,
-                       int32_t routing_id,
-                       const GURL& url,
-                       const std::string& header_value,
-                       int load_flags,
-                       OnClearSiteDataCallback callback) override;
 #if defined(OS_ANDROID)
   void OnGenerateHttpNegotiateAuthToken(
       const std::string& server_auth_token,
@@ -294,6 +288,10 @@ class CONTENT_EXPORT StoragePartitionImpl
       const scoped_refptr<net::HttpResponseHeaders>& head_headers,
       mojo::PendingRemote<network::mojom::AuthChallengeResponder>
           auth_challenge_responder) override;
+  void OnClearSiteData(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_;

content/public/browser/network_context_client_base.h

@@ -33,12 +33,6 @@ class CONTENT_EXPORT NetworkContextClientBase
   void OnCanSendDomainReliabilityUpload(
       const GURL& origin,
       OnCanSendDomainReliabilityUploadCallback callback) override;
-  void OnClearSiteData(int32_t process_id,
-                       int32_t routing_id,
-                       const GURL& url,
-                       const std::string& header_value,
-                       int load_flags,
-                       OnClearSiteDataCallback callback) override;
 #if defined(OS_ANDROID)
   void OnGenerateHttpNegotiateAuthToken(
       const std::string& server_auth_token,

services/network/network_service_network_delegate.cc

@@ -321,21 +321,24 @@ int NetworkServiceNetworkDelegate::HandleClearSiteDataHeader(
     net::CompletionOnceCallback callback,
     const net::HttpResponseHeaders* original_response_headers) {
   DCHECK(request);
-  if (!original_response_headers || !network_context_->client())
+  if (!original_response_headers)
     return net::OK;
 
   URLLoader* url_loader = URLLoader::ForRequest(*request);
   if (!url_loader)
     return net::OK;
 
+  auto* auth_cert_observer = url_loader->GetAuthCertObserver();
+  if (!auth_cert_observer)
+    return net::OK;
+
   std::string header_value;
   if (!original_response_headers->GetNormalizedHeader(kClearSiteDataHeader,
                                                       &header_value)) {
     return net::OK;
   }
 
-  network_context_->client()->OnClearSiteData(
-      url_loader->GetProcessId(), url_loader->GetRenderFrameId(),
+  auth_cert_observer->OnClearSiteData(
       request->url(), header_value, request->load_flags(),
       base::BindOnce(&NetworkServiceNetworkDelegate::FinishedClearSiteData,
                      weak_ptr_factory_.GetWeakPtr(), request->GetWeakPtr(),

services/network/network_service_unittest.cc

@@ -58,6 +58,7 @@
 #include "services/network/public/mojom/network_context.mojom.h"
 #include "services/network/public/mojom/network_service.mojom.h"
 #include "services/network/test/fake_test_cert_verifier_params_factory.h"
+#include "services/network/test/test_auth_cert_observer.h"
 #include "services/network/test/test_network_context_client.h"
 #include "services/network/test/test_network_service_client.h"
 #include "services/network/test/test_url_loader_client.h"
@@ -1408,24 +1409,32 @@ class NetworkServiceNetworkDelegateTest : public NetworkServiceTest {
         std::move(context_params));
   }
 
-  void LoadURL(const GURL& url, int options = mojom::kURLLoadOptionNone) {
+  void LoadURL(const GURL& url,
+               int options = mojom::kURLLoadOptionNone,
+               mojo::PendingRemote<mojom::AuthenticationAndCertificateObserver>
+                   auth_cert_observer = mojo::NullRemote()) {
     ResourceRequest request;
     request.url = url;
     request.method = "GET";
     request.request_initiator = url::Origin();
-    StartLoadingURL(request, 0 /* process_id */, options);
+    StartLoadingURL(request, 0 /* process_id */, options,
+                    std::move(auth_cert_observer));
     client_->RunUntilComplete();
   }
 
-  void StartLoadingURL(const ResourceRequest& request,
-                       uint32_t process_id,
-                       int options = mojom::kURLLoadOptionNone) {
+  void StartLoadingURL(
+      const ResourceRequest& request,
+      uint32_t process_id,
+      int options = mojom::kURLLoadOptionNone,
+      mojo::PendingRemote<mojom::AuthenticationAndCertificateObserver>
+          auth_cert_observer = mojo::NullRemote()) {
     client_.reset(new TestURLLoaderClient());
     mojo::Remote<mojom::URLLoaderFactory> loader_factory;
     mojom::URLLoaderFactoryParamsPtr params =
         mojom::URLLoaderFactoryParams::New();
     params->process_id = process_id;
     params->is_corb_enabled = false;
+    params->auth_cert_observer = std::move(auth_cert_observer);
     network_context_->CreateURLLoaderFactory(
         loader_factory.BindNewPipeAndPassReceiver(), std::move(params));
 
@@ -1483,16 +1492,12 @@ class NetworkServiceNetworkDelegateTest : public NetworkServiceTest {
   DISALLOW_COPY_AND_ASSIGN(NetworkServiceNetworkDelegateTest);
 };
 
-class ClearSiteDataNetworkContextClient : public TestNetworkContextClient {
+class ClearSiteDataAuthCertObserver : public TestAuthCertObserver {
  public:
-  explicit ClearSiteDataNetworkContextClient(
-      mojo::PendingReceiver<mojom::NetworkContextClient> receiver)
-      : receiver_(this, std::move(receiver)) {}
-  ~ClearSiteDataNetworkContextClient() override = default;
-
-  void OnClearSiteData(int32_t process_id,
-                       int32_t routing_id,
-                       const GURL& url,
+  explicit ClearSiteDataAuthCertObserver() = default;
+  ~ClearSiteDataAuthCertObserver() override = default;
+
+  void OnClearSiteData(const GURL& url,
                        const std::string& header_value,
                        int load_flags,
                        OnClearSiteDataCallback callback) override {
@@ -1515,34 +1520,30 @@ class ClearSiteDataNetworkContextClient : public TestNetworkContextClient {
  private:
   int on_clear_site_data_counter_ = 0;
   std::string last_on_clear_site_data_header_value_;
-  mojo::Receiver<mojom::NetworkContextClient> receiver_;
 };
 
 // Check that |NetworkServiceNetworkDelegate| handles Clear-Site-Data header
 // w/ and w/o |NetworkContextCient|.
-TEST_F(NetworkServiceNetworkDelegateTest, ClearSiteDataNetworkContextCient) {
+TEST_F(NetworkServiceNetworkDelegateTest, ClearSiteDataObserver) {
   const char kClearCookiesHeader[] = "Clear-Site-Data: \"cookies\"";
   CreateNetworkContext();
 
-  // Null |NetworkContextCient|. The request should complete without being
+  // Null |auth_cert_observer|. The request should complete without being
   // deferred.
   GURL url = https_server()->GetURL("/foo");
   url = AddQuery(url, "header", kClearCookiesHeader);
   LoadURL(url);
   EXPECT_EQ(net::OK, client()->completion_status().error_code);
 
-  // With |NetworkContextCient|. The request should go through
-  // |ClearSiteDataNetworkContextClient| and complete.
-  mojo::PendingRemote<mojom::NetworkContextClient> client_remote;
-  auto client_impl = std::make_unique<ClearSiteDataNetworkContextClient>(
-      client_remote.InitWithNewPipeAndPassReceiver());
-  network_context_->SetClient(std::move(client_remote));
+  // With |auth_cert_observer|. The request should go through
+  // |ClearSiteDataAuthCertObserver| and complete.
+  ClearSiteDataAuthCertObserver clear_site_observer;
   url = https_server()->GetURL("/bar");
   url = AddQuery(url, "header", kClearCookiesHeader);
-  EXPECT_EQ(0, client_impl->on_clear_site_data_counter());
-  LoadURL(url);
+  EXPECT_EQ(0, clear_site_observer.on_clear_site_data_counter());
+  LoadURL(url, mojom::kURLLoadOptionNone, clear_site_observer.Bind());
   EXPECT_EQ(net::OK, client()->completion_status().error_code);
-  EXPECT_EQ(1, client_impl->on_clear_site_data_counter());
+  EXPECT_EQ(1, clear_site_observer.on_clear_site_data_counter());
 }
 
 // Check that headers are handled and passed to the client correctly.
@@ -1551,10 +1552,7 @@ TEST_F(NetworkServiceNetworkDelegateTest, HandleClearSiteDataHeaders) {
   const char kClearCookiesHeader[] = "Clear-Site-Data: \"cookies\"";
   CreateNetworkContext();
 
-  mojo::PendingRemote<mojom::NetworkContextClient> client_remote;
-  auto client_impl = std::make_unique<ClearSiteDataNetworkContextClient>(
-      client_remote.InitWithNewPipeAndPassReceiver());
-  network_context_->SetClient(std::move(client_remote));
+  ClearSiteDataAuthCertObserver clear_site_observer;
 
   // |passed_header_value| are only checked if |should_call_client| is true.
   const struct TestCase {
@@ -1592,18 +1590,18 @@ TEST_F(NetworkServiceNetworkDelegateTest, HandleClearSiteDataHeaders) {
 
     GURL url = https_server()->GetURL("/foo");
     url = AddQuery(url, "header", test_case.response_headers);
-    EXPECT_EQ(0, client_impl->on_clear_site_data_counter());
-    LoadURL(url);
+    EXPECT_EQ(0, clear_site_observer.on_clear_site_data_counter());
+    LoadURL(url, mojom::kURLLoadOptionNone, clear_site_observer.Bind());
 
     EXPECT_EQ(net::OK, client()->completion_status().error_code);
     if (test_case.should_call_client) {
-      EXPECT_EQ(1, client_impl->on_clear_site_data_counter());
+      EXPECT_EQ(1, clear_site_observer.on_clear_site_data_counter());
       EXPECT_EQ(test_case.passed_header_value,
-                client_impl->last_on_clear_site_data_header_value());
+                clear_site_observer.last_on_clear_site_data_header_value());
     } else {
-      EXPECT_EQ(0, client_impl->on_clear_site_data_counter());
+      EXPECT_EQ(0, clear_site_observer.on_clear_site_data_counter());
     }
-    client_impl->ClearOnClearSiteDataCounter();
+    clear_site_observer.ClearOnClearSiteDataCounter();
   }
 }
 

services/network/public/mojom/auth_and_certificate_observer.mojom

@@ -67,6 +67,8 @@ interface AuthChallengeResponder {
 // This interface is used by an URLLoaderFactory to provide notification
 // of authentication and certificate events. This is typically implemented
 // by a trusted process such as the browser process.
+// TODO(dtapuska): Rename this interface. It is used for more than
+// authentication and certificate now.
 interface AuthenticationAndCertificateObserver {
   // Called when an SSL certificate is encountered.
   // The callback argument is a net::ERROR value. If it's net::OK, then the
@@ -113,6 +115,16 @@ interface AuthenticationAndCertificateObserver {
       HttpResponseHeaders? head_headers,
       pending_remote<AuthChallengeResponder> auth_challenge_responder);
 
+  // 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(url.mojom.Url url,
+                  string header_value,
+                  int32 load_flags) => ();
+
+
   // Used by the NetworkService to create a copy of this observer.
   // (e.g. when creating an observer for URLLoader from URLLoaderFactory's
   // observer).

services/network/public/mojom/network_context.mojom

@@ -713,17 +713,6 @@ 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(int32 process_id,
-                  int32 routing_id,
-                  url.mojom.Url url,
-                  string header_value,
-                  int32 load_flags) => ();
-
   // Called to generate an auth token for SPNEGO authentication on Android.
   [EnableIf=is_android]
   OnGenerateHttpNegotiateAuthToken(string server_auth_token, bool can_delegate,

services/network/test/test_auth_cert_observer.cc

@@ -43,6 +43,13 @@ void TestAuthCertObserver::OnAuthRequired(
     mojo::PendingRemote<mojom::AuthChallengeResponder>
         auth_challenge_responder) {}
 
+void TestAuthCertObserver::OnClearSiteData(const GURL& url,
+                                           const std::string& header_value,
+                                           int32_t load_flags,
+                                           OnClearSiteDataCallback callback) {
+  std::move(callback).Run();
+}
+
 void TestAuthCertObserver::Clone(
     mojo::PendingReceiver<AuthenticationAndCertificateObserver> observer) {
   receivers_.Add(this, std::move(observer));

services/network/test/test_auth_cert_observer.h

@@ -46,6 +46,10 @@ class TestAuthCertObserver
       const scoped_refptr<net::HttpResponseHeaders>& head_headers,
       mojo::PendingRemote<mojom::AuthChallengeResponder>
           auth_challenge_responder) override;
+  void OnClearSiteData(const GURL& url,
+                       const std::string& header_value,
+                       int32_t load_flags,
+                       OnClearSiteDataCallback callback) override;
   void Clone(mojo::PendingReceiver<AuthenticationAndCertificateObserver>
                  observer) override;
 

services/network/test/test_network_context_client.h

@@ -40,12 +40,6 @@ class TestNetworkContextClient : public network::mojom::NetworkContextClient {
   void OnCanSendDomainReliabilityUpload(
       const GURL& origin,
       OnCanSendDomainReliabilityUploadCallback callback) override {}
-  void OnClearSiteData(int32_t process_id,
-                       int32_t routing_id,
-                       const GURL& url,
-                       const std::string& header_value,
-                       int32_t load_flags,
-                       OnClearSiteDataCallback callback) override {}
 #if defined(OS_ANDROID)
   void OnGenerateHttpNegotiateAuthToken(
       const std::string& server_auth_token,