Closed Bug 1691585 Opened 2 years ago Closed 2 years ago

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

Categories

(DevTools :: Framework, task, P3)

task

Tracking

(Fission Milestone:MVP, firefox92 fixed)

RESOLVED FIXED
92 Branch
Fission Milestone MVP
Tracking Status
firefox92 --- fixed

People

(Reporter: cpeterson, Assigned: nchevobbe)

References

(Blocks 1 open bug)

Details

(Whiteboard: dt-fission-m3-mvp)

Attachments

(1 file)

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.

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.

Component: General → Framework
Whiteboard: dt-fission-triage → dt-fission-m3-triage
Whiteboard: dt-fission-m3-triage → dt-fission-m3-mvp

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

Fission Milestone: ? → M8
Type: defect → task
Priority: -- → P3

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.

Fission Milestone: M8 → MVP
No longer blocks: fission-tests

(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

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

Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED

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.

Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/826644ec677f
[devtools] Enable browser_target_list_service_workers_navigation.js on Fission. r=jdescottes.
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 92 Branch
You need to log in before you can comment on or make changes to this bug.