From 5e3d07bf114469cb2cfe19f234b9ed704c89f6aa Mon Sep 17 00:00:00 2001 From: vkuzkokov Date: Mon, 25 Aug 2014 09:09:03 -0700 Subject: Revert of DevTools: Removed refcounting from AndroidWebSocket (patchset #12 of https://codereview.chromium.org/449883002/) Reason for revert: There is a crash when port forwarding is turned off while there is at least one connected device. BUG=407137 Original issue's description: > DevTools: Removed refcounting from AndroidWebSocket > > Issue 387067 can be resolved by having port forwarding socket dependent on all other references to AndroidDeviceManager::Device. > > This requires for lifetime of AWS to be manageable in the first place. > > BUG=387067 > > Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289306 Review URL: https://codereview.chromium.org/505783002 Cr-Commit-Position: refs/heads/master@{#291690} --- .../devtools/device/adb/adb_client_socket.cc | 7 +- .../devtools/device/adb/adb_client_socket.h | 2 +- .../devtools/device/android_device_manager.cc | 42 ++-- .../devtools/device/android_device_manager.h | 22 +- .../browser/devtools/device/android_web_socket.cc | 233 ++++++++------------- .../devtools/device/devtools_android_bridge.cc | 51 +++-- .../devtools/device/devtools_android_bridge.h | 2 +- .../devtools/device/port_forwarding_controller.cc | 73 ++++--- .../devtools/device/port_forwarding_controller.h | 1 + .../devtools/device/self_device_provider.cc | 11 +- .../devtools/device/usb/usb_device_provider.cc | 18 +- 11 files changed, 221 insertions(+), 241 deletions(-) (limited to 'chrome/browser/devtools') diff --git a/chrome/browser/devtools/device/adb/adb_client_socket.cc b/chrome/browser/devtools/device/adb/adb_client_socket.cc index 41b3c9b..0845f92 100644 --- a/chrome/browser/devtools/device/adb/adb_client_socket.cc +++ b/chrome/browser/devtools/device/adb/adb_client_socket.cc @@ -20,6 +20,9 @@ const char kOkayResponse[] = "OKAY"; const char kHostTransportCommand[] = "host:transport:%s"; const char kLocalhost[] = "127.0.0.1"; +typedef base::Callback CommandCallback; +typedef base::Callback SocketCallback; + std::string EncodeMessage(const std::string& message) { static const char kHexChars[] = "0123456789ABCDEF"; @@ -70,14 +73,14 @@ class AdbTransportSocket : public AdbClientSocket { void OnSocketAvailable(int result, const std::string& response) { if (!CheckNetResultOrDie(result)) return; - callback_.Run(net::OK, socket_.Pass()); + callback_.Run(net::OK, socket_.release()); delete this; } bool CheckNetResultOrDie(int result) { if (result >= 0) return true; - callback_.Run(result, make_scoped_ptr(NULL)); + callback_.Run(result, NULL); delete this; return false; } diff --git a/chrome/browser/devtools/device/adb/adb_client_socket.h b/chrome/browser/devtools/device/adb/adb_client_socket.h index f331b02..f0d26ad 100644 --- a/chrome/browser/devtools/device/adb/adb_client_socket.h +++ b/chrome/browser/devtools/device/adb/adb_client_socket.h @@ -13,7 +13,7 @@ class AdbClientSocket { public: typedef base::Callback CommandCallback; typedef base::Callback)> SocketCallback; + net::StreamSocket*)> SocketCallback; static void AdbQuery(int port, const std::string& query, diff --git a/chrome/browser/devtools/device/android_device_manager.cc b/chrome/browser/devtools/device/android_device_manager.cc index c35cdeb..9020d60 100644 --- a/chrome/browser/devtools/device/android_device_manager.cc +++ b/chrome/browser/devtools/device/android_device_manager.cc @@ -50,9 +50,9 @@ static void PostSocketCallback( scoped_refptr response_message_loop, const AndroidDeviceManager::SocketCallback& callback, int result, - scoped_ptr socket) { - response_message_loop->PostTask( - FROM_HERE, base::Bind(callback, result, base::Passed(&socket))); + net::StreamSocket* socket) { + response_message_loop->PostTask(FROM_HERE, + base::Bind(callback, result, socket)); } class HttpRequest { @@ -61,41 +61,39 @@ class HttpRequest { typedef AndroidDeviceManager::SocketCallback SocketCallback; static void CommandRequest(const std::string& request, - const CommandCallback& callback, - int result, - scoped_ptr socket) { + const CommandCallback& callback, + int result, + net::StreamSocket* socket) { if (result != net::OK) { callback.Run(result, std::string()); return; } - new HttpRequest(socket.Pass(), request, callback); + new HttpRequest(socket, request, callback); } static void SocketRequest(const std::string& request, - const SocketCallback& callback, - int result, - scoped_ptr socket) { + const SocketCallback& callback, + int result, + net::StreamSocket* socket) { if (result != net::OK) { - callback.Run(result, make_scoped_ptr(NULL)); + callback.Run(result, NULL); return; } - new HttpRequest(socket.Pass(), request, callback); + new HttpRequest(socket, request, callback); } private: - HttpRequest(scoped_ptr socket, + HttpRequest(net::StreamSocket* socket, const std::string& request, const CommandCallback& callback) - : socket_(socket.Pass()), - command_callback_(callback), - body_pos_(0) { + : socket_(socket), command_callback_(callback), body_pos_(0) { SendRequest(request); } - HttpRequest(scoped_ptr socket, - const std::string& request, - const SocketCallback& callback) - : socket_(socket.Pass()), + HttpRequest(net::StreamSocket* socket, + const std::string& request, + const SocketCallback& callback) + : socket_(socket), socket_callback_(callback), body_pos_(0) { SendRequest(request); @@ -171,7 +169,7 @@ class HttpRequest { if (!command_callback_.is_null()) command_callback_.Run(net::OK, response_.substr(body_pos_)); else - socket_callback_.Run(net::OK, socket_.Pass()); + socket_callback_.Run(net::OK, socket_.release()); delete this; return; } @@ -193,7 +191,7 @@ class HttpRequest { if (!command_callback_.is_null()) command_callback_.Run(result, std::string()); else - socket_callback_.Run(result, make_scoped_ptr(NULL)); + socket_callback_.Run(result, NULL); delete this; return false; } diff --git a/chrome/browser/devtools/device/android_device_manager.h b/chrome/browser/devtools/device/android_device_manager.h index fbc00cd..219e5ce 100644 --- a/chrome/browser/devtools/device/android_device_manager.h +++ b/chrome/browser/devtools/device/android_device_manager.h @@ -24,8 +24,7 @@ class AndroidDeviceManager public base::NonThreadSafe { public: typedef base::Callback CommandCallback; - typedef base::Callback)> - SocketCallback; + typedef base::Callback SocketCallback; typedef base::Callback&)> SerialsCallback; struct BrowserInfo { @@ -54,21 +53,32 @@ class AndroidDeviceManager typedef base::Callback DeviceInfoCallback; - class AndroidWebSocket { + class AndroidWebSocket : public base::RefCountedThreadSafe { public: class Delegate { public: virtual void OnSocketOpened() = 0; virtual void OnFrameRead(const std::string& message) = 0; - virtual void OnSocketClosed() = 0; + virtual void OnSocketClosed(bool closed_by_device) = 0; protected: virtual ~Delegate() {} }; - virtual ~AndroidWebSocket() {} + AndroidWebSocket() {} + virtual void Connect() = 0; + virtual void Disconnect() = 0; virtual void SendFrame(const std::string& message) = 0; + virtual void ClearDelegate() = 0; + + protected: + virtual ~AndroidWebSocket() {} + + private: + friend class base::RefCountedThreadSafe; + + DISALLOW_COPY_AND_ASSIGN(AndroidWebSocket); }; class DeviceProvider; @@ -93,7 +103,7 @@ class AndroidDeviceManager const std::string& url, const SocketCallback& callback); - AndroidWebSocket* CreateWebSocket( + scoped_refptr CreateWebSocket( const std::string& socket_name, const std::string& url, AndroidWebSocket::Delegate* delegate); diff --git a/chrome/browser/devtools/device/android_web_socket.cc b/chrome/browser/devtools/device/android_web_socket.cc index a0054da..81d2627 100644 --- a/chrome/browser/devtools/device/android_web_socket.cc +++ b/chrome/browser/devtools/device/android_web_socket.cc @@ -2,7 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "base/memory/weak_ptr.h" #include "base/message_loop/message_loop.h" #include "base/rand_util.h" #include "chrome/browser/devtools/device/android_device_manager.h" @@ -19,127 +18,85 @@ namespace { const int kBufferSize = 16 * 1024; -class WebSocketImpl { +class WebSocketImpl : public AndroidDeviceManager::AndroidWebSocket { public: - typedef AndroidDeviceManager::AndroidWebSocket::Delegate Delegate; - - WebSocketImpl(Delegate* delegate, - scoped_ptr socket); - void StartListening(); - void SendFrame(const std::string& message); - - private: - void OnBytesRead(scoped_refptr response_buffer, int result); - void SendPendingRequests(int result); - void Disconnect(); - - Delegate* delegate_; - scoped_ptr socket_; - std::string response_buffer_; - std::string request_buffer_; - base::ThreadChecker thread_checker_; - DISALLOW_COPY_AND_ASSIGN(WebSocketImpl); -}; - -class DelegateWrapper - : public AndroidDeviceManager::AndroidWebSocket::Delegate { - public: - DelegateWrapper(base::WeakPtr weak_delegate, - scoped_refptr message_loop) - : weak_delegate_(weak_delegate), - message_loop_(message_loop) { - } - - virtual ~DelegateWrapper() {} - - // AndroidWebSocket::Delegate implementation - virtual void OnSocketOpened() OVERRIDE { - message_loop_->PostTask(FROM_HERE, - base::Bind(&Delegate::OnSocketOpened, weak_delegate_)); - } - - virtual void OnFrameRead(const std::string& message) OVERRIDE { - message_loop_->PostTask(FROM_HERE, - base::Bind(&Delegate::OnFrameRead, weak_delegate_, message)); - } - - virtual void OnSocketClosed() OVERRIDE { - message_loop_->PostTask(FROM_HERE, - base::Bind(&Delegate::OnSocketClosed, weak_delegate_)); - } + typedef AndroidDeviceManager::Device Device; + WebSocketImpl(scoped_refptr device_message_loop, + scoped_refptr device, + const std::string& socket_name, + const std::string& url, + Delegate* delegate); + + virtual void Connect() OVERRIDE; + virtual void Disconnect() OVERRIDE; + virtual void SendFrame(const std::string& message) OVERRIDE; + virtual void ClearDelegate() OVERRIDE; private: - base::WeakPtr weak_delegate_; - scoped_refptr message_loop_; -}; + friend class base::RefCountedThreadSafe; -class AndroidWebSocketImpl - : public AndroidDeviceManager::AndroidWebSocket, - public AndroidDeviceManager::AndroidWebSocket::Delegate { - public: - typedef AndroidDeviceManager::Device Device; - AndroidWebSocketImpl( - scoped_refptr device_message_loop, - scoped_refptr device, - const std::string& socket_name, - const std::string& url, - AndroidWebSocket::Delegate* delegate); + virtual ~WebSocketImpl(); - virtual ~AndroidWebSocketImpl(); - - // AndroidWebSocket implementation - virtual void SendFrame(const std::string& message) OVERRIDE; + void Connected(int result, net::StreamSocket* socket); + void StartListeningOnHandlerThread(); + void OnBytesRead(scoped_refptr response_buffer, int result); + void SendFrameOnHandlerThread(const std::string& message); + void SendPendingRequests(int result); + void DisconnectOnHandlerThread(bool closed_by_device); - // AndroidWebSocket::Delegate implementation - virtual void OnSocketOpened() OVERRIDE; - virtual void OnFrameRead(const std::string& message) OVERRIDE; - virtual void OnSocketClosed() OVERRIDE; - - private: - void Connected(int result, scoped_ptr socket); + void OnSocketOpened(); + void OnFrameRead(const std::string& message); + void OnSocketClosed(bool closed_by_device); scoped_refptr device_message_loop_; scoped_refptr device_; std::string socket_name_; std::string url_; - WebSocketImpl* connection_; - DelegateWrapper* delegate_wrapper_; - AndroidWebSocket::Delegate* delegate_; - base::WeakPtrFactory weak_factory_; - DISALLOW_COPY_AND_ASSIGN(AndroidWebSocketImpl); + scoped_ptr socket_; + Delegate* delegate_; + std::string response_buffer_; + std::string request_buffer_; }; -AndroidWebSocketImpl::AndroidWebSocketImpl( +WebSocketImpl::WebSocketImpl( scoped_refptr device_message_loop, scoped_refptr device, const std::string& socket_name, const std::string& url, - AndroidWebSocket::Delegate* delegate) + Delegate* delegate) : device_message_loop_(device_message_loop), device_(device), socket_name_(socket_name), url_(url), - delegate_(delegate), - weak_factory_(this) { + delegate_(delegate) { +} + +void WebSocketImpl::Connect() { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - DCHECK(delegate_); device_->HttpUpgrade( - socket_name_, url_, - base::Bind(&AndroidWebSocketImpl::Connected, weak_factory_.GetWeakPtr())); + socket_name_, url_, base::Bind(&WebSocketImpl::Connected, this)); } -void AndroidWebSocketImpl::SendFrame(const std::string& message) { +void WebSocketImpl::Disconnect() { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); device_message_loop_->PostTask( FROM_HERE, - base::Bind(&WebSocketImpl::SendFrame, - base::Unretained(connection_), message)); + base::Bind(&WebSocketImpl::DisconnectOnHandlerThread, this, false)); } void WebSocketImpl::SendFrame(const std::string& message) { - DCHECK(thread_checker_.CalledOnValidThread()); - if (!socket_) - return; + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + device_message_loop_->PostTask( + FROM_HERE, + base::Bind(&WebSocketImpl::SendFrameOnHandlerThread, this, message)); +} + +void WebSocketImpl::ClearDelegate() { + delegate_ = NULL; +} + +void WebSocketImpl::SendFrameOnHandlerThread(const std::string& message) { + DCHECK_EQ(device_message_loop_, base::MessageLoopProxy::current()); int mask = base::RandInt(0, 0x7FFFFFFF); std::string encoded_frame = WebSocket::EncodeFrameHybi17(message, mask); request_buffer_ += encoded_frame; @@ -147,55 +104,43 @@ void WebSocketImpl::SendFrame(const std::string& message) { SendPendingRequests(0); } -AndroidWebSocketImpl::~AndroidWebSocketImpl() { +WebSocketImpl::~WebSocketImpl() { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - device_message_loop_->DeleteSoon(FROM_HERE, connection_); - device_message_loop_->DeleteSoon(FROM_HERE, delegate_wrapper_); -} - -WebSocketImpl::WebSocketImpl(Delegate* delegate, - scoped_ptr socket) - : delegate_(delegate), - socket_(socket.Pass()) { - thread_checker_.DetachFromThread(); } -void AndroidWebSocketImpl::Connected(int result, - scoped_ptr socket) { +void WebSocketImpl::Connected(int result, net::StreamSocket* socket) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); if (result != net::OK || socket == NULL) { - OnSocketClosed(); + OnSocketClosed(true); return; } - delegate_wrapper_ = new DelegateWrapper(weak_factory_.GetWeakPtr(), - base::MessageLoopProxy::current()); - connection_ = new WebSocketImpl(delegate_wrapper_, socket.Pass()); + socket_.reset(socket); device_message_loop_->PostTask( FROM_HERE, - base::Bind(&WebSocketImpl::StartListening, - base::Unretained(connection_))); + base::Bind(&WebSocketImpl::StartListeningOnHandlerThread, this)); OnSocketOpened(); } -void WebSocketImpl::StartListening() { - DCHECK(thread_checker_.CalledOnValidThread()); - DCHECK(socket_); +void WebSocketImpl::StartListeningOnHandlerThread() { + DCHECK_EQ(device_message_loop_, base::MessageLoopProxy::current()); scoped_refptr response_buffer = new net::IOBuffer(kBufferSize); int result = socket_->Read( response_buffer.get(), kBufferSize, - base::Bind(&WebSocketImpl::OnBytesRead, - base::Unretained(this), response_buffer)); + base::Bind(&WebSocketImpl::OnBytesRead, this, response_buffer)); if (result != net::ERR_IO_PENDING) OnBytesRead(response_buffer, result); } -void WebSocketImpl::OnBytesRead(scoped_refptr response_buffer, - int result) { - DCHECK(thread_checker_.CalledOnValidThread()); +void WebSocketImpl::OnBytesRead( + scoped_refptr response_buffer, int result) { + DCHECK_EQ(device_message_loop_, base::MessageLoopProxy::current()); + if (!socket_) + return; + if (result <= 0) { - Disconnect(); + DisconnectOnHandlerThread(true); return; } @@ -209,30 +154,32 @@ void WebSocketImpl::OnBytesRead(scoped_refptr response_buffer, while (parse_result == WebSocket::FRAME_OK) { response_buffer_ = response_buffer_.substr(bytes_consumed); - delegate_->OnFrameRead(output); + BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, + base::Bind(&WebSocketImpl::OnFrameRead, this, output)); parse_result = WebSocket::DecodeFrameHybi17( response_buffer_, false, &bytes_consumed, &output); } if (parse_result == WebSocket::FRAME_ERROR || parse_result == WebSocket::FRAME_CLOSE) { - Disconnect(); + DisconnectOnHandlerThread(true); return; } result = socket_->Read( response_buffer.get(), kBufferSize, - base::Bind(&WebSocketImpl::OnBytesRead, - base::Unretained(this), response_buffer)); + base::Bind(&WebSocketImpl::OnBytesRead, this, response_buffer)); if (result != net::ERR_IO_PENDING) OnBytesRead(response_buffer, result); } void WebSocketImpl::SendPendingRequests(int result) { - DCHECK(thread_checker_.CalledOnValidThread()); + DCHECK_EQ(device_message_loop_, base::MessageLoopProxy::current()); + if (!socket_) + return; if (result < 0) { - Disconnect(); + DisconnectOnHandlerThread(true); return; } request_buffer_ = request_buffer_.substr(result); @@ -243,39 +190,43 @@ void WebSocketImpl::SendPendingRequests(int result) { new net::StringIOBuffer(request_buffer_); result = socket_->Write(buffer.get(), buffer->size(), base::Bind(&WebSocketImpl::SendPendingRequests, - base::Unretained(this))); + this)); if (result != net::ERR_IO_PENDING) SendPendingRequests(result); } -void WebSocketImpl::Disconnect() { - DCHECK(thread_checker_.CalledOnValidThread()); - socket_.reset(); - delegate_->OnSocketClosed(); +void WebSocketImpl::DisconnectOnHandlerThread(bool closed_by_device) { + DCHECK_EQ(device_message_loop_, base::MessageLoopProxy::current()); + if (!socket_) + return; + // Wipe out socket_ first since Disconnect can re-enter this method. + scoped_ptr socket(socket_.release()); + socket->Disconnect(); + BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, + base::Bind(&WebSocketImpl::OnSocketClosed, this, closed_by_device)); } -void AndroidWebSocketImpl::OnSocketOpened() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - delegate_->OnSocketOpened(); +void WebSocketImpl::OnSocketOpened() { + if (delegate_) + delegate_->OnSocketOpened(); } -void AndroidWebSocketImpl::OnFrameRead(const std::string& message) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - delegate_->OnFrameRead(message); +void WebSocketImpl::OnFrameRead(const std::string& message) { + if (delegate_) + delegate_->OnFrameRead(message); } -void AndroidWebSocketImpl::OnSocketClosed() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - delegate_->OnSocketClosed(); +void WebSocketImpl::OnSocketClosed(bool closed_by_device) { + if (delegate_) + delegate_->OnSocketClosed(closed_by_device); } } // namespace -AndroidDeviceManager::AndroidWebSocket* +scoped_refptr AndroidDeviceManager::Device::CreateWebSocket( const std::string& socket, const std::string& url, AndroidDeviceManager::AndroidWebSocket::Delegate* delegate) { - return new AndroidWebSocketImpl( - device_message_loop_, this, socket, url, delegate); + return new WebSocketImpl(device_message_loop_, this, socket, url, delegate); } diff --git a/chrome/browser/devtools/device/devtools_android_bridge.cc b/chrome/browser/devtools/device/devtools_android_bridge.cc index e58a857..c25c4699 100644 --- a/chrome/browser/devtools/device/devtools_android_bridge.cc +++ b/chrome/browser/devtools/device/devtools_android_bridge.cc @@ -186,12 +186,11 @@ class ProtocolCommand private: virtual void OnSocketOpened() OVERRIDE; virtual void OnFrameRead(const std::string& message) OVERRIDE; - virtual void OnSocketClosed() OVERRIDE; - virtual ~ProtocolCommand(); + virtual void OnSocketClosed(bool closed_by_device) OVERRIDE; const std::string command_; const base::Closure callback_; - scoped_ptr web_socket_; + scoped_refptr web_socket_; DISALLOW_COPY_AND_ASSIGN(ProtocolCommand); }; @@ -202,8 +201,9 @@ ProtocolCommand::ProtocolCommand( const std::string& command, const base::Closure callback) : command_(command), - callback_(callback), - web_socket_(browser->CreateWebSocket(debug_url, this)) { + callback_(callback){ + web_socket_ = browser->CreateWebSocket(debug_url, this); + web_socket_->Connect(); } void ProtocolCommand::OnSocketOpened() { @@ -211,16 +211,14 @@ void ProtocolCommand::OnSocketOpened() { } void ProtocolCommand::OnFrameRead(const std::string& message) { - delete this; -} - -void ProtocolCommand::OnSocketClosed() { - delete this; + web_socket_->Disconnect(); } -ProtocolCommand::~ProtocolCommand() { - if (!callback_.is_null()) +void ProtocolCommand::OnSocketClosed(bool closed_by_device) { + if (!callback_.is_null()) { callback_.Run(); + } + delete this; } } // namespace @@ -293,15 +291,14 @@ class AgentHostDelegate const std::string& message) OVERRIDE; virtual void OnSocketOpened() OVERRIDE; virtual void OnFrameRead(const std::string& message) OVERRIDE; - virtual void OnSocketClosed() OVERRIDE; + virtual void OnSocketClosed(bool closed_by_device) OVERRIDE; const std::string id_; - scoped_refptr browser_; - const std::string debug_url_; bool socket_opened_; + bool detached_; bool is_web_view_; std::vector pending_messages_; - scoped_ptr web_socket_; + scoped_refptr web_socket_; content::DevToolsAgentHost* agent_host_; content::DevToolsExternalAgentProxy* proxy_; DISALLOW_COPY_AND_ASSIGN(AgentHostDelegate); @@ -330,10 +327,10 @@ AgentHostDelegate::AgentHostDelegate( scoped_refptr browser, const std::string& debug_url) : id_(id), - browser_(browser), - debug_url_(debug_url), socket_opened_(false), + detached_(false), is_web_view_(browser->IsWebView()), + web_socket_(browser->CreateWebSocket(debug_url, this)), agent_host_(NULL), proxy_(NULL) { g_host_delegates.Get()[id] = this; @@ -341,17 +338,20 @@ AgentHostDelegate::AgentHostDelegate( AgentHostDelegate::~AgentHostDelegate() { g_host_delegates.Get().erase(id_); + web_socket_->ClearDelegate(); } void AgentHostDelegate::Attach(content::DevToolsExternalAgentProxy* proxy) { proxy_ = proxy; content::RecordAction(base::UserMetricsAction(is_web_view_ ? "DevTools_InspectAndroidWebView" : "DevTools_InspectAndroidPage")); - web_socket_.reset(browser_->CreateWebSocket(debug_url_, this)); + web_socket_->Connect(); } void AgentHostDelegate::Detach() { - web_socket_.reset(); + detached_ = true; + if (socket_opened_) + web_socket_->Disconnect(); } void AgentHostDelegate::SendMessageToBackend(const std::string& message) { @@ -362,6 +362,11 @@ void AgentHostDelegate::SendMessageToBackend(const std::string& message) { } void AgentHostDelegate::OnSocketOpened() { + if (detached_) { + web_socket_->Disconnect(); + return; + } + socket_opened_ = true; for (std::vector::iterator it = pending_messages_.begin(); it != pending_messages_.end(); ++it) { @@ -375,8 +380,8 @@ void AgentHostDelegate::OnFrameRead(const std::string& message) { proxy_->DispatchOnClientHost(message); } -void AgentHostDelegate::OnSocketClosed() { - if (proxy_) +void AgentHostDelegate::OnSocketClosed(bool closed_by_device) { + if (proxy_ && closed_by_device) proxy_->ConnectionClosed(); } @@ -609,7 +614,7 @@ DevToolsAndroidBridge::RemoteBrowser::GetAgentHost() { "adb:" + device_->serial() + ":" + socket_, this, kBrowserTargetSocket); } -DevToolsAndroidBridge::AndroidWebSocket* +scoped_refptr DevToolsAndroidBridge::RemoteBrowser::CreateWebSocket( const std::string& url, DevToolsAndroidBridge::AndroidWebSocket::Delegate* delegate) { diff --git a/chrome/browser/devtools/device/devtools_android_bridge.h b/chrome/browser/devtools/device/devtools_android_bridge.h index b9cbb76..ddc34bb 100644 --- a/chrome/browser/devtools/device/devtools_android_bridge.h +++ b/chrome/browser/devtools/device/devtools_android_bridge.h @@ -121,7 +121,7 @@ class DevToolsAndroidBridge scoped_refptr GetAgentHost(); - AndroidWebSocket* CreateWebSocket( + scoped_refptr CreateWebSocket( const std::string& url, DevToolsAndroidBridge::AndroidWebSocket::Delegate* delegate); diff --git a/chrome/browser/devtools/device/port_forwarding_controller.cc b/chrome/browser/devtools/device/port_forwarding_controller.cc index 35c3652..df610d1 100644 --- a/chrome/browser/devtools/device/port_forwarding_controller.cc +++ b/chrome/browser/devtools/device/port_forwarding_controller.cc @@ -59,11 +59,11 @@ class SocketTunnel : public base::NonThreadSafe { int port, const CounterCallback& callback, int result, - scoped_ptr socket) { + net::StreamSocket* socket) { if (result < 0) return; SocketTunnel* tunnel = new SocketTunnel(callback); - tunnel->Start(socket.Pass(), host, port); + tunnel->Start(socket, host, port); } private: @@ -75,9 +75,8 @@ class SocketTunnel : public base::NonThreadSafe { callback_.Run(1); } - void Start(scoped_ptr socket, - const std::string& host, int port) { - remote_socket_.swap(socket); + void Start(net::StreamSocket* socket, const std::string& host, int port) { + remote_socket_.reset(socket); host_resolver_ = net::HostResolver::CreateDefaultResolver(NULL); net::HostResolver::RequestInfo request_info(net::HostPortPair(host, port)); @@ -255,13 +254,15 @@ FindBestBrowserForTethering( } // namespace class PortForwardingController::Connection - : public DevToolsAndroidBridge::AndroidWebSocket::Delegate { + : public DevToolsAndroidBridge::AndroidWebSocket::Delegate, + public base::RefCountedThreadSafe< + Connection, + content::BrowserThread::DeleteOnUIThread> { public: Connection(Registry* registry, scoped_refptr device, scoped_refptr browser, const ForwardingMap& forwarding_map); - virtual ~Connection(); const PortStatusMap& GetPortStatusMap(); @@ -274,6 +275,7 @@ class PortForwardingController::Connection content::BrowserThread::UI>; friend class base::DeleteHelper; + virtual ~Connection(); typedef std::map ForwardingMap; @@ -290,25 +292,23 @@ class PortForwardingController::Connection void ProcessBindResponse(int port, PortStatus status); void ProcessUnbindResponse(int port, PortStatus status); - static void UpdateSocketCountOnHandlerThread( - base::WeakPtr weak_connection, int port, int increment); + void UpdateSocketCountOnHandlerThread(int port, int increment); void UpdateSocketCount(int port, int increment); // DevToolsAndroidBridge::AndroidWebSocket::Delegate implementation: virtual void OnSocketOpened() OVERRIDE; virtual void OnFrameRead(const std::string& message) OVERRIDE; - virtual void OnSocketClosed() OVERRIDE; + virtual void OnSocketClosed(bool closed_by_device) OVERRIDE; PortForwardingController::Registry* registry_; scoped_refptr device_; scoped_refptr browser_; - scoped_ptr web_socket_; + scoped_refptr web_socket_; int command_id_; bool connected_; ForwardingMap forwarding_map_; CommandCallbackMap pending_responses_; PortStatusMap port_status_; - base::WeakPtrFactory weak_factory_; DISALLOW_COPY_AND_ASSIGN(Connection); }; @@ -323,18 +323,27 @@ PortForwardingController::Connection::Connection( browser_(browser), command_id_(0), connected_(false), - forwarding_map_(forwarding_map), - weak_factory_(this) { + forwarding_map_(forwarding_map) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); (*registry_)[device_->serial()] = this; - web_socket_.reset( - browser->CreateWebSocket(kDevToolsRemoteBrowserTarget, this)); + web_socket_ = browser->CreateWebSocket(kDevToolsRemoteBrowserTarget, this); + web_socket_->Connect(); + AddRef(); // Balanced in OnSocketClosed(); +} + +void PortForwardingController::Connection::Shutdown() { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + registry_ = NULL; + // This will have no effect if the socket is not connected yet. + web_socket_->Disconnect(); } PortForwardingController::Connection::~Connection() { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - DCHECK(registry_->find(device_->serial()) != registry_->end()); - registry_->erase(device_->serial()); + if (registry_) { + DCHECK(registry_->find(device_->serial()) != registry_->end()); + registry_->erase(device_->serial()); + } } void PortForwardingController::Connection::UpdateForwardingMap( @@ -431,12 +440,10 @@ void PortForwardingController::Connection::ProcessUnbindResponse( port_status_.erase(it); } -// static void PortForwardingController::Connection::UpdateSocketCountOnHandlerThread( - base::WeakPtr weak_connection, int port, int increment) { + int port, int increment) { BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, - base::Bind(&Connection::UpdateSocketCount, - weak_connection, port, increment)); + base::Bind(&Connection::UpdateSocketCount, this, port, increment)); } void PortForwardingController::Connection::UpdateSocketCount( @@ -460,12 +467,19 @@ PortForwardingController::Connection::GetPortStatusMap() { void PortForwardingController::Connection::OnSocketOpened() { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + if (!registry_) { + // Socket was created after Shutdown was called. Disconnect immediately. + web_socket_->Disconnect(); + return; + } connected_ = true; SerializeChanges(kTetheringBind, ForwardingMap(), forwarding_map_); } -void PortForwardingController::Connection::OnSocketClosed() { - delete this; +void PortForwardingController::Connection::OnSocketClosed( + bool closed_by_device) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + Release(); // Balanced in the constructor. } void PortForwardingController::Connection::OnFrameRead( @@ -505,8 +519,7 @@ void PortForwardingController::Connection::OnFrameRead( std::string destination_host = tokens[0]; SocketTunnel::CounterCallback callback = - base::Bind(&Connection::UpdateSocketCountOnHandlerThread, - weak_factory_.GetWeakPtr(), port); + base::Bind(&Connection::UpdateSocketCountOnHandlerThread, this, port); device_->OpenSocket( connection_id.c_str(), @@ -593,7 +606,7 @@ void PortForwardingController::OnPrefsChange() { UpdateConnections(); } else { StopListening(); - STLDeleteValues(®istry_); + ShutdownConnections(); NotifyListeners(DevicesStatus()); } } @@ -624,6 +637,12 @@ void PortForwardingController::UpdateConnections() { it->second->UpdateForwardingMap(forwarding_map_); } +void PortForwardingController::ShutdownConnections() { + for (Registry::iterator it = registry_.begin(); it != registry_.end(); ++it) + it->second->Shutdown(); + registry_.clear(); +} + void PortForwardingController::NotifyListeners( const DevicesStatus& status) const { Listeners copy(listeners_); // Iterate over copy. diff --git a/chrome/browser/devtools/device/port_forwarding_controller.h b/chrome/browser/devtools/device/port_forwarding_controller.h index a904ef5..ae491d7 100644 --- a/chrome/browser/devtools/device/port_forwarding_controller.h +++ b/chrome/browser/devtools/device/port_forwarding_controller.h @@ -77,6 +77,7 @@ class PortForwardingController void StopListening(); void UpdateConnections(); + void ShutdownConnections(); void NotifyListeners(const DevicesStatus& status) const; diff --git a/chrome/browser/devtools/device/self_device_provider.cc b/chrome/browser/devtools/device/self_device_provider.cc index f6930b5..ee0f043 100644 --- a/chrome/browser/devtools/device/self_device_provider.cc +++ b/chrome/browser/devtools/device/self_device_provider.cc @@ -17,9 +17,9 @@ const char kSerial[] = "local"; static void RunSocketCallback( const AndroidDeviceManager::SocketCallback& callback, - scoped_ptr socket, + net::StreamSocket* socket, int result) { - callback.Run(result, socket.Pass()); + callback.Run(result, socket); } } // namespace @@ -61,8 +61,7 @@ void SelfAsDeviceProvider::OpenSocket(const std::string& serial, base::StringToInt(socket_name, &port); net::AddressList address_list = net::AddressList::CreateFromIPAddress(ip_number, port); - scoped_ptr socket(new net::TCPClientSocket( - address_list, NULL, net::NetLog::Source())); - socket->Connect( - base::Bind(&RunSocketCallback, callback, base::Passed(&socket))); + net::TCPClientSocket* socket = new net::TCPClientSocket( + address_list, NULL, net::NetLog::Source()); + socket->Connect(base::Bind(&RunSocketCallback, callback, socket)); } diff --git a/chrome/browser/devtools/device/usb/usb_device_provider.cc b/chrome/browser/devtools/device/usb/usb_device_provider.cc index 4931499..93c9429 100644 --- a/chrome/browser/devtools/device/usb/usb_device_provider.cc +++ b/chrome/browser/devtools/device/usb/usb_device_provider.cc @@ -19,12 +19,9 @@ const char kLocalAbstractCommand[] = "localabstract:%s"; const int kBufferSize = 16 * 1024; void OnOpenSocket(const UsbDeviceProvider::SocketCallback& callback, - net::StreamSocket* socket_raw, + net::StreamSocket* socket, int result) { - scoped_ptr socket(socket_raw); - if (result != net::OK) - socket.reset(); - callback.Run(result, socket.Pass()); + callback.Run(result, result == net::OK ? socket : NULL); } void OnRead(net::StreamSocket* socket, @@ -71,8 +68,7 @@ void RunCommand(scoped_refptr device, callback.Run(net::ERR_CONNECTION_FAILED, std::string()); return; } - int result = socket->Connect( - base::Bind(&OpenedForCommand, callback, socket)); + int result = socket->Connect(base::Bind(&OpenedForCommand, callback, socket)); if (result != net::ERR_IO_PENDING) callback.Run(result, std::string()); } @@ -111,21 +107,19 @@ void UsbDeviceProvider::OpenSocket(const std::string& serial, const SocketCallback& callback) { UsbDeviceMap::iterator it = device_map_.find(serial); if (it == device_map_.end()) { - callback.Run(net::ERR_CONNECTION_FAILED, - make_scoped_ptr(NULL)); + callback.Run(net::ERR_CONNECTION_FAILED, NULL); return; } std::string socket_name = base::StringPrintf(kLocalAbstractCommand, name.c_str()); net::StreamSocket* socket = it->second->CreateSocket(socket_name); if (!socket) { - callback.Run(net::ERR_CONNECTION_FAILED, - make_scoped_ptr(NULL)); + callback.Run(net::ERR_CONNECTION_FAILED, NULL); return; } int result = socket->Connect(base::Bind(&OnOpenSocket, callback, socket)); if (result != net::ERR_IO_PENDING) - callback.Run(result, make_scoped_ptr(NULL)); + callback.Run(result, NULL); } void UsbDeviceProvider::ReleaseDevice(const std::string& serial) { -- cgit v1.1