1556000 - Remove the Content Window Size Telemetry

Status: RESOLVED FIXED

(Core :: Window Management, task, P3)

Comment 1

[Tim Huang[:timhuang]]

Attachment: Bug 1556000 - Part 1: Remove the expired telemetry probe of the content window size. r?gijs!

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ef75e158b232
Remove the expired telemetry probe of the content window size. r=Gijs

Comment 3

[Tim Huang[:timhuang]]

Try looks good.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=316a461e803ea550d737ef0ff460f38d88652bb5

Comment 4

[Narcis Beleuzu [:NarcisB]]

Backed out for bc failures on browser_startup_syncIPC.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/dcfa084488a69b86a745816bb59cbe68c9f38e37
Push link: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Crunnable&selectedJob=250348726&revision=ef75e158b23219d9fde70dce371206861e7d31f4
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=250360329&repo=autoland&lineNumber=2145

Comment 5

[:Gijs (he/him)]

(In reply to Tim Huang[:timhuang] from comment #3)

Try looks good.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=316a461e803ea550d737ef0ff460f38d88652bb5

FWIW, the failures that got this backed out are in that trypush, too...

Please bump the count at https://searchfox.org/mozilla-central/source/browser/base/content/test/performance/browser_startup_syncIPC.js#62-65 to 3 instead of 2 and then reland.

Comment 6

[Tim Huang[:timhuang]]

Sorry for that I didn't catch this failure in the first place.

I tried to bump the count to 3, however, try still shows the same failure. [1]

Maybe we need to whitelist 'PCompositorBridge::Msg_MapAndNotifyChildCreated' for the stage 'before becoming idle'. But it's still unclear to me why this happened. Is it because of removing this telemetry changes the timing of the startup?

Could you give insights here, Florian?

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=dec0f7e72b5000a7fdf25810eb65193512cdbf5e&selectedJob=250404529

Comment 7

[Florian Quèze [:florian]]

(In reply to Tim Huang[:timhuang] from comment #6)

Maybe we need to whitelist 'PCompositorBridge::Msg_MapAndNotifyChildCreated' for the stage 'before becoming idle'. But it's still unclear to me why this happened. Is it because of removing this telemetry changes the timing of the startup?

Could you give insights here, Florian?

The "becoming idle" thing here was supposed to mean "once we are fully done with startup", but it's poorly implemented with a hack at https://searchfox.org/mozilla-central/rev/ee806041c6f76cc33aa3c9869107ca87cb3de371/browser/components/tests/startupRecorder.js#150

This seems to not fully cover the end of startup, especially on Windows. The sync IPC the test is reporting in the failures you experienced are related to starting content processes. I had already noticed that sometimes on Windows startup recorder stops before we have created all the content processes that are usually created during startup.

A side effect of your patch is to make the NewTabPagePreloading.maybeCreatePreloadedBrowser(window); line execute slightly earlier.

In the short term, don't worry about this and just whitelist the newly reported sync IPC.

The correct long term solution is to replace the hack in startupRecorder.js with something that waits for a promise that would be resolved once we are done with all the idle tasks running during late startup from BrowserGlue.jsm and browser.js. This will require significant work as we'll need to adjust whitelists in a bunch of startup tests; it's fine to keep this for a follow-up bug.

Comment 8

[Tim Huang[:timhuang]]

Attachment: Bug 1556000 - Part 2: Whitelist sync IPCs 'PCompositorBridge::Msg_MapAndNotifyChildCreated' and 'PCompositorBridge::Msg_NotifyChildCreated' for the test 'browser_startup_syncIPC.js'. r?florian!

We need to whistlist 'PCompositorBridge::Msg_MapAndNotifyChildCreated'
sync IPC in the stage 'before becoming idle' for Windows platform since
removing the content window telemetry probe causing this sync IPC been
sent earlier than before. And the IPC recorder sometimes doesn't fully
cover the stage in Windows platform. So, we will get this sync IPC in
the stage and we need to whitelist it.

Depends on D33788

Comment 9

[Florian Quèze [:florian]]

(In reply to Tim Huang[:timhuang] from comment #8)

Created attachment 9071218 [details]
Bug 1556000 - Part 2: Whitelist sync IPC 'PCompositorBridge::Msg_MapAndNotifyChildCreated' for the test 'browser_startup_syncIPC.js'. r?florian!

Is this enough to make the test green on try? I thought in comment 6 you were saying you need some more whitelisting. I would like to see a green try run with the TV (verify) jobs before r+'ing. Thanks!

Comment 10

[Tim Huang[:timhuang]]

No, this is not enough. I still get failure here [1].

We need to whitelist both 'PCompositorBridge::Msg_MapAndNotifyChildCreated' and 'PCompositorBridge::Msg_NotifyChildCreated' in order to get a green on try.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=a3b9bbdc083f05c379722c4e13fc5dc8df7e37b1&selectedJob=251124342

Comment 11

[Tim Huang[:timhuang]]

I updated the patch and try push[1] shows green on TV.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d3424682f0f83db935ea10658fa89aceb4805385

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e5fed2a257c4
Part 1: Remove the expired telemetry probe of the content window size. r=Gijs
https://hg.mozilla.org/integration/autoland/rev/e16ada7fe052
Part 2: Whitelist sync IPCs 'PCompositorBridge::Msg_MapAndNotifyChildCreated' and 'PCompositorBridge::Msg_NotifyChildCreated' for the test 'browser_startup_syncIPC.js'. r=florian

Comment 13

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/e5fed2a257c4
https://hg.mozilla.org/mozilla-central/rev/e16ada7fe052