1485661 - Convert RootClient to protocol.js front

Status: RESOLVED FIXED

(DevTools :: Framework, enhancement, P2)

Description

[Alexandre Poirot [:ochameau]]

RootClient is the client class to connect to the root actor (RootActor)
We should convert that client class to a protocol.js front.

It will help use manage global actor fronts (like device and preference) via protocol.js instead of doing it manually.

For example it will allow us to remove all the workarounds `this.manage(this)` we do in fronts as they are not managed by any parent front:
https://searchfox.org/mozilla-central/rev/f2ac80ab7dbde5400a3400d463e07331194dec94/devtools/shared/fronts/device.js#15
https://searchfox.org/mozilla-central/rev/f2ac80ab7dbde5400a3400d463e07331194dec94/devtools/shared/fronts/preference.js#13
(But that work should be done in a followup as it will change mainRoot.getFront implementation/behavior)

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1485661 - Convert RootClient to protocol.js front.

MozReview-Commit-ID: 1FfeXpVNYTk

Comment 2

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1485661 - Adapt addonListChanged to new front.

MozReview-Commit-ID: 7BJ9qZYP2YO

Depends on D8820

Comment 3

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1485661 - Adapt processListChanged to new front.

MozReview-Commit-ID: 1CSeNU9Nfll

Depends on D8821

Comment 4

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1485661 - Adapt workerListChanged to new front.

MozReview-Commit-ID: 4KinTpk0LY5

Depends on D8822

Comment 5

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1485661 - Adapt serviceWorkerRegistrationListChanged to new front.

MozReview-Commit-ID: 8LRQqXYXe9W

Depends on D8823

Comment 6

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1485661 - Adapt tabListChanged to new front.

MozReview-Commit-ID: 8LcTfats8c8

Depends on D8824

Comment 7

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1485661 - Special test fixes.

MozReview-Commit-ID: EqyijR2Bo1K

Depends on D8825

Comment 8

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1485661 - Refactor all RootClient usages to use promise API instead of callback style.

MozReview-Commit-ID: 2jEjUPHg34t

Depends on D8826

Comment 9

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1485661 - Wait for all listAddons request completion in devtools/client/aboutdebugging/test/browser_addons_reload.js.

MozReview-Commit-ID: GXWHSJhi3ck

Depends on D8827

Comment 10

[Alexandre Poirot [:ochameau]]

try run that should be (almost) green:
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=468efb68c811a500b766e38b2edcda65c8e3f254

Comment 11

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1485661 - Avoid using about:debugging page for a navigation test.

Opening about:debugging may lead to pending listAddons requests.
Tests that open about:debugging should be careful to wait for the end
of these requests, which this test doesn't do.

MozReview-Commit-ID: 6YyfdW78kOS

Depends on D8828

Comment 12

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1485661 - Avoid opening Add-ons panel when a location hash is passed to select another one.

Doing this avoid loading the addons panel and doing its related requests,
which may still be pending after closing about:debugging.

MozReview-Commit-ID: LJjaE5YVgXi

Depends on D8867

Comment 13

[Alexandre Poirot [:ochameau]]

Note that, I thought I was about to get a full green try and proceed with review requests, but I'm still having a couple of failure:
https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=205948340&revision=512297e6b2ca0caa3c1fe0f50ef9f0159524254e

Comment 14

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1485661 - Special test fixes.

* browser_two_tabs.js, adapt to protocol.js exception on RDP error.
  protocol.js throws strings instead of { error, message }.
* target-from-url.js, same, it made a test fail.
* browser_toolbox_tool_remote_reopen.js, ignore root front that isn't release on
  toolbox close, but on client close.
* test_protocol_formtype.js, ensure that test root front isn't overridden
  by regular root front's spec.

MozReview-Commit-ID: HCuyZTQpaCt

Depends on D8825

Comment 15

[Alexandre Poirot [:ochameau]]

