Bug 1505368 - Show error page in about:devtools-toolbox when target is destroyed r=jdescottes,daisuke
☠☠ backed out by d64cfe30769a ☠ ☠
authorBelén Albeza <balbeza@mozilla.com>
Wed, 20 Mar 2019 13:28:17 +0000
changeset 465209 a14e5a146b8dcfe063a91f0c0e435e0bf42b0613
parent 465208 77453788170e8775e2e0b9a2cb44779d583137ca
child 465210 87e8e93c2133e0e7dfe3e102d5c5e117dcf41548
push id112496
push usershindli@mozilla.com
push dateThu, 21 Mar 2019 04:37:39 +0000
treeherdermozilla-inbound@29476d3ca61d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdescottes, daisuke
bugs1505368
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1505368 - Show error page in about:devtools-toolbox when target is destroyed r=jdescottes,daisuke Differential Revision: https://phabricator.services.mozilla.com/D20315
devtools/client/aboutdebugging-new/test/browser/browser.ini
devtools/client/aboutdebugging-new/test/browser/browser_aboutdebugging_devtoolstoolbox_target_destroyed.js
devtools/client/aboutdebugging-new/test/browser/head.js
devtools/client/framework/components/DebugTargetInfo.js
devtools/client/framework/toolbox-init.js
devtools/client/framework/toolbox.js
--- a/devtools/client/aboutdebugging-new/test/browser/browser.ini
+++ b/devtools/client/aboutdebugging-new/test/browser/browser.ini
@@ -54,16 +54,18 @@ skip-if = (os == 'linux' && bits == 32) 
 [browser_aboutdebugging_debug-target-pane_usb_runtime.js]
 [browser_aboutdebugging_devtools.js]
 [browser_aboutdebugging_devtoolstoolbox_contextmenu.js]
 [browser_aboutdebugging_devtoolstoolbox_contextmenu_markupview.js]
 [browser_aboutdebugging_devtoolstoolbox_menubar.js]
 [browser_aboutdebugging_devtoolstoolbox_reload.js]
 [browser_aboutdebugging_devtoolstoolbox_shortcuts.js]
 skip-if = (os == "win" && ccov) # Bug 1521349
