Bug 1454503: Remove unneeded refcounting in nsAnimationManager / nsTransitionManager. r?hiro draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 17 Apr 2018 00:28:00 +0200
changeset 783314 1e1ac88aa7be01128b53a723513450542d43b31f
parent 783313 0e098d4ee6ae8ac91609e72b6d1c5c299dd8eb75
child 783315 ad083983cdf7602242f36dbf34d03cbacf6b4586
push id106661
push userbmo:emilio@crisal.io
push dateMon, 16 Apr 2018 22:36:55 +0000
reviewershiro
bugs1454503
milestone61.0a1
Bug 1454503: Remove unneeded refcounting in nsAnimationManager / nsTransitionManager. r?hiro MozReview-Commit-ID: 1zgUfDhH8bm
layout/base/nsPresContext.cpp
layout/base/nsPresContext.h
layout/style/nsAnimationManager.h
layout/style/nsTransitionManager.h
--- a/layout/base/nsPresContext.cpp
+++ b/layout/base/nsPresContext.cpp
@@ -838,18 +838,18 @@ nsPresContext::Init(nsDeviceContext* aDe
   if (mDeviceContext->SetFullZoom(mFullZoom))
     mDeviceContext->FlushFontCache();
   mCurAppUnitsPerDevPixel = AppUnitsPerDevPixel();
 
   mEventManager = new mozilla::EventStateManager();
 
   mAnimationEventDispatcher = new mozilla::AnimationEventDispatcher(this);
   mEffectCompositor = new mozilla::EffectCompositor(this);
-  mTransitionManager = new nsTransitionManager(this);
-  mAnimationManager = new nsAnimationManager(this);
+  mTransitionManager = MakeUnique<nsTransitionManager>(this);
+  mAnimationManager = MakeUnique<nsAnimationManager>(this);
 
   if (mDocument->GetDisplayDocument()) {
     NS_ASSERTION(mDocument->GetDisplayDocument()->GetPresContext(),
                  "Why are we being initialized?");
     mRefreshDriver = mDocument->GetDisplayDocument()->
       GetPresContext()->RefreshDriver();
   } else {
     nsIDocument* parent = mDocument->GetParentDocument();
--- a/layout/base/nsPresContext.h
+++ b/layout/base/nsPresContext.h
@@ -234,19 +234,19 @@ public:
     { return PresShell()->FrameConstructor(); }
 
   mozilla::AnimationEventDispatcher* AnimationEventDispatcher()
   {
     return mAnimationEventDispatcher;
   }
 
   mozilla::EffectCompositor* EffectCompositor() { return mEffectCompositor; }
-  nsTransitionManager* TransitionManager() { return mTransitionManager; }
-  nsAnimationManager* AnimationManager() { return mAnimationManager; }
-  const nsAnimationManager* AnimationManager() const { return mAnimationManager; }
+  nsTransitionManager* TransitionManager() { return mTransitionManager.get(); }
+  nsAnimationManager* AnimationManager() { return mAnimationManager.get(); }
+  const nsAnimationManager* AnimationManager() const { return mAnimationManager.get(); }
 
   nsRefreshDriver* RefreshDriver() { return mRefreshDriver; }
 
   mozilla::RestyleManager* RestyleManager() {
     MOZ_ASSERT(mRestyleManager);
     return mRestyleManager.get();
   }
 
@@ -1293,18 +1293,18 @@ protected:
                                             // better safe than sorry.
                                             // Cannot reintroduce cycles
                                             // since there is no dependency
                                             // from gfx back to layout.
   RefPtr<mozilla::EventStateManager> mEventManager;
   RefPtr<nsRefreshDriver> mRefreshDriver;
   RefPtr<mozilla::AnimationEventDispatcher> mAnimationEventDispatcher;
   RefPtr<mozilla::EffectCompositor> mEffectCompositor;
-  RefPtr<nsTransitionManager> mTransitionManager;
-  RefPtr<nsAnimationManager> mAnimationManager;
+  mozilla::UniquePtr<nsTransitionManager> mTransitionManager;
+  mozilla::UniquePtr<nsAnimationManager> mAnimationManager;
   mozilla::UniquePtr<mozilla::RestyleManager> mRestyleManager;
   RefPtr<mozilla::CounterStyleManager> mCounterStyleManager;
   nsAtom* MOZ_UNSAFE_REF("always a static atom") mMedium; // initialized by subclass ctors
   RefPtr<nsAtom> mMediaEmulated;
   RefPtr<gfxFontFeatureValueSet> mFontFeatureValuesLookup;
 
   // This pointer is nulled out through SetLinkHandler() in the destructors of
   // the classes which set it. (using SetLinkHandler() again).
--- a/layout/style/nsAnimationManager.h
+++ b/layout/style/nsAnimationManager.h
@@ -277,33 +277,33 @@ class nsAnimationManager final
   : public mozilla::CommonAnimationManager<mozilla::dom::CSSAnimation>
 {
 public:
   explicit nsAnimationManager(nsPresContext *aPresContext)
     : mozilla::CommonAnimationManager<mozilla::dom::CSSAnimation>(aPresContext)
   {
   }
 
-  NS_INLINE_DECL_REFCOUNTING(nsAnimationManager)
-
   typedef mozilla::AnimationCollection<mozilla::dom::CSSAnimation>
     CSSAnimationCollection;
   typedef nsTArray<RefPtr<mozilla::dom::CSSAnimation>>
     OwningCSSAnimationPtrArray;
 
+  ~nsAnimationManager() override = default;
 
   /**
    * This function does the same thing as the above UpdateAnimations()
    * but with servo's computed values.
    */
   void UpdateAnimations(
     mozilla::dom::Element* aElement,
     mozilla::CSSPseudoElementType aPseudoType,
     const mozilla::ComputedStyle* aComputedValues);
 
+
   // Utility function to walk through |aIter| to find the Keyframe with
   // matching offset and timing function but stopping as soon as the offset
   // differs from |aOffset| (i.e. it assumes a sorted iterator).
   //
   // If a matching Keyframe is found,
   //   Returns true and sets |aIndex| to the index of the matching Keyframe
   //   within |aIter|.
   //
@@ -332,19 +332,16 @@ public:
     return false;
   }
 
   bool AnimationMayBeReferenced(nsAtom* aName) const
   {
     return mMaybeReferencedAnimations.Contains(aName);
   }
 
-protected:
-  ~nsAnimationManager() override = default;
-
 private:
   // This includes all animation names referenced regardless of whether a
   // corresponding `@keyframes` rule is available.
   //
   // It may contain names which are no longer referenced, but it should always
   // contain names which are currently referenced, so that it is usable for
   // style invalidation.
   nsTHashtable<nsRefPtrHashKey<nsAtom>> mMaybeReferencedAnimations;
--- a/layout/style/nsTransitionManager.h
+++ b/layout/style/nsTransitionManager.h
@@ -303,34 +303,31 @@ class nsTransitionManager final
   : public mozilla::CommonAnimationManager<mozilla::dom::CSSTransition>
 {
 public:
   explicit nsTransitionManager(nsPresContext *aPresContext)
     : mozilla::CommonAnimationManager<mozilla::dom::CSSTransition>(aPresContext)
   {
   }
 
-  NS_INLINE_DECL_REFCOUNTING(nsTransitionManager)
+  ~nsTransitionManager() final = default;
 
   typedef mozilla::AnimationCollection<mozilla::dom::CSSTransition>
     CSSTransitionCollection;
 
-
   /**
    * Update transitions for stylo.
    */
   bool UpdateTransitions(
     mozilla::dom::Element *aElement,
     mozilla::CSSPseudoElementType aPseudoType,
     const mozilla::ComputedStyle& aOldStyle,
     const mozilla::ComputedStyle& aNewStyle);
 
-
 protected:
-  virtual ~nsTransitionManager() {}
 
   typedef nsTArray<RefPtr<mozilla::dom::CSSTransition>>
     OwningCSSTransitionPtrArray;
 
   // Update transitions. This will start new transitions,
   // replace existing transitions, and stop existing transitions
   // as needed. aDisp and aElement must be non-null.
   // aElementTransitions is the collection of current transitions, and it