Bug 1022917 - Do not store any strong references in the web audio editor. r=vp
authorJordan Santell <jsantell@gmail.com>
Wed, 11 Jun 2014 10:06:54 -0700
changeset 188316 254a1f0dc6419c3fd92d26448115ac323417eed5
parent 188315 155b952193bb423d67b1683f98ec71af67fe0f25
child 188317 125b6a8c66f22ffb58c7f581409c468b934b578d
push id26951
push useremorley@mozilla.com
push dateThu, 12 Jun 2014 14:07:43 +0000
treeherdermozilla-central@4f98802de6ce [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersvp
bugs1022917
milestone33.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1022917 - Do not store any strong references in the web audio editor. r=vp --- toolkit/devtools/server/actors/webaudio.js | 114 +++++++++++++++++++---------- 1 file changed, 77 insertions(+), 37 deletions(-)
toolkit/devtools/server/actors/webaudio.js
--- a/toolkit/devtools/server/actors/webaudio.js
+++ b/toolkit/devtools/server/actors/webaudio.js
@@ -103,38 +103,41 @@ const NODE_PROPERTIES = {
     "frequencyBinCount": { "readonly": true },
   },
   "AudioDestinationNode": {},
   "ChannelSplitterNode": {},
   "ChannelMergerNode": {}
 };
 
 /**
- * Track an array of audio nodes
-
-/**
  * An Audio Node actor allowing communication to a specific audio node in the
  * Audio Context graph.
  */
 let AudioNodeActor = exports.AudioNodeActor = protocol.ActorClass({
   typeName: "audionode",
 
   /**
    * Create the Audio Node actor.
    *
    * @param DebuggerServerConnection conn
    *        The server connection.
    * @param AudioNode node
    *        The AudioNode that was created.
    */
   initialize: function (conn, node) {
     protocol.Actor.prototype.initialize.call(this, conn);
-    this.node = unwrap(node);
+
+    // Store ChromeOnly property `id` to identify AudioNode,
+    // rather than storing a strong reference, and store a weak
+    // ref to underlying node for controlling.
+    this.nativeID = node.id;
+    this.node = Cu.getWeakReference(node);
+
     try {
-      this.type = getConstructorName(this.node);
+      this.type = getConstructorName(node);
     } catch (e) {
       this.type = "";
     }
   },
 
   /**
    * Returns the name of the audio type.
    * Examples: "OscillatorNode", "MediaElementAudioSourceNode"
@@ -160,21 +163,27 @@ let AudioNodeActor = exports.AudioNodeAc
    * on success, or a description of the error upon param set failure.
    *
    * @param String param
    *        Name of the AudioParam to change.
    * @param String value
    *        Value to change AudioParam to.
    */
   setParam: method(function (param, value) {
+    let node = this.node.get();
+
+    if (node === null) {
+      return CollectedAudioNodeError();
+    }
+
     try {
-      if (isAudioParam(this.node, param))
-        this.node[param].value = value;
+      if (isAudioParam(node, param))
+        node[param].value = value;
       else
-        this.node[param] = value;
+        node[param] = value;
       return undefined;
     } catch (e) {
       return constructError(e);
     }
   }, {
     request: {
       param: Arg(0, "string"),
       value: Arg(1, "nullable:primitive")
@@ -184,19 +193,25 @@ let AudioNodeActor = exports.AudioNodeAc
 
   /**
    * Gets a param on the audio node.
    *
    * @param String param
    *        Name of the AudioParam to fetch.
    */
   getParam: method(function (param) {
+    let node = this.node.get();
+
+    if (node === null) {
+      return CollectedAudioNodeError();
+    }
+
     // Check to see if it's an AudioParam -- if so,
     // return the `value` property of the parameter.
-    let value = isAudioParam(this.node, param) ? this.node[param].value : this.node[param];
+    let value = isAudioParam(node, param) ? node[param].value : node[param];
 
     // Return the grip form of the value; at this time,
     // there shouldn't be any non-primitives at the moment, other than
     // AudioBuffer or Float32Array references and the like,
     // so this just formats the value to be displayed in the VariablesView,
     // without using real grips and managing via actor pools.
     let grip;
     try {
@@ -257,17 +272,23 @@ let AudioNodeFront = protocol.FrontClass
  * high-level methods. After instantiating this actor, you'll need to set it
  * up by calling setup().
  */
 let WebAudioActor = exports.WebAudioActor = protocol.ActorClass({
   typeName: "webaudio",
   initialize: function(conn, tabActor) {
     protocol.Actor.prototype.initialize.call(this, conn);
     this.tabActor = tabActor;
+
     this._onContentFunctionCall = this._onContentFunctionCall.bind(this);
+
+    // Store ChromeOnly ID (`nativeID` property on AudioNodeActor) mapped
+    // to the associated actorID, so we don't have to expose `nativeID`
+    // to the client in any way.
+    this._nativeToActorID = new Map();
   },
 
   destroy: function(conn) {
     protocol.Actor.prototype.destroy.call(this, conn);
     this.finalize();
   },
 
   /**
@@ -277,24 +298,26 @@ let WebAudioActor = exports.WebAudioActo
    *
    * See ContentObserver and WebAudioInstrumenter for more details.
    */
   setup: method(function({ reload }) {
     // Used to track when something is happening with the web audio API
     // the first time, to ultimately fire `start-context` event
     this._firstNodeCreated = false;
 
+    // Clear out stored nativeIDs on reload as we do not want to track
+    // AudioNodes that are no longer on this document.
+    this._nativeToActorID.clear();
+
     if (this._initialized) {
       return;
     }
+
     this._initialized = true;
 
-    // Weak map mapping audio nodes to their corresponding actors
-    this._nodeActors = new Map();
-
     this._callWatcher = new CallWatcherActor(this.conn, this.tabActor);
     this._callWatcher.onCall = this._onContentFunctionCall;
     this._callWatcher.setup({
       tracedGlobals: AUDIO_GLOBALS,
       startRecording: true,
       performReload: reload
     });
   }, {
@@ -316,19 +339,19 @@ let WebAudioActor = exports.WebAudioActo
     }
     else if (WebAudioFront.NODE_CREATION_METHODS.has(name)) {
       this._handleCreationCall(functionCall);
     }
   },
 
   _handleRoutingCall: function(functionCall) {
     let { caller, args, window, name } = functionCall.details;
-    let source = unwrap(caller);
-    let dest = unwrap(args[0]);
-    let isAudioParam = dest instanceof unwrap(window.AudioParam);
+    let source = caller;
+    let dest = args[0];
+    let isAudioParam = dest instanceof window.AudioParam;
 
     // audionode.connect(param)
     if (name === "connect" && isAudioParam) {
       this._onConnectParam(source, dest);
     }
     // audionode.connect(node)
     else if (name === "connect") {
       this._onConnectNode(source, dest);
@@ -344,34 +367,35 @@ let WebAudioActor = exports.WebAudioActo
     // Keep track of the first node created, so we can alert
     // the front end that an audio context is being used since
     // we're not hooking into the constructor itself, just its
     // instance's methods.
     if (!this._firstNodeCreated) {
       // Fire the start-up event if this is the first node created
       // and trigger a `create-node` event for the context destination
       this._onStartContext();
-      this._onCreateNode(unwrap(caller.destination));
+      this._onCreateNode(caller.destination);
       this._firstNodeCreated = true;
     }
     this._onCreateNode(result);
   },
 
   /**
    * Stops listening for document global changes and puts this actor
    * to hibernation. This method is called automatically just before the
    * actor is destroyed.
    */
   finalize: method(function() {
     if (!this._initialized) {
       return;
     }
+    this.tabActor = null;
     this._initialized = false;
+    this._nativeToActorID = null;
     this._callWatcher.eraseRecording();
-
     this._callWatcher.finalize();
     this._callWatcher = null;
   }, {
    oneway: true
   }),
 
   /**
    * Events emitted by this actor.
@@ -407,50 +431,55 @@ let WebAudioActor = exports.WebAudioActo
   },
 
   /**
    * Helper for constructing an AudioNodeActor, assigning to
    * internal weak map, and tracking via `manage` so it is assigned
    * an `actorID`.
    */
   _constructAudioNode: function (node) {
+    // Ensure AudioNode is wrapped.
+    node = new XPCNativeWrapper(node);
+
     let actor = new AudioNodeActor(this.conn, node);
     this.manage(actor);
-    this._nodeActors.set(node, actor);
+    this._nativeToActorID.set(node.id, actor.actorID);
     return actor;
   },
 
   /**
    * Takes an AudioNode and returns the stored actor for it.
    * In some cases, we won't have an actor stored (for example,
    * connecting to an AudioDestinationNode, since it's implicitly
    * created), so make a new actor and store that.
    */
-  _actorFor: function (node) {
-    let actor = this._nodeActors.get(node);
-    if (!actor) {
-      actor = this._constructAudioNode(node);
-    }
+  _getActorByNativeID: function (nativeID) {
+    // Ensure we have a Number, rather than a string
+    // return via notification.
+    nativeID = ~~nativeID;
+
+    let actorID = this._nativeToActorID.get(nativeID);
+    let actor = actorID != null ? this.conn.getActor(actorID) : null;
     return actor;
   },
 
   /**
    * Called on first audio node creation, signifying audio context usage
    */
   _onStartContext: function () {
-    events.emit(this, "start-context");
+    emit(this, "start-context");
   },
 
   /**
    * Called when one audio node is connected to another.
    */
   _onConnectNode: function (source, dest) {
-    let sourceActor = this._actorFor(source);
-    let destActor = this._actorFor(dest);
-    events.emit(this, "connect-node", {
+    let sourceActor = this._getActorByNativeID(source.id);
+    let destActor = this._getActorByNativeID(dest.id);
+    emit(this, "connect-node", {
       source: sourceActor,
       dest: destActor
     });
   },
 
   /**
    * Called when an audio node is connected to an audio param.
    * Implement in bug 986705
@@ -458,38 +487,38 @@ let WebAudioActor = exports.WebAudioActo
   _onConnectParam: function (source, dest) {
     // TODO bug 986705
   },
 
   /**
    * Called when an audio node is disconnected.
    */
   _onDisconnectNode: function (node) {
-    let actor = this._actorFor(node);
-    events.emit(this, "disconnect-node", actor);
+    let actor = this._getActorByNativeID(node.id);
+    emit(this, "disconnect-node", actor);
   },
 
   /**
    * Called when a parameter changes on an audio node
    */
   _onParamChange: function (node, param, value) {
-    let actor = this._actorFor(node);
-    events.emit(this, "param-change", {
+    let actor = this._getActorByNativeID(node.id);
+    emit(this, "param-change", {
       source: actor,
       param: param,
       value: value
     });
   },
 
   /**
    * Called on node creation.
    */
   _onCreateNode: function (node) {
     let actor = this._constructAudioNode(node);
-    events.emit(this, "create-node", actor);
+    emit(this, "create-node", actor);
   }
 });
 
 /**
  * The corresponding Front object for the WebAudioActor.
  */
 let WebAudioFront = exports.WebAudioFront = protocol.FrontClass(WebAudioActor, {
   initialize: function(client, { webaudioActor }) {
@@ -525,23 +554,37 @@ function isAudioParam (node, prop) {
 function constructError (err) {
   return {
     message: err.message,
     type: err.constructor.name
   };
 }
 
 /**
+ * Creates and returns a JSON-able response used to indicate
+ * attempt to access an AudioNode that has been GC'd.
+ *
+ * @return Object
+ */
+function CollectedAudioNodeError () {
+  return {
+    message: "AudioNode has been garbage collected and can no longer be reached.",
+    type: "UnreachableAudioNode"
+  };
+}
+
+/**
  * Takes an object and converts it's `toString()` form, like
- * "[object OscillatorNode]" or "[object Float32Array]"
+ * "[object OscillatorNode]" or "[object Float32Array]",
+ * or XrayWrapper objects like "[object XrayWrapper [object Array]]"
  * to a string of just the constructor name, like "OscillatorNode",
  * or "Float32Array".
  */
 function getConstructorName (obj) {
-  return obj.toString().match(/\[object (.*)\]$/)[1];
+  return obj.toString().match(/\[object ([^\[\]]*)\]\]?$/)[1];
 }
 
 /**
  * Create a grip-like object to pass in renderable information
  * to the front-end for things like Float32Arrays, AudioBuffers,
  * without tracking them in an actor pool.
  */
 function createObjectGrip (value) {
@@ -549,11 +592,8 @@ function createObjectGrip (value) {
     type: "object",
     preview: {
       kind: "ObjectWithText",
       text: ""
     },
     class: getConstructorName(value)
   };
 }
-function unwrap (obj) {
-  return XPCNativeWrapper.unwrap(obj);
-}