Closed Bug 1506549 Opened 6 years ago Closed 6 years ago

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

Categories

(DevTools :: Framework, enhancement, P2)

enhancement

Tracking

(Fission Milestone:M4, firefox65 fixed)

RESOLVED FIXED
Firefox 65
Fission Milestone M4
Tracking Status
firefox65 --- fixed

People

(Reporter: ochameau, Assigned: ochameau)

References

Details

(Whiteboard: dt-fission)

Attachments

(6 files)

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.
MozReview-Commit-ID: EGWYEmAkbtr
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

Retroactively moving fixed bugs whose summaries mention "Fission" (or other Fission-related keywords) but are not assigned to a Fission Milestone to an appropriate Fission Milestone.

This will generate a lot of bugmail, so you can filter your bugmail for the following UUID and delete them en masse:

0ee3c76a-bc79-4eb2-8d12-05dc0b68e732

Fission Milestone: --- → M4
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: