1691585 - Fix and re-enable devtools/shared/resources/tests/browser_target_list_service_workers_navigation.js test for Fission

Status: RESOLVED FIXED

(DevTools :: Framework, task, P3)

Description

[Chris Peterson [:cpeterson]]

This test was disabled for Fission in bug 1641796.

https://searchfox.org/mozilla-central/rev/7067bbd8194f4346ec59d77c33cd88f06763e090/devtools/shared/resources/tests/browser.ini#60-66

[browser_target_list_service_workers_navigation.js]
skip-if = fission
# There are several issues to test TargetList navigation scenarios with fission.
# Without a toolbox linked to the target-list, the target list cannot switch
# targets. The legacy worker watchers are also not designed to support target
# switching, since they set this.target = targetList.targetFront just once in
# their constructor.

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'DevTools::Framework' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 2

[Chris Peterson [:cpeterson]]

Tracking DevTools test bugs for Fission M8 (blocking Release experiment).

Comment 3

[Chris Peterson [:cpeterson]]

Tracking DevTools Post-M8 bugs for Fission MVP milestone. They don't block the Fission Release channel experiment, but we would like them to be completed before we roll out Fission by default.

Comment 4

[Alexandre Poirot [:ochameau]]

(In reply to Chris Peterson [:cpeterson] from comment #0)

https://searchfox.org/mozilla-central/rev/7067bbd8194f4346ec59d77c33cd88f06763e090/devtools/shared/resources/tests/browser.ini#60-66

[browser_target_list_service_workers_navigation.js]
skip-if = fission
# There are several issues to test TargetList navigation scenarios with fission.
# Without a toolbox linked to the target-list, the target list cannot switch
# targets. The legacy worker watchers are also not designed to support target
# switching, since they set this.target = targetList.targetFront just once in
# their constructor.

This comment is still relevant and highlight what is wrong in the current code.
The legacy (service) workers listener doesn't support target switching.
But today, we probably have the right APIs to fix that and start using watchTargets, like here:
https://searchfox.org/mozilla-central/source/devtools/shared/commands/target/legacy-target-watchers/legacy-workers-watcher.js#145-149

Comment 5

[Alexandre Poirot [:ochameau]]

Note that this will touch the same code as bug 1715908.

Comment 6

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1691585 - [devtools] Enable browser_target_list_service_workers_navigation.js on Fission. r=jdescottes.

There were a few issues when Fission was being enabled:

1. the sw legacy listener was throwing when trying to compute the origin of the
   "current" target in _onProcessAvailable. That's because the function might be
   called while the "old" target was being destroyed, in which case its url property
   is nullified. We can't simply use targetCommand.target though, as we might
   be notified about the new process being created before about the new frame
   document. The fix is to store a currentTargetURL property in the sw legacy listener,
   and to update it when we receive will-navigate events.

2. A few functions were ignoring the targetCommand.destroyServiceWorkersOnNavigation
   flag and clearing caches when doing a target switch. This meant that we might
   not be notified about sw targets being unregistered after multiple navigations.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/826644ec677f
[devtools] Enable browser_target_list_service_workers_navigation.js on Fission. r=jdescottes.

Comment 9

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/826644ec677f