Bug 1266134 - address review for: Pull host management out of toolbox.xul.fix pull out host. r=jryans
MozReview-Commit-ID: 3rh0kzfwUlp
--- a/devtools/client/framework/devtools.js
+++ b/devtools/client/framework/devtools.js
@@ -5,25 +5,26 @@
"use strict";
const Services = require("Services");
const promise = require("promise");
const defer = require("devtools/shared/defer");
// Load gDevToolsBrowser toolbox lazily as they need gDevTools to be fully initialized
loader.lazyRequireGetter(this, "Toolbox", "devtools/client/framework/toolbox", true);
-loader.lazyRequireGetter(this, "ToolboxWrapper", "devtools/client/framework/toolbox-wrapper", true);
+loader.lazyRequireGetter(this, "ToolboxBrowserIntegration", "devtools/client/framework/toolbox-browser-integration", true);
loader.lazyRequireGetter(this, "gDevToolsBrowser", "devtools/client/framework/devtools-browser", true);
const {defaultTools: DefaultTools, defaultThemes: DefaultThemes} =
require("devtools/client/definitions");
const EventEmitter = require("devtools/shared/event-emitter");
const {JsonView} = require("devtools/client/jsonview/main");
const AboutDevTools = require("devtools/client/framework/about-devtools-toolbox");
const {when: unload} = require("sdk/system/unload");
+const {Task} = require("devtools/shared/task");
const FORBIDDEN_IDS = new Set(["toolbox", ""]);
const MAX_ORDINAL = 99;
/**
* DevTools is a class that represents a set of developer tools, it holds a
* set of tools and keeps track of open toolboxes in the browser.
*/
@@ -393,64 +394,52 @@ DevTools.prototype = {
* @param {Toolbox.HostType} hostType
* The type of host (bottom, window, side)
* @param {object} hostOptions
* Options for host specifically
*
* @return {Toolbox} toolbox
* The toolbox that was opened
*/
- showToolbox: function (target, toolId, hostType, hostOptions) {
- let deferred = defer();
-
+ showToolbox: Task.async(function* (target, toolId, hostType, hostOptions) {
let toolbox = this._toolboxes.get(target);
if (toolbox) {
- let hostPromise = (hostType != null && toolbox.hostType != hostType) ?
- toolbox.switchHost(hostType) :
- promise.resolve(null);
+ if (hostType != null && toolbox.hostType != hostType) {
+ yield toolbox.switchHost(hostType);
+ }
if (toolId != null && toolbox.currentToolId != toolId) {
- hostPromise = hostPromise.then(function () {
- return toolbox.selectTool(toolId);
- });
+ yield toolbox.selectTool(toolId);
}
- return hostPromise.then(function () {
- toolbox.raise();
- return toolbox;
- });
- }
- else {
- let wrapper = new ToolboxWrapper(target, hostType, hostOptions);
+ toolbox.raise();
+ } else {
+ let wrapper = new ToolboxBrowserIntegration(target, hostType, hostOptions);
- wrapper.create(toolId)
- .then(toolbox => {
- this._toolboxes.set(target, toolbox);
+ toolbox = yield wrapper.create(toolId);
+ this._toolboxes.set(target, toolbox);
- this.emit("toolbox-created", toolbox);
+ this.emit("toolbox-created", toolbox);
- toolbox.once("destroy", () => {
- this.emit("toolbox-destroy", target);
- });
+ toolbox.once("destroy", () => {
+ this.emit("toolbox-destroy", target);
+ });
- toolbox.once("destroyed", () => {
- this._toolboxes.delete(target);
- this.emit("toolbox-destroyed", target);
- });
+ toolbox.once("destroyed", () => {
+ this._toolboxes.delete(target);
+ this.emit("toolbox-destroyed", target);
+ });
- toolbox.open().then(() => {
- deferred.resolve(toolbox);
- this.emit("toolbox-ready", toolbox);
- });
- });
+ yield toolbox.open();
+ this.emit("toolbox-ready", toolbox);
}
- return deferred.promise;
- },
+ return toolbox;
+ }),
/**
* Return the toolbox for a given target.
*
* @param {object} target
* Target value e.g. the target that owns this toolbox
*
* @return {Toolbox} toolbox
--- a/devtools/client/framework/moz.build
+++ b/devtools/client/framework/moz.build
@@ -19,15 +19,15 @@ DevToolsModules(
'location-store.js',
'menu-item.js',
'menu.js',
'selection.js',
'sidebar.js',
'source-map-service.js',
'target-from-url.js',
'target.js',
+ 'toolbox-browser-integration.js',
'toolbox-highlighter-utils.js',
'toolbox-hosts.js',
'toolbox-options.js',
- 'toolbox-wrapper.js',
'toolbox.js',
'ToolboxProcess.jsm',
)
new file mode 100644
--- /dev/null
+++ b/devtools/client/framework/toolbox-browser-integration.js
@@ -0,0 +1,217 @@
+const Services = require("Services");
+const { Ci } = require("chrome");
+const {LocalizationHelper} = require("devtools/shared/l10n");
+const L10N = new LocalizationHelper("devtools/locale/toolbox.properties");
+const DevToolsUtils = require("devtools/shared/DevToolsUtils");
+const {Task} = require("devtools/shared/task");
+
+loader.lazyRequireGetter(this, "Toolbox", "devtools/client/framework/toolbox", true);
+loader.lazyRequireGetter(this, "Hosts", "devtools/client/framework/toolbox-hosts", true);
+
+/**
+ * Implement a wrapper on the chrome side to setup a Toolbox within Firefox UI.
+ *
+ * This component handles iframe creation within Firefox, in which we are loading
+ * the toolbox document. Then both the chrome and the toolbox document communicate
+ * via "message" events.
+ *
+ * Messages sent by the toolbox to the chrome:
+ * - switch-host: order to display the toolbox in another host (side, bottom
+ * or window)
+ * - toggle-minimize-mode: when using the bottom host, the toolbox can be
+ * miximized to only display the tool titles
+ * - maximize-host: when using the bottom host in minimized mode, revert back
+ * to regular mode in order to see tool titles and the tools
+ * - raise-host: focus the tools
+ * - set-host-title: when using the window host, update the window title
+ *
+ * Messages sent by the chrome to the toolbox:
+ * - host-minimized: the bottom host is done minimizing (after animation end)
+ * - host-maximized: the bottom host is done switching back to regular mode
+ * (after animation end)
+ * - switched-host: the `switch-host` command sent by the toolbox is done
+ */
+
+const LAST_HOST = "devtools.toolbox.host";
+let ID_COUNTER = 1;
+
+function ToolboxBrowserIntegration(target, hostType, hostOptions) {
+ this.target = target;
+
+ this.frameId = ID_COUNTER++;
+
+ if (!hostType) {
+ hostType = Services.prefs.getCharPref(LAST_HOST);
+ }
+ this.onHostMinimized = this.onHostMinimized.bind(this);
+ this.onHostMaximized = this.onHostMaximized.bind(this);
+ this.host = this.createHost(hostType, hostOptions);
+}
+
+ToolboxBrowserIntegration.prototype = {
+ create: Task.async(function* (toolId) {
+ yield this.host.create();
+
+ this.host.frame.setAttribute("aria-label", L10N.getStr("toolbox.label"));
+ this.host.frame.ownerDocument.defaultView.addEventListener("message", this);
+ this.host.frame.addEventListener("unload", this, true);
+
+ let toolbox = new Toolbox(this.target, toolId, this.host.type, this.host.frame.contentWindow, this.frameId);
+
+ // Prevent reloading the toolbox when loading the tools in a tab (e.g. from about:debugging)
+ if (!this.host.frame.contentWindow.location.href.startsWith("about:devtools-toolbox")) {
+ this.host.frame.setAttribute("src", "about:devtools-toolbox");
+ }
+
+ return toolbox;
+ }),
+
+ handleEvent(event) {
+ switch(event.type) {
+ case "message":
+ this.onMessage(event);
+ break;
+ case "unload":
+ // On unload, host iframe already lost its contentWindow attribute, so
+ // we can only compare against locations. Here we filter two very
+ // different cases: preliminary about:blank document as well as iframes
+ // like tool iframes.
+ if (!event.target.location.href.startsWith("about:devtools-toolbox")) {
+ break;
+ }
+ // Don't destroy the host during unload event (esp., don't remove the
+ // iframe from DOM!). Otherwise the unload event for the toolbox
+ // document doesn't fire within the toolbox *document*! (here this is
+ // the unload event that fires on the toolbox *iframe*.
+ DevToolsUtils.executeSoon(() => {
+ this.destroy();
+ });
+ break;
+ }
+ },
+
+ onMessage(event) {
+ if (!event.data) {
+ return;
+ }
+ // Toolbox document is still chrome and disallow identifying message
+ // origin via event.source as it is null. So use a custom id.
+ if (event.data.frameId != this.frameId) {
+ return;
+ }
+ switch (event.data.name) {
+ case "switch-host":
+ this.switchHost(event.data.hostType);
+ break;
+ case "maximize-host":
+ this.host.maximize();
+ break;
+ case "raise-host":
+ this.host.raise();
+ break;
+ case "toggle-minimize-mode":
+ this.host.toggleMinimizeMode(event.data.toolbarHeight);
+ break;
+ case "set-host-title":
+ this.host.setTitle(event.data.title);
+ break;
+ }
+ },
+
+ postMessage(data) {
+ let window = this.host.frame.contentWindow;
+ window.postMessage(data, "*");
+ },
+
+ destroy() {
+ this.destroyHost();
+ this.host = null;
+ this.target = null;
+ },
+
+ /**
+ * Create a host object based on the given host type.
+ *
+ * Warning: bottom and sidebar hosts require that the toolbox target provides
+ * a reference to the attached tab. Not all Targets have a tab property -
+ * make sure you correctly mix and match hosts and targets.
+ *
+ * @param {string} hostType
+ * The host type of the new host object
+ *
+ * @return {Host} host
+ * The created host object
+ */
+ createHost(hostType, options) {
+ if (!Hosts[hostType]) {
+ throw new Error("Unknown hostType: " + hostType);
+ }
+
+ let newHost = new Hosts[hostType](this.target.tab, options);
+ // Update the label and icon when the state changes.
+ newHost.on("minimized", this.onHostMinimized);
+ newHost.on("maximized", this.onHostMaximized);
+ return newHost;
+ },
+
+ onHostMinimized() {
+ this.postMessage({
+ name: "host-minimized"
+ });
+ },
+
+ onHostMaximized() {
+ this.postMessage({
+ name: "host-maximized"
+ });
+ },
+
+ switchHost: Task.async(function* (hostType) {
+ let iframe = this.host.frame;
+ let newHost = this.createHost(hostType);
+ let newIframe = yield newHost.create();
+ // change toolbox document's parent to the new host
+ newIframe.swapFrameLoaders(iframe);
+
+ // See bug 1022726, most probably because of swapFrameLoaders we need to
+ // first focus the window here, and then once again further from
+ // toolbox.js to make sure focus actually happens.
+ iframe.contentWindow.focus();
+
+ this.destroyHost();
+
+ this.host = newHost;
+ this.host.setTitle(this.host.frame.contentWindow.document.title);
+ this.host.frame.ownerDocument.defaultView.addEventListener("message", this);
+ this.host.frame.addEventListener("unload", this, true);
+
+ if (hostType != Toolbox.HostType.CUSTOM) {
+ Services.prefs.setCharPref(LAST_HOST, hostType);
+ }
+
+ // Tell the toolbox the host changed
+ this.postMessage({
+ name: "switched-host",
+ hostType
+ });
+ }),
+
+ /**
+ * Destroy the current host, and remove event listeners from its frame.
+ *
+ * @return {promise} to be resolved when the host is destroyed.
+ */
+ destroyHost() {
+ // When Firefox toplevel is closed, the frame may already be detached and
+ // the top level document gone
+ if (this.host.frame.ownerDocument.defaultView) {
+ this.host.frame.ownerDocument.defaultView.removeEventListener("message", this);
+ }
+ this.host.frame.removeEventListener("unload", this, true);
+
+ this.host.off("minimized", this.onHostMinimized);
+ this.host.off("maximized", this.onHostMaximized);
+ return this.host.destroy();
+ }
+};
+exports.ToolboxBrowserIntegration = ToolboxBrowserIntegration;
deleted file mode 100644
--- a/devtools/client/framework/toolbox-wrapper.js
+++ /dev/null
@@ -1,198 +0,0 @@
-const Services = require("Services");
-const { Ci } = require("chrome");
-const {LocalizationHelper} = require("devtools/shared/l10n");
-const L10N = new LocalizationHelper("devtools/locale/toolbox.properties");
-
-loader.lazyRequireGetter(this, "Toolbox", "devtools/client/framework/toolbox", true);
-loader.lazyRequireGetter(this, "Hosts", "devtools/client/framework/toolbox-hosts", true);
-
-/**
- * Implement a wrapper on the chrome side to setup a Toolbox within Firefox UI.
- *
- * This components handles iframe creation within Firefox, in which we are loading
- * the toolbox document. Then both the chrome and the toolbox document communicate
- * via "message" events.
- *
- * Messages sent by the toolbox to the chrome:
- * - switch-host: order to display the toolbox in another host (side, bottom or window)
- *
- * Messages sent by the chrome to the toolbox:
- * - host-will-change: tells the toolbox document that the host is about to change
- */
-
-const LAST_HOST = "devtools.toolbox.host";
-let ID_COUNTER = 1;
-
-function ToolboxWrapper(target, hostType, hostOptions) {
- this.target = target;
-
- this.frameId = ID_COUNTER++;
-
- if (!hostType) {
- hostType = Services.prefs.getCharPref(LAST_HOST);
- }
- this.onHostMinimized = this.onHostMinimized.bind(this);
- this.onHostMaximized = this.onHostMaximized.bind(this);
- this.host = this.createHost(hostType, hostOptions);
-}
-
-ToolboxWrapper.prototype = {
- create(toolId) {
- return this.host.create()
- .then(() => {
- this.host.frame.setAttribute("aria-label", L10N.getStr("toolbox.label"));
- this.host.frame.ownerDocument.defaultView.addEventListener("message", this);
- this.host.frame.addEventListener("unload", this, true);
-
- let toolbox = new Toolbox(this.target, toolId, this.host.type, this.host.frame.contentWindow, this.frameId);
-
- // Prevent reloading the toolbox when loading the tools in a tab (e.g. from about:debugging)
- if (!this.host.frame.contentWindow.location.href.startsWith("about:devtools-toolbox")) {
- this.host.frame.setAttribute("src", "about:devtools-toolbox");
- }
-
- return toolbox;
- });
- },
-
- handleEvent(event) {
- switch(event.type) {
- case "message":
- this.onMessage(event);
- break;
- case "unload":
- // On unload, host iframe already lost its contentWindow attribute, so
- // we can only compare against locations. Here we filter two very
- // different cases: preliminary about:blank document as well as iframes
- // like tool iframes.
- if (!event.target.location.href.startsWith("about:devtools-toolbox")) {
- break;
- }
- this.destroy();
- break;
- }
- },
-
- onMessage(event) {
- if (!event.data) return;
- // Toolbox document is still chrome and disallow identifying message
- // origin via event.source as it is null. So use a custom id.
- if (event.data.frameId != this.frameId) {
- return;
- }
- switch (event.data.name) {
- case "switch-host":
- this.switchHost(event.data.hostType);
- break;
- case "maximize-host":
- this.host.maximize();
- break;
- case "raise-host":
- this.host.raise();
- break;
- case "toggle-minimize-mode":
- this.host.toggleMinimizeMode(event.data.toolbarHeight);
- break;
- case "set-host-title":
- this.host.setTitle(event.data.title);
- break;
- case "destroy-host":
- this.destroy();
- break;
- }
- },
-
- postMessage(data) {
- let window = this.host.frame.contentWindow;
- window.postMessage(data, "*");
- },
-
- destroy() {
- this.destroyHost();
- this.host = null;
- this.target = null;
- },
-
- /**
- * Create a host object based on the given host type.
- *
- * Warning: some hosts require that the toolbox target provides a reference to
- * the attached tab. Not all Targets have a tab property - make sure you
- * correctly mix and match hosts and targets.
- *
- * @param {string} hostType
- * The host type of the new host object
- *
- * @return {Host} host
- * The created host object
- */
- createHost(hostType, options) {
- if (!Hosts[hostType]) {
- throw new Error("Unknown hostType: " + hostType);
- }
-
- let newHost = new Hosts[hostType](this.target.tab, options);
- // Update the label and icon when the state changes.
- newHost.on("minimized", this.onHostMinimized);
- newHost.on("maximized", this.onHostMaximized);
- return newHost;
- },
-
- onHostMinimized() {
- this.postMessage({
- name: "host-minimized"
- });
- },
- onHostMaximized() {
- this.postMessage({
- name: "host-maximized"
- });
- },
-
- switchHost(hostType) {
- let iframe = this.host.frame;
- let newHost = this.createHost(hostType);
- return newHost.create().then(newIframe => {
- // change toolbox document's parent to the new host
- newIframe.QueryInterface(Ci.nsIFrameLoaderOwner);
- newIframe.swapFrameLoaders(iframe);
-
- // See bug 1022726, most probably because of swapFrameLoaders we need to
- // first focus the window here, and then once again further from
- // toolbox.js to make sure focus actually happens.
- iframe.contentWindow.focus();
-
- this.destroyHost();
-
- this.host = newHost;
- this.host.setTitle(this.host.frame.contentWindow.document.title);
- this.host.frame.ownerDocument.defaultView.addEventListener("message", this);
- this.host.frame.addEventListener("unload", this, true);
-
- if (hostType != Toolbox.HostType.CUSTOM) {
- Services.prefs.setCharPref(LAST_HOST, hostType);
- }
-
- // Tell the toolbox the host changed
- this.postMessage({
- name: "switched-host",
- hostType
- });
- });
- },
-
- /**
- * Destroy the current host, and remove event listeners from its frame.
- *
- * @return {promise} to be resolved when the host is destroyed.
- */
- destroyHost() {
- this.host.frame.ownerDocument.defaultView.removeEventListener("message", this);
- this.host.frame.removeEventListener("unload", this, true);
-
- this.host.off("minimized", this.onHostMinimized);
- this.host.off("maximized", this.onHostMaximized);
- return this.host.destroy();
- }
-};
-exports.ToolboxWrapper = ToolboxWrapper;
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -96,16 +96,21 @@ const ToolboxButtons = exports.ToolboxBu
* the iframes where the tool panels will be living in.
*
* @param {object} target
* The object the toolbox is debugging.
* @param {string} selectedTool
* Tool to select initially
* @param {Toolbox.HostType} hostType
* Type of host that will host the toolbox (e.g. sidebar, window)
+ * @param {DOMWindow} contentWindow
+ * The window object of the toolbox document
+ * @param {string} frameId
+ * A unique identifier to differenciate toolbox documents from the
+ * chrome codebase when passing DOM messages
*/
function Toolbox(target, selectedTool, hostType, contentWindow, frameId) {
this._target = target;
this._win = contentWindow;
this.frameId = frameId;
this._toolPanels = new Map();
this._telemetry = new Telemetry();
@@ -129,17 +134,17 @@ function Toolbox(target, selectedTool, h
this._splitConsoleOnKeypress = this._splitConsoleOnKeypress.bind(this);
this.destroy = this.destroy.bind(this);
this.highlighterUtils = getHighlighterUtils(this);
this._highlighterReady = this._highlighterReady.bind(this);
this._highlighterHidden = this._highlighterHidden.bind(this);
this._prefChanged = this._prefChanged.bind(this);
this._saveSplitConsoleHeight = this._saveSplitConsoleHeight.bind(this);
this._onFocus = this._onFocus.bind(this);
- this._onHostMessage = this._onHostMessage.bind(this);
+ this._onBrowserMessage = this._onBrowserMessage.bind(this);
this._showDevEditionPromo = this._showDevEditionPromo.bind(this);
this._updateTextboxMenuItems = this._updateTextboxMenuItems.bind(this);
this._onBottomHostMinimized = this._onBottomHostMinimized.bind(this);
this._onBottomHostMaximized = this._onBottomHostMaximized.bind(this);
this._onToolSelectWhileMinimized = this._onToolSelectWhileMinimized.bind(this);
this._onPerformanceFrontEvent = this._onPerformanceFrontEvent.bind(this);
this._onBottomHostWillChange = this._onBottomHostWillChange.bind(this);
this._toggleMinimizeMode = this._toggleMinimizeMode.bind(this);
@@ -601,47 +606,49 @@ Toolbox.prototype = {
(name, event) => {
this.switchToPreviousHost();
event.preventDefault();
});
this.doc.addEventListener("keypress", this._splitConsoleOnKeypress, false);
this.doc.addEventListener("focus", this._onFocus, true);
this.win.addEventListener("unload", this.destroy);
- this.win.addEventListener("message", this._onHostMessage);
+ this.win.addEventListener("message", this._onBrowserMessage);
},
_removeHostListeners: function () {
// The host iframe's contentDocument may already be gone.
if (this.doc) {
this.doc.removeEventListener("keypress", this._splitConsoleOnKeypress, false);
this.doc.removeEventListener("focus", this._onFocus, true);
this.win.removeEventListener("unload", this.destroy);
- this.win.removeEventListener("message", this._onHostMessage);
+ this.win.removeEventListener("message", this._onBrowserMessage);
}
},
- // Called whenever the host, on the chrome side, send a message
- _onHostMessage: function (event) {
- if (!event.data) return;
+ // Called whenever the chrome send a message
+ _onBrowserMessage: function (event) {
+ if (!event.data) {
+ return;
+ }
switch (event.data.name) {
case "switched-host":
this._onSwitchedHost(event.data);
break;
case "host-minimized":
if (this.hostType == Toolbox.HostType.BOTTOM) {
this._onBottomHostMinimized();
}
break;
case "host-maximized":
if (this.hostType == Toolbox.HostType.BOTTOM) {
this._onBottomHostMaximized();
}
break;
- };
+ }
},
_registerOverlays: function () {
registerHarOverlay(this);
},
_saveSplitConsoleHeight: function () {
Services.prefs.setIntPref(SPLITCONSOLE_HEIGHT_PREF,
@@ -857,17 +864,17 @@ Toolbox.prototype = {
},
_onToolSelectWhileMinimized: function () {
this.postMessage({
name: "maximize-host"
});
},
- postMessage: function(msg) {
+ postMessage: function (msg) {
// We sometime try to send messages in middle of destroy(), where the
// toolbox iframe may already be detached and no longer have a parent.
if (this.win.parent) {
// Toolbox document is still chrome and disallow identifying message
// origin via event.source as it is null. So use a custom id.
msg.frameId = this.frameId;
this.win.parent.postMessage(msg, "*");
}
@@ -2139,23 +2146,25 @@ Toolbox.prototype = {
// Finish all outstanding tasks (which means finish destroying panels and
// then destroying the host, successfully or not) before destroying the
// target.
deferred.resolve(settleAll(outstanding)
.catch(console.error)
.then(() => {
this._removeHostListeners();
- // Tell the chrome the toolbox is almost destroyed and we can start
- // removing the toolbox from Firefox UI. Do it exactly here for
- // historical reason. If we do it sooner or later we appear to leak
- // the toolbox in many tests.
- this.postMessage({
- name: "destroy-host"
- });
+ // `location` may already be null if the toolbox document is already
+ // in process of destruction. Otherwise if it is still around, ensure
+ // releasing toolbox document and triggering cleanup thanks to unload
+ // event. We do that precisely here, before nullifying the target as
+ // various cleanup code depends on the target attribute to be still
+ // defined.
+ if (win.location) {
+ win.location.replace("about:blank");
+ }
// Targets need to be notified that the toolbox is being torn down.
// This is done after other destruction tasks since it may tear down
// fronts and the debugger transport which earlier destroy methods may
// require to complete.
if (!this._target) {
return null;
}