Extract debugger target enumeration into a separate class

content/browser/devtools/devtools_http_handler_impl.cc

 // Copyright (c) 2012 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/devtools_http_handler_impl.h"
 
 #include <algorithm>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
 #include "base/file_util.h"
 #include "base/json/json_writer.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/message_loop_proxy.h"
 #include "base/string_number_conversions.h"
-#include "base/stringprintf.h"
 #include "base/threading/thread.h"
-#include "base/utf_string_conversions.h"
-#include "base/values.h"
-#include "content/browser/devtools/devtools_agent_host_impl.h"
 #include "content/browser/devtools/devtools_browser_target.h"
 #include "content/browser/devtools/devtools_tracing_handler.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/common/devtools_messages.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/devtools_agent_host.h"
 #include "content/public/browser/devtools_client_host.h"
-#include "content/public/browser/devtools_http_handler_delegate.h"
 #include "content/public/browser/devtools_manager.h"
-#include "content/public/browser/favicon_status.h"
-#include "content/public/browser/navigation_entry.h"
+#include "content/public/browser/devtools_target_list.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
-#include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
-#include "content/public/browser/render_widget_host.h"
 #include "content/public/common/content_client.h"
 #include "content/public/common/url_constants.h"
 #include "googleurl/src/gurl.h"
 #include "grit/devtools_resources_map.h"
