Bug 1244590 - Part 8: Rewrite GetStyleContext code. draft
authorBoris Chiou <boris.chiou@gmail.com>
Wed, 11 May 2016 11:41:20 +0800
changeset 371287 49a66b65699e4857ac48f508a2d2e30df44fe412
parent 371286 81061db3273afdd71560fe10d33325becdce6afe
child 371288 ca6ac8725020ba8c112a70cf978dc7024af3ee4e
push id19290
push userbmo:boris.chiou@gmail.com
push dateThu, 26 May 2016 09:18:45 +0000
bugs1244590
milestone49.0a1
Bug 1244590 - Part 8: Rewrite GetStyleContext code. Do a simple refactor, so we can reuse the getter of nsStyleContext. MozReview-Commit-ID: 4BQ7f8HuFns
dom/animation/KeyframeEffect.cpp
dom/animation/KeyframeEffect.h
--- a/dom/animation/KeyframeEffect.cpp
+++ b/dom/animation/KeyframeEffect.cpp
@@ -441,27 +441,17 @@ KeyframeEffectReadOnly::SetKeyframes(JSC
   }
 
   nsTArray<Keyframe> keyframes =
     KeyframeUtils::GetKeyframesFromObject(aContext, aKeyframes, aRv);
   if (aRv.Failed()) {
     return;
   }
 
-  RefPtr<nsStyleContext> styleContext;
-  nsIPresShell* shell = doc->GetShell();
-  if (shell && mTarget) {
-    nsIAtom* pseudo =
-      mTarget->mPseudoType < CSSPseudoElementType::Count ?
-      nsCSSPseudoElements::GetPseudoAtom(mTarget->mPseudoType) : nullptr;
-    styleContext =
-      nsComputedDOMStyle::GetStyleContextForElement(mTarget->mElement,
-                                                    pseudo, shell);
-  }
-
+  RefPtr<nsStyleContext> styleContext = GetTargetStyleContext(doc);
   SetKeyframes(Move(keyframes), styleContext);
 }
 
 void
 KeyframeEffectReadOnly::SetKeyframes(nsTArray<Keyframe>&& aKeyframes,
                                   nsStyleContext* aStyleContext)
 {
   if (KeyframesEqualIgnoringComputedOffsets(aKeyframes, mKeyframes)) {
@@ -881,16 +871,38 @@ KeyframeEffectReadOnly::RequestRestyle(
   nsPresContext* presContext = GetPresContext();
   if (presContext && mTarget && mAnimation) {
     presContext->EffectCompositor()->
       RequestRestyle(mTarget->mElement, mTarget->mPseudoType,
                      aRestyleType, mAnimation->CascadeLevel());
   }
 }
 
+already_AddRefed<nsStyleContext>
+KeyframeEffectReadOnly::GetTargetStyleContext(nsIDocument* aDoc)
+{
+  if (!mTarget) {
+    return nullptr;
+  }
+
+  if (!aDoc) {
+    aDoc = mTarget->mElement->OwnerDoc();
+    if (!aDoc) {
+      return nullptr;
+    }
+  }
+
+  nsIAtom* pseudo = mTarget->mPseudoType < CSSPseudoElementType::Count
+                    ? nsCSSPseudoElements::GetPseudoAtom(mTarget->mPseudoType)
+                    : nullptr;
+  return nsComputedDOMStyle::GetStyleContextForElement(mTarget->mElement,
+                                                       pseudo,
+                                                       aDoc->GetShell());
+}
+
 #ifdef DEBUG
 void
 DumpAnimationProperties(nsTArray<AnimationProperty>& aAnimationProperties)
 {
   for (auto& p : aAnimationProperties) {
     printf("%s\n", nsCSSProps::GetStringValue(p.mProperty).get());
     for (auto& s : p.mSegments) {
       nsString fromValue, toValue;
@@ -1473,17 +1485,20 @@ KeyframeEffect::SetTarget(const Nullable
       nsNodeUtils::AnimationRemoved(mAnimation);
     }
   }
 
   mTarget = newTarget;
 
   if (mTarget) {
     UpdateTargetRegistration();
-    MaybeUpdateProperties();
+    RefPtr<nsStyleContext> styleContext = GetTargetStyleContext();
+    if (styleContext) {
+      UpdateProperties(styleContext);
+    }
 
     RequestRestyle(EffectCompositor::RestyleType::Layer);
 
     nsAutoAnimationMutationBatch mb(mTarget->mElement->OwnerDoc());
     if (mAnimation) {
       nsNodeUtils::AnimationAdded(mAnimation);
     }
   }
@@ -1493,35 +1508,10 @@ KeyframeEffect::~KeyframeEffect()
 {
   // mTiming is cycle collected, so we have to do null check first even though
   // mTiming shouldn't be null during the lifetime of KeyframeEffect.
   if (mTiming) {
     mTiming->Unlink();
   }
 }
 
-void
-KeyframeEffect::MaybeUpdateProperties()
-{
-  if (!mTarget) {
-    return;
-  }
-
-  nsIDocument* doc = mTarget->mElement->OwnerDoc();
-  if (!doc) {
-    return;
-  }
-
-  nsIAtom* pseudo = mTarget->mPseudoType < CSSPseudoElementType::Count ?
-                    nsCSSPseudoElements::GetPseudoAtom(mTarget->mPseudoType) :
-                    nullptr;
-  RefPtr<nsStyleContext> styleContext =
-    nsComputedDOMStyle::GetStyleContextForElement(mTarget->mElement, pseudo,
-                                                  doc->GetShell());
-  if (!styleContext) {
-    return;
-  }
-
-  UpdateProperties(styleContext);
-}
-
 } // namespace dom
 } // namespace mozilla
--- a/dom/animation/KeyframeEffect.h
+++ b/dom/animation/KeyframeEffect.h
@@ -371,16 +371,25 @@ protected:
   // owning Animation's timing.
   void UpdateTargetRegistration();
 
   // Remove the current effect target from its EffectSet.
   void UnregisterTarget();
 
   void RequestRestyle(EffectCompositor::RestyleType aRestyleType);
 
+  // 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);
+
   Maybe<OwningAnimationTarget> mTarget;
   RefPtr<Animation> mAnimation;
 
   RefPtr<AnimationEffectTimingReadOnly> mTiming;
   KeyframeEffectParams mEffectOptions;
 
   // The specified keyframes.
   nsTArray<Keyframe>          mKeyframes;
@@ -439,29 +448,23 @@ public:
   Constructor(const GlobalObject& aGlobal,
               const Nullable<ElementOrCSSPseudoElement>& aTarget,
               JS::Handle<JSObject*> aKeyframes,
               const UnrestrictedDoubleOrKeyframeAnimationOptions& aOptions,
               ErrorResult& aRv);
 
   void NotifySpecifiedTimingUpdated();
 
-  // This method calls MaybeUpdateProperties which is not safe to use when
+  // This method calls GetTargetStyleContext which is not safe to use when
   // we are in the middle of updating style. If we need to use this when
   // updating style, we should pass the nsStyleContext into this method and use
   // that to update the properties rather than calling
   // GetStyleContextForElement.
   void SetTarget(const Nullable<ElementOrCSSPseudoElement>& aTarget);
 
 protected:
   ~KeyframeEffect() override;
-
-  // 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.
-  void MaybeUpdateProperties();
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_KeyframeEffect_h