Closed Bug 1485661 Opened Last year Closed Last year

Convert RootClient to protocol.js front

Categories

(DevTools :: Framework, enhancement, P2)

enhancement

Tracking

(firefox65 fixed)

RESOLVED FIXED
Firefox 65
Tracking Status
firefox65 --- fixed

People

(Reporter: ochameau, Assigned: ochameau)

References

(Blocks 1 open bug)

Details

(Whiteboard: dt-fission)

Attachments

(10 files, 4 obsolete files)

46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
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)
Severity: normal → enhancement
Priority: -- → P2
Blocks: 1495657
Depends on: 1497292
Depends on: 1497644
Assignee: nobody → poirot.alex
MozReview-Commit-ID: 1FfeXpVNYTk
MozReview-Commit-ID: 7BJ9qZYP2YO

Depends on D8820
MozReview-Commit-ID: 1CSeNU9Nfll

Depends on D8821
MozReview-Commit-ID: 4KinTpk0LY5

Depends on D8822
MozReview-Commit-ID: 8LRQqXYXe9W

Depends on D8823
MozReview-Commit-ID: 8LcTfats8c8

Depends on D8824
Attached file Bug 1485661 - Special test fixes. (obsolete) —
MozReview-Commit-ID: EqyijR2Bo1K

Depends on D8825
MozReview-Commit-ID: 2jEjUPHg34t

Depends on D8826
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
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
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
* 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
Attachment #9017441 - Attachment is obsolete: true
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.
MozReview-Commit-ID: Eh7f5DeG73j
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.
Depends on: 1500068
Depends on: 1500070
Depends on: 1500079
Attachment #9017558 - Attachment is obsolete: true
Attachment #9017559 - Attachment is obsolete: true
Attachment #9018220 - Attachment is obsolete: true
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
Whiteboard: dt-fission
Pushed by apoirot@mozilla.com:
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
You need to log in before you can comment on or make changes to this bug.