Bug 1179111 part 5 - Remove Name() methods; r=jwatt

--HG--
extra : source : 22cafa2fd162a3cbe9af7fd13a92fd82cd3a2315

dom/animation/Animation.h

@@ -218,11 +218,6 @@ public:
    */
   Nullable<TimeDuration> GetCurrentOrPendingStartTime() const;
 
-  const nsString& Name() const
-  {
-    return mEffect ? mEffect->Name() : EmptyString();
-  }
-
   bool IsPausedOrPausing() const
   {
     return PlayState() == AnimationPlayState::Paused ||

dom/animation/KeyframeEffect.h

@@ -193,12 +193,10 @@ public:
   KeyframeEffectReadOnly(nsIDocument* aDocument,
                          Element* aTarget,
                          nsCSSPseudoElements::Type aPseudoType,
-                         const AnimationTiming &aTiming,
-                         const nsSubstring& aName)
+                         const AnimationTiming &aTiming)
     : AnimationEffectReadOnly(aDocument)
     , mTarget(aTarget)
     , mTiming(aTiming)
-    , mName(aName)
     , mIsFinishedTransition(false)
     , mPseudoType(aPseudoType)
   {
@@ -227,10 +225,6 @@ public:
                " pseudo-element is not yet supported.");
     return mTarget;
   }
-  void GetName(nsString& aRetVal) const
-  {
-    aRetVal = Name();
-  }
 
   // Temporary workaround to return both the target element and pseudo-type
   // until we implement PseudoElement.
@@ -239,12 +233,6 @@ public:
     aTarget = mTarget;
     aPseudoType = mPseudoType;
   }
-  // Alternative to GetName that returns a reference to the member for
-  // more efficient internal usage.
-  virtual const nsString& Name() const
-  {
-    return mName;
-  }
 
   void SetParentTime(Nullable<TimeDuration> aParentTime);
 
@@ -340,7 +328,6 @@ protected:
   Nullable<TimeDuration> mParentTime;
 
   AnimationTiming mTiming;
-  nsString mName;
   // A flag to mark transitions that have finished and are due to
   // be removed on the next throttle-able cycle.
   bool mIsFinishedTransition;

layout/style/nsAnimationManager.cpp

@@ -173,7 +173,7 @@ CSSAnimation::QueueEvents(EventArray& aEventsToDispatch)
     StickyTimeDuration elapsedTime =
       std::min(StickyTimeDuration(mEffect->InitialAdvance()),
                computedTiming.mActiveDuration);
-    AnimationEventInfo ei(target, Name(), NS_ANIMATION_START,
+    AnimationEventInfo ei(target, mAnimationName, NS_ANIMATION_START,
                           elapsedTime,
                           PseudoTypeAsString(targetPseudoType));
     aEventsToDispatch.AppendElement(ei);
@@ -196,7 +196,7 @@ CSSAnimation::QueueEvents(EventArray& aEventsToDispatch)
     elapsedTime = computedTiming.mActiveDuration;
   }
 
-  AnimationEventInfo ei(target, Name(), message, elapsedTime,
+  AnimationEventInfo ei(target, mAnimationName, message, elapsedTime,
                         PseudoTypeAsString(targetPseudoType));
   aEventsToDispatch.AppendElement(ei);
 }
