Bug 1279403 - Part 2: Set NS_FRAME_MAY_BE_TRANSFORMED bit if the target nsIFrame has transform when setting target or keyframes. r? draft
authorHiroyuki Ikezoe <hiikezoe@mozilla-japan.org>
Mon, 20 Jun 2016 15:32:51 +0900
changeset 379965 b897aecd13489aaee78d1c966f3b2c07c22ef33b
parent 379964 9787bc23019dc7c540704065acddfdea3baa0d52
child 379976 4e54a6f3ed72d87addf6eb2c3ac76b16366c284b
push id21101
push userhiikezoe@mozilla-japan.org
push dateMon, 20 Jun 2016 06:33:26 +0000
bugs1279403
milestone50.0a1
Bug 1279403 - Part 2: Set NS_FRAME_MAY_BE_TRANSFORMED bit if the target nsIFrame has transform when setting target or keyframes. r? MozReview-Commit-ID: 23lr8gZY7TH
dom/animation/KeyframeEffect.cpp
dom/animation/KeyframeEffect.h
layout/reftests/web-animations/reftest.list
layout/reftests/web-animations/stacking-context-transform-changing-keyframe.html
layout/reftests/web-animations/stacking-context-transform-changing-target.html
--- a/dom/animation/KeyframeEffect.cpp
+++ b/dom/animation/KeyframeEffect.cpp
@@ -468,16 +468,17 @@ KeyframeEffectReadOnly::SetKeyframes(nsT
   KeyframeUtils::ApplyDistributeSpacing(mKeyframes);
 
   if (mAnimation && mAnimation->IsRelevant()) {
     nsNodeUtils::AnimationChanged(mAnimation);
   }
 
   if (aStyleContext) {
     UpdateProperties(aStyleContext);
+    MaybeUpdateFrameForCompositor();
   }
 }
 
 const AnimationProperty*
 KeyframeEffectReadOnly::GetAnimationOfProperty(nsCSSProperty aProperty) const
 {
   for (size_t propIdx = 0, propEnd = mProperties.Length();
        propIdx != propEnd; ++propIdx) {
@@ -1354,16 +1355,39 @@ KeyframeEffectReadOnly::CanIgnoreIfNotVi
   }
 
   // FIXME: For further sophisticated optimization we need to check
   // change hint on the segment corresponding to computedTiming.progress.
   return NS_IsHintSubset(
     mCumulativeChangeHint, nsChangeHint_Hints_CanIgnoreIfNotVisible);
 }
 
+void
+KeyframeEffectReadOnly::MaybeUpdateFrameForCompositor()
+{
+  nsIFrame* frame = GetAnimationFrame();
+  if (!frame) {
+    return;
+  }
+
+  // We don't check mWinsInCascade flag here because, at this point,
+  // UpdateCascadeResults has not yet run.
+  // FIXME: Bug 1272495: If this effect does not win in the cascade, the
+  // NS_FRAME_MAY_BE_TRANSFORMED flag should be removed when the animation
+  // will be removed from effect set or the transform keyframes are removed
+  // by setKeyframes. The latter case will be hard to solve though.
+  for (const AnimationProperty& property : mProperties) {
+    if (property.mProperty == eCSSProperty_transform) {
+      // Set NS_FRAME_MAY_BE_TRANSFORMED on the associated frame.
+      frame->AddStateBits(NS_FRAME_MAY_BE_TRANSFORMED);
+      return;
+    }
+  }
+}
+
 //---------------------------------------------------------------------
 //
 // KeyframeEffect
 //
 //---------------------------------------------------------------------
 
 KeyframeEffect::KeyframeEffect(nsIDocument* aDocument,
                                const Maybe<OwningAnimationTarget>& aTarget,
@@ -1445,16 +1469,17 @@ KeyframeEffect::SetTarget(const Nullable
     }
   }
 
   mTarget = newTarget;
 
   if (mTarget) {
     UpdateTargetRegistration();
     MaybeUpdateProperties();
+    MaybeUpdateFrameForCompositor();
 
     RequestRestyle(EffectCompositor::RestyleType::Layer);
 
     nsAutoAnimationMutationBatch mb(mTarget->mElement->OwnerDoc());
     if (mAnimation) {
       nsNodeUtils::AnimationAdded(mAnimation);
     }
   }
