1685412 - Re-enable devtools/client/debugger/test/mochitest/browser_dbg-browser-toolbox-workers.js test for Fission

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P3)

Description

[Chris Peterson [:cpeterson]]

This devtools/client/debugger/test/mochitest/browser_dbg-toolbox-workers.js test is currently skipped for Fission (by bug 1665165):

[browser_dbg-toolbox-workers.js]
skip-if = asan || !nightly_build || fission # Bug 1591064, parent intercept mode is needed bug 1588154, Disable frequent fission intermittents Bug 1675020

https://searchfox.org/mozilla-central/rev/ef900cd2258d4c5d968093f612f807d96e6e7c98/devtools/client/debugger/test/mochitest/browser.ini#225-226

Is this test still failing with Fission?
Does fixing and re-enabling this test need to block shipping Fission MVP?

Comment 1

[Chris Peterson [:cpeterson]]

Tracking DevTools Fission bugs for Fission M7 (blocking Beta experiment).

Comment 2

[Julian Descottes [:jdescottes]]

This one is still failing frequently when enabling Fission, always when waiting for the service worker target (hasWorker("service-worker.sjs")).
There is a logged intermittent for this test, but I believe the frequency will be much higher if we reenable it for fission. We should investigate before enabling. Note that this test is about the multiprocess browser toolbox, so it might move out of the MVP.

Comment 3

[Neha Kochar [:neha]]

Honza said this is not part of dt-fission-m3-mvp and is not considered Fission M7 blocker.

Comment 4

[Chris Peterson [:cpeterson]]

Setting Fission Milestone to "Future" so our Fission triage queries will ignore this bug.

Comment 5

[Chris Peterson [:cpeterson]]

Moving "dt-fission-m3-reserve" bugs to "dt-fission-future" because they don't block Fission MVP.

Comment 6

[Chris Peterson [:cpeterson]]

I just corrected an error in my comment 0 above. This bug is about re-enabling test browser_dbg-toolbox-workers.js, not browser_dbg-bfcache.js:

[browser_dbg-toolbox-workers.js]
skip-if = asan || !nightly_build || fission # Bug 1591064, parent intercept mode is needed bug 1588154, Disable frequent fission intermittents Bug 1675020

This browser_dbg-toolbox-workers.js test is currently skipped in Beta and Release channels, even for non-Fission. The test annotation comment says the test is waiting for service worker parent interception (bug 1588154), but that was enabled back in 2019.

Comment 7

[Neha Kochar [:neha]]

Fission Future should be used for stuff that aren't issues for Fission. So bringing this back into MVP.

Comment 8

[Alexandre Poirot [:ochameau]]

This test failures don't sound related to bfcache.

At least it seems to pass with all patches from bug 1694651.
And surprisingly, for me, locally, it seems to pass with fission. (I haven't test --verify, just a couple of runs)

Comment 9

[Neha Kochar [:neha]]

Alex, can you unskip this test then?

Comment 10

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

given the comment 8 (and since this wasn't initially related to bfcache anyhow), this doesn't need to block fission-bfcache.

Comment 11

[Julian Descottes [:jdescottes]]

Note that the test was renamed to browser_dbg-browser-toolbox-workers.js

https://searchfox.org/mozilla-central/rev/b7bc94b4689a7f002c61d016c6e162e5e5708bf3/devtools/client/debugger/test/mochitest/browser.ini#229-230

Comment 12

[Chris Peterson [:cpeterson]]

This bug is currently assigned to Fission Milestone MVP, but it has a dt-fission-future whiteboard tag instead of a dt-fission-m3-mvp tag. I'm adding the dt-fission-m3-triage tag so DevTools Fission triage can decide whether this bug should be fixed for dt-fission-m3-mvp or can be postponed from Fission Milestone MVP to Future.

(In reply to Olli Pettay [:smaug] from comment #10)

given the comment 8 (and since this wasn't initially related to bfcache anyhow), this doesn't need to block fission-bfcache.

In that case, I will remove the fission-bfcache whiteboard tag.

Comment 13

[Alexandre Poirot [:ochameau]]

So the issue with this test is an intermittent race condition, which can be reproduced with --verify.

We miss service worker targets and so the source tree of the debugger in the MBT is missing the service worker sources.
This comes from this code:
https://searchfox.org/mozilla-central/rev/88cd13997fb0747cdcd78638fc762ff2d75e1fc5/devtools/shared/commands/target/legacy-target-watchers/legacy-workers-watcher.js#125
this._recordWorkerTarget(workerTarget) returns false, because the registration related to the SW isn't available yet.
What happens is that LegacyWorkersWatcher._workerListChanged is called before the registration are fetched:
This line:
https://searchfox.org/mozilla-central/rev/88cd13997fb0747cdcd78638fc762ff2d75e1fc5/devtools/shared/commands/target/legacy-target-watchers/legacy-workers-watcher.js#112

      this._processNewWorkerTarget(workerTarget, existingTargets)

is called before the upper class LegacyServiceWorkersWatcher._updateRegistrations resolves:
https://searchfox.org/mozilla-central/rev/88cd13997fb0747cdcd78638fc762ff2d75e1fc5/devtools/shared/commands/target/legacy-target-watchers/legacy-serviceworkers-watcher.js#258-262

It isn't clear yet why registrations aren't fetched before the worker targets. In theory we try to do that in the right order, for ex here:
https://searchfox.org/mozilla-central/rev/88cd13997fb0747cdcd78638fc762ff2d75e1fc5/devtools/shared/commands/target/legacy-target-watchers/legacy-serviceworkers-watcher.js#88-91

LegacyWorkersWatcher._workerListChanged seems to be called via a workerListChanged event.

Comment 14

[Alexandre Poirot [:ochameau]]

Bug 1651522 revised the implementation of service worker debugging and may have fixed this intermittent.
In bug 1907977, I used this disabled test to cover the fix, so I'll try to re-enable this test.

Comment 15

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1685412 - [devtools] Enable browser_dbg-browser-toolbox-workers.js test.

Comment 16

[Alexandre Poirot [:ochameau]]

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

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e2469838d51b
[devtools] Enable browser_dbg-browser-toolbox-workers.js test. r=devtools-reviewers,nchevobbe

Comment 18

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/e2469838d51b