Enable server side target switching
Categories
(DevTools :: Framework, task)
Tracking
(Fission Milestone:MVP, 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)
(deleted),
text/x-phabricator-request
|
Details |
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
Reporter | ||
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Updated•3 years ago
|
Comment 1•3 years ago
|
||
The navigateTo call just before these assertions with always introduce a target switching
when server targets are enabled.
Updated•3 years ago
|
Comment 2•3 years ago
|
||
Oops, attached the patch to the wrong bug.
Comment 3•3 years ago
|
||
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.
Comment 4•3 years ago
|
||
Also accept tests which were still failing with fission
that are now fixed with the enabling of server targets.
Comment 5•3 years ago
|
||
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).
Comment 6•3 years ago
|
||
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.
Comment 7•3 years ago
|
||
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.
Comment 8•3 years ago
|
||
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.
Updated•3 years ago
|
Comment 9•3 years ago
|
||
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...)
Assignee | ||
Comment 10•3 years ago
|
||
Updated•3 years ago
|
Comment 11•3 years ago
|
||
Comment 12•3 years ago
|
||
Backed out changeset c734901eec31 (Bug 1702715) for causing bc failures on browser_ext_devtools_network_targetSwitch.js.
Backout link
Push with failures
Failure Log
Reporter | ||
Updated•3 years ago
|
Comment 14•3 years ago
|
||
Comment 15•3 years ago
|
||
bugherder |
Updated•3 years ago
|
Assignee | ||
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Description
•