diff --git a/browser/devtools/webaudioeditor/controller.js b/browser/devtools/webaudioeditor/controller.js index 544650a91041..b69bc0609417 100644 --- a/browser/devtools/webaudioeditor/controller.js +++ b/browser/devtools/webaudioeditor/controller.js @@ -42,9 +42,21 @@ let WebAudioEditorController = { * Listen for events emitted by the current tab target. */ initialize: Task.async(function* () { + // Create a queue to manage all the events from the + // front so they can be executed in order + this.queue = new Queue(); + telemetry.toolOpened("webaudioeditor"); this._onTabNavigated = this._onTabNavigated.bind(this); this._onThemeChange = this._onThemeChange.bind(this); + this._onStartContext = this._onStartContext.bind(this); + + this._onCreateNode = this.queue.addHandler(this._onCreateNode.bind(this)); + this._onConnectNode = this.queue.addHandler(this._onConnectNode.bind(this)); + this._onConnectParam = this.queue.addHandler(this._onConnectParam.bind(this)); + this._onDisconnectNode = this.queue.addHandler(this._onDisconnectNode.bind(this)); + this._onChangeParam = this.queue.addHandler(this._onChangeParam.bind(this)); + this._onDestroyNode = this.queue.addHandler(this._onDestroyNode.bind(this)); gTarget.on("will-navigate", this._onTabNavigated); gTarget.on("navigate", this._onTabNavigated); @@ -75,7 +87,7 @@ let WebAudioEditorController = { /** * Remove events emitted by the current tab target. */ - destroy: function() { + destroy: Task.async(function*() { telemetry.toolClosed("webaudioeditor"); gTarget.off("will-navigate", this._onTabNavigated); gTarget.off("navigate", this._onTabNavigated); @@ -87,7 +99,9 @@ let WebAudioEditorController = { gFront.off("change-param", this._onChangeParam); gFront.off("destroy-node", this._onDestroyNode); gDevTools.off("pref-changed", this._onThemeChange); - }, + yield this.queue.clear(); + this.queue = null; + }), /** * Called when page is reloaded to show the reload notice and waiting @@ -100,23 +114,12 @@ let WebAudioEditorController = { PropertiesView.resetUI(); }, - // Since node events (create, disconnect, connect) are all async, - // we have to make sure to wait that the node has finished creating - // before performing an operation on it. - getNode: function* (nodeActor) { + /** + * Takes an AudioNodeActor and returns the corresponding AudioNodeModel. + */ + getNode: function (nodeActor) { let id = nodeActor.actorID; let node = gAudioNodes.get(id); - - if (!node) { - let { resolve, promise } = defer(); - gAudioNodes.on("add", function createNodeListener (createdNode) { - if (createdNode.id === id) { - gAudioNodes.off("add", createNodeListener); - resolve(createdNode); - } - }); - node = yield promise; - } return node; }, @@ -135,13 +138,16 @@ let WebAudioEditorController = { _onTabNavigated: Task.async(function* (event, {isFrameSwitching}) { switch (event) { case "will-navigate": { + yield this.queue.clear(); + gAudioNodes.reset(); + // Make sure the backend is prepared to handle audio contexts. if (!isFrameSwitching) { yield gFront.setup({ reload: false }); } // Clear out current UI. - this.reset(); + yield this.reset(); // When switching to an iframe, ensure displaying the reload button. // As the document has already been loaded without being hooked. @@ -156,9 +162,6 @@ let WebAudioEditorController = { $("#waiting-notice").hidden = false; } - // Clear out stored audio nodes - gAudioNodes.reset(); - window.emit(EVENTS.UI_RESET); break; } @@ -182,7 +185,7 @@ let WebAudioEditorController = { }, /** - * Called when a new node is created. Creates an `AudioNodeView` instance + * Called when a new node is created. Creates an `AudioNodeModel` instance * for tracking throughout the editor. */ _onCreateNode: Task.async(function* (nodeActor) { @@ -200,34 +203,34 @@ let WebAudioEditorController = { /** * Called when a node is connected to another node. */ - _onConnectNode: Task.async(function* ({ source: sourceActor, dest: destActor }) { - let source = yield WebAudioEditorController.getNode(sourceActor); - let dest = yield WebAudioEditorController.getNode(destActor); + _onConnectNode: function ({ source: sourceActor, dest: destActor }) { + let source = this.getNode(sourceActor); + let dest = this.getNode(destActor); source.connect(dest); - }), + }, /** * Called when a node is conneceted to another node's AudioParam. */ - _onConnectParam: Task.async(function* ({ source: sourceActor, dest: destActor, param }) { - let source = yield WebAudioEditorController.getNode(sourceActor); - let dest = yield WebAudioEditorController.getNode(destActor); + _onConnectParam: function ({ source: sourceActor, dest: destActor, param }) { + let source = this.getNode(sourceActor); + let dest = this.getNode(destActor); source.connect(dest, param); - }), + }, /** * Called when a node is disconnected. */ - _onDisconnectNode: Task.async(function* (nodeActor) { - let node = yield WebAudioEditorController.getNode(nodeActor); + _onDisconnectNode: function (nodeActor) { + let node = this.getNode(nodeActor); node.disconnect(); - }), + }, /** * Called when a node param is changed. */ - _onChangeParam: Task.async(function* ({ actor, param, value }) { - let node = yield WebAudioEditorController.getNode(actor); + _onChangeParam: function ({ actor, param, value }) { + let node = this.getNode(actor); window.emit(EVENTS.CHANGE_PARAM, node, param, value); - }) + } }; diff --git a/browser/devtools/webaudioeditor/includes.js b/browser/devtools/webaudioeditor/includes.js index 2e6b8ece4ea3..10a4ce36f709 100644 --- a/browser/devtools/webaudioeditor/includes.js +++ b/browser/devtools/webaudioeditor/includes.js @@ -16,6 +16,7 @@ const { require } = devtools; let { console } = Cu.import("resource://gre/modules/devtools/Console.jsm", {}); let { EventTarget } = require("sdk/event/target"); +const { Queue } = require("devtools/webaudioeditor/queue"); const { Task } = Cu.import("resource://gre/modules/Task.jsm", {}); const { Class } = require("sdk/core/heritage"); const EventEmitter = require("devtools/toolkit/event-emitter"); diff --git a/browser/devtools/webaudioeditor/modules/queue.js b/browser/devtools/webaudioeditor/modules/queue.js new file mode 100644 index 000000000000..02db5932acfc --- /dev/null +++ b/browser/devtools/webaudioeditor/modules/queue.js @@ -0,0 +1,96 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ +"use strict"; + +const Promise = require("sdk/core/promise"); + +/** + * Wraps an event handler that responds to an actor request and triggers + * them in the order they arrive. This is necessary because some handlers are async and + * introduce race conditions. Example (bug 1141261), connect two nodes, and we have to wait + * for both nodes to be created (another async handler), but the disconnect handler resolves faster + * as it only has to wait for one node, and none of these timings are reliable. + * + * So queue them up here and execute them in order when the previous handler completes. + * + * Usage: + * + * var q = new Queue(); + * + * let handler = q.addHandler(handler); + * gFront.on("event", handler); + */ + +function Queue () { + this._messages = []; + this._processing = false; + this._currentProcess = null; + this._process = this._process.bind(this); + this._asyncHandler = this._asyncHandler.bind(this); +} +exports.Queue = Queue; + +/** + * Wrap a function that returns a new function + * that executes the original in accordance with the queue. + */ +Queue.prototype.addHandler = function (fn) { + return (...args) => { + this._messages.push([fn, ...args]); + if (!this._processing) { + this._process(); + } + } +}; + +Queue.prototype._process = function () { + if (this._messages.length === 0) { + this._processing = false; + return; + } + + this._processing = true; + + let [fn, ...args] = this._messages.shift(); + let result = fn.apply(null, args); + if (result && result.then) { + // Store the current process if its async, so we + // can wait for it to finish if we clear. + this._currentProcess = result.then(this._asyncHandler, this._asyncHandler); + } else { + this._process(); + } +}; + +/** + * Used to wrap up an async message completion. + */ +Queue.prototype._asyncHandler = function () { + this._currentProcess = null; + this._process(); +}; + +/** + * Return the number of in-flight messages. + */ +Queue.prototype.getMessageCount = function () { + return this._messages.length; +}; + +/** + * Clear out all remaining messages. Returns a promise if there's + * an async message currently being processed that resolves upon + * the message completion. + */ +Queue.prototype.clear = function () { + this._messages.length = 0; + this._processing = false; + + // If currently waiting for the last async message to finish, + // wait for it, then clear out the messages. + if (this._currentProcess) { + return this._currentProcess; + } + return Promise.resolve(); +}; diff --git a/browser/devtools/webaudioeditor/moz.build b/browser/devtools/webaudioeditor/moz.build index 7fbea6cc9bd3..65b8de7edcd3 100644 --- a/browser/devtools/webaudioeditor/moz.build +++ b/browser/devtools/webaudioeditor/moz.build @@ -4,6 +4,7 @@ # file, You can obtain one at http://mozilla.org/MPL/2.0/. EXTRA_JS_MODULES.devtools.webaudioeditor += [ + 'modules/queue.js', 'panel.js' ] diff --git a/browser/devtools/webaudioeditor/test/browser.ini b/browser/devtools/webaudioeditor/test/browser.ini index d803cc9b0454..d2f0be3091e2 100644 --- a/browser/devtools/webaudioeditor/test/browser.ini +++ b/browser/devtools/webaudioeditor/test/browser.ini @@ -12,9 +12,10 @@ support-files = doc_connect-multi-param.html doc_iframe-context.html doc_automation.html + doc_bug_1112378.html doc_bug_1125817.html doc_bug_1130901.html - doc_bug_1112378.html + doc_bug_1141261.html 440hz_sine.ogg head.js @@ -52,6 +53,7 @@ skip-if = true # bug 1092571 [browser_wa_graph-render-04.js] [browser_wa_graph-render-05.js] skip-if = true # bug 1092571 +[browser_wa_graph-render-06.js] [browser_wa_graph-selected.js] [browser_wa_graph-zoom.js] [browser_wa_inspector.js] @@ -71,3 +73,6 @@ skip-if = true # bug 1010423 [browser_wa_reset-02.js] [browser_wa_reset-03.js] [browser_wa_reset-04.js] + +[browser_queue-01.js] +[browser_queue-02.js] diff --git a/browser/devtools/webaudioeditor/test/browser_queue-01.js b/browser/devtools/webaudioeditor/test/browser_queue-01.js new file mode 100644 index 000000000000..d17d3cc4bb88 --- /dev/null +++ b/browser/devtools/webaudioeditor/test/browser_queue-01.js @@ -0,0 +1,83 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +/** + * Tests to ensure the queue module created for bug 1141261 works as intended + */ + +add_task(function*() { + let { promise, resolve } = Promise.defer(); + let { Queue } = devtools.require("devtools/webaudioeditor/queue"); + + let q = new Queue(); + let messages = []; + + let create = q.addHandler(function (id) { + return Task.spawn(function () { + yield wait(1); + processed("create", id); + }); + }); + + let connect = q.addHandler(function (source, dest) { + processed("connect", source, dest); + }); + + create(1); + create(2); + connect(1, 2); + create(5); + connect(1, 5); + create(6); + + + function processed () { + messages.push(arguments); + + // If we have 6 messages, check the order + if (messages.length === 6) { + checkFirstBatch(); + + // Fire off more messages after the queue is done, + // waiting a tick to ensure it works after draining + // the queue + Task.spawn(function () { + create(10); + connect(1, 10); + }); + return; + } + + if (messages.length === 8) { + checkSecondBatch(); + resolve(); + } + } + + function checkFirstBatch () { + is(messages[0][0], "create", "first message check"); + is(messages[0][1], 1, "first message args"); + is(messages[1][0], "create", "second message check"); + is(messages[1][1], 2, "second message args"); + is(messages[2][0], "connect", "third message check (sync)"); + is(messages[2][1], 1, "third message args"); + is(messages[2][2], 2, "third message args"); + is(messages[3][0], "create", "fourth message check"); + is(messages[3][1], 5, "fourth message args"); + is(messages[4][0], "connect", "fifth message check (sync)"); + is(messages[4][1], 1, "fifth message args"); + is(messages[4][2], 5, "fifth message args"); + is(messages[5][0], "create", "sixth message check"); + is(messages[5][1], 6, "sixth message args"); + } + + function checkSecondBatch () { + is(messages[6][0], "create", "seventh message check"); + is(messages[6][1], 10, "seventh message args"); + is(messages[7][0], "connect", "eighth message check"); + is(messages[7][1], 1, "eighth message args"); + is(messages[7][2], 10, "eighth message args"); + } + + return promise; +}); diff --git a/browser/devtools/webaudioeditor/test/browser_queue-02.js b/browser/devtools/webaudioeditor/test/browser_queue-02.js new file mode 100644 index 000000000000..479c93ba9a32 --- /dev/null +++ b/browser/devtools/webaudioeditor/test/browser_queue-02.js @@ -0,0 +1,50 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +/** + * Tests queue module's clear and getMessageCount functions work. + */ + +add_task(function*() { + let { promise, resolve } = Promise.defer(); + let { Queue } = devtools.require("devtools/webaudioeditor/queue"); + + let q = new Queue(); + let messages = []; + let cleared = false; + let lastProcessedId = null; + + let processed = Task.async(function *(id) { + if (cleared) { + throw new Error("Messages should not be processed after clearing the queue"); + } + messages.push(id); + + is(q.getMessageCount(), 10 - messages.length, "getMessageCount() should count all queued up messages"); + + // On our 5th message, clear out all remaining tasks + if (messages.length === 5) { + is(lastProcessedId, 3, "Current message is not yet finished processing"); + yield q.clear(); + is(lastProcessedId, 4, "Current message is finished processing after yielding from queue.clear()"); + cleared = true; + + is(q.getMessageCount(), 0, "getMessageCount() returns 0 after being cleared"); + // Wait a bit before finishing to ensure no more messages get processed + setTimeout(resolve, 300); + } + }); + + let exec = q.addHandler(function (id) { + return Task.spawn(function () { + yield wait(1); + processed(id); + }).then(() => lastProcessedId = id); + }); + + for (let i = 0; i < 10; i++) { + exec(i); + } + + return promise; +}); diff --git a/browser/devtools/webaudioeditor/test/browser_wa_controller-01.js b/browser/devtools/webaudioeditor/test/browser_wa_controller-01.js index 3fb70b1edf12..01394b048d73 100644 --- a/browser/devtools/webaudioeditor/test/browser_wa_controller-01.js +++ b/browser/devtools/webaudioeditor/test/browser_wa_controller-01.js @@ -18,7 +18,8 @@ add_task(function*() { let [actors] = yield Promise.all([ once(gAudioNodes, "add", 2), - once(gAudioNodes, "disconnect") + once(gAudioNodes, "disconnect"), + waitForGraphRendered(panelWin, 2, 0) ]); ok(true, "Successfully disconnected a just-created node."); diff --git a/browser/devtools/webaudioeditor/test/browser_wa_graph-render-06.js b/browser/devtools/webaudioeditor/test/browser_wa_graph-render-06.js new file mode 100644 index 000000000000..e152f69531be --- /dev/null +++ b/browser/devtools/webaudioeditor/test/browser_wa_graph-render-06.js @@ -0,0 +1,25 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +/** + * Tests to ensure that param connections trigger graph redraws + */ + +const BUG_1141261_URL = EXAMPLE_URL + "doc_bug_1141261.html"; + +add_task(function*() { + let { target, panel } = yield initWebAudioEditor(BUG_1141261_URL); + let { panelWin } = panel; + let { gFront, $, $$, EVENTS } = panelWin; + + reload(target); + + let [actors] = yield Promise.all([ + getN(gFront, "create-node", 3), + waitForGraphRendered(panelWin, 3, 1, 0) + ]); + + ok(true, "Graph correctly shows gain node as disconnected"); + + yield teardown(target); +}); diff --git a/browser/devtools/webaudioeditor/test/browser_wa_reset-01.js b/browser/devtools/webaudioeditor/test/browser_wa_reset-01.js index 3208227acb96..d93a58f0d840 100644 --- a/browser/devtools/webaudioeditor/test/browser_wa_reset-01.js +++ b/browser/devtools/webaudioeditor/test/browser_wa_reset-01.js @@ -4,7 +4,7 @@ /////////////////// // // Whitelisting this test. -// As part of bug 1077403, the leaking uncaught rejection should be fixed. +// As part of bug 1077403, the leaking uncaught rejection should be fixed. // thisTestLeaksUncaughtRejectionsAndShouldBeFixed("Error: Connection closed"); @@ -53,6 +53,7 @@ add_task(function*() { reload(target); yield Promise.all([navigating, started]); + let rendered = waitForGraphRendered(panel.panelWin, 3, 2); is($("#reload-notice").hidden, true, "The 'reload this page' notice should be hidden after context found after reload."); @@ -61,5 +62,7 @@ add_task(function*() { is($("#content").hidden, false, "The tool's content should reappear without closing and reopening the toolbox."); + yield rendered; + yield teardown(target); }); diff --git a/browser/devtools/webaudioeditor/test/doc_bug_1141261.html b/browser/devtools/webaudioeditor/test/doc_bug_1141261.html new file mode 100644 index 000000000000..87c1210a4590 --- /dev/null +++ b/browser/devtools/webaudioeditor/test/doc_bug_1141261.html @@ -0,0 +1,25 @@ + + + + + + + Web Audio Editor test page + + + + + + + + diff --git a/browser/devtools/webaudioeditor/views/context.js b/browser/devtools/webaudioeditor/views/context.js index f2160acf8d2a..46b0ab51603e 100644 --- a/browser/devtools/webaudioeditor/views/context.js +++ b/browser/devtools/webaudioeditor/views/context.js @@ -3,7 +3,11 @@ * You can obtain one at http://mozilla.org/MPL/2.0/. */ "use strict"; -const { debounce } = require("sdk/lang/functional"); +// Previously used `debounce`, but after using an ordered queue +// for actor events, too many events were queued, causing large +// render delays, rather than incremental additions in rendering. +// So now we use `throttle`, which will render at most `n` milliseconds. +const { throttle } = require("sdk/lang/functional"); // Globals for d3 stuff // Default properties of the graph on rerender @@ -22,7 +26,8 @@ const MARKER_STYLING = { dark: "#CED3D9" }; -const GRAPH_DEBOUNCE_TIMER = 100; +// Render graph at most once every 500ms +const GRAPH_THROTTLE_TIMER = 500; // `gAudioNodes` events that should require the graph // to redraw @@ -41,7 +46,7 @@ let ContextView = { this._onStartContext = this._onStartContext.bind(this); this._onEvent = this._onEvent.bind(this); - this.draw = debounce(this.draw.bind(this), GRAPH_DEBOUNCE_TIMER); + this.draw = throttle(this.draw.bind(this), GRAPH_THROTTLE_TIMER); $("#graph-target").addEventListener("click", this._onGraphClick, false); window.on(EVENTS.THEME_CHANGE, this._onThemeChange);