Bug 1349004 part 1 - Make DoSetKeyframe template function and rename to SetKeyframe. r?hiro draft
authorMantaroh Yoshinaga <mantaroh@gmail.com>
Thu, 11 May 2017 15:36:53 +0900
changeset 576010 f7c8ddb2882726867f254c94d6a2e4651e0d57e6
parent 575938 d8762cb967423618ff0a488f14745f60964e5c49
child 576011 fafbcc81641a098a76ddc4f73eb0ff43703f6fa6
push id58237
push usermantaroh@gmail.com
push dateThu, 11 May 2017 06:37:20 +0000
reviewershiro
bugs1349004
milestone55.0a1
Bug 1349004 part 1 - Make DoSetKeyframe template function and rename to SetKeyframe. r?hiro MozReview-Commit-ID: 4Q8bTn9YEma
dom/animation/KeyframeEffectReadOnly.cpp
dom/animation/KeyframeEffectReadOnly.h
layout/style/nsAnimationManager.cpp
layout/style/nsTransitionManager.cpp
layout/style/nsTransitionManager.h
--- a/dom/animation/KeyframeEffectReadOnly.cpp
+++ b/dom/animation/KeyframeEffectReadOnly.cpp
@@ -181,40 +181,26 @@ KeyframeEffectReadOnly::SetKeyframes(JSC
 {
   nsTArray<Keyframe> keyframes =
     KeyframeUtils::GetKeyframesFromObject(aContext, mDocument, aKeyframes, aRv);
   if (aRv.Failed()) {
     return;
   }
 
   RefPtr<nsStyleContext> styleContext = GetTargetStyleContext();
-  SetKeyframes(Move(keyframes), styleContext);
-}
-
-void
-KeyframeEffectReadOnly::SetKeyframes(nsTArray<Keyframe>&& aKeyframes,
-                                     nsStyleContext* aStyleContext)
-{
-  DoSetKeyframes(Move(aKeyframes), Move(aStyleContext));
-}
-
-void
-KeyframeEffectReadOnly::SetKeyframes(
-  nsTArray<Keyframe>&& aKeyframes,
-  const ServoComputedValuesWithParent& aServoValues)
-{
-  DoSetKeyframes(Move(aKeyframes), aServoValues);
+  nsStyleContext* rawStyleContext = styleContext.get();
+  SetKeyframes(Move(keyframes), rawStyleContext);
 }
 
 template<typename StyleType>
 void
-KeyframeEffectReadOnly::DoSetKeyframes(nsTArray<Keyframe>&& aKeyframes,
-                                       StyleType&& aStyle)
+KeyframeEffectReadOnly::SetKeyframes(nsTArray<Keyframe>&& aKeyframes,
+                                     StyleType&& aStyle)
 {
-  static_assert(IsSame<StyleType, nsStyleContext*>::value ||
+  static_assert(IsSame<StyleType, nsStyleContext*&>::value ||
                 IsSame<StyleType, const ServoComputedValuesWithParent&>::value,
                 "StyleType should be nsStyleContext* or "
                 "const ServoComputedValuesWithParent&");
 
   if (KeyframesEqualIgnoringComputedOffsets(aKeyframes, mKeyframes)) {
     return;
   }
 
@@ -224,20 +210,18 @@ KeyframeEffectReadOnly::DoSetKeyframes(n
   // values from the keyframes since both operations require a style context
   // and need to be performed whenever the style context changes.
   KeyframeUtils::ApplyDistributeSpacing(mKeyframes);
 
   if (mAnimation && mAnimation->IsRelevant()) {
     nsNodeUtils::AnimationChanged(mAnimation);
   }
 
-  // We need to call UpdateProperties() if the StyleType is
-  // 'const ServoComputedValuesWithParent&' (i.e. not a pointer) or
-  // nsStyleContext* is not nullptr.
-  if (!IsPointer<StyleType>::value || aStyle) {
+  // We need to call UpdateProperties() if the StyleType is not nullptr.
+  if (aStyle) {
     UpdateProperties(aStyle);
     MaybeUpdateFrameForCompositor();
   }
 }
 
 const AnimationProperty*
 KeyframeEffectReadOnly::GetEffectiveAnimationOfProperty(
   nsCSSPropertyID aProperty) const
@@ -1880,10 +1864,21 @@ KeyframeEffectReadOnly::ComposeStyle<Ref
   const nsCSSPropertyIDSet& aPropertiesToSkip);
 
 template
 void
 KeyframeEffectReadOnly::ComposeStyle<RawServoAnimationValueMap&>(
   RawServoAnimationValueMap& aAnimationValues,
   const nsCSSPropertyIDSet& aPropertiesToSkip);
 
+template
+void
+KeyframeEffectReadOnly::SetKeyframes(nsTArray<Keyframe>&& aKeyframes,
+                                     nsStyleContext*& aStyleContext);
+
+template
+void
+KeyframeEffectReadOnly::SetKeyframes(
+  nsTArray<Keyframe>&& aKeyframes,
+  const ServoComputedValuesWithParent& aServoValues);
+
 } // namespace dom
 } // namespace mozilla
--- a/dom/animation/KeyframeEffectReadOnly.h
+++ b/dom/animation/KeyframeEffectReadOnly.h
@@ -162,20 +162,18 @@ public:
   {
     mEffectOptions.GetSpacingAsString(aRetVal);
   }
   void NotifyAnimationTimingUpdated();
   void RequestRestyle(EffectCompositor::RestyleType aRestyleType);
   void SetAnimation(Animation* aAnimation) override;
   void SetKeyframes(JSContext* aContext, JS::Handle<JSObject*> aKeyframes,
                     ErrorResult& aRv);
-  void SetKeyframes(nsTArray<Keyframe>&& aKeyframes,
-                    nsStyleContext* aStyleContext);
-  void SetKeyframes(nsTArray<Keyframe>&& aKeyframes,
-                    const ServoComputedValuesWithParent& aServoValues);
+  template<typename StyleType>
+  void SetKeyframes(nsTArray<Keyframe>&& aKeyframes, StyleType&& aStyle);
 
   // Returns true if the effect includes |aProperty| regardless of whether the
   // property is overridden by !important rule.
   bool HasAnimationOfProperty(nsCSSPropertyID aProperty) const;
 
   // GetEffectiveAnimationOfProperty returns AnimationProperty corresponding
   // to a given CSS property if the effect includes the property and the
   // property is not overridden by !important rules.
@@ -416,19 +414,16 @@ protected:
   // per effect:target pair so we use this member to record if we have already
   // reported a "ContentTooLarge" warning for the current target.
   bool mRecordedContentTooLarge = false;
 
 private:
   nsChangeHint mCumulativeChangeHint;
 
   template<typename StyleType>
-  void DoSetKeyframes(nsTArray<Keyframe>&& aKeyframes, StyleType&& aStyle);
-
-  template<typename StyleType>
   void DoUpdateProperties(StyleType&& aStyle);
 
   void ComposeStyleRule(RefPtr<AnimValuesStyleRule>& aStyleRule,
                         const AnimationProperty& aProperty,
                         const AnimationPropertySegment& aSegment,
                         const ComputedTiming& aComputedTiming);
 
   void ComposeStyleRule(RawServoAnimationValueMap& aAnimationValues,
--- a/layout/style/nsAnimationManager.cpp
+++ b/layout/style/nsAnimationManager.cpp
@@ -424,18 +424,19 @@ public:
     return styleSet->FillKeyframesForName(aSrc.GetName(),
                                           timingFunction,
                                           mComputedValues,
                                           aKeyframes);
   }
   void SetKeyframes(KeyframeEffectReadOnly& aEffect,
                     nsTArray<Keyframe>&& aKeyframes)
   {
-    aEffect.SetKeyframes(Move(aKeyframes),
-                         { mComputedValues, mParentComputedValues });
+    const ServoComputedValuesWithParent values =
+      { mComputedValues, mParentComputedValues };
+    aEffect.SetKeyframes(Move(aKeyframes), values);
   }
 
 private:
   const ServoComputedValues* mComputedValues;
   const ServoComputedValues* mParentComputedValues;
 };
 
 class MOZ_STACK_CLASS GeckoCSSAnimationBuilder final {
@@ -450,17 +451,18 @@ public:
   }
 
   bool BuildKeyframes(nsPresContext* aPresContext,
                       const StyleAnimation& aSrc,
                       nsTArray<Keyframe>& aKeyframs);
   void SetKeyframes(KeyframeEffectReadOnly& aEffect,
                     nsTArray<Keyframe>&& aKeyframes)
   {
-    aEffect.SetKeyframes(Move(aKeyframes), mStyleContext);
+    nsStyleContext* styleContext = mStyleContext.get();
+    aEffect.SetKeyframes(Move(aKeyframes), styleContext);
   }
 
 private:
   nsTArray<Keyframe> BuildAnimationFrames(nsPresContext* aPresContext,
                                           const StyleAnimation& aSrc,
                                           const nsCSSKeyframesRule* aRule);
   Maybe<ComputedTimingFunction> GetKeyframeTimingFunction(
     nsPresContext* aPresContext,
--- a/layout/style/nsTransitionManager.cpp
+++ b/layout/style/nsTransitionManager.cpp
@@ -31,16 +31,17 @@
 #include "FrameLayerBuilder.h"
 #include "nsCSSProps.h"
 #include "nsCSSPseudoElements.h"
 #include "nsDisplayList.h"
 #include "nsStyleChangeList.h"
 #include "nsStyleSet.h"
 #include "mozilla/RestyleManager.h"
 #include "mozilla/RestyleManagerInlines.h"
+#include "mozilla/TypeTraits.h"  // For Foward<>
 #include "nsDOMMutationObserver.h"
 
 using mozilla::TimeStamp;
 using mozilla::TimeDuration;
 using mozilla::dom::Animation;
 using mozilla::dom::AnimationPlayState;
 using mozilla::dom::CSSTransition;
 using mozilla::dom::KeyframeEffectReadOnly;
@@ -581,22 +582,23 @@ nsTransitionManager::StyleContextChanged
   }
 
   nsAutoAnimationMutationBatch mb(aElement->OwnerDoc());
 
   DebugOnly<bool> startedAny = false;
   // We don't have to update transitions if display:none, although we will
   // cancel them after restyling.
   if (!afterChangeStyle->IsInDisplayNoneSubtree()) {
+    nsStyleContext* styleContext = afterChangeStyle.get();
     startedAny = DoUpdateTransitions(disp,
                                      aElement,
                                      afterChangeStyle->GetPseudoType(),
                                      collection,
                                      aOldStyleContext,
-                                     afterChangeStyle.get());
+                                     styleContext);
   }
 
   MOZ_ASSERT(!startedAny || collection,
              "must have element transitions if we started any transitions");
 
   EffectCompositor::CascadeLevel cascadeLevel =
     EffectCompositor::CascadeLevel::Transitions;
 
@@ -644,18 +646,18 @@ nsTransitionManager::UpdateTransitions(
 
 template<typename StyleType>
 bool
 nsTransitionManager::DoUpdateTransitions(
   const nsStyleDisplay* aDisp,
   dom::Element* aElement,
   CSSPseudoElementType aPseudoType,
   CSSTransitionCollection*& aElementTransitions,
-  StyleType aOldStyle,
-  StyleType aNewStyle)
+  StyleType&& aOldStyle,
+  StyleType&& aNewStyle)
 {
   MOZ_ASSERT(aDisp, "Null nsStyleDisplay");
   MOZ_ASSERT(!aElementTransitions ||
              aElementTransitions->mElement == aElement, "Element mismatch");
 
   // Per http://lists.w3.org/Archives/Public/www-style/2009Aug/0109.html
   // I'll consider only the transitions from the number of items in
   // 'transition-property' on down, and later ones will override earlier
@@ -678,32 +680,32 @@ nsTransitionManager::DoUpdateTransitions
           property == eCSSProperty_UNKNOWN) {
         // Nothing to do, but need to exclude this from cases below.
       } else if (property == eCSSPropertyExtra_all_properties) {
         for (nsCSSPropertyID p = nsCSSPropertyID(0);
              p < eCSSProperty_COUNT_no_shorthands;
              p = nsCSSPropertyID(p + 1)) {
           ConsiderInitiatingTransition(p, t, aElement, aPseudoType,
                                        aElementTransitions,
-                                       aOldStyle, aNewStyle,
+                                       aOldStyle, Forward<StyleType>(aNewStyle),
                                        &startedAny, &whichStarted);
         }
       } else if (nsCSSProps::IsShorthand(property)) {
         CSSPROPS_FOR_SHORTHAND_SUBPROPERTIES(subprop, property,
                                              CSSEnabledState::eForAllContent)
         {
           ConsiderInitiatingTransition(*subprop, t, aElement, aPseudoType,
                                        aElementTransitions,
-                                       aOldStyle, aNewStyle,
+                                       aOldStyle, Forward<StyleType>(aNewStyle),
                                        &startedAny, &whichStarted);
         }
       } else {
         ConsiderInitiatingTransition(property, t, aElement, aPseudoType,
                                      aElementTransitions,
-                                     aOldStyle, aNewStyle,
+                                     aOldStyle, Forward<StyleType>(aNewStyle),
                                      &startedAny, &whichStarted);
       }
     }
   }
 
   // Stop any transitions for properties that are no longer in
   // 'transition-property', including finished transitions.
   // Also stop any transitions (and remove any finished transitions)
