Bug 1311737 - Prevent toolbox.selectTool from racing when calling multiple times. r=pbro
MozReview-Commit-ID: lZZGviAFl3
--- a/devtools/client/framework/test/browser_toolbox_select_event.js
+++ b/devtools/client/framework/test/browser_toolbox_select_event.js
@@ -38,39 +38,64 @@ add_task(function* () {
yield testSelectEvent("inspector");
yield testSelectEvent("webconsole");
yield testSelectEvent("styleeditor");
yield testSelectEvent("inspector");
yield testSelectEvent("webconsole");
yield testSelectEvent("styleeditor");
yield toolbox.destroy();
+ yield testSelectToolRace();
+
/**
* Assert that selecting the given toolId raises a select event
* @param {toolId} Id of the tool to test
*/
- function testSelectEvent(toolId) {
- return new Promise(resolve => {
- toolbox.once("select", (event, id) => {
- is(id, toolId, toolId + " selected");
- resolve();
- });
- toolbox.selectTool(toolId);
- });
+ function* testSelectEvent(toolId) {
+ let onSelect = toolbox.once("select");
+ toolbox.selectTool(toolId);
+ let id = yield onSelect;
+ is(id, toolId, toolId + " selected");
}
/**
* Assert that selecting the given toolId raises its corresponding
* selected event
* @param {toolId} Id of the tool to test
*/
- function testToolSelectEvent(toolId) {
- return new Promise(resolve => {
- toolbox.once(toolId + "-selected", () => {
- let msg = toolId + " tool selected";
- is(toolbox.currentToolId, toolId, msg);
- resolve();
- });
- toolbox.selectTool(toolId);
- });
+ function* testToolSelectEvent(toolId) {
+ let onSelected = toolbox.once(toolId + "-selected");
+ toolbox.selectTool(toolId);
+ yield onSelected;
+ is(toolbox.currentToolId, toolId, toolId + " tool selected");
+ }
+
+ /**
+ * Assert that two calls to selectTool won't race
+ */
+ function* testSelectToolRace() {
+ let toolbox = yield openToolboxForTab(tab, "webconsole");
+ let selected = false;
+ let onSelect = (event, id) => {
+ if (selected) {
+ ok(false, "Got more than one 'select' event");
+ } else {
+ selected = true;
+ }
+ };
+ toolbox.once("select", onSelect);
+ let p1 = toolbox.selectTool("inspector")
+ let p2 = toolbox.selectTool("inspector");
+ // Check that both promises don't resolve too early
+ let checkSelectToolResolution = panel => {
+ ok(selected, "selectTool resolves only after 'select' event is fired");
+ let inspector = toolbox.getPanel("inspector");
+ is(panel, inspector, "selecTool resolves to the panel instance");
+ };
+ p1.then(checkSelectToolResolution);
+ p2.then(checkSelectToolResolution);
+ yield p1;
+ yield p2;
+
+ yield toolbox.destroy();
}
});
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -1446,21 +1446,29 @@ Toolbox.prototype = {
let sep = this.doc.getElementById("toolbox-controls-separator");
if (id === "options") {
sep.setAttribute("invisible", "true");
} else {
sep.removeAttribute("invisible");
}
if (this.currentToolId == id) {
- // re-focus tool to get key events again
- this.focusTool(id);
+ let panel = this._toolPanels.get(id);
+ if (panel) {
+ // We have a panel instance, so the tool is already fully loaded.
+
+ // re-focus tool to get key events again
+ this.focusTool(id);
- // Return the existing panel in order to have a consistent return value.
- return promise.resolve(this._toolPanels.get(id));
+ // Return the existing panel in order to have a consistent return value.
+ return promise.resolve(panel);
+ }
+ // Otherwise, if there is no panel instance, it is still loading,
+ // so we are racing another call to selectTool with the same id.
+ return this.once("select").then(() => promise.resolve(this._toolPanels.get(id)));
}
if (!this.isReady) {
throw new Error("Can't select tool, wait for toolbox 'ready' event");
}
let tab = this.doc.getElementById("toolbox-tab-" + id);