Bug 1209970 - Fire scroll events early in the refresh tick. r=mats
authorMarkus Stange <mstange@themasta.com>
Thu, 17 Dec 2015 17:19:30 -0500
changeset 276993 ef961a0fe82f1b2938dcaa47f9911f5de0d08109
parent 276992 f7527101ddb1804186c7149c537c98590ab4aed8
child 276994 39214f4fa13439ef3e628c42ee22876ed4bb5716
push id16724
push usercbook@mozilla.com
push dateMon, 21 Dec 2015 11:00:52 +0000
treeherderfx-team@3f3f0361567c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmats
bugs1209970
milestone46.0a1
Bug 1209970 - Fire scroll events early in the refresh tick. r=mats With APZ we want to be firing scroll events to content more consistently, so we tie them to the refresh driver tick rather than firing them on paint or haphazardly on the next spin of the event loop. Patch by Markus Stange, test fixes by Kartikaya Gupta
browser/components/sessionstore/test/browser_scrollPositions.js
layout/generic/nsGfxScrollFrame.cpp
layout/generic/nsGfxScrollFrame.h
testing/mochitest/tests/SimpleTest/EventUtils.js
widget/tests/window_wheeltransaction.xul
--- a/browser/components/sessionstore/test/browser_scrollPositions.js
+++ b/browser/components/sessionstore/test/browser_scrollPositions.js
@@ -11,16 +11,18 @@ const URL_FRAMESET = BASE + "browser_scr
 const SCROLL_X = Math.round(100 * (1 + Math.random()));
 const SCROLL_Y = Math.round(200 * (1 + Math.random()));
 const SCROLL_STR = SCROLL_X + "," + SCROLL_Y;
 
 const SCROLL2_X = Math.round(300 * (1 + Math.random()));
 const SCROLL2_Y = Math.round(400 * (1 + Math.random()));
 const SCROLL2_STR = SCROLL2_X + "," + SCROLL2_Y;
 
