DevTools: return discovered targets asynchronously.

content/browser/devtools/protocol/browser_handler.cc

Index: content/browser/devtools/protocol/browser_handler.cc
diff --git a/content/browser/devtools/protocol/browser_handler.cc b/content/browser/devtools/protocol/browser_handler.cc
index 1b53abe690596958ab34ed054d626f9eedc1ab94..d03ee5ee8e9a16350c58fe14f9b247ae2e3f28a0 100644
--- a/content/browser/devtools/protocol/browser_handler.cc
+++ b/content/browser/devtools/protocol/browser_handler.cc
@@ -4,6 +4,9 @@
 
 #include "content/browser/devtools/protocol/browser_handler.h"
 
+#include <algorithm>
+
+#include "base/bind.h"
 #include "content/browser/devtools/devtools_manager.h"
 #include "content/public/browser/devtools_manager_delegate.h"
 
@@ -13,7 +16,8 @@ namespace browser {
 
 using Response = DevToolsProtocolClient::Response;
 
-BrowserHandler::BrowserHandler() {
+BrowserHandler::BrowserHandler()
+    : weak_factory_(this) {
 }
 
 BrowserHandler::~BrowserHandler() {
@@ -63,38 +67,62 @@ Response BrowserHandler::CloseTarget(const std::string& target_id,
   return Response::OK();
 }
 
-Response BrowserHandler::GetTargets(TargetInfos* infos) {
-  DevToolsAgentHost::List agents = DevToolsAgentHost::GetOrCreateAll();
-  for (DevToolsAgentHost::List::iterator it = agents.begin();
-       it != agents.end(); ++it) {
-    DevToolsAgentHost* agent_host = (*it).get();
+Response BrowserHandler::GetTargets(DevToolsCommandId command_id) {
+  DevToolsAgentHost::DiscoverAllHosts(
+      base::Bind(&BrowserHandler::RespondToGetTargets,
+                 weak_factory_.GetWeakPtr(),
+                 command_id));
+  return Response::OK();
+}
+
+void BrowserHandler::RespondToGetTargets(
+    DevToolsCommandId command_id,
+    DevToolsAgentHost::List agents) {
+  std::vector<scoped_refptr<devtools::browser::TargetInfo>> infos;
+  for (const auto& agent_host : agents) {
     scoped_refptr<devtools::browser::TargetInfo> info =
         devtools::browser::TargetInfo::Create()->
             set_target_id(agent_host->GetId())->
             set_type(agent_host->GetType())->
             set_title(agent_host->GetTitle())->
             set_url(agent_host->GetURL().spec());
-    infos->push_back(info);
+    infos.push_back(info);
   }
+  client_->SendGetTargetsResponse(
+      command_id,
+      GetTargetsResponse::Create()->set_target_info(std::move(infos)));
+}
+
+Response BrowserHandler::Attach(DevToolsCommandId command_id,
+                                const std::string& targetId) {

[dgozman, 2016/09/15 03:25:10]

target_id

+  DevToolsAgentHost::DiscoverAllHosts(
+      base::Bind(&BrowserHandler::RespondToAttach,
+                 weak_factory_.GetWeakPtr(), command_id, targetId));
   return Response::OK();
 }
 
-Response BrowserHandler::Attach(const std::string& targetId) {
+void BrowserHandler::RespondToAttach(DevToolsCommandId command_id,
+                                     const std::string& targetId,
+                                     DevToolsAgentHost::List agents) {
   scoped_refptr<DevToolsAgentHost> agent_host =
       DevToolsAgentHost::GetForId(targetId);

[dgozman, 2016/09/15 03:25:10]

Should we look for one in the list instead? Otherwise it's unclear why we needed
to discover.

-  if (!agent_host)
-    return Response::ServerError("No target with given id found");
-  bool success = agent_host->AttachClient(this);
-  return success ? Response::OK() :
-                   Response::ServerError("Target is already being debugged");
+  bool success = false;
+  if (agent_host)
+    success = agent_host->AttachClient(this);
+  attached_hosts_.push_back(agent_host);
+  client_->SendAttachResponse(
+      command_id,
+      AttachResponse::Create()->set_success(success));
 }
 
 Response BrowserHandler::Detach(const std::string& targetId) {
   scoped_refptr<DevToolsAgentHost> agent_host =
       DevToolsAgentHost::GetForId(targetId);
-  if (!agent_host)
-    return Response::ServerError("No target with given id found");
-  bool success = agent_host->DetachClient(this);
+  auto it = std::find(
+      attached_hosts_.begin(), attached_hosts_.end(), agent_host);
+  if (it != attached_hosts_.end())
+    attached_hosts_.erase(it);
+  bool success = agent_host && agent_host->DetachClient(this);
   return success ? Response::OK() :
                    Response::ServerError("Target is not being debugged");

[dgozman, 2016/09/15 03:25:10]

Let's make detach return success as well?

 }
@@ -118,6 +146,10 @@ void BrowserHandler::DispatchProtocolMessage(
 
 void BrowserHandler::AgentHostClosed(DevToolsAgentHost* agent_host,
                                      bool replaced_with_another_client) {
+  auto it = std::find(
+      attached_hosts_.begin(), attached_hosts_.end(), agent_host);
+  if (it != attached_hosts_.end())
+    attached_hosts_.erase(it);
 }
 
 }  // namespace browser