Bug 1229913 - Prevent race conditions when there are many animation mutations; r=miker draft
authorPatrick Brosset <pbrosset@mozilla.com>
Mon, 01 Feb 2016 15:58:04 +0100
changeset 327608 447e247f80805b88746e5ad76d0c325812e976fd
parent 327548 71957edda717276df5062b504c77eecbe2dca757
child 513724 595433393ee072d8955998f2cd031d4ca0a1a796
push id10264
push userpbrosset@mozilla.com
push dateMon, 01 Feb 2016 14:59:54 +0000
reviewersmiker
bugs1229913
milestone47.0a1
Bug 1229913 - Prevent race conditions when there are many animation mutations; r=miker
devtools/client/animationinspector/animation-controller.js
--- a/devtools/client/animationinspector/animation-controller.js
+++ b/devtools/client/animationinspector/animation-controller.js
@@ -139,16 +139,23 @@ var AnimationsController = {
     this.onAnimationMutations = this.onAnimationMutations.bind(this);
 
     let target = gToolbox.target;
     this.animationsFront = new AnimationsFront(target.client, target.form);
 
     // Expose actor capabilities.
     this.traits = yield getServerTraits(target);
 
+    // We want to handle animation mutation events synchronously to avoid race
+    // conditions when there are many rapid mutations. So when a mutation occurs
+    // and animations are removed, we don't release the corresponding actors
+    // in a blocking way, we just release asynchronously and don't wait for
+    // completion, but instead store the promise in this array.
+    this.nonBlockingPlayerReleases = [];
+
     if (this.destroyed) {
       console.warn("Could not fully initialize the AnimationsController");
       return;
     }
 
     // Let the AnimationsActor know what WalkerActor we're using. This will
     // come in handy later to return references to DOM Nodes.
     if (this.traits.hasSetWalkerActor) {
@@ -171,16 +178,19 @@ var AnimationsController = {
       return;
     }
     this.destroyed = promise.defer();
 
     this.stopListeners();
     yield this.destroyAnimationPlayers();
     this.nodeFront = null;
 
+    // Finish releasing players that haven't been released yet.
+    yield Promise.all(this.nonBlockingPlayerReleases);
+
     if (this.animationsFront) {
       this.animationsFront.destroy();
       this.animationsFront = null;
     }
 
     this.destroyed.resolve();
   }),
 
@@ -326,34 +336,35 @@ var AnimationsController = {
     // Start listening for animation mutations only after the first method call
     // otherwise events won't be sent.
     if (!this.isListeningToMutations && this.traits.hasMutationEvents) {
       this.animationsFront.on("mutations", this.onAnimationMutations);
       this.isListeningToMutations = true;
     }
   }),
 
-  onAnimationMutations: Task.async(function*(changes) {
+  onAnimationMutations: function(changes) {
     // Insert new players into this.animationPlayers when new animations are
     // added.
     for (let {type, player} of changes) {
       if (type === "added") {
         this.animationPlayers.push(player);
       }
 
       if (type === "removed") {
-        yield player.release();
+        // Don't wait for the release request to complete, we can do that later.
+        this.nonBlockingPlayerReleases.push(player.release());
         let index = this.animationPlayers.indexOf(player);
         this.animationPlayers.splice(index, 1);
       }
     }
 
     // Let the UI know the list has been updated.
     this.emit(this.PLAYERS_UPDATED_EVENT, this.animationPlayers);
-  }),
+  },
 
   /**
    * Get the latest known current time of document.timeline.
    * This value is sent along with all AnimationPlayerActors' states, but it
    * isn't updated after that, so this function loops over all know animations
    * to find the highest value.
    * @return {Number|Boolean} False is returned if this server version doesn't
    * provide document's current time.