Fix a couple of races around toolbox opening and tool selection by waiting for the actual tool panel to be selected (bug 988408). r=jwalker
authorPanos Astithas <past@mozilla.com>
Thu, 26 Jun 2014 18:01:02 +0300
changeset 191169 afb676164612507dc4e7af8568350fe806fdb490
parent 191168 af969bbd0b136bd9d6d550f77a09e79b2edfdf09
child 191170 7f1912a46efe6ee6aac69f77d8e37affe092445a
push id27035
push userkwierso@gmail.com
push dateFri, 27 Jun 2014 23:52:01 +0000
treeherdermozilla-central@f127b565e53f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwalker
bugs988408
milestone33.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
Fix a couple of races around toolbox opening and tool selection by waiting for the actual tool panel to be selected (bug 988408). r=jwalker
browser/devtools/framework/gDevTools.jsm
browser/devtools/framework/test/browser_devtools_api.js
browser/devtools/framework/test/browser_new_activation_workflow.js
browser/devtools/framework/test/browser_toolbox_tabsswitch_shortcuts.js
browser/devtools/framework/test/browser_toolbox_window_shortcuts.js
browser/devtools/webconsole/hudservice.js
--- a/browser/devtools/framework/gDevTools.jsm
+++ b/browser/devtools/framework/gDevTools.jsm
@@ -272,19 +272,20 @@ DevTools.prototype = {
       this._toolboxes.set(target, toolbox);
 
       toolbox.once("destroyed", () => {
         this._toolboxes.delete(target);
         this.emit("toolbox-destroyed", target);
       });
 
       // If we were asked for a specific tool then we need to wait for the
-      // tool to be ready, otherwise we can just wait for toolbox open
+      // tool to be ready and selected, otherwise we can just wait for the
+      // toolbox open promise.
       if (toolId != null) {
-        toolbox.once(toolId + "-ready", (event, panel) => {
+        toolbox.once(toolId + "-selected", (event, panel) => {
           this.emit("toolbox-ready", toolbox);
           deferred.resolve(toolbox);
         });
         toolbox.open();
       }
       else {
         toolbox.open().then(() => {
           deferred.resolve(toolbox);
--- a/browser/devtools/framework/test/browser_devtools_api.js
+++ b/browser/devtools/framework/test/browser_devtools_api.js
@@ -34,23 +34,17 @@ function runTests(aTab) {
     "The tool is not registered");
 
   gDevTools.registerTool(toolDefinition);
   is(gDevTools.getToolDefinitionMap().has(toolId), true,
     "The tool is registered");
 
   let target = TargetFactory.forTab(gBrowser.selectedTab);
 
-  gDevTools.showToolbox(target, toolId).then(function(toolbox) {
-    // Wait for the test tool to be visible and selected.
-    let { promise: testToolShown, resolve } = promise.defer();
-    toolbox.once("test-tool-selected", resolve);
-
-    return testToolShown.then(() => toolbox);
-  }).then(function (toolbox) {
+  gDevTools.showToolbox(target, toolId).then(function (toolbox) {
     is(toolbox.target, target, "toolbox target is correct");
     is(toolbox._host.hostTab, gBrowser.selectedTab, "toolbox host is correct");
     continueTests(toolbox);
   });
 }
 
 function continueTests(toolbox, panel) {
   ok(toolbox.getCurrentPanel(), "panel value is correct");
--- a/browser/devtools/framework/test/browser_new_activation_workflow.js
+++ b/browser/devtools/framework/test/browser_new_activation_workflow.js
@@ -48,20 +48,18 @@ function selectAndCheckById(id) {
 }
 
 function testToggle() {
   toolbox.once("destroyed", () => {
     // Cannot reuse a target after it's destroyed.
     target = TargetFactory.forTab(gBrowser.selectedTab);
     gDevTools.showToolbox(target, "styleeditor").then(function(aToolbox) {
       toolbox = aToolbox;
-      aToolbox.once("styleeditor-selected", () => {
-        is(toolbox.currentToolId, "styleeditor", "The style editor is selected");
-        finishUp();
-      });
+      is(toolbox.currentToolId, "styleeditor", "The style editor is selected");
+      finishUp();
     });
   });
 
   toolbox.destroy();
 }
 
 function finishUp() {
   toolbox.destroy().then(function() {
--- a/browser/devtools/framework/test/browser_toolbox_tabsswitch_shortcuts.js
+++ b/browser/devtools/framework/test/browser_toolbox_tabsswitch_shortcuts.js
@@ -19,17 +19,19 @@ function test() {
                   .map(def => def.id);
       gDevTools.showToolbox(target, toolIDs[0], Toolbox.HostType.BOTTOM)
                .then(testShortcuts);
     });
   });
 }
 
 function testShortcuts(aToolbox, aIndex) {
-  if (aIndex == toolIDs.length) {
+  if (aIndex === undefined) {
+    aIndex = 1;
+  } else if (aIndex == toolIDs.length) {
     aIndex = 0;
     if (secondTime) {
       secondTime = false;
       reverse = true;
       aIndex = toolIDs.length - 2;
     }
     else {
       secondTime = true;
@@ -50,30 +52,24 @@ function testShortcuts(aToolbox, aIndex)
                      .getAttribute("key");
     prevKey = toolbox.doc.getElementById("toolbox-previous-tool-key")
                      .getAttribute("key");
   }
   info("Toolbox fired a `ready` event");
 
   toolbox.once("select", onSelect);
 
-  if (aIndex != null) {
-    // This if block is to allow the call of onSelect without shortcut press for
-    // the first time. That happens because on opening of toolbox, one tool gets
-    // selected atleast.
-
-    let key = (reverse ? prevKey: nextKey);
-    let modifiers = {
-      accelKey: true
-    };
-    idIndex = aIndex;
-    info("Testing shortcut to switch to tool " + aIndex + ":" + toolIDs[aIndex] +
-         " using key " + key);
-    EventUtils.synthesizeKey(key, modifiers, toolbox.doc.defaultView);
-  }
+  let key = (reverse ? prevKey: nextKey);
+  let modifiers = {
+    accelKey: true
+  };
+  idIndex = aIndex;
+  info("Testing shortcut to switch to tool " + aIndex + ":" + toolIDs[aIndex] +
+       " using key " + key);
+  EventUtils.synthesizeKey(key, modifiers, toolbox.doc.defaultView);
 }
 
 function onSelect(event, id) {
   info("toolbox-select event from " + id);
 
   is(toolIDs.indexOf(id), idIndex,
      "Correct tool is selected on pressing the shortcut for " + id);
   // Execute soon to reset the stack trace.
--- a/browser/devtools/framework/test/browser_toolbox_window_shortcuts.js
+++ b/browser/devtools/framework/test/browser_toolbox_window_shortcuts.js
@@ -36,43 +36,39 @@ function test() {
     let target = TargetFactory.forTab(gBrowser.selectedTab);
     idIndex = 0;
     gDevTools.showToolbox(target, toolIDs[0], Toolbox.HostType.WINDOW)
              .then(testShortcuts);
   });
 }
 
 function testShortcuts(aToolbox, aIndex) {
-  if (aIndex == toolIDs.length) {
+  if (aIndex === undefined) {
+    aIndex = 1;
+  } else if (aIndex == toolIDs.length) {
     tidyUp();
     return;
   }
 
   toolbox = aToolbox;
   info("Toolbox fired a `ready` event");
 
   toolbox.once("select", selectCB);
 
-  if (aIndex != null) {
-    // This if block is to allow the call of selectCB without shortcut press for
-    // the first time. That happens because on opening of toolbox, one tool gets
-    // selected atleast.
-
-    let key = gDevTools._tools.get(toolIDs[aIndex]).key;
-    let toolModifiers = gDevTools._tools.get(toolIDs[aIndex]).modifiers;
-    let modifiers = {
-      accelKey: toolModifiers.contains("accel"),
-      altKey: toolModifiers.contains("alt"),
-      shiftKey: toolModifiers.contains("shift"),
-    };
-    idIndex = aIndex;
-    info("Testing shortcut for tool " + aIndex + ":" + toolIDs[aIndex] +
-         " using key " + key);
-    EventUtils.synthesizeKey(key, modifiers, toolbox.doc.defaultView.parent);
-  }
+  let key = gDevTools._tools.get(toolIDs[aIndex]).key;
+  let toolModifiers = gDevTools._tools.get(toolIDs[aIndex]).modifiers;
+  let modifiers = {
+    accelKey: toolModifiers.contains("accel"),
+    altKey: toolModifiers.contains("alt"),
+    shiftKey: toolModifiers.contains("shift"),
+  };
+  idIndex = aIndex;
+  info("Testing shortcut for tool " + aIndex + ":" + toolIDs[aIndex] +
+       " using key " + key);
+  EventUtils.synthesizeKey(key, modifiers, toolbox.doc.defaultView.parent);
 }
 
 function selectCB(event, id) {
   info("toolbox-select event from " + id);
 
   is(toolIDs.indexOf(id), idIndex,
      "Correct tool is selected on pressing the shortcut for " + id);
 
--- a/browser/devtools/webconsole/hudservice.js
+++ b/browser/devtools/webconsole/hudservice.js
@@ -487,18 +487,18 @@ WebConsole.prototype = {
     let showSource = ({ DebuggerView }) => {
       if (DebuggerView.Sources.containsValue(aSourceURL)) {
         DebuggerView.setEditorLocation(aSourceURL, aSourceLine,
                                        { noDebug: true }).then(() => {
           this.ui.emit("source-in-debugger-opened");
         });
         return;
       }
-      toolbox.selectTool("webconsole");
-      this.viewSource(aSourceURL, aSourceLine);
+      toolbox.selectTool("webconsole")
+             .then(() => this.viewSource(aSourceURL, aSourceLine));
     }
 
     // If the Debugger was already open, switch to it and try to show the
     // source immediately. Otherwise, initialize it and wait for the sources
     // to be added first.
     let debuggerAlreadyOpen = toolbox.getPanel("jsdebugger");
     toolbox.selectTool("jsdebugger").then(({ panelWin: dbg }) => {
       if (debuggerAlreadyOpen) {