Bug 1228407 - Avoid a scenario where content floods APZ with smooth-scroll requests and blocks it from syncing a new scroll offset back to the main thread. r=botond
authorKartikaya Gupta <kgupta@mozilla.com>
Tue, 08 Dec 2015 14:56:26 -0500
changeset 297586 3ddca5b65d9bd549770cb78806bc70dade3d127a
parent 297585 cc52b64c6eeeda808d0c27b9430ade9d50abb636
child 297587 32df7136da7571aa3fbd18aee70f4923fcf0403d
push id8824
push userraliiev@mozilla.com
push dateMon, 14 Dec 2015 20:18:56 +0000
treeherdermozilla-aurora@e2031358e2a6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
Bug 1228407 - Avoid a scenario where content floods APZ with smooth-scroll requests and blocks it from syncing a new scroll offset back to the main thread. r=botond
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@@ -2116,20 +2116,40 @@ ScrollFrameHelper::ScrollToWithOrigin(ns
         if (mAsyncScroll) {
           if (mAsyncScroll->mIsSmoothScroll) {
             currentVelocity = mAsyncScroll->VelocityAt(now);
           mAsyncScroll = nullptr;
         if (nsLayoutUtils::AsyncPanZoomEnabled(mOuter)) {
+          if (mApzSmoothScrollDestination == mDestination &&
+              mScrollGeneration == sScrollGenerationCounter) {
+            // If we already sent APZ a smooth-scroll request to this
+            // destination with this generation (i.e. it was the last request
+            // we sent), then don't send another one because it is redundant.
+            // This is to avoid a scenario where pages do repeated scrollBy
+            // calls, incrementing the generation counter, and blocking APZ from
+            // syncing the scroll offset back to the main thread.
+            // Note that if we get two smooth-scroll requests to the same
+            // destination with some other scroll in between, mDestination will
+            // get reset and so we shouldn't have the problem where this check
+            // discards a legitimate smooth-scroll.
+            // Note: if there are two separate scrollframes both getting smooth
+            // scrolled at the same time, sScrollGenerationCounter can get
+            // incremented and this early-exit won't get taken. Bug 1231177 is
+            // on file for this.
+            return;
+          }
           // The animation will be handled in the compositor, pass the
           // information needed to start the animation and skip the main-thread
           // animation for this scroll.
           mLastSmoothScrollOrigin = aOrigin;
+          mApzSmoothScrollDestination = mDestination;
           mScrollGeneration = ++sScrollGenerationCounter;
           if (!nsLayoutUtils::GetDisplayPort(mOuter->GetContent())) {
             // If this frame doesn't have a displayport then there won't be an
             // APZC instance for it and so there won't be anything to process
             // this smooth scroll request. We should set a displayport on this
             // frame to force an APZC which can handle the request.
--- a/layout/generic/nsGfxScrollFrame.h
+++ b/layout/generic/nsGfxScrollFrame.h
@@ -427,16 +427,17 @@ public:
   nsIFrame* mResizerBox;
   nsContainerFrame* mOuter;
   RefPtr<AsyncScroll> mAsyncScroll;
   RefPtr<AsyncSmoothMSDScroll> mAsyncSmoothMSDScroll;
   RefPtr<ScrollbarActivity> mScrollbarActivity;
   nsTArray<nsIScrollPositionListener*> mListeners;
   nsIAtom* mLastScrollOrigin;
   nsIAtom* mLastSmoothScrollOrigin;
+  nsPoint mApzSmoothScrollDestination;
   uint32_t mScrollGeneration;
   nsRect mScrollPort;
   // Where we're currently scrolling to, if we're scrolling asynchronously.
   // If we're not in the middle of an asynchronous scroll then this is
   // just the current scroll position. ScrollBy will choose its
   // destination based on this value.
   nsPoint mDestination;
   nsPoint mScrollPosAtLastPaint;