Bug 961280 - Prevent the APZ repaint request from clobbering a layout-driven async scroll. r=tn, a=bajaj
authorKartikaya Gupta <kgupta@mozilla.com>
Tue, 28 Jan 2014 17:54:59 -0500
changeset 169592 7fb6d40e554f2be90a32047142e3b48fd6e1f971
parent 169591 bdce57ad9ec4ec3b5e1aaf9ca628950fb8680b52
child 169593 555697fdea38e02dcc9840f9a73cf3f1cd9929a8
push id5110
push userryanvm@gmail.com
push dateWed, 29 Jan 2014 21:07:10 +0000
treeherdermozilla-aurora@555697fdea38 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstn, bajaj
bugs961280
milestone28.0a2
Bug 961280 - Prevent the APZ repaint request from clobbering a layout-driven async scroll. r=tn, a=bajaj
layout/generic/nsGfxScrollFrame.h
layout/generic/nsIScrollableFrame.h
widget/xpwidgets/APZCCallbackHelper.cpp
--- a/layout/generic/nsGfxScrollFrame.h
+++ b/layout/generic/nsGfxScrollFrame.h
@@ -265,16 +265,17 @@ public:
            (mHasHorizontalScrollbar ? nsIScrollableFrame::HORIZONTAL : 0);
   }
   nsMargin GetActualScrollbarSizes() const;
   nsMargin GetDesiredScrollbarSizes(nsBoxLayoutState* aState);
   nscoord GetNondisappearingScrollbarWidth(nsBoxLayoutState* aState);
   bool IsLTR() const;
   bool IsScrollbarOnRight() const;
   bool IsScrollingActive() const { return mScrollingActive || mShouldBuildScrollableLayer; }
+  bool IsProcessingAsyncScroll() const { return mAsyncScroll != nullptr; }
   void ResetScrollPositionForLayerPixelAlignment()
   {
     mScrollPosForLayerPixelAlignment = GetScrollPosition();
   }
 
   bool UpdateOverflow();
 
   void UpdateSticky();
@@ -631,16 +632,19 @@ public:
   }
   NS_IMETHOD PostScrolledAreaEventForCurrentArea() MOZ_OVERRIDE {
     mHelper.PostScrolledAreaEvent();
     return NS_OK;
   }
   virtual bool IsScrollingActive() MOZ_OVERRIDE {
     return mHelper.IsScrollingActive();
   }
+  virtual bool IsProcessingAsyncScroll() MOZ_OVERRIDE {
+    return mHelper.IsProcessingAsyncScroll();
+  }
   virtual void ResetScrollPositionForLayerPixelAlignment() MOZ_OVERRIDE {
     mHelper.ResetScrollPositionForLayerPixelAlignment();
   }
   virtual bool DidHistoryRestore() MOZ_OVERRIDE {
     return mHelper.mDidHistoryRestore;
   }
   virtual void ClearDidHistoryRestore() MOZ_OVERRIDE {
     mHelper.mDidHistoryRestore = false;
@@ -929,16 +933,19 @@ public:
   }
   NS_IMETHOD PostScrolledAreaEventForCurrentArea() MOZ_OVERRIDE {
     mHelper.PostScrolledAreaEvent();
     return NS_OK;
   }
   virtual bool IsScrollingActive() MOZ_OVERRIDE {
     return mHelper.IsScrollingActive();
   }
+  virtual bool IsProcessingAsyncScroll() MOZ_OVERRIDE {
+    return mHelper.IsProcessingAsyncScroll();
+  }
   virtual void ResetScrollPositionForLayerPixelAlignment() MOZ_OVERRIDE {
     mHelper.ResetScrollPositionForLayerPixelAlignment();
   }
   virtual bool DidHistoryRestore() MOZ_OVERRIDE {
     return mHelper.mDidHistoryRestore;
   }
   virtual void ClearDidHistoryRestore() MOZ_OVERRIDE {
     mHelper.mDidHistoryRestore = false;
--- a/layout/generic/nsIScrollableFrame.h
+++ b/layout/generic/nsIScrollableFrame.h
@@ -246,16 +246,21 @@ public:
 
   /**
    * Returns true if this scrollframe is being "actively scrolled".
    * This basically means that we should allocate resources in the
    * expectation that scrolling is going to happen.
    */
   virtual bool IsScrollingActive() = 0;
   /**
+   * Returns true if the scrollframe is currently processing an async
+   * or smooth scroll.
+   */
+  virtual bool IsProcessingAsyncScroll() = 0;
+  /**
    * Call this when the layer(s) induced by active scrolling are being
    * completely redrawn.
    */
   virtual void ResetScrollPositionForLayerPixelAlignment() = 0;
   /**
    * Was the current presentation state for this frame restored from history?
    */
   virtual bool DidHistoryRestore() = 0;
--- a/widget/xpwidgets/APZCCallbackHelper.cpp
+++ b/widget/xpwidgets/APZCCallbackHelper.cpp
@@ -87,26 +87,29 @@ MaybeAlignAndClampDisplayPort(mozilla::l
 
 static CSSPoint
 ScrollFrameTo(nsIScrollableFrame* aFrame, const CSSPoint& aPoint)
 {
   if (!aFrame) {
     return CSSPoint();
   }
 
-  // If the scrollable frame got a scroll request from something other than us
+  // If the scrollable frame is currently in the middle of an async or smooth
+  // scroll then we don't want to interrupt it (see bug 961280).
+  // Also if the scrollable frame got a scroll request from something other than us
   // since the last layers update, then we don't want to push our scroll request
   // because we'll clobber that one, which is bad.
   // Note that content may have just finished sending a layers update with a scroll
   // offset update to the APZ, in which case the origin will be reset to null and we
   // might actually be clobbering the content-side scroll offset with a stale APZ
   // scroll offset. This is unavoidable because of the async communication between
   // APZ and content; however the code in NotifyLayersUpdated should reissue a new
   // repaint request to bring everything back into sync.
-  if (!aFrame->OriginOfLastScroll() || aFrame->OriginOfLastScroll() == nsGkAtoms::apz) {
+  if (!aFrame->IsProcessingAsyncScroll() &&
+     (!aFrame->OriginOfLastScroll() || aFrame->OriginOfLastScroll() == nsGkAtoms::apz)) {
     aFrame->ScrollToCSSPixelsApproximate(aPoint, nsGkAtoms::apz);
   }
   // Return the final scroll position after setting it so that anything that relies
   // on it can have an accurate value. Note that even if we set it above re-querying it
   // is a good idea because it may have gotten clamped or rounded.
   return CSSPoint::FromAppUnits(aFrame->GetScrollPosition());
 }