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

chrome/browser/devtools/devtools_window.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 "chrome/browser/devtools/devtools_window.h"
 
 #include <algorithm>
 
 #include "base/command_line.h"
 #include "base/json/json_reader.h"
@@ skipping 449 matching lines @@
       return;
     window->bindings_->AttachTo(worker_agent);
   }
   window->ScheduleShow(DevToolsToggleAction::Show());
 }
 
 // static
 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, "",
                 "");
 }
 
 // static
 void DevToolsWindow::OpenDevToolsWindow(
     content::WebContents* inspected_web_contents) {
   ToggleDevToolsWindow(
         inspected_web_contents, true, DevToolsToggleAction::Show(), "");
 }
 
 // static
 void DevToolsWindow::OpenDevToolsWindow(
     scoped_refptr<content::DevToolsAgentHost> agent_host,
     Profile* profile) {
   if (!profile)
     profile = Profile::FromBrowserContext(agent_host->GetBrowserContext());
 
   if (!profile)
     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;
   }
 
   if (is_worker) {
     DevToolsWindow::OpenDevToolsWindowForWorker(profile, agent_host);
     return;
   }
 
   if (type == content::DevToolsAgentHost::kTypeFrame) {
     DevToolsWindow::OpenDevToolsWindowForFrame(profile, agent_host);
@@ skipping 11 matching lines @@
     const DevToolsToggleAction& action) {
   ToggleDevToolsWindow(inspected_web_contents, true, action, "");
 }
 
 // static
 void DevToolsWindow::OpenDevToolsWindowForFrame(
     Profile* profile,
     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)
       return;
     window->bindings_->AttachTo(agent_host);
   }
   window->ScheduleShow(DevToolsToggleAction::Show());
 }
 
 // static
@@ skipping 10 matching lines @@
       browser->tab_strip_model()->GetActiveWebContents(),
       action.type() == DevToolsToggleAction::kInspect,
       action, "");
 }
 
 // static
 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)
       return;
     window->bindings_->AttachTo(agent_host);
     window->close_on_detach_ = false;
   }
 
   window->ScheduleShow(DevToolsToggleAction::Show());
 }
 
 // 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;
   window->bindings_->AttachTo(DevToolsAgentHost::CreateForDiscovery());
   window->ScheduleShow(DevToolsToggleAction::Show());
 }
 
 // static
 void DevToolsWindow::ToggleDevToolsWindow(
     content::WebContents* inspected_web_contents,
@@ skipping 17 matching lines @@
         break;
       case DevToolsToggleAction::kShowConsolePanel:
         panel = "console";
         break;
       case DevToolsToggleAction::kShow:
       case DevToolsToggleAction::kToggle:
       case DevToolsToggleAction::kReveal:
       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;
     window->bindings_->AttachTo(agent.get());
     do_open = true;
   }
 
   // Update toolbar to reflect DevTools changes.
   window->UpdateBrowserToolbar();
 
@@ skipping 232 matching lines @@
   std::vector<base::Callback<void(DevToolsWindow*)>> copy(
       g_creation_callbacks.Get());
   for (const auto& callback : copy)
     callback.Run(this);
 }
 
 // static
 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) {
   if (profile->GetPrefs()->GetBoolean(prefs::kDevToolsDisabled) ||
       base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kKioskMode))
     return nullptr;
 
   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) ||
         browser->is_type_popup()) {
       can_dock = false;
     }
   }
 
   // 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(
       DecorateFrontendURL(url), content::Referrer(),
       ui::PAGE_TRANSITION_AUTO_TOPLEVEL, std::string());
   DevToolsUIBindings* bindings =
       DevToolsUIBindings::ForWebContents(main_web_contents.get());
   if (!bindings)
     return nullptr;
   if (!settings.empty())
     SetPreferencesFromJson(profile, settings);
   return new DevToolsWindow(profile, main_web_contents.release(), bindings,
                             inspected_web_contents, can_dock);
 }
 
 // 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 {
+  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())
     url_string += "&panel=" + panel;
   return DevToolsUIBindings::SanitizeFrontendURL(GURL(url_string));
 }
 
 // static
 DevToolsWindow* DevToolsWindow::FindDevToolsWindow(
     DevToolsAgentHost* agent_host) {
@@ skipping 455 matching lines @@
 bool DevToolsWindow::ReloadInspectedWebContents(bool bypass_cache) {
   // Only route reload via front-end if the agent is attached.
   WebContents* wc = GetInspectedWebContents();
   if (!wc || wc->GetCrashedStatus() != base::TERMINATION_STATUS_STILL_RUNNING)
     return false;
   base::Value bypass_cache_value(bypass_cache);
   bindings_->CallClientFunction("DevToolsAPI.reloadInspectedPage",
                                 &bypass_cache_value, nullptr, nullptr);
   return true;
 }