Bug 1498812 - Part 11: Use Visual Viewport for storing scroll position in the PresState. r=botond,tnikkel
authorJan Henning <jh+bugzilla@buttercookie.de>
Fri, 11 Jan 2019 19:50:24 +0000
changeset 453555 a99bf382e5f7
parent 453554 f0f5124781cc
child 453556 28f05d905fcf
push id35360
push usernbeleuzu@mozilla.com
push dateSat, 12 Jan 2019 09:39:47 +0000
treeherdermozilla-central@cb35977ae7a4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbotond, tnikkel
bugs1498812
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1498812 - Part 11: Use Visual Viewport for storing scroll position in the PresState. r=botond,tnikkel Differential Revision: https://phabricator.services.mozilla.com/D15691
gfx/layers/apz/util/APZCCallbackHelper.cpp
layout/base/PresState.ipdlh
layout/generic/nsGfxScrollFrame.cpp
layout/generic/nsGfxScrollFrame.h
layout/generic/nsIScrollableFrame.h
mobile/android/tests/browser/chrome/test_session_scroll_visual_viewport.html
--- a/gfx/layers/apz/util/APZCCallbackHelper.cpp
+++ b/gfx/layers/apz/util/APZCCallbackHelper.cpp
@@ -169,19 +169,21 @@ static ScreenMargin ScrollFrame(nsIConte
   // Scroll the window to the desired spot
   nsIScrollableFrame* sf =
       nsLayoutUtils::FindScrollableFrameFor(aRequest.GetScrollId());
   if (sf) {
     sf->ResetScrollInfoIfGeneration(aRequest.GetScrollGeneration());
     sf->SetScrollableByAPZ(!aRequest.IsScrollInfoLayer());
     if (sf->IsRootScrollFrameOfDocument()) {
       if (nsCOMPtr<nsIPresShell> shell = GetPresShell(aContent)) {
-        shell->SetVisualViewportOffset(
-            CSSPoint::ToAppUnits(aRequest.GetScrollOffset()),
-            shell->GetLayoutViewportOffset());
+        if (shell->SetVisualViewportOffset(
+                CSSPoint::ToAppUnits(aRequest.GetScrollOffset()),
+                shell->GetLayoutViewportOffset())) {
+          sf->MarkEverScrolled();
+        }
       }
     }
   }
   bool scrollUpdated = false;
   ScreenMargin displayPortMargins = aRequest.GetDisplayPortMargins();
   CSSPoint apzScrollOffset = aRequest.GetScrollOffset();
   CSSPoint actualScrollOffset = ScrollFrameTo(sf, aRequest, scrollUpdated);
 
--- a/layout/base/PresState.ipdlh
+++ b/layout/base/PresState.ipdlh
@@ -31,16 +31,18 @@ union PresContentData {
   CheckedContentData;
   // We can need to serialize blobs in order to transmit this type, so we need
   // to handle that in a custom handler.
   FileContentData[];
 };
 
 struct PresState {
   PresContentData contentData;
+  // For frames where layout and visual viewport aren't one and the same thing,
+  // scrollState will store the position of the *visual* viewport.
   nsPoint scrollState;
   bool allowScrollOriginDowngrade;
   float resolution;
   bool disabledSet;
   bool disabled;
   bool droppedDown;
 };
 
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@@ -2431,16 +2431,24 @@ static void RemoveDisplayPortCallback(ns
   nsLayoutUtils::RemoveDisplayPort(helper->mOuter->GetContent());
   nsLayoutUtils::ExpireDisplayPortOnAsyncScrollableAncestor(helper->mOuter);
   helper->mOuter->SchedulePaint();
   // Be conservative and unflag this this scrollframe as being scrollable by
   // APZ. If it is still scrollable this will get flipped back soon enough.
   helper->mScrollableByAPZ = false;
 }
 
+void ScrollFrameHelper::MarkEverScrolled() {
+  // Mark this frame as having been scrolled. If this is the root
+  // scroll frame of a content document, then IsAlwaysActive()
+  // will return true from now on and MarkNotRecentlyScrolled() won't
+  // have any effect.
+  mHasBeenScrolled = true;
+}
+
 void ScrollFrameHelper::MarkNotRecentlyScrolled() {
   if (!mHasBeenScrolledRecently) return;
 
   mHasBeenScrolledRecently = false;
   mOuter->SchedulePaint();
 }
 
 void ScrollFrameHelper::MarkRecentlyScrolled() {
@@ -2495,21 +2503,17 @@ void ScrollFrameHelper::TriggerDisplayPo
 
   if (!mDisplayPortExpiryTimer) {
     mDisplayPortExpiryTimer = NS_NewTimer();
   }
   ResetDisplayPortExpiryTimer();
 }
 
 void ScrollFrameHelper::ScrollVisual() {
-  // Mark this frame as having been scrolled. If this is the root
-  // scroll frame of a content document, then IsAlwaysActive()
-  // will return true from now on and MarkNotRecentlyScrolled() won't
-  // have any effect.
-  mHasBeenScrolled = true;
+  MarkEverScrolled();
 
   AdjustViews(mScrolledFrame);
   // We need to call this after fixing up the view positions
   // to be consistent with the frame hierarchy.
   MarkRecentlyScrolled();
 }
 
 /**
@@ -4314,46 +4318,46 @@ void ScrollFrameHelper::ScrollToRestored
   // it. if it does then the user must have moved it, and we no longer
   // need to restore.
   //
   // In the RTL case, we check whether the scroll position changed using the
   // logical scroll position, but we scroll to the physical scroll position in
   // all cases
 
   // if we didn't move, we still need to restore
-  if (GetLogicalScrollPosition() == mLastPos) {
+  if (GetLogicalVisualViewportOffset() == mLastPos) {
     // if our desired position is different to the scroll position, scroll.
     // remember that we could be incrementally loading so we may enter
     // and scroll many times.
-    if (mRestorePos != mLastPos /* GetLogicalScrollPosition() */) {
+    if (mRestorePos != mLastPos /* GetLogicalVisualViewportOffset() */) {
       LoadingState state = GetPageLoadingState();
       if (state == LoadingState::Stopped && !NS_SUBTREE_DIRTY(mOuter)) {
         return;
       }
       nsPoint scrollToPos = mRestorePos;
       if (!IsPhysicalLTR()) {
         // convert from logical to physical scroll position
-        scrollToPos.x = mScrollPort.x - (mScrollPort.XMost() - scrollToPos.x -
-                                         mScrolledFrame->GetRect().width);
+        scrollToPos.x -=
+            (GetVisualViewportSize().width - mScrolledFrame->GetRect().width);
       }
       AutoWeakFrame weakFrame(mOuter);
       // It's very important to pass nsGkAtoms::restore here, so
       // ScrollToWithOrigin won't clear out mRestorePos.
       ScrollToWithOrigin(scrollToPos, nsIScrollableFrame::INSTANT,
                          nsGkAtoms::restore, nullptr);
       if (!weakFrame.IsAlive()) {
         return;
       }
       if (state == LoadingState::Loading || NS_SUBTREE_DIRTY(mOuter)) {
         // If we're trying to do a history scroll restore, then we want to
         // keep trying this until we succeed, because the page can be loading
         // incrementally. So re-get the scroll position for the next iteration,
         // it might not be exactly equal to mRestorePos due to rounding and
         // clamping.
-        mLastPos = GetLogicalScrollPosition();
+        mLastPos = GetLogicalVisualViewportOffset();
         return;
       }
     }
     // If we get here, either we reached the desired position (mLastPos ==
     // mRestorePos) or we're not trying to do a history scroll restore, so
     // we can stop after the scroll attempt above.
     mRestorePos.y = -1;
     mLastPos.x = -1;
@@ -6135,17 +6139,17 @@ UniquePtr<PresState> ScrollFrameHelper::
   // valid and we haven't moved since the last update of mLastPos (same check
   // that ScrollToRestoredPosition uses). This ensures if a reframe occurs
   // while we're in the process of loading content to scroll to a restored
   // position, we'll keep trying after the reframe. Similarly, if we're in the
   // middle of a smooth scroll, store the destination so that when we restore
   // we'll jump straight to the end of the scroll animation, rather than
   // effectively dropping it. Note that the mRestorePos will override the
   // smooth scroll destination if both are present.
-  nsPoint pt = GetLogicalScrollPosition();
+  nsPoint pt = GetLogicalVisualViewportOffset();
   if (isInSmoothScroll) {
     pt = mDestination;
     allowScrollOriginDowngrade = false;
   }
   if (mRestorePos.y != -1 && pt == mLastPos) {
     pt = mRestorePos;
   }
   state->scrollState() = pt;
@@ -6158,17 +6162,17 @@ UniquePtr<PresState> ScrollFrameHelper::
   return state;
 }
 
 void ScrollFrameHelper::RestoreState(PresState* aState) {
   mRestorePos = aState->scrollState();
   MOZ_ASSERT(mLastScrollOrigin == nsGkAtoms::other);
   mAllowScrollOriginDowngrade = aState->allowScrollOriginDowngrade();
   mDidHistoryRestore = true;
-  mLastPos = mScrolledFrame ? GetLogicalScrollPosition() : nsPoint(0, 0);
+  mLastPos = mScrolledFrame ? GetLogicalVisualViewportOffset() : nsPoint(0, 0);
 
   // Resolution properties should only exist on root scroll frames.
   MOZ_ASSERT(mIsRoot || aState->resolution() == 1.0);
 
   if (mIsRoot) {
     nsIPresShell* presShell = mOuter->PresShell();
     presShell->SetResolutionAndScaleTo(aState->resolution(),
                                        nsIPresShell::ChangeOrigin::eMainThread);
--- a/layout/generic/nsGfxScrollFrame.h
+++ b/layout/generic/nsGfxScrollFrame.h
@@ -215,16 +215,28 @@ class ScrollFrameHelper : public nsIRefl
     return pt;
   }
   nsPoint GetApzScrollPosition() const { return mApzScrollPos; }
   nsRect GetScrollRange() const;
   // Get the scroll range assuming the viewport has size (aWidth, aHeight).
   nsRect GetScrollRange(nscoord aWidth, nscoord aHeight) const;
   nsSize GetVisualViewportSize() const;
   nsPoint GetVisualViewportOffset() const;
+  /**
+   * For LTR frames, this is the same as GetVisualViewportOffset().
+   * For RTL frames, we take the offset from the top right corner of the frame
+   * to the top right corner of the visual viewport.
+   */
+  nsPoint GetLogicalVisualViewportOffset() const {
+    nsPoint pt = GetVisualViewportOffset();
+    if (!IsPhysicalLTR()) {
+      pt.x += GetVisualViewportSize().width - mScrolledFrame->GetRect().width;
+    }
+    return pt;
+  }
   void ScrollSnap(
       nsIScrollableFrame::ScrollMode aMode = nsIScrollableFrame::SMOOTH_MSD);
   void ScrollSnap(
       const nsPoint& aDestination,
       nsIScrollableFrame::ScrollMode aMode = nsIScrollableFrame::SMOOTH_MSD);
 
  protected:
   nsRect GetScrollRangeForClamping() const;
@@ -405,16 +417,17 @@ class ScrollFrameHelper : public nsIRefl
   void LayoutScrollbars(nsBoxLayoutState& aState, const nsRect& aContentArea,
                         const nsRect& aOldScrollArea);
 
   void MarkScrollbarsDirtyForReflow() const;
 
   bool ShouldClampScrollPosition() const;
 
   bool IsAlwaysActive() const;
+  void MarkEverScrolled();
   void MarkRecentlyScrolled();
   void MarkNotRecentlyScrolled();
   nsExpirationState* GetExpirationState() { return &mActivityExpirationState; }
 
   void SetTransformingByAPZ(bool aTransforming) {
     if (mTransformingByAPZ && !aTransforming) {
       PostScrollEndEvent();
     }
@@ -536,20 +549,25 @@ class ScrollFrameHelper : public nsIRefl
   // just the current scroll position. ScrollBy will choose its
   // destination based on this value.
   nsPoint mDestination;
 
   // A goal position to try to scroll to as content loads. As long as mLastPos
   // matches the current logical scroll position, we try to scroll to
   // mRestorePos after every reflow --- because after each time content is
   // loaded/added to the scrollable element, there will be a reflow.
+  // Note that for frames where layout and visual viewport aren't one and the
+  // same thing, this scroll position will be the logical scroll position of
+  // the *visual* viewport, as its position will be more relevant to the user.
   nsPoint mRestorePos;
   // The last logical position we scrolled to while trying to restore
   // mRestorePos, or 0,0 when this is a new frame. Set to -1,-1 once we've
   // scrolled for any reason other than trying to restore mRestorePos.
+  // Just as with mRestorePos, this position will be the logical position of
+  // the *visual* viewport where available.
   nsPoint mLastPos;
 
   // The latest scroll position we've sent or received from APZ. This
   // represents the main thread's best knowledge of the APZ scroll position,
   // and is used to calculate relative scroll offset updates.
   nsPoint mApzScrollPos;
 
   nsExpirationState mActivityExpirationState;
@@ -961,16 +979,17 @@ class nsHTMLScrollFrame : public nsConta
     mHelper.ResetScrollPositionForLayerPixelAlignment();
   }
   virtual bool DidHistoryRestore() const override {
     return mHelper.mDidHistoryRestore;
   }
   virtual void ClearDidHistoryRestore() override {
     mHelper.mDidHistoryRestore = false;
   }
+  virtual void MarkEverScrolled() override { mHelper.MarkEverScrolled(); }
   virtual bool IsRectNearlyVisible(const nsRect& aRect) override {
     return mHelper.IsRectNearlyVisible(aRect);
   }
   virtual nsRect ExpandRectToNearlyVisible(const nsRect& aRect) const override {
     return mHelper.ExpandRectToNearlyVisible(aRect);
   }
   virtual nsAtom* LastScrollOrigin() override {
     return mHelper.LastScrollOrigin();
@@ -1428,16 +1447,17 @@ class nsXULScrollFrame final : public ns
     mHelper.ResetScrollPositionForLayerPixelAlignment();
   }
   virtual bool DidHistoryRestore() const override {
     return mHelper.mDidHistoryRestore;
   }
   virtual void ClearDidHistoryRestore() override {
     mHelper.mDidHistoryRestore = false;
   }
+  virtual void MarkEverScrolled() override { mHelper.MarkEverScrolled(); }
   virtual bool IsRectNearlyVisible(const nsRect& aRect) override {
     return mHelper.IsRectNearlyVisible(aRect);
   }
   virtual nsRect ExpandRectToNearlyVisible(const nsRect& aRect) const override {
     return mHelper.ExpandRectToNearlyVisible(aRect);
   }
   virtual nsAtom* LastScrollOrigin() override {
     return mHelper.LastScrollOrigin();
--- a/layout/generic/nsIScrollableFrame.h
+++ b/layout/generic/nsIScrollableFrame.h
@@ -371,16 +371,21 @@ class nsIScrollableFrame : public nsIScr
   virtual bool DidHistoryRestore() const = 0;
   /**
    * Clear the flag so that DidHistoryRestore() returns false until the next
    * RestoreState call.
    * @see nsIStatefulFrame::RestoreState
    */
   virtual void ClearDidHistoryRestore() = 0;
   /**
+   * Mark the frame as having been scrolled at least once, so that it remains
+   * active and we can also start storing its scroll position when saving state.
+   */
+  virtual void MarkEverScrolled() = 0;
+  /**
    * Determine if the passed in rect is nearly visible according to the frame
    * visibility heuristics for how close it is to the visible scrollport.
    */
   virtual bool IsRectNearlyVisible(const nsRect& aRect) = 0;
   /**
    * Expand the given rect taking into account which directions we can scroll
    * and how far we want to expand for frame visibility purposes.
    */
--- a/mobile/android/tests/browser/chrome/test_session_scroll_visual_viewport.html
+++ b/mobile/android/tests/browser/chrome/test_session_scroll_visual_viewport.html
@@ -133,38 +133,50 @@ https://bugzilla.mozilla.org/show_bug.cg
     is(scrolldata.scroll, getScrollString(scrollPos2), "stored scroll position is correct");
     ok(fuzzyEquals(scrolldata.zoom.resolution, scrollPos2.zoom), "stored zoom level is correct");
 
     // ... and the presState from the previous history entry.
     let {index} = tabData;
     index -= 1; // session history uses 1-based index
     let {entries} = tabData;
     let prevPage = entries[index - 1];
-    todo(prevPage.presState, "presState exists");
+    ok(prevPage.presState, "presState exists");
     if (prevPage.presState) {
       let presState = prevPage.presState[0];
       // The presState operates in app units, while all other scroll positions
       // in JS-land use CSS pixels.
       presState = presStateToCSSPx(presState);
-      todo_is(presState.scroll, getScrollString(scrollPos1), "stored scroll position for previous page is correct");
+      is(presState.scroll, getScrollString(scrollPos1), "stored scroll position for previous page is correct");
       ok(fuzzyEquals(presState.res, scrollPos1.zoom), "stored zoom level for previous page is correct");
     }
 
     // Restore the closed tab.
     let browser = ss.undoCloseTab(chromeWin, tabData);
     tabScroll = BrowserApp.getTabForBrowser(browser);
     let pageshow = promiseBrowserEvent(browser, "pageshow");
     let scroll = promiseBrowserEvent(browser, "mozvisualscroll",
                                      { mozSystemGroup: true });
     await pageshow;
     await scroll;
 
     // Check the scroll position and zoom level.
     checkScroll(browser, scrollPos2);
 
+    // Now go back in history and check that the scroll position
+    // is restored there as well.
+    is(browser.canGoBack, true, "can go back");
+    pageshow = promiseBrowserEvent(browser, "pageshow");
+    scroll = promiseBrowserEvent(browser, "mozvisualscroll",
+                                 { mozSystemGroup: true });
+    browser.goBack();
+    await pageshow;
+    await scroll;
+
+    checkScroll(browser, scrollPos1);
+
     // Remove the tab.
     cleanupTabs();
   });
 
   </script>
 </head>
 <body>
 <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1498812">Mozilla Bug 1498812</a>