Bug 1216555 - Makes client.close() failsafe by handling correctly closing connections. r=jryans
authorAlexandre Poirot <poirot.alex@gmail.com>
Thu, 22 Oct 2015 08:06:16 -0700
changeset 304176 727c6dc124c784d26db6d17879af243cbce016b9
parent 304175 d91e6b148b9077e2695aee5eee744b901e1a8515
child 304177 43d38fbbc50e84d9b9d9217e9346bf354918a91e
push id1001
push userraliiev@mozilla.com
push dateMon, 18 Jan 2016 19:06:03 +0000
treeherdermozilla-release@8b89261f3ac4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjryans
bugs1216555
milestone44.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1216555 - Makes client.close() failsafe by handling correctly closing connections. r=jryans
devtools/server/tests/unit/test_client_close.js
devtools/server/tests/unit/xpcshell.ini
devtools/shared/client/main.js
new file mode 100644
--- /dev/null
+++ b/devtools/server/tests/unit/test_client_close.js
@@ -0,0 +1,39 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+var gClient;
+var gDebuggee;
+
+function run_test()
+{
+  initTestDebuggerServer();
+  gDebuggee = testGlobal("test-1");
+  DebuggerServer.addTestGlobal(gDebuggee);
+
+  let transport = DebuggerServer.connectPipe();
+  gClient = new DebuggerClient(transport);
+  gClient.connect(function(aType, aTraits) {
+    attachTestTab(gClient, "test-1", function(aReply, aTabClient) {
+      test_close(transport);
+    });
+  });
+  do_test_pending();
+}
+
+function test_close(aTransport)
+{
+  // Check that, if we fake a transport shutdown
+  // (like if a device is unplugged)
+  // the client is automatically closed,
+  // and we can still call client.close.
+  let onClosed = function() {
+    gClient.removeListener("closed", onClosed);
+    ok(true, "Client emitted 'closed' event");
+    gClient.close(function() {
+      ok(true, "client.close() successfully called its callback");
+      do_test_finished();
+    });
+  }
+  gClient.addListener("closed", onClosed);
+  aTransport.close();
+}
--- a/devtools/server/tests/unit/xpcshell.ini
+++ b/devtools/server/tests/unit/xpcshell.ini
@@ -262,8 +262,9 @@ reason = bug 1014071
 [test_setBreakpoint-on-line.js]
 [test_setBreakpoint-on-line-in-gcd-script.js]
 [test_setBreakpoint-on-line-with-multiple-offsets.js]
 [test_setBreakpoint-on-line-with-multiple-statements.js]
 [test_setBreakpoint-on-line-with-no-offsets.js]
 [test_setBreakpoint-on-line-with-no-offsets-at-end-of-script.js]
 [test_setBreakpoint-on-line-with-no-offsets-in-gcd-script.js]
 [test_safe-getter.js]
+[test_client_close.js]
--- a/devtools/shared/client/main.js
+++ b/devtools/shared/client/main.js
@@ -351,33 +351,48 @@ DebuggerClient.prototype = {
    *        If specified, will be called when the debugging connection
    *        has been closed.
    */
   close: function (aOnClosed) {
     // Disable detach event notifications, because event handlers will be in a
     // cleared scope by the time they run.
     this._eventsEnabled = false;
 
+    let cleanup = () => {
+      this._transport.close();
+      this._transport = null;
+    };
+
+    // If the connection is already closed,
+    // there is no need to detach client
+    // as we won't be able to send any message.
+    if (this._closed) {
+      cleanup();
+      if (aOnClosed) {
+        aOnClosed();
+      }
+      return;
+    }
+
     if (aOnClosed) {
       this.addOneTimeListener('closed', function (aEvent) {
         aOnClosed();
       });
     }
 
     // Call each client's `detach` method by calling
     // lastly registered ones first to give a chance
     // to detach child clients first.
     let clients = [...this._clients.values()];
     this._clients.clear();
     const detachClients = () => {
       let client = clients.pop();
       if (!client) {
         // All clients detached.
-        this._transport.close();
-        this._transport = null;
+        cleanup();
         return;
       }
       if (client.detach) {
         client.detach(detachClients);
         return;
       }
       detachClients();
     };