Postpone binding of mojom::DataReductionProxy

Each Web Worker runs on a separate worker thread, and each worker has an
associated URLLoaderThrottleProvider.  When spawning a nested worker,
the outer worker's throttle provider is cloned on the outer worker's
thread and then used on the inner worker's thread.

To accomodate this scenario:

 - Postpone the creation of DataReductionProxyThrottleManager until a
   moment when we are running on the target thread.  This makes sure
   the DataReductionProxyThrottleConfigObserver binding is used on the
   same thread that it's created on.

 - Postpone the binding of DataReductionProxy until a moment when we
   are running on the target thread.  DataReductionProxyPtrInfo
   serves as a holder of the interface request that can move between
   threads.

Bug: 942011
Change-Id: Ieed17fb499bbcb0545e3e0323f751c1ea55a33d5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1565873Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Commit-Queue: Wojciech Dzierżanowski <wdzierzanowski@opera.com>
Cr-Commit-Position: refs/heads/master@{#654846}

chrome/browser/chrome_content_browser_client.cc

@@ -29,6 +29,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/system/sys_info.h"
 #include "base/task/post_task.h"
+#include "base/threading/sequenced_task_runner_handle.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
 #include "chrome/app/chrome_content_browser_overlay_manifest.h"
@@ -210,6 +211,7 @@
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_features.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_params.h"
+#include "components/data_reduction_proxy/core/common/data_reduction_proxy_throttle_manager.h"
 #include "components/dom_distiller/core/dom_distiller_switches.h"
 #include "components/dom_distiller/core/url_constants.h"
 #include "components/error_page/common/error_page_switches.h"
@@ -1101,7 +1103,11 @@ blink::UserAgentMetadata GetUserAgentMetadata() {
 
 ChromeContentBrowserClient::ChromeContentBrowserClient(
     StartupData* startup_data)
-    : startup_data_(startup_data), weak_factory_(this) {
+    : data_reduction_proxy_throttle_manager_(
+          nullptr,
+          base::OnTaskRunnerDeleter(nullptr)),
+      startup_data_(startup_data),
+      weak_factory_(this) {
 #if BUILDFLAG(ENABLE_PLUGINS)
   for (size_t i = 0; i < base::size(kPredefinedAllowedDevChannelOrigins); ++i)
     allowed_dev_channel_origins_.insert(kPredefinedAllowedDevChannelOrigins[i]);
@@ -4709,6 +4715,15 @@ ChromeContentBrowserClient::CreateURLLoaderThrottles(
   if (chrome_navigation_ui_data && io_data &&
       io_data->data_reduction_proxy_io_data() &&
       data_reduction_proxy::params::IsEnabledWithNetworkService()) {
+    if (!data_reduction_proxy_throttle_manager_) {
+      data_reduction_proxy_throttle_manager_ = std::unique_ptr<
+          data_reduction_proxy::DataReductionProxyThrottleManager,
+          base::OnTaskRunnerDeleter>(
+          new data_reduction_proxy::DataReductionProxyThrottleManager(
+              io_data->data_reduction_proxy_io_data(),
+              io_data->data_reduction_proxy_io_data()->CreateThrottleConfig()),
+          base::OnTaskRunnerDeleter(base::SequencedTaskRunnerHandle::Get()));
+    }
     net::HttpRequestHeaders headers;
     data_reduction_proxy::DataReductionProxyRequestOptions* request_options =
         io_data->data_reduction_proxy_io_data()->request_options();
@@ -4721,8 +4736,7 @@ ChromeContentBrowserClient::CreateURLLoaderThrottles(
     request_options->AddPageIDRequestHeader(&headers, page_id);
     result.push_back(std::make_unique<
                      data_reduction_proxy::DataReductionProxyURLLoaderThrottle>(
-        headers,
-        io_data->data_reduction_proxy_io_data()->GetThrottleManager()));
+        headers, data_reduction_proxy_throttle_manager_.get()));
   }
 
 #if defined(SAFE_BROWSING_DB_LOCAL) || defined(SAFE_BROWSING_DB_REMOTE)

chrome/browser/chrome_content_browser_client.h

@@ -18,6 +18,7 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
+#include "base/sequenced_task_runner.h"
 #include "build/build_config.h"
 #include "chrome/browser/chrome_service.h"
 #include "chrome/browser/startup_data.h"
@@ -51,6 +52,7 @@ class QuotaPermissionContext;
 
 namespace data_reduction_proxy {
 class DataReductionProxyData;
+class DataReductionProxyThrottleManager;
 }  // namespace data_reduction_proxy
 
 namespace previews {
@@ -670,6 +672,10 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
   scoped_refptr<safe_browsing::UrlCheckerDelegate>
       safe_browsing_url_checker_delegate_;
 
+  std::unique_ptr<data_reduction_proxy::DataReductionProxyThrottleManager,
+                  base::OnTaskRunnerDeleter>
+      data_reduction_proxy_throttle_manager_;
+
   std::unique_ptr<service_manager::BinderRegistry> frame_interfaces_;
   std::unique_ptr<
       service_manager::BinderRegistryWithArgs<content::RenderFrameHost*>>

chrome/browser/data_reduction_proxy/data_reduction_proxy_browsertest.cc

@@ -9,6 +9,7 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
+#include "base/strings/utf_string_conversions.h"
 #include "base/synchronization/lock.h"
 #include "base/test/bind_test_util.h"
 #include "base/test/metrics/histogram_tester.h"
@@ -95,6 +96,14 @@ std::unique_ptr<net::test_server::HttpResponse> IncrementRequestCount(
   return std::make_unique<net::test_server::BasicHttpResponse>();
 }
 
+// Given a |request| to a proxy server, returns the destination host name.
+std::string GetDestinationHost(const net::test_server::HttpRequest& request) {
+  const auto it = request.headers.find("Host");
+  if (it == request.headers.end())
+    return {};
+  return it->second;
+}
+
 void SimulateNetworkChange(network::mojom::ConnectionType type) {
   if (base::FeatureList::IsEnabled(network::features::kNetworkService) &&
       !content::IsInProcessNetworkService()) {
@@ -1326,4 +1335,58 @@ IN_PROC_BROWSER_TEST_F(DataReductionProxyBrowsertest,
   EXPECT_EQ(kExpectedLog2, event_log.GetAndReset());
 }
 
+IN_PROC_BROWSER_TEST_F(DataReductionProxyBrowsertest, NestedWebWorker) {
+  // Nested Web Workers exercise URLLoaderThrottles in interesting ways. Each
+  // worker runs on a separate thread and each one has an associated
+  // URLLoaderThrottleProvider. When spawning a nested worker, the outer
+  // worker's throttle provider is cloned on the outer worker's thread and then
+  // used on the inner worker's thread. This test verifies that the mojo
+  // connections between the throttles and DRP are set up correctly given the
+  // non-trivial threading scenario.
+  constexpr char kHtml[] = R"(
+    <html><body><script language="javascript">
+      function workerImpl() {
+        function nestedWorkerImpl() {
+          postMessage('done');
+        }
+        var blob = new Blob(['(' + nestedWorkerImpl.toString() + ')()'],
+                            { type: 'application/javascript' });
+        var nestedWorker = new Worker(URL.createObjectURL(blob));
+        nestedWorker.onmessage = (event) => postMessage(event.data);
+      }
+      var blob = new Blob(['(' + workerImpl.toString() + ')()'],
+                          { type: 'application/javascript' });
+      var worker = new Worker(URL.createObjectURL(blob));
+      worker.onmessage = (event) => document.title = event.data;
+    </script></body></html>
+  )";
+  constexpr char kDestinationHost[] = "some.host";
+
+  net::EmbeddedTestServer drp_server;
+  drp_server.RegisterRequestHandler(base::BindLambdaForTesting(
+      [&kHtml, &kDestinationHost](const net::test_server::HttpRequest& request)
+          -> std::unique_ptr<net::test_server::HttpResponse> {
+        if (GetDestinationHost(request) != kDestinationHost)
+          return nullptr;
+
+        auto response = std::make_unique<net::test_server::BasicHttpResponse>();
+        response->set_content(kHtml);
+        return response;
+      }));
+  ASSERT_TRUE(drp_server.Start());
+
+  // Change the DRP configuration so that |drp_server| is the current DRP.
+  SetConfig(CreateConfigForServer(drp_server));
+  SimulateNetworkChange(network::mojom::ConnectionType::CONNECTION_3G);
+  WaitForConfig();
+
+  ui_test_utils::NavigateToURL(browser(),
+                               GURL(std::string("http://") + kDestinationHost));
+
+  const auto kExpectedTitle = base::ASCIIToUTF16("done");
+  content::TitleWatcher title_watcher(
+      browser()->tab_strip_model()->GetActiveWebContents(), kExpectedTitle);
+  EXPECT_EQ(title_watcher.WaitAndGetTitle(), kExpectedTitle);
+}
+
 }  // namespace data_reduction_proxy

chrome/renderer/url_loader_throttle_provider_impl.cc

@@ -125,14 +125,9 @@ URLLoaderThrottleProviderImpl::URLLoaderThrottleProviderImpl(
   }
 
   if (data_reduction_proxy::params::IsEnabledWithNetworkService()) {
-    data_reduction_proxy::mojom::DataReductionProxyPtr drp;
     content::RenderThread::Get()->GetConnector()->BindInterface(
-        content::mojom::kBrowserServiceName, mojo::MakeRequest(&drp));
-
-    data_reduction_proxy_manager_ = std::make_unique<
-        data_reduction_proxy::DataReductionProxyThrottleManager>(
-        std::move(drp),
-        data_reduction_proxy::mojom::DataReductionProxyThrottleConfigPtr());
+        content::mojom::kBrowserServiceName,
+        mojo::MakeRequest(&data_reduction_proxy_info_));
   }
 }
 
@@ -147,9 +142,9 @@ URLLoaderThrottleProviderImpl::URLLoaderThrottleProviderImpl(
   DETACH_FROM_THREAD(thread_checker_);
   if (other.safe_browsing_)
     other.safe_browsing_->Clone(mojo::MakeRequest(&safe_browsing_info_));
-  if (other.data_reduction_proxy_manager_) {
-    data_reduction_proxy_manager_ =
-        other.data_reduction_proxy_manager_->Clone();
+  if (other.data_reduction_proxy_) {
+    other.data_reduction_proxy_->Clone(
+        mojo::MakeRequest(&data_reduction_proxy_info_));
   }
   // An ad_delay_factory_ is created, rather than cloning the existing one.
 }
@@ -159,6 +154,8 @@ URLLoaderThrottleProviderImpl::Clone() {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   if (safe_browsing_info_)
     safe_browsing_.Bind(std::move(safe_browsing_info_));
+  if (data_reduction_proxy_info_)
+    data_reduction_proxy_.Bind(std::move(data_reduction_proxy_info_));
   return base::WrapUnique(new URLLoaderThrottleProviderImpl(*this));
 }
 
@@ -180,7 +177,15 @@ URLLoaderThrottleProviderImpl::CreateThrottles(
   DCHECK(!is_frame_resource ||
          type_ == content::URLLoaderThrottleProviderType::kFrame);
 
-  if (data_reduction_proxy_manager_) {
+  if (data_reduction_proxy::params::IsEnabledWithNetworkService()) {
+    if (data_reduction_proxy_info_)
+      data_reduction_proxy_.Bind(std::move(data_reduction_proxy_info_));
+    if (!data_reduction_proxy_manager_) {
+      data_reduction_proxy_manager_ = std::make_unique<
+          data_reduction_proxy::DataReductionProxyThrottleManager>(
+          data_reduction_proxy_.get(),
+          data_reduction_proxy::mojom::DataReductionProxyThrottleConfigPtr());
+    }
     throttles.push_back(
         std::make_unique<
             data_reduction_proxy::DataReductionProxyURLLoaderThrottle>(

chrome/renderer/url_loader_throttle_provider_impl.h

@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "base/threading/thread_checker.h"
+#include "components/data_reduction_proxy/core/common/data_reduction_proxy.mojom.h"
 #include "components/safe_browsing/common/safe_browsing.mojom.h"
 #include "content/public/renderer/url_loader_throttle_provider.h"
 #include "extensions/buildflags/buildflags.h"
@@ -53,6 +54,9 @@ class URLLoaderThrottleProviderImpl
   safe_browsing::mojom::SafeBrowsingPtrInfo safe_browsing_info_;
   safe_browsing::mojom::SafeBrowsingPtr safe_browsing_;
 
+  data_reduction_proxy::mojom::DataReductionProxyPtrInfo
+      data_reduction_proxy_info_;
+  data_reduction_proxy::mojom::DataReductionProxyPtr data_reduction_proxy_;
   std::unique_ptr<data_reduction_proxy::DataReductionProxyThrottleManager>
       data_reduction_proxy_manager_;
 

components/data_reduction_proxy/content/common/data_reduction_proxy_url_loader_throttle_unittest.cc

@@ -11,7 +11,6 @@
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_server.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_throttle_manager.h"
 #include "content/public/common/previews_state.h"
-#include "mojo/public/cpp/bindings/strong_binding.h"
 #include "net/base/load_flags.h"
 #include "net/http/http_request_headers.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -26,22 +25,8 @@ class MockMojoDataReductionProxy : public mojom::DataReductionProxy {
   void MarkProxiesAsBad(base::TimeDelta bypass_duration,
                         const net::ProxyList& bad_proxies,
                         MarkProxiesAsBadCallback callback) override {
-    ASSERT_FALSE(waiting_for_mark_as_bad_closure_.is_null())
-        << "Unexpected call to MarkProxyAsBadAndRestart";
-
-    std::move(callback).Run();
-
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, std::move(waiting_for_mark_as_bad_closure_));
-  }
-
-  // Wait for MarkProxiesAsBad() to be called, and then reply.
-  void WaitUntilMarkProxiesAsBadCalled() {
-    ASSERT_TRUE(waiting_for_mark_as_bad_closure_.is_null());
-
-    base::RunLoop run_loop;
-    waiting_for_mark_as_bad_closure_ = run_loop.QuitClosure();
-    run_loop.Run();
+    base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
+                                                  std::move(callback));
   }
 
   void AddThrottleConfigObserver(
@@ -50,8 +35,6 @@ class MockMojoDataReductionProxy : public mojom::DataReductionProxy {
   void Clone(mojom::DataReductionProxyRequest request) override {}
 
  private:
-  base::RepeatingClosure waiting_for_mark_as_bad_closure_;
-
   DISALLOW_COPY_AND_ASSIGN(MockMojoDataReductionProxy);
 };
 
@@ -64,7 +47,11 @@ class MockDelegate : public content::URLLoaderThrottle::Delegate {
     FAIL() << "Should not be reached";
   }
 
-  void Resume() override { resume_called++; }
+  void Resume() override {
+    resume_called++;
+    if (resume_callback)
+      std::move(resume_callback).Run();
+  }
 
   void RestartWithFlags(int additional_load_flags) override {
     restart_with_flags_called++;
@@ -75,26 +62,17 @@ class MockDelegate : public content::URLLoaderThrottle::Delegate {
   size_t restart_with_flags_called = 0;
   int restart_additional_load_flags = 0;
 
+  base::OnceClosure resume_callback;
+
   DISALLOW_COPY_AND_ASSIGN(MockDelegate);
 };
 
 // Creates a DataReductionProxyThrottleManager which is bound to a
-// MockMojoDataReductionProxy, and has an initial throttle configuration
+// mojo::DataReductionProxy, and has an initial throttle configuration
 // containing |initial_drp_servers|.
-//
-// If |out_mock_drp| is non-null, it is assigned a non-owned pointer to the
-// underlying mock implementation. This pointer will remain valid while the
-// DataReductionProxyThrottleManager is alive.
 std::unique_ptr<DataReductionProxyThrottleManager> CreateManager(
-    const std::vector<DataReductionProxyServer>& initial_drp_servers = {},
-    MockMojoDataReductionProxy** out_mock_drp = nullptr) {
-  auto mock_drp = std::make_unique<MockMojoDataReductionProxy>();
-  if (out_mock_drp)
-    *out_mock_drp = mock_drp.get();
-
-  mojom::DataReductionProxyPtr drp;
-  mojo::MakeStrongBinding(std::move(mock_drp), mojo::MakeRequest(&drp));
-
+    mojom::DataReductionProxy* mojo_data_reduction_proxy,
+    const std::vector<DataReductionProxyServer>& initial_drp_servers = {}) {
   auto initial_throttle_config =
       initial_drp_servers.empty()
           ? mojom::DataReductionProxyThrottleConfigPtr()
@@ -102,7 +80,7 @@ std::unique_ptr<DataReductionProxyThrottleManager> CreateManager(
                 initial_drp_servers);
 
   return std::make_unique<DataReductionProxyThrottleManager>(
-      std::move(drp), std::move(initial_throttle_config));
+      mojo_data_reduction_proxy, std::move(initial_throttle_config));
 }
 
 DataReductionProxyServer MakeCoreDrpServer(const std::string pac_string) {
@@ -112,12 +90,18 @@ DataReductionProxyServer MakeCoreDrpServer(const std::string pac_string) {
 }
 
 class DataReductionProxyURLLoaderThrottleTest : public ::testing::Test {
+ public:
+  MockMojoDataReductionProxy* mock_mojo_data_reduction_proxy() {
+    return &mock_mojo_data_reduction_proxy_;
+  }
+
  private:
   base::test::ScopedTaskEnvironment scoped_task_environment_;
+  MockMojoDataReductionProxy mock_mojo_data_reduction_proxy_;
 };
 
 TEST_F(DataReductionProxyURLLoaderThrottleTest, AcceptTransformHeaderSet) {
-  auto manager = CreateManager();
+  auto manager = CreateManager(mock_mojo_data_reduction_proxy());
   DataReductionProxyURLLoaderThrottle throttle(net::HttpRequestHeaders(),
                                                manager.get());
   network::ResourceRequest request;
@@ -136,7 +120,7 @@ TEST_F(DataReductionProxyURLLoaderThrottleTest, AcceptTransformHeaderSet) {
 
 TEST_F(DataReductionProxyURLLoaderThrottleTest,
        AcceptTransformHeaderSetForMainFrame) {
-  auto manager = CreateManager();
+  auto manager = CreateManager(mock_mojo_data_reduction_proxy());
   DataReductionProxyURLLoaderThrottle throttle((net::HttpRequestHeaders()),
                                                manager.get());
   network::ResourceRequest request;
@@ -156,7 +140,7 @@ TEST_F(DataReductionProxyURLLoaderThrottleTest,
 
 TEST_F(DataReductionProxyURLLoaderThrottleTest,
        ConstructorHeadersAddedToPostCacheHeaders) {
-  auto manager = CreateManager();
+  auto manager = CreateManager(mock_mojo_data_reduction_proxy());
 
   net::HttpRequestHeaders headers;
   headers.SetHeader("foo", "bar");
@@ -174,7 +158,7 @@ TEST_F(DataReductionProxyURLLoaderThrottleTest,
 }
 
 TEST_F(DataReductionProxyURLLoaderThrottleTest, UseAlternateProxyList) {
-  auto manager = CreateManager();
+  auto manager = CreateManager(mock_mojo_data_reduction_proxy());
   DataReductionProxyURLLoaderThrottle throttle((net::HttpRequestHeaders()),
                                                manager.get());
   network::ResourceRequest request;
@@ -188,7 +172,7 @@ TEST_F(DataReductionProxyURLLoaderThrottleTest, UseAlternateProxyList) {
 }
 
 TEST_F(DataReductionProxyURLLoaderThrottleTest, DontUseAlternateProxyList) {
-  auto manager = CreateManager();
+  auto manager = CreateManager(mock_mojo_data_reduction_proxy());
   DataReductionProxyURLLoaderThrottle throttle((net::HttpRequestHeaders()),
                                                manager.get());
   network::ResourceRequest request;
@@ -201,11 +185,12 @@ TEST_F(DataReductionProxyURLLoaderThrottleTest, DontUseAlternateProxyList) {
   EXPECT_FALSE(request.custom_proxy_use_alternate_proxy_list);
 }
 
-void RestartBypassProxyAndCacheHelper(bool response_came_from_drp) {
+void RestartBypassProxyAndCacheHelper(
+    mojom::DataReductionProxy* mojo_data_reduction_proxy,
+    bool response_came_from_drp) {
   auto drp_server = MakeCoreDrpServer("HTTPS localhost");
 
-  MockMojoDataReductionProxy* drp;
-  auto manager = CreateManager({drp_server}, &drp);
+  auto manager = CreateManager(mojo_data_reduction_proxy, {drp_server});
   MockDelegate delegate;
   DataReductionProxyURLLoaderThrottle throttle((net::HttpRequestHeaders()),
                                                manager.get());
@@ -250,7 +235,8 @@ void RestartBypassProxyAndCacheHelper(bool response_came_from_drp) {
 // Tests that when "Chrome-Proxy: block-once" is received from a DRP response
 // the request is immediately restarted.
 TEST_F(DataReductionProxyURLLoaderThrottleTest, RestartBypassProxyAndCache) {
-  RestartBypassProxyAndCacheHelper(/*response_was_from_drp=*/true);
+  RestartBypassProxyAndCacheHelper(mock_mojo_data_reduction_proxy(),
+                                   /*response_was_from_drp=*/true);
 }
 
 // Tests that when "Chrome-proxy: block-once" is received from a non-DRP server
@@ -258,14 +244,15 @@ TEST_F(DataReductionProxyURLLoaderThrottleTest, RestartBypassProxyAndCache) {
 // restarted).
 TEST_F(DataReductionProxyURLLoaderThrottleTest,
        ChromeProxyDisregardedForNonDrp) {
-  RestartBypassProxyAndCacheHelper(/*response_was_from_drp=*/false);
+  RestartBypassProxyAndCacheHelper(mock_mojo_data_reduction_proxy(),
+                                   /*response_was_from_drp=*/false);
 }
 
 TEST_F(DataReductionProxyURLLoaderThrottleTest,
        DisregardChromeProxyFromDirect) {
   auto drp_server = MakeCoreDrpServer("HTTPS localhost");
 
-  auto manager = CreateManager({drp_server}, nullptr);
+  auto manager = CreateManager(mock_mojo_data_reduction_proxy(), {drp_server});
   MockDelegate delegate;
   DataReductionProxyURLLoaderThrottle throttle((net::HttpRequestHeaders()),
                                                manager.get());
@@ -297,8 +284,7 @@ TEST_F(DataReductionProxyURLLoaderThrottleTest,
 TEST_F(DataReductionProxyURLLoaderThrottleTest, MarkProxyAsBadAndRestart) {
   auto drp_server = MakeCoreDrpServer("HTTPS localhost");
 
-  MockMojoDataReductionProxy* drp;
-  auto manager = CreateManager({drp_server}, &drp);
+  auto manager = CreateManager(mock_mojo_data_reduction_proxy(), {drp_server});
   MockDelegate delegate;
   DataReductionProxyURLLoaderThrottle throttle((net::HttpRequestHeaders()),
                                                manager.get());
@@ -326,9 +312,11 @@ TEST_F(DataReductionProxyURLLoaderThrottleTest, MarkProxyAsBadAndRestart) {
   EXPECT_EQ(0u, delegate.resume_called);
   EXPECT_EQ(0u, delegate.restart_with_flags_called);
 
-  drp->WaitUntilMarkProxiesAsBadCalled();
-
   // The throttle should restart and resume.
+  base::RunLoop run_loop;
+  delegate.resume_callback = run_loop.QuitClosure();
+  run_loop.Run();
+
   EXPECT_EQ(1u, delegate.resume_called);
   EXPECT_EQ(1u, delegate.restart_with_flags_called);
   EXPECT_EQ(0, delegate.restart_additional_load_flags);

components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.cc

@@ -468,18 +468,6 @@ void DataReductionProxyIOData::SetCustomProxyConfigClient(
   UpdateCustomProxyConfig();
 }
 
-DataReductionProxyThrottleManager*
-DataReductionProxyIOData::GetThrottleManager() {
-  if (!throttle_manager_) {
-    mojom::DataReductionProxyPtr drp;
-    Clone(mojo::MakeRequest(&drp));
-    throttle_manager_ = std::make_unique<DataReductionProxyThrottleManager>(
-        std::move(drp), CreateThrottleConfig());
-  }
-
-  return throttle_manager_.get();
-}
-
 void DataReductionProxyIOData::MarkProxiesAsBad(
     base::TimeDelta bypass_duration,
     const net::ProxyList& bad_proxies,

components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h

@@ -50,7 +50,6 @@ class DataReductionProxyConfigurator;
 class DataReductionProxyServer;
 class DataReductionProxyService;
 class NetworkPropertiesManager;
-class DataReductionProxyThrottleManager;
 
 // Contains and initializes all Data Reduction Proxy objects that operate on
 // the IO thread.
@@ -176,7 +175,7 @@ class DataReductionProxyIOData : public mojom::DataReductionProxy {
     return proxy_delegate_.get();
   }
 
-  DataReductionProxyThrottleManager* GetThrottleManager();
+  mojom::DataReductionProxyThrottleConfigPtr CreateThrottleConfig() const;
 
   const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner() const {
     return io_task_runner_;
@@ -271,8 +270,6 @@ class DataReductionProxyIOData : public mojom::DataReductionProxy {
   // config.
   void UpdateThrottleConfig();
 
-  mojom::DataReductionProxyThrottleConfigPtr CreateThrottleConfig() const;
-
   // The type of Data Reduction Proxy client.
   const Client client_;
 
@@ -335,8 +332,6 @@ class DataReductionProxyIOData : public mojom::DataReductionProxy {
   // is unavailable, then the destruction will happen on the UI thread.
   std::unique_ptr<NetworkPropertiesManager> network_properties_manager_;
 
-  std::unique_ptr<DataReductionProxyThrottleManager> throttle_manager_;
-
   // Current estimate of the effective connection type.
   net::EffectiveConnectionType effective_connection_type_;
 

components/data_reduction_proxy/core/common/data_reduction_proxy_throttle_manager.cc

@@ -4,6 +4,7 @@
 
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_throttle_manager.h"
 
+#include "base/memory/ptr_util.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_params.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_server.h"
 
@@ -13,24 +14,29 @@ class HttpRequestHeaders;
 
 namespace data_reduction_proxy {
 
-DataReductionProxyThrottleManager::~DataReductionProxyThrottleManager() =
-    default;
-
 DataReductionProxyThrottleManager::DataReductionProxyThrottleManager(
-    mojom::DataReductionProxyPtr data_reduction_proxy,
+    mojom::DataReductionProxy* data_reduction_proxy,
     mojom::DataReductionProxyThrottleConfigPtr initial_config)
-    : data_reduction_proxy_(std::move(data_reduction_proxy)), binding_(this) {
-  data_reduction_proxy::mojom::DataReductionProxyThrottleConfigObserverPtr
-      observer;
+    : data_reduction_proxy_(data_reduction_proxy), binding_(this) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  mojom::DataReductionProxyThrottleConfigObserverPtr observer;
   binding_.Bind(mojo::MakeRequest(&observer));
+
   data_reduction_proxy_->AddThrottleConfigObserver(std::move(observer));
 
   OnThrottleConfigChanged(std::move(initial_config));
 }
 
+DataReductionProxyThrottleManager::~DataReductionProxyThrottleManager() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+}
+
 base::Optional<DataReductionProxyTypeInfo>
 DataReductionProxyThrottleManager::FindConfiguredDataReductionProxy(
     const net::ProxyServer& proxy_server) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   // TODO(https://crbug.com/721403): The non-NS code also searches through the
   // recently seen proxies, not just the current ones.
   return params::FindConfiguredProxyInVector(proxies_for_http_, proxy_server);
@@ -40,6 +46,8 @@ void DataReductionProxyThrottleManager::MarkProxiesAsBad(
     base::TimeDelta bypass_duration,
     const net::ProxyList& bad_proxies,
     mojom::DataReductionProxy::MarkProxiesAsBadCallback callback) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   // There is no need to handle the case where |callback| is never invoked
   // (possible on connection error). That would imply disconnection from the
   // browser, which is not recoverable.
@@ -47,23 +55,10 @@ void DataReductionProxyThrottleManager::MarkProxiesAsBad(
                                           std::move(callback));
 }
 
-std::unique_ptr<DataReductionProxyThrottleManager>
-DataReductionProxyThrottleManager::Clone() {
-  mojom::DataReductionProxyPtr data_reduction_proxy;
-
-  if (data_reduction_proxy_)
-    data_reduction_proxy_->Clone(mojo::MakeRequest(&data_reduction_proxy));
-
-  auto cloned_config = proxies_for_http_.empty()
-                           ? mojom::DataReductionProxyThrottleConfigPtr()
-                           : CreateConfig(proxies_for_http_);
-
-  return std::make_unique<DataReductionProxyThrottleManager>(
-      std::move(data_reduction_proxy), std::move(cloned_config));
-}
-
 void DataReductionProxyThrottleManager::OnThrottleConfigChanged(
     mojom::DataReductionProxyThrottleConfigPtr config) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   proxies_for_http_.clear();
 
   if (!config)
@@ -75,6 +70,7 @@ void DataReductionProxyThrottleManager::OnThrottleConfigChanged(
   }
 }
 
+// static
 mojom::DataReductionProxyThrottleConfigPtr
 DataReductionProxyThrottleManager::CreateConfig(
     const std::vector<DataReductionProxyServer>& proxies_for_http) {

components/data_reduction_proxy/core/common/data_reduction_proxy_throttle_manager.h

@@ -5,6 +5,7 @@
 #ifndef COMPONENTS_DATA_REDUCTION_PROXY_CORE_COMMON_DATA_REDUCTION_PROXY_THROTTLE_MANAGER_H_
 #define COMPONENTS_DATA_REDUCTION_PROXY_CORE_COMMON_DATA_REDUCTION_PROXY_THROTTLE_MANAGER_H_
 
+#include "base/sequence_checker.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy.mojom.h"
 #include "mojo/public/cpp/bindings/binding.h"
 
@@ -19,10 +20,10 @@ struct DataReductionProxyTypeInfo;
 class DataReductionProxyThrottleManager
     : public mojom::DataReductionProxyThrottleConfigObserver {
  public:
-  // Observes |data_reduction_proxy| for changes to the config, and starts off
-  // with the initial value (possibly empty) |initial_config|.
+  // Observes |data_reduction_proxy| for changes to the config, and starts
+  // off with the initial value (possibly empty) |initial_config|.
   DataReductionProxyThrottleManager(
-      mojom::DataReductionProxyPtr data_reduction_proxy,
+      mojom::DataReductionProxy* data_reduction_proxy_info,
       mojom::DataReductionProxyThrottleConfigPtr initial_config);
 
   ~DataReductionProxyThrottleManager() override;
@@ -35,8 +36,6 @@ class DataReductionProxyThrottleManager
       const net::ProxyList& bad_proxies,
       mojom::DataReductionProxy::MarkProxiesAsBadCallback callback);
 
-  std::unique_ptr<DataReductionProxyThrottleManager> Clone();
-
   // mojom::DataReductionProxyThrottleConfigObserver implementation.
   void OnThrottleConfigChanged(
       mojom::DataReductionProxyThrottleConfigPtr config) override;
@@ -47,7 +46,7 @@ class DataReductionProxyThrottleManager
  private:
   void SetDataReductionProxy(mojom::DataReductionProxyPtr data_reduction_proxy);
 
-  mojom::DataReductionProxyPtr data_reduction_proxy_;
+  mojom::DataReductionProxy* const data_reduction_proxy_;
 
   // The last seen config values.
   std::vector<DataReductionProxyServer> proxies_for_http_;
@@ -56,6 +55,8 @@ class DataReductionProxyThrottleManager
       data_reduction_proxy::mojom::DataReductionProxyThrottleConfigObserver>
       binding_;
 
+  SEQUENCE_CHECKER(sequence_checker_);
+
   DISALLOW_COPY_AND_ASSIGN(DataReductionProxyThrottleManager);
 };