Bug 1505368 - Show error page in about:devtools-toolbox when target is destroyed r=jdescottes,daisuke
authorBelén Albeza <balbeza@mozilla.com>
Mon, 08 Apr 2019 15:41:38 +0000
changeset 468372 6e4f040f66b7649277ac61bdb16c1b9aebadac97
parent 468371 9fcab041c5d73b9525b22afde2ad658574965992
child 468373 28a607fe5e9b85e81c20c361f4ccf563ae241293
push id35835
push useraciure@mozilla.com
push dateMon, 08 Apr 2019 19:00:29 +0000
treeherdermozilla-central@40456af7da1c [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/framework/components/DebugTargetInfo.js
devtools/client/framework/test/jest/components/__snapshots__/debug-target-info.test.js.snap
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 || asan # 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/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 js-target-title"}, 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/test/jest/components/__snapshots__/debug-target-info.test.js.snap
+++ b/devtools/client/framework/test/jest/components/__snapshots__/debug-target-info.test.js.snap
@@ -1,13 +1,13 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
 exports[`DebugTargetInfo component renders the expected snapshot for This Firefox target 1`] = `
 <header
-  className="debug-target-info"
+  className="debug-target-info js-debug-target-info"
 >
   <span
     className="iconized-label"
   >
     <img
       className="channel-icon"
       src="chrome://devtools/skin/images/aboutdebugging-firefox-release.svg"
     />
@@ -38,17 +38,17 @@ exports[`DebugTargetInfo component rende
       http://some.target/url
     </span>
   </span>
 </header>
 `;
 
 exports[`DebugTargetInfo component renders the expected snapshot for USB Release target 1`] = `
 <header
-  className="debug-target-info"
+  className="debug-target-info js-debug-target-info"
 >
   <span
     className="iconized-label js-connection-info"
   >
     <img
       alt="usb icon"
       src="chrome://devtools/skin/images/aboutdebugging-usb-icon.svg"
     />
@@ -90,17 +90,17 @@ exports[`DebugTargetInfo component rende
       http://some.target/url
     </span>
   </span>
 </header>
 `;
 
 exports[`DebugTargetInfo component renders the expected snapshot for a Toolbox with an unnamed target 1`] = `
 <header
-  className="debug-target-info"
+  className="debug-target-info js-debug-target-info"
 >
   <span
     className="iconized-label"
   >
     <img
       className="channel-icon"
       src="chrome://devtools/skin/images/aboutdebugging-firefox-release.svg"
     />
--- a/devtools/client/framework/toolbox-init.js
+++ b/devtools/client/framework/toolbox-init.js
@@ -123,12 +123,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
@@ -163,27 +163,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;
 
@@ -609,16 +610,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");
   },
 
@@ -3067,17 +3083,17 @@ Toolbox.prototype = {
           // This is done after other destruction tasks since it may tear down
           // fronts and the debugger transport which earlier destroy methods may
           // require to complete.
           if (!this._target) {
             return null;
           }
           const target = this._target;
           this._target = null;
-          target.off("close", this.destroy);
+          target.off("close", this._onTargetClosed);
           return target.destroy();
         }, console.error).then(() => {
           this.emit("destroyed");
 
           // Free _host after the call to destroyed in order to let a chance
           // to destroyed listeners to still query toolbox attributes
           this._host = null;
           this._win = null;