Bug 1204595 - Store audionode properties once via server rather than async fetching the unchanging properties in the tool. r=jryans

browser/devtools/webaudioeditor/controller.js

@@ -185,9 +185,9 @@ let WebAudioEditorController = {
    * Called when a new node is created. Creates an `AudioNodeView` instance
    * for tracking throughout the editor.
    */
-  _onCreateNode: Task.async(function* (nodeActor) {
-    yield gAudioNodes.add(nodeActor);
-  }),
+  _onCreateNode: function (nodeActor) {
+    gAudioNodes.add(nodeActor);
+  },
 
   /**
    * Called on `destroy-node` when an AudioNode is GC'd. Removes

browser/devtools/webaudioeditor/models.js

@@ -24,36 +24,12 @@ const AudioNodeModel = Class({
   initialize: function (actor) {
     this.actor = actor;
     this.id = actor.actorID;
+    this.type = actor.type;
+    this.bypassable = actor.bypassable;
+    this._bypassed = false;
     this.connections = [];
   },
 
-  /**
-   * After instantiating the AudioNodeModel, calling `setup` caches values
-   * from the actor onto the model. In this case, only the type of audio node.
-   *
-   * @return promise
-   */
-  setup: Task.async(function* () {
-    yield this.getType();
-
-    // Query bypass status on start up
-    this._bypassed = yield this.isBypassed();
-
-    // Store whether or not this node is bypassable in the first place
-    this.bypassable = !AUDIO_NODE_DEFINITION[this.type].unbypassable;
-  }),
-
-  /**
-   * A proxy for the underlying AudioNodeActor to fetch its type
-   * and subsequently assign the type to the instance.
-   *
-   * @return Promise->String
-   */
-  getType: Task.async(function* () {
-    this.type = yield this.actor.getType();
-    return this.type;
-  }),
-
   /**
    * Stores connection data inside this instance of this audio node connecting
    * to another node (destination). If connecting to another node's AudioParam,
@@ -84,10 +60,10 @@ const AudioNodeModel = Class({
   /**
    * Gets the bypass status of the audio node.
    *
-   * @return Promise->Boolean
+   * @return Boolean
    */
   isBypassed: function () {
-    return this.actor.isBypassed();
+    return this._bypassed;
   },
 
   /**
@@ -162,7 +138,9 @@ const AudioNodeModel = Class({
 
       graph.addEdge(null, this.id, edge.destination, options);
     }
-  }
+  },
+
+  toString: () => "[object AudioNodeModel]",
 });
 
 
@@ -200,25 +178,21 @@ const AudioNodesCollection = Class({
    * constructor, and adds the model to the internal collection store of this
    * instance.
    *
-   * Also calls `setup` on the model itself, and sets up event piping, so that
-   * events emitted on each model propagate to the collection itself.
-   *
    * Emits "add" event on instance when completed.
    *
    * @param Object obj
-   * @return Promise->AudioNodeModel
+   * @return AudioNodeModel
    */
-  add: Task.async(function* (obj) {
+  add: function (obj) {
     let node = new this.model(obj);
     node.collection = this;
-    yield node.setup();
 
     this.models.add(node);
 
     node.on("*", this._onModelEvent);
     coreEmit(this, "add", node);
     return node;
-  }),
+  },
 
   /**
    * Removes an AudioNodeModel from the internal collection. Calls `delete` method
@@ -308,5 +282,7 @@ const AudioNodesCollection = Class({
       // Pipe the event to the collection
       coreEmit(this, eventName, node, ...args);
     }
-  }
+  },
+
+  toString: () => "[object AudioNodeCollection]",
 });

browser/devtools/webaudioeditor/test/browser.ini

@@ -22,9 +22,10 @@ support-files =
 [browser_audionode-actor-get-params-01.js]
 [browser_audionode-actor-get-params-02.js]
 [browser_audionode-actor-get-set-param.js]
-[browser_audionode-actor-get-type.js]
-[browser_audionode-actor-is-source.js]
+[browser_audionode-actor-type.js]
+[browser_audionode-actor-source.js]
 [browser_audionode-actor-bypass.js]
+[browser_audionode-actor-bypassable.js]
 [browser_audionode-actor-connectnode-disconnect.js]
 [browser_audionode-actor-connectparam.js]
 skip-if = true # bug 1092571

browser/devtools/webaudioeditor/test/browser_audionode-actor-bypassable.js

@@ -0,0 +1,38 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * Test AudioNode#bypassable
+ */
+
+add_task(function*() {
+  let { target, front } = yield initBackend(SIMPLE_NODES_URL);
+  let [_, nodes] = yield Promise.all([
+    front.setup({ reload: true }),
+    getN(front, "create-node", 14)
+  ]);
+
+  let actualBypassability = nodes.map(node => node.bypassable);
+  let expectedBypassability = [
+    false, // AudioDestinationNode
+    true, // AudioBufferSourceNode
+    true, // ScriptProcessorNode
+    true, // AnalyserNode
+    true, // GainNode
+    true, // DelayNode
+    true, // BiquadFilterNode
+    true, // WaveShaperNode
+    true, // PannerNode
+    true, // ConvolverNode
+    false, // ChannelSplitterNode
+    false, // ChannelMergerNode
+    true, // DynamicsCompressNode
+    true, // OscillatorNode
+  ];
+
+  expectedBypassability.forEach((bypassable, i) => {
+    is(actualBypassability[i], bypassable, `${nodes[i].type} has correct ".bypassable" status`);
+  });
+
+  yield removeTab(target.tab);
+});

browser/devtools/webaudioeditor/test/browser_audionode-actor-source.js

@@ -2,7 +2,7 @@
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 /**
- * Test AudioNode#isSource()
+ * Test AudioNode#source
  */
 
 add_task(function*() {
@@ -12,15 +12,15 @@ add_task(function*() {
     getN(front, "create-node", 14)
   ]);
 
-  let actualTypes = yield Promise.all(nodes.map(node => node.getType()));
-  let isSourceResult = yield Promise.all(nodes.map(node => node.isSource()));
+  let actualTypes = nodes.map(node => node.type);
+  let isSourceResult = nodes.map(node => node.source);
 
   actualTypes.forEach((type, i) => {
     let shouldBeSource = type === "AudioBufferSourceNode" || type === "OscillatorNode";
     if (shouldBeSource)
-      is(isSourceResult[i], true, type + "'s isSource() yields into `true`");
+      is(isSourceResult[i], true, type + "'s `source` is `true`");
     else
-      is(isSourceResult[i], false, type + "'s isSource() yields into `false`");
+      is(isSourceResult[i], false, type + "'s `source` is `false`");
   });
 
   yield removeTab(target.tab);

browser/devtools/webaudioeditor/test/browser_audionode-actor-type.js

@@ -2,7 +2,7 @@
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 /**
- * Test AudioNode#getType()
+ * Test AudioNode#type
  */
 
 add_task(function*() {
@@ -12,7 +12,7 @@ add_task(function*() {
     getN(front, "create-node", 14)
   ]);
 
-  let actualTypes = yield Promise.all(nodes.map(node => node.getType()));
+  let actualTypes = nodes.map(node => node.type);
   let expectedTypes = [
     "AudioDestinationNode",
     "AudioBufferSourceNode", "ScriptProcessorNode", "AnalyserNode", "GainNode",

browser/devtools/webaudioeditor/test/browser_wa_inspector-bypass-01.js

@@ -18,12 +18,8 @@ add_task(function*() {
   ]);
   let nodeIds = actors.map(actor => actor.actorID);
 
-  click(panelWin, findGraphNode(panelWin, nodeIds[1]));
   // Wait for the node to be set as well as the inspector to come fully into the view
-  yield Promise.all([
-    waitForInspectorRender(panelWin, EVENTS),
-    once(panelWin, EVENTS.UI_INSPECTOR_TOGGLED)
-  ]);
+  yield clickGraphNode(panelWin, findGraphNode(panelWin, nodeIds[1]), true);
 
   let $bypass = $("toolbarbutton.bypass");
 
@@ -49,9 +45,7 @@ add_task(function*() {
   ok(!findGraphNode(panelWin, nodeIds[1]).classList.contains("bypassed"),
     "AudioNode no longer has 'bypassed' class.");
 
-  click(panelWin, findGraphNode(panelWin, nodeIds[0]));
-
-  yield once(panelWin, EVENTS.UI_INSPECTOR_NODE_SET);
+  yield clickGraphNode(panelWin, findGraphNode(panelWin, nodeIds[0]));
 
   is((yield actors[0].isBypassed()), false, "Unbypassable AudioNodeActor is not bypassed.");
   is($bypass.checked, false, "Button is 'off' for unbypassable nodes");

browser/devtools/webaudioeditor/test/browser_webaudio-actor-destroy-node.js

@@ -21,9 +21,8 @@ add_task(function*() {
 
   let destroyed = yield waitUntilDestroyed;
 
-  let destroyedTypes = yield Promise.all(destroyed.map(actor => actor.getType()));
-  destroyedTypes.forEach((type, i) => {
-    ok(type, "AudioBufferSourceNode", "Only buffer nodes are destroyed");
+  destroyed.forEach((node, i) => {
+    ok(node.type, "AudioBufferSourceNode", "Only buffer nodes are destroyed");
     ok(actorIsInList(created, destroyed[i]),
       "`destroy-node` called only on AudioNodes in current document.");
   });

browser/devtools/webaudioeditor/test/browser_webaudio-actor-simple.js

@@ -14,13 +14,9 @@ add_task(function*() {
     get2(front, "connect-node")
   ]);
 
-  let destType = yield destNode.getType();
-  let oscType = yield oscNode.getType();
-  let gainType = yield gainNode.getType();
-
-  is(destType, "AudioDestinationNode", "WebAudioActor:create-node returns AudioNodeActor for AudioDestination");
-  is(oscType, "OscillatorNode", "WebAudioActor:create-node returns AudioNodeActor");
-  is(gainType, "GainNode", "WebAudioActor:create-node returns AudioNodeActor");
+  is(destNode.type, "AudioDestinationNode", "WebAudioActor:create-node returns AudioNodeActor for AudioDestination");
+  is(oscNode.type, "OscillatorNode", "WebAudioActor:create-node returns AudioNodeActor");
+  is(gainNode.type, "GainNode", "WebAudioActor:create-node returns AudioNodeActor");
 
   let { source, dest } = connect1;
   is(source.actorID, oscNode.actorID, "WebAudioActor:connect-node returns correct actor with ID on source (osc->gain)");

browser/devtools/webaudioeditor/views/inspector.js

@@ -88,7 +88,7 @@ let InspectorView = {
     else {
       $("#web-audio-editor-details-pane-empty").setAttribute("hidden", "true");
       $("#web-audio-editor-tabs").removeAttribute("hidden");
-      yield this._buildToolbar();
+      this._buildToolbar();
       window.emit(EVENTS.UI_INSPECTOR_NODE_SET, this._currentNode.id);
     }
   }),
@@ -111,11 +111,11 @@ let InspectorView = {
     this.hideImmediately();
   },
 
-  _buildToolbar: Task.async(function* () {
+  _buildToolbar: function () {
     let node = this.getCurrentAudioNode();
 
     let bypassable = node.bypassable;
-    let bypassed = yield node.isBypassed();
+    let bypassed = node.isBypassed();
     let button = $("#audio-node-toolbar .bypass");
 
     if (!bypassable) {
@@ -129,7 +129,7 @@ let InspectorView = {
     } else {
       button.setAttribute("checked", true);
     }
-  }),
+  },
 
   /**
    * Event handlers

toolkit/devtools/server/actors/utils/audionodes.json

@@ -1,5 +1,6 @@
 {
   "OscillatorNode": {
+    "source": true,
     "properties": {
       "type": {},
       "frequency": {
@@ -17,6 +18,7 @@
     "properties": { "delayTime": { "param": true }}
   },
   "AudioBufferSourceNode": {
+    "source": true,
     "properties": {
       "buffer": { "Buffer": true },
       "playbackRate": {
@@ -91,8 +93,12 @@
   "ChannelMergerNode": {
     "unbypassable": true
   },
-  "MediaElementAudioSourceNode": {},
-  "MediaStreamAudioSourceNode": {},
+  "MediaElementAudioSourceNode": {
+    "source": true
+  },
+  "MediaStreamAudioSourceNode": {
+    "source": true
+  },
   "MediaStreamAudioDestinationNode": {
     "unbypassable": true,
     "properties": {

toolkit/devtools/server/actors/webaudio.js

@@ -8,13 +8,14 @@ const {Cc, Ci, Cu, Cr} = require("chrome");
 const Services = require("Services");
 
 const events = require("sdk/event/core");
+const promise = require("promise");
 const { on: systemOn, off: systemOff } = require("sdk/system/events");
 const protocol = require("devtools/server/protocol");
 const { CallWatcherActor, CallWatcherFront } = require("devtools/server/actors/call-watcher");
 const { createValueGrip } = require("devtools/server/actors/object");
 const AutomationTimeline = require("./utils/automation-timeline");
 const { on, once, off, emit } = events;
-const { types, method, Arg, Option, RetVal } = protocol;
+const { types, method, Arg, Option, RetVal, preEvent } = protocol;
 const AUDIO_NODE_DEFINITION = require("devtools/server/actors/utils/audionodes.json");
 const ENABLE_AUTOMATION = false;
 const AUTOMATION_GRANULARITY = 2000;
@@ -49,6 +50,19 @@ types.addActorType("audionode");
 let AudioNodeActor = exports.AudioNodeActor = protocol.ActorClass({
   typeName: "audionode",
 
+  form: function (detail) {
+    if (detail === "actorid") {
+      return this.actorID;
+    }
+
+    return {
+      actor: this.actorID, // actorID is set when this is added to a pool
+      type: this.type,
+      source: this.source,
+      bypassable: this.bypassable,
+    };
+  },
+
   /**
    * Create the Audio Node actor.
    *
@@ -75,6 +89,9 @@ let AudioNodeActor = exports.AudioNodeActor = protocol.ActorClass({
       this.type = "";
     }
 
+    this.source = !!AUDIO_NODE_DEFINITION[this.type].source;
+    this.bypassable = !AUDIO_NODE_DEFINITION[this.type].unbypassable;
+
     // Create automation timelines for all AudioParams
     Object.keys(AUDIO_NODE_DEFINITION[this.type].properties || {})
       .filter(isAudioParam.bind(null, node))
@@ -84,24 +101,13 @@ let AudioNodeActor = exports.AudioNodeActor = protocol.ActorClass({
   },
 
   /**
-   * Returns the name of the audio type.
-   * Examples: "OscillatorNode", "MediaElementAudioSourceNode"
+   * Returns the string name of the audio type.
+   *
+   * DEPRECATED: Use `audionode.type` instead, left here for legacy reasons.
    */
   getType: method(function () {
     return this.type;
-  }, {
-    response: { type: RetVal("string") }
-  }),
-
-  /**
-   * Returns a boolean indicating if the node is a source node,
-   * like BufferSourceNode, MediaElementAudioSourceNode, OscillatorNode, etc.
-   */
-  isSource: method(function () {
-    return !!~this.type.indexOf("Source") || this.type === "OscillatorNode";
-  }, {
-    response: { source: RetVal("boolean") }
-  }),
+  }, { response: { type: RetVal("string") }}),
 
   /**
    * Returns a boolean indicating if the AudioNode has been "bypassed",
@@ -139,8 +145,7 @@ let AudioNodeActor = exports.AudioNodeActor = protocol.ActorClass({
       return;
     }
 
-    let bypassable = !AUDIO_NODE_DEFINITION[this.type].unbypassable;
-    if (bypassable) {
+    if (this.bypassable) {
       node.passThrough = enable;
     }
 
@@ -455,8 +460,29 @@ let AudioNodeActor = exports.AudioNodeActor = protocol.ActorClass({
 
 /**
  * The corresponding Front object for the AudioNodeActor.
+ *
+ * @attribute {String} type
+ *            The type of audio node, like "OscillatorNode", "MediaElementAudioSourceNode"
+ * @attribute {Boolean} source
+ *            Boolean indicating if the node is a source node, like BufferSourceNode,
+ *            MediaElementAudioSourceNode, OscillatorNode, etc.
+ * @attribute {Boolean} bypassable
+ *            Boolean indicating if the audio node is bypassable (splitter,
+ *            merger and destination nodes, for example, are not)
  */
 let AudioNodeFront = protocol.FrontClass(AudioNodeActor, {
+  form: function (form, detail) {
+    if (detail === "actorid") {
+      this.actorID = form;
+      return;
+    }
+
+    this.actorID = form.actor;
+    this.type = form.type;
+    this.source = form.source;
+    this.bypassable = form.bypassable;
+  },
+
   initialize: function (client, form) {
     protocol.Front.prototype.initialize.call(this, client, form);
     // if we were manually passed a form, this was created manually and
@@ -864,7 +890,22 @@ let WebAudioFront = exports.WebAudioFront = protocol.FrontClass(WebAudioActor, {
   initialize: function(client, { webaudioActor }) {
     protocol.Front.prototype.initialize.call(this, client, { actor: webaudioActor });
     this.manage(this);
-  }
+  },
+
+  /**
+   * If connecting to older geckos (<Fx43), where audio node actor's do not
+   * contain `type`, `source` and `bypassable` properties, fetch
+   * them manually here.
+   */
+  _onCreateNode: preEvent("create-node", function (audionode) {
+    if (!audionode.type) {
+      return audionode.getType().then(type => {
+        audionode.type = type;
+        audionode.source = !!AUDIO_NODE_DEFINITION[type].source;
+        audionode.bypassable = !AUDIO_NODE_DEFINITION[type].unbypassable;
+      });
+    }
+  }),
 });
 
 WebAudioFront.AUTOMATION_METHODS = new Set(AUTOMATION_METHODS);

toolkit/devtools/server/docs/protocol.js.md

@@ -423,6 +423,12 @@ You might want to update your front's state when an event is fired, before emitt
         this.amountOfGoodNews++;
     });
 
+You can have events wait until an asynchronous action completes before firing by returning a promise. If you have multiple preEvents defined for a specific event, and atleast one fires asynchronously, then all preEvents most resolve before all events are fired.
+
+    countGoodNews: protocol.preEvent("good-news", function(news) {
+        return this.updateGoodNews().then(() => this.amountOfGoodNews++);
+    });
+
 On a somewhat related note, not every method needs to be request/response.  Just like an actor can emit a one-way event, a method can be marked as a one-way request.  Maybe we don't care about giveGoodNews returning anything:
 
     giveGoodNews: method(function(news) {

toolkit/devtools/server/protocol.js

@@ -1198,8 +1198,16 @@ let Front = Class({
         throw ex;
       }
       if (event.pre) {
-        event.pre.forEach((pre) => pre.apply(this, args));
+        let results = event.pre.map(pre => pre.apply(this, args));
+
+        // Check to see if any of the preEvents returned a promise -- if so,
+        // wait for their resolution before emitting. Otherwise, emit synchronously.
+        if (results.some(result => result && typeof result.then === "function")) {
+          promise.all(results).then(() => events.emit.apply(null, [this, event.name].concat(args)));
+          return;
+        }
       }
+
       events.emit.apply(null, [this, event.name].concat(args));
       return;
     }

toolkit/devtools/server/tests/unit/test_protocol_children.js

@@ -104,6 +104,7 @@ let ChildActor = protocol.ActorClass({
 
   emitEvents: method(function() {
     events.emit(this, "event1", 1, 2, 3);
+    events.emit(this, "event2", 4, 5, 6);
     events.emit(this, "named-event", 1, 2, 3);
     events.emit(this, "object-event", this);
     events.emit(this, "array-object-event", [this]);
@@ -119,6 +120,11 @@ let ChildActor = protocol.ActorClass({
       b: Arg(1),
       c: Arg(2)
     },
+    "event2" : {
+      a: Arg(0),
+      b: Arg(1),
+      c: Arg(2)
+    },
     "named-event": {
       type: "namedEvent",
       a: Arg(0),
@@ -161,6 +167,14 @@ let ChildFront = protocol.FrontClass(ChildActor, {
   onEvent1: preEvent("event1", function(a, b, c) {
     this.event1arg3 = c;
   }),
+
+  onEvent2a: preEvent("event2", function(a, b, c) {
+    return promise.resolve().then(() => this.event2arg3 = c);
+  }),
+
+  onEvent2b: preEvent("event2", function(a, b, c) {
+    this.event2arg2 = b;
+  }),
 });
 
 types.addDictType("manyChildrenDict", {
@@ -409,7 +423,7 @@ function run_test()
       // going to trigger events on the first child, so an event
       // triggered on the second should cause immediate failures.
 
-      let set = new Set(["event1", "named-event", "object-event", "array-object-event"]);
+      let set = new Set(["event1", "event2", "named-event", "object-event", "array-object-event"]);
 
       childFront.on("event1", (a, b, c) => {
         do_check_eq(a, 1);
@@ -419,6 +433,18 @@ function run_test()
         do_check_eq(childFront.event1arg3, 3);
         set.delete("event1");
       });
+      childFront.on("event2", (a, b, c) => {
+        do_check_eq(a, 4);
+        do_check_eq(b, 5);
+        do_check_eq(c, 6);
+        // Verify that the async pre-event handler was called,
+        // setting the property before this handler was called.
+        do_check_eq(childFront.event2arg3, 6);
+        // And check that the sync preEvent with the same name is also
+        // executed
+        do_check_eq(childFront.event2arg2, 5);
+        set.delete("event2");
+      });
       childFront.on("named-event", (a, b, c) => {
         do_check_eq(a, 1);
         do_check_eq(b, 2);
@@ -440,6 +466,7 @@ function run_test()
         do_throw("Unexpected event");
       }
       ret[1].on("event1", fail);
+      ret[1].on("event2", fail);
       ret[1].on("named-event", fail);
       ret[1].on("object-event", fail);
       ret[1].on("array-object-event", fail);
@@ -447,6 +474,7 @@ function run_test()
       return childFront.emitEvents().then(() => {
         trace.expectSend({"type":"emitEvents","to":"<actorid>"});
         trace.expectReceive({"type":"event1","a":1,"b":2,"c":3,"from":"<actorid>"});
+        trace.expectReceive({"type":"event2","a":4,"b":5,"c":6,"from":"<actorid>"});
         trace.expectReceive({"type":"namedEvent","a":1,"b":2,"c":3,"from":"<actorid>"});
         trace.expectReceive({"type":"objectEvent","detail":{"actor":"<actorid>","childID":"child1","detail":"detail1"},"from":"<actorid>"});
         trace.expectReceive({"type":"arrayObjectEvent","detail":[{"actor":"<actorid>","childID":"child1","detail":"detail2"}],"from":"<actorid>"});