Revert of [DevTools] Show icon in top toolbar when Node target is ava…

…ilable (patchset chromium#3 id:40001 of https://codereview.chromium.org/2890973002/ ) Reason for revert: See issue 727517 Original issue's description: > [DevTools] Show icon in top toolbar when Node target is available > > Clicking on it opens dedicated Node frontend. > > BUG=706916 > > Review-Url: https://codereview.chromium.org/2890973002 > Cr-Commit-Position: refs/heads/master@{#472739} > Committed: https://chromium.googlesource.com/chromium/src/+/ad3ffab411cc396ffb20c1d35dcff6de038f14b4 TBR=pfeldman@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=706916 Review-Url: https://codereview.chromium.org/2932703002 Cr-Commit-Position: refs/heads/master@{#478017}
Denger-Network · Jun 8, 2017 · 24309d8 · 24309d8
1 parent 8b9c7a2
commit 24309d8
Show file tree

Hide file tree

Showing 12 changed files with 59 additions and 118 deletions.
diff --git a/chrome/browser/devtools/devtools_window.cc b/chrome/browser/devtools/devtools_window.cc
@@ -585,13 +585,6 @@ void DevToolsWindow::OpenExternalFrontend(
 
 // static
 void DevToolsWindow::OpenNodeFrontendWindow(Profile* profile) {
-  for (DevToolsWindow* window : g_instances.Get()) {
-    if (window->frontend_type_ == kFrontendNode) {
-      window->ActivateWindow();
-      return;
-    }
-  }
-
   DevToolsWindow* window =
       Create(profile, nullptr, kFrontendNode, std::string(), false,
              std::string(), std::string());
@@ -813,14 +806,12 @@ void DevToolsWindow::OnPageCloseCanceled(WebContents* contents) {
   DevToolsWindow::OnPageCloseCanceled(window->main_web_contents_);
 }
 
-DevToolsWindow::DevToolsWindow(FrontendType frontend_type,
-                               Profile* profile,
+DevToolsWindow::DevToolsWindow(Profile* profile,
                                WebContents* main_web_contents,
                                DevToolsUIBindings* bindings,
                                WebContents* inspected_web_contents,
                                bool can_dock)
-    : frontend_type_(frontend_type),
-      profile_(profile),
+    : profile_(profile),
       main_web_contents_(main_web_contents),
       toolbox_web_contents_(nullptr),
       bindings_(bindings),
@@ -915,8 +906,8 @@ DevToolsWindow* DevToolsWindow::Create(
     return nullptr;
   if (!settings.empty())
     SetPreferencesFromJson(profile, settings);
-  return new DevToolsWindow(frontend_type, profile, main_web_contents.release(),
-                            bindings, inspected_web_contents, can_dock);
+  return new DevToolsWindow(profile, main_web_contents.release(), bindings,
+                            inspected_web_contents, can_dock);
 }
 
 // static

diff --git a/chrome/browser/devtools/devtools_window.h b/chrome/browser/devtools/devtools_window.h
@@ -247,8 +247,7 @@ class DevToolsWindow : public DevToolsUIBindings::Delegate,
     kFrontendNode
   };
 
-  DevToolsWindow(FrontendType frontend_type,
-                 Profile* profile,
+  DevToolsWindow(Profile* profile,
                  content::WebContents* main_web_contents,
                  DevToolsUIBindings* bindings,
                  content::WebContents* inspected_web_contents,
@@ -352,7 +351,6 @@ class DevToolsWindow : public DevToolsUIBindings::Delegate,
 
   std::unique_ptr<ObserverWithAccessor> inspected_contents_observer_;
 
-  FrontendType frontend_type_;
   Profile* profile_;
   content::WebContents* main_web_contents_;
   content::WebContents* toolbox_web_contents_;

diff --git a/content/browser/devtools/protocol/target_handler.cc b/content/browser/devtools/protocol/target_handler.cc
@@ -223,11 +223,9 @@ void TargetHandler::TargetDestroyedInternal(
 
 bool TargetHandler::AttachToTargetInternal(
     DevToolsAgentHost* host, bool waiting_for_debugger) {
-  attached_hosts_[host->GetId()] = host;
-  if (!host->AttachClient(this)) {
-    attached_hosts_.erase(host->GetId());
+  if (!host->AttachClient(this))
     return false;
-  }
+  attached_hosts_[host->GetId()] = host;
   frontend_->AttachedToTarget(CreateInfo(host), waiting_for_debugger);
   return true;
 }
@@ -417,8 +415,6 @@ bool TargetHandler::ShouldForceDevToolsAgentHostCreation() {
 }
 
 void TargetHandler::DevToolsAgentHostCreated(DevToolsAgentHost* agent_host) {
-  if (agent_host->GetType() == "node" && agent_host->IsAttached())
-    return;
   // If we start discovering late, all existing agent hosts will be reported,
   // but we could have already attached to some.
   TargetCreatedInternal(agent_host);
@@ -429,22 +425,6 @@ void TargetHandler::DevToolsAgentHostDestroyed(DevToolsAgentHost* agent_host) {
   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()) {
-    TargetDestroyedInternal(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()) {
-    TargetCreatedInternal(host);
-  }
-}
-
 // -------- ServiceWorkerDevToolsManager::Observer ----------
 
 void TargetHandler::WorkerCreated(

diff --git a/content/browser/devtools/protocol/target_handler.h b/content/browser/devtools/protocol/target_handler.h
@@ -94,8 +94,6 @@ class TargetHandler : public DevToolsDomainHandler,
   bool ShouldForceDevToolsAgentHostCreation() override;
   void DevToolsAgentHostCreated(DevToolsAgentHost* agent_host) override;
   void DevToolsAgentHostDestroyed(DevToolsAgentHost* agent_host) override;
-  void DevToolsAgentHostAttached(DevToolsAgentHost* agent_host) override;
-  void DevToolsAgentHostDetached(DevToolsAgentHost* agent_host) override;
 
   // DevToolsAgentHostClient implementation.
   void DispatchProtocolMessage(DevToolsAgentHost* agent_host,

diff --git a/third_party/WebKit/Source/devtools/BUILD.gn b/third_party/WebKit/Source/devtools/BUILD.gn
@@ -303,7 +303,6 @@ all_devtools_files = [
   "front_end/main/GCActionDelegate.js",
   "front_end/main/Main.js",
   "front_end/main/module.json",
-  "front_end/main/nodeIcon.css",
   "front_end/main/remoteDebuggingTerminatedScreen.css",
   "front_end/main/renderingOptions.css",
   "front_end/main/RenderingOptions.js",
@@ -819,7 +818,6 @@ devtools_image_files = [
   "front_end/Images/ic_warning_black_18dp.svg",
   "front_end/Images/navigationControls.png",
   "front_end/Images/navigationControls_2x.png",
-  "front_end/Images/nodeIcon.png",
   "front_end/Images/popoverArrows.png",
   "front_end/Images/profileGroupIcon.png",
   "front_end/Images/profileIcon.png",

diff --git a/third_party/WebKit/Source/devtools/front_end/Images/nodeIcon.png b/third_party/WebKit/Source/devtools/front_end/Images/nodeIcon.png
diff --git a/third_party/WebKit/Source/devtools/front_end/main/Main.js b/third_party/WebKit/Source/devtools/front_end/main/Main.js
@@ -728,37 +728,6 @@ Main.Main.MainMenuItem = class {
   }
 };
 
-/**
- * @implements {UI.ToolbarItem.Provider}
- */
-Main.Main.NodeIndicator = class {
-  constructor() {
-    var element = createElement('div');
-    var shadowRoot = UI.createShadowRootWithCoreStyles(element, 'main/nodeIcon.css');
-    this._element = shadowRoot.createChild('div', 'node-icon');
-    element.addEventListener('click', () => InspectorFrontendHost.openNodeFrontend(), false);
-    this._button = new UI.ToolbarItem(element);
-    this._button.setTitle(Common.UIString('Open dedicated DevTools for Node.js'));
-    SDK.targetManager.addEventListener(SDK.TargetManager.Events.AvailableNodeTargetsChanged, this._update, this);
-    this._button.setVisible(false);
-    this._update();
-  }
-
-  _update() {
-    this._element.classList.toggle('inactive', !SDK.targetManager.availableNodeTargetsCount());
-    if (SDK.targetManager.availableNodeTargetsCount())
-      this._button.setVisible(true);
-  }
-
-  /**
-   * @override
-   * @return {?UI.ToolbarItem}
-   */
-  item() {
-    return this._button;
-  }
-};
-
 Main.NetworkPanelIndicator = class {
   constructor() {
     // TODO: we should not access network from other modules.

diff --git a/third_party/WebKit/Source/devtools/front_end/main/module.json b/third_party/WebKit/Source/devtools/front_end/main/module.json
@@ -202,13 +202,6 @@
             "location": "main-toolbar-left",
             "order": 100
         },
-        {
-            "type": "@UI.ToolbarItem.Provider",
-            "className": "Main.Main.NodeIndicator",
-            "order": 2,
-            "location": "main-toolbar-left",
-            "condition": "!nodeFrontend"
-        },
         {
             "type": "@UI.ToolbarItem.Provider",
             "className": "Main.Main.WarningErrorCounter",
@@ -448,7 +441,6 @@
     ],
     "resources": [
         "errorWarningCounter.css",
-        "nodeIcon.css",
         "remoteDebuggingTerminatedScreen.css",
         "renderingOptions.css",
         "targetCrashedScreen.css"

diff --git a/third_party/WebKit/Source/devtools/front_end/main/nodeIcon.css b/third_party/WebKit/Source/devtools/front_end/main/nodeIcon.css
diff --git a/third_party/WebKit/Source/devtools/front_end/sdk/TargetManager.js b/third_party/WebKit/Source/devtools/front_end/sdk/TargetManager.js
@@ -328,9 +328,9 @@ SDK.TargetManager = class extends Common.Object {
   _connectAndCreateMainTarget() {
     if (Runtime.queryParam('nodeFrontend')) {
       var target = new SDK.Target(
-          this, 'main', Common.UIString('Node.js'), SDK.Target.Capability.Target, this._createMainConnection.bind(this),
+          this, 'main', Common.UIString('Node'), SDK.Target.Capability.Target, this._createMainConnection.bind(this),
           null);
-      target.setInspectedURL('Node.js');
+      target.setInspectedURL('Node');
       this._childTargetManagers.set(target, new SDK.ChildTargetManager(this, target));
       Host.userMetrics.actionTaken(Host.UserMetrics.Action.ConnectToNodeJSFromFrontend);
       return;
@@ -410,15 +410,13 @@ SDK.ChildTargetManager = class {
     if (Runtime.experiments.isEnabled('autoAttachToCrossProcessSubframes'))
       this._targetAgent.setAttachToFrames(true);
 
-    if (!parentTarget.parentTarget()) {
+    if (!parentTarget.parentTarget())
       this._targetAgent.setDiscoverTargets(true);
-      if (Runtime.queryParam('nodeFrontend')) {
-        InspectorFrontendHost.setDevicesUpdatesEnabled(true);
-        InspectorFrontendHost.events.addEventListener(
-            InspectorFrontendHostAPI.Events.DevicesDiscoveryConfigChanged, this._devicesDiscoveryConfigChanged, this);
-      } else {
-        this._targetAgent.setRemoteLocations([{host: 'localhost', port: 9229}]);
-      }
+
+    if (Runtime.queryParam('nodeFrontend') && !this._parentTarget.parentTarget()) {
+      InspectorFrontendHost.setDevicesUpdatesEnabled(true);
+      InspectorFrontendHost.events.addEventListener(
+          InspectorFrontendHostAPI.Events.DevicesDiscoveryConfigChanged, this._devicesDiscoveryConfigChanged, this);
     }
   }
 
@@ -431,8 +429,7 @@ SDK.ChildTargetManager = class {
     for (var address of config.networkDiscoveryConfig) {
       var parts = address.split(':');
       var port = parseInt(parts[1], 10);
-      if (parts[0] && port)
-        locations.push({host: parts[0], port: port});
+      locations.push({host: parts[0] || 'localhost', port: port || 9229});
     }
     this._targetAgent.setRemoteLocations(locations);
   }
@@ -516,7 +513,7 @@ SDK.ChildTargetManager = class {
   attachedToTarget(targetInfo, waitingForDebugger) {
     var targetName = '';
     if (targetInfo.type === 'node') {
-      targetName = Common.UIString('Node.js: %s', targetInfo.url);
+      targetName = Common.UIString('Node: %s', targetInfo.url);
     } else if (targetInfo.type !== 'iframe') {
       var parsedURL = targetInfo.url.asParsedURL();
       targetName =

diff --git a/third_party/WebKit/Source/devtools/front_end/sources/ThreadsSidebarPane.js b/third_party/WebKit/Source/devtools/front_end/sources/ThreadsSidebarPane.js
@@ -16,7 +16,14 @@ Sources.ThreadsSidebarPane = class extends UI.VBox {
     this._list = new UI.ListControl(this._items, this, UI.ListMode.NonViewport);
     this.contentElement.appendChild(this._list.element);
 
+    this._availableNodeTargetsElement = this.contentElement.createChild('div', 'hidden available-node-targets');
+
     UI.context.addFlavorChangeListener(SDK.Target, this._targetFlavorChanged, this);
+
+    SDK.targetManager.addEventListener(
+        SDK.TargetManager.Events.AvailableNodeTargetsChanged, this._availableNodeTargetsChanged, this);
+    this._availableNodeTargetsChanged();
+
     SDK.targetManager.observeModels(SDK.DebuggerModel, this);
   }
 
@@ -25,7 +32,27 @@ Sources.ThreadsSidebarPane = class extends UI.VBox {
    */
   static shouldBeShown() {
     var minJSTargets = Runtime.queryParam('nodeFrontend') ? 1 : 2;
-    return SDK.targetManager.models(SDK.DebuggerModel).length >= minJSTargets;
+    if (SDK.targetManager.models(SDK.DebuggerModel).length >= minJSTargets)
+      return true;
+    return !!SDK.targetManager.availableNodeTargetsCount();
+  }
+
+  _availableNodeTargetsChanged() {
+    var count = SDK.targetManager.availableNodeTargetsCount();
+    if (!count) {
+      this._availableNodeTargetsElement.classList.add('hidden');
+      return;
+    }
+    this._availableNodeTargetsElement.removeChildren();
+    this._availableNodeTargetsElement.createTextChild(
+        count === 1 ? Common.UIString('Node instance available.') :
+                      Common.UIString('%d Node instances available.', count));
+    var link = this._availableNodeTargetsElement.createChild('span', 'link');
+    link.textContent = Common.UIString('Connect');
+    link.addEventListener('click', () => {
+      InspectorFrontendHost.openNodeFrontend();
+    }, false);
+    this._availableNodeTargetsElement.classList.remove('hidden');
   }
 
   /**

diff --git a/third_party/WebKit/Source/devtools/front_end/sources/threadsSidebarPane.css b/third_party/WebKit/Source/devtools/front_end/sources/threadsSidebarPane.css
@@ -4,6 +4,20 @@
  * found in the LICENSE file.
  */
 
+.available-node-targets {
+    height: 22px;
+    display: flex;
+    align-items: center;
+    justify-content: flex-start;
+    padding-left: 20px;
+    border-top: 1px solid #eee;
+    font-style: italic;
+}
+
+.available-node-targets > span {
+    margin-left: 5px;
+}
+
 .thread-item {
     padding: 3px 8px 3px 20px;
     position: relative;