Make Target classes be protocol.js fronts and merge them with old clients

RESOLVED FIXED in Firefox 67

Status

enhancement
P2
normal
RESOLVED FIXED
Last year
3 months ago

People

(Reporter: jryans, Assigned: ochameau)

Tracking

(Blocks 2 bugs)

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

Firefox Tracking Flags

(firefox67 fixed)

Details

(Whiteboard: dt-fission)

User Story

We should remove the existing client-side targets:

* TabTarget
* WorkerTarget

and create "TargetFronts" for all "TargetActors" using inheritance to share common functions, similar to actor side:

* BrowsingContextTargetFront (abstract)
* FrameTargetFront
* AddonTargetFront
* WorkerTargetFront
etc.

While doing that, we will have to merge with existing client classes:
* TabClient
* WorkerClient
* AddonClient

Attachments

(9 attachments, 6 obsolete attachments)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
The "target" API has accumulated lots of cruft over the years.  We should explore a new design alongside work for Fission.

The user story field contains a potential approach.
Product: Firefox → DevTools
Depends on: 1222047
Depends on: 1485605
Rephrased this bug, as the goal is now clearer and also includes merging with old clients from devtools/shared/client, like TabClient and WorkerClient.
Summary: Redesign target API into something new → Make Target classes be protocol.js fronts and merge them with old clients
User Story: (updated)
Depends on: 1485660
Depends on: 1485676
Depends on: 1492434
Depends on: 1492830
Depends on: 1495551
Depends on: 1497470
Whiteboard: dt-fission
Depends on: 1503184
Depends on: 1503628
Depends on: 1506545
Depends on: 1506546
Depends on: 1506548
Depends on: 1506549
Depends on: 1508285
Depends on: 1514817
Depends on: 1514819
Assignee: nobody → poirot.alex
If any callback fired by DebuggerClient.close ends up calling close again, it will introduce an infinite loop.

Depends on D15826
This patch uses a mixin in order to make it so that all target fronts
compose with Target class. Target fronts do not inherit from Target class.
Instead, a mixin a created against Target and each target front.

We could have gone the other way around and have all target front to inherit from Target.
But as of today, we expect Target methods like attach to be called first and only then
the target front equivalents. So it feel more natural to call Target methods first
and have it call front methods via super.
But we could go the other way around and ensure calling Target methods from fronts early.

Depends on D15830
Depends on: 1520771
Depends on: 1520772
Depends on: 1520773
Blocks: 1520774
Blocks: 1520835
Attachment #9034699 - Attachment is obsolete: true
Attachment #9034701 - Attachment is obsolete: true

Depends on D16873

Attachment #9037301 - Attachment is obsolete: true
Blocks: 1522106

When destroying the target, Target.destroy (for local tabs) only calls DebuggerClient.close,
which isn't going to call detach. But we still do need to unregister
the tabNavigated/frameUpdate listener to prevent unecessary event from firing.

Depends on D17609

When destroying the target, Target.destroy (for local tabs) only calls DebuggerClient.close,
which isn't going to call detach. But we still do need to unregister
the tabNavigated/frameUpdate listener to prevent unecessary event from firing.

Attachment #9039130 - Attachment is obsolete: true
Pushed by apoirot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a9b181f32189
Convert browser_two_tabs.js to async. r=yulia
https://hg.mozilla.org/integration/autoland/rev/00d325ab677a
Convert canvas front instantiation to Target.getFront. r=yulia
https://hg.mozilla.org/integration/autoland/rev/d9ae7cd34c1a
Always consider that Target.activeTab is set. r=yulia
https://hg.mozilla.org/integration/autoland/rev/734646bf9829
Use Target.title to fetch target's title instead of using its form. r=yulia
https://hg.mozilla.org/integration/autoland/rev/52728b761d5d
Merge all target fronts with Target class. r=yulia,jdescottes
https://hg.mozilla.org/integration/autoland/rev/11413ebfbcaf
Remove Target.activeTab property. r=yulia
https://hg.mozilla.org/integration/autoland/rev/9ec017a91e78
Prevent netmonitor from destroying all "close" event listeners. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/e8e363f98525
Ensure removing BrowsingContextTarget front events when destroying it. r=jdescottes

This is an hotfix for: Bug 1465635 - Convert canvas front instantiation to Target.getFront.

We were registering these key shortcut twice.
A first time from devtools-startup.js and another time from Toolbox.
Both shortcut listeners were called when the toolbox was running,
leading the toolbox to reopen while we were expecting it to be closed.

Depends on D17610

Attachment #9040138 - Attachment is obsolete: true
Attachment #9040140 - Attachment is obsolete: true
Flags: needinfo?(poirot.alex)
Blocks: 1269919
Pushed by apoirot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2fe3a0ae216b
Convert browser_two_tabs.js to async. r=yulia
https://hg.mozilla.org/integration/autoland/rev/9c7fe2ba8434
Convert canvas front instantiation to Target.getFront. r=yulia
https://hg.mozilla.org/integration/autoland/rev/e6efb1191ec6
Always consider that Target.activeTab is set. r=yulia
https://hg.mozilla.org/integration/autoland/rev/fead89ec2d1b
Use Target.title to fetch target's title instead of using its form. r=yulia
https://hg.mozilla.org/integration/autoland/rev/1dfe4f2eb472
Merge all target fronts with Target class. r=yulia,jdescottes
https://hg.mozilla.org/integration/autoland/rev/aff30c79fa91
Remove Target.activeTab property. r=yulia
https://hg.mozilla.org/integration/autoland/rev/cb3aef2a4aff
Prevent netmonitor from destroying all "close" event listeners. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/76ea544a5404
Ensure removing BrowsingContextTarget front events when destroying it. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/756f2a2d2017
Listen to toggle and close key short only for window host type. r=jdescottes
You need to log in before you can comment on or make changes to this bug.