Convert WebConsoleClient to protocol.js front

RESOLVED FIXED in Firefox 67

Status

enhancement
P1
normal
RESOLVED FIXED
Last year
Last month

People

(Reporter: ochameau, Assigned: yulia)

Tracking

(Depends on 1 bug, Blocks 3 bugs)

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

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(14 attachments, 12 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
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
WebConsoleClient is the client class to connect to the console actor.
We should convert that client class to a protocol.js front.

It may help us later in fission ongoing work to more easily manage multiple console fronts.

Note that this can be done before converting the actor to protocol.js. Both refactoring are independant.
Whiteboard: [boogaloo-mvp]
Depends on: 1500000
Priority: P3 → P2
Whiteboard: [boogaloo-mvp] → dt-fission
Whiteboard: dt-fission → [boogaloo-mvp]
Blocks: 1441711
Assignee: nobody → ystartsev
Status: NEW → ASSIGNED
Priority: P2 → P1
Depends on D14393
Depends on D14394
Depends on D14395
Depends on D14396
Attachment #9031075 - Attachment is obsolete: true
Attachment #9031076 - Attachment is obsolete: true
Attachment #9031085 - Attachment is obsolete: true
Attachment #9031078 - Attachment is obsolete: true
Attachment #9031080 - Attachment is obsolete: true
Attachment #9031082 - Attachment is obsolete: true
Attachment #9031083 - Attachment is obsolete: true
Attachment #9031077 - Attachment is obsolete: true
Attachment #9031079 - Attachment is obsolete: true
Attachment #9031081 - Attachment is obsolete: true
Attachment #9031084 - Attachment is obsolete: true
Depends on: 1518777
Blocks: 1519082
Depends on: 1519102
Depends on: 1519107
Depends on: 1519133
Whiteboard: [boogaloo-mvp]

initial move of the webconsoleClient to a front. Events do not work yet, but methods do.

this patch introduces events for the webconsoleFront -- not all of them are being listened
to by their subscribers yet, but this is the bulk of the effort to move events to Protocol.js

this migrates the proxy to the new webconsole front events. This resulted in a number of
test failures once the migration was finished, and those were fixed

We have an unusual intermittent: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8533e14e032856ab5308f17b4a0fe2e5297fa07e&selectedJob=226504889

The issue is in d6, devtools/client/webconsole/test/mochitest/browser_webconsole_eval_in_debugger_stackframe.js

it can be reproduced by applying this patch queue, and then running the above test in a debug build with the following

./mach mochitest devtools/client/webconsole/test/mochitest/browser_webconsole_eval_in_debugger_stackframe.js --verify --headless

I haven't tried it with other machines, but it is reproducible on mac in a debug build. It looks like it happens frequently on windows debug builds as well.

I have traced the issue to here: https://searchfox.org/mozilla-central/source/devtools/server/actors/webconsole/eval-with-debugger.js#132

The string we pass in is : foo = 'abba'; foo3 = 'bug783499'; foo + foo3
The expected result is : “abbabug783499”
Instead we are seeing: "fooFirstCallfoo3FirstCall"

Blocks: 1526886
Pushed by ystartsev@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1ab1ab7f3bd6
remove unused inspectObjectProperties; r=ochameau
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Keywords: leave-open
Blocks: 1520835

Some more information about the destroy path problem:

before, we were running the code to remove clients (webconsole client and threadclient) first on the debugger client
https://searchfox.org/mozilla-central/rev/ee40541496d3ad738097eebadaf4965ca1343b7a/devtools/shared/client/debugger-client.js#234-252

then we would do the cleanup if there was a problem or a sudden disconnection
https://searchfox.org/mozilla-central/rev/ee40541496d3ad738097eebadaf4965ca1343b7a/devtools/shared/client/debugger-client.js#812-831

now, the destroy path is a little different
we inform the target to start destroying, but we don’t have a mechanism for waiting for it to destroy. The webconsole front is now treated as any other front.

We don't wait for the webconsole to close, instead we inform the targets that the client has closed: https://searchfox.org/mozilla-central/rev/ee40541496d3ad738097eebadaf4965ca1343b7a/devtools/shared/client/debugger-client.js#808
via this listener: https://searchfox.org/mozilla-central/rev/ee40541496d3ad738097eebadaf4965ca1343b7a/devtools/shared/fronts/targets/target-mixin.js#391

and the target starts destroying fronts, asynchronously (since the fronts are loaded async) : https://searchfox.org/mozilla-central/rev/ee40541496d3ad738097eebadaf4965ca1343b7a/devtools/shared/fronts/targets/target-mixin.js#475-478

so, simultaneously, we start destroying things in the pool, because there is no way to wait for the fronts: https://searchfox.org/mozilla-central/rev/ee40541496d3ad738097eebadaf4965ca1343b7a/devtools/shared/client/debugger-client.js#830

Attachment #9041769 - Attachment description: Bug 1485664 - remove echoed events; r=ochameau → Bug 1485664 - remove echoed events and unused events; r=ochameau
Attachment #9043939 - Attachment description: Bug 1485664 - Fix failing test due to race; r=ochameau → Bug 1485664 - Fix stackframe tests so that they wait for the debugger to have all necessary data; r=ochameau
Attachment #9043939 - Attachment description: Bug 1485664 - Fix stackframe tests so that they wait for the debugger to have all necessary data; r=ochameau → Bug 1485664 - Fix failing test due to race; r=ochameau
Attachment #9041779 - Attachment description: Bug 1485664 - do not use a custom startListeners call for the console; r=ochameau → Bug 1485664 - Change "attachConsole" in the debuggerClient so that it is compatible with a front; r=ochameau
Attachment #9044621 - Attachment description: Bug 1485664 - test fixes for getting client via getfront; r=ochameau → Bug 1485664 - guard webconsole destroy method from being called when there is no client; r=ochameau
Pushed by ystartsev@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b1392e0b33f7
convert webconsoleClient to front; r=ochameau
https://hg.mozilla.org/integration/autoland/rev/feb85ff28604
remove echoed events and unused events; r=ochameau
https://hg.mozilla.org/integration/autoland/rev/c1b5af58756f
adjust events to front events; r=ochameau
https://hg.mozilla.org/integration/autoland/rev/b7535075b575
update har collector to use webconsole events; r=ochameau
https://hg.mozilla.org/integration/autoland/rev/f3acf0651a2c
adjust webconsole-connection-proxy to listen to webconsole front events, and fix r=ochameau
https://hg.mozilla.org/integration/autoland/rev/a6be93699bf6
use protocol js methods instead of hard coding it in the console front; r=ochameau
https://hg.mozilla.org/integration/autoland/rev/a895bb6fe591
update console stub generators; r=ochameau
https://hg.mozilla.org/integration/autoland/rev/fac93d4a57ba
remove from field from jsterm tests; r=ochameau
https://hg.mozilla.org/integration/autoland/rev/4ce9378a9784
Change "attachConsole" in the debuggerClient so that it is compatible with a front; r=ochameau
https://hg.mozilla.org/integration/autoland/rev/1cbd54c219ca
move webconsole client to front directory; r=ochameau
https://hg.mozilla.org/integration/autoland/rev/9544d90c6e2d
Fix failing test due to race; r=ochameau
https://hg.mozilla.org/integration/autoland/rev/fa0e2e873b16
remove attachConsole from the debugger client; r=ochameau
https://hg.mozilla.org/integration/autoland/rev/6fc5d53dd6d3
guard webconsole destroy method from being called when there is no client; r=ochameau
Status: REOPENED → RESOLVED
Closed: 6 months ago6 months ago
Keywords: leave-open
Resolution: --- → FIXED
Depends on: 1530554

Sorry for the clutter here, I just filed a separate bug for the worker evaluation problem.

Flags: needinfo?(ystartsev)
Flags: needinfo?(poirot.alex)
Attachment #9046549 - Attachment is obsolete: true
Blocks: 1566850
You need to log in before you can comment on or make changes to this bug.