Reland "Convert tcp_device_provider.cc to mojo host resolver"

This is a reland of ac880b3a

Switched to using
ChromeDevToolsManagerDelegate::GetDefaultBrowserContext() which
outlives TCPDeviceProvider. The original was reverted because of a
crash when TCPDeviceProvider tried to access a destroyed
BrowserContext, see http://crbug.com/879060.

Original change's description:
> Convert tcp_device_provider.cc to mojo host resolver
>
> Bug: 874653
> Change-Id: I95b968427344a4c9821c0985ae1f89c153036bcd
> Reviewed-on: https://chromium-review.googlesource.com/1185626
> Commit-Queue: Clark DuVall <cduvall@chromium.org>
> Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#586738}

Bug: 874653
Change-Id: Ia6c00ba51020473f820add8c4257f3bf16fd5b24
Reviewed-on: https://chromium-review.googlesource.com/1197265Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587750}

chrome/browser/devtools/chrome_devtools_manager_delegate.cc

@@ -294,7 +294,8 @@ void ChromeDevToolsManagerDelegate::UpdateDeviceDiscovery() {
       device_manager_ = AndroidDeviceManager::Create();
 
     AndroidDeviceManager::DeviceProviders providers;
-    providers.push_back(new TCPDeviceProvider(remote_locations));
+    providers.push_back(
+        new TCPDeviceProvider(remote_locations, GetDefaultBrowserContext()));
     device_manager_->SetDeviceProviders(providers);
 
     device_discovery_.reset(new DevToolsDeviceDiscovery(device_manager_.get(),

chrome/browser/devtools/device/cast_device_provider.cc

@@ -141,7 +141,11 @@ class CastDeviceProvider::DeviceListerDelegate
   std::unique_ptr<ServiceDiscoveryDeviceLister> device_lister_;
 };
 
-CastDeviceProvider::CastDeviceProvider() : weak_factory_(this) {}
+CastDeviceProvider::CastDeviceProvider(content::BrowserContext* context)
+    : tcp_provider_(base::MakeRefCounted<TCPDeviceProvider>(
+          TCPDeviceProvider::HostPortSet(),
+          context)),
+      weak_factory_(this) {}
 
 CastDeviceProvider::~CastDeviceProvider() {}
 
@@ -157,7 +161,7 @@ void CastDeviceProvider::QueryDevices(const SerialsCallback& callback) {
   std::set<net::HostPortPair> targets;
   for (const auto& device_entry : device_info_map_)
     targets.insert(net::HostPortPair(device_entry.first, kCastInspectPort));
-  tcp_provider_ = new TCPDeviceProvider(targets);
+  tcp_provider_->set_targets(std::move(targets));
   tcp_provider_->QueryDevices(callback);
 }
 

chrome/browser/devtools/device/cast_device_provider.h

@@ -23,7 +23,7 @@ class CastDeviceProvider
     : public AndroidDeviceManager::DeviceProvider,
       public local_discovery::ServiceDiscoveryDeviceLister::Delegate {
  public:
-  CastDeviceProvider();
+  explicit CastDeviceProvider(content::BrowserContext* context);
 
   // DeviceProvider implementation:
   void QueryDevices(const SerialsCallback& callback) override;

chrome/browser/devtools/device/cast_device_provider_unittest.cc

@@ -5,6 +5,8 @@
 #include "chrome/browser/devtools/device/cast_device_provider.h"
 
 #include "chrome/browser/devtools/device/android_device_manager.h"
+#include "chrome/test/base/testing_profile.h"
+#include "content/public/test/test_browser_thread_bundle.h"
 #include "net/base/host_port_pair.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -36,7 +38,10 @@ void DummyCallback(bool* was_run, const DeviceInfo& device_info) {
 }  // namespace
 
 TEST(CastDeviceProviderTest, ServiceDiscovery) {
-  scoped_refptr<CastDeviceProvider> device_provider_ = new CastDeviceProvider();
+  content::TestBrowserThreadBundle thread_bundle;
+  TestingProfile profile;
+  scoped_refptr<CastDeviceProvider> device_provider_ =
+      new CastDeviceProvider(&profile);
 
   // Create a cast service.
   const std::string cast_service_display_name = "FakeCast1337";

chrome/browser/devtools/device/devtools_android_bridge.cc

@@ -331,7 +331,8 @@ static std::set<net::HostPortPair> ParseTargetDiscoveryPreferenceValue(
 }
 
 static scoped_refptr<TCPDeviceProvider> CreateTCPDeviceProvider(
-    const base::ListValue* targetDiscoveryConfig) {
+    const base::ListValue* targetDiscoveryConfig,
+    Profile* profile) {
   base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
   std::set<net::HostPortPair> targets =
       ParseTargetDiscoveryPreferenceValue(targetDiscoveryConfig);
@@ -352,7 +353,7 @@ static scoped_refptr<TCPDeviceProvider> CreateTCPDeviceProvider(
   }
   if (targets.empty())
     return nullptr;
-  return new TCPDeviceProvider(targets);
+  return new TCPDeviceProvider(targets, profile);
 }
 
 void DevToolsAndroidBridge::CreateDeviceProviders() {
@@ -362,7 +363,8 @@ void DevToolsAndroidBridge::CreateDeviceProviders() {
       service->GetBoolean(prefs::kDevToolsDiscoverTCPTargetsEnabled)
           ? service->GetList(prefs::kDevToolsTCPDiscoveryConfig)
           : nullptr;
-  scoped_refptr<TCPDeviceProvider> provider = CreateTCPDeviceProvider(targets);
+  scoped_refptr<TCPDeviceProvider> provider =
+      CreateTCPDeviceProvider(targets, profile_);
   if (tcp_provider_callback_)
     tcp_provider_callback_.Run(provider);
 
@@ -370,7 +372,7 @@ void DevToolsAndroidBridge::CreateDeviceProviders() {
     device_providers.push_back(provider);
 
 #if BUILDFLAG(ENABLE_SERVICE_DISCOVERY)
-  device_providers.push_back(new CastDeviceProvider());
+  device_providers.push_back(new CastDeviceProvider(profile_));
 #endif
 
   device_providers.push_back(new AdbDeviceProvider());

chrome/browser/devtools/device/port_forwarding_browsertest.cc

@@ -85,7 +85,7 @@ IN_PROC_BROWSER_TEST_F(PortForwardingTest,
   AndroidDeviceManager::DeviceProviders device_providers;
 
   device_providers.push_back(
-      TCPDeviceProvider::CreateForLocalhost(kDefaultDebuggingPort));
+      TCPDeviceProvider::CreateForLocalhost(kDefaultDebuggingPort, profile));
   DevToolsAndroidBridge::Factory::GetForProfile(profile)->
       set_device_providers_for_test(device_providers);
 
@@ -145,7 +145,8 @@ IN_PROC_BROWSER_TEST_F(PortForwardingDisconnectTest, DisconnectOnRelease) {
   AndroidDeviceManager::DeviceProviders device_providers;
 
   scoped_refptr<TCPDeviceProvider> self_provider(
-      TCPDeviceProvider::CreateForLocalhost(kAlternativeDebuggingPort));
+      TCPDeviceProvider::CreateForLocalhost(kAlternativeDebuggingPort,
+                                            profile));
   device_providers.push_back(self_provider);
 
   DevToolsAndroidBridge::Factory::GetForProfile(profile)->

chrome/browser/devtools/device/tcp_device_provider.cc

@@ -12,11 +12,14 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "chrome/browser/devtools/device/adb/adb_client_socket.h"
+#include "content/public/browser/storage_partition.h"
+#include "mojo/public/cpp/bindings/binding.h"
 #include "net/base/net_errors.h"
 #include "net/dns/host_resolver.h"
 #include "net/log/net_log_source.h"
 #include "net/log/net_log_with_source.h"
 #include "net/socket/tcp_client_socket.h"
+#include "services/network/public/mojom/network_context.mojom.h"
 
 namespace {
 
@@ -30,31 +33,34 @@ static void RunSocketCallback(
   callback.Run(result, std::move(socket));
 }
 
-class ResolveHostAndOpenSocket final {
+class ResolveHostAndOpenSocket final
+    : public network::mojom::ResolveHostClient {
  public:
   ResolveHostAndOpenSocket(const net::HostPortPair& address,
-                           const AdbClientSocket::SocketCallback& callback)
-      : callback_(callback) {
-    host_resolver_ = net::HostResolver::CreateDefaultResolver(nullptr);
-    net::HostResolver::RequestInfo request_info(address);
-    int result = host_resolver_->Resolve(
-        request_info, net::DEFAULT_PRIORITY, &address_list_,
-        base::Bind(&ResolveHostAndOpenSocket::OnResolved,
-                   base::Unretained(this)),
-        &request_, net::NetLogWithSource());
-    if (result != net::ERR_IO_PENDING)
-      OnResolved(result);
+                           const AdbClientSocket::SocketCallback& callback,
+                           network::mojom::HostResolverPtr* host_resolver)
+      : callback_(callback), binding_(this) {
+    network::mojom::ResolveHostClientPtr client_ptr;
+    binding_.Bind(mojo::MakeRequest(&client_ptr));
+    binding_.set_connection_error_handler(
+        base::BindOnce(&ResolveHostAndOpenSocket::OnComplete,
+                       base::Unretained(this), net::ERR_FAILED, base::nullopt));
+
+    (*host_resolver)->ResolveHost(address, nullptr, std::move(client_ptr));
   }
 
  private:
-  void OnResolved(int result) {
-    if (result < 0) {
+  // network::mojom::ResolveHostClient implementation:
+  void OnComplete(
+      int result,
+      const base::Optional<net::AddressList>& resolved_addresses) override {
+    if (result != net::OK) {
       RunSocketCallback(callback_, nullptr, result);
       delete this;
       return;
     }
     std::unique_ptr<net::StreamSocket> socket(new net::TCPClientSocket(
-        address_list_, NULL, NULL, net::NetLogSource()));
+        resolved_addresses.value(), nullptr, nullptr, net::NetLogSource()));
     net::StreamSocket* socket_ptr = socket.get();
     net::CompletionCallback on_connect =
         base::Bind(&RunSocketCallback, callback_, base::Passed(&socket));
@@ -64,23 +70,24 @@ class ResolveHostAndOpenSocket final {
     delete this;
   }
 
-  std::unique_ptr<net::HostResolver> host_resolver_;
-  std::unique_ptr<net::HostResolver::Request> request_;
-  net::AddressList address_list_;
   AdbClientSocket::SocketCallback callback_;
+  mojo::Binding<network::mojom::ResolveHostClient> binding_;
 };
 
 }  // namespace
 
 scoped_refptr<TCPDeviceProvider> TCPDeviceProvider::CreateForLocalhost(
-    uint16_t port) {
+    uint16_t port,
+    content::BrowserContext* context) {
   TCPDeviceProvider::HostPortSet targets;
   targets.insert(net::HostPortPair("127.0.0.1", port));
-  return new TCPDeviceProvider(targets);
+  return new TCPDeviceProvider(targets, context);
 }
 
-TCPDeviceProvider::TCPDeviceProvider(const HostPortSet& targets)
-    : targets_(targets) {
+TCPDeviceProvider::TCPDeviceProvider(const HostPortSet& targets,
+                                     content::BrowserContext* context)
+    : targets_(targets), context_(context) {
+  DCHECK(context_);
 }
 
 void TCPDeviceProvider::QueryDevices(const SerialsCallback& callback) {
@@ -123,7 +130,14 @@ void TCPDeviceProvider::OpenSocket(const std::string& serial,
   int port;
   base::StringToInt(socket_name, &port);
   net::HostPortPair host_port(serial, port);
-  new ResolveHostAndOpenSocket(host_port, callback);
+
+  // OpenSocket() is run on the devtools ADB thread, while TCPDeviceProvider is
+  // created on the UI thread, so do any initialization of |host_resolver_|
+  // here.
+  if (!host_resolver_) {
+    InitializeHostResolver();
+  }
+  new ResolveHostAndOpenSocket(host_port, callback, &host_resolver_);
 }
 
 void TCPDeviceProvider::ReleaseDevice(const std::string& serial) {
@@ -138,3 +152,19 @@ void TCPDeviceProvider::set_release_callback_for_test(
 
 TCPDeviceProvider::~TCPDeviceProvider() {
 }
+
+void TCPDeviceProvider::InitializeHostResolver() {
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::BindOnce(&TCPDeviceProvider::InitializeHostResolverOnUI, this,
+                     mojo::MakeRequest(&host_resolver_)));
+  host_resolver_.set_connection_error_handler(base::BindOnce(
+      &TCPDeviceProvider::InitializeHostResolver, base::Unretained(this)));
+}
+
+void TCPDeviceProvider::InitializeHostResolverOnUI(
+    network::mojom::HostResolverRequest request) {
+  content::BrowserContext::GetDefaultStoragePartition(context_)
+      ->GetNetworkContext()
+      ->CreateHostResolver(std::move(request));
+}

chrome/browser/devtools/device/tcp_device_provider.h

@@ -16,10 +16,13 @@
 // BUILDFLAG is set.
 class TCPDeviceProvider : public AndroidDeviceManager::DeviceProvider {
  public:
-  static scoped_refptr<TCPDeviceProvider> CreateForLocalhost(uint16_t port);
+  static scoped_refptr<TCPDeviceProvider> CreateForLocalhost(
+      uint16_t port,
+      content::BrowserContext* context);
 
   using HostPortSet = std::set<net::HostPortPair>;
-  explicit TCPDeviceProvider(const HostPortSet& targets);
+  TCPDeviceProvider(const HostPortSet& targets,
+                    content::BrowserContext* context);
 
   void QueryDevices(const SerialsCallback& callback) override;
 
@@ -32,6 +35,8 @@ class TCPDeviceProvider : public AndroidDeviceManager::DeviceProvider {
 
   void ReleaseDevice(const std::string& serial) override;
 
+  void set_targets(HostPortSet targets) { targets_ = std::move(targets); }
+
   void set_release_callback_for_test(const base::Closure& callback);
 
   HostPortSet get_targets_for_test() { return targets_; }
@@ -39,8 +44,13 @@ class TCPDeviceProvider : public AndroidDeviceManager::DeviceProvider {
  private:
   ~TCPDeviceProvider() override;
 
+  void InitializeHostResolver();
+  void InitializeHostResolverOnUI(network::mojom::HostResolverRequest request);
+
   HostPortSet targets_;
   base::Closure release_callback_;
+  content::BrowserContext* context_;
+  network::mojom::HostResolverPtr host_resolver_;
 };
 
 #endif  // CHROME_BROWSER_DEVTOOLS_DEVICE_TCP_DEVICE_PROVIDER_H_