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

--HG--
extra : moz-landing-system : lando

Author: emilio

dom/base/nsGlobalWindowOuter.cpp

@@ -3852,6 +3852,11 @@ void nsGlobalWindowOuter::SetCSSViewportWidthAndHeight(nscoord aInnerWidth,
   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);
 }
 

gfx/layers/apz/test/gtest/mvm/TestMobileViewportManager.cpp

@@ -82,8 +82,7 @@ class MockMVMContext : public MVMContext {
     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);
   }

layout/base/GeckoMVMContext.cpp

@@ -176,7 +176,7 @@ void GeckoMVMContext::UpdateDisplayPortMargins() {
   }
 }
 
-void GeckoMVMContext::Reflow(const CSSSize& aNewSize, const CSSSize& aOldSize,
+void GeckoMVMContext::Reflow(const CSSSize& aNewSize,
                              ResizeEventFlag aResizeEventFlag) {
   MOZ_ASSERT(mPresShell);
 
@@ -189,8 +189,7 @@ void GeckoMVMContext::Reflow(const CSSSize& aNewSize, const CSSSize& aOldSize,
   presShell->ResizeReflowIgnoreOverride(
       nsPresContext::CSSPixelsToAppUnits(aNewSize.width),
       nsPresContext::CSSPixelsToAppUnits(aNewSize.height),
-      nsPresContext::CSSPixelsToAppUnits(aOldSize.width),
-      nsPresContext::CSSPixelsToAppUnits(aOldSize.height), reflowOptions);
+      reflowOptions);
 }
 
 }  // namespace mozilla

layout/base/GeckoMVMContext.h

@@ -55,8 +55,7 @@ class GeckoMVMContext : public MVMContext {
   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;

layout/base/MVMContext.h

@@ -63,8 +63,7 @@ class MVMContext {
     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

layout/base/MobileViewportManager.cpp

@@ -589,15 +589,13 @@ void MobileViewportManager::RefreshViewportSize(bool aForceAdjustResolution) {
     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);
 

layout/base/PresShell.cpp

@@ -1838,7 +1838,6 @@ void PresShell::sPaintSuppressionCallback(nsITimer* aTimer, void* aPresShell) {
 }
 
 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
@@ -1856,22 +1855,22 @@ nsresult PresShell::ResizeReflow(nscoord aWidth, nscoord aHeight,
     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);
   }
@@ -1880,18 +1879,18 @@ void PresShell::SimpleResizeReflow(nscoord aWidth, nscoord aHeight,
 
   // 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;
   }
 
@@ -1911,12 +1910,13 @@ nsresult PresShell::ResizeReflowIgnoreOverride(nscoord aWidth, nscoord aHeight,
   };
 
   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

layout/base/PresShell.h

@@ -344,20 +344,17 @@ class PresShell final : public nsStubDocumentObserver,
    * 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:
   /**

layout/base/PresShellForwards.h

@@ -41,6 +41,12 @@ enum class ResizeReflowOptions : uint32_t {
   // 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)

layout/base/nsDocumentViewer.cpp

@@ -3176,7 +3176,7 @@ nsresult nsDocumentViewer::GetContentSizeInternal(int32_t* aWidth,
     prefWidth = aMaxWidth;
   }
 
-  nsresult rv = presShell->ResizeReflow(prefWidth, aMaxHeight, 0, 0,
+  nsresult rv = presShell->ResizeReflow(prefWidth, aMaxHeight,
                                         ResizeReflowOptions::BSizeLimit);
   NS_ENSURE_SUCCESS(rv, rv);