Bug 1583534 - Further simplify PresShell::ResizeReflow. r=botond
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 25 Sep 2019 19:12:44 +0000
changeset 494966 a7180808eaaeb815467d7794477f69a71b62a93b
parent 494965 81c77136eb578e19f946df1660e2ab2d3010df02
child 494967 a0c6e1a16a2de01d775d815c784f38b5870069f7
push id114131
push userdluca@mozilla.com
push dateThu, 26 Sep 2019 09:47:34 +0000
treeherdermozilla-inbound@1dc1a755079a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbotond
bugs1583534, 1528052
milestone71.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 1583534 - Further simplify PresShell::ResizeReflow. r=botond In particular, not let ResizeReflow take the old and new size. Most of the callers pass dummy values anyway. Instead, use the old size of the layout viewport. This ensures we fire resize events only if the layout viewport actually changes. This is important because the first resize of the mobile viewport manager after a navigation has an "old size" of 0x0, even though the layout viewport is initialized on presshell initialization to the right size. Thus, we fire resize events unnecessarily in that case, which is the root cause for bug 1528052. To do this, we need to shuffle a bit of code in nsDocumentViewer that deals with delayed resizes, to set the visible area _and_ invalidate layout, rather than setting the visible area and _then_ relying on doing a resize reflow. Further cleanup is possible, though not required for my android resizing fix, so will do separately. Differential Revision: https://phabricator.services.mozilla.com/D46944
dom/base/nsGlobalWindowOuter.cpp
gfx/layers/apz/test/gtest/mvm/TestMobileViewportManager.cpp
layout/base/GeckoMVMContext.cpp
layout/base/GeckoMVMContext.h
layout/base/MVMContext.h
layout/base/MobileViewportManager.cpp
layout/base/PresShell.cpp
layout/base/PresShell.h
layout/base/PresShellForwards.h
layout/base/nsDocumentViewer.cpp
view/nsViewManager.cpp
view/nsViewManager.h
--- a/dom/base/nsGlobalWindowOuter.cpp
+++ b/dom/base/nsGlobalWindowOuter.cpp
@@ -3847,16 +3847,21 @@ nsresult nsGlobalWindowOuter::SetDocShel
 void nsGlobalWindowOuter::SetCSSViewportWidthAndHeight(nscoord aInnerWidth,
                                                        nscoord aInnerHeight) {
   RefPtr<nsPresContext> presContext = mDocShell->GetPresContext();
 
   nsRect shellArea = presContext->GetVisibleArea();
   shellArea.SetHeight(aInnerHeight);
   shellArea.SetWidth(aInnerWidth);
 
+  // FIXME(emilio): This doesn't seem to be ok, this doesn't reflow or
+  // anything... Should go through PresShell::ResizeReflow.
+  //
+  // But I don't think this can be reached by content, as we don't allow to set
+  // inner{Width,Height}.
   presContext->SetVisibleArea(shellArea);
 }
 
 // NOTE: Arguments to this function should have values scaled to
 // CSS pixels, not device pixels.
 void nsGlobalWindowOuter::CheckSecurityLeftAndTop(int32_t* aLeft, int32_t* aTop,
                                                   CallerType aCallerType) {
   // This one is harder. We have to get the screen size and window dimensions.
--- a/gfx/layers/apz/test/gtest/mvm/TestMobileViewportManager.cpp
+++ b/gfx/layers/apz/test/gtest/mvm/TestMobileViewportManager.cpp
@@ -77,18 +77,17 @@ class MockMVMContext : public MVMContext
   bool IsInReaderMode() const { return false; }
   bool IsDocumentLoading() const { return false; }
 
   void SetResolutionAndScaleTo(float aResolution,
                                ResolutionChangeOrigin aOrigin) {
     mResolution = aResolution;
     mMVM->ResolutionUpdated(aOrigin);
   }
-  void Reflow(const CSSSize& aNewSize, const CSSSize& aOldSize,
-              ResizeEventFlag aResizeEventFlag) {
+  void Reflow(const CSSSize& aNewSize, ResizeEventFlag) {
     mICBSize = aNewSize;
     mContentSize = mLayoutFunction(mICBSize);
   }
 
   // Allow test code to modify the input metrics.
   void SetMinScale(CSSToScreenScale aMinScale) { mMinScale = aMinScale; }
   void SetMaxScale(CSSToScreenScale aMaxScale) { mMaxScale = aMaxScale; }
   void SetInitialScale(CSSToScreenScale aInitialScale) {
--- a/layout/base/GeckoMVMContext.cpp
+++ b/layout/base/GeckoMVMContext.cpp
@@ -171,26 +171,25 @@ void GeckoMVMContext::UpdateDisplayPortM
     nsLayoutUtils::SetDisplayPortBaseIfNotSet(root->GetContent(),
                                               displayportBase);
     nsIScrollableFrame* scrollable = do_QueryFrame(root);
     nsLayoutUtils::CalculateAndSetDisplayPortMargins(
         scrollable, nsLayoutUtils::RepaintMode::Repaint);
   }
 }
 
-void GeckoMVMContext::Reflow(const CSSSize& aNewSize, const CSSSize& aOldSize,
+void GeckoMVMContext::Reflow(const CSSSize& aNewSize,
                              ResizeEventFlag aResizeEventFlag) {
   MOZ_ASSERT(mPresShell);
 
   ResizeReflowOptions reflowOptions = ResizeReflowOptions::NoOption;
   if (aResizeEventFlag == ResizeEventFlag::Suppress) {
     reflowOptions |= ResizeReflowOptions::SuppressResizeEvent;
   }
 
   RefPtr<PresShell> presShell = mPresShell;
   presShell->ResizeReflowIgnoreOverride(
       nsPresContext::CSSPixelsToAppUnits(aNewSize.width),
       nsPresContext::CSSPixelsToAppUnits(aNewSize.height),
-      nsPresContext::CSSPixelsToAppUnits(aOldSize.width),
-      nsPresContext::CSSPixelsToAppUnits(aOldSize.height), reflowOptions);
+      reflowOptions);
 }
 
 }  // namespace mozilla
