Open Bug 1741927 Opened 3 years ago Updated 1 year ago

Remove non-EFT codepath

Categories

(DevTools :: Framework, task)

task

Tracking

(Not tracked)

ASSIGNED

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.

With EFT hitting release this week I think it's a good time to look into this :)

Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED

Would be nice to fix the regression with frames & framesets before removing the pref: Bug 1755266

Depends on: 1755266

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: nchevobbe → poirot.alex
Blocks: 1763588

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.

Depends on: 1764102

From both frontend and server sides.

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.

Depends on: 1754452
Depends on: 1625939
Depends on: 1785266
Blocks: 1755002
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: