Bug 912646 - Part 2: Leave client of remote tab open. r=dcamp
authorJ. Ryan Stinnett <jryans@gmail.com>
Fri, 06 Sep 2013 17:37:18 -0500
changeset 146129 84caf75159b2505779228b0dc71bd3de1b915f1f
parent 146128 bd1111b18be1147eaf1b6222b586481e4a216c35
child 146130 e89edb607559cd9cd0ae32e5fb24f0791ca02c96
push id25242
push useremorley@mozilla.com
push dateMon, 09 Sep 2013 12:13:52 +0000
treeherdermozilla-central@218d4334d29e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdcamp
bugs912646
milestone26.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 912646 - Part 2: Leave client of remote tab open. r=dcamp * * * [mq]: leave-remote-open * * * imported patch add-close-calls * * * [mq]: remain-con-test
browser/devtools/app-manager/test/Makefile.in
browser/devtools/app-manager/test/test_remain_connected.html
browser/devtools/framework/connect/connect.js
browser/devtools/framework/target.js
browser/devtools/framework/toolbox.js
--- a/browser/devtools/app-manager/test/Makefile.in
+++ b/browser/devtools/app-manager/test/Makefile.in
@@ -2,10 +2,11 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 MOCHITEST_CHROME_FILES = \
 		test_template.html \
 		test_connection_store.html \
 		test_device_store.html \
 		test_projects_store.html \
+		test_remain_connected.html \
 		hosted_app.manifest \
 		$(NULL)
