Closed Bug 1702715 Opened 4 years ago Closed 3 years ago

Enable server side target switching

Categories

(DevTools :: Framework, task)

task

Tracking

(Fission Milestone:MVP, firefox92 wontfix, firefox93 fixed)

RESOLVED FIXED
93 Branch
Fission Milestone MVP
Tracking Status
firefox92 --- wontfix
firefox93 --- fixed

People

(Reporter: Honza, Assigned: nchevobbe)

References

(Blocks 1 open bug, Regressed 1 open bug)

Details

(Whiteboard: dt-fission-m3-mvp)

Attachments

(1 file, 2 obsolete files)

Enable server side switching for all panels

Toggle devtools.target-switching.server.enabled to true by default.

(this is cloned from bug 1698891, which is meta and so not in our backlog)

Honza

Whiteboard: dt-fission-m3-triage
Summary: Enable server side target switching for all panels → Enable server side target switching
Fission Milestone: --- → M8
Whiteboard: dt-fission-m3-triage → dt-fission-m3-mvp
Depends on: 1710077
Fission Milestone: M8 → MVP

The navigateTo call just before these assertions with always introduce a target switching
when server targets are enabled.

Assignee: nobody → poirot.alex
Status: NEW → ASSIGNED

Oops, attached the patch to the wrong bug.

Assignee: poirot.alex → nobody
Status: ASSIGNED → NEW

Comment on attachment 9232578 [details]
Bug 1702715 - [devtools] Fix browser_tab_descriptor_fission.js with server side targets.

Revision D120589 was moved to bug 1721790. Setting attachment 9232578 [details] to obsolete.

Attachment #9232578 - Attachment is obsolete: true

Also accept tests which were still failing with fission
that are now fixed with the enabling of server targets.

This depends on bug 1721823, which revise how we use the preference and should then only make it used by TabDescriptor.
So that the preference will no longer apply to about:debugging or extension toolboxes.
Meaning, we no longer have to support extension usecase to flip the pref (bug 1666534).

Depends on: 1721823
No longer depends on: 1666534

This bug is a soft blocker for Fission MVP. We'd like to fix it before our Release channel rollout, but we won't delay the rollout waiting for it.

Whiteboard: dt-fission-m3-mvp → dt-fission-m3-mvp, fission-soft-blocker

Here is a possible overview of the performance impact of server targets on DevTools:
https://treeherder.mozilla.org/perfherder/comparesubtest?originalProject=try&newProject=try&newRevision=fa84b0fe6eda9c9552cf9fdd1b1293ff9ca86dfc&originalSignature=3130913&newSignature=3130913&framework=12&originalRevision=4cc4c0addccb678dd6c55f4848d8c9521d76094d&page=1&showOnlyConfident=1

6.5% on complicated.inspector.reload
15% on custom.inspector.reload <= something to look into to see if there is any good low hanging fruits
-2% on custom.jsdebugger.pause
-2% on custom.jsdebugger.preview
9% on panelsInBackground.reload
and many changes so simple.* tests, which are slightly less important.

Depends on: 1722810

A quick overview on what's left before toggling this preference.
All necessary work is being listed in bug 1698891 in the blocking bug list. I ensured the bug list what up-to-date:
https://bugzilla.mozilla.org/showdependencytree.cgi?id=1698891&maxdepth=1&hide_resolved=1

As of right now we still have opened:

  • bug 1704029: Touch simulation isn't turned on when navigating to a different origin
    I think this bug can be closed, this has been fixed by some other bug.
  • bug 1719156: Iframe dropdown is always empty when using server side targets
    We can enable the preference without breaking any test as that's not properly covered. But that would be nice to fix before toggling.
  • bug 1721907: browser_styleeditor_loading.js fails with server side target switching enabled
    This test still fails for me locally. A patch is attached, I think the patch is nice to land as-is but mostly miss a CommandFactory test involving a loading tab.
  • bug 1721991: Intermittent devtools/shared/commands/target/tests/browser_target_list_tab_workers_bfcache_navigation.js | Uncaught exception - at chrome://mochitests/content/browser/devtools/client/shared/test/shared-head.js:761 - Error: Failed waitFor(): Wait for the
    In autoland.
  • bug 1722462: browser_device_change.js (and many other RDM tests) fail with server targets
    r+, to be landed.
  • bug 1722543: browser_dbg-windowless-service-workers.js fails with server targets
    Need investigation, but given that it covers a preffed off feature, we may disable this test. That what I do in my current stack.
  • bug 1722575: browser_inspector_inspect_node_contextmenu_nested.js fails with server targets
    Need investigation, I disabled it in my stack.
  • bug 1722663: browser_touch_pointerevents.js fails intermittently with server targets
    Attached a patch, but may not be perfect.
  • bug 1722714: browser_max_touchpoints.js fails intermittently with server targets
    Need investigation.
  • bug 1722749: browser_ext_devtools_inspectedWindow_targetSwitch.js fails with server targets
    Working on a patch.
  • bug 1722800: A few mochitest chrome tests fail with server targets
    A patch ready for review.
  • bug 1722801: test_webconsole-node-grip.html fails with server targets
    A patch, to be reviewed, not sure that's the best option.
  • bug 1722805: browser_storage_cache_navigation.js fails with server targets
    To be investigated.

With all the in-flight patches, we end up which such results on try:
https://treeherder.mozilla.org/jobs?repo=try&selectedTaskRun=PLSOdvWUSLy3XlVzFnzcMA.0&revision=e81da0d9e5cae003d679dd6656df346d6abbe2b6
browser_max_touchpoints.js (bug 1722714) and browser_storage_cache_navigation.js (bug 1722805) are still visibly failing as I don't have any patch for these tests, nor have I disabled them.
Other than that, it looks like we have many existing intermittents. It might be hard to guess which one are really related to server targets.

Attachment #9233277 - Attachment description: Bug 1702715 - [devtools] enable server side target switching → Bug 1702715 - [devtools] enable server side target switching when fission is enabled.

I just submited a patch on bug 1722810 to tweak the enabling in order to only enable server targets when fission is enabled.
It may drastically reduce the number of failures on try, as I think we would only impact browser mochitest with fission enabled.
Doing this would help enabling this codepath in Fx92 and improve our support of fission, without introducing any risk for non-fission.
We can then enable server targets for non-fission in a few weeks, when Fx93 just starts.

Here is a try run with all the patches, rebased with this change:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c2b486dbad7d48ff361efdbc700966d556ba94f4
(hopefully my patch from bug 1722810 is correct...)

Depends on: 1724989
Depends on: 1725013
Depends on: 1725362
Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED
Pushed by nchevobbe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c734901eec31 [devtools] Enable server side target switching. r=bomsy,Honza,jdescottes,ladybenko.

Backed out changeset c734901eec31 (Bug 1702715) for causing bc failures on browser_ext_devtools_network_targetSwitch.js.
Backout link
Push with failures
Failure Log

Flags: needinfo?(nchevobbe)
Depends on: 1719963
Pushed by hmanilla@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/62ee94c26f8f [devtools] Enable server side target switching. r=bomsy,Honza,jdescottes,ladybenko.
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 93 Branch
Regressions: 1726238
Regressions: 1726240
Flags: needinfo?(nchevobbe)
Regressions: 1729500
Whiteboard: dt-fission-m3-mvp, fission-soft-blocker → dt-fission-m3-mvp
Attachment #9233277 - Attachment is obsolete: true
Regressions: 1750182
Regressions: 1770110
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: