Fix DRP syncing config client to multiple NetworkContexts

When multiple network contexts are created for a profile (e.g. from
storage partitions for extensions), DRP stopped syncing the config to
the first network context. This probably didn't show up before
http://crrev.com/c/1746682 because of timing issues with the thread
hops.

Also cleans up the mojo types to use the new versions.

Bug: 996334
Change-Id: I1c92e7da6fd9a1df7f46cfc5f8e435fb1fc11d72
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1764897Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Robert Ogden <robertogden@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#689723}

chrome/browser/net/profile_network_context_service.cc

@@ -559,10 +559,10 @@ ProfileNetworkContextService::CreateNetworkContextParams(
     auto* drp_settings =
         DataReductionProxyChromeSettingsFactory::GetForBrowserContext(profile_);
     if (drp_settings) {
-      network::mojom::CustomProxyConfigClientPtrInfo config_client_info;
+      mojo::Remote<network::mojom::CustomProxyConfigClient> config_client;
       network_context_params->custom_proxy_config_client_request =
-          mojo::MakeRequest(&config_client_info);
-      drp_settings->SetCustomProxyConfigClient(std::move(config_client_info));
+          config_client.BindNewPipeAndPassReceiver();
+      drp_settings->AddCustomProxyConfigClient(std::move(config_client));
     }
   }
 

components/data_reduction_proxy/core/browser/data_reduction_proxy_service.cc

@@ -265,8 +265,8 @@ void DataReductionProxyService::SetProxyPrefs(bool enabled, bool at_startup) {
 
   // If Data Saver is disabled, reset data reduction proxy state.
   if (!enabled) {
-    if (proxy_config_client_)
-      proxy_config_client_->ClearBadProxiesCache();
+    for (auto& client : proxy_config_clients_)
+      client->ClearBadProxiesCache();
   }
 }
 
@@ -319,9 +319,9 @@ void DataReductionProxyService::SetIgnoreLongTermBlackListRules(
   settings_->SetIgnoreLongTermBlackListRules(ignore_long_term_black_list_rules);
 }
 
-void DataReductionProxyService::SetCustomProxyConfigClient(
-    network::mojom::CustomProxyConfigClientPtrInfo config_client_info) {
-  proxy_config_client_.Bind(std::move(config_client_info));
+void DataReductionProxyService::AddCustomProxyConfigClient(
+    mojo::Remote<network::mojom::CustomProxyConfigClient> config_client) {
+  proxy_config_clients_.Add(std::move(config_client));
   UpdateCustomProxyConfig();
 }
 
@@ -430,8 +430,8 @@ void DataReductionProxyService::MarkProxiesAsBad(
     }
   }
 