So. I had to updated a couple of existing revisions:
* https://phabricator.services.mozilla.com/D8824
  To keep listening for workerListChanged on DebuggerClient. That's because we also spawn some ContentProcessTargetActors (from RootFront.listAllWorkers), but we don't have front for them yet, so we have to listen on DebuggerClient.
  It won't necessarily be easy to keep listen for these events once they have fronts as it means that the callsite of listAllWorkers will have to retrieve the list of fronts and listen on them! We may need the list'n listen API to do that sanely...

* https://phabricator.services.mozilla.com/D8983
  I had to fix a couple of other tests. Hopefully that's all.
  More detail in the revision's comment.

* https://phabricator.services.mozilla.com/D8828
  I had to tweak even more this about:debugging test. This is quite sad... Watching for DOM updates is not enough. Sometimes the DOM don't update and we still do send listAddons requests that we have to wait for. 
  The position side effect of that is that it highlight the too high number of updates, about:debugging is doing.
  Remember that we do re-request the whole add-on list, on every update.

Comment 16

[Alexandre Poirot [:ochameau]]

https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=205948391&revision=76cebfb0d25c9922fd1ed8b6b524a732edb6a03c

Comment 17

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1485661 - Rename all "root client" to "root front".

MozReview-Commit-ID: Eh7f5DeG73j

Comment 18

[Alexandre Poirot [:ochameau]]

Last try was green before addressing last review comments.
Here is a last one, running on all platforms:
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=6fc180ebf1ea618b97b3e80b65bcf0f2cc8d7b33

I piled up yet another patch to rename all rootClient to rootFront. There was just a couple of occurrences, not enough to justify a followup.

Comment 19

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1485661 - Ensure that new about:debugging tests wait for initial load requests before proceeding with the tests.

MozReview-Commit-ID: 9ClZ2lkdq01

Comment 20

[Alexandre Poirot [:ochameau]]

I moved a couple of patches related to about:debugging (old and new) to dedicated bugs to be landed first.
But good news, with these patches, I finally get a green try:
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=066891203629245177a6b1b755b1c2764f3c21d4

Comment 21

[Alexandre Poirot [:ochameau]]

One green try run before trying to land this patch queue:
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=4fdf455c9d2714b552f46f1c63d32ca9367672e6

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/59c18bcae1e6
Convert RootClient to protocol.js front. r=yulia
https://hg.mozilla.org/integration/autoland/rev/7cab66bee823
Adapt addonListChanged to new front. r=yulia
https://hg.mozilla.org/integration/autoland/rev/b9bc2b2b60c9
Adapt processListChanged to new front. r=yulia
https://hg.mozilla.org/integration/autoland/rev/364278594660
Adapt workerListChanged to new front. r=yulia
https://hg.mozilla.org/integration/autoland/rev/8c08572362b2
Adapt serviceWorkerRegistrationListChanged to new front. r=yulia
https://hg.mozilla.org/integration/autoland/rev/a512888e47d3
Adapt tabListChanged to new front. r=yulia
https://hg.mozilla.org/integration/autoland/rev/e7049ede9e00
Special test fixes. r=yulia
https://hg.mozilla.org/integration/autoland/rev/b4fc15ff6c40
Refactor all RootClient usages to use promise API instead of callback style. r=yulia
https://hg.mozilla.org/integration/autoland/rev/82cf8e67a2dc
Wait for all listAddons request completion in devtools/client/aboutdebugging/test/browser_addons_reload.js. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/33b809a4c025
Rename all "root client" to "root front". r=yulia

Comment 23

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/59c18bcae1e6
https://hg.mozilla.org/mozilla-central/rev/7cab66bee823
https://hg.mozilla.org/mozilla-central/rev/b9bc2b2b60c9
https://hg.mozilla.org/mozilla-central/rev/364278594660
https://hg.mozilla.org/mozilla-central/rev/8c08572362b2
https://hg.mozilla.org/mozilla-central/rev/a512888e47d3
https://hg.mozilla.org/mozilla-central/rev/e7049ede9e00
https://hg.mozilla.org/mozilla-central/rev/b4fc15ff6c40
https://hg.mozilla.org/mozilla-central/rev/82cf8e67a2dc
https://hg.mozilla.org/mozilla-central/rev/33b809a4c025