[DevTools] Support multiple sessions in Target domain

content/browser/devtools/protocol/target_handler.cc

Index: content/browser/devtools/protocol/target_handler.cc
diff --git a/content/browser/devtools/protocol/target_handler.cc b/content/browser/devtools/protocol/target_handler.cc
index dd1b8b8b67493134a9e17ee0a817c202e9bfc311..fe9b7d56205bbb64412ebf465a401658e3ec44a3 100644
--- a/content/browser/devtools/protocol/target_handler.cc
+++ b/content/browser/devtools/protocol/target_handler.cc
@@ -4,6 +4,7 @@
 
 #include "content/browser/devtools/protocol/target_handler.h"
 
+#include "base/strings/stringprintf.h"
 #include "content/browser/devtools/devtools_manager.h"
 #include "content/browser/devtools/devtools_session.h"
 #include "content/browser/devtools/render_frame_devtools_agent_host.h"
@@ -11,6 +12,7 @@
 #include "content/browser/frame_host/frame_tree.h"
 #include "content/browser/frame_host/frame_tree_node.h"
 #include "content/browser/frame_host/render_frame_host_impl.h"
+#include "content/public/browser/devtools_agent_host_client.h"
 
 namespace content {
 namespace protocol {
@@ -94,6 +96,54 @@ std::unique_ptr<Target::TargetInfo> CreateInfo(DevToolsAgentHost* host) {
 
 }  // namespace
 
+class TargetHandler::Session : public DevToolsAgentHostClient {
+ public:
+  Session(TargetHandler* handler,
+          DevToolsAgentHost* agent_host,

[caseq, 2017/06/27 18:29:29]

pass as scoped_refptr<> to indicate we keep it.

+          const std::string& id,
+          bool is_auto_attach)
+      : handler_(handler),
+        agent_host_(agent_host),
+        id_(id),
+        is_auto_attach_(is_auto_attach) {}
+
+  ~Session() override {
+    if (agent_host_)
+      agent_host_->DetachClient(this);
+  }
+
+  bool is_auto_attach() const { return is_auto_attach_; }
+  DevToolsAgentHost* agent_host() const { return agent_host_.get(); }
+
+  void Attach() {
+    static_cast<DevToolsAgentHostImpl*>(agent_host_.get())
+        ->AttachMultiClient(this);
+  }
+
+  // DevToolsAgentHostClient implementation.
+  void DispatchProtocolMessage(DevToolsAgentHost* agent_host,
+                               const std::string& message) override {
+    handler_->frontend_->ReceivedMessageFromTarget(id_, message);
+  }
+
+  void AgentHostClosed(DevToolsAgentHost* agent_host,
+                       bool replaced_with_another_client) override {
+    handler_->frontend_->DetachedFromTarget(id_);

[caseq, 2017/06/27 18:29:29]

Most of the logic in this methods looks like it could be in TargetHandler. Can
you extract a method there?

+    if (is_auto_attach_)
+      handler_->auto_attached_hosts_.erase(agent_host->GetId());
+    agent_host_ = nullptr;
+    handler_->attached_sessions_.erase(id_);  // |this| is deleted here.
+  }
+
+ private:
+  TargetHandler* handler_;
+  scoped_refptr<DevToolsAgentHost> agent_host_;
+  std::string id_;
+  bool is_auto_attach_;
+
+  DISALLOW_COPY_AND_ASSIGN(Session);
+};
+
 TargetHandler::TargetHandler()
     : DevToolsDomainHandler(Target::Metainfo::domainName),
       discover_(false),
@@ -126,9 +176,7 @@ void TargetHandler::SetRenderFrameHost(RenderFrameHostImpl* render_frame_host) {
 Response TargetHandler::Disable() {
   SetAutoAttach(false, false);
   SetDiscoverTargets(false);
-  for (const auto& id_host : attached_hosts_)
-    id_host.second->DetachClient(this);
-  attached_hosts_.clear();
+  attached_sessions_.clear();
   return Response::OK();
 }
 
@@ -189,20 +237,26 @@ void TargetHandler::UpdateServiceWorkers(bool waiting_for_debugger) {
       new_hosts, DevToolsAgentHost::kTypeServiceWorker, waiting_for_debugger);
 }
 
-void TargetHandler::ReattachTargetsOfType(
-    const HostsMap& new_hosts,
-    const std::string& type,
-    bool waiting_for_debugger) {
-  HostsMap old_hosts = attached_hosts_;
-  for (const auto& pair : old_hosts) {
-    if (pair.second->GetType() == type &&
-        new_hosts.find(pair.first) == new_hosts.end()) {
-      DetachFromTargetInternal(pair.second.get());
+void TargetHandler::ReattachTargetsOfType(const HostsMap& new_hosts,
+                                          const std::string& type,
+                                          bool waiting_for_debugger) {
+  std::vector<std::string> ids_to_detach;
+  for (const auto& pair : attached_sessions_) {
+    Session* session = pair.second.get();
+    if (session->is_auto_attach() && session->agent_host()->GetType() == type &&
+        new_hosts.find(session->agent_host()->GetId()) == new_hosts.end()) {
+      ids_to_detach.push_back(pair.first);
+      auto_attached_hosts_.erase(session->agent_host()->GetId());
     }
   }
+  for (const std::string& session_id : ids_to_detach)
+    DetachFromSessionInternal(session_id);
   for (const auto& pair : new_hosts) {
-    if (old_hosts.find(pair.first) == old_hosts.end())
-      AttachToTargetInternal(pair.second.get(), waiting_for_debugger);
+    if (auto_attached_hosts_.find(pair.first) == auto_attached_hosts_.end()) {
+      DevToolsAgentHost* host = pair.second.get();
+      AttachToTargetInternal(host, waiting_for_debugger, true);
+      auto_attached_hosts_[host->GetId()] = host;
+    }
   }
 }
 
@@ -223,24 +277,34 @@ void TargetHandler::TargetDestroyedInternal(
   reported_hosts_.erase(it);
 }
 
-bool TargetHandler::AttachToTargetInternal(
-    DevToolsAgentHost* host, bool waiting_for_debugger) {
-  attached_hosts_[host->GetId()] = host;
-  if (!host->AttachClient(this)) {
-    attached_hosts_.erase(host->GetId());
+std::string TargetHandler::AttachToTargetInternal(DevToolsAgentHost* host,
+                                                  bool waiting_for_debugger,
+                                                  bool is_auto_attach) {
+  std::string session_id =
+      base::StringPrintf("%s:%d", host->GetId().c_str(), ++last_session_id_);
+  Session* session = new Session(this, host, session_id, is_auto_attach);
+  attached_sessions_[session_id].reset(session);
+  session->Attach();
+  frontend_->AttachedToTarget(session_id, CreateInfo(host),
+                              waiting_for_debugger);
+  return session_id;
+}
+
+bool TargetHandler::DetachFromSessionInternal(const std::string& session_id) {
+  auto it = attached_sessions_.find(session_id);
+  if (it == attached_sessions_.end())
     return false;
-  }
-  frontend_->AttachedToTarget(CreateInfo(host), waiting_for_debugger);
+  frontend_->DetachedFromTarget(session_id);
+  attached_sessions_.erase(it);
   return true;
 }
 
-void TargetHandler::DetachFromTargetInternal(DevToolsAgentHost* host) {
-  auto it = attached_hosts_.find(host->GetId());
-  if (it == attached_hosts_.end())
-    return;
-  host->DetachClient(this);
-  frontend_->DetachedFromTarget(host->GetId());
-  attached_hosts_.erase(it);
+bool TargetHandler::IsAttachedToAgentHost(DevToolsAgentHost* agent_host) {
+  for (auto& it : attached_sessions_) {
+    if (it.second->agent_host() == agent_host)
+      return true;
+  }
+  return false;
 }
 
 // ----------------- Protocol ----------------------
@@ -298,32 +362,28 @@ Response TargetHandler::SetRemoteLocations(
 }
 
 Response TargetHandler::AttachToTarget(const std::string& target_id,
-                                       bool* out_success) {
+                                       Maybe<std::string>* out_session_id) {
   // TODO(dgozman): only allow reported hosts.
   scoped_refptr<DevToolsAgentHost> agent_host =
       DevToolsAgentHost::GetForId(target_id);
   if (!agent_host)
     return Response::InvalidParams("No target with given id found");
-  *out_success = AttachToTargetInternal(agent_host.get(), false);
+  *out_session_id = AttachToTargetInternal(agent_host.get(), false, false);
   return Response::OK();
 }
 
-Response TargetHandler::DetachFromTarget(const std::string& target_id) {
-  auto it = attached_hosts_.find(target_id);
-  if (it == attached_hosts_.end())
-    return Response::Error("Not attached to the target");
-  DevToolsAgentHost* agent_host = it->second.get();
-  DetachFromTargetInternal(agent_host);
+Response TargetHandler::DetachFromTarget(const std::string& session_id) {
+  if (!DetachFromSessionInternal(session_id))
+    return Response::Error("No session with given id");
   return Response::OK();
 }
 
-Response TargetHandler::SendMessageToTarget(
-    const std::string& target_id,
-    const std::string& message) {
-  auto it = attached_hosts_.find(target_id);
-  if (it == attached_hosts_.end())
+Response TargetHandler::SendMessageToTarget(const std::string& session_id,
+                                            const std::string& message) {
+  auto it = attached_sessions_.find(session_id);
+  if (it == attached_sessions_.end())
     return Response::FallThrough();
-  it->second->DispatchProtocolMessage(this, message);
+  it->second->agent_host()->DispatchProtocolMessage(it->second.get(), message);
   return Response::OK();
 }
 
@@ -393,25 +453,6 @@ Response TargetHandler::GetTargets(
   return Response::OK();
 }
 
-// ---------------- DevToolsAgentHostClient ----------------
-
-void TargetHandler::DispatchProtocolMessage(
-    DevToolsAgentHost* host,
-    const std::string& message) {
-  auto it = attached_hosts_.find(host->GetId());
-  if (it == attached_hosts_.end())
-    return;  // Already disconnected.
-
-  frontend_->ReceivedMessageFromTarget(host->GetId(), message);
-}
-
-void TargetHandler::AgentHostClosed(
-    DevToolsAgentHost* host,
-    bool replaced_with_another_client) {
-  frontend_->DetachedFromTarget(host->GetId());
-  attached_hosts_.erase(host->GetId());
-}
-
 // -------------- DevToolsAgentHostObserver -----------------
 
 bool TargetHandler::ShouldForceDevToolsAgentHostCreation() {
@@ -427,14 +468,15 @@ void TargetHandler::DevToolsAgentHostCreated(DevToolsAgentHost* agent_host) {
 }
 
 void TargetHandler::DevToolsAgentHostDestroyed(DevToolsAgentHost* agent_host) {
-  DCHECK(attached_hosts_.find(agent_host->GetId()) == attached_hosts_.end());
+  DCHECK(auto_attached_hosts_.find(agent_host->GetId()) ==
+         auto_attached_hosts_.end());
   TargetDestroyedInternal(agent_host);
 }
 
 void TargetHandler::DevToolsAgentHostAttached(DevToolsAgentHost* host) {
   if (host->GetType() == "node" &&
       reported_hosts_.find(host->GetId()) != reported_hosts_.end() &&
-      attached_hosts_.find(host->GetId()) == attached_hosts_.end()) {
+      !IsAttachedToAgentHost(host)) {
     TargetDestroyedInternal(host);
   }
 }
@@ -442,7 +484,7 @@ void TargetHandler::DevToolsAgentHostAttached(DevToolsAgentHost* host) {
 void TargetHandler::DevToolsAgentHostDetached(DevToolsAgentHost* host) {
   if (host->GetType() == "node" &&
       reported_hosts_.find(host->GetId()) == reported_hosts_.end() &&
-      attached_hosts_.find(host->GetId()) == attached_hosts_.end()) {
+      !IsAttachedToAgentHost(host)) {
     TargetCreatedInternal(host);
   }
 }