@@ -841,18 +843,18 @@ IsAnimatable(nsCSSPropertyID aProperty, 
 template<typename StyleType>
 void
 nsTransitionManager::ConsiderInitiatingTransition(
   nsCSSPropertyID aProperty,
   const StyleTransition& aTransition,
   dom::Element* aElement,
   CSSPseudoElementType aPseudoType,
   CSSTransitionCollection*& aElementTransitions,
-  StyleType aOldStyle,
-  StyleType aNewStyle,
+  StyleType&& aOldStyle,
+  StyleType&& aNewStyle,
   bool* aStartedAny,
   nsCSSPropertyIDSet* aWhichStarted)
 {
   // IsShorthand itself will assert if aProperty is not a property.
   MOZ_ASSERT(!nsCSSProps::IsShorthand(aProperty),
              "property out of range");
   NS_ASSERTION(!aElementTransitions ||
                aElementTransitions->mElement == aElement, "Element mismatch");
@@ -1021,17 +1023,17 @@ nsTransitionManager::ConsiderInitiatingT
   KeyframeEffectParams effectOptions;
   RefPtr<ElementPropertyTransition> pt =
     new ElementPropertyTransition(aElement->OwnerDoc(), target, timing,
                                   startForReversingTest, reversePortion,
                                   effectOptions);
 
   pt->SetKeyframes(GetTransitionKeyframes(aProperty,
                                           Move(startValue), Move(endValue), tf),
-                   aNewStyle);
+                   Forward<StyleType>(aNewStyle));
 
   RefPtr<CSSTransition> animation =
     new CSSTransition(mPresContext->Document()->GetScopeObject());
   animation->SetOwningElement(OwningElementRef(*aElement, aPseudoType));
   animation->SetTimelineNoUpdate(timeline);
   animation->SetCreationSequence(
     mPresContext->RestyleManager()->GetAnimationGeneration());
   animation->SetEffectFromStyle(pt);
--- a/layout/style/nsTransitionManager.h
+++ b/layout/style/nsTransitionManager.h
@@ -426,27 +426,27 @@ protected:
   // as needed. aDisp and aElement must be non-null.
   // aElementTransitions is the collection of current transitions, and it
   // could be a nullptr if we don't have any transitions.
   template<typename StyleType> bool
   DoUpdateTransitions(const nsStyleDisplay* aDisp,
                       mozilla::dom::Element* aElement,
                       mozilla::CSSPseudoElementType aPseudoType,
                       CSSTransitionCollection*& aElementTransitions,
-                      StyleType aOldStyle,
-                      StyleType aNewStyle);
+                      StyleType&& aOldStyle,
+                      StyleType&& aNewStyle);
 
   template<typename StyleType> void
   ConsiderInitiatingTransition(nsCSSPropertyID aProperty,
                                const mozilla::StyleTransition& aTransition,
                                mozilla::dom::Element* aElement,
                                mozilla::CSSPseudoElementType aPseudoType,
                                CSSTransitionCollection*& aElementTransitions,
-                               StyleType aOldStyle,
-                               StyleType aNewStyle,
+                               StyleType&& aOldStyle,
+                               StyleType&& aNewStyle,
                                bool* aStartedAny,
                                nsCSSPropertyIDSet* aWhichStarted);
 
   bool mInAnimationOnlyStyleUpdate;
 
   mozilla::DelayedEventDispatcher<mozilla::TransitionEventInfo>
       mEventDispatcher;
 };