DevTools: Merge WorkerInfo into EmbeddedWorkerDevToolsAgentHost

content/browser/devtools/embedded_worker_devtools_manager.cc

Index: content/browser/devtools/embedded_worker_devtools_manager.cc
diff --git a/content/browser/devtools/embedded_worker_devtools_manager.cc b/content/browser/devtools/embedded_worker_devtools_manager.cc
index 0d9b7e9cd34444aaa3c098531be5cd9daca1278d..45fad1756aef9977c131b55175f1d39f7ab6fff2 100644
--- a/content/browser/devtools/embedded_worker_devtools_manager.cc
+++ b/content/browser/devtools/embedded_worker_devtools_manager.cc
@@ -41,6 +41,14 @@ bool SendMessageToWorker(
   return true;
 }
 
+enum WorkerState {
+  WORKER_UNINSPECTED,
+  WORKER_INSPECTED,
+  WORKER_TERMINATED,
+  WORKER_PAUSED_FOR_DEBUG_ON_START,
+  WORKER_PAUSED_FOR_REATTACH,
+};
+
 }  // namespace
 
 EmbeddedWorkerDevToolsManager::ServiceWorkerIdentifier::ServiceWorkerIdentifier(
@@ -62,44 +70,28 @@ bool EmbeddedWorkerDevToolsManager::ServiceWorkerIdentifier::Matches(
          service_worker_version_id_ == other.service_worker_version_id_;
 }
 
-EmbeddedWorkerDevToolsManager::WorkerInfo::WorkerInfo(
-    const SharedWorkerInstance& instance)
-    : shared_worker_instance_(new SharedWorkerInstance(instance)),
-      state_(WORKER_UNINSPECTED),
-      agent_host_(NULL) {
-}
-
-EmbeddedWorkerDevToolsManager::WorkerInfo::WorkerInfo(
-    const ServiceWorkerIdentifier& service_worker_id)
-    : service_worker_id_(new ServiceWorkerIdentifier(service_worker_id)),
-      state_(WORKER_UNINSPECTED),
-      agent_host_(NULL) {
-}
-
-bool EmbeddedWorkerDevToolsManager::WorkerInfo::Matches(
-    const SharedWorkerInstance& other) {
-  if (!shared_worker_instance_)
-    return false;
-  return shared_worker_instance_->Matches(other);
-}
-
-bool EmbeddedWorkerDevToolsManager::WorkerInfo::Matches(
-    const ServiceWorkerIdentifier& other) {
-  if (!service_worker_id_)
-    return false;
-  return service_worker_id_->Matches(other);
-}
-
-EmbeddedWorkerDevToolsManager::WorkerInfo::~WorkerInfo() {
-}
-
 class EmbeddedWorkerDevToolsManager::EmbeddedWorkerDevToolsAgentHost
     : public IPCDevToolsAgentHost,
       public IPC::Listener {
  public:
-  explicit EmbeddedWorkerDevToolsAgentHost(WorkerId worker_id)
-      : worker_id_(worker_id), worker_attached_(false) {
-    AttachToWorker();
+  EmbeddedWorkerDevToolsAgentHost(
+      WorkerId worker_id,
+      const SharedWorkerInstance& shared_worker)
+      : shared_worker_(new SharedWorkerInstance(shared_worker)),
+        state_(WORKER_UNINSPECTED),
+        worker_id_(worker_id),
+        worker_attached_(false) {
+    WorkerCreated();
+  }
+
+  EmbeddedWorkerDevToolsAgentHost(
+      WorkerId worker_id,
+      const ServiceWorkerIdentifier& service_worker)
+      : service_worker_(new ServiceWorkerIdentifier(service_worker)),
+        state_(WORKER_UNINSPECTED),
+        worker_id_(worker_id),
+        worker_attached_(false) {
+    WorkerCreated();
   }
 
   // DevToolsAgentHost override.
@@ -139,7 +131,7 @@ class EmbeddedWorkerDevToolsManager::EmbeddedWorkerDevToolsAgentHost
     if (!IsAttached())
       return;
     AttachToWorker();
-    Reattach(state_);
+    Reattach(saved_agent_state_);
   }
 
   void DetachFromWorker() {
@@ -148,10 +140,28 @@ class EmbeddedWorkerDevToolsManager::EmbeddedWorkerDevToolsAgentHost
     worker_attached_ = false;
     if (RenderProcessHost* host = RenderProcessHost::FromID(worker_id_.first))
       host->RemoveRoute(worker_id_.second);
+  }
+
+  void WorkerCreated() {
+    AddRef();
+  }
+
+  void WorkerDestroyed() {
     Release();
   }
 
   WorkerId worker_id() const { return worker_id_; }
+  WorkerState state() const { return state_; }
+  void set_worker_id(WorkerId worker_id) { worker_id_ = worker_id; }
+  void set_state(WorkerState state) { state_ = state; };
+
+  bool Matches(const SharedWorkerInstance& other) {
+    return shared_worker_ && shared_worker_->Matches(other);
+  }
+
+  bool Matches(const ServiceWorkerIdentifier& other) {
+    return service_worker_ && service_worker_->Matches(other);
+  }
 
  private:
   virtual ~EmbeddedWorkerDevToolsAgentHost() {
@@ -165,20 +175,25 @@ class EmbeddedWorkerDevToolsManager::EmbeddedWorkerDevToolsAgentHost
                                                                     message);
   }
 
-  void OnSaveAgentRuntimeState(const std::string& state) { state_ = state; }
+  void OnSaveAgentRuntimeState(const std::string& state) {
+    saved_agent_state_ = state;
+  }
 
   void AttachToWorker() {
     if (worker_attached_)
       return;
     worker_attached_ = true;
-    AddRef();
+    set_state(WORKER_INSPECTED);
     if (RenderProcessHost* host = RenderProcessHost::FromID(worker_id_.first))
       host->AddRoute(worker_id_.second, this);
   }
 
+  scoped_ptr<SharedWorkerInstance> shared_worker_;
+  scoped_ptr<ServiceWorkerIdentifier> service_worker_;
+  WorkerState state_;
   WorkerId worker_id_;
   bool worker_attached_;
-  std::string state_;
+  std::string saved_agent_state_;
   DISALLOW_COPY_AND_ASSIGN(EmbeddedWorkerDevToolsAgentHost);
 };
 
