Bug 1322291 - Part 2: Make sure that the base style is set even if additive or accumulates animations are in the delay phase. r?birtles draft
authorHiroyuki Ikezoe <hiikezoe@mozilla-japan.org>
Wed, 21 Dec 2016 13:52:21 +0900
changeset 452170 f634da356ea9b58a3e00d2cd7df527ec48845d67
parent 452169 0522b9c09b877837b88395d0fb93ac0f85dced37
child 540164 2143179c4a768d6157350d4820dba3549002bc50
push id39335
push userhiikezoe@mozilla-japan.org
push dateWed, 21 Dec 2016 07:23:19 +0000
reviewersbirtles
bugs1322291
milestone53.0a1
Bug 1322291 - Part 2: Make sure that the base style is set even if additive or accumulates animations are in the delay phase. r?birtles Before this patch we skipped KeyframeEffectReadOnly::ComposeStyle() for animations that are not in effect. After this patch we call KeyframeEffectReadOnly::ComposeStyle() even if the animation is not in-effect state in order to prepare the base style for properties that can be run on the compositor because the in-effect animation will be sent to the compositor and might be composed onto the base style on the compositor after the animation gets out of its delay phase. MozReview-Commit-ID: FuAZv4jqVMJ
dom/animation/Animation.cpp
dom/animation/KeyframeEffectReadOnly.cpp
dom/animation/KeyframeEffectReadOnly.h
dom/animation/test/crashtests/1322291-1.html
dom/animation/test/crashtests/crashtests.list
dom/animation/test/style/file_missing-keyframe-on-compositor.html
--- a/dom/animation/Animation.cpp
+++ b/dom/animation/Animation.cpp
@@ -893,20 +893,16 @@ Animation::HasLowerCompositeOrderThan(co
 void
 Animation::ComposeStyle(RefPtr<AnimValuesStyleRule>& aStyleRule,
                         const nsCSSPropertyIDSet& aPropertiesToSkip)
 {
   if (!mEffect) {
     return;
   }
 
-  if (!IsInEffect()) {
-    return;
-  }
-
   // In order to prevent flicker, there are a few cases where we want to use
   // a different time for rendering that would otherwise be returned by
   // GetCurrentTime. These are:
   //
   // (a) For animations that are pausing but which are still running on the
   //     compositor. In this case we send a layer transaction that removes the
   //     animation but which also contains the animation values calculated on
   //     the main thread. To prevent flicker when this occurs we want to ensure
--- a/dom/animation/KeyframeEffectReadOnly.cpp
+++ b/dom/animation/KeyframeEffectReadOnly.cpp
@@ -383,16 +383,59 @@ KeyframeEffectReadOnly::CompositeValue(
 
   return CompositeValue(aProperty,
                         aValueToComposite,
                         result,
                         aCompositeOperation);
 }
 
 void
+KeyframeEffectReadOnly::EnsureBaseStylesForCompositor(
+  const nsCSSPropertyIDSet& aPropertiesToSkip)
+{
+  if (!mTarget) {
+    return;
+  }
+
+  RefPtr<nsStyleContext> styleContext;
+
+  for (const AnimationProperty& property : mProperties) {
+    if (!nsCSSProps::PropHasFlags(property.mProperty,
+                                  CSS_PROPERTY_CAN_ANIMATE_ON_COMPOSITOR)) {
+      continue;
+    }
+
+    if (aPropertiesToSkip.HasProperty(property.mProperty)) {
+      continue;
+    }
+
+    for (const AnimationPropertySegment& segment : property.mSegments) {
+      if (segment.mFromComposite == dom::CompositeOperation::Replace &&
+          segment.mToComposite == dom::CompositeOperation::Replace) {
+        continue;
+      }
+
+      if (!styleContext) {
+        styleContext = GetTargetStyleContext();
+      }
+      MOZ_RELEASE_ASSERT(styleContext);
+
+      Unused << EffectCompositor::GetBaseStyle(property.mProperty,
+                                               styleContext,
+                                               *mTarget->mElement,
+                                               mTarget->mPseudoType);
+      // Make this property as needing a base style so that we send the (now
+      // cached) base style to the compositor.
+      SetNeedsBaseStyle(property.mProperty);
+      break;
+    }
+  }
+}
+
+void
 KeyframeEffectReadOnly::ComposeStyle(
   RefPtr<AnimValuesStyleRule>& aStyleRule,
   const nsCSSPropertyIDSet& aPropertiesToSkip)
 {
   if (mIsComposingStyle) {
     return;
   }
 
@@ -401,16 +444,29 @@ KeyframeEffectReadOnly::ComposeStyle(
 
   ComputedTiming computedTiming = GetComputedTiming();
   mProgressOnLastCompose = computedTiming.mProgress;
   mCurrentIterationOnLastCompose = computedTiming.mCurrentIteration;
 
   // If the progress is null, we don't have fill data for the current
   // time so we shouldn't animate.
   if (computedTiming.mProgress.IsNull()) {
+    // If we are not in-effect, this effect might still be sent to the
+    // compositor and later become in-effect (e.g. if it is in the delay phase).
+    // In that case, we might need the base style in order to perform
+    // additive/accumulative animation on the compositor.
+
+    // In case of properties that can be run on the compositor, we need the base
+    // styles for such properties because those animation will be sent to
+    // compositor while they are in delay phase so that we can composite this
+    // animation on the compositor once the animation is out of the delay phase
+    // on the compositor.
+    if (computedTiming.mPhase == ComputedTiming::AnimationPhase::Before) {
+      EnsureBaseStylesForCompositor(aPropertiesToSkip);
+    }
     return;
   }
 
   mNeedsBaseStyleSet.Empty();
 
   for (size_t propIdx = 0, propEnd = mProperties.Length();
        propIdx != propEnd; ++propIdx)
   {
--- a/dom/animation/KeyframeEffectReadOnly.h
+++ b/dom/animation/KeyframeEffectReadOnly.h
@@ -422,16 +422,21 @@ protected:
     const RefPtr<AnimValuesStyleRule>& aAnimationRule,
     const StyleAnimationValue& aValueToComposite,
     CompositeOperation aCompositeOperation);
 
   // Set a bit in mNeedsBaseStyleSet if |aProperty| can be run on the
   // compositor.
   void SetNeedsBaseStyle(nsCSSPropertyID aProperty);
 
+  // Ensure the base styles is available for any properties that can be run on
+  // the compositor and which are not includes in |aPropertiesToSkip|.
+  void EnsureBaseStylesForCompositor(
+    const nsCSSPropertyIDSet& aPropertiesToSkip);
+
   Maybe<OwningAnimationTarget> mTarget;
 
   KeyframeEffectParams mEffectOptions;
 
   // The specified keyframes.
   nsTArray<Keyframe>          mKeyframes;
 
   // A set of per-property value arrays, derived from |mKeyframes|.
new file mode 100644
--- /dev/null
+++ b/dom/animation/test/crashtests/1322291-1.html
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<script>
+document.addEventListener("DOMContentLoaded", boom);
+function boom(){
+  let o1 = (function(){
+    let e=document.createElement("frameset");
+    document.documentElement.appendChild(e);
+    return e;
+  })();
+  let a1 = o1.animate({ "transform": "rotate3d(22,73,26,374grad)" },
+                      { duration: 10, delay: 100 });
+
+  // We need to wait the end of the delay to ensure that the animation is
+  // composited on the compositor, but there is no event for script animation
+  // that fires after the delay phase finished. So we wait for finished promise
+  // instead.
+  a1.finished.then(function() {
+    document.documentElement.className = "";
+  });
+}
+</script>
+</body>
+</html>
--- a/dom/animation/test/crashtests/crashtests.list
+++ b/dom/animation/test/crashtests/crashtests.list
@@ -8,10 +8,11 @@ pref(dom.animations-api.core.enabled,tru
 pref(dom.animations-api.core.enabled,true) load 1216842-6.html
 pref(dom.animations-api.core.enabled,true) load 1272475-1.html
 pref(dom.animations-api.core.enabled,true) load 1272475-2.html
 pref(dom.animations-api.core.enabled,true) load 1278485-1.html
 pref(dom.animations-api.core.enabled,true) load 1277272-1.html
 pref(dom.animations-api.core.enabled,true) load 1290535-1.html
 pref(dom.animations-api.core.enabled,true) load 1304886-1.html
 pref(dom.animations-api.core.enabled,true) load 1322382-1.html
+pref(dom.animations-api.core.enabled,true) load 1322291-1.html
 pref(dom.animations-api.core.enabled,true) load 1323114-1.html
 pref(dom.animations-api.core.enabled,true) load 1323114-2.html
--- a/dom/animation/test/style/file_missing-keyframe-on-compositor.html
+++ b/dom/animation/test/style/file_missing-keyframe-on-compositor.html
@@ -466,11 +466,30 @@ promise_test(t => {
     assert_matrix_equals(transform, 'matrix(1, 0, 0, 1, 250, 0)',
       'Composed transform value should be composed onto the value of ' +
       'lower-priority animation with fill:forwards during positive endDelay');
   });
 }, 'Transform value for animation with no keyframe at offset 0 at 50% when ' +
    'composed onto a underlying animation with fill:forwards during positive ' +
    'endDelay');
 
+promise_test(t => {
+  useTestRefreshMode(t);
+
+  var div = addDiv(t, { style: 'transform: translateX(100px)' });
+  div.animate({ transform: 'translateX(200px)' },
+              { duration: 100 * MS_PER_SEC, delay: 50 * MS_PER_SEC });
+
+  return waitForPaintsFlushed().then(() => {
+    SpecialPowers.DOMWindowUtils.advanceTimeAndRefresh(100 * MS_PER_SEC);
+
+    var transform =
+      SpecialPowers.DOMWindowUtils.getOMTAStyle(div, 'transform');
+    assert_matrix_equals(transform, 'matrix(1, 0, 0, 1, 150, 0)',
+      'Transform value for animation with positive delay should be composed ' +
+      'onto the base style');
+  });
+}, 'Transform value for animation with no keyframe at offset 0 and with ' +
+   'positive delay');
+
 done();
 </script>
 </body>