Bug 1363107 - Check if the startTime is set before using it in SampleAnimationForEachNode; r=hiro, a=gchang
authorBrian Birtles <birtles@gmail.com>
Wed, 17 May 2017 18:51:00 -0400
changeset 394019 9bbb4cd88b37f24ae7ebe42da5b99b428206f093
parent 394018 e609219b9f585e3400d1c7a4f6947e6b74554766
child 394020 72da880beaeb8018297ae114eed9d7b04295f06d
push id7331
push userryanvm@gmail.com
push dateFri, 19 May 2017 17:36:20 +0000
treeherdermozilla-beta@72da880beaeb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershiro, gchang
bugs1363107, 1334583
milestone54.0
Bug 1363107 - Check if the startTime is set before using it in SampleAnimationForEachNode; r=hiro, a=gchang We are seeing occasional failed release assertions from calling animation.startTime().get_TimeDuration() in SampleAnimationForEachNode on Windows. My theory is that in some circumstances (perhaps graphic-driver related?) when creating a layer transaction we fail to call Layer::StartPendingAnimations and end up sending pending animations to the compositor. Prior to bug 1334583 that would have only triggered a debug assertion so it may have gone unnoticed if it depends on the system configuration. This patch makes us check that the startTime is set before we try to access it in order to avoid triggering a release-time assertion. If the startTime is not set we will use the hold time which should give us the correct behavior for a still-pending animation. (Furthermore, the holdTime is set unconditionally when we create animations so it should be correct -- but even if it were not set explicitly, its initial zero value would still likely produce a reasonable result until the start time was updated on a subsequent layer transaction. At very least, it should not crash. Likewise, if it was set to an incorrect value.) This patch also strengthens the debug assertion in SampleAnimationForEachNode to check that not only is start time not-null, but that it is set to a TimeDuration since MaybeTimeDuration also includes a third uninitialized "None" state.
gfx/layers/AnimationHelper.cpp
--- a/gfx/layers/AnimationHelper.cpp
+++ b/gfx/layers/AnimationHelper.cpp
@@ -94,23 +94,26 @@ AnimationHelper::SampleAnimationForEachN
   // Process in order, since later aAnimations override earlier ones.
   for (size_t i = 0, iEnd = aAnimations.Length(); i < iEnd; ++i) {
     Animation& animation = aAnimations[i];
     AnimData& animData = aAnimationData[i];
 
     activeAnimations = true;
 
     MOZ_ASSERT((!animation.originTime().IsNull() &&
-                animation.startTime().type() != MaybeTimeDuration::Tnull_t) ||
+                animation.startTime().type() ==
+                  MaybeTimeDuration::TTimeDuration) ||
                animation.isNotPlaying(),
                "If we are playing, we should have an origin time and a start"
                " time");
-    // If the animation is not currently playing , e.g. paused or
+    // If the animation is not currently playing, e.g. paused or
     // finished, then use the hold time to stay at the same position.
-    TimeDuration elapsedDuration = animation.isNotPlaying()
+    TimeDuration elapsedDuration =
+      animation.isNotPlaying() ||
+      animation.startTime().type() != MaybeTimeDuration::TTimeDuration
       ? animation.holdTime()
       : (aPoint - animation.originTime() -
          animation.startTime().get_TimeDuration())
         .MultDouble(animation.playbackRate());
     TimingParams timing;
     timing.mDuration.emplace(animation.duration());
     timing.mDelay = animation.delay();
     timing.mEndDelay = animation.endDelay();