@@ -191,29 +206,16 @@ EmbeddedWorkerDevToolsManager* EmbeddedWorkerDevToolsManager::GetInstance() {
 DevToolsAgentHost* EmbeddedWorkerDevToolsManager::GetDevToolsAgentHostForWorker(
     int worker_process_id,
     int worker_route_id) {
-  WorkerId id(worker_process_id, worker_route_id);
-
-  WorkerInfoMap::iterator it = workers_.find(id);
-  if (it == workers_.end())
-    return NULL;
-
-  WorkerInfo* info = it->second;
-  if (info->state() != WORKER_UNINSPECTED &&
-      info->state() != WORKER_PAUSED_FOR_DEBUG_ON_START) {
-    return info->agent_host();
-  }
-
-  EmbeddedWorkerDevToolsAgentHost* agent_host =
-      new EmbeddedWorkerDevToolsAgentHost(id);
-  info->set_agent_host(agent_host);
-  info->set_state(WORKER_INSPECTED);
-  return agent_host;
+  AgentHostMap::iterator it = workers_.find(
+      WorkerId(worker_process_id, worker_route_id));
+  return it == workers_.end() ? NULL : it->second;
 }
 
 DevToolsAgentHost*
 EmbeddedWorkerDevToolsManager::GetDevToolsAgentHostForServiceWorker(
     const ServiceWorkerIdentifier& service_worker_id) {
-  WorkerInfoMap::iterator it = FindExistingServiceWorkerInfo(service_worker_id);
+  AgentHostMap::iterator it =
+      FindExistingServiceWorkerAgentHost(service_worker_id);
   if (it == workers_.end())
     return NULL;
   return GetDevToolsAgentHostForWorker(it->first.first, it->first.second);
@@ -232,13 +234,14 @@ bool EmbeddedWorkerDevToolsManager::SharedWorkerCreated(
     const SharedWorkerInstance& instance) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   const WorkerId id(worker_process_id, worker_route_id);
-  WorkerInfoMap::iterator it = FindExistingSharedWorkerInfo(instance);
+  AgentHostMap::iterator it = FindExistingSharedWorkerAgentHost(instance);
   if (it == workers_.end()) {
-    scoped_ptr<WorkerInfo> info(new WorkerInfo(instance));
-    workers_.set(id, info.Pass());
+    EmbeddedWorkerDevToolsAgentHost* agent_host =

[yurys, 2014/08/06 11:26:19]

inline agent_host as it is used only once

[vkuzkokov, 2014/08/06 11:43:04]

Done.

+        new EmbeddedWorkerDevToolsAgentHost(id, instance);
+    workers_[id] = agent_host;
     return false;
   }
-  MoveToPausedState(id, it);
+  WorkerRestarted(id, it);
   return true;
 }
 
@@ -248,15 +251,17 @@ bool EmbeddedWorkerDevToolsManager::ServiceWorkerCreated(
     const ServiceWorkerIdentifier& service_worker_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   const WorkerId id(worker_process_id, worker_route_id);
-  WorkerInfoMap::iterator it = FindExistingServiceWorkerInfo(service_worker_id);
+  AgentHostMap::iterator it =
+      FindExistingServiceWorkerAgentHost(service_worker_id);
   if (it == workers_.end()) {
-    scoped_ptr<WorkerInfo> info(new WorkerInfo(service_worker_id));
+    EmbeddedWorkerDevToolsAgentHost* agent_host =
+        new EmbeddedWorkerDevToolsAgentHost(id, service_worker_id);
     if (debug_service_worker_on_start_)
-      info->set_state(WORKER_PAUSED_FOR_DEBUG_ON_START);
-    workers_.set(id, info.Pass());
+      agent_host->set_state(WORKER_PAUSED_FOR_DEBUG_ON_START);
+    workers_[id] = agent_host;
     return debug_service_worker_on_start_;
   }
-  MoveToPausedState(id, it);
+  WorkerRestarted(id, it);
   return true;
 }
 
@@ -264,28 +269,24 @@ void EmbeddedWorkerDevToolsManager::WorkerDestroyed(int worker_process_id,
                                                     int worker_route_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   const WorkerId id(worker_process_id, worker_route_id);
-  WorkerInfoMap::iterator it = workers_.find(id);
+  AgentHostMap::iterator it = workers_.find(id);
   DCHECK(it != workers_.end());
-  WorkerInfo* info = it->second;
-  switch (info->state()) {
+  EmbeddedWorkerDevToolsAgentHost* agent_host = it->second;
+  switch (agent_host->state()) {
     case WORKER_UNINSPECTED:
     case WORKER_PAUSED_FOR_DEBUG_ON_START:
-      workers_.erase(it);
+      agent_host->DetachFromWorker();

[dgozman, 2014/08/06 10:11:36]

Calling DetachFromWorker here is strange. Agent host should not be attached.

[yurys, 2014/08/06 11:26:19]

I agree with Dmitry. Looks like we need a DCHECK here that no one is attached to
the EWDTAH

[vkuzkokov, 2014/08/06 11:43:04]

It is strange. Kept as DCHECK.

       break;
     case WORKER_INSPECTED: {
-      EmbeddedWorkerDevToolsAgentHost* agent_host = info->agent_host();
-      info->set_state(WORKER_TERMINATED);
-      if (!agent_host->IsAttached()) {
-        agent_host->DetachFromWorker();
-        return;
+      if (agent_host->IsAttached()) {

[dgozman, 2014/08/06 10:11:36]

How can agent host not be attached in this case?

[vkuzkokov, 2014/08/06 11:43:04]

It most likely can't. Fixed.

+        // Client host is debugging this worker agent host.
+        std::string notification =
+            DevToolsProtocol::CreateNotification(
+                devtools::Worker::disconnectedFromWorker::kName, NULL)
+                ->Serialize();
+        DevToolsManagerImpl::GetInstance()->DispatchOnInspectorFrontend(
+            agent_host, notification);
       }
-      // Client host is debugging this worker agent host.
-      std::string notification =
-          DevToolsProtocol::CreateNotification(
-              devtools::Worker::disconnectedFromWorker::kName, NULL)
-              ->Serialize();
-      DevToolsManagerImpl::GetInstance()->DispatchOnInspectorFrontend(
-          agent_host, notification);
       agent_host->DetachFromWorker();
       break;
     }
@@ -293,67 +294,41 @@ void EmbeddedWorkerDevToolsManager::WorkerDestroyed(int worker_process_id,
       NOTREACHED();
       break;
     case WORKER_PAUSED_FOR_REATTACH: {
-      scoped_ptr<WorkerInfo> worker_info = workers_.take_and_erase(it);
-      worker_info->set_state(WORKER_TERMINATED);
-      const WorkerId old_id = worker_info->agent_host()->worker_id();
-      workers_.set(old_id, worker_info.Pass());
+      workers_.erase(it);
+      workers_[agent_host->worker_id()] = agent_host;

[yurys, 2014/08/06 11:26:19]

Isn't it->first == agent_host->worker_id() ?

[vkuzkokov, 2014/08/06 11:43:04]

It didn't use to be. Now it is. Fixed.

       break;
     }
   }
+  agent_host->set_state(WORKER_TERMINATED);
+  agent_host->WorkerDestroyed();
 }
 
 void EmbeddedWorkerDevToolsManager::WorkerContextStarted(int worker_process_id,
                                                          int worker_route_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   const WorkerId id(worker_process_id, worker_route_id);
-  WorkerInfoMap::iterator it = workers_.find(id);
+  AgentHostMap::iterator it = workers_.find(id);
   DCHECK(it != workers_.end());
-  WorkerInfo* info = it->second;
-  if (info->state() == WORKER_PAUSED_FOR_DEBUG_ON_START) {
+  EmbeddedWorkerDevToolsAgentHost* agent_host = it->second;
+  if (agent_host->state() == WORKER_PAUSED_FOR_DEBUG_ON_START) {
     RenderProcessHost* rph = RenderProcessHost::FromID(worker_process_id);
-    scoped_refptr<DevToolsAgentHost> agent_host(
-        GetDevToolsAgentHostForWorker(worker_process_id, worker_route_id));
-    DevToolsManagerImpl::GetInstance()->Inspect(rph->GetBrowserContext(),
-                                                agent_host.get());
-  } else if (info->state() == WORKER_PAUSED_FOR_REATTACH) {
-    info->agent_host()->ReattachToWorker(id);
-    info->set_state(WORKER_INSPECTED);
+    DevToolsManagerImpl::GetInstance()->Inspect(
+        rph->GetBrowserContext(), agent_host);
+  } else if (agent_host->state() == WORKER_PAUSED_FOR_REATTACH) {
+    agent_host->ReattachToWorker(id);
   }
 }
 
 void EmbeddedWorkerDevToolsManager::RemoveInspectedWorkerData(
     EmbeddedWorkerDevToolsAgentHost* agent_host) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  const WorkerId id(agent_host->worker_id());
-  scoped_ptr<WorkerInfo> worker_info = workers_.take_and_erase(id);
-  if (worker_info) {
-    DCHECK_EQ(worker_info->agent_host(), agent_host);
-    if (worker_info->state() == WORKER_TERMINATED)
-      return;
-    DCHECK_EQ(worker_info->state(), WORKER_INSPECTED);
-    worker_info->set_agent_host(NULL);
-    worker_info->set_state(WORKER_UNINSPECTED);
-    workers_.set(id, worker_info.Pass());
-    return;
-  }
-  for (WorkerInfoMap::iterator it = workers_.begin(); it != workers_.end();
-       ++it) {
-    if (it->second->agent_host() == agent_host) {
-      DCHECK_EQ(WORKER_PAUSED_FOR_REATTACH, it->second->state());
-      SendMessageToWorker(
-          it->first,
-          new DevToolsAgentMsg_ResumeWorkerContext(it->first.second));
-      it->second->set_agent_host(NULL);
-      it->second->set_state(WORKER_UNINSPECTED);
-      return;
-    }
-  }
+  workers_.erase(agent_host->worker_id());
 }
 
-EmbeddedWorkerDevToolsManager::WorkerInfoMap::iterator
-EmbeddedWorkerDevToolsManager::FindExistingSharedWorkerInfo(
+EmbeddedWorkerDevToolsManager::AgentHostMap::iterator
+EmbeddedWorkerDevToolsManager::FindExistingSharedWorkerAgentHost(
     const SharedWorkerInstance& instance) {
-  WorkerInfoMap::iterator it = workers_.begin();
+  AgentHostMap::iterator it = workers_.begin();
   for (; it != workers_.end(); ++it) {
     if (it->second->Matches(instance))
       break;
@@ -361,10 +336,10 @@ EmbeddedWorkerDevToolsManager::FindExistingSharedWorkerInfo(
   return it;
 }
 
-EmbeddedWorkerDevToolsManager::WorkerInfoMap::iterator
-EmbeddedWorkerDevToolsManager::FindExistingServiceWorkerInfo(
+EmbeddedWorkerDevToolsManager::AgentHostMap::iterator
+EmbeddedWorkerDevToolsManager::FindExistingServiceWorkerAgentHost(
     const ServiceWorkerIdentifier& service_worker_id) {
-  WorkerInfoMap::iterator it = workers_.begin();
+  AgentHostMap::iterator it = workers_.begin();
   for (; it != workers_.end(); ++it) {
     if (it->second->Matches(service_worker_id))
       break;
@@ -372,13 +347,16 @@ EmbeddedWorkerDevToolsManager::FindExistingServiceWorkerInfo(
   return it;
 }
 
-void EmbeddedWorkerDevToolsManager::MoveToPausedState(
+void EmbeddedWorkerDevToolsManager::WorkerRestarted(
     const WorkerId& id,
-    const WorkerInfoMap::iterator& it) {
+    const AgentHostMap::iterator& it) {
   DCHECK_EQ(WORKER_TERMINATED, it->second->state());
-  scoped_ptr<WorkerInfo> info = workers_.take_and_erase(it);
-  info->set_state(WORKER_PAUSED_FOR_REATTACH);
-  workers_.set(id, info.Pass());
+  EmbeddedWorkerDevToolsAgentHost* agent_host = it->second;
+  agent_host->set_state(WORKER_PAUSED_FOR_REATTACH);
+  agent_host->set_worker_id(id);
+  agent_host->WorkerCreated();
+  workers_.erase(it);
+  workers_[id] = agent_host;
 }
 
 void EmbeddedWorkerDevToolsManager::ResetForTesting() {