Bug 1328004 - Fix race when closing the devtools while a previous instance is still loading. r=jryans
MozReview-Commit-ID: 2a58l4DjCtv
--- a/devtools/client/framework/devtools-browser.js
+++ b/devtools/client/framework/devtools-browser.js
@@ -67,17 +67,17 @@ var gDevToolsBrowser = exports.gDevTools
toggleToolboxCommand: function (gBrowser) {
let target = TargetFactory.forTab(gBrowser.selectedTab);
let toolbox = gDevTools.getToolbox(target);
// If a toolbox exists, using toggle from the Main window :
// - should close a docked toolbox
// - should focus a windowed toolbox
let isDocked = toolbox && toolbox.hostType != Toolbox.HostType.WINDOW;
- isDocked ? toolbox.destroy() : gDevTools.showToolbox(target);
+ isDocked ? gDevTools.closeToolbox(target) : gDevTools.showToolbox(target);
},
/**
* This function ensures the right commands are enabled in a window,
* depending on their relevant prefs. It gets run when a window is registered,
* or when any of the devtools prefs change.
*/
updateCommandAvailability: function (win) {
@@ -188,17 +188,17 @@ var gDevToolsBrowser = exports.gDevTools
(toolbox.currentToolId == toolId ||
(toolId == "webconsole" && toolbox.splitConsole)))
{
toolbox.fireCustomKey(toolId);
if (toolDefinition.preventClosingOnKey || toolbox.hostType == Toolbox.HostType.WINDOW) {
toolbox.raise();
} else {
- toolbox.destroy();
+ gDevTools.closeToolbox(target);
}
gDevTools.emit("select-tool-command", toolId);
} else {
gDevTools.showToolbox(target, toolId).then(() => {
let target = TargetFactory.forTab(gBrowser.selectedTab);
let toolbox = gDevTools.getToolbox(target);
toolbox.fireCustomKey(toolId);
--- a/devtools/client/framework/devtools.js
+++ b/devtools/client/framework/devtools.js
@@ -460,23 +460,27 @@ DevTools.prototype = {
/**
* Close the toolbox for a given target
*
* @return promise
* This promise will resolve to false if no toolbox was found
* associated to the target. true, if the toolbox was successfully
* closed.
*/
- closeToolbox: function DT_closeToolbox(target) {
- let toolbox = this._toolboxes.get(target);
- if (toolbox == null) {
- return promise.resolve(false);
+ closeToolbox: Task.async(function* (target) {
+ let toolbox = yield this._creatingToolboxes.get(target);
+ if (!toolbox) {
+ toolbox = this._toolboxes.get(target);
}
- return toolbox.destroy().then(() => true);
- },
+ if (!toolbox) {
+ return false;
+ }
+ yield toolbox.destroy();
+ return true;
+ }),
/**
* Either the SDK Loader has been destroyed by the add-on contribution
* workflow, or firefox is shutting down.
* @param {boolean} shuttingDown
* True if firefox is currently shutting down. We may prevent doing
* some cleanups to speed it up. Otherwise everything need to be
--- a/devtools/client/framework/test/browser.ini
+++ b/devtools/client/framework/test/browser.ini
@@ -55,16 +55,17 @@ skip-if = true # Bug 1177463 - Temporari
[browser_toolbox_options.js]
[browser_toolbox_options_disable_buttons.js]
[browser_toolbox_options_disable_cache-01.js]
[browser_toolbox_options_disable_cache-02.js]
[browser_toolbox_options_disable_js.js]
[browser_toolbox_options_enable_serviceworkers_testing.js]
# [browser_toolbox_raise.js] # Bug 962258
# skip-if = os == "win"
+[browser_toolbox_races.js]
[browser_toolbox_ready.js]
[browser_toolbox_remoteness_change.js]
run-if = e10s
[browser_toolbox_select_event.js]
skip-if = e10s # Bug 1069044 - destroyInspector may hang during shutdown
[browser_toolbox_selected_tool_unavailable.js]
[browser_toolbox_sidebar.js]
[browser_toolbox_sidebar_events.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/framework/test/browser_toolbox_races.js
@@ -0,0 +1,81 @@
+/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim: set ft=javascript ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Test toggling the toolbox quickly and see if there is any race breaking it.
+
+const URL = "data:text/html;charset=utf-8,Toggling devtools quickly";
+
+add_task(function* () {
+ // Make sure this test starts with the selectedTool pref cleared. Previous
+ // tests select various tools, and that sets this pref.
+ Services.prefs.clearUserPref("devtools.toolbox.selectedTool");
+
+ let tab = yield addTab(URL);
+
+ let created = 0, ready = 0, destroy = 0, destroyed = 0;
+ let onCreated = () => {
+ created++;
+ };
+ let onReady = () => {
+ ready++;
+ };
+ let onDestroy = () => {
+ destroy++;
+ };
+ let onDestroyed = () => {
+ destroyed++;
+ };
+ gDevTools.on("toolbox-created", onCreated);
+ gDevTools.on("toolbox-ready", onReady);
+ gDevTools.on("toolbox-destroy", onDestroy);
+ gDevTools.on("toolbox-destroyed", onDestroyed);
+
+ // The current implementation won't toggle the toolbox many times,
+ // instead it will ignore toggles that happens while the toolbox is still
+ // creating or still destroying.
+
+ // Toggle the toolbox at least 3 times.
+ info("Trying to toggle the toolbox 3 times");
+ while (created < 3) {
+ // Sent multiple event to try to race the code during toolbox creation and destruction
+ toggle();
+ toggle();
+ toggle();
+
+ // Release the event loop to let a chance to actually create or destroy the toolbox!
+ yield wait(50);
+ }
+ info("Toggled the toolbox 3 times");
+
+ // Now wait for the 3rd toolbox to be fully ready before closing it.
+ // We close the last toolbox manually, out of the first while() loop to
+ // avoid races and be sure we end up we no toolbox and waited for all the
+ // requests to be done.
+ while (ready != 3) {
+ yield wait(100);
+ }
+ toggle();
+ while (destroyed != 3) {
+ yield wait(100);
+ }
+
+ is(created, 3, "right number of created events");
+ is(ready, 3, "right number of ready events");
+ is(destroy, 3, "right number of destroy events");
+ is(destroyed, 3, "right number of destroyed events");
+
+ gDevTools.off("toolbox-created", onCreated);
+ gDevTools.off("toolbox-ready", onReady);
+ gDevTools.off("toolbox-destroy", onDestroy);
+ gDevTools.off("toolbox-destroyed", onDestroyed);
+
+ gBrowser.removeCurrentTab();
+});
+
+function toggle() {
+ EventUtils.synthesizeKey("VK_F12", {});
+}