1583252 - devtools mochitests should always close their DebuggerClient

Status: RESOLVED FIXED

(DevTools :: Framework, enhancement, P3)

Description

[Alexandre Poirot [:ochameau] PTO, back May 4th]

When working on bug 1565263, I realized that DebuggerClient were left opened in many tests.
Adding an explicit assertion highlights that:
https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=267925487&revision=cb1787d8176d3aae322a8da926b5a1b97b13e912

I'm wondering if this also relates to bug 1571703?
By automatically closing the clients, we may more easily hide unclosed clients in production code.

So we should probably evaluate the two options we have here:

• automatically closing the clients, versus,
• asserting unclosed clients and fixing each individual test.

Comment 1

[Alexandre Poirot [:ochameau] PTO, back May 4th]

Attachment: Bug 1583252 - Assert that there is no pending client at end of each test.

Comment 2

[Alexandre Poirot [:ochameau] PTO, back May 4th]

With the previous try run, I got many failures, some weren't relevants.
Let's see with this new one if we mostly have one related to about:debugging, which may indicate we would better fix bug 1571703 and assert rather than closing clients.
https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=267926185&revision=8891d590477494a504cdd6fd7866eaf795d81672

Comment 3

[Alexandre Poirot [:ochameau] PTO, back May 4th]

This work would actually help me track that I do not leave even more DebuggerClient opened because of bug 1565263's patch.

Comment 4

[Alexandre Poirot [:ochameau] PTO, back May 4th]

Attachment: Bug 1583252 - Shutdown content process DebuggerServer only once the last connection closes.

When multiple DebuggerClients starts debugging the same content process target,
the first one to close will destroy the other one as we destroy the server
and the loader as soon as we receive the debug:content-process-destroy message.
We should destroy them only once the last client closes.
i.e. when the last connection registered in the content process is destroyed.

Comment 5

[Alexandre Poirot [:ochameau] PTO, back May 4th]

https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=269093492&revision=83805de4f487bff473c4e4f9ebf289b8dc4f2be4

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4d362b8b7a0d
Assert that there is no pending client at end of each test. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/655923166066
Shutdown content process DebuggerServer only once the last connection closes. r=jdescottes

Comment 7

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/4d362b8b7a0d
https://hg.mozilla.org/mozilla-central/rev/655923166066