Bug 1509575 - Extend the internal visual scroll API to allow specifying "restore" vs. regular priority. r=kats
authorBotond Ballo <botond@mozilla.com>
Tue, 15 Jan 2019 01:30:43 +0000
changeset 453884 52501b57785575d3e894f9e1cb88f2d7a246e011
parent 453883 ecba81cf5e7485ae6acd4974a3cbe7ee15dd093c
child 453885 eba0c068601df32e0f8c5ec19d871bcfc60c68fb
push id35378
push userbtara@mozilla.com
push dateTue, 15 Jan 2019 16:08:45 +0000
treeherdermozilla-central@5090d461e169 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskats
bugs1509575
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 1509575 - Extend the internal visual scroll API to allow specifying "restore" vs. regular priority. r=kats The distinction is not exposed at the JS level which currently always uses "restore", but it could be if necessary. Differential Revision: https://phabricator.services.mozilla.com/D16346
dom/base/nsDOMWindowUtils.cpp
gfx/layers/apz/src/AsyncPanZoomController.cpp
layout/base/nsIPresShell.h
layout/base/nsLayoutUtils.cpp
--- a/dom/base/nsDOMWindowUtils.cpp
+++ b/dom/base/nsDOMWindowUtils.cpp
@@ -1401,18 +1401,22 @@ nsDOMWindowUtils::ScrollToVisual(float a
   NS_ENSURE_STATE(doc);
 
   nsPresContext* presContext = doc->GetPresContext();
   NS_ENSURE_TRUE(presContext, NS_ERROR_NOT_AVAILABLE);
 
   // This should only be called on the root content document.
   NS_ENSURE_TRUE(presContext->IsRootContentDocument(), NS_ERROR_INVALID_ARG);
 
-  presContext->PresShell()->SetPendingVisualViewportOffset(
-      Some(CSSPoint::ToAppUnits(CSSPoint(aOffsetX, aOffsetY))));
+  // Use |eRestore| as the priority for now, as it's the conservative choice.
+  // If a JS call site needs higher priority, we can expose the update type
+  // as a parameter.
+  presContext->PresShell()->SetPendingVisualScrollUpdate(
+      CSSPoint::ToAppUnits(CSSPoint(aOffsetX, aOffsetY)),
+      FrameMetrics::eRestore);
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDOMWindowUtils::GetVisualViewportOffsetRelativeToLayoutViewport(
     float* aOffsetX, float* aOffsetY) {
   *aOffsetX = 0;
--- a/gfx/layers/apz/src/AsyncPanZoomController.cpp
+++ b/gfx/layers/apz/src/AsyncPanZoomController.cpp
@@ -4257,28 +4257,44 @@ void AsyncPanZoomController::NotifyLayer
   // we want to take the new scroll offset. Check the scroll generation as well
   // to filter duplicate calls to NotifyLayersUpdated with the same scroll
   // offset update message.
   bool scrollOffsetUpdated =
       aLayerMetrics.GetScrollOffsetUpdated() &&
       (aLayerMetrics.GetScrollGeneration() != Metrics().GetScrollGeneration());
 
   if (scrollOffsetUpdated && userScrolled &&
-      aLayerMetrics.GetScrollUpdateType() ==
-          FrameMetrics::ScrollOffsetUpdateType::eRestore) {
+      aLayerMetrics.GetScrollUpdateType() == FrameMetrics::eRestore) {
     APZC_LOG(
         "%p dropping scroll update of type eRestore because of user scroll\n",
         this);
     scrollOffsetUpdated = false;
   }
 
   bool smoothScrollRequested =
       aLayerMetrics.GetDoSmoothScroll() &&
       (aLayerMetrics.GetScrollGeneration() != Metrics().GetScrollGeneration());
 
+  // The main thread may also ask us to scroll the visual viewport to a
+  // particular location. This is different from a layout viewport offset update
+  // in that the layout viewport offset is limited to the layout scroll range
+  // (this will be enforced by the main thread once bug 1516056 is fixed),
+  // while the visual viewport offset is not.
+  // The update type indicates the priority; an eMainThread layout update (or
+  // a smooth scroll request which is similar) takes precedence over an eRestore
+  // visual update, but we allow the possibility for the main thread to ask us
+  // to scroll both the layout and visual viewports to distinct (but compatible)
+  // locations (via e.g. both updates being eRestore).
+  bool visualScrollOffsetUpdated =
+      aLayerMetrics.GetVisualScrollUpdateType() != FrameMetrics::eNone;
+  if (aLayerMetrics.GetScrollUpdateType() == FrameMetrics::eMainThread ||
+      smoothScrollRequested) {
+    visualScrollOffsetUpdated = false;
+  }
+
   // TODO if we're in a drag and scrollOffsetUpdated is set then we want to
   // ignore it
 
   bool needContentRepaint = false;
   RepaintUpdateType contentRepaintType = RepaintUpdateType::eNone;
   bool viewportUpdated = false;
 
   // We usually don't entertain viewport updates on the same transaction as
@@ -4495,28 +4511,17 @@ void AsyncPanZoomController::NotifyLayer
       Metrics().ApplySmoothScrollUpdateFrom(aLayerMetrics);
     }
     needContentRepaint = true;
     mExpectedGeckoMetrics = aLayerMetrics;
 
     SmoothScrollTo(Metrics().GetSmoothScrollOffset());
   }
 
