1702715 - Enable server side target switching

Status: RESOLVED FIXED

(DevTools :: Framework, task)

Description

[Jan Honza Odvarko [:Honza] (always need-info? me)]

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

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1702715 - [devtools] Fix browser_tab_descriptor_fission.js with server side targets.

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

Comment 2

[Alexandre Poirot [:ochameau]]

Oops, attached the patch to the wrong bug.

Comment 3

[Phabricator Automation]

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

[Alexandre Poirot [:ochameau]]

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

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

Comment 5

[Alexandre Poirot [:ochameau]]

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

[Chris Peterson [:cpeterson]]

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

[Alexandre Poirot [:ochameau]]

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

[Alexandre Poirot [:ochameau]]

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.

Comment 9

[Alexandre Poirot [:ochameau]]

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...)

Comment 10

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1702715 - [devtools] Enable server side target switching. r=bomsy,Honza,jdescottes,ladybenko.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c734901eec31
[devtools] Enable server side target switching. r=bomsy,Honza,jdescottes,ladybenko.

Comment 12

[Marian-Vasile Laza]

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

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/62ee94c26f8f
[devtools] Enable server side target switching. r=bomsy,Honza,jdescottes,ladybenko.

Comment 15

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/62ee94c26f8f