DevTools: prepare workers for DevToolsManager::Attach(Detach)ClientHost removal.

content/browser/devtools/worker_devtools_manager.cc

Index: content/browser/devtools/worker_devtools_manager.cc
diff --git a/content/browser/devtools/worker_devtools_manager.cc b/content/browser/devtools/worker_devtools_manager.cc
index f341c8291485e4f97eeb6335aab9d55bd4026b9a..a837f554044d98a20e8ad39f0d1704e6ef9fcf3f 100644
--- a/content/browser/devtools/worker_devtools_manager.cc
+++ b/content/browser/devtools/worker_devtools_manager.cc
@@ -42,7 +42,8 @@ public:
     instance_->map_[id] = host;
   }
   static void Remove(WorkerId id) {
-    DCHECK(instance_);
+    if (!instance_)
+      return;
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
     Instances& map = instance_->map_;
     map.erase(id);
@@ -91,9 +92,18 @@ struct WorkerDevToolsManager::TerminatedInspectedWorker {
 class WorkerDevToolsManager::WorkerDevToolsAgentHost
     : public DevToolsAgentHost {
  public:
-  explicit WorkerDevToolsAgentHost(WorkerId worker_id)
-      : worker_id_(worker_id) {
-    AgentHosts::Add(worker_id, this);
+  explicit WorkerDevToolsAgentHost(WorkerId worker_id) : attached_(false) {
+    AttachToWorker(worker_id, false);
+  }
+
+  bool attached() { return attached_; }

[yurys, 2012/12/20 11:45:40]

Could you pick another name to avoid confusion with attachment to client?

[pfeldman, 2012/12/20 16:53:54]

Done.

+
+  void AttachToWorker(WorkerId worker_id, bool reattach) {
+    AgentHosts::Remove(worker_id_);
+    worker_id_ = worker_id;
+    AgentHosts::Add(worker_id_, this);
+    attached_ = true;
+
     BrowserThread::PostTask(
         BrowserThread::IO,
         FROM_HERE,
@@ -101,13 +111,25 @@ class WorkerDevToolsManager::WorkerDevToolsAgentHost
             &RegisterAgent,
             worker_id.first,
             worker_id.second));
+
+    if (reattach)
+      Reattach(state_);
   }
 
-  void WorkerDestroyed() {
-    NotifyCloseListener();
+  virtual void Detach() {
+    DevToolsAgentHost::Detach();
+    AgentHosts::Remove(worker_id_);
     delete this;
   }
 
+  void DetachFromWorker() {
+    attached_ = false;
+  }
+
+  void SaveAgentRuntimeState(const std::string& state) {
+    state_ = state;
+  }
+
  private:
   virtual ~WorkerDevToolsAgentHost() {
     AgentHosts::Remove(worker_id_);
@@ -130,6 +152,8 @@ class WorkerDevToolsManager::WorkerDevToolsAgentHost
 
   // DevToolsAgentHost implementation.
   virtual void SendMessageToAgent(IPC::Message* message) OVERRIDE {
+    if (!attached_)
+      return;

[yurys, 2012/12/20 11:45:40]

The message object will leak here.

[pfeldman, 2012/12/20 16:53:54]

Done.

     BrowserThread::PostTask(
         BrowserThread::IO, FROM_HERE,
         base::Bind(
@@ -138,11 +162,14 @@ class WorkerDevToolsManager::WorkerDevToolsAgentHost
             worker_id_.second,
             base::Owned(message)));
   }
+
   virtual void NotifyClientAttaching() OVERRIDE {}
   virtual void NotifyClientDetaching() OVERRIDE {}
   virtual int GetRenderProcessId() OVERRIDE { return -1; }
 
   WorkerId worker_id_;
+  bool attached_;
+  std::string state_;
 
   DISALLOW_COPY_AND_ASSIGN(WorkerDevToolsAgentHost);
 };
@@ -152,8 +179,11 @@ class WorkerDevToolsManager::DetachedClientHosts {
  public:
   static void WorkerReloaded(WorkerId old_id, WorkerId new_id) {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-    if (instance_ && instance_->ReattachClient(old_id, new_id))
+    WorkerDevToolsAgentHost* agent = AgentHosts::GetAgentHost(old_id);
+    if (agent && agent->attached()) {
+      agent->AttachToWorker(new_id, true);
       return;
+    }
     RemovePendingWorkerData(old_id);
   }
 
@@ -164,44 +194,21 @@ class WorkerDevToolsManager::DetachedClientHosts {
       RemovePendingWorkerData(id);
       return;
     }
-    DevToolsManagerImpl::GetInstance()->DispatchOnInspectorFrontend(
+    DevToolsManagerImpl* devtools_manager = DevToolsManagerImpl::GetInstance();
+    devtools_manager->DispatchOnInspectorFrontend(
         agent,
         WebKit::WebDevToolsAgent::workerDisconnectedFromWorkerEvent().utf8());
-    int cookie = DevToolsManagerImpl::GetInstance()->DetachClientHost(agent);
-    agent->WorkerDestroyed();
-    if (cookie == -1) {
+    DevToolsClientHost* client_host =
+        devtools_manager->GetDevToolsClientHostFor(agent);
+    if (!client_host)
       RemovePendingWorkerData(id);
       return;
-    }
-    if (!instance_)
-      new DetachedClientHosts();
-    instance_->worker_id_to_cookie_[id] = cookie;
+    agent->DetachFromWorker();
   }
 
  private:
-  DetachedClientHosts() {
-    instance_ = this;
-  }
-  ~DetachedClientHosts() {
-    instance_ = NULL;
-  }
-
-  bool ReattachClient(WorkerId old_id, WorkerId new_id) {
-    WorkerIdToCookieMap::iterator it = worker_id_to_cookie_.find(old_id);
-    if (it == worker_id_to_cookie_.end())
-      return false;
-    DevToolsAgentHost* agent =
-        WorkerDevToolsManager::GetDevToolsAgentHostForWorker(
-            new_id.first,
-            new_id.second);
-    DevToolsManagerImpl::GetInstance()->AttachClientHost(
-        it->second,
-        agent);
-    worker_id_to_cookie_.erase(it);
-    if (worker_id_to_cookie_.empty())
-      delete this;
-    return true;
-  }
+  DetachedClientHosts() { }
+  ~DetachedClientHosts() { }
 
   static void RemovePendingWorkerData(WorkerId id) {
     BrowserThread::PostTask(
@@ -212,15 +219,8 @@ class WorkerDevToolsManager::DetachedClientHosts {
   static void RemoveInspectedWorkerDataOnIOThread(WorkerId id) {
     WorkerDevToolsManager::GetInstance()->RemoveInspectedWorkerData(id);
   }
-
-  static DetachedClientHosts* instance_;
-  typedef std::map<WorkerId, int> WorkerIdToCookieMap;
-  WorkerIdToCookieMap worker_id_to_cookie_;
 };
 
-WorkerDevToolsManager::DetachedClientHosts*
-    WorkerDevToolsManager::DetachedClientHosts::instance_ = NULL;
-
 struct WorkerDevToolsManager::InspectedWorker {
   InspectedWorker(WorkerProcessHost* host, int route_id, const GURL& url,
                   const string16& name)
@@ -367,7 +367,6 @@ void WorkerDevToolsManager::RegisterDevToolsAgentHostForWorker(
       }
     }
   }
-  NotifyWorkerDestroyedOnIOThread(worker_process_id, worker_route_id);

[yurys, 2012/12/20 11:45:40]

Now that the notification is removed who will take care of notifying
WorkerDevToolsAgentHost that the worker has gone?

[pfeldman, 2012/12/20 16:53:54]

Done.

 }
 
 void WorkerDevToolsManager::ForwardToDevToolsClient(
@@ -422,7 +421,7 @@ void WorkerDevToolsManager::ForwardToDevToolsClientOnUIThread(
   WorkerDevToolsAgentHost* agent_host = AgentHosts::GetAgentHost(WorkerId(
       worker_process_id,
       worker_route_id));
-  if (!agent_host)
+  if (!agent_host || !agent_host->attached())
     return;
   DevToolsManagerImpl::GetInstance()->DispatchOnInspectorFrontend(agent_host,
                                                                   message);
@@ -436,31 +435,9 @@ void WorkerDevToolsManager::SaveAgentRuntimeStateOnUIThread(
   WorkerDevToolsAgentHost* agent_host = AgentHosts::GetAgentHost(WorkerId(
       worker_process_id,
       worker_route_id));
-  if (!agent_host)
+  if (!agent_host || !agent_host->attached())
     return;
-  DevToolsManagerImpl::GetInstance()->SaveAgentRuntimeState(agent_host, state);
-}
-
-// static
-void WorkerDevToolsManager::NotifyWorkerDestroyedOnIOThread(
-    int worker_process_id,
-    int worker_route_id) {
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      base::Bind(
-          &WorkerDevToolsManager::NotifyWorkerDestroyedOnUIThread,
-          worker_process_id,
-          worker_route_id));
-}
-
-// static
-void WorkerDevToolsManager::NotifyWorkerDestroyedOnUIThread(
-    int worker_process_id,
-    int worker_route_id) {
-  WorkerDevToolsAgentHost* host =
-      AgentHosts::GetAgentHost(WorkerId(worker_process_id, worker_route_id));
-  if (host)
-    host->WorkerDestroyed();
+  agent_host->SaveAgentRuntimeState(state);
 }
 
 // static