Bug 1235899 - Don't allow frame reconstruction to clobber the APZ scroll offset. r?botond draft
authorKartikaya Gupta <kgupta@mozilla.com>
Wed, 10 Feb 2016 15:56:07 -0500
changeset 330160 e6fb9a8498984d7ef89dc1bd96b5ee2eb6367e42
parent 330112 ac39fba33c6daf95b2cda71e588ca18e2eb752ab
child 330161 38d8f1e0fe2789d61c08d043397d22eb1897bc83
push id10692
push userkgupta@mozilla.com
push dateWed, 10 Feb 2016 20:56:24 +0000
reviewersbotond
bugs1235899
milestone47.0a1
Bug 1235899 - Don't allow frame reconstruction to clobber the APZ scroll offset. r?botond MozReview-Commit-ID: 1Px2th3KGoR
dom/base/nsGkAtomList.h
gfx/layers/apz/util/APZCCallbackHelper.cpp
layout/base/nsLayoutUtils.cpp
layout/base/nsLayoutUtils.h
layout/generic/nsGfxScrollFrame.cpp
--- a/dom/base/nsGkAtomList.h
+++ b/dom/base/nsGkAtomList.h
@@ -99,17 +99,16 @@ GK_ATOM(anonid, "anonid")
 GK_ATOM(anonlocation, "anonlocation")
 GK_ATOM(any, "any")
 GK_ATOM(mozapp, "mozapp")
 GK_ATOM(mozwidget, "mozwidget")
 GK_ATOM(applet, "applet")
 GK_ATOM(applyImports, "apply-imports")
 GK_ATOM(applyTemplates, "apply-templates")
 GK_ATOM(mozapptype, "mozapptype")
-GK_ATOM(apz, "apz")
 GK_ATOM(archive, "archive")
 GK_ATOM(area, "area")
 GK_ATOM(arrow, "arrow")
 GK_ATOM(article, "article")
 GK_ATOM(ascending, "ascending")
 GK_ATOM(aside, "aside")
 GK_ATOM(aspectRatio, "aspect-ratio")
 GK_ATOM(assign, "assign")
@@ -2265,23 +2264,30 @@ GK_ATOM(Close, "Close")
 GK_ATOM(Save, "Save")
 GK_ATOM(Find, "Find")
 GK_ATOM(Help, "Help")
 GK_ATOM(Print, "Print")
 GK_ATOM(SendMail, "SendMail")
 GK_ATOM(ForwardMail, "ForwardMail")
 GK_ATOM(ReplyToMail, "ReplyToMail")
 
-// Smooth scroll events origins
+// Scroll origins (these are used in various scrolling functions in
+// nsIScrollableFrame and nsGfxScrollFrame). These are divided into two lists
+// - origins in the first one have smooth-scrolling prefs associated with them,
+// under the "general.smoothScroll.<origin>.*" pref branch. Origins in the
+// second one do not.
 GK_ATOM(mouseWheel, "mouseWheel")  // For discrete wheel events (e.g. not OSX magic mouse)
 GK_ATOM(pixels,     "pixels")
 GK_ATOM(lines,      "lines")
 GK_ATOM(pages,      "pages")
 GK_ATOM(scrollbars, "scrollbars")
 GK_ATOM(other,      "other")
+// Scroll origins without smooth-scrolling prefs
+GK_ATOM(apz,        "apz")
+GK_ATOM(restore,    "restore")
 
 #ifdef ACCESSIBILITY
 GK_ATOM(alert, "alert")
 GK_ATOM(alertdialog, "alertdialog")
 GK_ATOM(application, "application")
 GK_ATOM(aria_activedescendant, "aria-activedescendant")
 GK_ATOM(aria_atomic, "aria-atomic")
 GK_ATOM(aria_autocomplete, "aria-autocomplete")
--- a/gfx/layers/apz/util/APZCCallbackHelper.cpp
+++ b/gfx/layers/apz/util/APZCCallbackHelper.cpp
@@ -87,21 +87,21 @@ ScrollFrameTo(nsIScrollableFrame* aFrame
     targetScrollPosition.y = geckoScrollPosition.y;
   }
   if (aFrame->GetScrollbarStyles().mHorizontal == NS_STYLE_OVERFLOW_HIDDEN) {
     targetScrollPosition.x = geckoScrollPosition.x;
   }
 
   // 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
+  // Also if the scrollable frame got a scroll request from a higher priority origin
   // since the last layers update, then we don't want to push our scroll request
   // because we'll clobber that one, which is bad.
   bool scrollInProgress = aFrame->IsProcessingAsyncScroll()
-      || (aFrame->LastScrollOrigin() && aFrame->LastScrollOrigin() != nsGkAtoms::apz)
+      || nsLayoutUtils::CanScrollOriginClobberApz(aFrame->LastScrollOrigin())
       || aFrame->LastSmoothScrollOrigin();
   if (!scrollInProgress) {
     aFrame->ScrollToCSSPixelsApproximate(targetScrollPosition, nsGkAtoms::apz);
     geckoScrollPosition = CSSPoint::FromAppUnits(aFrame->GetScrollPosition());
     aSuccessOut = true;
   }
   // 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
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -8576,16 +8576,24 @@ nsLayoutUtils::SetScrollPositionClamping
   // layout depends on the size of the screen.  Since when the size
   // of the screen changes, the scroll position clamping scroll port
   // size also changes, we hook in the needed updates here rather
   // than adding a separate notification just for this change.
   nsPresContext* presContext = aPresShell->GetPresContext();
   MaybeReflowForInflationScreenSizeChange(presContext);
 }
 
