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
#01: mozilla::PresShell::MaybeReflowForInflationScreenSizeChange() [layout/base/PresShell.cpp:11148]
#2: mozilla::PresShell::CompleteChangeToVisualViewportSize() [layout/base/PresShell.cpp:11177]
#3: MobileViewportManager::UpdateVisualViewportSize(mozilla::gfx::IntSizeTyped<mozilla::ScreenPixel> const&, mozilla::gfx::ScaleFactor<mozilla::CSSPixel, mozilla::ScreenPixel> const&) [layout/base/MobileViewportManager.cpp:504]
#4: MobileViewportManager::RefreshVisualViewportSize() [layout/base/MobileViewportManager.cpp:557]
#5: nsHTMLScrollFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) [layout/generic/nsGfxScrollFrame.cpp:1340]
#6: nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, int, int, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*) [layout/generic/nsContainerFrame.cpp:1115]
#7: mozilla::ViewportFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) [layout/generic/ViewportFrame.cpp:297]
#8: mozilla::PresShell::DoReflow(nsIFrame*, bool, mozilla::OverflowChangedTracker*) [layout/base/PresShell.cpp:9650]
#9: 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
  • Loading branch information
tnikkel committed Sep 15, 2020
1 parent ab59939 commit 9a66133
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();
}
}

// Changing any of these potentially changes the value of @media
// (prefers-contrast).
if (prefName.EqualsLiteral("layout.css.prefers-contrast.enabled") ||
Expand Down

0 comments on commit 9a66133

Please sign in to comment.