Bug 1518816 - Replace nsLayoutUtils::HasCurrentTransition with something that takes an element/pseudo pair; r=hiro
authorBrian Birtles <birtles@gmail.com>
Mon, 18 Mar 2019 04:09:55 +0000
changeset 464770 b6614bc4be3f279d70fbd9a85f5a84b8445699ab
parent 464769 925ae534d52120a5d3a6be483a1271272d52c9b8
child 464771 8d9300956e10179cac87aff5f1a14d44776b2bf3
push id112481
push userrgurzau@mozilla.com
push dateMon, 18 Mar 2019 21:51:19 +0000
treeherdermozilla-inbound@6ed5e5a3e39e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershiro
bugs1518816
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1518816 - Replace nsLayoutUtils::HasCurrentTransition with something that takes an element/pseudo pair; r=hiro The trouble with utility functions that take an nsIFrame is it's not clear what the caller's intention is. For example, with nsLayoutUtils::HasCurrentTransition, is the caller asking for transitions on that frame? Or animations on _both_ that frame and its corresponding style/primary frame? Probably the caller hasn't even thought about it and there are likely to be bugs when display:table content is encountered. Where practical it's much better to take an element/pseudo pair since it's clear that the caller is concerned with all animations (or transitions in this case) on the element regardless of how it is represented in the frame tree. This patch updates nsLayoutUtils::HasCurrentTransition to take an element/pseudo pair and moves it to mozilla::AnimationUtils at the same time. Differential Revision: https://phabricator.services.mozilla.com/D23280
dom/animation/AnimationUtils.cpp
dom/animation/AnimationUtils.h
layout/base/nsLayoutUtils.cpp
layout/base/nsLayoutUtils.h
layout/xul/nsMenuPopupFrame.cpp
layout/xul/nsXULPopupManager.cpp
--- a/dom/animation/AnimationUtils.cpp
+++ b/dom/animation/AnimationUtils.cpp
@@ -79,9 +79,30 @@ bool AnimationUtils::EffectSetContainsAn
     if (effect->ContainsAnimatedScale(aFrame)) {
       return true;
     }
   }
 
   return false;
 }
 
+/* static */
+bool AnimationUtils::HasCurrentTransitions(const Element* aElement,
+                                           PseudoStyleType aPseudoType) {
+  MOZ_ASSERT(aElement);
+
+  EffectSet* effectSet = EffectSet::GetEffectSet(aElement, aPseudoType);
+  if (!effectSet) {
+    return false;
+  }
+
+  for (const dom::KeyframeEffect* effect : *effectSet) {
+    // If |effect| is current, it must have an associated Animation
+    // so we don't need to null-check the result of GetAnimation().
+    if (effect->IsCurrent() && effect->GetAnimation()->AsCSSTransition()) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
 }  // namespace mozilla
--- a/dom/animation/AnimationUtils.h
+++ b/dom/animation/AnimationUtils.h
@@ -78,13 +78,20 @@ class AnimationUtils {
   static bool IsOffscreenThrottlingEnabled();
 
   /**
    * Returns true if the given EffectSet contains a current effect that animates
    * scale. |aFrame| is used for calculation of scale values.
    */
   static bool EffectSetContainsAnimatedScale(EffectSet& aEffects,
                                              const nsIFrame* aFrame);
+
+  /**
+   * Returns true if the given (pseudo-)element has any transitions that are
+   * current (playing or waiting to play) or in effect (e.g. filling forwards).
+   */
+  static bool HasCurrentTransitions(const dom::Element* aElement,
+                                    PseudoStyleType aPseudoType);
 };
 
 }  // namespace mozilla
 
 #endif
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -234,24 +234,16 @@ static bool HasMatchingAnimations(const 
   EffectSet* effects = EffectSet::GetEffectSet(aFrame);
   if (!effects) {
     return false;
   }
 
   return HasMatchingAnimations(effects, aTest);
 }
 
-bool nsLayoutUtils::HasCurrentTransitions(const nsIFrame* aFrame) {
-  return HasMatchingAnimations(aFrame, [](KeyframeEffect& aEffect) {
-    // Since |aEffect| is current, it must have an associated Animation
-    // so we don't need to null-check the result of GetAnimation().
-    return aEffect.IsCurrent() && aEffect.GetAnimation()->AsCSSTransition();
-  });
-}
-
 template <typename EffectSetOrFrame>
 static bool MayHaveAnimationOfPropertySet(
     const EffectSetOrFrame* aTarget, const nsCSSPropertyIDSet& aPropertySet) {
   MOZ_ASSERT(aTarget);
   if (aPropertySet.Equals(nsCSSPropertyIDSet::OpacityProperties())) {
     return aTarget->MayHaveOpacityAnimation();
   }
 
--- a/layout/base/nsLayoutUtils.h
+++ b/layout/base/nsLayoutUtils.h
@@ -2280,23 +2280,16 @@ class nsLayoutUtils {
    *    (void)SizeOfTextRunsForFrames(rootFrame, nullptr, true);
    *    total = SizeOfTextRunsForFrames(rootFrame, mallocSizeOf, false);
    */
   static size_t SizeOfTextRunsForFrames(nsIFrame* aFrame,
                                         mozilla::MallocSizeOf aMallocSizeOf,
                                         bool clear);
 
   /**
-   * Returns true if the frame has any current CSS transitions.
-   * A current transition is any transition that has not yet finished playing
-   * including paused transitions.
-   */
-  static bool HasCurrentTransitions(const nsIFrame* aFrame);
-
-  /**
    * Returns true if |aFrame| has an animation of a property in |aPropertySet|
    * regardless of whether any property in the set is overridden by !important
    * rule.
    */
   static bool HasAnimationOfPropertySet(const nsIFrame* aFrame,
                                         const nsCSSPropertyIDSet& aPropertySet);
 
   /**
--- a/layout/xul/nsMenuPopupFrame.cpp
+++ b/layout/xul/nsMenuPopupFrame.cpp
@@ -39,16 +39,17 @@
 #include "nsIBaseWindow.h"
 #include "nsISound.h"
 #include "nsIScreenManager.h"
 #include "nsIServiceManager.h"
 #include "nsStyleConsts.h"
 #include "nsTransitionManager.h"
 #include "nsDisplayList.h"
 #include "nsIDOMXULSelectCntrlItemEl.h"
+#include "mozilla/AnimationUtils.h"
 #include "mozilla/EventDispatcher.h"
 #include "mozilla/EventStateManager.h"
 #include "mozilla/EventStates.h"
 #include "mozilla/Preferences.h"
 #include "mozilla/LookAndFeel.h"
 #include "mozilla/MouseEvents.h"
 #include "mozilla/dom/Element.h"
 #include "mozilla/dom/Event.h"
@@ -566,17 +567,18 @@ void nsMenuPopupFrame::LayoutPopup(nsBox
     }
 
 #ifndef MOZ_WIDGET_GTK
     // If the animate attribute is set to open, check for a transition and wait
     // for it to finish before firing the popupshown event.
     if (mContent->AsElement()->AttrValueIs(kNameSpaceID_None,
                                            nsGkAtoms::animate, nsGkAtoms::open,
                                            eCaseMatters) &&
-        nsLayoutUtils::HasCurrentTransitions(this)) {
+        AnimationUtils::HasCurrentTransitions(mContent->AsElement(),
+                                              PseudoStyleType::NotPseudo)) {
       mPopupShownDispatcher = new nsXULPopupShownEvent(mContent, pc);
       mContent->AddSystemEventListener(NS_LITERAL_STRING("transitionend"),
                                        mPopupShownDispatcher, false, false);
       return;
     }
 #endif
 
     // If there are no transitions, fire the popupshown event right away.
--- a/layout/xul/nsXULPopupManager.cpp
+++ b/layout/xul/nsXULPopupManager.cpp
@@ -26,16 +26,17 @@
 #include "nsIInterfaceRequestorUtils.h"
 #include "nsIBaseWindow.h"
 #include "nsCaret.h"
 #include "mozilla/dom/Document.h"
 #include "nsPIWindowRoot.h"
 #include "nsFrameManager.h"
 #include "nsIObserverService.h"
 #include "XULDocument.h"
+#include "mozilla/AnimationUtils.h"
 #include "mozilla/dom/Element.h"
 #include "mozilla/dom/Event.h"  // for Event
 #include "mozilla/dom/KeyboardEvent.h"
 #include "mozilla/dom/KeyboardEventBinding.h"
 #include "mozilla/dom/MouseEvent.h"
 #include "mozilla/dom/UIEvent.h"
 #include "mozilla/EventDispatcher.h"
 #include "mozilla/EventStateManager.h"
@@ -1448,17 +1449,18 @@ void nsXULPopupManager::FirePopupHidingE
         if (!animate.EqualsLiteral("false") &&
             (!animate.EqualsLiteral("cancel") || aIsCancel)) {
           presShell->FlushPendingNotifications(FlushType::Layout);
 
           // Get the frame again in case the flush caused it to go away
           popupFrame = do_QueryFrame(aPopup->GetPrimaryFrame());
           if (!popupFrame) return;
 
-          if (nsLayoutUtils::HasCurrentTransitions(popupFrame)) {
+          if (AnimationUtils::HasCurrentTransitions(
+                  aPopup->AsElement(), PseudoStyleType::NotPseudo)) {
             RefPtr<TransitionEnder> ender =
                 new TransitionEnder(aPopup, aDeselectMenu);
             aPopup->AddSystemEventListener(NS_LITERAL_STRING("transitionend"),
                                            ender, false, false);
             return;
           }
         }
       }