Bug 1388692 - When doing hit testing, only flush animations if there are throttled compositor animations. r=birtles
☠☠ backed out by 8fba8ca208f2 ☠ ☠
authorHiroyuki Ikezoe <hikezoe@mozilla.com>
Mon, 14 Aug 2017 19:59:59 +0900
changeset 374558 27e08c76b767ec5296084fe3db2e981fc114e35b
parent 374557 988651d2348a5ae4e7688a1a6d020dababf47d67
child 374559 4bc49a585e26df37d713a19d6a100280001d206e
push id32332
push userkwierso@gmail.com
push dateTue, 15 Aug 2017 00:28:32 +0000
treeherdermozilla-central@92f3de33d97f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbirtles
bugs1388692
milestone57.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 1388692 - When doing hit testing, only flush animations if there are throttled compositor animations. r=birtles This patch changes UpdateAnimationOnlyStyles to only flush animation styles if there are throttled animation styles that could affect hit-testing and renames the function to UpdateAnimationStylesForHitTesting at the same time. For GeckoRestyleManager, the original UpdateAnimationOnlyStyles which flushes animation styles if there are any pending animation styles, is renamed to UpdateAnimationStyles for consistency. MozReview-Commit-ID: 89UleXjI2OE
layout/base/GeckoRestyleManager.cpp
layout/base/GeckoRestyleManager.h
layout/base/PresShell.cpp
layout/base/RestyleManager.h
layout/base/RestyleManagerInlines.h
layout/base/ServoRestyleManager.cpp
layout/base/ServoRestyleManager.h
--- a/layout/base/GeckoRestyleManager.cpp
+++ b/layout/base/GeckoRestyleManager.cpp
@@ -545,37 +545,37 @@ GeckoRestyleManager::ProcessPendingResty
   // Before we process any restyles, we need to ensure that style
   // resulting from any animations is up-to-date, so that if any style
   // changes we cause trigger transitions, we have the correct old style
   // for starting the transition.
   bool haveNonAnimation =
     mHavePendingNonAnimationRestyles || mDoRebuildAllStyleData;
   if (haveNonAnimation) {
     ++mAnimationGeneration;
-    UpdateOnlyAnimationStyles();
+    UpdateAnimationStyles();
   } else {
     // If we don't have non-animation style updates, then we have queued
     // up animation style updates from the refresh driver tick.  This
     // doesn't necessarily include *all* animation style updates, since
     // we might be suppressing main-thread updates for some animations,
-    // so we don't want to call UpdateOnlyAnimationStyles, which updates
+    // so we don't want to call UpdateAnimationStyles, which updates
     // all animations.  In other words, the work that we're about to do
     // to process the pending restyles queue is a *subset* of the work
-    // that UpdateOnlyAnimationStyles would do, since we're *not*
+    // that UpdateAnimationStyles would do, since we're *not*
     // updating transitions that are running on the compositor thread
     // and suppressed on the main thread.
     //
     // But when we update those styles, we want to suppress updates to
-    // transitions just like we do in UpdateOnlyAnimationStyles.  So we
+    // transitions just like we do in UpdateAnimationStyles.  So we
     // want to tell the transition manager to act as though we're in
-    // UpdateOnlyAnimationStyles.
+    // UpdateAnimationStyles.
     //
     // FIXME: In the future, we might want to refactor the way the
     // animation and transition manager do their refresh driver ticks so
-    // that we can use UpdateOnlyAnimationStyles, with a different
+    // that we can use UpdateAnimationStyles, with a different
     // boolean argument, for this update as well, instead of having them
     // post style updates in their WillRefresh methods.
     PresContext()->TransitionManager()->SetInAnimationOnlyStyleUpdate(true);
   }
 
   ProcessRestyles(mPendingRestyles);
 
   if (!haveNonAnimation) {
@@ -637,17 +637,17 @@ GeckoRestyleManager::EndProcessingRestyl
   PresContext()->FrameConstructor()->EndUpdate();
 
 #ifdef DEBUG
   PresContext()->PresShell()->VerifyStyleTree();
 #endif
 }
 
 void
