commit 8188dec434a7b65ed44c8caf35967479754113b6
parent 92e5a93352e46c7d019cd79f362336116e7cd543
Author: Nicolas Chevobbe <nchevobbe@mozilla.com>
Date: Sun, 23 Nov 2025 22:45:24 +0000
Bug 2001590 - [devtools] Filter out `AnimationPlayerActor` instances that aren't handled by `AnimationsActor`. r=devtools-reviewers,bomsy.
`AnimationActor` `pauseSome`, `playSome`, `setCurrentTimes` and `setPlaybackRates`
all take an array of `AnimationPlayerActor` instances for which we'll perform
some operation on their underlying animation.
It can happen that some of those actors are not handled by the `AnimationsActor`
anymore (e.g. the animation stopped, but the client wasn't updated yet).
Performing any action on those unhandled player (pausing, setting time, …) might
actually trigger a new, unwanted, mutation, which will cause both the server
state and the client state to be out of date.
This patch filters out actors which are not into `AnimationsActor#actors` to avoid
such issue.
Differential Revision: https://phabricator.services.mozilla.com/D273562
Diffstat:
3 files changed, 139 insertions(+), 17 deletions(-)
diff --git a/devtools/server/actors/animation.js b/devtools/server/actors/animation.js
@@ -829,11 +829,20 @@ exports.AnimationsActor = class AnimationsActor extends Actor {
* @param {Array} actors A list of AnimationPlayerActor.
*/
pauseSome(actors) {
- for (const { player } of actors) {
- this.pauseSync(player);
+ const handledActors = [];
+ for (const actor of actors) {
+ // The client could call this with actors that we no longer handle, as it might
+ // not have received the mutations event yet for removed animations.
+ // In such case, ignore the actor, as pausing the animation again might trigger a
+ // new mutation, which would cause problems here and on the client.
+ if (!this.actors.includes(actor)) {
+ continue;
+ }
+ this.pauseSync(actor.player);
+ handledActors.push(actor);
}
- return this.waitForNextFrame(actors);
+ return this.waitForNextFrame(handledActors);
}
/**
@@ -842,22 +851,39 @@ exports.AnimationsActor = class AnimationsActor extends Actor {
* @param {Array} actors A list of AnimationPlayerActor.
*/
playSome(actors) {
- for (const { player } of actors) {
- this.playSync(player);
+ const handledActors = [];
+ for (const actor of actors) {
+ // The client could call this with actors that we no longer handle, as it might
+ // not have received the mutations event yet for removed animations.
+ // In such case, ignore the actor, as playing the animation again might trigger a
+ // new mutation, which would cause problems here and on the client.
+ if (!this.actors.includes(actor)) {
+ continue;
+ }
+ this.playSync(actor.player);
+ handledActors.push(actor);
}
- return this.waitForNextFrame(actors);
+ return this.waitForNextFrame(handledActors);
}
/**
* Set the current time of several animations at the same time.
*
- * @param {Array} players A list of AnimationPlayerActor.
+ * @param {Array} actors A list of AnimationPlayerActor.
* @param {number} time The new currentTime.
* @param {boolean} shouldPause Should the players be paused too.
*/
- setCurrentTimes(players, time, shouldPause) {
- for (const actor of players) {
+ setCurrentTimes(actors, time, shouldPause) {
+ const handledActors = [];
+ for (const actor of actors) {
+ // The client could call this with actors that we no longer handle, as it might
+ // not have received the mutations event yet for removed animations.
+ // In such case, ignore the actor, as setting the time might trigger a
+ // new mutation, which would cause problems here and on the client.
+ if (!this.actors.includes(actor)) {
+ continue;
+ }
const player = actor.player;
if (shouldPause) {
@@ -869,9 +895,10 @@ exports.AnimationsActor = class AnimationsActor extends Actor {
? time - actor.createdTime
: actor.createdTime - time;
player.currentTime = currentTime * Math.abs(player.playbackRate);
+ handledActors.push(actor);
}
- return this.waitForNextFrame(players);
+ return this.waitForNextFrame(handledActors);
}
/**
@@ -880,13 +907,20 @@ exports.AnimationsActor = class AnimationsActor extends Actor {
* @param {Array} actors A list of AnimationPlayerActor.
* @param {number} rate The new rate.
*/
- setPlaybackRates(players, rate) {
- return Promise.all(
- players.map(({ player }) => {
- player.updatePlaybackRate(rate);
- return player.ready;
- })
- );
+ setPlaybackRates(actors, rate) {
+ const readyPromises = [];
+ for (const actor of actors) {
+ // The client could call this with actors that we no longer handle, as it might
+ // not have received the mutations event yet for removed animations.
+ // In such case, ignore the actor, as setting the playback rate might trigger a
+ // new mutation, which would cause problems here and on the client.
+ if (!this.actors.includes(actor)) {
+ continue;
+ }
+ actor.player.updatePlaybackRate(rate);
+ readyPromises.push(actor.player.ready);
+ }
+ return Promise.all(readyPromises);
}
/**
diff --git a/devtools/server/tests/browser/browser.toml b/devtools/server/tests/browser/browser.toml
@@ -112,6 +112,8 @@ skip-if = [
["browser_animation_simple.js"]
+["browser_animation_unhandledActorPlayers.js"]
+
["browser_animation_updatedState.js"]
["browser_application_manifest.js"]
diff --git a/devtools/server/tests/browser/browser_animation_unhandledActorPlayers.js b/devtools/server/tests/browser/browser_animation_unhandledActorPlayers.js
@@ -0,0 +1,86 @@
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Check that calling AnimationsActor method taking AnimationPlayerActor arrays (pauseSome,
+// playSome, setCurrentTimes, setPlaybackRates) with instances that are not handled by
+// the AnimationsActor anymore doesn't throw nor trigger unexpected animations (see Bug 2001590).
+
+add_task(async function () {
+ const { target, walker, animations } = await initAnimationsFrontForUrl(
+ `data:text/html,<meta charset=utf8>${encodeURIComponent(`
+ <style>
+ #target {
+ animation: my-anim 1s infinite alternate;
+
+ &.still {
+ animation: none;
+ }
+ }
+ @keyframes my-anim {
+ to {
+ background-color: tomato;
+ }
+ }
+ </style>
+ <div id=target>Hello</div>`)}`
+ );
+
+ info("Retrieve an animated node");
+ const node = await walker.querySelector(walker.rootNode, "#target");
+
+ const getAnimationPlayersForTargetNode = () =>
+ animations.getAnimationPlayersForNode(node);
+
+ info("Retrieve the animation player for the node");
+ const players = await getAnimationPlayersForTargetNode();
+ is(players.length, 1, "Got one animation player");
+ const animationPlayer = players[0];
+
+ info("Stop the animation on the node");
+ await node.modifyAttributes([
+ {
+ attributeName: "class",
+ newValue: "still",
+ },
+ ]);
+
+ // Wait until we're not getting the animation anymore
+ await waitFor(async () => {
+ return (await getAnimationPlayersForTargetNode()).length === 0;
+ });
+
+ info("Call methodes with outdated animationplayer front");
+ const onPause = animations.pauseSome([animationPlayer]);
+ const onPlay = animations.playSome([animationPlayer]);
+ const onCurrentTimeSet = animations.setCurrentTimes(
+ [animationPlayer],
+ 1,
+ true
+ );
+ const onPlaybackRateSet = animations.setPlaybackRates([animationPlayer], 10);
+
+ await onPause;
+ ok(true, "pauseSome succeeded");
+
+ await onPlay;
+ ok(true, "playSome succedded");
+
+ await onCurrentTimeSet;
+ ok(true, "setCurrentTimes succedded");
+
+ await onPlaybackRateSet;
+ ok(true, "setPlaybackRates succedded");
+
+ // wait for a bit so we would get notified about new animations
+ await wait(500);
+ is(
+ (await getAnimationPlayersForTargetNode()).length,
+ 0,
+ "No players were created after calling those methods"
+ );
+
+ await target.destroy();
+ gBrowser.removeCurrentTab();
+});
