From 848d89d65f6ef75f1be0f602541cc091136971b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 25 Sep 2019 19:12:44 +0000 Subject: [PATCH] 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 --- dom/base/nsGlobalWindowOuter.cpp | 5 +++ .../gtest/mvm/TestMobileViewportManager.cpp | 3 +- layout/base/GeckoMVMContext.cpp | 5 ++- layout/base/GeckoMVMContext.h | 3 +- layout/base/MVMContext.h | 3 +- layout/base/MobileViewportManager.cpp | 4 +-- layout/base/PresShell.cpp | 24 +++++++------- layout/base/PresShell.h | 11 +++---- layout/base/PresShellForwards.h | 6 ++++ layout/base/nsDocumentViewer.cpp | 2 +- view/nsViewManager.cpp | 31 +++++++++---------- view/nsViewManager.h | 2 +- 12 files changed, 50 insertions(+), 49 deletions(-) diff --git a/dom/base/nsGlobalWindowOuter.cpp b/dom/base/nsGlobalWindowOuter.cpp index 53c9ea1e0d73..189a973ae36c 100644 --- a/dom/base/nsGlobalWindowOuter.cpp +++ b/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); } diff --git a/gfx/layers/apz/test/gtest/mvm/TestMobileViewportManager.cpp b/gfx/layers/apz/test/gtest/mvm/TestMobileViewportManager.cpp index 85ff648186fa..9481c15a7a98 100644 --- a/gfx/layers/apz/test/gtest/mvm/TestMobileViewportManager.cpp +++ b/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); } diff --git a/layout/base/GeckoMVMContext.cpp b/layout/base/GeckoMVMContext.cpp index 16a8aa6be6cc..dfd2b72c208c 100644 --- a/layout/base/GeckoMVMContext.cpp +++ b/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 diff --git a/layout/base/GeckoMVMContext.h b/layout/base/GeckoMVMContext.h index 74c5d19372df..53f3758a3cfb 100644 --- a/layout/base/GeckoMVMContext.h +++ b/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 mDocument; diff --git a/layout/base/MVMContext.h b/layout/base/MVMContext.h index b3cbec4690cd..71fcf3f781c9 100644 --- a/layout/base/MVMContext.h +++ b/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 diff --git a/layout/base/MobileViewportManager.cpp b/layout/base/MobileViewportManager.cpp index 19e82f0003bb..67a8cbbcdc80 100644 --- a/layout/base/MobileViewportManager.cpp +++ b/layout/base/MobileViewportManager.cpp @@ -589,15 +589,13 @@ void MobileViewportManager::RefreshViewportSize(bool aForceAdjustResolution) { UpdateDisplayPortMargins(); } - CSSSize oldSize = mMobileViewportSize; - // Update internal state. mMobileViewportSize = viewport; RefPtr strongThis(this); // Kick off a reflow. - mContext->Reflow(viewport, oldSize, + mContext->Reflow(viewport, mIsFirstPaint ? MVMContext::ResizeEventFlag::Suppress : MVMContext::ResizeEventFlag::IfNecessary); diff --git a/layout/base/PresShell.cpp b/layout/base/PresShell.cpp index 809ff6e2f7be..b47ccd92e4f8 100644 --- a/layout/base/PresShell.cpp +++ b/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,14 +1855,14 @@ 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) { @@ -1871,7 +1870,7 @@ void PresShell::SimpleResizeReflow(nscoord aWidth, nscoord aHeight, } 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 diff --git a/layout/base/PresShell.h b/layout/base/PresShell.h index 169bc0f75ade..f5f6d65c22dd 100644 --- a/layout/base/PresShell.h +++ b/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: /** diff --git a/layout/base/PresShellForwards.h b/layout/base/PresShellForwards.h index f30598ea3988..a437dd10c1b7 100644 --- a/layout/base/PresShellForwards.h +++ b/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) diff --git a/layout/base/nsDocumentViewer.cpp b/layout/base/nsDocumentViewer.cpp index 952538d0b510..898b514cf568 100644 --- a/layout/base/nsDocumentViewer.cpp +++ b/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); diff --git a/view/nsViewManager.cpp b/view/nsViewManager.cpp index 75afd92b3e6e..7e2fa0a5284b 100644 --- a/view/nsViewManager.cpp +++ b/view/nsViewManager.cpp @@ -171,16 +171,22 @@ void nsViewManager::GetWindowDimensions(nscoord* aWidth, nscoord* aHeight) { } } -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 = 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 = mPresShell) { + auto options = ResizeReflowOptions::NoOption; + if (!aDoReflow) { + options |= ResizeReflowOptions::SuppressReflow; } + presShell->ResizeReflow(aWidth, aHeight, options); } } @@ -213,7 +219,7 @@ void nsViewManager::SetWindowDimensions(nscoord aWidth, nscoord aHeight, FlushDelayedResize(false); } mDelayedResize.SizeTo(NSCOORD_NONE, NSCOORD_NONE); - DoSetWindowDimensions(aWidth, aHeight); + DoSetWindowDimensions(aWidth, aHeight, /* aDoReflow = */ true); } else { mDelayedResize.SizeTo(aWidth, aHeight); if (mPresShell) { @@ -226,15 +232,8 @@ void nsViewManager::SetWindowDimensions(nscoord aWidth, nscoord aHeight, 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); } } diff --git a/view/nsViewManager.h b/view/nsViewManager.h index 351c809d0dca..7fab62e40f0b 100644 --- a/view/nsViewManager.h +++ b/view/nsViewManager.h @@ -364,7 +364,7 @@ class nsViewManager final { 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; }