Skip to content

Commit

Permalink
Bug 1786513 - Make SimpleResizeReflow not flush by default. r=jfktham…
Browse files Browse the repository at this point in the history 
…e,layout-reviewers

Only GeckoMVMContext really needs the flush, to measure scrolled height
afterwards. Do that explicitly.

This shouldn't change behavior, for the most part; there was a preload
test that relied on the flush when changing DPI to start a run really
clean, but other than that this looks green on try.

Should at best be neutral (just code clean-up), or be a performance
improvement.

In a follow-up, we can possibly remove the DelayedResize code from the
view manager, though I need to think how to possibly coalesce the MVM
reflows, so let's not do that yet.

Differential Revision: https://phabricator.services.mozilla.com/D155385
  • Loading branch information
@emilio
emilio committed Aug 26, 2022
1 parent bf4a80b commit 7c34c8d
Show file tree
Hide file tree
Showing 13 changed files with 56 additions and 121 deletions.
28 changes: 1 addition & 27 deletions docshell/base/nsDocShellTreeOwner.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -470,33 +470,7 @@ nsDocShellTreeOwner::SizeShellTo(nsIDocShellTreeItem* aShellItem, int32_t aCX,
return NS_ERROR_NOT_IMPLEMENTED;
}

NS_ENSURE_TRUE(aShellItem, NS_ERROR_FAILURE);

RefPtr<Document> document = aShellItem->GetDocument();
NS_ENSURE_TRUE(document, NS_ERROR_FAILURE);

NS_ENSURE_TRUE(document->GetDocumentElement(), NS_ERROR_FAILURE);

// Set the preferred Size
// XXX
NS_ERROR("Implement this");
/*
Set the preferred size on the aShellItem.
*/

RefPtr<nsPresContext> presContext = mWebBrowser->mDocShell->GetPresContext();
NS_ENSURE_TRUE(presContext, NS_ERROR_FAILURE);

RefPtr<PresShell> presShell = presContext->GetPresShell();
NS_ENSURE_TRUE(presShell, NS_ERROR_FAILURE);

NS_ENSURE_SUCCESS(
presShell->ResizeReflow(NS_UNCONSTRAINEDSIZE, NS_UNCONSTRAINEDSIZE),
NS_ERROR_FAILURE);

// XXX: this is weird, but we used to call a method here
// (webBrowserChrome->SizeBrowserTo()) whose implementations all failed like
// this, so...
MOZ_ASSERT_UNREACHABLE("This is unimplemented, API should be cleaned up");
return NS_ERROR_NOT_IMPLEMENTED;
}

