Bug 1340684 - Fire the scroll event before the style flush. r=mstange draft
authorBotond Ballo <botond@mozilla.com>
Wed, 09 Aug 2017 21:08:38 -0400
changeset 647807 eac492c773f991a333571b650fca12c424582e7a
parent 643173 4c5fbf49376351679dcc49f4cff26c3c2e055ccc
child 726637 d12b158aa4a89b36caab7ca2df08cceb4920e313
push id74545
push userbballo@mozilla.com
push dateWed, 16 Aug 2017 23:24:41 +0000
reviewersmstange
bugs1340684
milestone57.0a1
Bug 1340684 - Fire the scroll event before the style flush. r=mstange This ensures that if the scroll event triggers style changes, they are reflected on the same paint. This is accomplished by having the refresh driver fire scroll events as an explicit step after FlushType::Style observers and rAF callbacks, and before the actual style flush. MozReview-Commit-ID: 4kgauD5SgVo
layout/base/nsRefreshDriver.cpp
layout/base/nsRefreshDriver.h
layout/generic/nsGfxScrollFrame.cpp
layout/generic/nsGfxScrollFrame.h
layout/generic/nsIScrollableFrame.h
--- a/layout/base/nsRefreshDriver.cpp
+++ b/layout/base/nsRefreshDriver.cpp
@@ -1300,16 +1300,39 @@ bool
 nsRefreshDriver::RemoveRefreshObserver(nsARefreshObserver* aObserver,
                                        FlushType aFlushType)
 {
   ObserverArray& array = ArrayFor(aFlushType);
   return array.RemoveElement(aObserver);
 }
 
 void