-#include "net/base/escape.h"
 #include "net/base/io_buffer.h"
 #include "net/base/ip_endpoint.h"
 #include "net/server/http_server_request_info.h"
 #include "third_party/WebKit/Source/Platform/chromium/public/WebString.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebDevToolsAgent.h"
 #include "ui/base/layout.h"
 #include "webkit/user_agent/user_agent.h"
 #include "webkit/user_agent/user_agent_util.h"
 
 namespace content {
 
 const int kBufferSize = 16 * 1024;
 
 namespace {
 
 static const char* kDevToolsHandlerThreadName = "Chrome_DevToolsHandlerThread";
 
-class DevToolsDefaultBindingHandler
-    : public DevToolsHttpHandler::DevToolsAgentHostBinding {
- public:
-  DevToolsDefaultBindingHandler() {
-  }
-
-  void GarbageCollect() {
-    AgentsMap::iterator it = agents_map_.begin();
-    while (it != agents_map_.end()) {
-      if (!it->second->GetRenderViewHost())
-        agents_map_.erase(it++);
-      else
-        ++it;
-    }
-  }
-
-  virtual std::string GetIdentifier(DevToolsAgentHost* agent_host) OVERRIDE {
-    GarbageCollect();
-    DevToolsAgentHostImpl* agent_host_impl =
-        static_cast<DevToolsAgentHostImpl*>(agent_host);
-    std::string id = base::StringPrintf("%d", agent_host_impl->id());
-    agents_map_[id] = agent_host;
-    return id;
-  }
-
-  virtual DevToolsAgentHost* ForIdentifier(
-      const std::string& identifier) OVERRIDE {
-    GarbageCollect();
-    AgentsMap::iterator it = agents_map_.find(identifier);
-    if (it != agents_map_.end())
-      return it->second;
-    return NULL;
-  }
-
- private:
-  typedef std::map<std::string, scoped_refptr<DevToolsAgentHost> > AgentsMap;
-  AgentsMap agents_map_;
-};
-
 // An internal implementation of DevToolsClientHost that delegates
 // messages sent for DevToolsClient to a DebuggerShell instance.
 class DevToolsClientHostImpl : public DevToolsClientHost {
  public:
   DevToolsClientHostImpl(
       MessageLoop* message_loop,
       net::HttpServer* server,
       int connection_id)
       : message_loop_(message_loop),
         server_(server),
@@ skipping 118 matching lines @@
       BrowserThread::FILE, FROM_HERE,
       base::Bind(&DevToolsHttpHandlerImpl::StopHandlerThread, this),
       base::Bind(&DevToolsHttpHandlerImpl::ResetHandlerThreadAndRelease, this));
 }
 
 void DevToolsHttpHandlerImpl::SetDevToolsAgentHostBinding(
     DevToolsAgentHostBinding* binding) {
   if (binding)
     binding_ = binding;
   else
-    binding_ = default_binding_.get();
+    binding_ = DevToolsTargetList::GetInstance();

[pfeldman, 2013/03/01 08:49:30]

It is especially clear here how two concepts are mixed.

[Vladislav Kaznacheev, 2013/03/01 12:58:35]

Rolled back.
On 2013/03/01 08:49:30, pfeldman wrote:

 }
 
 GURL DevToolsHttpHandlerImpl::GetFrontendURL(DevToolsAgentHost* agent_host) {
   net::IPEndPoint ip_address;
   if (server_->GetLocalAddress(&ip_address))
     return GURL();
   if (!agent_host) {
     return GURL(std::string("http://") + ip_address.ToString() +
         overridden_frontend_url_);
   }
@@ skipping 138 matching lines @@
 void DevToolsHttpHandlerImpl::OnClose(int connection_id) {
   BrowserThread::PostTask(
       BrowserThread::UI,
       FROM_HERE,
       base::Bind(
           &DevToolsHttpHandlerImpl::OnCloseUI,
           this,
           connection_id));
 }
 
-struct DevToolsHttpHandlerImpl::PageInfo {
-  PageInfo()
-      : attached(false) {
-  }
-
-  std::string id;
-  std::string url;
-  std::string type;
-  bool attached;
-  std::string title;
-  std::string thumbnail_url;
-  std::string favicon_url;
-  base::TimeTicks last_selected_time;
-};
+static base::TimeTicks GetLastSelectedTime(DevToolsAgentHost* agent) {
+  WebContents* web_contents =

[pfeldman, 2013/03/01 08:49:30]

Once you start operating TargetList, some of your DTAHs will be non-RVH-based.
So you need to further abstract from it and move GetLastAccessTime into the
DTAH.

[Vladislav Kaznacheev, 2013/03/01 12:58:35]

This will not solve the problem from your next comment though.

On 2013/03/01 08:49:30, pfeldman wrote:

+      agent->GetRenderViewHost()->GetDelegate()->GetAsWebContents();
+  return web_contents ? web_contents->GetLastSelectedTime() : base::TimeTicks();
+}
 
 // static
-bool DevToolsHttpHandlerImpl::SortPageListByTime(const PageInfo& info1,
-                                                 const PageInfo& info2) {
-  return info1.last_selected_time > info2.last_selected_time;
+bool DevToolsHttpHandlerImpl::SortPageListByTime(DevToolsAgentHost* agent1,
+                                                 DevToolsAgentHost* agent2) {
+  return GetLastSelectedTime(agent1) > GetLastSelectedTime(agent2);

[pfeldman, 2013/03/01 08:49:30]

This is scary. GetLastSelectedTime will return different values for the same
target as you sort!

[Vladislav Kaznacheev, 2013/03/01 12:58:35]

I am scared too.
On 2013/03/01 08:49:30, pfeldman wrote:

 }
 
 DevToolsHttpHandlerImpl::PageList DevToolsHttpHandlerImpl::GeneratePageList() {
   PageList page_list;
-  for (RenderProcessHost::iterator it(RenderProcessHost::AllHostsIterator());
-       !it.IsAtEnd(); it.Advance()) {
-    RenderProcessHost* render_process_host = it.GetCurrentValue();
-    DCHECK(render_process_host);
-
-    // Ignore processes that don't have a connection, such as crashed contents.
-    if (!render_process_host->HasConnection())
-      continue;
-
-    RenderProcessHost::RenderWidgetHostsIterator rwit(
-        render_process_host->GetRenderWidgetHostsIterator());
-    for (; !rwit.IsAtEnd(); rwit.Advance()) {
-      const RenderWidgetHost* widget = rwit.GetCurrentValue();
-      DCHECK(widget);
-      if (!widget || !widget->IsRenderView())
-        continue;
-
-      RenderViewHost* host =
-          RenderViewHost::From(const_cast<RenderWidgetHost*>(widget));
-      page_list.push_back(CreatePageInfo(host, delegate_->GetTargetType(host)));
-    }
-  }
+  DevToolsTargetList::AgentsMap& agents_map =
+      DevToolsTargetList::GetInstance()->GetAgentsMap();
+  for (DevToolsTargetList::iterator i = agents_map.begin();
+      i != agents_map.end(); ++i)
+    page_list.push_back(i->second);
   std::sort(page_list.begin(), page_list.end(), SortPageListByTime);
   return page_list;
 }
 
 std::string DevToolsHttpHandlerImpl::GetFrontendURLInternal(
     const std::string rvh_id,
     const std::string& host) {
   return base::StringPrintf(
       "%s%sws=%s/devtools/page/%s",
       overridden_frontend_url_.c_str(),
@@ skipping 86 matching lines @@
   if (command == "new") {
     RenderViewHost* rvh = delegate_->CreateNewTarget();
     if (!rvh) {
       SendJson(connection_id,
                net::HTTP_INTERNAL_SERVER_ERROR,
                NULL,
                "Could not create new page",
                jsonp);
       return;
     }
-    PageInfo page_info =
-        CreatePageInfo(rvh, DevToolsHttpHandlerDelegate::kTargetTypeTab);
     std::string host = info.headers["Host"];
     scoped_ptr<base::DictionaryValue> dictionary(
-        SerializePageInfo(page_info, host));
+        SerializePageInfo(DevToolsAgentHost::GetFor(rvh), host));
     SendJson(connection_id, net::HTTP_OK, dictionary.get(), "", jsonp);
     return;
   }
 
   if (command == "activate" || command == "close") {
     DevToolsAgentHost* agent_host = binding_->ForIdentifier(target_id);
     RenderViewHost* rvh = agent_host ? agent_host->GetRenderViewHost() : NULL;
     if (!rvh) {
       SendJson(connection_id,
                net::HTTP_NOT_FOUND,
@@ skipping 143 matching lines @@
 DevToolsHttpHandlerImpl::DevToolsHttpHandlerImpl(
     const net::StreamListenSocketFactory* socket_factory,
     const std::string& frontend_url,
     DevToolsHttpHandlerDelegate* delegate)
     : overridden_frontend_url_(frontend_url),
       socket_factory_(socket_factory),
       delegate_(delegate) {
   if (overridden_frontend_url_.empty())
       overridden_frontend_url_ = "/devtools/devtools.html";
 
-  default_binding_.reset(new DevToolsDefaultBindingHandler);
-  binding_ = default_binding_.get();
+  binding_ = DevToolsTargetList::GetInstance();
 
   registrar_.Add(this, NOTIFICATION_RENDERER_PROCESS_TERMINATED,
                  NotificationService::AllBrowserContextsAndSources());
   registrar_.Add(this, NOTIFICATION_RENDERER_PROCESS_CLOSED,
                  NotificationService::AllBrowserContextsAndSources());
 
   // Balanced in ResetHandlerThreadAndRelease().
   AddRef();
 }
 
@@ skipping 102 matching lines @@
     int connection_id,
     const net::HttpServerRequestInfo& request) {
   if (!thread_.get())
     return;
   thread_->message_loop()->PostTask(
       FROM_HERE,
       base::Bind(&net::HttpServer::AcceptWebSocket, server_.get(),
                  connection_id, request));
 }
 
-DevToolsHttpHandlerImpl::PageInfo
-DevToolsHttpHandlerImpl::CreatePageInfo(RenderViewHost* rvh,
-    DevToolsHttpHandlerDelegate::TargetType type) {
-  RenderViewHostDelegate* host_delegate = rvh->GetDelegate();
-  scoped_refptr<DevToolsAgentHost> agent(DevToolsAgentHost::GetFor(rvh));
-  DevToolsClientHost* client_host = DevToolsManager::GetInstance()->
-      GetDevToolsClientHostFor(agent);
-  PageInfo page_info;
-  page_info.id = binding_->GetIdentifier(agent);
-  page_info.attached = client_host != NULL;
-  page_info.url = host_delegate->GetURL().spec();
-
-  switch (type) {
+base::DictionaryValue* DevToolsHttpHandlerImpl::SerializePageInfo(
+    DevToolsAgentHost* agent_host,
+    const std::string& host) {
+  base::DictionaryValue* dictionary = new base::DictionaryValue;
+  dictionary->SetString("title", agent_host->title());
+  dictionary->SetString("url", agent_host->url().spec());
+  switch (delegate_->GetTargetType(agent_host->GetRenderViewHost())) {
     case DevToolsHttpHandlerDelegate::kTargetTypeTab:
-      page_info.type = "page";
+      dictionary->SetString("type", "page");
       break;
     default:
-      page_info.type = "other";
+      dictionary->SetString("type", "other");
   }
-
-  WebContents* web_contents = host_delegate->GetAsWebContents();
-  if (web_contents) {
-    page_info.title = UTF16ToUTF8(
-        net::EscapeForHTML(web_contents->GetTitle()));
-    page_info.last_selected_time = web_contents->GetLastSelectedTime();
-
-    NavigationController& controller = web_contents->GetController();
-    NavigationEntry* entry = controller.GetActiveEntry();
-    if (entry != NULL && entry->GetURL().is_valid()) {
-      page_info.thumbnail_url = "/thumb/" + entry->GetURL().spec();
-      page_info.favicon_url = entry->GetFavicon().url.spec();
-    }
-  }
-  return page_info;
-}
-
-base::DictionaryValue* DevToolsHttpHandlerImpl::SerializePageInfo(
-    const PageInfo& page_info,
-    const std::string& host) {
-  base::DictionaryValue* dictionary = new base::DictionaryValue;
-  dictionary->SetString("title", page_info.title);
-  dictionary->SetString("url", page_info.url);
-  dictionary->SetString("type", page_info.type);
-  dictionary->SetString("id", page_info.id);
-  dictionary->SetString("thumbnailUrl", page_info.thumbnail_url);
-  dictionary->SetString("faviconUrl", page_info.favicon_url);
-  if (!page_info.attached) {
+  std::string id = binding_->GetIdentifier(agent_host);
+  dictionary->SetString("id", id);
+  dictionary->SetString("thumbnailUrl", agent_host->thumbnail_url().spec());
+  dictionary->SetString("faviconUrl", agent_host->favicon_url().spec());
+  if (!agent_host->attached()) {
     dictionary->SetString("webSocketDebuggerUrl",
                           base::StringPrintf("ws://%s/devtools/page/%s",
                                              host.c_str(),
-                                             page_info.id.c_str()));
+                                             id.c_str()));
     std::string devtools_frontend_url = GetFrontendURLInternal(
-        page_info.id.c_str(),
+        id.c_str(),
         host);
     dictionary->SetString("devtoolsFrontendUrl", devtools_frontend_url);
   }
   return dictionary;
 }
 
 }  // namespace content