Commit 6af9f9ee authored by Clark DuVall's avatar Clark DuVall Committed by Commit Bot

Reland "Reland "Convert tcp_device_provider.cc to mojo host resolver""

This is a reland of 6493250f

There were reports of crashes when opening devtools because of a DCHECK
deep inside GetActiveUserProfile. This switches to using the system
network context, as suggested by mmenke@. This also simplifies things
quite a bit as we don't have to pass around a BrowserContext.

Original change's description:
> 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/1197265
> Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
> Commit-Queue: Clark DuVall <cduvall@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#587750}

Bug: 874653
Change-Id: Ifc6eddc0d05bd65792165d5542fe98b09ff0fb05
Reviewed-on: https://chromium-review.googlesource.com/1199848Reviewed-by: default avatarEric Orth <ericorth@chromium.org>
Reviewed-by: default avatarAndrey Kosyakov <caseq@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588890}
parent 337ba90a
...@@ -11,12 +11,16 @@ ...@@ -11,12 +11,16 @@
#include "base/stl_util.h" #include "base/stl_util.h"
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
#include "base/threading/thread_task_runner_handle.h" #include "base/threading/thread_task_runner_handle.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/devtools/device/adb/adb_client_socket.h" #include "chrome/browser/devtools/device/adb/adb_client_socket.h"
#include "chrome/browser/net/system_network_context_manager.h"
#include "mojo/public/cpp/bindings/binding.h"
#include "net/base/net_errors.h" #include "net/base/net_errors.h"
#include "net/dns/host_resolver.h" #include "net/dns/host_resolver.h"
#include "net/log/net_log_source.h" #include "net/log/net_log_source.h"
#include "net/log/net_log_with_source.h" #include "net/log/net_log_with_source.h"
#include "net/socket/tcp_client_socket.h" #include "net/socket/tcp_client_socket.h"
#include "services/network/public/mojom/network_context.mojom.h"
namespace { namespace {
...@@ -30,31 +34,34 @@ static void RunSocketCallback( ...@@ -30,31 +34,34 @@ static void RunSocketCallback(
callback.Run(result, std::move(socket)); callback.Run(result, std::move(socket));
} }
class ResolveHostAndOpenSocket final { class ResolveHostAndOpenSocket final
: public network::mojom::ResolveHostClient {
public: public:
ResolveHostAndOpenSocket(const net::HostPortPair& address, ResolveHostAndOpenSocket(const net::HostPortPair& address,
const AdbClientSocket::SocketCallback& callback) const AdbClientSocket::SocketCallback& callback,
: callback_(callback) { network::mojom::HostResolverPtr* host_resolver)
host_resolver_ = net::HostResolver::CreateDefaultResolver(nullptr); : callback_(callback), binding_(this) {
net::HostResolver::RequestInfo request_info(address); network::mojom::ResolveHostClientPtr client_ptr;
int result = host_resolver_->Resolve( binding_.Bind(mojo::MakeRequest(&client_ptr));
request_info, net::DEFAULT_PRIORITY, &address_list_, binding_.set_connection_error_handler(
base::Bind(&ResolveHostAndOpenSocket::OnResolved, base::BindOnce(&ResolveHostAndOpenSocket::OnComplete,
base::Unretained(this)), base::Unretained(this), net::ERR_FAILED, base::nullopt));
&request_, net::NetLogWithSource());
if (result != net::ERR_IO_PENDING) (*host_resolver)->ResolveHost(address, nullptr, std::move(client_ptr));
OnResolved(result);
} }
private: private:
void OnResolved(int result) { // network::mojom::ResolveHostClient implementation:
if (result < 0) { void OnComplete(
int result,
const base::Optional<net::AddressList>& resolved_addresses) override {
if (result != net::OK) {
RunSocketCallback(callback_, nullptr, result); RunSocketCallback(callback_, nullptr, result);
delete this; delete this;
return; return;
} }
std::unique_ptr<net::StreamSocket> socket(new net::TCPClientSocket( 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::StreamSocket* socket_ptr = socket.get();
net::CompletionCallback on_connect = net::CompletionCallback on_connect =
base::Bind(&RunSocketCallback, callback_, base::Passed(&socket)); base::Bind(&RunSocketCallback, callback_, base::Passed(&socket));
...@@ -64,10 +71,8 @@ class ResolveHostAndOpenSocket final { ...@@ -64,10 +71,8 @@ class ResolveHostAndOpenSocket final {
delete this; delete this;
} }
std::unique_ptr<net::HostResolver> host_resolver_;
std::unique_ptr<net::HostResolver::Request> request_;
net::AddressList address_list_;
AdbClientSocket::SocketCallback callback_; AdbClientSocket::SocketCallback callback_;
mojo::Binding<network::mojom::ResolveHostClient> binding_;
}; };
} // namespace } // namespace
...@@ -123,7 +128,14 @@ void TCPDeviceProvider::OpenSocket(const std::string& serial, ...@@ -123,7 +128,14 @@ void TCPDeviceProvider::OpenSocket(const std::string& serial,
int port; int port;
base::StringToInt(socket_name, &port); base::StringToInt(socket_name, &port);
net::HostPortPair host_port(serial, 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) { void TCPDeviceProvider::ReleaseDevice(const std::string& serial) {
...@@ -138,3 +150,19 @@ void TCPDeviceProvider::set_release_callback_for_test( ...@@ -138,3 +150,19 @@ void TCPDeviceProvider::set_release_callback_for_test(
TCPDeviceProvider::~TCPDeviceProvider() { 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) {
g_browser_process->system_network_context_manager()
->GetContext()
->CreateHostResolver(std::move(request));
}
...@@ -39,8 +39,12 @@ class TCPDeviceProvider : public AndroidDeviceManager::DeviceProvider { ...@@ -39,8 +39,12 @@ class TCPDeviceProvider : public AndroidDeviceManager::DeviceProvider {
private: private:
~TCPDeviceProvider() override; ~TCPDeviceProvider() override;
void InitializeHostResolver();
void InitializeHostResolverOnUI(network::mojom::HostResolverRequest request);
HostPortSet targets_; HostPortSet targets_;
base::Closure release_callback_; base::Closure release_callback_;
network::mojom::HostResolverPtr host_resolver_;
}; };
#endif // CHROME_BROWSER_DEVTOOLS_DEVICE_TCP_DEVICE_PROVIDER_H_ #endif // CHROME_BROWSER_DEVTOOLS_DEVICE_TCP_DEVICE_PROVIDER_H_
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment