Closed Bug 1485660 Opened Last year Closed Last year

Convert TabClient to a protocol.js front

Categories

(DevTools :: Framework, enhancement, P2)

enhancement

Tracking

(firefox64 fixed)

RESOLVED FIXED
Firefox 64
Tracking Status
firefox64 --- fixed

People

(Reporter: ochameau, Assigned: ochameau)

References

(Blocks 3 open bugs)

Details

(Whiteboard: dt-fission)

Attachments

(9 files)

TabClient is the client class to connect to Target actors like FrameTargetActor.
We should convert that client class to a protocol.js front.

It is going to help later merge TabTarget into that front (bug 1465635)

We may have a base target front class that is shared between all target front classes. That's because there is multiple target actors (frame, addon, process, ...) and they may end up having distinct front classes.
This work will also help getting rid of all the `this.manage(this)` workarounds:
  https://searchfox.org/mozilla-central/search?q=this.manage(this)%3B&case=false&regexp=false&path=
where we had to self-manage all the target scoped actors because they were not managed by any parent front.
Note that getting rid of this should be done in a followup as it is going to change target.getFront implementation as is independant from refactoring to a front.
Severity: normal → enhancement
Priority: -- → P2
Depends on: 1485671
Assignee: nobody → poirot.alex
Depends on: 1494305
Blocks: 1494632
Blocks: 1495551
Blocks: 1495657
Try is green except for one webextension test failure on verify:
  https://treeherder.mozilla.org/logviewer.html#?job_id=202876952&repo=try&lineNumber=3335
I imagine these patches triggers bug 1299001 on verify permanently, but it is not clear why:
[task 2018-10-02T14:02:18.325Z] 14:02:18     INFO - TEST-PASS | devtools/client/aboutdebugging/test/browser_addons_debug_webextension_popup.js | Found its debug button - 
[task 2018-10-02T14:02:18.327Z] 14:02:18     INFO - Buffered messages logged at 14:00:50
[task 2018-10-02T14:02:18.328Z] 14:02:18     INFO - Console message: Webconsole context has changed
[task 2018-10-02T14:02:18.330Z] 14:02:18     INFO - Buffered messages logged at 14:00:51
[task 2018-10-02T14:02:18.331Z] 14:02:18     INFO - TEST-PASS | devtools/client/aboutdebugging/test/browser_addons_debug_webextension_popup.js | Got the browserAction button from the browser UI - 
[task 2018-10-02T14:02:18.333Z] 14:02:18     INFO - Clicked on the browserAction button
[task 2018-10-02T14:02:18.335Z] 14:02:18     INFO - Console message: Webconsole context has changed
[task 2018-10-02T14:02:18.337Z] 14:02:18     INFO - Console message: Webconsole context has changed
[task 2018-10-02T14:02:18.339Z] 14:02:18     INFO - Console message: Webconsole context has changed
[task 2018-10-02T14:02:18.340Z] 14:02:18     INFO - Buffered messages logged at 14:01:32
[task 2018-10-02T14:02:18.342Z] 14:02:18     INFO - Longer timeout required, waiting longer...  Remaining timeouts: 1
[task 2018-10-02T14:02:18.344Z] 14:02:18     INFO - Buffered messages finished
[task 2018-10-02T14:02:18.346Z] 14:02:18     INFO - TEST-UNEXPECTED-FAIL | devtools/client/aboutdebugging/test/browser_addons_debug_webextension_popup.js | Test timed out - 

Unfortunately, I'm not able to reproduce it locally, which makes it hard to investigate it further.

In the meantime, I'll proceed with the reviews as there is a couple of things to discuss.
Especially the questions I inlined into: devtools/shared/fronts/targets/browsing-context.js, before `navigateTo` method.
Depends on: 1299001
MozReview-Commit-ID: uACM0VtJ5E
TabClient appears to be a client for any actor that inherits from browsing context target actor.
So let it be a front for that.

MozReview-Commit-ID: KmpClxJ53N7

Depends on D7457
* debugger-controller and events.js are special and require to support two cases because this is
the only production codepath that can have a TabTarget or a WorkerTarget.
Thus, leading to either TargetFront or WorkerClient on target.activeTab.
* webide.js doesn't need to listen for tabNavigated, this is redundant with tabListChanged.
* application's initializer. In case you are wondering this code can't be spawn against a WorkerTarget.
The application panel doesn't work in worker toolboxes.
* The code modified in target is in TabTarget, so we don't have to support the WorkerClient case, we always have a TargetFront here.
* I tried to update the doc file the best I can but this all feel outdated.

MozReview-Commit-ID: 2hGchebfIub

Depends on D7458
* browser_addons_debug_webextension_popup: It looks like frame-update events are now fired earlier.
I had to move the listener to an earlier step in order to make it work.
* helper_disable_cache + toolbox.js: this test wasn't correctly listening for reconfigure request's end.
  Not clear how this test was passing before without high rate of intermittent...
* test_webextension-addon-debugging-connect.html: We can no longer listen for frame-update *before* the target object is created.
  (because we now need a TabTarget object or the TargetFront and not just the DebuggerClient)

MozReview-Commit-ID: 49qvWSCn6nq

Depends on D7459
MozReview-Commit-ID: KLqx50gwSXS

Depends on D7460
MozReview-Commit-ID: FMuFBTseBIx

Depends on D7461
Blocks: 1497150
Blocks: 1497269
Attachment #9013662 - Attachment description: Bug 1485660 - Allows calling DebuggerClient.close twice in a row → Bug 1485660 - Allows calling DebuggerClient.close twice in a row.
Attachment #9013663 - Attachment description: Bug 1485660 - Avoid exceptions when the connection is closed in middle of the actor's id retrieval → Bug 1485660 - Avoid exceptions when the connection is closed in middle of the actor's id retrieval.
Attachment #9013664 - Attachment description: Bug 1485660 - Convert TabClient to a front → Bug 1485660 - Convert TabClient to a front.
Attachment #9013665 - Attachment description: Bug 1485660 - Switch from listening from DebuggerClient to TargetFront → Bug 1485660 - Switch from listening from DebuggerClient to TargetFront.
Attachment #9013666 - Attachment description: Bug 1485660 - Special test fixes → Bug 1485660 - Special test fixes.
MozReview-Commit-ID: 1dC5opkgLcQ

Depends on D7459
Attachment #9013667 - Attachment description: Bug 1485660 - Convert browser_db_listtabs-03.js to async and make it use TargetFront → Bug 1485660 - Convert browser_db_listtabs-03.js to async and make it use TargetFront.
Attachment #9013668 - Attachment description: Bug 1485660 - Remove gcli leftover → Bug 1485660 - Remove gcli leftover.
Blocks: 1497911
Pushed by apoirot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dccca441b980
Allows calling DebuggerClient.close twice in a row. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/9026679753b2
Avoid exceptions when the connection is closed in middle of the actor's id retrieval. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/3642f2d66777
Convert TabClient to a front. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/cfb160882db9
Switch from listening from DebuggerClient to TargetFront. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/40a3c2dd1b38
Adapt TabClient API to protocol one. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/9bb2f56f219d
Special test fixes. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/26e82fa62444
Convert browser_db_listtabs-03.js to async and make it use TargetFront. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/7a2f29bbe2a8
Remove gcli leftover. r=jdescottes
I can't believe I keep breaking DAMP...

New try run:
  https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=205019000&revision=c34ed013ee4b2faa6068ee0a8d11a9e3f27a4e90
Flags: needinfo?(poirot.alex)
MozReview-Commit-ID: EWka8fMBcK5

Depends on D7462
Blocks: 1499022
Pushed by apoirot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0ea045e6e57f
Allows calling DebuggerClient.close twice in a row. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/f0d97e8bb081
Avoid exceptions when the connection is closed in middle of the actor's id retrieval. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/174ed200a5d2
Convert TabClient to a front. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/8cf41a5970be
Switch from listening from DebuggerClient to TargetFront. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/09ff21b58c0b
Adapt TabClient API to protocol one. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/4ac958d94290
Special test fixes. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/278e3ec5091b
Convert browser_db_listtabs-03.js to async and make it use TargetFront. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/9b5cb8d01b72
Remove gcli leftover. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/1c7a643768d1
Adapt DebuggerClient._pools to accept Front's and (now) Pool's. r=jdescottes
Whiteboard: dt-fission
You need to log in before you can comment on or make changes to this bug.