+/* static */ bool
+nsLayoutUtils::CanScrollOriginClobberApz(nsIAtom* aScrollOrigin)
+{
+  return aScrollOrigin != nullptr
+      && aScrollOrigin != nsGkAtoms::apz
+      && aScrollOrigin != nsGkAtoms::restore;
+}
+
 /* static */ FrameMetrics
 nsLayoutUtils::ComputeFrameMetrics(nsIFrame* aForFrame,
                                    nsIFrame* aScrollFrame,
                                    nsIContent* aContent,
                                    const nsIFrame* aReferenceFrame,
                                    Layer* aLayer,
                                    ViewID aScrollParentId,
                                    const nsRect& aViewport,
@@ -8632,21 +8640,20 @@ nsLayoutUtils::ComputeFrameMetrics(nsIFr
 
   if (scrollableFrame) {
     nsPoint scrollPosition = scrollableFrame->GetScrollPosition();
     metrics.SetScrollOffset(CSSPoint::FromAppUnits(scrollPosition));
 
     nsPoint smoothScrollPosition = scrollableFrame->LastScrollDestination();
     metrics.SetSmoothScrollOffset(CSSPoint::FromAppUnits(smoothScrollPosition));
 
-    // If the frame was scrolled since the last layers update, and by
-    // something other than the APZ code, we want to tell the APZ to update
+    // If the frame was scrolled since the last layers update, and by something
+    // that is higher priority than APZ, we want to tell the APZ to update
     // its scroll offset.
-    nsIAtom* lastScrollOrigin = scrollableFrame->LastScrollOrigin();
-    if (lastScrollOrigin && lastScrollOrigin != nsGkAtoms::apz) {
+    if (CanScrollOriginClobberApz(scrollableFrame->LastScrollOrigin())) {
       metrics.SetScrollOffsetUpdated(scrollableFrame->CurrentScrollGeneration());
     }
     nsIAtom* lastSmoothScrollOrigin = scrollableFrame->LastSmoothScrollOrigin();
     if (lastSmoothScrollOrigin) {
       metrics.SetSmoothScrollOffsetUpdated(scrollableFrame->CurrentScrollGeneration());
     }
 
     nsSize lineScrollAmount = scrollableFrame->GetLineScrollAmount();
--- a/layout/base/nsLayoutUtils.h
+++ b/layout/base/nsLayoutUtils.h
@@ -2717,16 +2717,25 @@ public:
   /**
    * Set the scroll port size for the purpose of clamping the scroll position
    * for the root scroll frame of this document
    * (see nsIDOMWindowUtils.setScrollPositionClampingScrollPortSize).
    */
   static void SetScrollPositionClampingScrollPortSize(nsIPresShell* aPresShell,
                                                       CSSSize aSize);
 
+  /**
+   * Returns true if the given scroll origin is "higher priority" than APZ.
+   * In general any content programmatic scrolls (e.g. scrollTo calls) are
+   * higher priority, and take precedence over APZ scrolling. This function
+   * returns true for those, and returns false for other origins like APZ
+   * itself, or scroll position updates from the history restore code.
+   */
+  static bool CanScrollOriginClobberApz(nsIAtom* aScrollOrigin);
+
   static FrameMetrics ComputeFrameMetrics(nsIFrame* aForFrame,
                                           nsIFrame* aScrollFrame,
                                           nsIContent* aContent,
                                           const nsIFrame* aReferenceFrame,
                                           Layer* aLayer,
                                           ViewID aScrollParentId,
                                           const nsRect& aViewport,
                                           const mozilla::Maybe<nsRect>& aClipRect,
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@@ -1720,17 +1720,20 @@ private:
 
 /*
  * Calculate duration, possibly dynamically according to events rate and event origin.
  * (also maintain previous timestamps - which are only used here).
  */
 void
 ScrollFrameHelper::AsyncScroll::InitPreferences(TimeStamp aTime, nsIAtom *aOrigin)
 {
-  if (!aOrigin){
+  if (!aOrigin || aOrigin == nsGkAtoms::restore) {
+    // We don't have special prefs for "restore", just treat it as "other".
+    // "restore" scrolls are (for now) always instant anyway so unless something
+    // changes we should never have aOrigin == nsGkAtoms::restore here.
     aOrigin = nsGkAtoms::other;
   }
 
   // Read preferences only on first iteration or for a different event origin.
   if (!mIsFirstIteration && aOrigin == mOrigin) {
     return;
   }
 
@@ -3933,17 +3936,18 @@ ScrollFrameHelper::ScrollToRestoredPosit
     // and scroll many times.
     if (mRestorePos != mLastPos /* GetLogicalScrollPosition() */) {
       nsPoint scrollToPos = mRestorePos;
       if (!IsLTR())
         // convert from logical to physical scroll position
         scrollToPos.x = mScrollPort.x -
           (mScrollPort.XMost() - scrollToPos.x - mScrolledFrame->GetRect().width);
       nsWeakFrame weakFrame(mOuter);
-      ScrollTo(scrollToPos, nsIScrollableFrame::INSTANT);
+      ScrollToWithOrigin(scrollToPos, nsIScrollableFrame::INSTANT,
+                         nsGkAtoms::restore, nullptr);
       if (!weakFrame.IsAlive()) {
         return;
       }
       // Re-get the scroll position, it might not be exactly equal to
       // mRestorePos due to rounding and clamping.
       mLastPos = GetLogicalScrollPosition();
     } else {
       // if we reached the position then stop