1856449 - Updating the UI when iframes are being loaded slows down speedometer3

Status: RESOLVED FIXED

(Firefox :: General, enhancement)

Description

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

Assuming I didn't break anything too badly:
https://treeherder.mozilla.org/perfherder/compare?originalProject=try&originalRevision=bd3f2b5d947ce9e7a7bb10ce9e83c57bff8f4e0e&newProject=try&newRevision=399ac27fc59188c1d67ca203d6e72f643dacc5cf&page=1&framework=13
That 0.9% is rather major.

I believe some other browsers don't have the same behavior, and it is a bit odd to show anything in the status bar / reload|stop-button when an iframe is loading but not doing the same for fetch() or XHR etc.

Mossop, do you think you could find someone to take a look at this a bit? I know next to nothing about the relevant browser.js code, nor do I know the reasoning for the current UI (I expect the reason is just historical).

I know jesup has complained (maybe even filed a bug) about the issue from user experience point of view - the current behavior is visually a bit annoying.

Comment 1

[Brian Grinstead [:bgrins]]

To save a few clicks, the patch in question from the compare is: https://hg.mozilla.org/try/rev/84cd432dded3379582fd830f4d34c45a4a7ff4ec. I can't think of a reason offhand to show this status UI for iframe loads.

Comment 2

[Brian Grinstead [:bgrins]]

Would be interesting to check if Bug 1838040 is fixed by the patch

Comment 3

[Brian Grinstead [:bgrins]]

Would be interesting to check if Bug 1838040 is fixed by the patch

Checked in a local build, and it does seem to be fixed

Comment 4

[Brian Grinstead [:bgrins]]

Setting aside any performance implications, I think we should change the behavior:

