Bug 1171817 part 15 - Factor out common code for comparing owning elements into a separate class; r=dbaron

--HG--
extra : commitid : 2j2k4UORffU
extra : rebase_source : ab639c3c163670bc3a291f20d657bf2aef015d95

layout/style/AnimationCommon.h

@@ -22,6 +22,7 @@
 #include "mozilla/Attributes.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/FloatingPoint.h"
+#include "nsContentUtils.h"
 #include "nsCSSPseudoElements.h"
 #include "nsCycleCollectionParticipant.h"
 #include "nsCSSPropertySet.h"
@@ -455,6 +456,61 @@ public:
 #endif
 };
 
-}
+/**
+ * Utility class for referencing the element that created a CSS animation or
+ * transition. It is non-owning (i.e. it uses a raw pointer) since it is only
+ * expected to be set by the owned animation while it actually being managed
+ * by the owning element.
+ *
+ * This class also abstracts the comparison of an element/pseudo-class pair
+ * for the sake of composite ordering since this logic is common to both CSS
+ * animations and transitions.
+ *
+ * (We call this OwningElementRef instead of just OwningElement so that we can
+ * call the getter on CSSAnimation/CSSTransition OwningElement() without
+ * clashing with this object's contructor.)
+ */
+class OwningElementRef final
+{
+public:
+  OwningElementRef()
+    : mElement(nullptr)
+    , mPseudoType(nsCSSPseudoElements::ePseudo_NotPseudoElement)
+  { }
+
+  OwningElementRef(dom::Element& aElement,
+                   nsCSSPseudoElements::Type aPseudoType)
+    : mElement(&aElement)
+    , mPseudoType(aPseudoType)
+  { }
+
+  bool Equals(const OwningElementRef& aOther) const
+  {
+    return mElement == aOther.mElement &&
+           mPseudoType == aOther.mPseudoType;
+  }
+
+  bool LessThan(const OwningElementRef& aOther) const
+  {
+    MOZ_ASSERT(mElement && aOther.mElement,
+               "Elements to compare should not be null");
+
+    if (mElement != aOther.mElement) {
+      return nsContentUtils::PositionIsBefore(mElement, aOther.mElement);
+    }
+
+    return mPseudoType == nsCSSPseudoElements::ePseudo_NotPseudoElement ||
+          (mPseudoType == nsCSSPseudoElements::ePseudo_before &&
+           aOther.mPseudoType == nsCSSPseudoElements::ePseudo_after);
+  }
+
+  bool IsSet() const { return !!mElement; }
+
+private:
+  dom::Element* MOZ_NON_OWNING_REF mElement;
+  nsCSSPseudoElements::Type        mPseudoType;
+};
+
+} // namespace mozilla
 
 #endif /* !defined(mozilla_css_AnimationCommon_h) */

layout/style/nsAnimationManager.cpp

@@ -15,7 +15,6 @@
 #include "nsPresContext.h"
 #include "nsStyleSet.h"
 #include "nsStyleChangeList.h"
-#include "nsContentUtils.h"
 #include "nsCSSRules.h"
 #include "RestyleManager.h"
 #include "nsLayoutUtils.h"
@@ -146,26 +145,11 @@ CSSAnimation::HasLowerCompositeOrderThan(const Animation& aOther) const
   }
 
   // 3. Sort by document order
