chromeos: Remove ProxyResolutionServiceProvider::Delegate.

Remove the Delegate interface from
ProxyResolutionServiceProvider. This code lives in
//chrome/browser/chromeos/dbus now, so it's able to access
profiles directly.

Bug: 843392
Change-Id: I1fe04762b260fdd4f4cc792f8921c085f1ded7b8
Reviewed-on: https://chromium-review.googlesource.com/1187727Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Dan Erat <derat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585686}

chrome/browser/chromeos/BUILD.gn

@@ -595,8 +595,6 @@ source_set("chromeos") {
     "customization/customization_wallpaper_util.h",
     "dbus/chrome_features_service_provider.cc",
     "dbus/chrome_features_service_provider.h",
-    "dbus/chrome_proxy_resolution_service_provider_delegate.cc",
-    "dbus/chrome_proxy_resolution_service_provider_delegate.h",
     "dbus/component_updater_service_provider.cc",
     "dbus/component_updater_service_provider.h",
     "dbus/drive_file_stream_service_provider.cc",

chrome/browser/chromeos/chrome_browser_main_chromeos.cc

@@ -46,7 +46,6 @@
 #include "chrome/browser/chromeos/arc/voice_interaction/voice_interaction_controller_client.h"
 #include "chrome/browser/chromeos/boot_times_recorder.h"
 #include "chrome/browser/chromeos/dbus/chrome_features_service_provider.h"
-#include "chrome/browser/chromeos/dbus/chrome_proxy_resolution_service_provider_delegate.h"
 #include "chrome/browser/chromeos/dbus/component_updater_service_provider.h"
 #include "chrome/browser/chromeos/dbus/drive_file_stream_service_provider.h"
 #include "chrome/browser/chromeos/dbus/kiosk_info_service_provider.h"
@@ -328,9 +327,7 @@ class DBusServices {
     proxy_resolution_service_ = CrosDBusService::Create(
         kNetworkProxyServiceName, dbus::ObjectPath(kNetworkProxyServicePath),
         CrosDBusService::CreateServiceProviderList(
-            std::make_unique<ProxyResolutionServiceProvider>(
-                std::make_unique<
-                    ChromeProxyResolutionServiceProviderDelegate>())));
+            std::make_unique<ProxyResolutionServiceProvider>()));
 
     kiosk_info_service_ = CrosDBusService::Create(
         kKioskAppServiceName, dbus::ObjectPath(kKioskAppServicePath),

chrome/browser/chromeos/dbus/chrome_proxy_resolution_service_provider_delegate.cc

-// Copyright 2014 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 "chrome/browser/chromeos/dbus/chrome_proxy_resolution_service_provider_delegate.h"
-
-#include "chrome/browser/profiles/profile_manager.h"
-#include "net/url_request/url_request_context_getter.h"
-
-namespace chromeos {
-
-ChromeProxyResolutionServiceProviderDelegate::
-    ChromeProxyResolutionServiceProviderDelegate() {}
-
-ChromeProxyResolutionServiceProviderDelegate::
-    ~ChromeProxyResolutionServiceProviderDelegate() {}
-
-scoped_refptr<net::URLRequestContextGetter>
-ChromeProxyResolutionServiceProviderDelegate::GetRequestContext() {
-  Profile* profile = ProfileManager::GetPrimaryUserProfile();
-  return profile->GetRequestContext();
-}
-
-}  // namespace chromeos

chrome/browser/chromeos/dbus/chrome_proxy_resolution_service_provider_delegate.h

-// Copyright 2014 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 CHROME_BROWSER_CHROMEOS_DBUS_CHROME_PROXY_RESOLUTION_SERVICE_PROVIDER_DELEGATE_H_
-#define CHROME_BROWSER_CHROMEOS_DBUS_CHROME_PROXY_RESOLUTION_SERVICE_PROVIDER_DELEGATE_H_
-
-#include "base/macros.h"
-#include "chrome/browser/chromeos/dbus/proxy_resolution_service_provider.h"
-
-namespace chromeos {
-
-// Chrome's implementation of ProxyResolutionServiceProvider::Delegate.
-class ChromeProxyResolutionServiceProviderDelegate
-    : public ProxyResolutionServiceProvider::Delegate {
- public:
-  ChromeProxyResolutionServiceProviderDelegate();
-  ~ChromeProxyResolutionServiceProviderDelegate() override;
-
-  // ProxyResolutionServiceProvider::Delegate:
-  scoped_refptr<net::URLRequestContextGetter> GetRequestContext() override;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(ChromeProxyResolutionServiceProviderDelegate);
-};
-
-}  // namespace chromeos
-
-#endif  // CHROME_BROWSER_CHROMEOS_DBUS_CHROME_PROXY_RESOLUTION_SERVICE_PROVIDER_DELEGATE_H_

chrome/browser/chromeos/dbus/proxy_resolution_service_provider.cc

@@ -11,6 +11,8 @@
 #include "base/bind_helpers.h"
 #include "base/macros.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_manager.h"
 #include "dbus/bus.h"
 #include "dbus/message.h"
 #include "net/base/net_errors.h"
@@ -63,10 +65,8 @@ struct ProxyResolutionServiceProvider::Request {
   DISALLOW_COPY_AND_ASSIGN(Request);
 };
 
-ProxyResolutionServiceProvider::ProxyResolutionServiceProvider(
-    std::unique_ptr<Delegate> delegate)
-    : delegate_(std::move(delegate)),
-      origin_thread_(base::ThreadTaskRunnerHandle::Get()),
+ProxyResolutionServiceProvider::ProxyResolutionServiceProvider()
+    : origin_thread_(base::ThreadTaskRunnerHandle::Get()),
       weak_ptr_factory_(this) {}
 
 ProxyResolutionServiceProvider::~ProxyResolutionServiceProvider() {
@@ -118,7 +118,9 @@ void ProxyResolutionServiceProvider::ResolveProxy(
   std::unique_ptr<dbus::Response> response =
       dbus::Response::FromMethodCall(method_call);
   scoped_refptr<net::URLRequestContextGetter> context_getter =
-      delegate_->GetRequestContext();
+      request_context_getter_for_test_
+          ? request_context_getter_for_test_
+          : ProfileManager::GetPrimaryUserProfile()->GetRequestContext();
 
   std::unique_ptr<Request> request = std::make_unique<Request>(
       source_url, std::move(response), response_sender, context_getter);

chrome/browser/chromeos/dbus/proxy_resolution_service_provider.h

@@ -56,21 +56,14 @@ namespace chromeos {
 class ProxyResolutionServiceProvider
     : public CrosDBusService::ServiceProviderInterface {
  public:
-  // Delegate interface providing additional resources to
-  // ProxyResolutionServiceProvider.
-  // TODO(derat): Move the delegate into this class.
-  class CHROMEOS_EXPORT Delegate {
-   public:
-    virtual ~Delegate() {}
-
-    // Returns the request context used to perform proxy resolution.
-    // Always called on UI thread.
-    virtual scoped_refptr<net::URLRequestContextGetter> GetRequestContext() = 0;
-  };
-
-  explicit ProxyResolutionServiceProvider(std::unique_ptr<Delegate> delegate);
+  ProxyResolutionServiceProvider();
   ~ProxyResolutionServiceProvider() override;
 
+  void set_request_context_getter_for_test(
+      const scoped_refptr<net::URLRequestContextGetter>& getter) {
+    request_context_getter_for_test_ = getter;
+  }
+
   // CrosDBusService::ServiceProviderInterface:
   void Start(scoped_refptr<dbus::ExportedObject> exported_object) override;
 
@@ -118,9 +111,9 @@ class ProxyResolutionServiceProvider
   // information to the client over D-Bus.
   void NotifyProxyResolved(std::unique_ptr<Request> request);
 
-  std::unique_ptr<Delegate> delegate_;
   scoped_refptr<dbus::ExportedObject> exported_object_;
   scoped_refptr<base::SingleThreadTaskRunner> origin_thread_;
+  scoped_refptr<net::URLRequestContextGetter> request_context_getter_for_test_;
   base::WeakPtrFactory<ProxyResolutionServiceProvider> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ProxyResolutionServiceProvider);

chrome/browser/chromeos/dbus/proxy_resolution_service_provider_unittest.cc

@@ -115,42 +115,57 @@ class TestProxyResolverFactory : public net::ProxyResolverFactory {
   DISALLOW_COPY_AND_ASSIGN(TestProxyResolverFactory);
 };
 
-// Test ProxyResolutionServiceProvider::Delegate implementation.
-class TestDelegate : public ProxyResolutionServiceProvider::Delegate {
+}  // namespace
+
+class ProxyResolutionServiceProviderTest : public testing::Test {
  public:
-  explicit TestDelegate(
-      const scoped_refptr<base::SingleThreadTaskRunner>& network_task_runner,
-      net::ProxyResolver* proxy_resolver)
-      : proxy_resolver_(proxy_resolver),
-        context_getter_(
-            new net::TestURLRequestContextGetter(network_task_runner)) {
-    network_task_runner->PostTask(
-        FROM_HERE,
-        base::BindOnce(
-            &TestDelegate::CreateProxyResolutionServiceOnNetworkThread,
-            base::Unretained(this)));
-    RunPendingTasks(network_task_runner);
-  }
+  ProxyResolutionServiceProviderTest() : network_thread_("NetworkThread") {
+    CHECK(network_thread_.Start());
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner =
+        network_thread_.task_runner();
 
-  ~TestDelegate() override {
-    context_getter_->GetNetworkTaskRunner()->PostTask(
+    task_runner->PostTask(
         FROM_HERE,
-        base::BindOnce(&TestDelegate::DeleteProxyServiceOnNetworkThread,
+        base::BindOnce(&ProxyResolutionServiceProviderTest ::
+                           CreateProxyResolutionServiceOnNetworkThread,
                        base::Unretained(this)));
-    RunPendingTasks(context_getter_->GetNetworkTaskRunner());
+    RunPendingTasks(task_runner);
+
+    service_provider_ = std::make_unique<ProxyResolutionServiceProvider>();
+    service_provider_->set_request_context_getter_for_test(context_getter_);
+
+    test_helper_.SetUp(
+        kNetworkProxyServiceName, dbus::ObjectPath(kNetworkProxyServicePath),
+        kNetworkProxyServiceInterface, kNetworkProxyServiceResolveProxyMethod,
+        service_provider_.get());
   }
 
-  // ProxyResolutionServiceProvider::Delegate:
-  scoped_refptr<net::URLRequestContextGetter> GetRequestContext() override {
-    return context_getter_;
+  ~ProxyResolutionServiceProviderTest() override {
+    test_helper_.TearDown();
+
+    // URLRequestContextGetter posts a task to delete itself to its task runner,
+    // so give it a chance to do that.
+    service_provider_.reset();
+    network_thread_.task_runner()->PostTask(
+        FROM_HERE, base::BindOnce(&ProxyResolutionServiceProviderTest::
+                                      DeleteProxyServiceOnNetworkThread,
+                                  base::Unretained(this)));
+    RunPendingTasks(network_thread_.task_runner());
+
+    network_thread_.Stop();
   }
 
- private:
+ protected:
   // Helper method for the constructor that initializes
   // |proxy_resolution_service_| and injects it into |context_getter_|'s
-  // context.
+  // context. Also initializes |context_getter_| and |proxy_resolver_|.
   void CreateProxyResolutionServiceOnNetworkThread() {
-    CHECK(context_getter_->GetNetworkTaskRunner()->BelongsToCurrentThread());
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner =
+        network_thread_.task_runner();
+    CHECK(task_runner->BelongsToCurrentThread());
+
+    context_getter_ = new net::TestURLRequestContextGetter(task_runner);
+    proxy_resolver_ = std::make_unique<TestProxyResolver>(task_runner);
 
     // Setting a mandatory PAC URL makes |proxy_resolution_service_| query
     // |proxy_resolver_| and also lets us generate
@@ -162,7 +177,7 @@ class TestDelegate : public ProxyResolutionServiceProvider::Delegate {
         std::make_unique<net::ProxyConfigServiceFixed>(
             net::ProxyConfigWithAnnotation(config,
                                            TRAFFIC_ANNOTATION_FOR_TESTS)),
-        std::make_unique<TestProxyResolverFactory>(proxy_resolver_),
+        std::make_unique<TestProxyResolverFactory>(proxy_resolver_.get()),
         nullptr /* net_log */);
     context_getter_->GetURLRequestContext()->set_proxy_resolution_service(
         proxy_resolution_service_.get());
@@ -170,51 +185,11 @@ class TestDelegate : public ProxyResolutionServiceProvider::Delegate {
 
   // Helper method for the destructor that resets |proxy_resolution_service_|.
   void DeleteProxyServiceOnNetworkThread() {
-    CHECK(context_getter_->GetNetworkTaskRunner()->BelongsToCurrentThread());
+    CHECK(network_thread_.task_runner()->BelongsToCurrentThread());
     proxy_resolution_service_.reset();
+    context_getter_ = nullptr;
   }
 
-  net::ProxyResolver* proxy_resolver_;  // Not owned.
-
-  // Created, used, and destroyed on the network thread (since
-  // net::ProxyResolutionService is thread-affine (uses ThreadChecker)).
-  std::unique_ptr<net::ProxyResolutionService> proxy_resolution_service_;
-
-  scoped_refptr<net::TestURLRequestContextGetter> context_getter_;
-
-  DISALLOW_COPY_AND_ASSIGN(TestDelegate);
-};
-
-}  // namespace
-
-class ProxyResolutionServiceProviderTest : public testing::Test {
- public:
-  ProxyResolutionServiceProviderTest() : network_thread_("NetworkThread") {
-    CHECK(network_thread_.Start());
-
-    proxy_resolver_ =
-        std::make_unique<TestProxyResolver>(network_thread_.task_runner());
-    service_provider_ = std::make_unique<ProxyResolutionServiceProvider>(
-        std::make_unique<TestDelegate>(network_thread_.task_runner(),
-                                       proxy_resolver_.get()));
-    test_helper_.SetUp(
-        kNetworkProxyServiceName, dbus::ObjectPath(kNetworkProxyServicePath),
-        kNetworkProxyServiceInterface, kNetworkProxyServiceResolveProxyMethod,
-        service_provider_.get());
-  }
-
-  ~ProxyResolutionServiceProviderTest() override {
-    test_helper_.TearDown();
-
-    // URLRequestContextGetter posts a task to delete itself to its task runner,
-    // so give it a chance to do that.
-    service_provider_.reset();
-    RunPendingTasks(network_thread_.task_runner());
-
-    network_thread_.Stop();
-  }
-
- protected:
   // Makes a D-Bus call to |service_provider_|'s ResolveProxy method and returns
   // the response.
   std::unique_ptr<dbus::Response> CallMethod(const std::string& source_url) {
@@ -229,6 +204,12 @@ class ProxyResolutionServiceProviderTest : public testing::Test {
   base::Thread network_thread_;
 
   std::unique_ptr<TestProxyResolver> proxy_resolver_;
+
+  // Created, used, and destroyed on the network thread (since
+  // net::ProxyResolutionService is thread-affine (uses ThreadChecker)).
+  std::unique_ptr<net::ProxyResolutionService> proxy_resolution_service_;
+
+  scoped_refptr<net::TestURLRequestContextGetter> context_getter_;
   std::unique_ptr<ProxyResolutionServiceProvider> service_provider_;
   ServiceProviderTestHelper test_helper_;