-  // If the main thread asked us to scroll the visual viewport to a particular
-  // location, do so. This is different from a layout viewport offset update
-  // in that the layout viewport offset is limited to the layout scroll range
-  // (this will be enforced by the main thread once bug 1516056 is fixed),
-  // while the visual viewport offset is not. The main thread can also ask
-  // us to scroll both the layout and visual viewports to distinct (but
-  // compatible) locations.
-  FrameMetrics::ScrollOffsetUpdateType visualUpdateType =
-      aLayerMetrics.GetVisualScrollUpdateType();
-  MOZ_ASSERT(visualUpdateType == FrameMetrics::eNone ||
-             visualUpdateType == FrameMetrics::eMainThread);
-  if (visualUpdateType == FrameMetrics::eMainThread) {
+  if (visualScrollOffsetUpdated) {
     Metrics().ClampAndSetScrollOffset(aLayerMetrics.GetVisualViewportOffset());
 
     // The rest of this branch largely follows the code in the
     // |if (scrollOffsetUpdated)| branch above.
     Metrics().RecalculateLayoutViewportOffset();
     mCompositedLayoutViewport = Metrics().GetLayoutViewport();
     mCompositedScrollOffset = Metrics().GetScrollOffset();
     mExpectedGeckoMetrics = aLayerMetrics;
--- a/layout/base/nsIPresShell.h
+++ b/layout/base/nsIPresShell.h
@@ -13,16 +13,17 @@
 #include "mozilla/EventForwards.h"
 #include "mozilla/FlushType.h"
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/ServoStyleSet.h"
 #include "mozilla/StaticPtr.h"
 #include "mozilla/StyleSheet.h"
 #include "mozilla/UniquePtr.h"
 #include "mozilla/WeakPtr.h"
+#include "FrameMetrics.h"
 #include "GeckoProfiler.h"
 #include "gfxPoint.h"
 #include "nsTHashtable.h"
 #include "nsHashKeys.h"
 #include "nsISupports.h"
 #include "nsIContent.h"
 #include "nsISelectionController.h"
 #include "nsQueryFrame.h"
@@ -171,16 +172,17 @@ enum nsRectVisibility {
  */
 
 class nsIPresShell : public nsStubDocumentObserver {
  public:
   NS_DECLARE_STATIC_IID_ACCESSOR(NS_IPRESSHELL_IID)
 
  protected:
   typedef mozilla::dom::Document Document;
+  typedef mozilla::layers::FrameMetrics FrameMetrics;
   typedef mozilla::layers::LayerManager LayerManager;
   typedef mozilla::gfx::SourceSurface SourceSurface;
 
   enum eRenderFlag {
     STATE_IGNORING_VIEWPORT_SCROLLING = 0x1,
     STATE_DRAWWINDOW_NOT_FLUSHING = 0x2
   };
   typedef uint8_t RenderFlags;  // for storing the above flags
@@ -1655,28 +1657,42 @@ class nsIPresShell : public nsStubDocume
    */
   bool SetVisualViewportOffset(const nsPoint& aScrollOffset,
                                const nsPoint& aPrevLayoutScrollPos);
 
   nsPoint GetVisualViewportOffset() const { return mVisualViewportOffset; }
 
   nsPoint GetVisualViewportOffsetRelativeToLayoutViewport() const;
 
-  // Ask APZ in the next transaction to scroll to the given visual viewport 
+  // Represents an update to the visual scroll offset that will be sent to APZ.
+  // The update type is used to determine priority compared to other scroll
+  // updates.
+  struct VisualScrollUpdate {
+    nsPoint mVisualScrollOffset;
+    FrameMetrics::ScrollOffsetUpdateType mUpdateType;
+  };
+
+  // Ask APZ in the next transaction to scroll to the given visual viewport
   // offset (relative to the document).
   // Use this sparingly, as it will clobber JS-driven scrolling that happens
   // in the same frame. This is mostly intended to be used in special
   // situations like "first paint" or session restore.
   // Please request APZ review if adding a new call site.
-  void SetPendingVisualViewportOffset(
-      const mozilla::Maybe<nsPoint>& aPendingVisualViewportOffset) {
-    mPendingVisualViewportOffset = aPendingVisualViewportOffset;
+  void SetPendingVisualScrollUpdate(
+      const nsPoint& aVisualViewportOffset,
+      FrameMetrics::ScrollOffsetUpdateType aUpdateType) {
+    mPendingVisualScrollUpdate =
+        mozilla::Some(VisualScrollUpdate{aVisualViewportOffset, aUpdateType});
   }
-  const mozilla::Maybe<nsPoint>& GetPendingVisualViewportOffset() const {
-    return mPendingVisualViewportOffset;
+  void ClearPendingVisualScrollUpdate() {
+    mPendingVisualScrollUpdate = mozilla::Nothing();
+  }
+  const mozilla::Maybe<VisualScrollUpdate>& GetPendingVisualScrollUpdate()
+      const {
+    return mPendingVisualScrollUpdate;
   }
 
   nsPoint GetLayoutViewportOffset() const;
 
   virtual void WindowSizeMoveDone() = 0;
   virtual void SysColorChanged() = 0;
   virtual void ThemeChanged() = 0;
   virtual void BackingScaleFactorChanged() = 0;
@@ -1750,20 +1766,20 @@ class nsIPresShell : public nsStubDocume
 
   // Count of the number of times this presshell has been painted to a window.
   uint64_t mPaintCount;
 
   nsSize mVisualViewportSize;
 
   nsPoint mVisualViewportOffset;
 
-  // A pending visual viewport offset that we will ask APZ to scroll to
+  // A pending visual scroll offset that we will ask APZ to scroll to
   // during the next transaction. Cleared when we send the transaction.
   // Only applicable to the RCD pres shell.
-  mozilla::Maybe<nsPoint> mPendingVisualViewportOffset;
+  mozilla::Maybe<VisualScrollUpdate> mPendingVisualScrollUpdate;
 
   // A list of stack weak frames. This is a pointer to the last item in the
   // list.
   AutoWeakFrame* mAutoWeakFrames;
 
   // A hash table of heap allocated weak frames.
   nsTHashtable<nsPtrHashKey<WeakFrame>> mWeakFrames;
 
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -8703,21 +8703,22 @@ static void MaybeReflowForInflationScree
     CSSPoint scrollPosition =
         CSSPoint::FromAppUnits(scrollableFrame->GetScrollPosition());
     CSSPoint apzScrollPosition =
         CSSPoint::FromAppUnits(scrollableFrame->GetApzScrollPosition());
     metrics.SetScrollOffset(scrollPosition);
     metrics.SetBaseScrollOffset(apzScrollPosition);
 
     if (aIsRootContent) {
-      if (const Maybe<nsPoint>& visualOffset =
-              presShell->GetPendingVisualViewportOffset()) {
-        metrics.SetVisualViewportOffset(CSSPoint::FromAppUnits(*visualOffset));
-        metrics.SetVisualScrollUpdateType(FrameMetrics::eMainThread);
-        presShell->SetPendingVisualViewportOffset(Nothing());
+      if (const Maybe<nsIPresShell::VisualScrollUpdate>& visualUpdate =
+              presShell->GetPendingVisualScrollUpdate()) {
+        metrics.SetVisualViewportOffset(
+            CSSPoint::FromAppUnits(visualUpdate->mVisualScrollOffset));
+        metrics.SetVisualScrollUpdateType(visualUpdate->mUpdateType);
+        presShell->ClearPendingVisualScrollUpdate();
       }
     }
 
     CSSRect viewport = metrics.GetLayoutViewport();
     viewport.MoveTo(scrollPosition);
     metrics.SetLayoutViewport(viewport);
 
     nsPoint smoothScrollPosition = scrollableFrame->LastScrollDestination();