Enable WS Monitor in Nightly
Categories
(DevTools :: Netmonitor, task, P2)
Tracking
(firefox70 fixed)
Tracking | Status | |
---|---|---|
firefox70 | --- | fixed |
People
(Reporter: Honza, Assigned: tanhengyeow)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review |
Let's enable WS monitor in Nightly since it's already usable and we are at the beginning of the cycle. This way we can get more testers and feedback.
Honza
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Comment 1•5 years ago
|
||
This bug 1562839 can be used as an example of how to enable a feature only in Nightly
Changesets:
https://hg.mozilla.org/mozilla-central/rev/5289c222f0d0
Honza
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Assignee | ||
Comment 2•5 years ago
|
||
Enable WS Monitor in Nightly
Reporter | ||
Updated•5 years ago
|
Comment 4•5 years ago
|
||
Backed out for devtools failures e.g. /browser_toolbox_tool_remote_reopen.js
backout: https://hg.mozilla.org/integration/autoland/rev/920ebfd35011bedee14ea1c6138c7afcb19d3f91
failure log 1: devtools/client/framework/test/browser_toolbox_tool_remote_reopen.js | Front for server1.conn101.child1/webSocketActor18 still held in pool! - https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=256839570&repo=autoland&lineNumber=8399
[task 2019-07-17T01:43:08.230Z] 01:43:08 INFO - TEST-UNEXPECTED-FAIL | devtools/client/framework/test/browser_toolbox_tool_remote_reopen.js | Front for server1.conn101.child1/webSocketActor18 still held in pool! -
[task 2019-07-17T01:43:08.231Z] 01:43:08 INFO - Stack trace:
[task 2019-07-17T01:43:08.233Z] 01:43:08 INFO - chrome://mochikit/content/browser-test.js:test_ok:1576
[task 2019-07-17T01:43:08.235Z] 01:43:08 INFO - chrome://mochitests/content/browser/devtools/client/framework/test/browser_toolbox_tool_remote_reopen.js:test/<:99
[task 2019-07-17T01:43:08.237Z] 01:43:08 INFO - chrome://mochitests/content/browser/devtools/client/framework/test/browser_toolbox_tool_remote_reopen.js:test:107
[task 2019-07-17T01:43:08.239Z] 01:43:08 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1383
[task 2019-07-17T01:43:08.241Z] 01:43:08 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:1209
[task 2019-07-17T01:43:08.243Z] 01:43:08 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:803
failure log 2: devtools/client/netmonitor/test/browser_net_pause.js | leaked 1 docShell(s) until shutdown https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=256836388&repo=autoland&lineNumber=10900
[task 2019-07-17T01:33:14.731Z] 01:33:14 INFO - TEST-INFO | Main app process: exit 0
[task 2019-07-17T01:33:14.732Z] 01:33:14 ERROR - TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/test/browser_net_pause.js | leaked 1 docShell(s) until shutdown
[task 2019-07-17T01:33:14.733Z] 01:33:14 INFO - TEST-INFO | devtools/client/netmonitor/test/browser_net_pause.js | docShell(s) leaked: [pid = 1205] [id = {ff186d21-556e-4133-bdf0-d1f0b4d004b6}]
[task 2019-07-17T01:33:14.737Z] 01:33:14 INFO - TEST-INFO | devtools/client/netmonitor/test/browser_net_pause.js | This test created 1 hidden window(s)
[task 2019-07-17T01:33:14.739Z] 01:33:14 INFO - TEST-INFO | devtools/client/netmonitor/test/browser_net_pause.js | This test created 1 hidden docshell(s)
Reporter | ||
Comment 5•5 years ago
|
||
Julian, there is a leak reported here:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=256839570&repo=autoland&lineNumber=8399
It says:
INFO - TEST-UNEXPECTED-FAIL | devtools/client/framework/test/browser_toolbox_tool_remote_reopen.js | Front for server1.conn101.child1/webSocketActor18 still held in pool! -
We are creating the front here:
https://searchfox.org/mozilla-central/rev/22b330ecb3edba1536a54887060cbdd09db21c59/devtools/client/netmonitor/src/connector/firefox-connector.js#139
...and destroying here:
https://searchfox.org/mozilla-central/rev/22b330ecb3edba1536a54887060cbdd09db21c59/devtools/client/netmonitor/src/connector/firefox-connector.js#181
https://searchfox.org/mozilla-central/rev/22b330ecb3edba1536a54887060cbdd09db21c59/devtools/client/netmonitor/src/connector/firefox-connector.js#103
Do you see anything missing or anything we are doing wrong?
Honza
Comment 6•5 years ago
|
||
The issue seems to come from the following code:
/**
* Close the WebSocketFront.
*
*/
destroy() {
if (!this._client) {
return null;
}
[...]
return super.destroy();
}
this._client
is not set anywhere by the websocket front, so the destroy always bails out. I think this if (!this._client)
is just a copy paste leftover and should be removed.
Assignee | ||
Comment 7•5 years ago
|
||
Thanks Julian, appreciate it!
Hi Honza, I've updated the patch to reflect this :D
Reporter | ||
Comment 8•5 years ago
|
||
Thanks Julian!
I pushed the new patch to try, but I am still seeing a leak
https://treeherder.mozilla.org/#/jobs?repo=try&revision=262817ae513e8c059c612e666f9a0c8f959a1307&selectedJob=256924639
devtools/client/netmonitor/test/browser_net_pause.js | leaked 1 docShell(s) until shutdown
Any other tips?
Honza
Comment 9•5 years ago
|
||
A general tip for leaks is to check if the destroy of your fronts and actors run to completion without errors (I simply add console logs usually).
Here the destroy of the actor crashes when calling webSocketEventService.removeListener
.
First I saw that this.targetActor.window.windowUtils.currentInnerWindowID
is not always the same between startListening
and stopListening
.
So I stored the id of the window in the actor instead of just storing a flag. But after that webSocketEventService.removeListener
was still crashing, so I simply added a try catch around it :)
https://gist.github.com/juliandescottes/9dcd14021ffed248fab3e408745b9941
This failure reproduces in local debug builds, so you can take my patch as a starting point and figure out a cleaner way of addressing this, maybe with some help from platform folks?
Reporter | ||
Comment 10•5 years ago
|
||
Thanks Julian for the analysis!
@hengyeow: I am attaching a patch that fixes the leaks for me (based on Julian's work), please push it to Phab and set me and Julian as reviewers, thanks.
Honza
Comment 11•5 years ago
|
||
Comment 13•5 years ago
|
||
bugherder |
Description
•