Bug 1247243 - Animations are shown only every 2 reloads. r?pbrosset
MozReview-Commit-ID: 71XsHc9WXHw
--- a/devtools/client/animationinspector/animation-controller.js
+++ b/devtools/client/animationinspector/animation-controller.js
@@ -140,23 +140,16 @@ 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) {
@@ -176,22 +169,19 @@ var AnimationsController = {
if (this.destroyed) {
yield this.destroyed.promise;
return;
}
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;
}
this.destroyed.resolve();
}),
@@ -230,17 +220,17 @@ var AnimationsController = {
this.nodeFront === gInspector.selection.nodeFront) {
return;
}
let done = gInspector.updating("animationscontroller");
if (!gInspector.selection.isConnected() ||
!gInspector.selection.isElementNode()) {
- yield this.destroyAnimationPlayers();
+ this.destroyAnimationPlayers();
this.emit(this.PLAYERS_UPDATED_EVENT);
done();
return;
}
this.nodeFront = gInspector.selection.nodeFront;
yield this.refreshAnimationPlayers(this.nodeFront);
this.emit(this.PLAYERS_UPDATED_EVENT, this.animationPlayers);
@@ -326,17 +316,17 @@ var AnimationsController = {
// AnimationPlayerFront objects are managed by this controller. They are
// retrieved when refreshAnimationPlayers is called, stored in the
// animationPlayers array, and destroyed when refreshAnimationPlayers is
// called again.
animationPlayers: [],
refreshAnimationPlayers: Task.async(function*(nodeFront) {
- yield this.destroyAnimationPlayers();
+ this.destroyAnimationPlayers();
this.animationPlayers = yield this.animationsFront
.getAnimationPlayersForNode(nodeFront);
// 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);
@@ -348,18 +338,16 @@ var AnimationsController = {
// 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") {
- // 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);
},
@@ -378,24 +366,14 @@ var AnimationsController = {
if (!state.documentCurrentTime) {
return false;
}
time = Math.max(time, state.documentCurrentTime);
}
return time;
},
- destroyAnimationPlayers: Task.async(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();
- }
+ destroyAnimationPlayers: function() {
this.animationPlayers = [];
- })
+ }
};
EventEmitter.decorate(AnimationsController);
--- a/devtools/client/animationinspector/components/animation-timeline.js
+++ b/devtools/client/animationinspector/components/animation-timeline.js
@@ -139,16 +139,17 @@ AnimationsTimeline.prototype = {
}
this[name] = [];
},
unrender: function() {
for (let animation of this.animations) {
animation.off("changed", this.onAnimationStateChanged);
}
+ this.stopAnimatingScrubber();
TimeScale.reset();
this.destroySubComponents("targetNodes");
this.destroySubComponents("timeBlocks");
this.destroySubComponents("details", [{
event: "frame-selected",
fn: this.onFrameSelected
}]);
this.animationsEl.innerHTML = "";
--- a/devtools/client/animationinspector/test/browser_animation_playerFronts_are_refreshed.js
+++ b/devtools/client/animationinspector/test/browser_animation_playerFronts_are_refreshed.js
@@ -23,29 +23,14 @@ add_task(function*() {
"One AnimationPlayerFront has been created");
info("Selecting a node with mutliple animations");
yield selectNode(".multi", inspector);
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);
});
--- a/devtools/server/actors/animation.js
+++ b/devtools/server/actors/animation.js
@@ -565,19 +565,22 @@ var AnimationsActor = exports.Animations
* /devtools/server/actors/inspector
*/
getAnimationPlayersForNode: method(function(nodeActor) {
let animations = [
...nodeActor.rawNode.getAnimations(),
...this.getAllAnimations(nodeActor.rawNode)
];
- // No care is taken here to destroy the previously stored actors because it
- // is assumed that the client is responsible for lifetimes of actors.
+ // Destroy previously stored 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);
}
// When a front requests the list of players for a node, start listening
// for animation mutations on this node to send updates to the front, until
// either getAnimationPlayersForNode is called again or
--- a/devtools/server/tests/browser/browser_animation_playPauseSeveral.js
+++ b/devtools/server/tests/browser/browser_animation_playPauseSeveral.js
@@ -9,18 +9,16 @@
// List of selectors that match "all" animated nodes in the test page.
// This list misses a bunch of animated nodes on purpose. Only the ones that
// have infinite animations are listed. This is done to avoid intermittents
// caused when finite animations are already done playing by the time the test
// 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} =
yield initAnimationsFrontForUrl(MAIN_DOMAIN + "animation.html");
info("Pause all animations in the test document");
yield animations.pauseAll();
yield checkStates(walker, animations, ALL_ANIMATED_NODES, "paused");
@@ -32,29 +30,44 @@ add_task(function*() {
info("Pause all animations in the test document using toggleAll");
yield animations.toggleAll();
yield checkStates(walker, animations, ALL_ANIMATED_NODES, "paused");
info("Play all animations in the test document using toggleAll");
yield animations.toggleAll();
yield checkStates(walker, animations, ALL_ANIMATED_NODES, "running");
- info("Pause a given list of animations only");
- let players = [];
- for (let selector of SOME_ANIMATED_NODES) {
- let [player] = yield getPlayersFor(walker, animations, selector);
- 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 all animations from multiple animated node using toggleSeveral");
+ let players = yield getPlayersFor(walker, animations,
+ [".multiple-animations"]);
+ is(players.length, 2, "Node has 2 animation players");
+ yield animations.toggleSeveral(players, false);
+ 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("Play the same list of animations");
- yield animations.toggleSeveral(players, false);
- yield checkStates(walker, animations, ALL_ANIMATED_NODES, "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();
});
function* checkStates(walker, animations, selectors, playState) {
info("Checking the playState of all the nodes that have infinite running " +
"animations");
--- a/devtools/server/tests/browser/browser_animation_setCurrentTime.js
+++ b/devtools/server/tests/browser/browser_animation_setCurrentTime.js
@@ -49,25 +49,26 @@ function* testSetCurrentTime(walker, ani
let updatedState2 = yield player.getCurrentState();
is(Math.round(updatedState2.currentTime - updatedState1.currentTime), -2000,
"The currentTime was updated to -2s");
}
function* testSetCurrentTimes(walker, animations) {
ok(animations.setCurrentTimes, "The AnimationsActor has the right method");
- info("Retrieve multiple animated nodes and their animation players");
- let node1 = yield walker.querySelector(walker.rootNode, ".simple-animation");
- let player1 = (yield animations.getAnimationPlayersForNode(node1))[0];
- let node2 = yield walker.querySelector(walker.rootNode, ".delayed-animation");
- let player2 = (yield animations.getAnimationPlayersForNode(node2))[0];
+ info("Retrieve multiple animated node and its animation players");
+
+ let nodeMulti = yield walker.querySelector(walker.rootNode,
+ ".multiple-animations");
+ 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");
- let state1 = yield player1.getCurrentState();
- let state2 = yield player2.getCurrentState();
- is(state1.playState, "paused", "Player 1 is paused");
- is(state2.playState, "paused", "Player 2 is paused");
- is(state1.currentTime, 500, "Player 1 has the right currentTime");
- is(state2.currentTime, 500, "Player 2 has the right currentTime");
+ info("Get the states of players and verify their correctness");
+ for (let i = 0; i < players.length; i++) {
+ let state = yield players[i].getCurrentState();
+ is(state.playState, "paused", `Player ${i + 1} is paused`);
+ is(state.currentTime, 500, `Player ${i + 1} has the right currentTime`);
+ }
}
new file mode 100644
--- /dev/null
+++ b/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>
--- a/devtools/server/tests/mochitest/chrome.ini
+++ b/devtools/server/tests/mochitest/chrome.ini
@@ -1,12 +1,13 @@
[DEFAULT]
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
director-helpers.js
hello-actor.js
inspector_getImageData.html
inspector-delay-image-response.sjs
inspector-helpers.js
@@ -17,16 +18,17 @@ support-files =
large-image.jpg
memory-helpers.js
memprof-helpers.js
nonchrome_unsafeDereference.html
small-image.gif
setup-in-child.js
setup-in-parent.js
+[test_animation_actor-lifetime.html]
[test_connection-manager.html]
skip-if = buildapp == 'mulet'
[test_connectToChild.html]
skip-if = buildapp == 'mulet'
[test_css-logic.html]
[test_css-logic-inheritance.html]
[test_css-logic-media-queries.html]
[test_css-logic-specificity.html]
new file mode 100644
--- /dev/null
+++ b/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>