Bug 1277456 part 6 - Use the composed document of the target effect (if any) when computing keyframe values; r?hiro draft
authorBrian Birtles <birtles@gmail.com>
Wed, 13 Jul 2016 13:22:25 +0900
changeset 386963 548b28645091d97093663cbb6d6af6f692e33f95
parent 386962 80b698a27db3a93cd2777f61ceb486144a2d1116
child 386964 29b01adc2b02997f72dcd510b971520f80d27081
child 388493 94fff51558e890bea83e60a442bf763ae4c451f4
push id22865
push userbbirtles@mozilla.com
push dateWed, 13 Jul 2016 04:26:23 +0000
reviewershiro
bugs1277456
milestone50.0a1
Bug 1277456 part 6 - Use the composed document of the target effect (if any) when computing keyframe values; r?hiro Previously, when fetching an nsPresShell, we would look up the current realm document and get the pres shell for it. This patch makes us call GetPresShell() which uses GetRenderedDocument() which corresponds to the composed document of the target effect which seems more consistent since it is the target effect we will use as context for computing CSS values (as required by [1]). [1] https://w3c.github.io/web-animations/#calculating-computed-keyframes MozReview-Commit-ID: 9S55041rfTp
dom/animation/KeyframeEffect.cpp
dom/animation/KeyframeEffect.h
--- a/dom/animation/KeyframeEffect.cpp
+++ b/dom/animation/KeyframeEffect.cpp
@@ -451,29 +451,23 @@ KeyframesEqualIgnoringComputedOffsets(co
 }
 
 // https://w3c.github.io/web-animations/#dom-keyframeeffect-setkeyframes
 void
 KeyframeEffectReadOnly::SetKeyframes(JSContext* aContext,
                                      JS::Handle<JSObject*> aKeyframes,
                                      ErrorResult& aRv)
 {
-  nsIDocument* doc = AnimationUtils::GetCurrentRealmDocument(aContext);
-  if (!doc) {
-    aRv.Throw(NS_ERROR_FAILURE);
-    return;
-  }
-
   nsTArray<Keyframe> keyframes =
     KeyframeUtils::GetKeyframesFromObject(aContext, mDocument, aKeyframes, aRv);
   if (aRv.Failed()) {
     return;
   }
 
-  RefPtr<nsStyleContext> styleContext = GetTargetStyleContext(doc);
+  RefPtr<nsStyleContext> styleContext = GetTargetStyleContext();
   SetKeyframes(Move(keyframes), styleContext);
 }
 
 void
 KeyframeEffectReadOnly::SetKeyframes(nsTArray<Keyframe>&& aKeyframes,
                                   nsStyleContext* aStyleContext)
 {
   if (KeyframesEqualIgnoringComputedOffsets(aKeyframes, mKeyframes)) {
@@ -939,35 +933,31 @@ KeyframeEffectReadOnly::RequestRestyle(
   if (presContext && mTarget && mAnimation) {
     presContext->EffectCompositor()->
       RequestRestyle(mTarget->mElement, mTarget->mPseudoType,
                      aRestyleType, mAnimation->CascadeLevel());
   }
 }
 
 already_AddRefed<nsStyleContext>
-KeyframeEffectReadOnly::GetTargetStyleContext(nsIDocument* aDoc)
+KeyframeEffectReadOnly::GetTargetStyleContext()
 {
-  if (!mTarget) {
+  nsIPresShell* shell = GetPresShell();
+  if (!shell) {
     return nullptr;
   }
 
-  if (!aDoc) {
-    aDoc = mTarget->mElement->OwnerDoc();
-    if (!aDoc) {
-      return nullptr;
-    }
-  }
+  MOZ_ASSERT(mTarget,
+             "Should only have a presshell when we have a target element");
 
   nsIAtom* pseudo = mTarget->mPseudoType < CSSPseudoElementType::Count
                     ? nsCSSPseudoElements::GetPseudoAtom(mTarget->mPseudoType)
                     : nullptr;
   return nsComputedDOMStyle::GetStyleContextForElement(mTarget->mElement,
-                                                       pseudo,
-                                                       aDoc->GetShell());
+                                                       pseudo, shell);
 }
 
 #ifdef DEBUG
 void
 DumpAnimationProperties(nsTArray<AnimationProperty>& aAnimationProperties)
 {
   for (auto& p : aAnimationProperties) {
     printf("%s\n", nsCSSProps::GetStringValue(p.mProperty).get());
--- a/dom/animation/KeyframeEffect.h
+++ b/dom/animation/KeyframeEffect.h
@@ -395,19 +395,18 @@ protected:
   // 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);
+  GetTargetStyleContext();
 
   Maybe<OwningAnimationTarget> mTarget;
   RefPtr<Animation> mAnimation;
 
   RefPtr<AnimationEffectTimingReadOnly> mTiming;
   KeyframeEffectParams mEffectOptions;
 
   // The specified keyframes.