DevTools: [regression] remote debugging to a tcp port does not work via chrome:inspect

chrome/browser/devtools/devtools_window.cc

Index: chrome/browser/devtools/devtools_window.cc
diff --git a/chrome/browser/devtools/devtools_window.cc b/chrome/browser/devtools/devtools_window.cc
index a8f0f0c76bb6b4c2f0291d76c1638c6855213eb3..dd1ecd22403c8ed0665d89456d354cfdae1ce824 100644
--- a/chrome/browser/devtools/devtools_window.cc
+++ b/chrome/browser/devtools/devtools_window.cc
@@ -467,7 +467,7 @@ void DevToolsWindow::OpenDevToolsWindowForWorker(
 DevToolsWindow* DevToolsWindow::CreateDevToolsWindowForWorker(
     Profile* profile) {
   content::RecordAction(base::UserMetricsAction("DevTools_InspectWorker"));
-  return Create(profile, NULL, true, false, false, std::string(), false, "",
+  return Create(profile, nullptr, kFrontendWorker, std::string(), false, "",
                 "");
 }
 
@@ -489,13 +489,19 @@ void DevToolsWindow::OpenDevToolsWindow(
     return;
 
   std::string type = agent_host->GetType();
+
   bool is_worker = type == DevToolsAgentHost::kTypeServiceWorker ||
                    type == DevToolsAgentHost::kTypeSharedWorker;
 
   if (!agent_host->GetFrontendURL().empty()) {
-    bool is_v8_only = type == "node";
+    FrontendType frontend_type = kFrontendRemote;
+    if (is_worker) {
+      frontend_type = kFrontendWorker;
+    } else if (type == "node") {
+      frontend_type = kFrontendNode;

[dgozman, 2017/03/17 17:40:59]

kFrontendV8

[pfeldman, 2017/03/20 19:30:23]

Ouch.

+    }
     DevToolsWindow::OpenExternalFrontend(profile, agent_host->GetFrontendURL(),
-                                         agent_host, is_worker, is_v8_only);
+                                         agent_host, frontend_type);
     return;
   }
 
@@ -527,7 +533,7 @@ void DevToolsWindow::OpenDevToolsWindowForFrame(
     const scoped_refptr<content::DevToolsAgentHost>& agent_host) {
   DevToolsWindow* window = FindDevToolsWindow(agent_host.get());
   if (!window) {
-    window = DevToolsWindow::Create(profile, nullptr, false, false, false,
+    window = DevToolsWindow::Create(profile, nullptr, kFrontendDefault,
                                     std::string(), false, std::string(),
                                     std::string());
     if (!window)
@@ -558,11 +564,10 @@ void DevToolsWindow::OpenExternalFrontend(
     Profile* profile,
     const std::string& frontend_url,
     const scoped_refptr<content::DevToolsAgentHost>& agent_host,
-    bool is_worker,
-    bool is_v8_only) {
+    FrontendType frontend_type) {
   DevToolsWindow* window = FindDevToolsWindow(agent_host.get());
   if (!window) {
-    window = Create(profile, nullptr, is_worker, is_v8_only, false,
+    window = Create(profile, nullptr, frontend_type,
                     DevToolsUI::GetProxyURL(frontend_url).spec(), false,
                     std::string(), std::string());
     if (!window)
@@ -577,7 +582,7 @@ void DevToolsWindow::OpenExternalFrontend(
 // static
 void DevToolsWindow::OpenNodeFrontendWindow(Profile* profile) {
   DevToolsWindow* window =
-      Create(profile, nullptr, false, true, true, std::string(), false,
+      Create(profile, nullptr, kFrontendNode, std::string(), false,
              std::string(), std::string());
   if (!window)
     return;
@@ -615,7 +620,7 @@ void DevToolsWindow::ToggleDevToolsWindow(
       case DevToolsToggleAction::kNoOp:
         break;
     }
-    window = Create(profile, inspected_web_contents, false, false, false,
+    window = Create(profile, inspected_web_contents, kFrontendDefault,
                     std::string(), true, settings, panel);
     if (!window)
       return;
@@ -868,10 +873,8 @@ DevToolsWindow::DevToolsWindow(Profile* profile,
 DevToolsWindow* DevToolsWindow::Create(
     Profile* profile,
     content::WebContents* inspected_web_contents,
-    bool shared_worker_frontend,
-    bool v8_only_frontend,
-    bool node_frontend,
-    const std::string& remote_frontend,
+    FrontendType frontend_type,
+    const std::string& frontend_url,
     bool can_dock,
     const std::string& settings,
     const std::string& panel) {
@@ -881,7 +884,7 @@ DevToolsWindow* DevToolsWindow::Create(
 
   if (inspected_web_contents) {
     // Check for a place to dock.
-    Browser* browser = NULL;
+    Browser* browser = nullptr;
     int tab;
     if (!FindInspectedBrowserAndTabIndex(inspected_web_contents,
                                          &browser, &tab) ||
@@ -891,8 +894,8 @@ DevToolsWindow* DevToolsWindow::Create(
   }
 
   // Create WebContents with devtools.
-  GURL url(GetDevToolsURL(profile, shared_worker_frontend, v8_only_frontend,
-                          node_frontend, remote_frontend, can_dock, panel));
+  GURL url(
+      GetDevToolsURL(profile, frontend_type, frontend_url, can_dock, panel));
   std::unique_ptr<WebContents> main_web_contents(
       WebContents::Create(WebContents::CreateParams(profile)));
   main_web_contents->GetController().LoadURL(
@@ -910,29 +913,34 @@ DevToolsWindow* DevToolsWindow::Create(
 
 // static
 GURL DevToolsWindow::GetDevToolsURL(Profile* profile,
-                                    bool shared_worker_frontend,
-                                    bool v8_only_frontend,
-                                    bool node_frontend,
-                                    const std::string& remote_frontend,
+                                    FrontendType frontend_type,
+                                    const std::string& frontend_url,
                                     bool can_dock,
                                     const std::string& panel) {
-  std::string frontend_url(!remote_frontend.empty()
-                               ? remote_frontend
-                               : chrome::kChromeUIDevToolsURL);
-  std::string url_string(
-      frontend_url +
-      ((frontend_url.find("?") == std::string::npos) ? "?" : "&"));
-  if (shared_worker_frontend)
-    url_string += "&isSharedWorker=true";
-  if (v8_only_frontend)
-    url_string += "&v8only=true";
-  if (node_frontend)
-    url_string += "&nodeFrontend=true";
-  if (remote_frontend.size()) {
-    url_string += "&remoteFrontend=true";
-  } else {
-    url_string += "&remoteBase=" + DevToolsUI::GetRemoteBaseURL().spec();
+  std::string url(!frontend_url.empty() ? frontend_url
+                                        : chrome::kChromeUIDevToolsURL);
+  std::string url_string(url +
+                         ((url.find("?") == std::string::npos) ? "?" : "&"));
+  switch (frontend_type) {
+    case kFrontendRemote:
+      url_string += "&remoteFrontend=true";
+      break;
+    case kFrontendWorker:
+      url_string += "&isSharedWorker=true";
+      break;
+    case kFrontendNode:
+      url_string += "&nodeFrontend=true";
+    // Fall through
+    case kFrontendV8:
+      url_string += "&v8only=true";
+      break;
+    case kFrontendDefault:
+    default:
+      break;
   }
+
+  if (frontend_url.empty())

[dgozman, 2017/03/17 17:40:59]

This was checking remote_frontend before, not the frontend_url. Do they always
come together?

[pfeldman, 2017/03/20 19:30:23]

I renamed remote_frontend to frontend_url.

+    url_string += "&remoteBase=" + DevToolsUI::GetRemoteBaseURL().spec();
   if (can_dock)
     url_string += "&can_dock=true";
   if (panel.size())