-GeckoRestyleManager::UpdateOnlyAnimationStyles()
+GeckoRestyleManager::UpdateAnimationStyles()
 {
   bool doCSS = PresContext()->EffectCompositor()->HasPendingStyleUpdates();
 
   nsIDocument* document = PresContext()->Document();
   nsSMILAnimationController* animationController =
     document->HasAnimationController() ?
     document->GetAnimationController() :
     nullptr;
@@ -675,16 +675,37 @@ GeckoRestyleManager::UpdateOnlyAnimation
   }
 
   ProcessRestyles(tracker);
 
   transitionManager->SetInAnimationOnlyStyleUpdate(false);
 }
 
 void
+GeckoRestyleManager::UpdateAnimationStylesForHitTesting()
+{
+  MOZ_ASSERT(PresContext()->EffectCompositor()->HasThrottledStyleUpdates(),
+             "Should have throttled animation");
+
+  nsTransitionManager* transitionManager = PresContext()->TransitionManager();
+
+  transitionManager->SetInAnimationOnlyStyleUpdate(true);
+
+  RestyleTracker tracker(ELEMENT_HAS_PENDING_ANIMATION_ONLY_RESTYLE |
+                         ELEMENT_IS_POTENTIAL_ANIMATION_ONLY_RESTYLE_ROOT);
+  tracker.Init(this);
+
+  PresContext()->EffectCompositor()->AddStyleUpdatesTo(tracker);
+
+  ProcessRestyles(tracker);
+
+  transitionManager->SetInAnimationOnlyStyleUpdate(false);
+}
+
+void
 GeckoRestyleManager::PostRestyleEventInternal()
 {
   // Make sure we're not in a style refresh; if we are, we still have
   // a call to ProcessPendingRestyles coming and there's no need to
   // add ourselves as a refresh observer until then.
   nsIPresShell* presShell = PresContext()->PresShell();
   if (!mInStyleRefresh) {
     presShell->ObserveStyleFlushes();
--- a/layout/base/GeckoRestyleManager.h
+++ b/layout/base/GeckoRestyleManager.h
@@ -212,41 +212,40 @@ public:
 private:
   // ProcessPendingRestyles calls into one of our RestyleTracker
   // objects.  It then calls back to these functions at the beginning
   // and end of its work.
   void BeginProcessingRestyles(RestyleTracker& aRestyleTracker);
   void EndProcessingRestyles();
 
 public:
-  // Update styles for animations that are running on the compositor and
-  // whose updating is suppressed on the main thread (to save
-  // unnecessary work), while leaving all other aspects of style
-  // out-of-date.
+  // Perform any pending animation restyles.
   //
-  // Performs an animation-only style flush to make styles from
-  // throttled transitions up-to-date prior to processing an unrelated
-  // style change, so that any transitions triggered by that style
-  // change produce correct results.
+  // This is a superset of the work performed by
+  // UpdateAnimationStylesForHitTesting since it includes not only any
+  // animations whose restyles might have been suppressed on the main thread
+  // because they are running on the compositor, but *any* animations with
+  // pending restyles including SMIL animation restyles.
   //
   // In more detail:  when we're able to run animations on the
   // compositor, we sometimes "throttle" these animations by skipping
   // updating style data on the main thread.  However, whenever we
   // process a normal (non-animation) style change, any changes in
   // computed style on elements that have transition-* properties set
   // may need to trigger new transitions; this process requires knowing
   // both the old and new values of the property.  To do this correctly,
   // we need to have an up-to-date *old* value of the property on the
   // primary frame.  So the purpose of the mini-flush is to update the
   // style for all throttled transitions and animations to the current
   // animation state without making any other updates, so that when we
   // process the queued style updates we'll have correct old data to
   // compare against.  When we do this, we don't bother touching frames
   // other than primary frames.
-  void UpdateOnlyAnimationStyles();
+  void UpdateAnimationStyles();
+  void UpdateAnimationStylesForHitTesting();
 
   // Rebuilds all style data by throwing out the old rule tree and
   // building a new one, and additionally applying aExtraHint (which
   // must not contain nsChangeHint_ReconstructFrame) to the root frame.
   //
   // aRestyleHint says which restyle hint to use for the computation;
   // the only sensible values to use are eRestyle_Subtree (which says
   // that the rebuild must run selector matching) and nsRestyleHint(0)
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -6924,27 +6924,27 @@ nsIFrame* GetNearestFrameContainingPresS
   if (view) {
     frame = view->GetFrame();
   }
 
   return frame;
 }
 
 static bool
-FlushThrottledStyles(nsIDocument *aDocument, void *aData)
+FlushAnimationsForHitTesting(nsIDocument *aDocument, void *aData)
 {
   nsIPresShell* shell = aDocument->GetShell();
   if (shell && shell->IsVisible()) {
     nsPresContext* presContext = shell->GetPresContext();
     if (presContext) {
-      presContext->RestyleManager()->UpdateOnlyAnimationStyles();
-    }
-  }
-
-  aDocument->EnumerateSubDocuments(FlushThrottledStyles, nullptr);
+      presContext->RestyleManager()->UpdateAnimationStylesForHitTesting();
+    }
+  }
+
+  aDocument->EnumerateSubDocuments(FlushAnimationsForHitTesting, nullptr);
   return true;
 }
 
 /*
  * This function handles the preventDefault behavior of pointerdown. When user
  * preventDefault on pointerdown, We have to mark the active pointer to prevent
  * sebsequent mouse events (except mouse transition events) and default
  * behaviors.
@@ -7360,17 +7360,17 @@ PresShell::HandleEvent(nsIFrame* aFrame,
     if (mDocument) {
       if (aEvent->mClass == eTouchEventClass) {
         nsIDocument::UnlockPointer();
       }
 
       AutoWeakFrame weakFrame(frame);
       {  // scope for scriptBlocker.
         nsAutoScriptBlocker scriptBlocker;
-        FlushThrottledStyles(GetRootPresShell()->GetDocument(), nullptr);
+        FlushAnimationsForHitTesting(GetRootPresShell()->GetDocument(), nullptr);
       }
 
 
       if (!weakFrame.IsAlive()) {
         frame = GetNearestFrameContainingPresShell(this);
       }
     }
 
--- a/layout/base/RestyleManager.h
+++ b/layout/base/RestyleManager.h
@@ -187,17 +187,27 @@ public:
                                   const nsAttrValue* aNewValue);
   inline void AttributeChanged(dom::Element* aElement,
                                int32_t aNameSpaceID,
                                nsIAtom* aAttribute,
                                int32_t aModType,
                                const nsAttrValue* aOldValue);
   inline nsresult ReparentStyleContext(nsIFrame* aFrame);
 
-  inline void UpdateOnlyAnimationStyles();
+  // Update styles for animations if there are animations that are running on
+  // the compositor and whose updating is suppressed on the main thread (to save
+  // unnecessary work). This is needed to ensure that transform animations
+  // running on the compositor are updated on the main thread giving elements
+  // their up-to-date geometry before we attempt to perform hit-testing.
+  //
+  // Although SMIL animations may also be throttled, such animations do not
+  // affect hit-testing since only animations on elements in display:none
+  // subtree are throttled for SMIL. As a result, this method does not update
+  // throttled SMIL animations.
+  inline void UpdateAnimationStylesForHitTesting();
 
   // Get a counter that increments on every style change, that we use to
   // track whether off-main-thread animations are up-to-date.
   uint64_t GetAnimationGeneration() const { return mAnimationGeneration; }
 
   static uint64_t GetAnimationGenerationForFrame(nsIFrame* aFrame);
 
   // Update the animation generation count to mark that animation state
--- a/layout/base/RestyleManagerInlines.h
+++ b/layout/base/RestyleManagerInlines.h
@@ -75,16 +75,25 @@ RestyleManager::AttributeChanged(dom::El
 
 nsresult
 RestyleManager::ReparentStyleContext(nsIFrame* aFrame)
 {
   MOZ_STYLO_FORWARD(ReparentStyleContext, (aFrame));
 }
 
 void
-RestyleManager::UpdateOnlyAnimationStyles()
+RestyleManager::UpdateAnimationStylesForHitTesting()
 {
-  MOZ_STYLO_FORWARD(UpdateOnlyAnimationStyles, ());
+  // Throttled SMIL samples never affect hit-testing since they only apply to
+  // display:none subtrees so we don't need to handle them here.
+  //
+  // Bug 1353212: We don't need to flush opacity animations running on the
+  // compositor since opacity animations don't affect hit-testing.
+  if (!PresContext()->EffectCompositor()->HasThrottledStyleUpdates()) {
+    return;
+  }
+
+  MOZ_STYLO_FORWARD(UpdateAnimationStylesForHitTesting, ());
 }
 
 } // namespace mozilla
 
 #endif // mozilla_RestyleManagerInlines_h
--- a/layout/base/ServoRestyleManager.cpp
+++ b/layout/base/ServoRestyleManager.cpp
@@ -1158,24 +1158,20 @@ ServoRestyleManager::DoProcessPendingRes
 
 void
 ServoRestyleManager::ProcessPendingRestyles()
 {
   DoProcessPendingRestyles(ServoTraversalFlags::Empty);
 }
 
 void
-ServoRestyleManager::UpdateOnlyAnimationStyles()
+ServoRestyleManager::UpdateAnimationStylesForHitTesting()
 {
-  // Bug 1365855: We also need to implement this for SMIL.
-  bool doCSS = PresContext()->EffectCompositor()->HasPendingStyleUpdates();
-  if (!doCSS) {
-    return;
-  }
-
+  MOZ_ASSERT(PresContext()->EffectCompositor()->HasThrottledStyleUpdates(),
+             "Should have throttled animation");
   DoProcessPendingRestyles(ServoTraversalFlags::FlushThrottledAnimations);
 }
 
 void
 ServoRestyleManager::ContentStateChanged(nsIContent* aContent,
                                          EventStates aChangedBits)
 {
   MOZ_ASSERT(!mInStyleRefresh);
--- a/layout/base/ServoRestyleManager.h
+++ b/layout/base/ServoRestyleManager.h
@@ -181,30 +181,17 @@ public:
                         nsChangeHint aMinChangeHint);
   void PostRestyleEventForCSSRuleChanges();
   void RebuildAllStyleData(nsChangeHint aExtraHint,
                            nsRestyleHint aRestyleHint);
   void PostRebuildAllStyleDataEvent(nsChangeHint aExtraHint,
                                     nsRestyleHint aRestyleHint);
   void ProcessPendingRestyles();
 
-  /**
-   * Performs a Servo animation-only traversal to compute style for all nodes
-   * with the animation-only dirty bit in the document.
-   *
-   * This processes just the traversal for animation-only restyles and skips the
-   * normal traversal for other restyles unrelated to animations.
-   * This is used to bring throttled animations up-to-date such as when we need
-   * to get correct position for transform animations that are throttled because
-   * they are running on the compositor.
-   *
-   * This will traverse all of the document's style roots (that is, its document
-   * element, and the roots of the document-level native anonymous content).
-   */
-  void UpdateOnlyAnimationStyles();
+  void UpdateAnimationStylesForHitTesting();
 
   void ContentStateChanged(nsIContent* aContent, EventStates aStateMask);
   void AttributeWillChange(dom::Element* aElement,
                            int32_t aNameSpaceID,
                            nsIAtom* aAttribute,
                            int32_t aModType,
                            const nsAttrValue* aNewValue);
   void ClassAttributeWillBeChangedBySMIL(dom::Element* aElement);