+requestLongerTimeout(2);
+
 /**
  * This test ensures that we properly serialize and restore scroll positions
  * for an average page without any frames.
  */
 add_task(function test_scroll() {
   let tab = gBrowser.addTab(URL);
   let browser = tab.linkedBrowser;
   yield promiseBrowserLoaded(browser);
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@@ -4220,28 +4220,38 @@ void ScrollFrameHelper::CurPosAttributeC
   ScrollToWithOrigin(dest,
                      isSmooth ? nsIScrollableFrame::SMOOTH : nsIScrollableFrame::INSTANT,
                      nsGkAtoms::scrollbars, &allowedRange);
   // 'this' might be destroyed here
 }
 
 /* ============= Scroll events ========== */
 
-NS_IMETHODIMP
-ScrollFrameHelper::ScrollEvent::Run()
-{
-  if (mHelper)
-    mHelper->FireScrollEvent();
-  return NS_OK;
+ScrollFrameHelper::ScrollEvent::ScrollEvent(ScrollFrameHelper* aHelper)
+  : mHelper(aHelper)
+{
+  mHelper->mOuter->PresContext()->RefreshDriver()->AddRefreshObserver(this, Flush_Style);
+}
+
+ScrollFrameHelper::ScrollEvent::~ScrollEvent()
+{
+  mHelper->mOuter->PresContext()->RefreshDriver()->RemoveRefreshObserver(this, Flush_Style);
+}
+
+void
+ScrollFrameHelper::ScrollEvent::WillRefresh(mozilla::TimeStamp aTime)
+{
+  mHelper->FireScrollEvent();
 }
 
 void
 ScrollFrameHelper::FireScrollEvent()
 {
-  mScrollEvent.Forget();
+  MOZ_ASSERT(mScrollEvent);
+  mScrollEvent = nullptr;
 
   ActiveLayerTracker::SetCurrentScrollHandlerFrame(mOuter);
   WidgetGUIEvent event(true, eScroll, nullptr);
   nsEventStatus status = nsEventStatus_eIgnore;
   nsIContent* content = mOuter->GetContent();
   nsPresContext* prescontext = mOuter->PresContext();
   // Fire viewport scroll events at the document (where they
   // will bubble to the window)
@@ -4258,24 +4268,21 @@ ScrollFrameHelper::FireScrollEvent()
     EventDispatcher::Dispatch(content, prescontext, &event, nullptr, &status);
   }
   ActiveLayerTracker::SetCurrentScrollHandlerFrame(nullptr);
 }
 
 void
 ScrollFrameHelper::PostScrollEvent()
 {
-  if (mScrollEvent.IsPending())
+  if (mScrollEvent)
     return;
 
-  nsRootPresContext* rpc = mOuter->PresContext()->GetRootPresContext();
-  if (!rpc)
-    return;
+  // The ScrollEvent constructor registers itself with the refresh driver.
   mScrollEvent = new ScrollEvent(this);
-  rpc->AddWillPaintObserver(mScrollEvent.get());
 }
 
 NS_IMETHODIMP
 ScrollFrameHelper::AsyncScrollPortEvent::Run()
 {
   if (mHelper) {
     mHelper->mOuter->PresContext()->GetPresShell()->
       FlushPendingNotifications(Flush_InterruptibleLayout);
--- a/layout/generic/nsGfxScrollFrame.h
+++ b/layout/generic/nsGfxScrollFrame.h
@@ -14,16 +14,17 @@
 #include "nsBoxFrame.h"
 #include "nsIScrollableFrame.h"
 #include "nsIScrollbarMediator.h"
 #include "nsIStatefulFrame.h"
 #include "nsThreadUtils.h"
 #include "nsIReflowCallback.h"
 #include "nsBoxLayoutState.h"
 #include "nsQueryFrame.h"
+#include "nsRefreshDriver.h"
 #include "nsExpirationTracker.h"
 #include "TextOverflow.h"
 #include "ScrollVelocityQueue.h"
 
 class nsPresContext;
 class nsIPresShell;
 class nsIContent;
 class nsIAtom;
@@ -96,21 +97,23 @@ public:
 
   void PostScrollEvent();
   void FireScrollEvent();
   void PostScrolledAreaEvent();
   void FireScrolledAreaEvent();
 
   bool IsSmoothScrollingEnabled();
 
-  class ScrollEvent : public nsRunnable {
+  class ScrollEvent : public nsARefreshObserver {
   public:
-    NS_DECL_NSIRUNNABLE
-    explicit ScrollEvent(ScrollFrameHelper *helper) : mHelper(helper) {}
-    void Revoke() { mHelper = nullptr; }
+    NS_INLINE_DECL_REFCOUNTING(ScrollEvent, override)
+    explicit ScrollEvent(ScrollFrameHelper *helper);
+    void WillRefresh(mozilla::TimeStamp aTime) override;
+  protected:
+    virtual ~ScrollEvent();
   private:
     ScrollFrameHelper *mHelper;
   };
 
   class AsyncScrollPortEvent : public nsRunnable {
   public:
     NS_DECL_NSIRUNNABLE
     explicit AsyncScrollPortEvent(ScrollFrameHelper *helper) : mHelper(helper) {}
@@ -412,17 +415,17 @@ public:
                       = nsIScrollbarMediator::DISABLE_SNAP);
 
   // owning references to the nsIAnonymousContentCreator-built content
   nsCOMPtr<nsIContent> mHScrollbarContent;
   nsCOMPtr<nsIContent> mVScrollbarContent;
   nsCOMPtr<nsIContent> mScrollCornerContent;
   nsCOMPtr<nsIContent> mResizerContent;
 
-  nsRevocableEventPtr<ScrollEvent> mScrollEvent;
+  RefPtr<ScrollEvent> mScrollEvent;
   nsRevocableEventPtr<AsyncScrollPortEvent> mAsyncScrollPortEvent;
   nsRevocableEventPtr<ScrolledAreaEvent> mScrolledAreaEvent;
   nsIFrame* mHScrollbarBox;
   nsIFrame* mVScrollbarBox;
   nsIFrame* mScrolledFrame;
   nsIFrame* mScrollCornerBox;
   nsIFrame* mResizerBox;
   nsContainerFrame* mOuter;
--- a/testing/mochitest/tests/SimpleTest/EventUtils.js
+++ b/testing/mochitest/tests/SimpleTest/EventUtils.js
@@ -521,18 +521,20 @@ function sendWheelAndPaint(aTarget, aOff
   var onwheel = function() {
     window.removeEventListener("wheel", onwheel);
 
     // Wait one frame since the wheel event has not caused a refresh observer
     // to be added yet.
     setTimeout(function() {
       utils.advanceTimeAndRefresh(1000);
 
-      if (!aCallback)
+      if (!aCallback) {
+        utils.advanceTimeAndRefresh(0);
         return;
+      }
 
       var waitForPaints = function () {
         SpecialPowers.Services.obs.removeObserver(waitForPaints, "apz-repaints-flushed", false);
         aWindow.waitForAllPaintsFlushed(function() {
           utils.restoreNormalRefresh();
           aCallback();
         });
       }
--- a/widget/tests/window_wheeltransaction.xul
+++ b/widget/tests/window_wheeltransaction.xul
@@ -1029,16 +1029,17 @@ function initElements()
   gSubView1.style.display = kDisplay;
   gSubView2.style.display = kDisplay;
   gSubView3.style.display = kDisplay;
 
   resetScrollPosition(gRootView);
   resetScrollPosition(gSubView1);
   resetScrollPosition(gSubView2);
   resetScrollPosition(gSubView3);
+  _getDOMWindowUtils(window).advanceTimeAndRefresh(0);
 
   runNextTestStep();
 }
 
 function clearWheelTransaction()
 {
   _clearTimer();
   _clearTransaction();