--- a/dom/animation/KeyframeEffect.h
+++ b/dom/animation/KeyframeEffect.h
@@ -380,16 +380,20 @@ protected:
   // owning Animation's timing.
   void UpdateTargetRegistration();
 
   // Remove the current effect target from its EffectSet.
   void UnregisterTarget();
 
   void RequestRestyle(EffectCompositor::RestyleType aRestyleType);
 
+  // Update the associated frame state to send the effect to the compositor
+  // or create a stacking context if necessary.
+  void MaybeUpdateFrameForCompositor();
+
   Maybe<OwningAnimationTarget> mTarget;
   RefPtr<Animation> mAnimation;
 
   RefPtr<AnimationEffectTimingReadOnly> mTiming;
 
   // The specified keyframes.
   nsTArray<Keyframe>          mKeyframes;
 
--- a/layout/reftests/web-animations/reftest.list
+++ b/layout/reftests/web-animations/reftest.list
@@ -1,7 +1,11 @@
 test-pref(dom.animations-api.core.enabled,true) == 1246046-1.html green-box.html
 test-pref(dom.animations-api.core.enabled,true) == 1267937-1.html 1267937-ref.html
 test-pref(dom.animations-api.core.enabled,true) == stacking-context-transform-none-animation-before-appending-element.html stacking-context-animation-ref.html
 test-pref(dom.animations-api.core.enabled,true) == stacking-context-opacity-changing-keyframe.html stacking-context-animation-ref.html
 test-pref(dom.animations-api.core.enabled,true) == stacking-context-opacity-changing-target.html stacking-context-animation-changing-target-ref.html
+test-pref(dom.animations-api.core.enabled,true) == stacking-context-transform-changing-keyframe.html stacking-context-animation-ref.html
+test-pref(dom.animations-api.core.enabled,true) == stacking-context-transform-changing-target.html stacking-context-animation-changing-target-ref.html
 test-pref(layers.offmainthreadcomposition.async-animations,false) test-pref(dom.animations-api.core.enabled,true) == stacking-context-opacity-changing-keyframe.html stacking-context-animation-ref.html
 test-pref(layers.offmainthreadcomposition.async-animations,false) test-pref(dom.animations-api.core.enabled,true) == stacking-context-opacity-changing-target.html stacking-context-animation-changing-target-ref.html
+test-pref(layers.offmainthreadcomposition.async-animations,false) test-pref(dom.animations-api.core.enabled,true) == stacking-context-transform-changing-keyframe.html stacking-context-animation-ref.html
+test-pref(layers.offmainthreadcomposition.async-animations,false) test-pref(dom.animations-api.core.enabled,true) == stacking-context-transform-changing-target.html stacking-context-animation-changing-target-ref.html
new file mode 100644
--- /dev/null
+++ b/layout/reftests/web-animations/stacking-context-transform-changing-keyframe.html
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<title>Changing keyframes to transform frames creates a stacking context</title>
+<style>
+span {
+  height: 100px;
+  width: 100px;
+  position: fixed;
+  background: green;
+  top: 50px;
+}
+#test {
+  width: 100px; height: 100px;
+  background: blue;
+}
+</style>
+<span></span>
+<div id="test"></div>
+<script>
+  var anim = document.getElementById("test")
+    .animate({ }, { duration: 100000 });
+  anim.ready.then(function() {
+    anim.effect.setKeyframes({ transform: ['none', 'none'] });
+    requestAnimationFrame(function() {
+      document.documentElement.classList.remove("reftest-wait");
+    });
+  });
+</script>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/web-animations/stacking-context-transform-changing-target.html
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<title>
+Transform animation creates a stacking context when changing its target
+</title>
+<style>
+span {
+  height: 100px;
+  width: 100px;
+  position: fixed;
+  background: green;
+  top: 50px;
+}
+div {
+  width: 100px; height: 100px;
+  background: blue;
+}
+</style>
+<span></span>
+<div id="test"></div>
+<div id="another"></div>
+<script>
+  var anim = document.getElementById("test")
+    .animate({ transform: ['none', 'none'] }, { duration: 100000 });
+  anim.ready.then(function() {
+    anim.effect.target = document.getElementById("another");
+    requestAnimationFrame(function() {
+      document.documentElement.classList.remove("reftest-wait");
+    });
+  });
+</script>