Revert of DevTools: Removed refcounting from AndroidWebSocket (patchset #12 of...

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}

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<void(int, const std::string&)> CommandCallback;
+typedef base::Callback<void(int, net::StreamSocket*)> 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<net::StreamSocket>(NULL));
+    callback_.Run(result, NULL);
     delete this;
     return false;
   }

chrome/browser/devtools/device/adb/adb_client_socket.h

@@ -13,7 +13,7 @@ class AdbClientSocket {
  public:
   typedef base::Callback<void(int, const std::string&)> CommandCallback;
   typedef base::Callback<void(int result,
-                              scoped_ptr<net::StreamSocket>)> SocketCallback;
+                              net::StreamSocket*)> SocketCallback;
 
   static void AdbQuery(int port,
                        const std::string& query,

chrome/browser/devtools/device/android_device_manager.cc

@@ -50,9 +50,9 @@ static void PostSocketCallback(
     scoped_refptr<base::MessageLoopProxy> response_message_loop,
     const AndroidDeviceManager::SocketCallback& callback,
     int result,
-    scoped_ptr<net::StreamSocket> 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<net::StreamSocket> 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<net::StreamSocket> socket) {
+                          const SocketCallback& callback,
+                          int result,
+                          net::StreamSocket* socket) {
     if (result != net::OK) {
-      callback.Run(result, make_scoped_ptr<net::StreamSocket>(NULL));
+      callback.Run(result, NULL);
       return;
     }
-    new HttpRequest(socket.Pass(), request, callback);
+    new HttpRequest(socket, request, callback);
   }
 
  private:
-  HttpRequest(scoped_ptr<net::StreamSocket> 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<net::StreamSocket> 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<net::StreamSocket>(NULL));
+      socket_callback_.Run(result, NULL);
     delete this;
     return false;
   }

chrome/browser/devtools/device/android_device_manager.h

