1763451 - Initial start slows bookmarks toolbar if refresh rate is higher than 250Hz

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[dj2000al]

Attachment: support_text_data.txt

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:99.0) Gecko/20100101 Firefox/99.0

Steps to reproduce:

1. Install FF stable 99.0.
2. Set monitor refresh rate 251Hz or higher.
3. Launch initial start of the Firefox (no other firefox.exe must be in the memory).

FF 98.0.2 doesn't have this problem. Refresh rate 250Hz or lower = no problem. After mozregression it shows that this bug is regressed Bug 1756528.
https://www.reddit.com/r/firefox/comments/txniv7/ff_from_990_slows_bookmarks_toolbar_with_280hz/

Actual results:

Firefox window opens with empty bookmarks toolbar (you see the toolbar but no bookmarks and "Other bookmarks" button. Then after 1 second delay bookmarks and "Other bookmarks" button appear.

Expected results:

Firefox window opens with bookmarks toolbar and instantly loads bookmarks and "Other bookmarks" button in this area.

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'Firefox::Bookmarks & History' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

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

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

Comment 3

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

:Jamie, since you are the author of the regressor, bug 1756528, could you take a look?
For more information, please visit auto_nag documentation.

Comment 4

[Mark Banner (:standard8)]

Tentatively moving across to Core/Disability Access APIs since that's where this was introduced.

Comment 5

[Markus Stange [:mstange]]

The regression range seems suspect.

If we use requestIdleCallback to fill the bookmarks toolbar, then I think we don't call those handlers unless the event loop is idle for >= 4ms. With a 250Hz animation, there are no >=4ms gaps.

Comment 6

[Kestrel]

The reported regression range includes other bugs:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=8b1c0d6df739e32fcef922e4aba4b73e11be974c&tochange=325ad2f9160cd59cffc6aee2c9924cfc36c72047

More likely regressed by Bug 1754562.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

This is a frontend bug. Idle callbacks aren't guaranteed to be run ever (if something else is busy).

But given that there is such code in the frontend, perhaps we need to limit how much the RefreshDrivers in child processes can affect idle processing in parent.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

But I guess I'll deal with this on core side anyhow.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Attachment: Bug 1763451, when refresh rate is very high, don't let it affect idle processing in the parent process too much, r=mstange

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6840c33437ae
when refresh rate is very high, don't let it affect idle processing in the parent process too much, r=mstange

Comment 11

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/6840c33437ae

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 9271667 [details]
Bug 1763451, when refresh rate is very high, don't let it affect idle processing in the parent process too much, r=mstange

Beta/Release Uplift Approval Request

• User impact if declined: Possibly higher cpu usage, or things which Firefox frontend expects to execute soon, are executed a bit later.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce: https://bugzilla.mozilla.org/show_bug.cgi?id=1764197#c3 says this is verified.
  This is a bit hard to test if one doesn't have machine with high refresh rate monitor.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This just reduces the affect bug 1754562 has.
• String changes made/needed: NA

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 9271667 [details]
Bug 1763451, when refresh rate is very high, don't let it affect idle processing in the parent process too much, r=mstange

Beta/Release Uplift Approval Request

• User impact if declined: See the beta approval. If there will be another release build, perhaps we could take this there too.
  The issue happens on not-so-common machines, which why we didn't get any bug reports during Nightly or beta cycle.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):
• String changes made/needed:

Comment 15

[Dianna Smith [:diannaS]]

Comment on attachment 9271667 [details]
Bug 1763451, when refresh rate is very high, don't let it affect idle processing in the parent process too much, r=mstange

Approved for 100.0b6

Comment 16

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/7f85cc5aefbb

Comment 17

[Donal Meehan [:dmeehan]]

Comment on attachment 9271667 [details]
Bug 1763451, when refresh rate is very high, don't let it affect idle processing in the parent process too much, r=mstange

Now in RC week for fx100, there has been no driver for a 99.0.2 release