Instantiate all parent/content process Targets via the BrowsingContextTargetFront/ContentProcessTargetFront rather than its form

RESOLVED FIXED in Firefox 65

Status

enhancement
P2
normal
RESOLVED FIXED
9 months ago
3 months ago

People

(Reporter: ochameau, Assigned: ochameau)

Tracking

unspecified
Firefox 65
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox65 fixed)

Details

(Whiteboard: dt-fission)

Attachments

(6 attachments)

For now, the Target for the browser toolbox or browser content toolbox are instantiated via the form of the BrowsingContextTargetActor or ContentProcessTargetActor.
Instead it should be instantiated via its front, by passing the front as Target constructor argument.
It allows to send the event through the front rather than DebuggerClient.

MozReview-Commit-ID: H8zEwAlUWDb

Depends on D11622
This was only used by test and isn't much useful.

MozReview-Commit-ID: DeIimVmMOOs

Depends on D11623
MozReview-Commit-ID: EKWTGhGo0VM

Depends on D11624
Now that this event is correctly fired on the Front rather than DebuggerClient,
we have to listen for this event on each individual front.

MozReview-Commit-ID: 9MqdHuAEr7U

Depends on D11625
MozReview-Commit-ID: BWsvAcWthnF
Comment on attachment 9024396 [details]
Bug 1506549 - Return target fronts out of RootFront.getProcess and getMainProcess.

This is the very first patch of a long serie, where I want to instantiate Target with a front rather than a form.
For content process target, I only did just and only that in bug 1506545 (https://bugzilla.mozilla.org/attachment.cgi?id=9024381).
I moved the "attachContentProcessTarget" from Target.attach to the TargetFactory.forRemoteTab callsites. In existing codebase (except for Worker target), the target front is always instantiated late, from Target.attach in one of the "attach*****Target" method.
The plan here is to have Target.attach only call the "attach" request of the given target (if it has one, content process target for example doesn't).
And so, the callsites of TargetFactory.forRemoteTab have to somehow instantiate the front.
They already have the actor's form, so they could in theory instantiate the front out of it, but it ends up being easier if we stop passing around the form, and let the original methods returning the form use the right protocol.js's specification and let protocol.js automatically create the front via marshalling layer.
That's what I'm doing in bug 1506548. But not here as getProcess may return two kind of actors/fronts: BrowsingContextTargetFront for parent process of ContentProcessTargetFront for content processes. So I'm not using specification here, but still, the RootFront.getProcess method returns the right Front and RootFront manages the returned front itself. We no longer have to register the Front in DebuggerClient.fronts Pool. It is much more natural regarding protocol.js conventions.
Comment on attachment 9024397 [details]
Bug 1506549 - correctly type workerListChanged on ContentProcessTarget spec.

I don't quite understand why it wasn't failing before... but we were missing this event.
If I don't do that, with previous patch, worker tests start failing!
Comment on attachment 9024398 [details]
Bug 1506549 - Stop returning the actor from Pool.manage.

This was only used by tests and isn't that useful.
I'm removing this as it helps a bit implementing the next patch:
  https://phabricator.services.mozilla.com/D11625
Comment on attachment 9024399 [details]
Bug 1506549 - Introduce API to listen for new child fronts of a given type.

This is clearly a fork of your current work on Target.onFront, but put on all the Fronts.

I need that for the last patch of this queue:
  https://phabricator.services.mozilla.com/D11626

In about:debugging I don't have a target object, I only have the RootFront (client.mainRoot) from which I have to listen for all ContentProcessTarget actors being created.
So I can't reuse Target.onFront, also, in its current shape, Target.onFront only allows to listen for target scoped actors.

This patch is slightly more generic:
* it allows to listen for any kind child fronts
* on any given Front, it is not restricted to Target objects/fronts

Unfortunately we can't share the code between Target and Front.
1) Target doesn't compose/inherit with Front
2) Target now always have a Front attribute (i.e. activeTab), but it is only available after Target.attach().

We will be able to share the code once Target and Target fronts are merged, or at least once Target always receive a Front in its constructor. I'm very actively working on this via these patches!
Comment on attachment 9024566 [details]
Bug 1506549 - Either pass a front or a form to Target constructor.

Actually, this patch is the very first of this patch queue.

It helps a lot simplifying the implementation of the second patch:
  https://phabricator.services.mozilla.com/D11622
  As well as bug 1506548:
    https://phabricator.services.mozilla.com/D11763

Ideally, I would like to stop exposing/using Target.form, but that's yet another quest.
Hopefully it clarifies a bit how Target.form is fetch from the front class...
Blocks: 1507075
Pushed by apoirot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4f259de6cce0
Either pass a front or a form to Target constructor. r=yulia
https://hg.mozilla.org/integration/autoland/rev/97554f465eca
Return target fronts out of RootFront.getProcess and getMainProcess. r=yulia
https://hg.mozilla.org/integration/autoland/rev/490eeba8f9f7
correctly type workerListChanged on ContentProcessTarget spec. r=yulia
https://hg.mozilla.org/integration/autoland/rev/da0d76d0e8fc
Stop returning the actor from Pool.manage. r=yulia
https://hg.mozilla.org/integration/autoland/rev/6be66dea928c
Introduce API to listen for new child fronts of a given type. r=yulia
I forgot to land the topmost patch of this queue...
Too bad lando doesn't warn you about landing part of a patch queue!
Flags: needinfo?(poirot.alex)
Pushed by apoirot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/80ebeb322e14
Either pass a front or a form to Target constructor. r=yulia
https://hg.mozilla.org/integration/autoland/rev/bd6fcbbf257d
Return target fronts out of RootFront.getProcess and getMainProcess. r=yulia
https://hg.mozilla.org/integration/autoland/rev/6aa455d0de46
correctly type workerListChanged on ContentProcessTarget spec. r=yulia
https://hg.mozilla.org/integration/autoland/rev/71da82d5b981
Stop returning the actor from Pool.manage. r=yulia
https://hg.mozilla.org/integration/autoland/rev/0ceaf305b926
Introduce API to listen for new child fronts of a given type. r=yulia
https://hg.mozilla.org/integration/autoland/rev/1fdfda9f7f12
Listen for workerListChanged on all content process target fronts. r=yulia
You need to log in before you can comment on or make changes to this bug.