new file mode 100644
--- /dev/null
+++ b/browser/devtools/app-manager/test/test_remain_connected.html
@@ -0,0 +1,119 @@
+<!DOCTYPE html>
+
+<!--
+Bug 912646 - Closing app toolbox causes phone to disconnect
+-->
+
+<html>
+
+  <head>
+    <meta charset="utf8">
+    <title></title>
+
+    <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+    <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css">
+  </head>
+
+  <body>
+
+    <script type="application/javascript;version=1.8">
+      const Cu = Components.utils;
+
+      Cu.import("resource://gre/modules/devtools/dbg-server.jsm");
+      DebuggerServer.init(function () { return true; });
+      DebuggerServer.addBrowserActors();
+
+      window.onload = function() {
+        SimpleTest.waitForExplicitFinish();
+
+        Cu.import("resource:///modules/devtools/gDevTools.jsm");
+
+        const {devtools} =
+          Cu.import("resource://gre/modules/devtools/Loader.jsm", {});
+        const {require} = devtools;
+
+        const {Connection, ConnectionManager} =
+          require("devtools/client/connection-manager");
+        const ConnectionStore =
+          require("devtools/app-manager/connection-store");
+
+        let connection = ConnectionManager.createConnection();
+
+        connection.host = null; // force pipe
+        connection.port = null;
+
+        let been_through_connecting = false;
+        let been_through_connected = false;
+        let been_through_disconnected = false;
+
+        is(connection.status, Connection.Status.DISCONNECTED,
+           "status updated (diconnected)");
+
+        connection.once("connecting", () => {
+          SimpleTest.executeSoon(() => {
+            been_through_connecting = true;
+            is(connection.status, Connection.Status.CONNECTING,
+               "status updated (connecting)");
+          })
+        });
+
+        connection.once("connected", () => {
+          SimpleTest.executeSoon(() => {
+            been_through_connected = true;
+            is(connection.status, Connection.Status.CONNECTED,
+               "status updated (connected)");
+            cycleToolbox();
+          })
+        });
+
+        function cycleToolbox() {
+          connection.client.listTabs(response => {
+            let options = {
+              form: response.tabs[0],
+              client: connection.client,
+              chrome: true
+            };
+            devtools.TargetFactory.forRemoteTab(options).then(target => {
+              let hostType = devtools.Toolbox.HostType.WINDOW;
+              gDevTools.showToolbox(target,
+                                    null,
+                                    hostType).then(toolbox => {
+                SimpleTest.executeSoon(() => {
+                  toolbox.once("destroyed", onDestroyToolbox);
+                  toolbox.destroy();
+                });
+              });
+            });
+          });
+        }
+
+        function onDestroyToolbox() {
+          is(connection.status, Connection.Status.CONNECTED,
+             "toolbox cycled, still connected");
+          connection.disconnect();
+        }
+
+        connection.once("disconnected", () => {
+          SimpleTest.executeSoon(() => {
+            been_through_disconnected = true;
+            is(connection.status, Connection.Status.DISCONNECTED,
+               "status updated (disconnected)");
+            connection.destroy();
+            finishUp();
+          })
+        });
+
+        function finishUp() {
+          ok(been_through_connecting &&
+             been_through_connected &&
+             been_through_disconnected, "All updates happened");
+          DebuggerServer.destroy();
+          SimpleTest.finish();
+        }
+
+        connection.connect();
+      }
+
+    </script>
+  </body>
+</html>
--- a/browser/devtools/framework/connect/connect.js
+++ b/browser/devtools/framework/connect/connect.js
@@ -164,12 +164,17 @@ function handleConnectionTimeout() {
  */
 function openToolbox(form, chrome=false) {
   let options = {
     form: form,
     client: gClient,
     chrome: chrome
   };
   devtools.TargetFactory.forRemoteTab(options).then((target) => {
-    gDevTools.showToolbox(target, "webconsole", devtools.Toolbox.HostType.WINDOW);
+    let hostType = devtools.Toolbox.HostType.WINDOW;
+    gDevTools.showToolbox(target, "webconsole", hostType).then((toolbox) => {
+      toolbox.once("destroyed", function() {
+        gClient.close();
+      });
+    });
     window.close();
   });
 }
--- a/browser/devtools/framework/target.js
+++ b/browser/devtools/framework/target.js
@@ -41,17 +41,18 @@ exports.TargetFactory = {
   },
 
   /**
    * Return a promise of a Target for a remote tab.
    * @param {Object} options
    *        The options object has the following properties:
    *        {
    *          form: the remote protocol form of a tab,
-   *          client: a DebuggerClient instance,
+   *          client: a DebuggerClient instance
+   *                  (caller owns this and is responsible for closing),
    *          chrome: true if the remote target is the whole process
    *        }
    *
    * @return A promise of a target object
    */
   forRemoteTab: function TF_forRemoteTab(options) {
     let targetPromise = promiseTargets.get(options);
     if (targetPromise == null) {
@@ -441,20 +442,29 @@ TabTarget.prototype = {
     if (this._tab && !this._client) {
       this._cleanup();
       this._destroyer.resolve(null);
     } else if (this._client) {
       // If, on the other hand, this target was remoted, the promise will be
       // resolved after the remote connection is closed.
       this._teardownRemoteListeners();
 
-      this._client.close(() => {
+      if (this.isLocalTab) {
+        // We started with a local tab and created the client ourselves, so we
+        // should close it.
+        this._client.close(() => {
+          this._cleanup();
+          this._destroyer.resolve(null);
+        });
+      } else {
+        // The client was handed to us, so we are not responsible for closing
+        // it.
         this._cleanup();
         this._destroyer.resolve(null);
-      });
+      }
     }
 
     return this._destroyer.promise;
   },
 
   /**
    * Clean up references to what this target points to.
    */
--- a/browser/devtools/framework/toolbox.js
+++ b/browser/devtools/framework/toolbox.js
@@ -842,18 +842,17 @@ Toolbox.prototype = {
     while(container.firstChild) {
       container.removeChild(container.firstChild);
     }
 
     outstanding.push(this._host.destroy());
 
     this._telemetry.destroy();
 
-    // Targets need to be notified that the toolbox is being torn down, so that
-    // remote protocol connections can be gracefully terminated.
+    // Targets need to be notified that the toolbox is being torn down.
     if (this._target) {
       this._target.off("close", this.destroy);
       outstanding.push(this._target.destroy());
     }
     this._target = null;
 
     promise.all(outstanding).then(function() {
       this.emit("destroyed");