Bug 1505368 - Show error page in about:devtools-toolbox when target is destroyed r=jdescottes,daisuke

Differential Revision: https://phabricator.services.mozilla.com/D20315

--HG--
extra : moz-landing-system : lando

devtools/client/aboutdebugging-new/test/browser/browser.ini

@@ -59,6 +59,8 @@ skip-if = (os == 'linux' && bits == 32) # ADB start() fails on linux 32, see Bug
 [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]

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);
+});

devtools/client/aboutdebugging-new/test/browser/head.js

@@ -64,12 +64,12 @@ async function openAboutDebugging({ enableWorkerUpdates } = {}) {
   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),

devtools/client/framework/components/DebugTargetInfo.js

@@ -116,7 +116,7 @@ class DebugTargetInfo extends PureComponent {
   render() {
     return dom.header(
       {
-        className: "debug-target-info",
+        className: "debug-target-info js-debug-target-info",
       },
       this.shallRenderConnection() ? this.renderConnection() : null,
       this.renderRuntime(),

devtools/client/framework/toolbox-init.js

@@ -120,7 +120,14 @@ async function initToolbox(url, host) {
 
 // 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

devtools/client/framework/toolbox.js

@@ -166,6 +166,7 @@ function Toolbox(target, selectedTool, hostType, contentWindow, frameId,
   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);
@@ -176,7 +177,7 @@ function Toolbox(target, selectedTool, hostType, contentWindow, frameId,
   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);
@@ -612,6 +613,21 @@ Toolbox.prototype = {
     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).
@@ -2904,6 +2920,7 @@ Toolbox.prototype = {
     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);