Bug 1247243 - Animations are shown only every 2 reloads. r=pbrosset

MozReview-Commit-ID: 71XsHc9WXHw

devtools/client/animationinspector/animation-controller.js

@@ -148,13 +148,6 @@ var AnimationsController = {
     // 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;
@@ -184,12 +177,9 @@ var AnimationsController = {
     this.destroyed = promise.defer();
 
     this.stopListeners();
-    yield this.destroyAnimationPlayers();
+    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;
@@ -238,7 +228,7 @@ var AnimationsController = {
 
     if (!gInspector.selection.isConnected() ||
         !gInspector.selection.isElementNode()) {
-      yield this.destroyAnimationPlayers();
+      this.destroyAnimationPlayers();
       this.emit(this.PLAYERS_UPDATED_EVENT);
       done();
       return;
@@ -334,7 +324,7 @@ var AnimationsController = {
   animationPlayers: [],
 
   refreshAnimationPlayers: Task.async(function*(nodeFront) {
-    yield this.destroyAnimationPlayers();
+    this.destroyAnimationPlayers();
 
     this.animationPlayers = yield this.animationsFront
                                       .getAnimationPlayersForNode(nodeFront);
@@ -356,8 +346,6 @@ var AnimationsController = {
       }
 
       if (type === "removed") {
-        // 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);
       }
@@ -386,19 +374,9 @@ var AnimationsController = {
     return time;
   },
 
-  destroyAnimationPlayers: Task.async(function*() {
+  destroyAnimationPlayers: function() {
-    // Let the server know that we're not interested in receiving updates about
-    // players for the current node. We're either being destroyed or a new node
-    // has been selected.
-    if (this.traits.hasMutationEvents) {
-      yield this.animationsFront.stopAnimationPlayerUpdates();
-    }
-
-    for (let front of this.animationPlayers) {
-      yield front.release();
-    }
     this.animationPlayers = [];
-  })
+  }
 };
 
 EventEmitter.decorate(AnimationsController);

devtools/client/animationinspector/components/animation-timeline.js

@@ -154,6 +154,7 @@ AnimationsTimeline.prototype = {
     for (let animation of this.animations) {
       animation.off("changed", this.onAnimationStateChanged);
     }
+    this.stopAnimatingScrubber();
     TimeScale.reset();
     this.destroySubComponents("targetNodes");
     this.destroySubComponents("timeBlocks");

devtools/client/animationinspector/test/browser_animation_playerFronts_are_refreshed.js

@@ -28,24 +28,9 @@ add_task(function*() {
   is(controller.animationPlayers.length, 2,
     "2 AnimationPlayerFronts have been created");
 
-  // Hold on to one of the AnimationPlayerFront objects and mock its release
-  // method to test that it is released correctly and that its auto-refresh is
-  // stopped.
-  let retainedFront = controller.animationPlayers[0];
-  let oldRelease = retainedFront.release;
-  let releaseCalled = false;
-  retainedFront.release = () => {
-    releaseCalled = true;
-  };
-
   info("Selecting a node with no animations");
   yield selectNode(".still", inspector);
 
   is(controller.animationPlayers.length, 0,
     "There are no more AnimationPlayerFront objects");
-
-  info("Checking the destroyed AnimationPlayerFront object");
-  ok(releaseCalled, "The AnimationPlayerFront has been released");
-
-  yield oldRelease.call(retainedFront);
 });

devtools/server/actors/animation.js

@@ -561,6 +561,10 @@ var AnimationsActor = exports.AnimationsActor = ActorClass({
   /**
    * Retrieve the list of AnimationPlayerActor actors for currently running
    * animations on a node and its descendants.
+   * Note that calling this method a second time will destroy all previously
+   * retrieved AnimationPlayerActors. Indeed, the lifecycle of these actors
+   * is managed here on the server and tied to getAnimationPlayersForNode
+   * being called.
    * @param {NodeActor} nodeActor The NodeActor as defined in
    * /devtools/server/actors/inspector
    */
@@ -570,9 +574,12 @@ var AnimationsActor = exports.AnimationsActor = ActorClass({
       ...this.getAllAnimations(nodeActor.rawNode)
     ];
 
-    // No care is taken here to destroy the previously stored actors because it
+    // Destroy previously stored actors
-    // is assumed that the client is responsible for lifetimes of actors.
+    if (this.actors) {
+      this.actors.forEach(actor => actor.destroy());
+    }
     this.actors = [];
+
     for (let i = 0; i < animations.length; i++) {
       let actor = AnimationPlayerActor(this, animations[i]);
       this.actors.push(actor);

devtools/server/tests/browser/browser_animation_playPauseSeveral.js

@@ -14,8 +14,6 @@
 // runs.
 const ALL_ANIMATED_NODES = [".simple-animation", ".multiple-animations",
                             ".delayed-animation"];
-// List of selectors that match some animated nodes in the test page only.
-const SOME_ANIMATED_NODES = [".simple-animation", ".delayed-animation"];
 
 add_task(function*() {
   let {client, walker, animations} =
@@ -37,19 +35,34 @@ add_task(function*() {
   yield animations.toggleAll();
   yield checkStates(walker, animations, ALL_ANIMATED_NODES, "running");
 
-  info("Pause a given list of animations only");
+  info("Play all animations from multiple animated node using toggleSeveral");
-  let players = [];
+  let players = yield getPlayersFor(walker, animations,
-  for (let selector of SOME_ANIMATED_NODES) {
+                                   [".multiple-animations"]);
-    let [player] = yield getPlayersFor(walker, animations, selector);
+  is(players.length, 2, "Node has 2 animation players");
-    players.push(player);
-  }
-  yield animations.toggleSeveral(players, true);
-  yield checkStates(walker, animations, SOME_ANIMATED_NODES, "paused");
-  yield checkStates(walker, animations, [".multiple-animations"], "running");
-
-  info("Play the same list of animations");
   yield animations.toggleSeveral(players, false);
-  yield checkStates(walker, animations, ALL_ANIMATED_NODES, "running");
+  let state1 = yield players[0].getCurrentState();
+  is(state1.playState, "running",
+    "The playState of the first player is running");
+  let state2 = yield players[1].getCurrentState();
+  is(state2.playState, "running",
+    "The playState of the second player is running");
+
+  info("Pause one animation from a multiple animated node using toggleSeveral");
+  yield animations.toggleSeveral([players[0]], true);
+  state1 = yield players[0].getCurrentState();
+  is(state1.playState, "paused", "The playState of the first player is paused");
+  state2 = yield players[1].getCurrentState();
+  is(state2.playState, "running",
+    "The playState of the second player is running");
+
+  info("Play the same animation");
+  yield animations.toggleSeveral([players[0]], false);
+  state1 = yield players[0].getCurrentState();
+  is(state1.playState, "running",
+    "The playState of the first player is running");
+  state2 = yield players[1].getCurrentState();
+  is(state2.playState, "running",
+    "The playState of the second player is running");
 
   yield closeDebuggerClient(client);
   gBrowser.removeCurrentTab();

devtools/server/tests/browser/browser_animation_setCurrentTime.js

@@ -54,20 +54,21 @@ function* testSetCurrentTime(walker, animations) {
 function* testSetCurrentTimes(walker, animations) {
   ok(animations.setCurrentTimes, "The AnimationsActor has the right method");
 
-  info("Retrieve multiple animated nodes and their animation players");
+  info("Retrieve multiple animated node and its animation players");
-  let node1 = yield walker.querySelector(walker.rootNode, ".simple-animation");
+
-  let player1 = (yield animations.getAnimationPlayersForNode(node1))[0];
+  let nodeMulti = yield walker.querySelector(walker.rootNode,
-  let node2 = yield walker.querySelector(walker.rootNode, ".delayed-animation");
+    ".multiple-animations");
-  let player2 = (yield animations.getAnimationPlayersForNode(node2))[0];
+  let players = (yield animations.getAnimationPlayersForNode(nodeMulti));
+
+  ok(players.length > 1, "Node has more than 1 animation player");
 
   info("Try to set multiple current times at once");
-  yield animations.setCurrentTimes([player1, player2], 500, true);
+  yield animations.setCurrentTimes(players, 500, true);
 
-  info("Get the states of both players and verify their correctness");
+  info("Get the states of players and verify their correctness");
-  let state1 = yield player1.getCurrentState();
+  for (let i = 0; i < players.length; i++) {
-  let state2 = yield player2.getCurrentState();
+    let state = yield players[i].getCurrentState();
-  is(state1.playState, "paused", "Player 1 is paused");
+    is(state.playState, "paused", `Player ${i + 1} is paused`);
-  is(state2.playState, "paused", "Player 2 is paused");
+    is(state.currentTime, 500, `Player ${i + 1} has the right currentTime`);
-  is(state1.currentTime, 500, "Player 1 has the right currentTime");
+  }
-  is(state2.currentTime, 500, "Player 2 has the right currentTime");
 }

devtools/server/tests/mochitest/animation-data.html

@@ -0,0 +1,120 @@
+<html>
+<head>
+  <meta charset="UTF-8">
+  <title>Animation Test Data</title>
+  <style>
+    .ball {
+      width: 80px;
+      height: 80px;
+      border-radius: 50%;
+      background: #f06;
+
+      position: absolute;
+    }
+
+    .still {
+      top: 0;
+      left: 10px;
+    }
+
+    .animated {
+      top: 100px;
+      left: 10px;
+
+      animation: simple-animation 2s infinite alternate;
+    }
+
+    .multi {
+      top: 200px;
+      left: 10px;
+
+      animation: simple-animation 2s infinite alternate,
+                 other-animation 5s infinite alternate;
+    }
+
+    .delayed {
+      top: 300px;
+      left: 10px;
+      background: rebeccapurple;
+
+      animation: simple-animation 3s 60s 10;
+    }
+
+    .multi-finite {
+      top: 400px;
+      left: 10px;
+      background: yellow;
+
+      animation: simple-animation 3s,
+                 other-animation 4s;
+    }
+
+    .short {
+      top: 500px;
+      left: 10px;
+      background: red;
+
+      animation: simple-animation 2s;
+    }
+
+    .long {
+      top: 600px;
+      left: 10px;
+      background: blue;
+
+      animation: simple-animation 120s;
+    }
+
+    .negative-delay {
+      top: 700px;
+      left: 10px;
+      background: gray;
+
+      animation: simple-animation 15s -10s;
+      animation-fill-mode: forwards;
+    }
+
+    .no-compositor {
+      top: 0;
+      right: 10px;
+      background: gold;
+
+      animation: no-compositor 10s cubic-bezier(.57,-0.02,1,.31) forwards;
+    }
+
+    @keyframes simple-animation {
+      100% {
+        transform: translateX(300px);
+      }
+    }
+
+    @keyframes other-animation {
+      100% {
+        background: blue;
+      }
+    }
+
+    @keyframes no-compositor {
+      100% {
+        margin-right: 600px;
+      }
+    }
+  </style>
+  <script type="text/javascript">
+    window.onload = function() {
+      window.opener.postMessage('ready', '*');
+    };
+  </script>
+</head>
+</body>
+  <div class="ball still"></div>
+  <div class="ball animated"></div>
+  <div class="ball multi"></div>
+  <div class="ball delayed"></div>
+  <div class="ball multi-finite"></div>
+  <div class="ball short"></div>
+  <div class="ball long"></div>
+  <div class="ball negative-delay"></div>
+  <div class="ball no-compositor"></div>
+</body>
+</html>

devtools/server/tests/mochitest/chrome.ini

@@ -2,6 +2,7 @@
 tags = devtools
 skip-if = buildapp == 'b2g' || os == 'android'
 support-files =
+  animation-data.html
   Debugger.Source.prototype.element.js
   Debugger.Source.prototype.element-2.js
   Debugger.Source.prototype.element.html
@@ -22,6 +23,7 @@ support-files =
   setup-in-child.js
   setup-in-parent.js
 
+[test_animation_actor-lifetime.html]
 [test_connection-manager.html]
 skip-if = buildapp == 'mulet'
 [test_connectToChild.html]

devtools/server/tests/mochitest/test_animation_actor-lifetime.html

@@ -0,0 +1,92 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1247243
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 1247243</title>
+
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css">
+  <script type="application/javascript;version=1.8" src="inspector-helpers.js"></script>
+  <script type="application/javascript;version=1.8">
+window.onload = function() {
+  const Ci = Components.interfaces;
+  const {AnimationsFront} = require("devtools/server/actors/animation");
+  const {InspectorFront} = require("devtools/server/actors/inspector");
+
+  SimpleTest.waitForExplicitFinish();
+
+  let gWalker = null;
+  let gClient = null;
+  let animationsFront = null;
+
+  addTest(function setup() {
+    info ("Setting up inspector and animation actors.");
+
+    let url = document.getElementById("animationContent").href;
+    attachURL(url, function(err, client, tab, doc) {
+      let inspector = InspectorFront(client, tab);
+
+      animationsFront = new AnimationsFront(client, tab);
+
+      promiseDone(inspector.getWalker().then(walker => {
+        ok(walker, "getWalker() should return an actor.");
+        gClient = client;
+        gWalker = walker;
+      }).then(runNextTest));
+
+    });
+  });
+
+  addAsyncTest(function* testActorLifetime() {
+
+    info ("Testing animated node actor");
+    let animatedNodeActor = yield gWalker.querySelector(gWalker.rootNode,
+      ".animated");
+    yield animationsFront.getAnimationPlayersForNode(animatedNodeActor);
+
+    let serverConnection = animationsFront.conn._transport._serverConnection;
+    let animationsActor = serverConnection.getActor(animationsFront.actorID);
+
+    is(animationsActor.actors.length, 1,
+      "AnimationActor have 1 AnimationPlayerActors");
+
+    info ("Testing AnimationPlayerActors release");
+    let stillNodeActor = yield gWalker.querySelector(gWalker.rootNode,
+      ".still");
+    yield animationsFront.getAnimationPlayersForNode(stillNodeActor);
+    is(animationsActor.actors.length, 0,
+      "AnimationActor does not have any AnimationPlayerActors anymore");
+
+    info ("Testing multi animated node actor");
+    let multiNodeActor = yield gWalker.querySelector(gWalker.rootNode,
+      ".multi");
+    yield animationsFront.getAnimationPlayersForNode(multiNodeActor);
+    is(animationsActor.actors.length, 2,
+      "AnimationActor has now 2 AnimationPlayerActors");
+
+    info ("Testing single animated node actor");
+    yield animationsFront.getAnimationPlayersForNode(animatedNodeActor);
+    is(animationsActor.actors.length, 1,
+      "AnimationActor has only one AnimationPlayerActors");
+
+    info ("Testing AnimationPlayerActors release again");
+    yield animationsFront.getAnimationPlayersForNode(stillNodeActor);
+    is(animationsActor.actors.length, 0,
+      "AnimationActor does not have any AnimationPlayerActors anymore");
+
+    runNextTest();
+  });
+
+
+  runNextTest();
+};
+  </script>
+</head>
+<body>
+  <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1247243">Mozilla Bug 1247243</a>
+  <a id="animationContent" target="_blank" href="animation-data.html">Test Document</a>
+</body>
+</html>