Bug 1467399 - Part 2: Guard from removed animation during updating the animation state. r?pbro
MozReview-Commit-ID: 9Ocb2tQuvoE
--- a/devtools/client/inspector/animation/animation.js
+++ b/devtools/client/inspector/animation/animation.js
@@ -321,20 +321,17 @@ class AnimationInspector {
animation.off("changed", this.onAnimationStateChanged);
}
}
// Update existing other animations as well since the currentTime would be proceeded
// sice the scrubber position is related the currentTime.
// Also, don't update the state of removed animations since React components
// may refer to the same instance still.
- await this.updateAnimations(animations);
-
- // Get rid of animations that were removed during async updateAnimations().
- animations = animations.filter(animation => !!animation.state.type);
+ animations = await this.updateAnimations(animations);
this.updateState(animations.concat(addedAnimations));
}
onElementPickerStarted() {
this.inspector.store.dispatch(updateElementPickerEnabled(true));
}
@@ -398,65 +395,65 @@ class AnimationInspector {
async setAnimationsCurrentTime(currentTime, shouldRefresh) {
this.stopAnimationsCurrentTimeTimer();
this.onAnimationsCurrentTimeUpdated(currentTime);
if (!shouldRefresh && this.isCurrentTimeSet) {
return;
}
- const { animations } = this.state;
+ let animations = this.state.animations;
this.isCurrentTimeSet = true;
try {
await this.doSetCurrentTimes(currentTime);
- await this.updateAnimations(animations);
+ animations = await this.updateAnimations(animations);
} catch (e) {
// Expected if we've already been destroyed or other node have been selected
// in the meantime.
console.error(e);
return;
}
this.isCurrentTimeSet = false;
if (shouldRefresh) {
- this.updateState([...animations]);
+ this.updateState(animations);
}
}
async setAnimationsPlaybackRate(playbackRate) {
- const animations = this.state.animations;
+ let animations = this.state.animations;
// "changed" event on each animation will fire respectively when the playback
// rate changed. Since for each occurrence of event, change of UI is urged.
// To avoid this, disable the listeners once in order to not capture the event.
this.setAnimationStateChangedListenerEnabled(false);
try {
await this.animationsFront.setPlaybackRates(animations, playbackRate);
- await this.updateAnimations(animations);
+ animations = await this.updateAnimations(animations);
} catch (e) {
// Expected if we've already been destroyed or other node have been selected
// in the meantime.
console.error(e);
return;
} finally {
this.setAnimationStateChangedListenerEnabled(true);
}
- await this.updateState([...animations]);
+ await this.updateState(animations);
}
async setAnimationsPlayState(doPlay) {
if (typeof this.hasPausePlaySome === "undefined") {
this.hasPausePlaySome =
await this.inspector.target.actorHasMethod("animations", "pauseSome");
}
- const { animations, timeScale } = this.state;
+ let { animations, timeScale } = this.state;
try {
if (doPlay && animations.every(animation =>
timeScale.getEndTime(animation) <= animation.state.currentTime)) {
await this.doSetCurrentTimes(0);
}
// If the server does not support pauseSome/playSome function, (which happens
@@ -469,25 +466,25 @@ class AnimationInspector {
await this.animationsFront.pauseSome(animations);
}
} else if (doPlay) {
await this.animationsFront.playAll();
} else {
await this.animationsFront.pauseAll();
}
- await this.updateAnimations(animations);
+ animations = await this.updateAnimations(animations);
} catch (e) {
// Expected if we've already been destroyed or other node have been selected
// in the meantime.
console.error(e);
return;
}
- await this.updateState([...animations]);
+ await this.updateState(animations);
}
/**
* Enable/disable the animation state change listener.
* If set true, observe "changed" event on current animations.
* Otherwise, quit observing the "changed" event.
*
* @param {Bool} isEnabled
@@ -624,41 +621,39 @@ class AnimationInspector {
? await this.animationsFront.getAnimationPlayersForNode(selection.nodeFront)
: [];
this.updateState(animations);
this.setAnimationStateChangedListenerEnabled(true);
done();
}
- updateAnimations(animations) {
- if (!animations.length) {
- return Promise.resolve();
- }
+ async updateAnimations(animations) {
+ let error = null;
- return new Promise((resolve, reject) => {
- let count = 0;
- let error = null;
-
- for (const animation of animations) {
+ const promises = animations.map(animation => {
+ return new Promise(resolve => {
animation.refreshState().catch(e => {
error = e;
}).finally(() => {
- count += 1;
+ resolve();
+ });
+ });
+ });
+ await Promise.all(promises);
- if (count === animations.length) {
- if (error) {
- reject(error);
- } else {
- resolve();
- }
- }
- });
- }
- });
+ if (error) {
+ throw new Error(error);
+ }
+
+ // Even when removal animation on inspected document, updateAnimations
+ // might be called before onAnimationsMutation due to the async timing.
+ // Return the animations as result of updateAnimations after getting rid of
+ // the animations since they should not display.
+ return animations.filter(anim => !!anim.state.type);
}
updateState(animations) {
// Animation inspector already destroyed
if (!this.inspector) {
return;
}