DevTools: Removed refcounting from AndroidWebSocket

chrome/browser/devtools/device/port_forwarding_controller.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/devtools/device/port_forwarding_controller.h"
 
 #include <algorithm>
 #include <map>
 
 #include "base/bind.h"
@@ skipping 41 matching lines @@
 const int kMinVersionPortForwarding = 28;
 
 class SocketTunnel : public base::NonThreadSafe {
  public:
   typedef base::Callback<void(int)> CounterCallback;
 
   static void StartTunnel(const std::string& host,
                           int port,
                           const CounterCallback& callback,
                           int result,
-                          net::StreamSocket* socket) {
+                          scoped_ptr<net::StreamSocket> socket) {
     if (result < 0)
       return;
     SocketTunnel* tunnel = new SocketTunnel(callback);
-    tunnel->Start(socket, host, port);
+    tunnel->Start(socket.Pass(), host, port);
   }
 
  private:
   explicit SocketTunnel(const CounterCallback& callback)
       : pending_writes_(0),
         pending_destruction_(false),
         callback_(callback),
         about_to_destroy_(false) {
     callback_.Run(1);
   }
 
-  void Start(net::StreamSocket* socket, const std::string& host, int port) {
-    remote_socket_.reset(socket);
+  void Start(scoped_ptr<net::StreamSocket> socket,
+             const std::string& host, int port) {
+    remote_socket_.swap(socket);
 
     host_resolver_ = net::HostResolver::CreateDefaultResolver(NULL);
     net::HostResolver::RequestInfo request_info(net::HostPortPair(host, port));
     int result = host_resolver_->Resolve(
         request_info,
         net::DEFAULT_PRIORITY,
         &address_list_,
         base::Bind(&SocketTunnel::OnResolved, base::Unretained(this)),
         NULL,
         net::BoundNetLog());
@@ skipping 157 matching lines @@
       best_browser = browser;
       newest_version = current_version;
     }
   }
   return best_browser;
 }
 
 }  // namespace
 
 class PortForwardingController::Connection
-    : public DevToolsAndroidBridge::AndroidWebSocket::Delegate,
-      public base::RefCountedThreadSafe<
-          Connection,
-          content::BrowserThread::DeleteOnUIThread> {
+    : public DevToolsAndroidBridge::AndroidWebSocket::Delegate {
  public:
   Connection(Registry* registry,
              scoped_refptr<DevToolsAndroidBridge::RemoteDevice> device,
              scoped_refptr<DevToolsAndroidBridge::RemoteBrowser> browser,
              const ForwardingMap& forwarding_map);
+  virtual ~Connection();
 
   const PortStatusMap& GetPortStatusMap();
 
   void UpdateForwardingMap(const ForwardingMap& new_forwarding_map);
 
   void Shutdown();
 
  private:
   friend struct content::BrowserThread::DeleteOnThread<
       content::BrowserThread::UI>;
   friend class base::DeleteHelper<Connection>;
 
-  virtual ~Connection();
 
   typedef std::map<int, std::string> ForwardingMap;
 
   typedef base::Callback<void(PortStatus)> CommandCallback;
   typedef std::map<int, CommandCallback> CommandCallbackMap;
 
   void SerializeChanges(const std::string& method,
                         const ForwardingMap& old_map,
                         const ForwardingMap& new_map);
 
   void SendCommand(const std::string& method, int port);
   bool ProcessResponse(const std::string& json);
 
   void ProcessBindResponse(int port, PortStatus status);
   void ProcessUnbindResponse(int port, PortStatus status);
 
-  void UpdateSocketCountOnHandlerThread(int port, int increment);
+  static void UpdateSocketCountOnHandlerThread(
+      base::WeakPtr<Connection> weak_connection, 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(bool closed_by_device) OVERRIDE;
+  virtual void OnSocketClosed() OVERRIDE;
 
   PortForwardingController::Registry* registry_;
   scoped_refptr<DevToolsAndroidBridge::RemoteDevice> device_;
   scoped_refptr<DevToolsAndroidBridge::RemoteBrowser> browser_;
-  scoped_refptr<DevToolsAndroidBridge::AndroidWebSocket> web_socket_;
+  scoped_ptr<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);
 };
 
 PortForwardingController::Connection::Connection(
     Registry* registry,
     scoped_refptr<DevToolsAndroidBridge::RemoteDevice> device,
     scoped_refptr<DevToolsAndroidBridge::RemoteBrowser> browser,
     const ForwardingMap& forwarding_map)
     : registry_(registry),
       device_(device),
       browser_(browser),
       command_id_(0),
       connected_(false),
-      forwarding_map_(forwarding_map) {
+      forwarding_map_(forwarding_map),
+      weak_factory_(this) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   (*registry_)[device_->serial()] = 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();
+  web_socket_.reset(
+      browser->CreateWebSocket(kDevToolsRemoteBrowserTarget, this));
 }
 
 PortForwardingController::Connection::~Connection() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (registry_) {
-    DCHECK(registry_->find(device_->serial()) != registry_->end());
-    registry_->erase(device_->serial());
-  }
+  DCHECK(registry_->find(device_->serial()) != registry_->end());
+  registry_->erase(device_->serial());
 }
 
 void PortForwardingController::Connection::UpdateForwardingMap(
     const ForwardingMap& new_forwarding_map) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (connected_) {
     SerializeChanges(kTetheringUnbind, new_forwarding_map, forwarding_map_);
     SerializeChanges(kTetheringBind, forwarding_map_, new_forwarding_map);
   }
   forwarding_map_ = new_forwarding_map;