@@ -24,8 +24,7 @@ class AndroidDeviceManager
       public base::NonThreadSafe {
  public:
   typedef base::Callback<void(int, const std::string&)> CommandCallback;
-  typedef base::Callback<void(int result, scoped_ptr<net::StreamSocket>)>
-      SocketCallback;
+  typedef base::Callback<void(int result, net::StreamSocket*)> SocketCallback;
   typedef base::Callback<void(const std::vector<std::string>&)> SerialsCallback;
 
   struct BrowserInfo {
@@ -54,21 +53,32 @@ class AndroidDeviceManager
 
   typedef base::Callback<void(const DeviceInfo&)> DeviceInfoCallback;
 
-  class AndroidWebSocket {
+  class AndroidWebSocket : public base::RefCountedThreadSafe<AndroidWebSocket> {
    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<AndroidWebSocket>;
+
+    DISALLOW_COPY_AND_ASSIGN(AndroidWebSocket);
   };
 
   class DeviceProvider;
@@ -93,7 +103,7 @@ class AndroidDeviceManager
                      const std::string& url,
                      const SocketCallback& callback);
 
-    AndroidWebSocket* CreateWebSocket(
+    scoped_refptr<AndroidWebSocket> CreateWebSocket(
         const std::string& socket_name,
         const std::string& url,
         AndroidWebSocket::Delegate* delegate);

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<net::StreamSocket> socket);
-  void StartListening();
-  void SendFrame(const std::string& message);
-
- private:
-  void OnBytesRead(scoped_refptr<net::IOBuffer> response_buffer, int result);
-  void SendPendingRequests(int result);
-  void Disconnect();
-
-  Delegate* delegate_;
-  scoped_ptr<net::StreamSocket> 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<Delegate> weak_delegate,
-                  scoped_refptr<base::MessageLoopProxy> 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<base::MessageLoopProxy> device_message_loop,
+                scoped_refptr<Device> 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<Delegate> weak_delegate_;
-  scoped_refptr<base::MessageLoopProxy> message_loop_;
-};
+  friend class base::RefCountedThreadSafe<AndroidWebSocket>;
 
-class AndroidWebSocketImpl
-    : public AndroidDeviceManager::AndroidWebSocket,
-      public AndroidDeviceManager::AndroidWebSocket::Delegate {
- public:
-  typedef AndroidDeviceManager::Device Device;
-  AndroidWebSocketImpl(
-      scoped_refptr<base::MessageLoopProxy> device_message_loop,
-      scoped_refptr<Device> 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<net::IOBuffer> 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<net::StreamSocket> socket);
+  void OnSocketOpened();
+  void OnFrameRead(const std::string& message);
+  void OnSocketClosed(bool closed_by_device);
 
   scoped_refptr<base::MessageLoopProxy> device_message_loop_;
   scoped_refptr<Device> device_;
   std::string socket_name_;
   std::string url_;
-  WebSocketImpl* connection_;
-  DelegateWrapper* delegate_wrapper_;
-  AndroidWebSocket::Delegate* delegate_;
-  base::WeakPtrFactory<AndroidWebSocketImpl> weak_factory_;
-  DISALLOW_COPY_AND_ASSIGN(AndroidWebSocketImpl);
+  scoped_ptr<net::StreamSocket> socket_;
+  Delegate* delegate_;
+  std::string response_buffer_;
+  std::string request_buffer_;
 };
 
-AndroidWebSocketImpl::AndroidWebSocketImpl(
+WebSocketImpl::WebSocketImpl(
     scoped_refptr<base::MessageLoopProxy> device_message_loop,
     scoped_refptr<Device> 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<net::StreamSocket> socket)
-                             : delegate_(delegate),
-                               socket_(socket.Pass()) {
-  thread_checker_.DetachFromThread();
 }
 
-void AndroidWebSocketImpl::Connected(int result,
-                                     scoped_ptr<net::StreamSocket> 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<net::IOBuffer> 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<net::IOBuffer> response_buffer,
-                                int result) {
-  DCHECK(thread_checker_.CalledOnValidThread());
+void WebSocketImpl::OnBytesRead(
+    scoped_refptr<net::IOBuffer> 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<net::IOBuffer> 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<net::StreamSocket> 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::AndroidWebSocket>
 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);
 }

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<DevToolsAndroidBridge::AndroidWebSocket> web_socket_;
+  scoped_refptr<DevToolsAndroidBridge::AndroidWebSocket> 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<DevToolsAndroidBridge::RemoteBrowser> browser_;
-  const std::string debug_url_;
   bool socket_opened_;
+  bool detached_;
   bool is_web_view_;
   std::vector<std::string> pending_messages_;
-  scoped_ptr<DevToolsAndroidBridge::AndroidWebSocket> web_socket_;
+  scoped_refptr<DevToolsAndroidBridge::AndroidWebSocket> web_socket_;
   content::DevToolsAgentHost* agent_host_;
   content::DevToolsExternalAgentProxy* proxy_;
   DISALLOW_COPY_AND_ASSIGN(AgentHostDelegate);
@@ -330,10 +327,10 @@ AgentHostDelegate::AgentHostDelegate(
     scoped_refptr<DevToolsAndroidBridge::RemoteBrowser> 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<std::string>::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::AndroidWebSocket>
 DevToolsAndroidBridge::RemoteBrowser::CreateWebSocket(
     const std::string& url,
     DevToolsAndroidBridge::AndroidWebSocket::Delegate* delegate) {

chrome/browser/devtools/device/devtools_android_bridge.h

@@ -121,7 +121,7 @@ class DevToolsAndroidBridge
 
     scoped_refptr<content::DevToolsAgentHost> GetAgentHost();
 
-    AndroidWebSocket* CreateWebSocket(
+    scoped_refptr<AndroidWebSocket> CreateWebSocket(
         const std::string& url,
         DevToolsAndroidBridge::AndroidWebSocket::Delegate* delegate);
 

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<net::StreamSocket> 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<net::StreamSocket> 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<DevToolsAndroidBridge::RemoteDevice> device,
              scoped_refptr<DevToolsAndroidBridge::RemoteBrowser> browser,
              const ForwardingMap& forwarding_map);
-  virtual ~Connection();
 
   const PortStatusMap& GetPortStatusMap();
 
@@ -274,6 +275,7 @@ class PortForwardingController::Connection
       content::BrowserThread::UI>;
   friend class base::DeleteHelper<Connection>;
 
+  virtual ~Connection();
 
   typedef std::map<int, std::string> 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<Connection> 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<DevToolsAndroidBridge::RemoteDevice> device_;
   scoped_refptr<DevToolsAndroidBridge::RemoteBrowser> browser_;
-  scoped_ptr<DevToolsAndroidBridge::AndroidWebSocket> web_socket_;
+  scoped_refptr<DevToolsAndroidBridge::AndroidWebSocket> web_socket_;
   int command_id_;
   bool connected_;
   ForwardingMap forwarding_map_;
   CommandCallbackMap pending_responses_;
   PortStatusMap port_status_;
-  base::WeakPtrFactory<Connection> 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<Connection> 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(&registry_);
+    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.

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;
 

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<net::StreamSocket> 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<net::StreamSocket> 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));
 }

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<net::StreamSocket> 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<AndroidUsbDevice> 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<net::StreamSocket>(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<net::StreamSocket>(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<net::StreamSocket>(NULL));
+    callback.Run(result, NULL);
 }
 
 void UsbDeviceProvider::ReleaseDevice(const std::string& serial) {