-  Element* ourElement;
-  nsCSSPseudoElements::Type ourPseudoType;
-  GetOwningElement(ourElement, ourPseudoType);
-
-  Element* otherElement;
-  nsCSSPseudoElements::Type otherPseudoType;
-  otherAnimation->GetOwningElement(otherElement, otherPseudoType);
-  MOZ_ASSERT(ourElement && otherElement,
+  MOZ_ASSERT(mOwningElement.IsSet() && otherAnimation->OwningElement().IsSet(),
              "Animations using custom composite order should have an "
              "owning element");
-
-  if (ourElement != otherElement) {
-    return nsContentUtils::PositionIsBefore(ourElement, otherElement);
-  }
-
-  // 3b. Sort by pseudo: (none) < before < after
-  if (ourPseudoType != otherPseudoType) {
-    return ourPseudoType == nsCSSPseudoElements::ePseudo_NotPseudoElement ||
-           (ourPseudoType == nsCSSPseudoElements::ePseudo_before &&
-            otherPseudoType == nsCSSPseudoElements::ePseudo_after);
+  if (!mOwningElement.Equals(otherAnimation->OwningElement())) {
+    return mOwningElement.LessThan(otherAnimation->OwningElement());
   }
 
   // 4. (Same element and pseudo): Sort by position in animation-name
@@ -614,7 +598,8 @@ nsAnimationManager::BuildAnimations(nsStyleContext* aStyleContext,
     }
 
     nsRefPtr<CSSAnimation> dest = new CSSAnimation(aTimeline, src.GetName());
-    dest->SetOwningElement(*aTarget, aStyleContext->GetPseudoType());
+    dest->SetOwningElement(
+      OwningElementRef(*aTarget, aStyleContext->GetPseudoType()));
     dest->SetAnimationIndex(static_cast<uint64_t>(animIdx));
     aAnimations.AppendElement(dest);
 

layout/style/nsAnimationManager.h

@@ -60,8 +60,6 @@ public:
                        const nsSubstring& aAnimationName)
     : dom::Animation(aTimeline)
     , mAnimationName(aAnimationName)
-    , mOwningElement(nullptr)
-    , mOwningPseudoType(nsCSSPseudoElements::ePseudo_NotPseudoElement)
     , mIsStylePaused(false)
     , mPauseShouldStick(false)
     , mPreviousPhaseOrIteration(PREVIOUS_PHASE_BEFORE)
@@ -97,9 +95,7 @@ public:
   void PauseFromStyle();
   void CancelFromStyle() override
   {
-    mOwningElement = nullptr;
-    mOwningPseudoType = nsCSSPseudoElements::ePseudo_NotPseudoElement;
-
+    mOwningElement = OwningElementRef();
     Animation::CancelFromStyle();
     MOZ_ASSERT(mSequenceNum == kUnsequenced);
   }
@@ -107,7 +103,10 @@ public:
   bool IsStylePaused() const { return mIsStylePaused; }
 
   bool HasLowerCompositeOrderThan(const Animation& aOther) const override;
-  bool IsUsingCustomCompositeOrder() const override { return !!mOwningElement; }
+  bool IsUsingCustomCompositeOrder() const override
+  {
+    return mOwningElement.IsSet();
+  }
 
   void SetAnimationIndex(uint64_t aIndex)
   {
@@ -138,28 +137,16 @@ public:
   // c) If this object is generated from script using the CSSAnimation
   //    constructor.
   //
-  // For (b) and (c) the returned owning element will by nullptr and the
-  // pseudo-type will be nsCSSPseudoElements::ePseudo_NotPseudoElement.
-  void GetOwningElement(dom::Element*& aElement,
-                        nsCSSPseudoElements::Type& aPseudoType) const {
-    MOZ_ASSERT(mOwningElement != nullptr ||
-               mOwningPseudoType ==
-                 nsCSSPseudoElements::ePseudo_NotPseudoElement,
-               "When there is no owning element there should be no "
-               "pseudo-type");
-    aElement = mOwningElement;
-    aPseudoType = mOwningPseudoType;
-  }
+  // For (b) and (c) the returned owning element will return !IsSet().
+  const OwningElementRef& OwningElement() const { return mOwningElement; }
 
-  // Sets the owning element and pseudo-type which is used for determining
-  // the composite order of CSSAnimation objects generated from CSS markup.
+  // Sets the owning element which is used for determining the composite
+  // order of CSSAnimation objects generated from CSS markup.
   //
-  // @see GetOwningElement.
-  void SetOwningElement(dom::Element& aElement,
-                        nsCSSPseudoElements::Type aPseudoType)
+  // @see OwningElement()
+  void SetOwningElement(const OwningElementRef& aElement)
   {
-    mOwningElement = &aElement;
-    mOwningPseudoType = aPseudoType;
+    mOwningElement = aElement;
   }
 
   // Is this animation currently in effect for the purposes of computing
@@ -172,8 +159,8 @@ public:
 protected:
   virtual ~CSSAnimation()
   {
-    MOZ_ASSERT(!mOwningElement, "Owning element should be cleared before a "
-                                "CSS animation is destroyed");
+    MOZ_ASSERT(!mOwningElement.IsSet(), "Owning element should be cleared "
+                                        "before a CSS animation is destroyed");
   }
   virtual css::CommonAnimationManager* GetAnimationManager() const override;
 
@@ -183,11 +170,7 @@ protected:
 
   // The (pseudo-)element whose computed animation-name refers to this
   // animation (if any).
-  //
-  // Raw pointer because this is only ever set when this object is part
-  // of mOwningElement's AnimationCollection.
-  dom::Element* MOZ_NON_OWNING_REF mOwningElement;
-  nsCSSPseudoElements::Type        mOwningPseudoType;
+  OwningElementRef mOwningElement;
 
   // When combining animation-play-state with play() / pause() the following
   // behavior applies:

layout/style/nsTransitionManager.cpp

@@ -158,26 +158,11 @@ CSSTransition::HasLowerCompositeOrderThan(const Animation& aOther) const
   }
 
   // 3. Sort by document order
-  Element* ourElement;
-  nsCSSPseudoElements::Type ourPseudoType;
-  GetOwningElement(ourElement, ourPseudoType);
-
-  Element* otherElement;
-  nsCSSPseudoElements::Type otherPseudoType;
-  otherTransition->GetOwningElement(otherElement, otherPseudoType);
-  MOZ_ASSERT(ourElement && otherElement,
-             "Transitions using custom composite order should have an "
-             "owning element");
-
-  if (ourElement != otherElement) {
-    return nsContentUtils::PositionIsBefore(ourElement, otherElement);
-  }
-
-  // 3b. Sort by pseudo: (none) < before < after
-  if (ourPseudoType != otherPseudoType) {
-    return ourPseudoType == nsCSSPseudoElements::ePseudo_NotPseudoElement ||
-           (ourPseudoType == nsCSSPseudoElements::ePseudo_before &&
-            otherPseudoType == nsCSSPseudoElements::ePseudo_after);
+  MOZ_ASSERT(mOwningElement.IsSet() && otherTransition->OwningElement().IsSet(),
+             "Transitions using custom composite order should have an owning "
+             "element");
+  if (!mOwningElement.Equals(otherTransition->OwningElement())) {
+    return mOwningElement.LessThan(otherTransition->OwningElement());
   }
 
   // 4. (Same element and pseudo): Sort by transition generation
@@ -647,7 +632,8 @@ nsTransitionManager::ConsiderStartingTransition(
   segment.mTimingFunction.Init(tf);
 
   nsRefPtr<CSSTransition> animation = new CSSTransition(timeline);
-  animation->SetOwningElement(*aElement, aNewStyleContext->GetPseudoType());
+  animation->SetOwningElement(
+    OwningElementRef(*aElement, aNewStyleContext->GetPseudoType()));
   animation->SetCreationSequence(
     mPresContext->RestyleManager()->GetAnimationGeneration());
   // The order of the following two calls is important since PlayFromStyle

layout/style/nsTransitionManager.h

@@ -81,8 +81,6 @@ class CSSTransition final : public Animation
 public:
  explicit CSSTransition(dom::DocumentTimeline* aTimeline)
     : dom::Animation(aTimeline)
-    , mOwningElement(nullptr)
-    , mOwningPseudoType(nsCSSPseudoElements::ePseudo_NotPseudoElement)
   {
   }
 
@@ -111,9 +109,7 @@ public:
 
   void CancelFromStyle() override
   {
-    mOwningElement = nullptr;
-    mOwningPseudoType = nsCSSPseudoElements::ePseudo_NotPseudoElement;
-
+    mOwningElement = OwningElementRef();
     Animation::CancelFromStyle();
     MOZ_ASSERT(mSequenceNum == kUnsequenced);
   }
@@ -121,7 +117,10 @@ public:
   nsCSSProperty TransitionProperty() const;
 
   bool HasLowerCompositeOrderThan(const Animation& aOther) const override;
-  bool IsUsingCustomCompositeOrder() const override { return !!mOwningElement; }
+  bool IsUsingCustomCompositeOrder() const override
+  {
+    return mOwningElement.IsSet();
+  }
 
   void SetCreationSequence(uint64_t aIndex)
   {
@@ -131,8 +130,7 @@ public:
 
   // Returns the element or pseudo-element whose transition-property property
   // this CSSTransition corresponds to (if any). This is used for determining
-  // the relative priority of transitions generated from CSS transitions
-  // markup.
+  // the relative composite order of transitions generated from CSS markup.
   //
   // Typically this will be the same as the target element of the keyframe
   // effect associated with this transition. However, it can differ in the
@@ -144,46 +142,30 @@ public:
   // c) If this object is generated from script using the CSSTransition
   //    constructor.
   //
-  // For (b) and (c) the returned owning element will by nullptr and the
-  // pseudo-type will be nsCSSPseudoElements::ePseudo_NotPseudoElement.
-  void GetOwningElement(dom::Element*& aElement,
-                        nsCSSPseudoElements::Type& aPseudoType) const {
-    MOZ_ASSERT(mOwningElement != nullptr ||
-               mOwningPseudoType ==
-                 nsCSSPseudoElements::ePseudo_NotPseudoElement,
-               "When there is no owning element there should be no "
-               "pseudo-type");
-    aElement = mOwningElement;
-    aPseudoType = mOwningPseudoType;
-  }
+  // For (b) and (c) the returned owning element will return !IsSet().
+  const OwningElementRef& OwningElement() const { return mOwningElement; }
 
-  // Sets the owning element and pseudo-type which is used for prioritizing
-  // CSSTransition objects generated from CSS markup.
+  // Sets the owning element which is used for determining the composite
+  // oder of CSSTransition objects generated from CSS markup.
   //
-  // @see GetOwningElement.
-  void SetOwningElement(dom::Element& aElement,
-                        nsCSSPseudoElements::Type aPseudoType)
+  // @see OwningElement()
+  void SetOwningElement(const OwningElementRef& aElement)
   {
-    mOwningElement = &aElement;
-    mOwningPseudoType = aPseudoType;
+    mOwningElement = aElement;
   }
 
 protected:
   virtual ~CSSTransition()
   {
-    MOZ_ASSERT(!mOwningElement, "Owning element should be cleared before a "
-                                "CSS transition is destroyed");
+    MOZ_ASSERT(!mOwningElement.IsSet(), "Owning element should be cleared "
+                                        "before a CSS transition is destroyed");
   }
 
   virtual css::CommonAnimationManager* GetAnimationManager() const override;
 
   // The (pseudo-)element whose computed transition-property refers to this
   // transition (if any).
-  //
-  // Raw pointer because this is only ever set when this object is part
-  // of mOwningElement's AnimationCollection.
-  dom::Element* MOZ_NON_OWNING_REF mOwningElement;
-  nsCSSPseudoElements::Type        mOwningPseudoType;
+  OwningElementRef mOwningElement;
 };
 
 } // namespace dom