-  proxy_config_client_->MarkProxiesAsBad(bypass_duration, bad_proxies,
-                                         std::move(callback));
+  for (auto& client : proxy_config_clients_)
+    client->MarkProxiesAsBad(bypass_duration, bad_proxies, std::move(callback));
 }
 
 void DataReductionProxyService::AddThrottleConfigObserver(
@@ -446,13 +446,12 @@ void DataReductionProxyService::Clone(
 }
 
 void DataReductionProxyService::UpdateCustomProxyConfig() {
-  if (!proxy_config_client_)
-    return;
-
-  proxy_config_client_->OnCustomProxyConfigUpdated(CreateCustomProxyConfig(
+  network::mojom::CustomProxyConfigPtr config = CreateCustomProxyConfig(
       !base::FeatureList::IsEnabled(
           features::kDataReductionProxyDisableProxyFailedWarmup),
-      config_->GetProxiesForHttp()));
+      config_->GetProxiesForHttp());
+  for (auto& client : proxy_config_clients_)
+    client->OnCustomProxyConfigUpdated(config->Clone());
 }
 
 void DataReductionProxyService::UpdateThrottleConfig() {

components/data_reduction_proxy/core/browser/data_reduction_proxy_service.h

@@ -23,6 +23,7 @@
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy.mojom.h"
 #include "components/data_use_measurement/core/data_use_measurement.h"
 #include "mojo/public/cpp/bindings/interface_ptr_set.h"
+#include "mojo/public/cpp/bindings/remote_set.h"
 #include "net/nqe/effective_connection_type.h"
 #include "services/network/public/cpp/network_connection_tracker.h"
 #include "services/network/public/cpp/network_quality_tracker.h"
@@ -157,10 +158,10 @@ class DataReductionProxyService
   // Sends the given |headers| to |DataReductionProxySettings|.
   void UpdateProxyRequestHeaders(const net::HttpRequestHeaders& headers);
 
-  // Sets a config client that can be used to update Data Reduction Proxy
-  // settings when the network service is enabled.
-  void SetCustomProxyConfigClient(
-      network::mojom::CustomProxyConfigClientPtrInfo config_client_info);
+  // Adds a config client that can be used to update Data Reduction Proxy
+  // settings.
+  void AddCustomProxyConfigClient(
+      mojo::Remote<network::mojom::CustomProxyConfigClient> config_client);
 
   // mojom::DataReductionProxy implementation:
   void MarkProxiesAsBad(base::TimeDelta bypass_duration,
@@ -321,7 +322,9 @@ class DataReductionProxyService
   // is unavailable, then the destruction will happen on the UI thread.
   std::unique_ptr<NetworkPropertiesManager> network_properties_manager_;
 
-  network::mojom::CustomProxyConfigClientPtr proxy_config_client_;
+  // The set of clients that will get updates about changes to the proxy config.
+  mojo::RemoteSet<network::mojom::CustomProxyConfigClient>
+      proxy_config_clients_;
 
   mojo::BindingSet<mojom::DataReductionProxy> drp_bindings_;
 

components/data_reduction_proxy/core/browser/data_reduction_proxy_service_unittest.cc

@@ -82,8 +82,9 @@ class TestCustomProxyConfigClient
     : public network::mojom::CustomProxyConfigClient {
  public:
   TestCustomProxyConfigClient(
-      network::mojom::CustomProxyConfigClientRequest request)
-      : binding_(this, std::move(request)) {}
+      mojo::PendingReceiver<network::mojom::CustomProxyConfigClient>
+          pending_receiver)
+      : receiver_(this, std::move(pending_receiver)) {}
 
   // network::mojom::CustomProxyConfigClient implementation:
   void OnCustomProxyConfigUpdated(
@@ -101,7 +102,7 @@ class TestCustomProxyConfigClient
   int num_clear_cache_calls = 0;
 
  private:
-  mojo::Binding<network::mojom::CustomProxyConfigClient> binding_;
+  mojo::Receiver<network::mojom::CustomProxyConfigClient> receiver_;
 };
 
 TEST_F(DataReductionProxyServiceTest, TestResetBadProxyListOnDisableDataSaver) {
@@ -113,10 +114,11 @@ TEST_F(DataReductionProxyServiceTest, TestResetBadProxyListOnDisableDataSaver) {
   drp_test_context->SetDataReductionProxyEnabled(true);
   drp_test_context->InitSettings();
 
-  network::mojom::CustomProxyConfigClientPtrInfo client_ptr_info;
-  TestCustomProxyConfigClient client(mojo::MakeRequest(&client_ptr_info));
-  drp_test_context->data_reduction_proxy_service()->SetCustomProxyConfigClient(
-      std::move(client_ptr_info));
+  mojo::Remote<network::mojom::CustomProxyConfigClient> client_remote;
+  TestCustomProxyConfigClient client(
+      client_remote.BindNewPipeAndPassReceiver());
+  drp_test_context->data_reduction_proxy_service()->AddCustomProxyConfigClient(
+      std::move(client_remote));
   base::RunLoop().RunUntilIdle();
 
   // Turn Data Saver off.
@@ -158,9 +160,10 @@ TEST_F(DataReductionProxyServiceTest, TestCustomProxyConfigClient) {
   service->config_client()->ApplySerializedConfig(
       CreateEncodedConfig({DataReductionProxyServer(proxy_server)}));
 
-  network::mojom::CustomProxyConfigClientPtrInfo client_ptr_info;
-  TestCustomProxyConfigClient client(mojo::MakeRequest(&client_ptr_info));
-  service->SetCustomProxyConfigClient(std::move(client_ptr_info));
+  mojo::Remote<network::mojom::CustomProxyConfigClient> client_remote;
+  TestCustomProxyConfigClient client(
+      client_remote.BindNewPipeAndPassReceiver());
+  service->AddCustomProxyConfigClient(std::move(client_remote));
   base::RunLoop().RunUntilIdle();
 
   EXPECT_EQ(client.config->rules.proxies_for_http.Get(), proxy_server);
@@ -178,10 +181,11 @@ TEST_F(DataReductionProxyServiceTest, TestCustomProxyConfigUpdatedOnECTChange) {
       ->ReportEffectiveConnectionTypeForTesting(
           net::EFFECTIVE_CONNECTION_TYPE_4G);
 
-  network::mojom::CustomProxyConfigClientPtrInfo client_ptr_info;
-  TestCustomProxyConfigClient client(mojo::MakeRequest(&client_ptr_info));
-  drp_test_context->data_reduction_proxy_service()->SetCustomProxyConfigClient(
-      std::move(client_ptr_info));
+  mojo::Remote<network::mojom::CustomProxyConfigClient> client_remote;
+  TestCustomProxyConfigClient client(
+      client_remote.BindNewPipeAndPassReceiver());
+  drp_test_context->data_reduction_proxy_service()->AddCustomProxyConfigClient(
+      std::move(client_remote));
   base::RunLoop().RunUntilIdle();
 
   std::string value;
@@ -206,9 +210,10 @@ TEST_F(DataReductionProxyServiceTest,
   DataReductionProxyService* service =
       drp_test_context->data_reduction_proxy_service();
 
-  network::mojom::CustomProxyConfigClientPtrInfo client_ptr_info;
-  TestCustomProxyConfigClient client(mojo::MakeRequest(&client_ptr_info));
-  service->SetCustomProxyConfigClient(std::move(client_ptr_info));
+  mojo::Remote<network::mojom::CustomProxyConfigClient> client_remote;
+  TestCustomProxyConfigClient client(
+      client_remote.BindNewPipeAndPassReceiver());
+  service->AddCustomProxyConfigClient(std::move(client_remote));
   base::RunLoop().RunUntilIdle();
 
   std::string value;
@@ -241,9 +246,10 @@ TEST_F(DataReductionProxyServiceTest,
   service->config_client()->ApplySerializedConfig(
       CreateEncodedConfig({DataReductionProxyServer(proxy_server1)}));
 
-  network::mojom::CustomProxyConfigClientPtrInfo client_ptr_info;
-  TestCustomProxyConfigClient client(mojo::MakeRequest(&client_ptr_info));
-  service->SetCustomProxyConfigClient(std::move(client_ptr_info));
+  mojo::Remote<network::mojom::CustomProxyConfigClient> client_remote;
+  TestCustomProxyConfigClient client(
+      client_remote.BindNewPipeAndPassReceiver());
+  service->AddCustomProxyConfigClient(std::move(client_remote));
   base::RunLoop().RunUntilIdle();
 
   EXPECT_EQ(client.config->rules.proxies_for_http.Get(), proxy_server1);
@@ -276,9 +282,10 @@ TEST_F(DataReductionProxyServiceTest,
       CreateEncodedConfig({DataReductionProxyServer(core_proxy_server),
                            DataReductionProxyServer(second_proxy_server)}));
 
-  network::mojom::CustomProxyConfigClientPtrInfo client_ptr_info;
-  TestCustomProxyConfigClient client(mojo::MakeRequest(&client_ptr_info));
-  service->SetCustomProxyConfigClient(std::move(client_ptr_info));
+  mojo::Remote<network::mojom::CustomProxyConfigClient> client_remote;
+  TestCustomProxyConfigClient client(
+      client_remote.BindNewPipeAndPassReceiver());
+  service->AddCustomProxyConfigClient(std::move(client_remote));
   base::RunLoop().RunUntilIdle();
 
   net::ProxyConfig::ProxyRules expected_rules;
@@ -302,9 +309,10 @@ TEST_F(DataReductionProxyServiceTest, TestCustomProxyConfigProperties) {
       &network_properties_manager);
   service->config()->UpdateConfigForTesting(true, true, true);
 
-  network::mojom::CustomProxyConfigClientPtrInfo client_ptr_info;
-  TestCustomProxyConfigClient client(mojo::MakeRequest(&client_ptr_info));
-  service->SetCustomProxyConfigClient(std::move(client_ptr_info));
+  mojo::Remote<network::mojom::CustomProxyConfigClient> client_remote;
+  TestCustomProxyConfigClient client(
+      client_remote.BindNewPipeAndPassReceiver());
+  service->AddCustomProxyConfigClient(std::move(client_remote));
   base::RunLoop().RunUntilIdle();
 
   EXPECT_TRUE(client.config->assume_https_proxies_support_quic);

components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.cc

@@ -84,9 +84,10 @@ void DataReductionProxySettings::InitDataReductionProxySettings(
   for (auto& observer : observers_)
     observer.OnSettingsInitialized();
 
-  if (proxy_config_client_) {
-    data_reduction_proxy_service_->SetCustomProxyConfigClient(
-        std::move(proxy_config_client_));
+  while (!proxy_config_clients_.empty()) {
+    data_reduction_proxy_service_->AddCustomProxyConfigClient(
+        std::move(proxy_config_clients_.back()));
+    proxy_config_clients_.pop_back();
   }
 }
 
@@ -325,15 +326,15 @@ void DataReductionProxySettings::RemoveDataReductionProxySettingsObserver(
   observers_.RemoveObserver(observer);
 }
 
-void DataReductionProxySettings::SetCustomProxyConfigClient(
-    network::mojom::CustomProxyConfigClientPtrInfo proxy_config_client) {
+void DataReductionProxySettings::AddCustomProxyConfigClient(
+    mojo::Remote<network::mojom::CustomProxyConfigClient> proxy_config_client) {
   if (data_reduction_proxy_service_) {
-    data_reduction_proxy_service_->SetCustomProxyConfigClient(
+    data_reduction_proxy_service_->AddCustomProxyConfigClient(
         std::move(proxy_config_client));
     return;
   }
 
-  proxy_config_client_ = std::move(proxy_config_client);
+  proxy_config_clients_.push_back(std::move(proxy_config_client));
 }
 
 // Metrics methods

components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h

@@ -21,6 +21,7 @@
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_server.h"
 #include "components/prefs/pref_change_registrar.h"
 #include "components/prefs/pref_member.h"
+#include "mojo/public/cpp/bindings/remote.h"
 #include "net/http/http_request_headers.h"
 #include "services/network/public/mojom/network_context.mojom.h"
 #include "url/gurl.h"
@@ -178,10 +179,11 @@ class DataReductionProxySettings {
   void RemoveDataReductionProxySettingsObserver(
       DataReductionProxySettingsObserver* observer);
 
-  // Sets a config client that can be used to update Data Reduction Proxy
-  // settings when the network service is enabled.
-  void SetCustomProxyConfigClient(
-      network::mojom::CustomProxyConfigClientPtrInfo proxy_config_client);
+  // Addds a config client that can be used to update Data Reduction Proxy
+  // settings.
+  void AddCustomProxyConfigClient(
+      mojo::Remote<network::mojom::CustomProxyConfigClient>
+          proxy_config_client);
 
   DataReductionProxyService* data_reduction_proxy_service() {
     return data_reduction_proxy_service_.get();
@@ -297,7 +299,10 @@ class DataReductionProxySettings {
   // The headers to use for requests to the proxy server.
   net::HttpRequestHeaders proxy_request_headers_;
 
-  network::mojom::CustomProxyConfigClientPtrInfo proxy_config_client_;
+  // A list of CustomProxyConfigClients that may have been added before
+  // the DataReductionProxyService was available.
+  std::vector<mojo::Remote<network::mojom::CustomProxyConfigClient>>
+      proxy_config_clients_;
 
   base::ThreadChecker thread_checker_;