Skip to content

Commit

Permalink
Bug 1663562. Call MaybeReflowForInflationScreenSizeChange when any pr…
Browse files Browse the repository at this point in the history
…efs that could affect font inflation change. r=kats

When I start setting the pref ui.useOverlayScrollbars in bug 1663537 we trigger this assert

```
###!!! ASSERTION: can't mark frame dirty during reflow: '!mIsReflowing', file /builds/worker/checkouts/gecko/layout/base/PresShell.cpp, line 2677
#1: mozilla::PresShell::MaybeReflowForInflationScreenSizeChange() [layout/base/PresShell.cpp:11148]
#2: mozilla::PresShell::CompleteChangeToVisualViewportSize() [layout/base/PresShell.cpp:11177]
#03: MobileViewportManager::UpdateVisualViewportSize(mozilla::gfx::IntSizeTyped<mozilla::ScreenPixel> const&, mozilla::gfx::ScaleFactor<mozilla::CSSPixel, mozilla::ScreenPixel> const&) [layout/base/MobileViewportManager.cpp:504]
#04: MobileViewportManager::RefreshVisualViewportSize() [layout/base/MobileViewportManager.cpp:557]
#05: nsHTMLScrollFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) [layout/generic/nsGfxScrollFrame.cpp:1340]
#06: nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, int, int, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*) [layout/generic/nsContainerFrame.cpp:1115]
#07: mozilla::ViewportFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) [layout/generic/ViewportFrame.cpp:297]
#08: mozilla::PresShell::DoReflow(nsIFrame*, bool, mozilla::OverflowChangedTracker*) [layout/base/PresShell.cpp:9650]
#09: mozilla::PresShell::ProcessReflowCommands(bool) [layout/base/PresShell.cpp:9816]
#10: mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) [layout/base/PresShell.cpp:4239]
#11: nsRefreshDriver::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) [layout/base/nsRefreshDriver.cpp:2139]
```

This happens after the test is finish when we unset the ui.useOverlayScrollbars pref which (I'm assuming because it must) causes reflow. When running a font-inflation related reftest we also unset the font inflation related prefs that were specified in the reftest.list file. This causes font-inflation to go from enabled to disabled and we detect that for the first time while reflowing the scroll frame.

Instead we should reflow when any pref that could affect font inflation is changed. I scanned the font-inflation code in PresShell and Document::GetViewportInfo for prefs are consulted, but I didn't go a super exhaustive search.

Differential Revision: https://phabricator.services.mozilla.com/D89409

UltraBlame original commit: cb92660b87e5a63ed15e7c2f6b47f03a549d82c6
  • Loading branch information
marco-c committed Sep 16, 2020
1 parent 37be8fe commit 5e969b3
Showing 1 changed file with 12 additions and 1 deletion.
13 changes: 12 additions & 1 deletion layout/base/nsPresContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -269,11 +269,13 @@ static const char* gExactCallbackPrefs[] = {
"nglayout.debug.paint_flashing",
"nglayout.debug.paint_flashing_chrome",
"intl.accept_languages",
"dom.meta-viewport.enabled",
nullptr,
};

static const char* gPrefixCallbackPrefs[] = {
"font.", "browser.display.", "bidi.", "gfx.font_rendering.", nullptr,
"font.", "browser.display.", "browser.viewport.",
"bidi.", "gfx.font_rendering.", nullptr,
};

void nsPresContext::Destroy() {
Expand Down Expand Up @@ -497,6 +499,15 @@ void nsPresContext::PreferenceChanged(const char* aPrefName) {
}
return;
}

if (StringBeginsWith(prefName, "browser.viewport."_ns) ||
StringBeginsWith(prefName, "font.size.inflation."_ns) ||
prefName.EqualsLiteral("dom.meta-viewport.enabled")) {
if (mPresShell) {
mPresShell->MaybeReflowForInflationScreenSizeChange();
}
}



if (prefName.EqualsLiteral("layout.css.prefers-contrast.enabled") ||
Expand Down

0 comments on commit 5e969b3

Please sign in to comment.