• It’s not clear what should happen if you manage to press “stop” or how that would be beneficial
• The status information isn’t particularly actionable - it’s generally an implementation detail that would be better navigated in devtools for technical users (e.g. doesn't flicker away and the request can be inspected / debugged in a lot more detail)
• It's manifesting / appearing as bugs in cases where pages have many / repeated iframe navigations

I'd suggest we take patch similar to the one in the comments here and land it in Bug 1838040, then mark this one as dependent on that (using this one to confirm any perf implications after that one is fixed). Mossop, thoughts on that plan?

Comment 5

[Brian Grinstead [:bgrins]]

I did a try push at https://treeherder.mozilla.org/jobs?repo=try&revision=09ada14f0c94e9f078caaee8c9fd322d09a70748 to see what it's looking like.

Comment 6

[Dave Townsend [:mossop]]

(In reply to Brian Grinstead [:bgrins] from comment #4)

I'd suggest we take patch similar to the one in the comments here and land it in Bug 1838040, then mark this one as dependent on that (using this one to confirm any perf implications after that one is fixed). Mossop, thoughts on that plan?

Yes this seems like a good plan.

Comment 7

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1856449 - Do not update chrome UI for iframe loads r?mconley

This is just Olli's patch unedited. Still in the process of double
checking on try, but it appears that this doesn't break any tests
and seems to work just fine. This patch just avoids showing the
status bar and updating the stop/reload button when loading iframe
content. This should not affect top level loads, and this matches
Chrome's behavior.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6b874f1506e7
Do not update chrome UI for iframe loads r=mconley

Comment 9

[Natalia Csoregi [:nataliaCs]]

Backed out for causing almost perma failures on browser_preferences.js.

• backout: https://hg.mozilla.org/integration/autoland/rev/fb1c93363030ee78f4a3ea0da9854fb688a630b8
• push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&selectedTaskRun=a1Jx9873SN6EcAs2XQYp0Q.0&revision=6b874f1506e7256d52307d7c805b73daf98a96a7
• failure log: https://treeherder.mozilla.org/logviewer?job_id=432068474&repo=autoland&lineNumber=5588

[task 2023-10-11T03:27:06.156Z] 03:27:06     INFO - TEST-PASS | browser/tools/mozscreenshots/preferences/browser_preferences.js | A valid bounding box was found - 
[task 2023-10-11T03:27:07.394Z] 03:27:07     INFO - _onConfigurationReady
[task 2023-10-11T03:27:07.394Z] 03:27:07     INFO - Combination 02/10: prefsGeneral-browsingGroup
[task 2023-10-11T03:27:07.395Z] 03:27:07     INFO - promising prefsGeneral-browsingGroup
[task 2023-10-11T03:27:07.395Z] 03:27:07     INFO - calling prefsGeneral-browsingGroup
[task 2023-10-11T03:27:07.396Z] 03:27:07     INFO - called prefsGeneral-browsingGroup
[task 2023-10-11T03:27:28.016Z] 03:27:28     INFO - GECKO(3020) | console.error: (new Error("Polling for changes failed: Unexpected content-type \"text/plain;charset=US-ASCII\".", "resource://services-settings/remote-settings.sys.mjs", 321))
[task 2023-10-11T03:27:48.681Z] 03:27:48     INFO - Longer timeout required, waiting longer...  Remaining timeouts: 99
[task 2023-10-11T03:28:07.658Z] 03:28:07     INFO - TEST-INFO | started process screentopng
[task 2023-10-11T03:28:07.778Z] 03:28:07     INFO - TEST-INFO | screentopng: exit 0
[task 2023-10-11T03:28:07.779Z] 03:28:07     INFO - Buffered messages logged at 03:27:03
[task 2023-10-11T03:28:07.779Z] 03:28:07     INFO - Entering setup bound setup
[task 2023-10-11T03:28:07.779Z] 03:28:07     INFO - Buffered messages finished
[task 2023-10-11T03:28:07.779Z] 03:28:07     INFO - TEST-UNEXPECTED-FAIL | browser/tools/mozscreenshots/preferences/browser_preferences.js | Unexpected exception in [ prefsGeneral-browsingGroup ]: Timed out - 
[task 2023-10-11T03:28:07.779Z] 03:28:07     INFO - Stack trace:
[task 2023-10-11T03:28:07.780Z] 03:28:07     INFO - chrome://mochikit/content/browser-test.js:test_ok:1583
[task 2023-10-11T03:28:07.780Z] 03:28:07     INFO - resource://mozscreenshots/TestRunner.sys.mjs:_performCombo:417
[task 2023-10-11T03:28:07.780Z] 03:28:07     INFO - resource://mozscreenshots/TestRunner.sys.mjs:start:142
[task 2023-10-11T03:28:07.780Z] 03:28:07     INFO - chrome://mochitests/content/browser/browser/tools/mozscreenshots/preferences/browser_preferences.js:capture:15
[task 2023-10-11T03:28:07.780Z] 03:28:07     INFO - chrome://mochikit/content/browser-test.js:handleTask:1134
[task 2023-10-11T03:28:07.780Z] 03:28:07     INFO - chrome://mochikit/content/browser-test.js:_runTaskBasedTest:1206
[task 2023-10-11T03:28:07.780Z] 03:28:07     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1348
[task 2023-10-11T03:28:07.780Z] 03:28:07     INFO - chrome://mochikit/content/browser-test.js:nextTest/<:1123
[task 2023-10-11T03:28:07.780Z] 03:28:07     INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/<:1058
[task 2023-10-11T03:28:07.780Z] 03:28:07     INFO - 	Timed out
[task 2023-10-11T03:28:07.780Z] 03:28:07     INFO - Combination 03/10: prefsGeneral-connectionDialog
[task 2023-10-11T03:28:07.780Z] 03:28:07     INFO - promising prefsGeneral-connectionDialog
[task 2023-10-11T03:28:07.780Z] 03:28:07     INFO - calling prefsGeneral-connectionDialog
[task 2023-10-11T03:28:07.780Z] 03:28:07     INFO - called prefsGeneral-connectionDialog
[task 2023-10-11T03:28:35.401Z] 03:28:35     INFO - Longer timeout required, waiting longer...  Remaining timeouts: 98

This has been backed out based on this range of retriggers and backfills: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&fromchange=4199c8139be39bc8b91ba26b726cddeec2386aab&searchStr=linux%2C18.04%2Cx64%2Cwebrender%2Copt%2Cmochitests%2Cwithout%2Cfission%2Cenabled%2Ctest-linux1804-64-qr%2Fopt-browser-screenshots-nofis%2Css&selectedTaskRun=a1Jx9873SN6EcAs2XQYp0Q.0&tochange=7e0a19b6e34d23dcff89882d0e1b4655980cee2e

The pass rate is 1 out of 8 retriggers, which prompted us to do the backout.
In case this is mistaken and the failure was not actually caused by this, we'll have the patch relanded.
Thank you!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/576a3a6fd4d4
Do not update chrome UI for iframe loads r=mconley

Comment 11

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/576a3a6fd4d4