+nsRefreshDriver::PostScrollEvent(ScrollEvent* aScrollEvent)
+{
+  mScrollEvents.AppendElement(aScrollEvent);
+  EnsureTimerStarted();
+}
+
+bool
+nsRefreshDriver::RemoveScrollEvent(ScrollEvent* aScrollEvent)
+{
+  return mScrollEvents.RemoveElement(aScrollEvent);
+}
+
+void
+nsRefreshDriver::DispatchScrollEvents()
+{
+  ScrollEventArray::EndLimitedIterator etor(mScrollEvents);
+  while (etor.HasMore()) {
+    RefPtr<ScrollEvent> event = etor.GetNext();
+    event->Fire();
+  }
+}
+
+void
 nsRefreshDriver::AddPostRefreshObserver(nsAPostRefreshObserver* aObserver)
 {
   mPostRefreshObservers.AppendElement(aObserver);
 }
 
 void
 nsRefreshDriver::RemovePostRefreshObserver(nsAPostRefreshObserver* aObserver)
 {
@@ -1803,17 +1826,17 @@ nsRefreshDriver::Tick(int64_t aNowEpoch,
   if (mRootRefresh) {
     mRootRefresh->RemoveRefreshObserver(this, FlushType::Style);
     mRootRefresh = nullptr;
   }
   mSkippedPaints = false;
   mWarningThreshold = 1;
 
   nsCOMPtr<nsIPresShell> presShell = mPresContext->GetPresShell();
-  if (!presShell || (ObserverCount() == 0 && ImageRequestCount() == 0)) {
+  if (!presShell || (ObserverCount() == 0 && ImageRequestCount() == 0 && mScrollEvents.Length() == 0)) {
     // Things are being destroyed, or we no longer have any observers.
     // We don't want to stop the timer when observers are initially
     // removed, because sometimes observers can be added and removed
     // often depending on what other things are going on and in that
     // situation we don't want to thrash our timer.  So instead we
     // wait until we get a Notify() call when we have no observers
     // before stopping the timer.
     StopTimer();
@@ -1862,16 +1885,17 @@ nsRefreshDriver::Tick(int64_t aNowEpoch,
     }
 
     if (i == 0) {
       // This is the FlushType::Style case.
 
       DispatchAnimationEvents();
       DispatchPendingEvents();
       RunFrameRequestCallbacks(aNowTime);
+      DispatchScrollEvents();
 
       if (mPresContext && mPresContext->GetPresShell()) {
         Maybe<AutoProfilerTracing> tracingStyleFlush;
         AutoTArray<nsIPresShell*, 16> observers;
         observers.AppendElements(mStyleFlushObservers);
         for (uint32_t j = observers.Length();
              j && mPresContext && mPresContext->GetPresShell(); --j) {
           // Make sure to not process observers which might have been removed
--- a/layout/base/nsRefreshDriver.h
+++ b/layout/base/nsRefreshDriver.h
@@ -17,16 +17,17 @@
 #include "mozilla/Vector.h"
 #include "mozilla/WeakPtr.h"
 #include "nsTObserverArray.h"
 #include "nsTArray.h"
 #include "nsTHashtable.h"
 #include "nsTObserverArray.h"
 #include "nsClassHashtable.h"
 #include "nsHashKeys.h"
+#include "nsIScrollableFrame.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/Maybe.h"
 #include "GeckoProfiler.h"
 #include "mozilla/layers/TransactionIdAllocator.h"
 
 class nsPresContext;
 class nsIPresShell;
 class nsIDocument;
@@ -69,16 +70,18 @@ class nsAPostRefreshObserver {
 public:
   virtual void DidRefresh() = 0;
 };
 
 class nsRefreshDriver final : public mozilla::layers::TransactionIdAllocator,
                               public nsARefreshObserver
 {
 public:
+  typedef nsIScrollableFrame::ScrollEvent ScrollEvent;
+
   explicit nsRefreshDriver(nsPresContext *aPresContext);
   ~nsRefreshDriver();
 
   /**
    * Methods for testing, exposed via nsIDOMWindowUtils.  See
    * nsIDOMWindowUtils.advanceTimeAndRefresh for description.
    */
   void AdvanceTimeAndRefresh(int64_t aMilliseconds);
@@ -120,16 +123,20 @@ public:
    *
    * The observer will be called even if there is no other activity.
    */
   bool AddRefreshObserver(nsARefreshObserver *aObserver,
                           mozilla::FlushType aFlushType);
   bool RemoveRefreshObserver(nsARefreshObserver *aObserver,
                              mozilla::FlushType aFlushType);
 
+  void PostScrollEvent(ScrollEvent* aScrollEvent);
+  bool RemoveScrollEvent(ScrollEvent* aScrollEvent);
+  void DispatchScrollEvents();
+
   /**
    * Add an observer that will be called after each refresh. The caller
    * must remove the observer before it is deleted. This does not trigger
    * refresh driver ticks.
    */
   void AddPostRefreshObserver(nsAPostRefreshObserver *aObserver);
   void RemovePostRefreshObserver(nsAPostRefreshObserver *aObserver);
 
@@ -351,16 +358,17 @@ public:
 
   bool SkippedPaints() const
   {
     return mSkippedPaints;
   }
 
 private:
   typedef nsTObserverArray<nsARefreshObserver*> ObserverArray;
+  typedef nsTObserverArray<ScrollEvent*> ScrollEventArray;
   typedef nsTHashtable<nsISupportsHashKey> RequestTable;
   struct ImageStartData {
     ImageStartData()
     {
     }
 
     mozilla::Maybe<mozilla::TimeStamp> mStartTime;
     RequestTable mEntries;
@@ -457,16 +465,17 @@ private:
   mozilla::TimeStamp mNextThrottledFrameRequestTick;
   mozilla::TimeStamp mNextRecomputeVisibilityTick;
 
   // separate arrays for each flush type we support
   ObserverArray mObservers[3];
   RequestTable mRequests;
   ImageStartTable mStartTable;
   AutoTArray<nsCOMPtr<nsIRunnable>, 16> mEarlyRunners;
+  ScrollEventArray mScrollEvents;
 
   struct PendingEvent {
     nsCOMPtr<nsINode> mTarget;
     nsCOMPtr<nsIDOMEvent> mEvent;
   };
 
   AutoTArray<nsIPresShell*, 16> mStyleFlushObservers;
   AutoTArray<nsIPresShell*, 16> mLayoutFlushObservers;
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@@ -4708,37 +4708,39 @@ void ScrollFrameHelper::CurPosAttributeC
   ScrollToWithOrigin(dest,
                      isSmooth ? nsIScrollableFrame::SMOOTH : nsIScrollableFrame::INSTANT,
                      nsGkAtoms::scrollbars, &allowedRange);
   // 'this' might be destroyed here
 }
 
 /* ============= Scroll events ========== */
 
-ScrollFrameHelper::ScrollEvent::ScrollEvent(ScrollFrameHelper* aHelper)
-  : mHelper(aHelper)
-{
-  mDriver = mHelper->mOuter->PresContext()->RefreshDriver();
-  mDriver->AddRefreshObserver(this, FlushType::Layout);
-}
-
-ScrollFrameHelper::ScrollEvent::~ScrollEvent()
+nsIScrollableFrame::ScrollEvent::ScrollEvent(nsIScrollableFrame* aScrollableFrame)
+  : mScrollableFrame(aScrollableFrame)
+{
+  nsIFrame* frame = do_QueryFrame(aScrollableFrame);
+  MOZ_ASSERT(frame);
+  mDriver = frame->PresContext()->RefreshDriver();
+  mDriver->PostScrollEvent(this);
+}
+
+nsIScrollableFrame::ScrollEvent::~ScrollEvent()
 {
   if (mDriver) {
-    mDriver->RemoveRefreshObserver(this, FlushType::Layout);
+    mDriver->RemoveScrollEvent(this);
     mDriver = nullptr;
   }
 }
 
 void
-ScrollFrameHelper::ScrollEvent::WillRefresh(mozilla::TimeStamp aTime)
-{
-  mDriver->RemoveRefreshObserver(this, FlushType::Layout);
+nsIScrollableFrame::ScrollEvent::Fire()
+{
+  mDriver->RemoveScrollEvent(this);
   mDriver = nullptr;
-  mHelper->FireScrollEvent();
+  mScrollableFrame->FireScrollEvent();
 }
 
 void
 ScrollFrameHelper::FireScrollEvent()
 {
   AutoProfilerTracing tracing("Paint", "FireScrollEvent");
   MOZ_ASSERT(mScrollEvent);
   mScrollEvent = nullptr;
@@ -4769,17 +4771,18 @@ ScrollFrameHelper::FireScrollEvent()
 void
 ScrollFrameHelper::PostScrollEvent()
 {
   if (mScrollEvent) {
     return;
   }
 
   // The ScrollEvent constructor registers itself with the refresh driver.
-  mScrollEvent = new ScrollEvent(this);
+  nsIScrollableFrame* sf = do_QueryFrame(mOuter);
+  mScrollEvent = new nsIScrollableFrame::ScrollEvent(sf);
 }
 
 NS_IMETHODIMP
 ScrollFrameHelper::AsyncScrollPortEvent::Run()
 {
   if (mHelper) {
     mHelper->mOuter->PresContext()->GetPresShell()->
       FlushPendingNotifications(FlushType::InterruptibleLayout);
--- a/layout/generic/nsGfxScrollFrame.h
+++ b/layout/generic/nsGfxScrollFrame.h
@@ -94,51 +94,16 @@ public:
 
   void PostScrollEvent();
   void FireScrollEvent();
   void PostScrolledAreaEvent();
   void FireScrolledAreaEvent();
 
   bool IsSmoothScrollingEnabled();
 
-  /**
-   * This class handles the dispatching of scroll events to content.
-   *
-   * nsRefreshDriver maintains three lists of refresh observers, one for each
-   * flush type: FlushType::Style, FlushType::Layout, and FlushType::Display.
-   *
-   * During a tick, it runs through each list of observers, in order, and runs
-   * them. To iterate over each list, it uses an EndLimitedIterator, which is
-   * designed to iterate only over elements present when the iterator was
-   * created, not elements added afterwards. This means that, for a given flush
-   * type, a refresh observer added during the execution of another refresh
-   * observer of that flush type, will not run until the next tick.
-   *
-   * During main-thread animation-driven scrolling, ScrollEvents are *posted*
-   * by AsyncScroll::WillRefresh(). AsyncScroll registers itself as a FlushType::Style
-   * refresh observer.
-   *
-   * Posting a scroll event, as of bug 1250550, registers a FlushType::Layout
-   * refresh observer, which *fires* the event when run. This allows the event
-   * to be fired to content in the same refresh driver tick as it is posted.
-   * This is an important invariant to maintain to reduce scroll event latency
-   * for main-thread scrolling.
-   */
-  class ScrollEvent : public nsARefreshObserver {
-  public:
-    NS_INLINE_DECL_REFCOUNTING(ScrollEvent, override)
-    explicit ScrollEvent(ScrollFrameHelper *helper);
-    void WillRefresh(mozilla::TimeStamp aTime) override;
-  protected:
-    virtual ~ScrollEvent();
-  private:
-    ScrollFrameHelper *mHelper;
-    RefPtr<nsRefreshDriver> mDriver;
-  };
-
   class AsyncScrollPortEvent : public Runnable {
   public:
     NS_DECL_NSIRUNNABLE
     explicit AsyncScrollPortEvent(ScrollFrameHelper* helper)
       : Runnable("ScrollFrameHelper::AsyncScrollPortEvent")
       , mHelper(helper)
     {
     }
@@ -481,17 +446,17 @@ public:
   bool IsRootScrollFrameOfDocument() const { return mIsRoot; }
 
   // owning references to the nsIAnonymousContentCreator-built content
   nsCOMPtr<nsIContent> mHScrollbarContent;
   nsCOMPtr<nsIContent> mVScrollbarContent;
   nsCOMPtr<nsIContent> mScrollCornerContent;
   nsCOMPtr<nsIContent> mResizerContent;
 
-  RefPtr<ScrollEvent> mScrollEvent;
+  RefPtr<nsIScrollableFrame::ScrollEvent> mScrollEvent;
   nsRevocableEventPtr<AsyncScrollPortEvent> mAsyncScrollPortEvent;
   nsRevocableEventPtr<ScrolledAreaEvent> mScrolledAreaEvent;
   nsIFrame* mHScrollbarBox;
   nsIFrame* mVScrollbarBox;
   nsIFrame* mScrolledFrame;
   nsIFrame* mScrollCornerBox;
   nsIFrame* mResizerBox;
   nsContainerFrame* mOuter;
@@ -1060,16 +1025,19 @@ public:
 #ifdef DEBUG_FRAME_DUMP
   virtual nsresult GetFrameName(nsAString& aResult) const override;
 #endif
 
 #ifdef ACCESSIBILITY
   virtual mozilla::a11y::AccType AccessibleType() override;
 #endif
 
+  virtual void FireScrollEvent() override {
+    mHelper.FireScrollEvent();
+  }
 protected:
   nsHTMLScrollFrame(nsStyleContext* aContext, bool aIsRoot)
     : nsHTMLScrollFrame(aContext, kClassID, aIsRoot)
   {}
 
   nsHTMLScrollFrame(nsStyleContext* aContext,
                     nsIFrame::ClassID aID,
                     bool aIsRoot);
@@ -1499,16 +1467,19 @@ public:
   void AppendDirectlyOwnedAnonBoxes(nsTArray<OwnedAnonBox>& aResult) override {
     aResult.AppendElement(OwnedAnonBox(mHelper.GetScrolledFrame()));
   }
 
 #ifdef DEBUG_FRAME_DUMP
   virtual nsresult GetFrameName(nsAString& aResult) const override;
 #endif
 
+  virtual void FireScrollEvent() override {
+    mHelper.FireScrollEvent();
+  }
 protected:
   nsXULScrollFrame(nsStyleContext* aContext, bool aIsRoot,
                    bool aClipAllDescendants);
 
   void ClampAndSetBounds(nsBoxLayoutState& aState,
                          nsRect& aRect,
                          nsPoint aScrollPosition,
                          bool aRemoveOverflowAreas = false) {
--- a/layout/generic/nsIScrollableFrame.h
+++ b/layout/generic/nsIScrollableFrame.h
@@ -25,16 +25,17 @@
 class gfxContext;
 class nsBoxLayoutState;
 class nsIScrollPositionListener;
 class nsIFrame;
 class nsPresContext;
 class nsIContent;
 class nsIAtom;
 class nsDisplayListBuilder;
+class nsRefreshDriver;
 
 namespace mozilla {
 struct ContainerLayerParameters;
 namespace layers {
 class Layer;
 } // namespace layers
 } // namespace mozilla
 
@@ -487,11 +488,44 @@ public:
   virtual void AsyncScrollbarDragRejected() = 0;
 
   /**
    * Returns whether this scroll frame is the root scroll frame of the document
    * that it is in. Note that some documents don't have root scroll frames at
    * all (ie XUL documents) even though they may contain other scroll frames.
    */
   virtual bool IsRootScrollFrameOfDocument() const = 0;
+
+  /**
+   * This class handles the dispatching of scroll events to content.
+   * 
+   * Scroll events are posted to the refresh driver via 
+   * nsRefreshDriver::PostScrollEvent(), and they are fired during a refresh
+   * driver tick, after running requestAnimationFrame callbacks but before
+   * the style flush. This allows rAF callbacks to perform scrolling and have
+   * that scrolling be reflected on the same refresh driver tick, while at
+   * the same time allowing scroll event listeners to make style changes and
+   * have those style changes be reflected on the same refresh driver tick.
+   * 
+   * ScrollEvents cannot be refresh observers, because none of the existing
+   * categories of refresh observers (FlushType::Style, FlushType::Layout,
+   * and FlushType::Display) are run at the desired time in a refresh driver
+   * tick. Otherwise they behave similarly to refresh observers (e.g. they 
+   * cause the refresh driver to tick while it has registered scroll events).
+   * 
+   * ScrollEvents remove themselves from the refresh driver when they fire.
+   */
+  class ScrollEvent {
+  public:
+    NS_INLINE_DECL_REFCOUNTING(ScrollEvent)
+    explicit ScrollEvent(nsIScrollableFrame* aScrollableFrame);
+    void Fire();
+  protected:
+    virtual ~ScrollEvent();
+  private:
+    nsIScrollableFrame* mScrollableFrame;
+    RefPtr<nsRefreshDriver> mDriver;
+  };
+
+  virtual void FireScrollEvent() = 0;
 };
 
 #endif