Remove non-EFT codepath
Categories
(DevTools :: Framework, task)
Tracking
(Not tracked)
People
(Reporter: ochameau, Assigned: ochameau)
References
(Depends on 2 open bugs, Blocks 6 open bugs)
Details
Attachments
(4 files)
Once bug 1731740 settled a bit and we got rid of all tests still disabling EFT, we should consider removing the devtools.every-frame-target.enabled
preference and start removing all codepath and code branches which were kept to support code running with EFT being disabled.
In bug 1741669, I'll land code that will be worth cleaning up once that's the case.
Comment 1•3 years ago
|
||
With EFT hitting release this week I think it's a good time to look into this :)
Comment 2•3 years ago
|
||
Would be nice to fix the regression with frames & framesets before removing the pref: Bug 1755266
Assignee | ||
Comment 3•3 years ago
|
||
I started looking, there is quite a few things to do!
If we want to really start cleaning things up in the backend, we will start dropping same-process iframe inspection.
I don't know if any 3rd party tool using RDP would be really impacted. It is hard to know how much they care about iframe debugging.
VS.code currently spawns a TabDescriptorActor via listTabs and retrieve a top level target from it:
https://github.com/firefox-devtools/vscode-firefox-debug/blob/8b7c81a6360ae005a40d3e626c877e2537cd8248/src/adapter/firefox/actorProxy/root.ts#L144
So they are spawning a target actor that debugs same-process iframe.
That's because we toggle WindowGlobalTargetActor.ignoreSubFrames=true only when we spawn the target via JSWindowActor and the Watcher actor.
VS.code is also still using the WebConsoleActor to listen for messages:
https://github.com/firefox-devtools/vscode-firefox-debug/blob/8b7c81a6360ae005a40d3e626c877e2537cd8248/src/adapter/firefox/actorProxy/console.ts#L31-L38
Ideally, it would be migrated to the Watcher Actor and use resource watching methods...
Because of this I'll open a followup bug about dropping ignoreSubFrames
flag so that we can right away simplify the frontend and tests.
Note that in any case, Codepaths used by VS.code were already poorly covered, but they are going to become even less covered!
Assignee | ||
Comment 4•3 years ago
|
||
Another lesson learned while removing EFT is that WebExtension still depend on same-process debugging.
We might also have to depend on bug 1754452 to fully remove non-EFT codepaths.
Assignee | ||
Comment 5•3 years ago
|
||
Assignee | ||
Comment 6•3 years ago
|
||
Assignee | ||
Comment 7•3 years ago
|
||
From both frontend and server sides.
Assignee | ||
Comment 8•3 years ago
|
||
As this is now always true.
Assignee | ||
Comment 9•3 years ago
|
||
Quick status: the patches should be almost good to go.
But this depends on migrating WebExt entirely to EFT (bug 1754452),
as well as may be also fully deprecate the Browser Toolbox, which actually still use all that!
So. We might only be able to proceed with all these patches after the end of the browser toolbox project.
Once MBT is enabled by default and regular non-multiprocess browser toolbox is entirely removed.
(In reply to Alexandre Poirot [:ochameau] from comment #8)
Created attachment 9273400 [details]
Bug 1741927 - [devtools] Remove WindowGlobalTargetActor.ignoreSubFrames attribute.As this is now always true.
Per comment 3, this patch should probably be kept for a followup in order to avoid breaking same-process iframe inspection in vscode, or other 3rd party tools.
(In reply to Alexandre Poirot [:ochameau] from comment #7)
Created attachment 9273399 [details]
Bug 1741927 - [devtools] Remove WindowGlobalTarget frame picking feature.From both frontend and server sides.
In this patch, I missed removing switchToFrame
method:
https://searchfox.org/mozilla-central/rev/6da1ebe13b260efabd88eb98dec5fa8ee65987b2/devtools/server/actors/targets/window-global.js#803-825
I think it is fine cleaning up server codebase as I don't think that 3rd party tools would be using any of this.
Description
•