Expand Down
5 changes: 2 additions & 3 deletions dom/base/Element.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1031,9 +1031,8 @@ nsRect Element::GetClientAreaRect() {
doc->IsScrollingElement(this)) {
if (PresShell* presShell = doc->GetPresShell()) {
// Ensure up to date dimensions, but don't reflow
RefPtr<nsViewManager> viewManager = presShell->GetViewManager();
if (viewManager) {
viewManager->FlushDelayedResize(false);
if (RefPtr<nsViewManager> viewManager = presShell->GetViewManager()) {
viewManager->FlushDelayedResize();
}
return nsRect(nsPoint(), presContext->GetVisibleArea().Size());
}
Expand Down
5 changes: 2 additions & 3 deletions dom/base/nsGlobalWindowOuter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3496,9 +3496,8 @@ nsresult nsGlobalWindowOuter::GetInnerSize(CSSSize& aSize) {

// Whether or not the css viewport has been overridden, we can get the
// correct value by looking at the visible area of the presContext.
RefPtr<nsViewManager> viewManager = presShell->GetViewManager();
if (viewManager) {
viewManager->FlushDelayedResize(false);
if (RefPtr<nsViewManager> viewManager = presShell->GetViewManager()) {
viewManager->FlushDelayedResize();
}

// FIXME: Bug 1598487 - Return the layout viewport instead of the ICB.
Expand Down
14 changes: 9 additions & 5 deletions layout/base/GeckoMVMContext.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,12 +194,16 @@ void GeckoMVMContext::UpdateDisplayPortMargins() {
}

void GeckoMVMContext::Reflow(const CSSSize& aNewSize) {
MOZ_ASSERT(mPresShell);
RefPtr doc = mDocument;
RefPtr ps = mPresShell;

MOZ_ASSERT(doc);
MOZ_ASSERT(ps);

RefPtr<PresShell> presShell = mPresShell;
presShell->ResizeReflowIgnoreOverride(CSSPixel::ToAppUnits(aNewSize.width),
CSSPixel::ToAppUnits(aNewSize.height),
ResizeReflowOptions::NoOption);
if (ps->ResizeReflowIgnoreOverride(CSSPixel::ToAppUnits(aNewSize.width),
CSSPixel::ToAppUnits(aNewSize.height))) {
doc->FlushPendingNotifications(FlushType::InterruptibleLayout);
}
}

} // namespace mozilla
45 changes: 16 additions & 29 deletions layout/base/PresShell.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1967,8 +1967,8 @@ void PresShell::RefreshZoomConstraintsForScreenSizeChange() {
}
}

nsresult PresShell::ResizeReflow(nscoord aWidth, nscoord aHeight,
ResizeReflowOptions aOptions) {
void PresShell::ResizeReflow(nscoord aWidth, nscoord aHeight,
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
Expand All @@ -1983,21 +1983,19 @@ nsresult PresShell::ResizeReflow(nscoord aWidth, nscoord aHeight,
// the MVM.
MOZ_ASSERT(mMobileViewportManager);
mMobileViewportManager->RequestReflow(false);
return NS_OK;
return;
}

return ResizeReflowIgnoreOverride(aWidth, aHeight, aOptions);
ResizeReflowIgnoreOverride(aWidth, aHeight, aOptions);
}

void PresShell::SimpleResizeReflow(nscoord aWidth, nscoord aHeight,
ResizeReflowOptions aOptions) {
bool PresShell::SimpleResizeReflow(nscoord aWidth, nscoord aHeight) {
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;
return false;
}
WritingMode wm = rootFrame->GetWritingMode();
bool isBSizeChanging =
Expand All @@ -2011,14 +2009,7 @@ void PresShell::SimpleResizeReflow(nscoord aWidth, nscoord aHeight,
if (mMobileViewportManager) {
mMobileViewportManager->UpdateSizesBeforeReflow();
}

// For compat with the old code path which always reflowed as long as there
// was a root frame.
bool suppressReflow = (aOptions & ResizeReflowOptions::SuppressReflow) ||
mPresContext->SuppressingResizeReflow();
if (!suppressReflow) {
mDocument->FlushPendingNotifications(FlushType::InterruptibleLayout);
}
return true;
}

void PresShell::AddResizeEventFlushObserverIfNeeded() {
Expand All @@ -2029,8 +2020,8 @@ void PresShell::AddResizeEventFlushObserverIfNeeded() {
}
}

nsresult PresShell::ResizeReflowIgnoreOverride(nscoord aWidth, nscoord aHeight,
ResizeReflowOptions aOptions) {
bool PresShell::ResizeReflowIgnoreOverride(nscoord aWidth, nscoord aHeight,
ResizeReflowOptions aOptions) {
MOZ_ASSERT(!mIsReflowing, "Shouldn't be in reflow here!");

// Historically we never fired resize events if there was no root frame by the
Expand All @@ -2047,18 +2038,14 @@ nsresult PresShell::ResizeReflowIgnoreOverride(nscoord aWidth, nscoord aHeight,
if (!(aOptions & ResizeReflowOptions::BSizeLimit)) {
nsSize oldSize = mPresContext->GetVisibleArea().Size();
if (oldSize == nsSize(aWidth, aHeight)) {
return NS_OK;
return false;
}

SimpleResizeReflow(aWidth, aHeight, aOptions);
bool changed = SimpleResizeReflow(aWidth, aHeight);
postResizeEventIfNeeded();
return NS_OK;
return changed;
}

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.
//
Expand All @@ -2075,13 +2062,13 @@ nsresult PresShell::ResizeReflowIgnoreOverride(nscoord aWidth, nscoord aHeight,
if (aHeight == NS_UNCONSTRAINEDSIZE || aWidth == NS_UNCONSTRAINEDSIZE) {
// We can't do the work needed for SizeToContent without a root
// frame, and we want to return before setting the visible area.
return NS_ERROR_NOT_AVAILABLE;
return false;
}

mPresContext->SetVisibleArea(nsRect(0, 0, aWidth, aHeight));
// There isn't anything useful we can do if the initial reflow hasn't
// happened.
return NS_OK;
return true;
}

WritingMode wm = rootFrame->GetWritingMode();
Expand Down Expand Up @@ -2141,7 +2128,7 @@ nsresult PresShell::ResizeReflowIgnoreOverride(nscoord aWidth, nscoord aHeight,
"height should not be NS_UNCONSTRAINEDSIZE after reflow");

postResizeEventIfNeeded();
return NS_OK; // XXX this needs to be real. MMP
return true;
}

void PresShell::FireResizeEvent() {
Expand Down Expand Up @@ -4329,7 +4316,7 @@ void PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush aFlush) {
// Process pending restyles, since any flush of the presshell wants
// up-to-date style data.
if (MOZ_LIKELY(!mIsDestroying)) {
viewManager->FlushDelayedResize(false);
viewManager->FlushDelayedResize();
mPresContext->FlushPendingMediaFeatureValuesChanged();
}

Expand Down
18 changes: 10 additions & 8 deletions layout/base/PresShell.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -346,15 +346,17 @@ class PresShell final : public nsStubDocumentObserver,
MOZ_CAN_RUN_SCRIPT_BOUNDARY nsresult Initialize();

/**
* Reflow the frame model into a new width and height. The
* Schedule a reflow for the frame model into a new width and height. The
* coordinates for aWidth and aHeight must be in standard nscoord's.
*
* Returns whether layout might have changed.
*/
MOZ_CAN_RUN_SCRIPT nsresult
ResizeReflow(nscoord aWidth, nscoord aHeight,
ResizeReflowOptions = ResizeReflowOptions::NoOption);
MOZ_CAN_RUN_SCRIPT nsresult ResizeReflowIgnoreOverride(nscoord aWidth,
nscoord aHeight,
ResizeReflowOptions);
MOZ_CAN_RUN_SCRIPT void ResizeReflow(
nscoord aWidth, nscoord aHeight,
ResizeReflowOptions = ResizeReflowOptions::NoOption);
MOZ_CAN_RUN_SCRIPT bool ResizeReflowIgnoreOverride(
nscoord aWidth, nscoord aHeight,
ResizeReflowOptions = ResizeReflowOptions::NoOption);

/**
* Add this pres shell to the refresh driver to be observed for resize
Expand Down Expand Up @@ -382,7 +384,7 @@ class PresShell final : public nsStubDocumentObserver,
* This is what ResizeReflowIgnoreOverride does when not shrink-wrapping (that
* is, when ResizeReflowOptions::BSizeLimit is not specified).
*/
void SimpleResizeReflow(nscoord aWidth, nscoord aHeight, ResizeReflowOptions);
bool SimpleResizeReflow(nscoord aWidth, nscoord aHeight);

public:
/**
Expand Down
6 changes: 0 additions & 6 deletions layout/base/PresShellForwards.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,6 @@ enum class ResizeReflowOptions : uint32_t {
// the resulting BSize can be less than the given one, producing
// shrink-to-fit sizing in the block dimension
BSizeLimit = 1 << 0,
// 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 << 1,
};

MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(ResizeReflowOptions)
Expand Down
5 changes: 2 additions & 3 deletions layout/base/nsDocumentViewer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2634,9 +2634,8 @@ nsresult nsDocumentViewer::GetContentSizeInternal(int32_t* aWidth,

nscoord height = wm.IsVertical() ? prefISize : aMaxHeight;
nscoord width = wm.IsVertical() ? aMaxWidth : prefISize;
nsresult rv =
presShell->ResizeReflow(width, height, ResizeReflowOptions::BSizeLimit);
NS_ENSURE_SUCCESS(rv, rv);

presShell->ResizeReflow(width, height, ResizeReflowOptions::BSizeLimit);

RefPtr<nsPresContext> presContext = GetPresContext();
NS_ENSURE_TRUE(presContext, NS_ERROR_FAILURE);
Expand Down
15 changes: 1 addition & 14 deletions layout/base/nsPresContext.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,6 @@ nsPresContext::nsPresContext(dom::Document* aDocument, nsPresContextType aType)
mUsesFontMetricDependentFontUnits(false),
mCounterStylesDirty(true),
mFontFeatureValuesDirty(true),
mSuppressResizeReflow(false),
mIsVisual(false),
mHasWarnedAboutPositionedTableParts(false),
mHasWarnedAboutTooLargeDashedOrDottedRadius(false),
Expand Down Expand Up @@ -1285,24 +1284,13 @@ void nsPresContext::SetFullZoom(float aZoom) {
float oldHeightDevPixels = oldHeightAppUnits / float(mCurAppUnitsPerDevPixel);
mDeviceContext->SetFullZoom(aZoom);

NS_ASSERTION(!mSuppressResizeReflow,
"two zooms happening at the same time? Impossible!");

// We can't suppress the resize reflow if we support APZ zooming, as MVM
// relies on ResizeReflowIgnoreOverride() actually updating layout to update
// the viewport based on that.
RefPtr<MobileViewportManager> mvm = mPresShell->GetMobileViewportManager();
mSuppressResizeReflow = !mvm;

mFullZoom = aZoom;

AppUnitsPerDevPixelChanged();

mPresShell->GetViewManager()->SetWindowDimensions(
NSToCoordRound(oldWidthDevPixels * AppUnitsPerDevPixel()),
NSToCoordRound(oldHeightDevPixels * AppUnitsPerDevPixel()));

mSuppressResizeReflow = false;
}

void nsPresContext::SetOverrideDPPX(float aDPPX) {
Expand Down Expand Up @@ -2836,8 +2824,7 @@ void nsPresContext::SetDynamicToolbarMaxHeight(ScreenIntCoord aHeight) {
nscoord currentWidth, currentHeight;
presShell->GetViewManager()->GetWindowDimensions(&currentWidth,
&currentHeight);
presShell->ResizeReflow(currentWidth, currentHeight,
ResizeReflowOptions::NoOption);
presShell->ResizeReflow(currentWidth, currentHeight);
}
}

Expand Down
6 changes: 0 additions & 6 deletions layout/base/nsPresContext.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -906,8 +906,6 @@ class nsPresContext : public nsISupports, public mozilla::SupportsWeakPtr {
// Is this presentation in a chrome docshell?
bool IsChrome() const;

bool SuppressingResizeReflow() const { return mSuppressResizeReflow; }

gfxUserFontSet* GetUserFontSet();

// Should be called whenever the set of fonts available in the user
Expand Down Expand Up @@ -1364,10 +1362,6 @@ class nsPresContext : public nsISupports, public mozilla::SupportsWeakPtr {
// Is the current mFontFeatureValuesLookup valid?
unsigned mFontFeatureValuesDirty : 1;

// resize reflow is suppressed when the only change has been to zoom
// the document rather than to change the document's dimensions
unsigned mSuppressResizeReflow : 1;

unsigned mIsVisual : 1;

unsigned mHasWarnedAboutPositionedTableParts : 1;
Expand Down
6 changes: 4 additions & 2 deletions parser/htmlparser/tests/mochitest/test_img_picture_preload.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@
<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
</head>
<body>

<script type="text/javascript">
<script>
// Ensure srcset/picture are enabled, re-run the test at different DPI
// levels to ensure preload step does the right responsive image selection

Expand Down Expand Up @@ -67,6 +66,9 @@
iframe.addEventListener("load", function() {
SpecialPowers.pushPrefEnv({"set": [ [ "layout.css.devPixelsPerPx", currentDPI ]] },
function() {
// Make sure we trigger a layout flush so that the frame is sized
// appropriately after the DPI changes.
iframe.getBoundingClientRect();
// Clear image cache for next run (we don't try to validate cached items
// in preload).
SpecialPowers.Cc["@mozilla.org/image/tools;1"]
Expand Down
Loading

0 comments on commit 7c34c8d

Please sign in to comment.