Introduce the global registry of DevToolsAgentHost instances.

Every DevToolsAgentHost is registered in a static map (id->raw pointer). It is retained by its debug target (RVH or worker) e.g. it exists at least as long as the debug target.

This eliminates the fairly elaborate garbage-collected two way id<->agenthost mapping in DevToolsHttpHandlerImpl.

BUG=None
TBR=joi@chromium.org

Review URL: https://codereview.chromium.org/12426007

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

7 files changed, 50 insertions, 40 deletions

diff --git a/chrome/browser/devtools/devtools_sanity_browsertest.cc b/chrome/browser/devtools/devtools_sanity_browsertest.cc
index 58af8f9..37e6690 100644
--- a/chrome/browser/devtools/devtools_sanity_browsertest.cc
+++ b/chrome/browser/devtools/devtools_sanity_browsertest.cc
@@ -614,6 +614,23 @@ IN_PROC_BROWSER_TEST_F(DevToolsSanityTest, TestAddMessageToConsole) {
   CloseDevToolsWindow();
 }
 
+class DevToolsAgentHostTest : public InProcessBrowserTest {};
+
+// Tests DevToolsAgentHost retention by its target.
+IN_PROC_BROWSER_TEST_F(DevToolsAgentHostTest, TestAgentHostReleased) {
+  ui_test_utils::NavigateToURL(browser(), GURL("about:blank"));
+  RenderViewHost* rvh = browser()->tab_strip_model()->GetWebContentsAt(0)->
+      GetRenderViewHost();
+  DevToolsAgentHost* agent_raw = DevToolsAgentHost::GetFor(rvh);
+  const std::string agent_id = agent_raw->GetId();
+  ASSERT_EQ(agent_raw, DevToolsAgentHost::GetForId(agent_id)) <<
+      "DevToolsAgentHost cannot be found by id";
+  browser()->tab_strip_model()->
+      CloseWebContentsAt(0, TabStripModel::CLOSE_NONE);
+  ASSERT_FALSE(DevToolsAgentHost::GetForId(agent_id)) <<
+      "DevToolsAgentHost is not released when the tab is closed";
+}
+
 class DISABLED_RemoteDebuggingTest : public ExtensionBrowserTest {
 
   class ResultCatcher : public content::NotificationObserver {
diff --git a/content/browser/devtools/devtools_agent_host_impl.cc b/content/browser/devtools/devtools_agent_host_impl.cc
index cb2b31e..7ccc15a 100644
--- a/content/browser/devtools/devtools_agent_host_impl.cc
+++ b/content/browser/devtools/devtools_agent_host_impl.cc
@@ -4,19 +4,38 @@
 
 #include "content/browser/devtools/devtools_agent_host_impl.h"
 
+#include <map>
+
 #include "base/basictypes.h"
-#include "base/stringprintf.h"
+#include "base/guid.h"
+#include "base/lazy_instance.h"
 #include "content/common/devtools_messages.h"
 
 namespace content {
 
 namespace {
-static int g_next_agent_host_id = 0;
+typedef std::map<std::string, DevToolsAgentHostImpl*> Instances;
+base::LazyInstance<Instances>::Leaky g_instances = LAZY_INSTANCE_INITIALIZER;
 }  // namespace
 
 DevToolsAgentHostImpl::DevToolsAgentHostImpl()
     : close_listener_(NULL),
-      id_(base::StringPrintf("%d", ++g_next_agent_host_id)) {
+      id_(base::GenerateGUID()) {
+  g_instances.Get()[id_] = this;
+}
+
+DevToolsAgentHostImpl::~DevToolsAgentHostImpl() {
+  g_instances.Get().erase(g_instances.Get().find(id_));
+}
+
+scoped_refptr<DevToolsAgentHost> DevToolsAgentHost::GetForId(
+    const std::string& id) {
+  if (g_instances == NULL)
+    return NULL;
+  Instances::iterator it = g_instances.Get().find(id);
+  if (it == g_instances.Get().end())
+    return NULL;
+  return it->second;
 }
 
 void DevToolsAgentHostImpl::Attach() {
diff --git a/content/browser/devtools/devtools_agent_host_impl.h b/content/browser/devtools/devtools_agent_host_impl.h
index 5e9cdfb..f0f3ce8 100644
--- a/content/browser/devtools/devtools_agent_host_impl.h
+++ b/content/browser/devtools/devtools_agent_host_impl.h
@@ -48,7 +48,7 @@ class CONTENT_EXPORT DevToolsAgentHostImpl : public DevToolsAgentHost {
 
  protected:
   DevToolsAgentHostImpl();
-  virtual ~DevToolsAgentHostImpl() {}
+  virtual ~DevToolsAgentHostImpl();
 
   virtual void SendMessageToAgent(IPC::Message* msg) = 0;
   virtual void NotifyClientAttaching() = 0;
diff --git a/content/browser/devtools/devtools_http_handler_impl.cc b/content/browser/devtools/devtools_http_handler_impl.cc
index e2affcb..53ec417 100644
--- a/content/browser/devtools/devtools_http_handler_impl.cc
+++ b/content/browser/devtools/devtools_http_handler_impl.cc
@@ -10,7 +10,6 @@
 #include "base/bind.h"
 #include "base/compiler_specific.h"
 #include "base/file_util.h"
-#include "base/guid.h"
 #include "base/json/json_writer.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
@@ -63,45 +62,13 @@ class DevToolsDefaultBindingHandler
   DevToolsDefaultBindingHandler() {
   }
 
-  void GarbageCollect() {
-    AgentsMap::iterator it = guid_to_agent_.begin();
-    while (it != guid_to_agent_.end()) {
-      DevToolsAgentHost* agent = it->second;
-      if (!agent->GetRenderViewHost()) {
-        GUIDMap::iterator agent_it = agent_to_guid_.find(agent);
-        DCHECK(agent_it != agent_to_guid_.end());
-        agent_to_guid_.erase(agent_it);
-        guid_to_agent_.erase(it++);
-      } else
-        ++it;
-    }
-  }
-
   virtual std::string GetIdentifier(DevToolsAgentHost* agent_host) OVERRIDE {
-    GarbageCollect();
-    if (agent_to_guid_.find(agent_host) == agent_to_guid_.end()) {
-      std::string guid = base::GenerateGUID();
-      agent_to_guid_[agent_host] = guid;
-      guid_to_agent_[guid] = agent_host;
-    }
-    return agent_to_guid_[agent_host];
+    return agent_host->GetId();
   }
 
-  virtual DevToolsAgentHost* ForIdentifier(
-      const std::string& identifier) OVERRIDE {
-    GarbageCollect();
-    AgentsMap::iterator it = guid_to_agent_.find(identifier);
-    if (it != guid_to_agent_.end())
-      return it->second;
-    return NULL;
+  virtual DevToolsAgentHost* ForIdentifier(const std::string& id) OVERRIDE {
+    return DevToolsAgentHost::GetForId(id);
   }
-
- private:
-  typedef std::map<std::string, scoped_refptr<DevToolsAgentHost> > AgentsMap;
-  AgentsMap guid_to_agent_;
-
-  typedef std::map<DevToolsAgentHost*, std::string> GUIDMap;
-  GUIDMap agent_to_guid_;
 };
 
 // An internal implementation of DevToolsClientHost that delegates
diff --git a/content/browser/devtools/render_view_devtools_agent_host.cc b/content/browser/devtools/render_view_devtools_agent_host.cc
index 5fe5ff7..6de7307 100644
--- a/content/browser/devtools/render_view_devtools_agent_host.cc
+++ b/content/browser/devtools/render_view_devtools_agent_host.cc
@@ -173,6 +173,7 @@ RenderViewDevToolsAgentHost::RenderViewDevToolsAgentHost(
   RenderViewHostDelegate* delegate = render_view_host_->GetDelegate();
   if (delegate && delegate->GetAsWebContents())
     Observe(delegate->GetAsWebContents());
+  AddRef();  // Balanced in RenderViewHostDestroyed.
 }
 
 RenderViewHost* RenderViewDevToolsAgentHost::GetRenderViewHost() {
@@ -290,6 +291,7 @@ void RenderViewDevToolsAgentHost::RenderViewHostDestroyed(
   scoped_refptr<RenderViewDevToolsAgentHost> protect(this);
   NotifyCloseListener();
   render_view_host_ = NULL;
+  Release();
 }
 
 bool RenderViewDevToolsAgentHost::OnRvhMessageReceived(
diff --git a/content/browser/devtools/worker_devtools_manager.cc b/content/browser/devtools/worker_devtools_manager.cc
index 6e5443f..ae32629 100644
--- a/content/browser/devtools/worker_devtools_manager.cc
+++ b/content/browser/devtools/worker_devtools_manager.cc
@@ -61,6 +61,7 @@ class WorkerDevToolsManager::WorkerDevToolsAgentHost
   explicit WorkerDevToolsAgentHost(WorkerId worker_id)
       : has_worker_id_(false) {
     SetWorkerId(worker_id, false);
+    AddRef();  //  Balanced in ResetWorkerId.
   }
 
   void SetWorkerId(WorkerId worker_id, bool reattach) {
@@ -83,6 +84,7 @@ class WorkerDevToolsManager::WorkerDevToolsAgentHost
   void ResetWorkerId() {
     g_agent_map.Get().erase(worker_id_);
     has_worker_id_ = false;
+    Release();
   }
 
   void SaveAgentRuntimeState(const std::string& state) {
diff --git a/content/public/browser/devtools_agent_host.h b/content/public/browser/devtools_agent_host.h
index 8395b2d..fcf9ed6 100644
--- a/content/public/browser/devtools_agent_host.h
+++ b/content/public/browser/devtools_agent_host.h
@@ -21,6 +21,9 @@ class WebContents;
 class CONTENT_EXPORT DevToolsAgentHost
     : public base::RefCounted<DevToolsAgentHost> {
  public:
+  // Returns DevToolsAgentHost with a given |id| or NULL of it does not exist.
+  static scoped_refptr<DevToolsAgentHost> GetForId(const std::string& id);
+
   // Returns DevToolsAgentHost that can be used for inspecting |rvh|.
   // New DevToolsAgentHost will be created if it does not exist.
   static scoped_refptr<DevToolsAgentHost> GetFor(RenderViewHost* rvh);