Bug 1467399 - Part 2: Guard from removed animation during updating the animation state. r=pbro
authorDaisuke Akatsuka <dakatsuka@mozilla.com>
Wed, 20 Jun 2018 20:11:42 +0900
changeset 479816 ba634b003ba14a436be1300b035761ef078e23d8
parent 479815 4dbd6590b2354c675d750e928357dd31fcdbe282
child 479817 ff4b9d0d9216094485af3be7e35c0443b2ee9d65
push id1757
push userffxbld-merge
push dateFri, 24 Aug 2018 17:02:43 +0000
treeherdermozilla-release@736023aebdb1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbro
bugs1467399
milestone62.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1467399 - Part 2: Guard from removed animation during updating the animation state. r=pbro MozReview-Commit-ID: 9Ocb2tQuvoE
devtools/client/inspector/animation/animation.js
--- 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;
     }