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

content/browser/devtools/worker_devtools_manager.cc

 // Copyright (c) 2011 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 "content/browser/devtools/worker_devtools_manager.h"
 
 #include <list>
 #include <map>
 
 #include "base/bind.h"
@@ skipping 24 matching lines @@
 
 class WorkerDevToolsManager::AgentHosts {
 public:
   static void Add(WorkerId id, WorkerDevToolsAgentHost* host) {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
     if (!instance_)
       instance_ = new AgentHosts();
     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);
     if (map.empty()) {
       delete instance_;
       instance_ = NULL;
     }
   }
 
   static WorkerDevToolsAgentHost* GetAgentHost(WorkerId id) {
@@ skipping 28 matching lines @@
         worker_name(name) {}
   WorkerId old_worker_id;
   GURL worker_url;
   string16 worker_name;
 };
 
 
 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,
         base::Bind(
             &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_);
   }
 
   static void RegisterAgent(
       int worker_process_id,
       int worker_route_id) {
     WorkerDevToolsManager::GetInstance()->RegisterDevToolsAgentHostForWorker(
         worker_process_id, worker_route_id);
   }
 
   static void ForwardToWorkerDevToolsAgent(
       int worker_process_id,
       int worker_route_id,
       IPC::Message* message) {
     WorkerDevToolsManager::GetInstance()->ForwardToWorkerDevToolsAgent(
         worker_process_id, worker_route_id, *message);
   }
 
   // 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(
             &WorkerDevToolsAgentHost::ForwardToWorkerDevToolsAgent,
             worker_id_.first,
             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);
 };
 
 
 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);
   }
 
   static void WorkerDestroyed(WorkerId id) {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
     WorkerDevToolsAgentHost* agent = AgentHosts::GetAgentHost(id);
     if (!agent) {
       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(
         BrowserThread::IO, FROM_HERE,
         base::Bind(&RemoveInspectedWorkerDataOnIOThread, id));
   }
 
   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)
       : host(host),
         route_id(route_id),
         worker_url(url),
         worker_name(name) {}
   WorkerProcessHost* const host;
   int const route_id;
   GURL worker_url;
@@ skipping 126 matching lines @@
          i != instances.end(); ++i) {
       if (i->worker_route_id() == worker_route_id) {
         DCHECK(FindInspectedWorker(worker_process_id, worker_route_id) ==
                inspected_workers_.end());
         inspected_workers_.push_back(
             InspectedWorker(process, worker_route_id, i->url(), i->name()));
         return;
       }
     }
   }
-  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(
     int worker_process_id,
     int worker_route_id,
     const std::string& message) {
   if (FindInspectedWorker(worker_process_id, worker_route_id) ==
           inspected_workers_.end()) {
       NOTREACHED();
       return;
@@ skipping 34 matching lines @@
 }
 
 // static
 void WorkerDevToolsManager::ForwardToDevToolsClientOnUIThread(
     int worker_process_id,
     int worker_route_id,
     const std::string& message) {
   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);
 }
 
 // static
 void WorkerDevToolsManager::SaveAgentRuntimeStateOnUIThread(
     int worker_process_id,
     int worker_route_id,
     const std::string& state) {
   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);
+  agent_host->SaveAgentRuntimeState(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();
-}
-
-// static
 void WorkerDevToolsManager::SendResumeToWorker(const WorkerId& id) {
   if (WorkerProcessHost* process = FindWorkerProcess(id.first))
     process->Send(new DevToolsAgentMsg_ResumeWorkerContext(id.second));
 }
 
 }  // namespace content