@@ skipping 76 matching lines @@
     int port, PortStatus status) {
   PortStatusMap::iterator it = port_status_.find(port);
   if (it == port_status_.end())
     return;
   if (status == kStatusError)
     it->second = status;
   else
     port_status_.erase(it);
 }
 
+// static
 void PortForwardingController::Connection::UpdateSocketCountOnHandlerThread(
-    int port, int increment) {
+    base::WeakPtr<Connection> weak_connection, int port, int increment) {
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-     base::Bind(&Connection::UpdateSocketCount, this, port, increment));
+     base::Bind(&Connection::UpdateSocketCount,
+                weak_connection, port, increment));
 }
 
 void PortForwardingController::Connection::UpdateSocketCount(
     int port, int increment) {
 #if defined(DEBUG_DEVTOOLS)
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   PortStatusMap::iterator it = port_status_.find(port);
   if (it == port_status_.end())
     return;
   if (it->second < 0 || (it->second == 0 && increment < 0))
     return;
   it->second += increment;
 #endif  // defined(DEBUG_DEVTOOLS)
 }
 
 const PortForwardingController::PortStatusMap&
 PortForwardingController::Connection::GetPortStatusMap() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   return port_status_;
 }
 
 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(
-    bool closed_by_device) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  Release();  // Balanced in the constructor.
+void PortForwardingController::Connection::OnSocketClosed() {
+  delete this;
 }
 
 void PortForwardingController::Connection::OnFrameRead(
     const std::string& message) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (ProcessResponse(message))
     return;
 
   scoped_ptr<DevToolsProtocol::Notification> notification(
       DevToolsProtocol::ParseNotification(message));
@@ skipping 19 matching lines @@
 
   std::string location = it->second;
   std::vector<std::string> tokens;
   Tokenize(location, ":", &tokens);
   int destination_port = 0;
   if (tokens.size() != 2 || !base::StringToInt(tokens[1], &destination_port))
     return;
   std::string destination_host = tokens[0];
 
   SocketTunnel::CounterCallback callback =
-      base::Bind(&Connection::UpdateSocketCountOnHandlerThread, this, port);
+      base::Bind(&Connection::UpdateSocketCountOnHandlerThread,
+                 weak_factory_.GetWeakPtr(), port);
 
   device_->OpenSocket(
       connection_id.c_str(),
       base::Bind(&SocketTunnel::StartTunnel,
                  destination_host,
                  destination_port,
                  callback));
 }
 
 PortForwardingController::PortForwardingController(Profile* profile)
@@ skipping 66 matching lines @@
           dict->GetString(it.key(), &location))
         forwarding_map_[port_num] = location;
     }
   }
 
   if (!forwarding_map_.empty()) {
     StartListening();
     UpdateConnections();
   } else {
     StopListening();
-    ShutdownConnections();
+    STLDeleteValues(&registry_);

[vkuzkokov, 2014/09/23 13:50:22]

FYI, this is the place where crash happened:
delete called registry_->erase which invalidated iterator inside
STLDeleteValues.

     NotifyListeners(DevicesStatus());
   }
 }
 
 void PortForwardingController::StartListening() {
   if (listening_)
     return;
   listening_ = true;
   DevToolsAndroidBridge* android_bridge =
       DevToolsAndroidBridge::Factory::GetForProfile(profile_);
@@ skipping 10 matching lines @@
       DevToolsAndroidBridge::Factory::GetForProfile(profile_);
   if (android_bridge)
     android_bridge->RemoveDeviceListListener(this);
 }
 
 void PortForwardingController::UpdateConnections() {
   for (Registry::iterator it = registry_.begin(); it != registry_.end(); ++it)
     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.
   for (Listeners::const_iterator it = copy.begin(); it != copy.end(); ++it)
     (*it)->PortStatusChanged(status);
 }
 
 // static
 PortForwardingController::Factory*
 PortForwardingController::Factory::GetInstance() {
@@ skipping 12 matching lines @@
           "PortForwardingController",
           BrowserContextDependencyManager::GetInstance()) {}
 
 PortForwardingController::Factory::~Factory() {}
 
 KeyedService* PortForwardingController::Factory::BuildServiceInstanceFor(
     content::BrowserContext* context) const {
   Profile* profile = Profile::FromBrowserContext(context);
   return new PortForwardingController(profile);
 }