Bug 1571703 - Close DebuggerClient's opened by about:debugging toolboxes. r=jdescottes

When debugging firefox instance, we open about:devtools toolboxes,
without any remoteId attribute. So that we create a brand new DebuggerClient
instance. For now, this instance is only ever closed if the debugged target
is a local tab. We should try to close the clients for all cases.
Remote debugging clients are ignored and I think are still left opened.

Differential Revision: https://phabricator.services.mozilla.com/D47112

--HG--
extra : moz-landing-system : lando

devtools/client/framework/target-from-url.js

@@ -54,8 +54,9 @@ exports.targetFromURL = async function targetFromURL(url) {
   const type = params.get("type");
   const chrome = params.has("chrome");
 
+  let target;
   try {
-    return await _targetFromURL(client, id, type, chrome);
+    target = await _targetFromURL(client, id, type, chrome);
   } catch (e) {
     if (!isCachedClient) {
       // If the client was not cached, then the client was created here. If the target
@@ -64,6 +65,15 @@ exports.targetFromURL = async function targetFromURL(url) {
     }
     throw e;
   }
+
+  // If this isn't a cached client, it means that we just created a new client
+  // in `clientFromURL` and we have to destroy it at some point.
+  // In such case, force the Target to destroy the client as soon as it gets
+  // destroyed. This typically happens only for about:debugging toolboxes
+  // opened for local Firefox's targets.
+  target.shouldCloseClient = !isCachedClient;
+
+  return target;
 };
 
 async function _targetFromURL(client, id, type, chrome) {

devtools/client/framework/toolbox-init.js

@@ -97,6 +97,7 @@ async function initToolbox(url, host) {
     if (url.searchParams.has("target")) {
       // Attach toolbox to a given browser iframe (<xul:browser> or <html:iframe
       // mozbrowser>) whose reference is set on the host iframe.
+      // Note that so far, this is no real usage of it. It is only used by a test.
 
       // `iframe` is the targeted document to debug
       let iframe = host.wrappedJSObject
@@ -121,6 +122,8 @@ async function initToolbox(url, host) {
       await client.connect();
       // Creates a target for a given browser iframe.
       target = await client.mainRoot.getTab({ tab });
+      // Instruct the Target to automatically close the client on destruction.
+      target.shouldCloseClient = true;
     } else {
       target = await targetFromURL(url);
       const toolbox = gDevTools.getToolbox(target);

devtools/shared/fronts/targets/target-mixin.js

@@ -453,10 +453,13 @@ function TargetMixin(parentClass) {
 
         this.threadFront = null;
 
-        if (this.isLocalTab) {
+        if (this.isLocalTab || this.shouldCloseClient) {
-          // We started with a local tab and created the client ourselves, so we
+          // Local tab targets are typically instantiated from TargetFactory.
-          // should close it. Ignore any errors while closing, since there is
+          // And we ought to destroy their client at some point. We do it from here.
-          // not much that can be done at this point.
+          // There is also the clients created by about:debugging toolboxes opened
+          // for local Firefox's targets, which sets the `shouldCloseClient` attribute.
+          // Ignore any errors while closing, since there is not much that can be done
+          // at this point.
           try {
             await this._client.close();
           } catch (e) {