@@ -373,7 +373,8 @@ nsAnimationManager::CheckAnimationRule(nsStyleContext* aStyleContext,
           CSSAnimation* a = collection->mAnimations[oldIdx]->AsCSSAnimation();
           MOZ_ASSERT(a, "All animations in the CSS Animation collection should"
                         " be CSSAnimation objects");
-          if (a->Name() == newAnim->Name()) {
+          if (a->AnimationName() ==
+              newAnim->AsCSSAnimation()->AnimationName()) {
             oldAnim = a;
             break;
           }
@@ -550,7 +551,7 @@ nsAnimationManager::BuildAnimations(nsStyleContext* aStyleContext,
       continue;
     }
 
-    nsRefPtr<CSSAnimation> dest = new CSSAnimation(aTimeline);
+    nsRefPtr<CSSAnimation> dest = new CSSAnimation(aTimeline, src.GetName());
     aAnimations.AppendElement(dest);
 
     AnimationTiming timing;
@@ -563,8 +564,7 @@ nsAnimationManager::BuildAnimations(nsStyleContext* aStyleContext,
 
     nsRefPtr<KeyframeEffectReadOnly> destEffect =
       new KeyframeEffectReadOnly(mPresContext->Document(), aTarget,
-                                 aStyleContext->GetPseudoType(), timing,
-                                 src.GetName());
+                                 aStyleContext->GetPseudoType(), timing);
     dest->SetEffect(destEffect);
 
     if (src.GetPlayState() == NS_STYLE_ANIMATION_PLAY_STATE_PAUSED) {

layout/style/nsAnimationManager.h

@@ -56,12 +56,18 @@ namespace dom {
 class CSSAnimation final : public Animation
 {
 public:
- explicit CSSAnimation(DocumentTimeline* aTimeline)
+ explicit CSSAnimation(DocumentTimeline* aTimeline,
+                       const nsSubstring& aAnimationName)
     : Animation(aTimeline)
+    , mAnimationName(aAnimationName)
     , mIsStylePaused(false)
     , mPauseShouldStick(false)
     , mPreviousPhaseOrIteration(PREVIOUS_PHASE_BEFORE)
   {
+    // We might need to drop this assertion once we add a script-accessible
+    // constructor but for animations generated from CSS markup the
+    // animation-name should never be empty.
+    MOZ_ASSERT(!mAnimationName.IsEmpty(), "animation-name should not be empty");
   }
 
   JSObject* WrapObject(JSContext* aCx,
@@ -70,7 +76,11 @@ public:
   virtual CSSAnimation* AsCSSAnimation() override { return this; }
 
   // CSSAnimation interface
-  void GetAnimationName(nsString& aRetVal) const { aRetVal = Name(); }
+  void GetAnimationName(nsString& aRetVal) const { aRetVal = mAnimationName; }
+
+  // Alternative to GetAnimationName that returns a reference to the member
+  // for more efficient internal usage.
+  const nsString& AnimationName() const { return mAnimationName; }
 
   // Animation interface overrides
   virtual Promise* GetReady(ErrorResult& aRv) override;
@@ -100,6 +110,8 @@ protected:
 
   static nsString PseudoTypeAsString(nsCSSPseudoElements::Type aPseudoType);
 
+  nsString mAnimationName;
+
   // When combining animation-play-state with play() / pause() the following
   // behavior applies:
   // 1. pause() is sticky and always overrides the underlying

layout/style/nsTransitionManager.cpp

@@ -41,16 +41,6 @@ using mozilla::dom::KeyframeEffectReadOnly;
 using namespace mozilla;
 using namespace mozilla::css;
 
-const nsString&
-ElementPropertyTransition::Name() const
-{
-   if (!mName.Length()) {
-     const_cast<ElementPropertyTransition*>(this)->mName =
-       NS_ConvertUTF8toUTF16(nsCSSProps::GetStringValue(TransitionProperty()));
-   }
-   return dom::KeyframeEffectReadOnly::Name();
-}
-
 double
 ElementPropertyTransition::CurrentValuePortion() const
 {
@@ -93,6 +83,18 @@ CSSTransition::WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto)
   return dom::CSSTransitionBinding::Wrap(aCx, this, aGivenProto);
 }
 
+void
+CSSTransition::GetTransitionProperty(nsString& aRetVal) const
+{
+  // Once we make the effect property settable (bug 1049975) we will need
+  // to store the transition property on the CSSTransition itself but for
+  // now we can just query the effect.
+  MOZ_ASSERT(mEffect && mEffect->AsTransition(),
+             "Transitions should have a transition effect");
+  nsCSSProperty prop = mEffect->AsTransition()->TransitionProperty();
+  aRetVal = NS_ConvertUTF8toUTF16(nsCSSProps::GetStringValue(prop));
+}
+
 AnimationPlayState
 CSSTransition::PlayStateFromJS() const
 {

layout/style/nsTransitionManager.h

@@ -35,15 +35,12 @@ struct ElementPropertyTransition : public dom::KeyframeEffectReadOnly
                             dom::Element* aTarget,
                             nsCSSPseudoElements::Type aPseudoType,
                             const AnimationTiming &aTiming)
-    : dom::KeyframeEffectReadOnly(aDocument, aTarget, aPseudoType,
-                                  aTiming, EmptyString())
+    : dom::KeyframeEffectReadOnly(aDocument, aTarget, aPseudoType, aTiming)
   { }
 
   virtual ElementPropertyTransition* AsTransition() override { return this; }
   virtual const ElementPropertyTransition* AsTransition() const override { return this; }
 
-  virtual const nsString& Name() const override;
-
   nsCSSProperty TransitionProperty() const {
     MOZ_ASSERT(Properties().Length() == 1,
                "Transitions should have exactly one animation property. "
@@ -90,7 +87,7 @@ public:
   virtual CSSTransition* AsCSSTransition() override { return this; }
 
   // CSSTransition interface
-  void GetTransitionProperty(nsString& aRetVal) const { aRetVal = Name(); }
+  void GetTransitionProperty(nsString& aRetVal) const;
 
   // Animation interface overrides
   virtual AnimationPlayState PlayStateFromJS() const override;