Review URL: http://codereview.chromium.org/87034

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@14187 0039d316-1c4b-4281-b951-d872f2087c98

12 files changed, 89 insertions, 79 deletions

diff --git a/chrome/browser/debugger/debugger_remote_service.cc b/chrome/browser/debugger/debugger_remote_service.cc
index 7053ab7..a856fb6 100644
--- a/chrome/browser/debugger/debugger_remote_service.cc
+++ b/chrome/browser/debugger/debugger_remote_service.cc
@@ -140,6 +140,10 @@ void DebuggerRemoteService::HandleMessage(
   }
 }
 
+void DebuggerRemoteService::OnConnectionLost() {
+  delegate_->inspectable_tab_proxy()->OnRemoteDebuggerDetached();
+}
+
 void DebuggerRemoteService::SendResponse(const Value& response,
                                          const std::string& tool,
                                          const std::string& destination) {
@@ -212,7 +216,7 @@ void DebuggerRemoteService::AttachTab(const std::string& destination,
   if (g_browser_process->devtools_manager()->GetDevToolsClientHostFor(
       *web_contents) == NULL) {
     DevToolsClientHost* client_host =
-        InspectableTabProxy::NewClientHost(tab_uid, this);
+        delegate_->inspectable_tab_proxy()->NewClientHost(tab_uid, this);
     DevToolsManager* manager = g_browser_process->devtools_manager();
     if (manager != NULL) {
       manager->RegisterDevToolsClientHostFor(*web_contents, client_host);
diff --git a/chrome/browser/debugger/debugger_remote_service.h b/chrome/browser/debugger/debugger_remote_service.h
index da46863..53d06ba 100644
--- a/chrome/browser/debugger/debugger_remote_service.h
+++ b/chrome/browser/debugger/debugger_remote_service.h
@@ -9,6 +9,7 @@
 
 #include "base/basictypes.h"
 #include "base/scoped_ptr.h"
+#include "chrome/browser/debugger/devtools_protocol_handler.h"
 #include "chrome/browser/debugger/devtools_remote.h"
 
 class DevToolsProtocolHandler;
@@ -37,8 +38,16 @@ class DebuggerRemoteService : public DevToolsRemoteListener {
   // Handles a JSON message from the tab_id-associated V8 debugger.
   void DebuggerOutput(int32 tab_id, const std::string& message);
 
+  // Expose to public so that we can detach from tab
+  // on remote debugger connection loss. If |response| is not NULL,
+  // the operation result will be written as the "result" field in |response|,
+  // otherwise it will not be propagated back to the caller.
+  void DetachTab(const std::string& destination,
+                 DictionaryValue* response);
+
   // DevToolsRemoteListener interface
   virtual void HandleMessage(const DevToolsRemoteMessage& message);
+  virtual void OnConnectionLost();
 
   static const std::string kToolName;
 
@@ -54,8 +63,6 @@ class DebuggerRemoteService : public DevToolsRemoteListener {
 
   void AttachTab(const std::string& destination,
                  DictionaryValue* response);
-  void DetachTab(const std::string& destination,
-                 DictionaryValue* response);
   WebContents* ToWebContents(int32 tab_uid);
   void SendResponse(const Value& response,
                     const std::string& tool,
diff --git a/chrome/browser/debugger/devtools_protocol_handler.cc b/chrome/browser/debugger/devtools_protocol_handler.cc
index 5350298..5b7644c 100644
--- a/chrome/browser/debugger/devtools_protocol_handler.cc
+++ b/chrome/browser/debugger/devtools_protocol_handler.cc
@@ -104,4 +104,11 @@ void DevToolsProtocolHandler::DidClose(ListenSocket *sock) {
   DCHECK(connection_ == sock);
   connection_ = NULL;
   sock->Release();
+  for (ToolToListenerMap::const_iterator it = tool_to_listener_map_.begin(),
+       end = tool_to_listener_map_.end();
+       it != end;
+       ++it) {
+    ui_loop_->PostTask(FROM_HERE, NewRunnableMethod(
+        it->second.get(), &DevToolsRemoteListener::OnConnectionLost));
+  }
 }
diff --git a/chrome/browser/debugger/devtools_protocol_handler.h b/chrome/browser/debugger/devtools_protocol_handler.h
index 4af8ebb..7080794 100644
--- a/chrome/browser/debugger/devtools_protocol_handler.h
+++ b/chrome/browser/debugger/devtools_protocol_handler.h
@@ -56,6 +56,7 @@ class DevToolsProtocolHandler
 
   // DevToolsRemoteListener interface
   virtual void HandleMessage(const DevToolsRemoteMessage& message);
+  virtual void OnConnectionLost() {}
 
   // OutboundSocketDelegate interface
   virtual void Send(const DevToolsRemoteMessage& message);
diff --git a/chrome/browser/debugger/devtools_remote.h b/chrome/browser/debugger/devtools_remote.h
index c475030..d5664a5 100644
--- a/chrome/browser/debugger/devtools_remote.h
+++ b/chrome/browser/debugger/devtools_remote.h
@@ -18,6 +18,9 @@ class DevToolsRemoteListener
   DevToolsRemoteListener() {}
   virtual ~DevToolsRemoteListener() {}
   virtual void HandleMessage(const DevToolsRemoteMessage& message) = 0;
+  // This method is invoked on the UI thread whenever the debugger connection
+  // has been lost.
+  virtual void OnConnectionLost() = 0;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(DevToolsRemoteListener);
diff --git a/chrome/browser/debugger/devtools_remote_listen_socket_unittest.h b/chrome/browser/debugger/devtools_remote_listen_socket_unittest.h
index 73d8714..a1dbc62 100644
--- a/chrome/browser/debugger/devtools_remote_listen_socket_unittest.h
+++ b/chrome/browser/debugger/devtools_remote_listen_socket_unittest.h
@@ -99,6 +99,7 @@ class DevToolsRemoteListenSocketTester :
 
   // DevToolsRemoteMessageHandler interface
   virtual void HandleMessage(const DevToolsRemoteMessage& message);
+  virtual void OnConnectionLost() {}
 
   // read all pending data from the test socket
   int ClearTestSocket();
diff --git a/chrome/browser/debugger/devtools_remote_message.cc b/chrome/browser/debugger/devtools_remote_message.cc
index 09e62d5..97c6875 100644
--- a/chrome/browser/debugger/devtools_remote_message.cc
+++ b/chrome/browser/debugger/devtools_remote_message.cc
@@ -12,7 +12,7 @@ const char DevToolsRemoteMessageHeaders::kDestination[] = "Destination";
 const char DevToolsRemoteMessage::kEmptyValue[] = "";
 
 DevToolsRemoteMessageBuilder& DevToolsRemoteMessageBuilder::instance() {
-  static DevToolsRemoteMessageBuilder instance_(new IdGeneratorImpl);
+  static DevToolsRemoteMessageBuilder instance_;
   return instance_;
 }
 
diff --git a/chrome/browser/debugger/devtools_remote_message.h b/chrome/browser/debugger/devtools_remote_message.h
index 7665269..ec75679 100644
--- a/chrome/browser/debugger/devtools_remote_message.h
+++ b/chrome/browser/debugger/devtools_remote_message.h
@@ -85,49 +85,16 @@ class DevToolsRemoteMessage {
 // DevToolsRemote messages.
 class DevToolsRemoteMessageBuilder {
  public:
-  class IdGenerator {
-   public:
-    virtual ~IdGenerator() {}
-    virtual int32 NextId() = 0;
-  };
   // A singleton instance getter.
   static DevToolsRemoteMessageBuilder& instance();
   // Creates a message given the certain header values and a payload.
   DevToolsRemoteMessage* Create(const std::string& tool,
                                 const std::string& destination,
                                 const std::string& payload);
-  // Sets a message ID generator instance. The builder then owns this instance
-  // and deletes it upon termination.
-  void set_id_generator(IdGenerator* id_generator) {
-    if (id_generator_ != NULL) {
-      delete id_generator_;
-      id_generator_ = id_generator;
-    } else {
-      NOTREACHED();
-    }
-  }
 
  private:
-  class IdGeneratorImpl : public IdGenerator {
-   public:
-    IdGeneratorImpl() : id_(1) {}
-    virtual int32 NextId() {
-      return id_++;
-    }
-   private:
-    int32 id_;
-  };
-
-  explicit DevToolsRemoteMessageBuilder(IdGenerator* id_generator)
-      : id_generator_(id_generator) {}
-  ~DevToolsRemoteMessageBuilder() {
-    delete id_generator_;
-  }
-  int32 NextMessageId() {
-    return id_generator_->NextId();
-  }
-
-  IdGenerator* id_generator_;
+  DevToolsRemoteMessageBuilder() {}
+  virtual ~DevToolsRemoteMessageBuilder() {}
   DISALLOW_COPY_AND_ASSIGN(DevToolsRemoteMessageBuilder);
 };
 
diff --git a/chrome/browser/debugger/devtools_remote_message_unittest.cc b/chrome/browser/debugger/devtools_remote_message_unittest.cc
index ad0c3a5..bc84a4c 100644
--- a/chrome/browser/debugger/devtools_remote_message_unittest.cc
+++ b/chrome/browser/debugger/devtools_remote_message_unittest.cc
@@ -9,14 +9,6 @@
 #include "chrome/browser/debugger/devtools_remote_message.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-namespace {
-class TestIdGenerator : public DevToolsRemoteMessageBuilder::IdGenerator {
-  virtual int32 NextId() {
-    return 123;
-  }
-};
-}
-
 class DevToolsRemoteMessageTest : public testing::Test {
  public:
   DevToolsRemoteMessageTest() : testing::Test() {
@@ -48,8 +40,6 @@ TEST_F(DevToolsRemoteMessageTest, ConstructInstanceManually) {
 }
 
 TEST_F(DevToolsRemoteMessageTest, ConstructWithBuilder) {
-  DevToolsRemoteMessageBuilder::instance().set_id_generator(
-      new TestIdGenerator);
   std::string content = "Responsecontent";
   testing::internal::scoped_ptr<DevToolsRemoteMessage> message(
       DevToolsRemoteMessageBuilder::instance().Create(
diff --git a/chrome/browser/debugger/devtools_remote_service.h b/chrome/browser/debugger/devtools_remote_service.h
index 96bdab4..5a4f3cd 100644
--- a/chrome/browser/debugger/devtools_remote_service.h
+++ b/chrome/browser/debugger/devtools_remote_service.h
@@ -30,6 +30,7 @@ class DevToolsRemoteService : public DevToolsRemoteListener {
 
   // DevToolsRemoteListener interface
   virtual void HandleMessage(const DevToolsRemoteMessage& message);
+  virtual void OnConnectionLost() {}
 
   static const std::string kToolName;
 
diff --git a/chrome/browser/debugger/inspectable_tab_proxy.cc b/chrome/browser/debugger/inspectable_tab_proxy.cc
index c216766..48e0155 100644
--- a/chrome/browser/debugger/inspectable_tab_proxy.cc
+++ b/chrome/browser/debugger/inspectable_tab_proxy.cc
@@ -16,29 +16,6 @@
 #include "chrome/browser/tabs/tab_strip_model.h"
 #include "chrome/common/devtools_messages.h"
 
-namespace {
-
-// An internal implementation of DevToolsClientHost that delegates
-// messages sent for DevToolsClient to a DebuggerShell instance.
-class DevToolsClientHostImpl : public DevToolsClientHost {
- public:
-  DevToolsClientHostImpl(int32 id, DebuggerRemoteService* service)
-      : id_(id),
-        service_(service) {}
-
-  // DevToolsClientHost interface
-  virtual void InspectedTabClosing();
-  virtual void SendMessageToClient(const IPC::Message& msg);
-
- private:
-  // Message handling routines
-  void OnRpcMessage(const std::string& msg);
-  void DebuggerOutput(const std::string& msg);
-
-  int32 id_;
-  DebuggerRemoteService* service_;
-};
-
 void DevToolsClientHostImpl::InspectedTabClosing() {
   NotifyCloseListener();
   delete this;
@@ -76,8 +53,6 @@ void DevToolsClientHostImpl::DebuggerOutput(const std::string& msg) {
   service_->DebuggerOutput(id_, msg);
 }
 
-}  // namespace
-
 const InspectableTabProxy::ControllersMap&
     InspectableTabProxy::controllers_map() {
   controllers_map_.clear();
@@ -93,9 +68,19 @@ const InspectableTabProxy::ControllersMap&
   return controllers_map_;
 }
 
-// static
 DevToolsClientHost* InspectableTabProxy::NewClientHost(
     int32 id,
     DebuggerRemoteService* service) {
-  return new DevToolsClientHostImpl(id, service);
+  DevToolsClientHostImpl* client_host =
+      new DevToolsClientHostImpl(id, service, &id_to_client_host_map_);
+  id_to_client_host_map_[id] = client_host;
+  return client_host;
+}
+
+void InspectableTabProxy::OnRemoteDebuggerDetached() {
+  while (id_to_client_host_map_.size() > 0) {
+    IdToClientHostMap::iterator it = id_to_client_host_map_.begin();
+    it->second->debugger_remote_service()->DetachTab(IntToString(it->first),
+                                                     NULL);
+  }
 }
diff --git a/chrome/browser/debugger/inspectable_tab_proxy.h b/chrome/browser/debugger/inspectable_tab_proxy.h
index fdf82d9..c47c4d5 100644
--- a/chrome/browser/debugger/inspectable_tab_proxy.h
+++ b/chrome/browser/debugger/inspectable_tab_proxy.h
@@ -9,33 +9,77 @@
 
 #include "base/basictypes.h"
 #include "base/hash_tables.h"
+#include "chrome/browser/debugger/devtools_client_host.h"
 
 class DebuggerRemoteService;
 class DevToolsClientHost;
+class DevToolsClientHostImpl;
 class NavigationController;
 
 // Proxies debugged tabs' NavigationControllers using their UIDs.
 class InspectableTabProxy {
  public:
   typedef base::hash_map<int32, NavigationController*> ControllersMap;
+  typedef base::hash_map<int32, DevToolsClientHostImpl*> IdToClientHostMap;
 
   InspectableTabProxy() {}
   virtual ~InspectableTabProxy() {}
 
   // Returns a map of NavigationControllerKeys to NavigationControllers
-  // for all Browser instances.
+  // for all Browser instances. Clients should not keep the result around
+  // for extended periods of time as tabs might get closed thus invalidating
+  // the map.
   const ControllersMap& controllers_map();
 
   // Creates a new DevToolsClientHost implementor instance.
   // |id| is the UID of the tab to debug.
   // |service| is the DebuggerRemoteService instance the DevToolsClient
   //         messages shall be dispatched to.
-  static DevToolsClientHost* NewClientHost(int32 id,
-                                           DebuggerRemoteService* service);
+  DevToolsClientHost* NewClientHost(int32 id,
+                                    DebuggerRemoteService* service);
+
+  // Gets invoked when a remote debugger is detached. In this case we should
+  // send the corresponding message to the V8 debugger for each of the tabs
+  // the debugger is attached to, and invoke InspectedTabClosing().
+  void OnRemoteDebuggerDetached();
 
  private:
   ControllersMap controllers_map_;
+  IdToClientHostMap id_to_client_host_map_;
   DISALLOW_COPY_AND_ASSIGN(InspectableTabProxy);
 };
 
+
+// An internal implementation of DevToolsClientHost that delegates
+// messages sent for DevToolsClient to a DebuggerShell instance.
+class DevToolsClientHostImpl : public DevToolsClientHost {
+ public:
+  DevToolsClientHostImpl(
+    int32 id,
+    DebuggerRemoteService* service,
+    InspectableTabProxy::IdToClientHostMap* map)
+      : id_(id),
+        service_(service),
+        map_(map) {}
+  ~DevToolsClientHostImpl() {
+    map_->erase(this->id_);
+  }
+  DebuggerRemoteService* debugger_remote_service() {
+    return service_;
+  }
+
+  // DevToolsClientHost interface
+  virtual void InspectedTabClosing();
+  virtual void SendMessageToClient(const IPC::Message& msg);
+
+ private:
+  // Message handling routines
+  void OnRpcMessage(const std::string& msg);
+  void DebuggerOutput(const std::string& msg);
+
+  int32 id_;
+  DebuggerRemoteService* service_;
+  InspectableTabProxy::IdToClientHostMap* map_;
+};
+
 #endif  // CHROME_BROWSER_DEBUGGER_INSPECTABLE_TAB_PROXY_H_