Bug 1279403 - Part 2: Set NS_FRAME_MAY_BE_TRANSFORMED bit if the target nsIFrame has transform when setting target or keyframes. r?birtles draft
authorHiroyuki Ikezoe <hiikezoe@mozilla-japan.org>
Mon, 11 Jul 2016 08:29:14 +0900
changeset 386060 179f6ebe5d409c0f3285b59219f9c1572de55690
parent 386059 55b93c1809cbf3c2cfd23094d61d79389a996700
child 386113 710fed6e08324309a73d78e6d5c07f44185b12e9
push id22611
push userhiikezoe@mozilla-japan.org
push dateMon, 11 Jul 2016 02:42:29 +0000
reviewersbirtles
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?birtles MozReview-Commit-ID: InQyXpENsSY
dom/animation/KeyframeEffect.cpp
dom/animation/KeyframeEffect.h
layout/reftests/web-animations/reftest.list
layout/reftests/web-animations/stacking-context-transform-changing-display-property.html
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
@@ -488,16 +488,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) {
@@ -1499,16 +1500,38 @@ 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) {
+      frame->AddStateBits(NS_FRAME_MAY_BE_TRANSFORMED);
+      return;
+    }
+  }
+}
+
 //---------------------------------------------------------------------
 //
 // KeyframeEffect
 //
 //---------------------------------------------------------------------
 
 KeyframeEffect::KeyframeEffect(nsIDocument* aDocument,
                                const Maybe<OwningAnimationTarget>& aTarget,
@@ -1598,16 +1621,18 @@ KeyframeEffect::SetTarget(const Nullable
     UpdateTargetRegistration();
     RefPtr<nsStyleContext> styleContext = GetTargetStyleContext();
     if (styleContext) {
       UpdateProperties(styleContext);
     } else if (mEffectOptions.mSpacingMode == SpacingMode::paced) {
       KeyframeUtils::ApplyDistributeSpacing(mKeyframes);
     }
 
+    MaybeUpdateFrameForCompositor();
+
     RequestRestyle(EffectCompositor::RestyleType::Layer);
 
     nsAutoAnimationMutationBatch mb(mTarget->mElement->OwnerDoc());
     if (mAnimation) {
       nsNodeUtils::AnimationAdded(mAnimation);
     }
   } else if (mEffectOptions.mSpacingMode == SpacingMode::paced) {
     // New target is null, so fall back to distribute spacing.
--- a/dom/animation/KeyframeEffect.h
+++ b/dom/animation/KeyframeEffect.h
@@ -384,16 +384,22 @@ 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 bits so that, if necessary, a stacking
+  // context will be created and the effect sent to the compositor.  We
+  // typically need to do this when the properties referenced by the keyframe
+  // have changed, or when the target frame might have changed.
+  void MaybeUpdateFrameForCompositor();
+
   // Looks up the style context associated with the target element, if any.
   // We need to be careful to *not* call this when we are updating the style
   // context. That's because calling GetStyleContextForElement when we are in
   // the process of building a style context may trigger various forms of
   // infinite recursion.
   // If aDoc is nullptr, we will use the owner doc of the target element.
   already_AddRefed<nsStyleContext>
   GetTargetStyleContext(nsIDocument* aDoc = nullptr);
--- a/layout/reftests/web-animations/reftest.list
+++ b/layout/reftests/web-animations/reftest.list
@@ -1,5 +1,8 @@
 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(dom.animations-api.core.enabled,true) == stacking-context-transform-changing-display-property.html stacking-context-animation-ref.html
new file mode 100644
--- /dev/null
+++ b/layout/reftests/web-animations/stacking-context-transform-changing-display-property.html
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<html class="reftest-wait reftest-no-flush">
+<title>
+Transform animation creates a stacking context when changing its display style
+</title>
+<style>
+span {
+  height: 100px;
+  width: 100px;
+  position: fixed;
+  background: green;
+  top: 50px;
+}
+div {
+  width: 100px; height: 100px;
+  background: blue;
+  display: none;
+}
+</style>
+<span></span>
+<div id="test"></div>
+<script>
+  var anim = document.getElementById("test")
+    .animate({ transform: ['none', 'none'] }, { duration: 100000 });
+  anim.ready.then(function() {
+    anim.effect.target.style.display = 'block';
+    requestAnimationFrame(function() {
+      document.documentElement.classList.remove("reftest-wait");
+    });
+  });
+</script>
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 reftest-no-flush">
+<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 reftest-no-flush">
+<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>