+[browser_aboutdebugging_devtoolstoolbox_target_destroyed.js]
+skip-if = debug # This test leaks. See bug 1529005
 [browser_aboutdebugging_devtoolstoolbox_tooltip_markupview.js]
 [browser_aboutdebugging_navigate.js]
 [browser_aboutdebugging_persist_connection.js]
 [browser_aboutdebugging_process_category.js]
 [browser_aboutdebugging_process_main.js]
 [browser_aboutdebugging_profiler_dialog.js]
 [browser_aboutdebugging_real_usb_runtime_page_runtime_info.js]
 [browser_aboutdebugging_real_usb_sidebar.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/aboutdebugging-new/test/browser/browser_aboutdebugging_devtoolstoolbox_target_destroyed.js
@@ -0,0 +1,30 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Test that the expected supported categories are displayed for USB runtimes.
+add_task(async function() {
+  const targetTab = await addTab("about:home");
+
+  const { document, tab, window } = await openAboutDebugging();
+
+  // go to This Firefox and inspect the new tab
+  info("Inspecting a new tab in This Firefox");
+  await selectThisFirefoxPage(document, window.AboutDebugging.store);
+  const { devtoolsDocument, devtoolsTab, devtoolsWindow } =
+    await openAboutDevtoolsToolbox(document, tab, window, "about:home");
+  const targetInfoHeader = devtoolsDocument.querySelector(".js-debug-target-info");
+  ok(targetInfoHeader.textContent.includes("about:home"),
+    "about:devtools-toolbox is open for the target");
+
+  // close the inspected tab and check that error page is shown
+  info("removing the inspected tab");
+  await removeTab(targetTab);
+  await waitUntil(() => devtoolsWindow.document.querySelector(".js-error-page"));
+
+  info("closing the toolbox");
+  await removeTab(devtoolsTab);
+  info("removing about:debugging tab");
+  await removeTab(tab);
+});
--- a/devtools/client/aboutdebugging-new/test/browser/head.js
+++ b/devtools/client/aboutdebugging-new/test/browser/head.js
@@ -59,22 +59,22 @@ async function openAboutDebugging({ enab
   const window = browser.contentWindow;
 
   info("Wait until Connect page is displayed");
   await waitUntil(() => document.querySelector(".js-connect-page"));
 
   return { tab, document, window };
 }
 
-async function openAboutDevtoolsToolbox(doc, tab, win) {
+async function openAboutDevtoolsToolbox(doc, tab, win, targetTitle = "about:debugging") {
   info("Open about:devtools-toolbox page");
-  const target = findDebugTargetByText("about:debugging", doc);
+  const target = findDebugTargetByText(targetTitle, doc);
   ok(target, "about:debugging tab target appeared");
   const inspectButton = target.querySelector(".js-debug-target-inspect-button");
-  ok(inspectButton, "Inspect button for about:debugging appeared");
+  ok(inspectButton, `Inspect button for ${targetTitle} appeared`);
   inspectButton.click();
   await Promise.all([
     waitUntil(() => tab.nextElementSibling),
     waitForRequestsToSettle(win.AboutDebugging.store),
     gDevTools.once("toolbox-ready"),
   ]);
 
   info("Wait for about:devtools-toolbox tab will be selected");
--- a/devtools/client/framework/components/DebugTargetInfo.js
+++ b/devtools/client/framework/components/DebugTargetInfo.js
@@ -111,17 +111,17 @@ class DebugTargetInfo extends PureCompon
       title ? dom.b({ className: "devtools-ellipsis-text"}, title) : null,
       dom.span({ className: "devtools-ellipsis-text" }, url),
     );
   }
 
   render() {
     return dom.header(
       {
-        className: "debug-target-info",
+        className: "debug-target-info js-debug-target-info",
       },
       this.shallRenderConnection() ? this.renderConnection() : null,
       this.renderRuntime(),
       this.renderTarget(),
     );
   }
 }
 
--- a/devtools/client/framework/toolbox-init.js
+++ b/devtools/client/framework/toolbox-init.js
@@ -115,12 +115,19 @@ async function initToolbox(url, host) {
     // When an error occurs, show error page with message.
     console.error("Exception while loading the toolbox", error);
     showErrorPage(host.contentDocument, `${error}`);
   }
 }
 
 // Only use this method to attach the toolbox if some query parameters are given
 if (url.search.length > 1) {
-  initToolbox(url, host);
+  // show error page if 'disconnected' param appears in the querystring
+  if (url.searchParams.has("disconnected")) {
+    const error = new Error("Debug target was disconnected");
+    showErrorPage(host.contentDocument, `${error}`);
+  // otherwise, try to init the toolbox
+  } else {
+    initToolbox(url, host);
+  }
 }
 // TODO: handle no params in about:devtool-toolbox
 // https://bugzilla.mozilla.org/show_bug.cgi?id=1526996
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -161,27 +161,28 @@ function Toolbox(target, selectedTool, h
   this._onPickerKeypress = this._onPickerKeypress.bind(this);
   this._onPickerStarting = this._onPickerStarting.bind(this);
   this._onPickerStarted = this._onPickerStarted.bind(this);
   this._onPickerStopped = this._onPickerStopped.bind(this);
   this._onPickerCanceled = this._onPickerCanceled.bind(this);
   this._onInspectObject = this._onInspectObject.bind(this);
   this._onNewSelectedNodeFront = this._onNewSelectedNodeFront.bind(this);
   this._onToolSelected = this._onToolSelected.bind(this);
+  this._onTargetClosed = this._onTargetClosed.bind(this);
   this.updateToolboxButtonsVisibility = this.updateToolboxButtonsVisibility.bind(this);
   this.updateToolboxButtons = this.updateToolboxButtons.bind(this);
   this.selectTool = this.selectTool.bind(this);
   this._pingTelemetrySelectTool = this._pingTelemetrySelectTool.bind(this);
   this.toggleSplitConsole = this.toggleSplitConsole.bind(this);
   this.toggleOptions = this.toggleOptions.bind(this);
   this.togglePaintFlashing = this.togglePaintFlashing.bind(this);
   this.toggleDragging = this.toggleDragging.bind(this);
   this.isPaintFlashing = false;
 
-  this._target.on("close", this.destroy);
+  this._target.on("close", this._onTargetClosed);
 
   if (!selectedTool) {
     selectedTool = Services.prefs.getCharPref(this._prefs.LAST_TOOL);
   }
   this._defaultToolId = selectedTool;
 
   this._hostType = hostType;
 
@@ -607,16 +608,31 @@ Toolbox.prototype = {
   _getDeviceDescription: async function() {
     const deviceFront = await this.target.client.mainRoot.getFront("device");
     const description = await deviceFront.getDescription();
     const remoteId = new this.win.URLSearchParams(this.win.location.href).get("remoteId");
     const connectionType = remoteClientManager.getConnectionTypeByRemoteId(remoteId);
     return Object.assign({}, description, { connectionType });
   },
 
+  _onTargetClosed: async function() {
+    const win = this.win; // .destroy() will set this.win to null
+
+    // clean up the toolbox
+    this.destroy();
+    // NOTE: we should await this.destroy() to ensure a proper clean up.
+    //       See https://bugzilla.mozilla.org/show_bug.cgi?id=1536144
+
+    // redirect to about:toolbox error page if we are connected to a remote
+    // target and we lose it
+    if (this.hostType === Toolbox.HostType.PAGE) {
+      win.location.replace("about:devtools-toolbox?disconnected");
+    }
+  },
+
   /**
    * loading React modules when needed (to avoid performance penalties
    * during Firefox start up time).
    */
   get React() {
     return this.browserRequire("devtools/client/shared/vendor/react");
   },
 
@@ -2899,16 +2915,17 @@ Toolbox.prototype = {
 
   _destroyToolbox: async function() {
     this.emit("destroy");
 
     this._target.off("inspect-object", this._onInspectObject);
     this._target.off("will-navigate", this._onWillNavigate);
     this._target.off("navigate", this._refreshHostTitle);
     this._target.off("frame-update", this._updateFrames);
+    this._target.off("close", this._onTargetClosed);
     this.off("select", this._onToolSelected);
     this.off("host-changed", this._refreshHostTitle);
 
     gDevTools.off("tool-registered", this._toolRegistered);
     gDevTools.off("tool-unregistered", this._toolUnregistered);
 
     Services.prefs.removeObserver("devtools.cache.disabled", this._applyCacheSettings);
     Services.prefs.removeObserver("devtools.serviceWorkers.testing.enabled",