Bug 1117067 - Harden toolbox cleanup paths, async promises for WebIDE tests. r=ochameau a=lmandel
authorJ. Ryan Stinnett <jryans@gmail.com>
Wed, 11 Feb 2015 22:14:38 -0600
changeset 249711 0b12195e1e05b412b26a15b4e4b5d4b4ae04550d
parent 249710 857055f5c6a976368ef4a4949344366c9eb10536
child 249712 8a7929c589d1ff42bdbbebf834aa9f08f58d99c4
push id4489
push userraliiev@mozilla.com
push dateMon, 23 Feb 2015 15:17:55 +0000
treeherdermozilla-beta@fd7c3dc24146 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersochameau, lmandel
bugs1117067
milestone37.0a2
Bug 1117067 - Harden toolbox cleanup paths, async promises for WebIDE tests. r=ochameau a=lmandel
browser/devtools/framework/toolbox-hosts.js
browser/devtools/webide/content/webide.js
browser/devtools/webide/test/head.js
browser/devtools/webide/test/test_autoconnect_runtime.html
browser/devtools/webide/test/test_fullscreenToolbox.html
browser/devtools/webide/test/test_runtime.html
--- a/browser/devtools/framework/toolbox-hosts.js
+++ b/browser/devtools/framework/toolbox-hosts.js
@@ -282,16 +282,19 @@ function CustomHost(hostTab, options) {
 
 CustomHost.prototype = {
   type: "custom",
 
   _sendMessageToTopWindow: function CH__sendMessageToTopWindow(msg, data) {
     // It's up to the custom frame owner (parent window) to honor
     // "close" or "raise" instructions.
     let topWindow = this.frame.ownerDocument.defaultView;
+    if (!topWindow) {
+      return;
+    }
     let json = {name:"toolbox-" + msg, uid: this.uid};
     if (data) {
       json.data = data;
     }
     topWindow.postMessage(JSON.stringify(json), "*");
   },
 
   /**
--- a/browser/devtools/webide/content/webide.js
+++ b/browser/devtools/webide/content/webide.js
@@ -888,44 +888,57 @@ let UI = {
 
   onMessage: function(event) {
     // The custom toolbox sends a message to its parent
     // window.
     try {
       let json = JSON.parse(event.data);
       switch (json.name) {
         case "toolbox-close":
-          this.closeToolboxUI();
+          // There are many ways to close a toolbox:
+          // * Close button inside the toolbox
+          // * Toggle toolbox wrench in WebIDE
+          // * Disconnect the current runtime gracefully
+          // * Yank cord out of device
+          // We can't know for sure which one was used here, so reset the
+          // |toolboxPromise| since someone must be destroying it to reach here,
+          // and call our own close method.
+          this.toolboxPromise = null;
+          this._closeToolboxUI();
           break;
       }
     } catch(e) { console.error(e); }
   },
 
   destroyToolbox: function() {
+    // Only have a live toolbox if |this.toolboxPromise| exists
     if (this.toolboxPromise) {
-      return this.toolboxPromise.then(toolbox => {
-        toolbox.destroy();
-        this.toolboxPromise = null;
-      }, console.error);
+      let toolboxPromise = this.toolboxPromise;
+      this.toolboxPromise = null;
+      return toolboxPromise.then(toolbox => {
+        return toolbox.destroy();
+      }).catch(console.error)
+        .then(() => this._closeToolboxUI())
+        .catch(console.error);
     }
     return promise.resolve();
   },
 
   createToolbox: function() {
+    // If |this.toolboxPromise| exists, there is already a live toolbox
+    if (this.toolboxPromise) {
+      return this.toolboxPromise;
+    }
     this.toolboxPromise = AppManager.getTarget().then((target) => {
-      return this.showToolbox(target);
+      return this._showToolbox(target);
     }, console.error);
     return this.busyUntil(this.toolboxPromise, "opening toolbox");
   },
 
-  showToolbox: function(target) {
-    if (this.toolboxIframe) {
-      return;
-    }
-
+  _showToolbox: function(target) {
     let splitter = document.querySelector(".devtools-horizontal-splitter");
     splitter.removeAttribute("hidden");
 
     let iframe = document.createElement("iframe");
     iframe.id = "toolbox";
 
     document.querySelector("notificationbox").insertBefore(iframe, splitter.nextSibling);
     let host = devtools.Toolbox.HostType.CUSTOM;
@@ -939,26 +952,30 @@ let UI = {
 
     this.updateToolboxFullscreenState();
     return gDevTools.showToolbox(target, null, host, options);
   },
 
   updateToolboxFullscreenState: function() {
     let panel = document.querySelector("#deck").selectedPanel;
     let nbox = document.querySelector("#notificationbox");
-    if (panel.id == "deck-panel-details" &&
+    if (panel && panel.id == "deck-panel-details" &&
         AppManager.selectedProject.type != "packaged" &&
         this.toolboxIframe) {
       nbox.setAttribute("toolboxfullscreen", "true");
     } else {
       nbox.removeAttribute("toolboxfullscreen");
     }
   },
 
-  closeToolboxUI: function() {
+  _closeToolboxUI: function() {
+    if (!this.toolboxIframe) {
+      return;
+    }
+
     this.resetFocus();
     Services.prefs.setIntPref("devtools.toolbox.footer.height", this.toolboxIframe.height);
 
     // We have to destroy the iframe, otherwise, the keybindings of webide don't work
     // properly anymore.
     this.toolboxIframe.remove();
     this.toolboxIframe = null;
 
--- a/browser/devtools/webide/test/head.js
+++ b/browser/devtools/webide/test/head.js
@@ -4,19 +4,19 @@
 "use strict";
 
 const {utils: Cu, classes: Cc, interfaces: Ci} = Components;
 
 Cu.import('resource://gre/modules/Services.jsm');
 Cu.import("resource://gre/modules/FileUtils.jsm");
 Cu.import("resource://gre/modules/Task.jsm");
 
-const {Promise: promise} = Cu.import("resource://gre/modules/devtools/deprecated-sync-thenables.js", {});
 const {devtools} = Cu.import("resource://gre/modules/devtools/Loader.jsm", {});
 const {require} = devtools;
+const promise = require("promise");
 const {AppProjects} = require("devtools/app-manager/app-projects");
 
 let TEST_BASE;
 if (window.location === "chrome://browser/content/browser.xul") {
   TEST_BASE = "chrome://mochitests/content/browser/browser/devtools/webide/test/";
 } else {
   TEST_BASE = "chrome://mochitests/content/chrome/browser/devtools/webide/test/";
 }
--- a/browser/devtools/webide/test/test_autoconnect_runtime.html
+++ b/browser/devtools/webide/test/test_autoconnect_runtime.html
@@ -66,17 +66,17 @@
 
           is(Object.keys(DebuggerServer._connections).length, 0, "Disconnected");
 
           win = yield openWebIDE();
 
           win.AppManager.runtimeList.usb.push(fakeRuntime);
           win.AppManager.update("runtimelist");
 
-          yield waitForUpdate(win, "list-tabs-response");
+          yield waitForUpdate(win, "runtime-apps-found");
 
           is(Object.keys(DebuggerServer._connections).length, 1, "Automatically reconnected");
 
           yield win.Cmds.disconnectRuntime();
 
           yield closeWebIDE(win);
 
           DebuggerServer.destroy();
--- a/browser/devtools/webide/test/test_fullscreenToolbox.html
+++ b/browser/devtools/webide/test/test_fullscreenToolbox.html
@@ -30,17 +30,17 @@
 
         Task.spawn(function* () {
           Cu.import("resource://gre/modules/devtools/dbg-server.jsm");
           win = yield openWebIDE();
           win.AppManager.update("runtimelist");
 
           yield connectToLocal(win);
 
-          yield waitForUpdate(win, "list-tabs-response");
+          yield waitForUpdate(win, "runtime-apps-found");
 
           // Select main process
           yield win.Cmds.showProjectPanel();
           SimpleTest.executeSoon(() => {
             win.document.querySelectorAll("#project-panel-runtimeapps .panel-item")[0].click();
           });
 
           yield waitForUpdate(win, "project");
--- a/browser/devtools/webide/test/test_runtime.html
+++ b/browser/devtools/webide/test/test_runtime.html
@@ -79,17 +79,17 @@
 
           items[0].click();
 
           ok(win.document.querySelector("window").className, "busy", "UI is busy");
           yield win.UI._busyPromise;
 
           is(Object.keys(DebuggerServer._connections).length, 1, "Connected");
 
-          yield waitForUpdate(win, "list-tabs-response");
+          yield waitForUpdate(win, "runtime-apps-found");
 
           ok(isPlayActive(), "play button is enabled 1");
           ok(!isStopActive(), "stop button is disabled 1");
           let oldProject = win.AppManager.selectedProject;
           win.AppManager.selectedProject = null;
 
           yield nextTick();
 
@@ -109,17 +109,17 @@
           is(Object.keys(DebuggerServer._connections).length, 0, "Disconnected");
 
           ok(win.AppManager.selectedProject, "A project is still selected");
           ok(!isPlayActive(), "play button is disabled 4");
           ok(!isStopActive(), "stop button is disabled 4");
 
           win.document.querySelectorAll(".runtime-panel-item-other")[1].click();
 
-          yield waitForUpdate(win, "list-tabs-response");
+          yield waitForUpdate(win, "runtime-apps-found");
 
           is(Object.keys(DebuggerServer._connections).length, 1, "Locally connected");
 
           ok(win.AppManager.isMainProcessDebuggable(), "Main process available");
 
           // Select main process
           yield win.Cmds.showProjectPanel();
           SimpleTest.executeSoon(() => {