--- a/layout/base/GeckoMVMContext.h
+++ b/layout/base/GeckoMVMContext.h
@@ -50,18 +50,17 @@ class GeckoMVMContext : public MVMContex
   bool IsInReaderMode() const override;
   bool IsDocumentLoading() const override;
 
   void SetResolutionAndScaleTo(float aResolution,
                                ResolutionChangeOrigin aOrigin) override;
   void SetVisualViewportSize(const CSSSize& aSize) override;
   void UpdateDisplayPortMargins() override;
   MOZ_CAN_RUN_SCRIPT_BOUNDARY
-  void Reflow(const CSSSize& aNewSize, const CSSSize& aOldSize,
-              ResizeEventFlag aResizeEventFlag) override;
+  void Reflow(const CSSSize& aNewSize, ResizeEventFlag) override;
 
  private:
   RefPtr<dom::Document> mDocument;
   // raw ref since the presShell owns this
   PresShell* MOZ_NON_OWNING_REF mPresShell;
   nsCOMPtr<dom::EventTarget> mEventTarget;
 };
 
--- a/layout/base/MVMContext.h
+++ b/layout/base/MVMContext.h
@@ -58,15 +58,14 @@ class MVMContext {
                                        ResolutionChangeOrigin aOrigin) = 0;
   virtual void SetVisualViewportSize(const CSSSize& aSize) = 0;
   virtual void UpdateDisplayPortMargins() = 0;
 
   enum class ResizeEventFlag {
     IfNecessary,  // resize events will be fired if necessary.
     Suppress,     // resize events will never be fired.
   };
-  virtual void Reflow(const CSSSize& aNewSize, const CSSSize& aOldSize,
-                      ResizeEventFlag aResizeEventFlag) = 0;
+  virtual void Reflow(const CSSSize& aNewSize, ResizeEventFlag) = 0;
 };
 
 }  // namespace mozilla
 
 #endif  // MVMContext_h_
--- a/layout/base/MobileViewportManager.cpp
+++ b/layout/base/MobileViewportManager.cpp
@@ -584,25 +584,23 @@ void MobileViewportManager::RefreshViewp
     // Even without zoom, we need to update that the visual viewport size
     // has changed.
     RefreshVisualViewportSize();
   }
   if (gfxPlatform::AsyncPanZoomEnabled()) {
     UpdateDisplayPortMargins();
   }
 
-  CSSSize oldSize = mMobileViewportSize;
-
   // Update internal state.
   mMobileViewportSize = viewport;
 
   RefPtr<MobileViewportManager> strongThis(this);
 
   // Kick off a reflow.
-  mContext->Reflow(viewport, oldSize,
+  mContext->Reflow(viewport,
                    mIsFirstPaint ? MVMContext::ResizeEventFlag::Suppress
                                  : MVMContext::ResizeEventFlag::IfNecessary);
 
   // We are going to fit the content to the display width if the initial-scale
   // is not specied and if the content is still wider than the display width.
   ShrinkToDisplaySizeIfNeeded(viewportInfo, displaySize);
 
   mIsFirstPaint = false;
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -1833,17 +1833,16 @@ nsresult PresShell::Initialize() {
 }
 
 void PresShell::sPaintSuppressionCallback(nsITimer* aTimer, void* aPresShell) {
   RefPtr<PresShell> self = static_cast<PresShell*>(aPresShell);
   if (self) self->UnsuppressPainting();
 }
 
 nsresult PresShell::ResizeReflow(nscoord aWidth, nscoord aHeight,
-                                 nscoord aOldWidth, nscoord aOldHeight,
                                  ResizeReflowOptions aOptions) {
   if (mZoomConstraintsClient) {
     // If we have a ZoomConstraintsClient and the available screen area
     // changed, then we might need to disable double-tap-to-zoom, so notify
     // the ZCC to update itself.
     mZoomConstraintsClient->ScreenSizeChanged();
   }
   if (mMobileViewportManager) {
@@ -1851,52 +1850,52 @@ nsresult PresShell::ResizeReflow(nscoord
     // recompute the final CSS viewport and trigger a call to
     // ResizeReflowIgnoreOverride if it changed. We don't force adjusting
     // of resolution, because that is only necessary when we are destroying
     // the MVM.
     mMobileViewportManager->RequestReflow(false);
     return NS_OK;
   }
 
-  return ResizeReflowIgnoreOverride(aWidth, aHeight, aOldWidth, aOldHeight,
-                                    aOptions);
+  return ResizeReflowIgnoreOverride(aWidth, aHeight, aOptions);
 }
 
 void PresShell::SimpleResizeReflow(nscoord aWidth, nscoord aHeight,
-                                   nscoord aOldWidth, nscoord aOldHeight) {
+                                   ResizeReflowOptions aOptions) {
   MOZ_ASSERT(aWidth != NS_UNCONSTRAINEDSIZE);
   MOZ_ASSERT(aHeight != NS_UNCONSTRAINEDSIZE);
+  nsSize oldSize = mPresContext->GetVisibleArea().Size();
   mPresContext->SetVisibleArea(nsRect(0, 0, aWidth, aHeight));
   nsIFrame* rootFrame = GetRootFrame();
   if (!rootFrame) {
     return;
   }
   WritingMode wm = rootFrame->GetWritingMode();
   bool isBSizeChanging =
-      wm.IsVertical() ? aOldWidth != aWidth : aOldHeight != aHeight;
+      wm.IsVertical() ? oldSize.width != aWidth : oldSize.height != aHeight;
   if (isBSizeChanging) {
     nsLayoutUtils::MarkIntrinsicISizesDirtyIfDependentOnBSize(rootFrame);
   }
   FrameNeedsReflow(rootFrame, IntrinsicDirty::Resize,
                    NS_FRAME_HAS_DIRTY_CHILDREN);
 
   // For compat with the old code path which always reflowed as long as there
   // was a root frame.
-  if (!mPresContext->SuppressingResizeReflow()) {
+  bool suppressReflow = (aOptions & ResizeReflowOptions::SuppressReflow) ||
+                        mPresContext->SuppressingResizeReflow();
+  if (!suppressReflow) {
     mDocument->FlushPendingNotifications(FlushType::InterruptibleLayout);
   }
 }
 
 nsresult PresShell::ResizeReflowIgnoreOverride(nscoord aWidth, nscoord aHeight,
-                                               nscoord aOldWidth,
-                                               nscoord aOldHeight,
                                                ResizeReflowOptions aOptions) {
   MOZ_ASSERT(!mIsReflowing, "Shouldn't be in reflow here!");
-
-  if (aWidth == aOldWidth && aHeight == aOldHeight) {
+  nsSize oldSize = mPresContext->GetVisibleArea().Size();
+  if (oldSize == nsSize(aWidth, aHeight)) {
     return NS_OK;
   }
 
   // Historically we never fired resize events if there was no root frame by the
   // time this function got called.
   const bool initialized = mDidInitialize;
   RefPtr<PresShell> kungFuDeathGrip(this);
 
@@ -1906,22 +1905,23 @@ nsresult PresShell::ResizeReflowIgnoreOv
       mResizeEventPending = true;
       if (MOZ_LIKELY(!mDocument->GetBFCacheEntry())) {
         mPresContext->RefreshDriver()->AddResizeEventFlushObserver(this);
       }
     }
   };
 
   if (!(aOptions & ResizeReflowOptions::BSizeLimit)) {
-    SimpleResizeReflow(aWidth, aHeight, aOldWidth, aOldHeight);
+    SimpleResizeReflow(aWidth, aHeight, aOptions);
     postResizeEventIfNeeded();
     return NS_OK;
   }
 
-  MOZ_ASSERT(!mPresContext->SuppressingResizeReflow(),
+  MOZ_ASSERT(!mPresContext->SuppressingResizeReflow() &&
+             !(aOptions & ResizeReflowOptions::SuppressReflow),
              "Can't suppress resize reflow and shrink-wrap at the same time");
 
   // Make sure that style is flushed before setting the pres context
   // VisibleArea.
   //
   // Otherwise we may end up with bogus viewport units resolved against the
   // unconstrained bsize, or restyling the whole document resolving viewport
   // units against targetWidth, which may end up doing wasteful work.
--- a/layout/base/PresShell.h
+++ b/layout/base/PresShell.h
@@ -339,30 +339,27 @@ class PresShell final : public nsStubDoc
    */
   nsresult Initialize();
 
   /**
    * Reflow the frame model into a new width and height.  The
    * coordinates for aWidth and aHeight must be in standard nscoord's.
    */
   MOZ_CAN_RUN_SCRIPT nsresult
-  ResizeReflow(nscoord aWidth, nscoord aHeight, nscoord aOldWidth = 0,
-               nscoord aOldHeight = 0,
-               ResizeReflowOptions aOptions = ResizeReflowOptions::NoOption);
+  ResizeReflow(nscoord aWidth, nscoord aHeight,
+               ResizeReflowOptions = ResizeReflowOptions::NoOption);
   MOZ_CAN_RUN_SCRIPT nsresult ResizeReflowIgnoreOverride(
-      nscoord aWidth, nscoord aHeight, nscoord aOldWidth, nscoord aOldHeight,
-      ResizeReflowOptions aOptions = ResizeReflowOptions::NoOption);
+      nscoord aWidth, nscoord aHeight, ResizeReflowOptions);
 
  private:
   /**
    * This is what ResizeReflowIgnoreOverride does when not shrink-wrapping (that
    * is, when ResizeReflowOptions::BSizeLimit is not specified).
    */
-  void SimpleResizeReflow(nscoord aWidth, nscoord aHeight, nscoord aOldWidth,
-                          nscoord aOldHeight);
+  void SimpleResizeReflow(nscoord aWidth, nscoord aHeight, ResizeReflowOptions);
 
  public:
   /**
    * Returns true if this document has a potentially zoomable viewport,
    * allowing for its layout and visual viewports to diverge.
    */
   bool GetIsViewportOverridden() const {
     return (mMobileViewportManager != nullptr);
--- a/layout/base/PresShellForwards.h
+++ b/layout/base/PresShellForwards.h
@@ -36,16 +36,22 @@ enum class ResizeReflowOptions : uint32_
   // the resulting BSize can be less than the given one, producing
   // shrink-to-fit sizing in the block dimension
   BSizeLimit = 1 << 0,
   // suppress resize events even if the content size is changed due to the
   // reflow.  This flag is used for mobile since on mobile we need to do an
   // additional reflow to zoom the content by the initial-scale or auto scaling
   // and we don't want any resize events during the initial paint.
   SuppressResizeEvent = 1 << 1,
+  // Invalidate layout, but don't reflow.
+  //
+  // TODO(emilio): Ideally this should just become the default, or we should
+  // unconditionally not reflow and rely on the caller to do so, having a
+  // separate API for shrink-to-fit.
+  SuppressReflow = 1 << 2,
 };
 
 MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(ResizeReflowOptions)
 
 // This is actually pref-controlled, but we use this value if we fail to get
 // the pref for any reason.
 #define PAINTLOCK_EVENT_DELAY 5
 
--- a/layout/base/nsDocumentViewer.cpp
+++ b/layout/base/nsDocumentViewer.cpp
@@ -3171,17 +3171,17 @@ nsresult nsDocumentViewer::GetContentSiz
   {
     RefPtr<gfxContext> rcx(presShell->CreateReferenceRenderingContext());
     prefWidth = root->GetPrefISize(rcx);
   }
   if (prefWidth > aMaxWidth) {
     prefWidth = aMaxWidth;
   }
 
-  nsresult rv = presShell->ResizeReflow(prefWidth, aMaxHeight, 0, 0,
+  nsresult rv = presShell->ResizeReflow(prefWidth, aMaxHeight,
                                         ResizeReflowOptions::BSizeLimit);
   NS_ENSURE_SUCCESS(rv, rv);
 
   RefPtr<nsPresContext> presContext = GetPresContext();
   NS_ENSURE_TRUE(presContext, NS_ERROR_FAILURE);
 
   // Protect against bogus returns here
   nsRect shellArea = presContext->GetVisibleArea();
--- a/view/nsViewManager.cpp
+++ b/view/nsViewManager.cpp
@@ -166,26 +166,32 @@ void nsViewManager::GetWindowDimensions(
       *aHeight = mDelayedResize.height;
     }
   } else {
     *aWidth = 0;
     *aHeight = 0;
   }
 }
 
-void nsViewManager::DoSetWindowDimensions(nscoord aWidth, nscoord aHeight) {
+void nsViewManager::DoSetWindowDimensions(nscoord aWidth, nscoord aHeight,
+                                          bool aDoReflow) {
   nsRect oldDim = mRootView->GetDimensions();
   nsRect newDim(0, 0, aWidth, aHeight);
   // We care about resizes even when one dimension is already zero.
-  if (!oldDim.IsEqualEdges(newDim)) {
-    // Don't resize the widget. It is already being set elsewhere.
-    mRootView->SetDimensions(newDim, true, false);
-    if (RefPtr<PresShell> presShell = mPresShell) {
-      presShell->ResizeReflow(aWidth, aHeight, oldDim.Width(), oldDim.Height());
+  if (oldDim.IsEqualEdges(newDim)) {
+    return;
+  }
+  // Don't resize the widget. It is already being set elsewhere.
+  mRootView->SetDimensions(newDim, true, false);
+  if (RefPtr<PresShell> presShell = mPresShell) {
+    auto options = ResizeReflowOptions::NoOption;
+    if (!aDoReflow) {
+      options |= ResizeReflowOptions::SuppressReflow;
     }
+    presShell->ResizeReflow(aWidth, aHeight, options);
   }
 }
 
 bool nsViewManager::ShouldDelayResize() const {
   MOZ_ASSERT(mRootView);
   if (!mRootView->IsEffectivelyVisible() || !mPresShell ||
       !mPresShell->IsVisible()) {
     return true;
@@ -208,38 +214,31 @@ void nsViewManager::SetWindowDimensions(
         // been flushed to the PresContext so we need to update the PresContext
         // with the new size because if the new size is exactly the same as the
         // root view's current size then DoSetWindowDimensions will not
         // request a resize reflow (which would correct it). See bug 617076.
         mDelayedResize = nsSize(aWidth, aHeight);
         FlushDelayedResize(false);
       }
       mDelayedResize.SizeTo(NSCOORD_NONE, NSCOORD_NONE);
-      DoSetWindowDimensions(aWidth, aHeight);
+      DoSetWindowDimensions(aWidth, aHeight, /* aDoReflow = */ true);
     } else {
       mDelayedResize.SizeTo(aWidth, aHeight);
       if (mPresShell) {
         mPresShell->SetNeedStyleFlush();
         mPresShell->SetNeedLayoutFlush();
       }
     }
   }
 }
 
 void nsViewManager::FlushDelayedResize(bool aDoReflow) {
   if (mDelayedResize != nsSize(NSCOORD_NONE, NSCOORD_NONE)) {
-    if (aDoReflow) {
-      DoSetWindowDimensions(mDelayedResize.width, mDelayedResize.height);
-      mDelayedResize.SizeTo(NSCOORD_NONE, NSCOORD_NONE);
-    } else if (mPresShell && !mPresShell->GetIsViewportOverridden()) {
-      nsPresContext* presContext = mPresShell->GetPresContext();
-      if (presContext) {
-        presContext->SetVisibleArea(nsRect(nsPoint(0, 0), mDelayedResize));
-      }
-    }
+    DoSetWindowDimensions(mDelayedResize.width, mDelayedResize.height, aDoReflow);
+    mDelayedResize.SizeTo(NSCOORD_NONE, NSCOORD_NONE);
   }
 }
 
 // Convert aIn from being relative to and in appunits of aFromView, to being
 // relative to and in appunits of aToView.
 static nsRegion ConvertRegionBetweenViews(const nsRegion& aIn,
                                           nsView* aFromView, nsView* aToView) {
   nsRegion out = aIn;
--- a/view/nsViewManager.h
+++ b/view/nsViewManager.h
@@ -359,17 +359,17 @@ class nsViewManager final {
   /**
    * Intersects aRect with aView's bounds and then transforms it from aView's
    * coordinate system to the coordinate system of the widget attached to
    * aView.
    */
   LayoutDeviceIntRect ViewToWidget(nsView* aView, const nsRect& aRect) const;
 
   MOZ_CAN_RUN_SCRIPT_BOUNDARY
-  void DoSetWindowDimensions(nscoord aWidth, nscoord aHeight);
+  void DoSetWindowDimensions(nscoord aWidth, nscoord aHeight, bool aDoReflow);
   bool ShouldDelayResize() const;
 
   bool IsPainting() const { return RootViewManager()->mPainting; }
 
   void SetPainting(bool aPainting) { RootViewManager()->mPainting = aPainting; }
 
   void InvalidateView(nsView* aView, const nsRect& aRect);