1675908 - linux64 perf_reftest_singletons scrollbar-styles-1.html regression 2020-10-12

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Karl Tomlinson (:karlt)]

https://bugzilla.mozilla.org/show_bug.cgi?id=1554850#c22 reports a regression in scrollbar-styles-1.html.

The autoland graph is not so helpful identifying the regressing changeset because the linux64-shippable builds are not deterministic, and so two shippable builds are not necessarily directly comparable. Multiple builds from the same source code may give different results. AIUI there are no other linux64 build options with talos support on try. With respect to scrollbar-styles-1.html, however, it seems there are two common builds produced, which I'll call "fast" and "slow" builds. I assume slow builds from one changeset may be compared against slow builds from another changeset. The bimodality in bloom-basic.html is extreme across two builds on the same code, so I'll ignore that.

This regression from changeset d85aaf27109c is detected in both fast and slow builds.

Comment 1

[Karl Tomlinson (:karlt)]

Skipping both gdk_screen_get_default() and gtk_settings_get_for_screen() reduces the regression, but does not restore to previous results for when comparing fast builds builds. (I don't have a slow build with this optimization.)

Caching the dpi and invalidating on "resolution" notifications removes the regression in scrollbar-styles-1.html for fast and slow builds.

Comment 2

[Karl Tomlinson (:karlt)]

Attachment: Bug 1675908 restore cache for GetFontScaleDPI() r?stransky

This was removed in https://hg.mozilla.org/mozilla-central/rev/d85aaf27109c
but that led to a regression in
perf_reftest_singletons scrollbar-styles-1.html.

Comment 3

[Karl Tomlinson (:karlt)]

Attachment: Bug 1675908 refresh cached screens on resolution change r?stransky

When reducing dpi from a larger value at Firefox startup, the hamburger menu
drop-down was limited in size unnecessarily because Firefox still had the
initial concept of screen size. This derived from the use of
gfxPlatformGtk::GetFontScaleFactor() in initializing screen data.
https://searchfox.org/mozilla-central/rev/8b7aa8af652f87d39349067a5bc9c0256bf6dedc/widget/gtk/ScreenHelperGTK.cpp#151.

A RefreshScreens() on resolution change resolves that.

Depends on D96306

Comment 4

[Vincent Bernat]

I have applied both patches and everything looks fine for me!

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1554850

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f9ea8ab6f473
restore cache for GetFontScaleDPI() r=stransky
https://hg.mozilla.org/integration/autoland/rev/bbcbf95ea2b5
refresh cached screens on resolution change r=stransky

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/f9ea8ab6f473
https://hg.mozilla.org/mozilla-central/rev/bbcbf95ea2b5

Comment 8

[Vincent Bernat]

Shouldn't this land on 83 instead?

Comment 9

[Karl Tomlinson (:karlt)]

(In reply to Vincent Bernat from comment #8)

Shouldn't this land on 83 instead?

Almost everything lands on m-c first, because it will be needed for future versions, and that also provides some opportunity for potential issues to show up.

Regressions are typically considered for uplift to other branches.
This close to the 83 release, however, I don't think the benefit observed in only one subtest is worth the risk of such a late uplift.