Bug 878292 - Profiler actor leaves the profiler running in some cases. r=anton, r=robcee

toolkit/devtools/server/actors/profiler.js

@@ -4,7 +4,7 @@
 
 "use strict";
 
-var connCount = 0;
+var startedProfilers = 0;
 var startTime = 0;
 
 function getCurrentTime() {
@@ -20,7 +20,6 @@ function ProfilerActor(aConnection)
   this._profiler = Cc["@mozilla.org/tools/profiler;1"].getService(Ci.nsIProfiler);
   this._started = false;
   this._observedEvents = [];
-  connCount += 1;
 }
 
 ProfilerActor.prototype = {
@@ -31,30 +30,36 @@ ProfilerActor.prototype = {
       Services.obs.removeObserver(this, event);
     }
 
-    // We stop the profiler only after the last client is
+    this.stopProfiler();
-    // disconnected. Otherwise there's a problem where
+    this._profiler = null;
+  },
+
+  stopProfiler: function() {
+    // We stop the profiler only after the last client has
+    // stopped profiling. Otherwise there's a problem where
     // we stop the profiler as soon as you close the devtools
     // panel in one tab even though there might be other
     // profiler instances running in other tabs.
-
+    if (!this._started) {
-    connCount -= 1;
+      return;
-    if (connCount <= 0 && this._profiler && this._started) {
+    }
+    this._started = false;
+    startedProfilers -= 1;
+    if (startedProfilers <= 0) {
       this._profiler.StopProfiler();
     }
-
-    this._profiler = null;
   },
 
   onStartProfiler: function(aRequest) {
     this._profiler.StartProfiler(aRequest.entries, aRequest.interval,
                            aRequest.features, aRequest.features.length);
     this._started = true;
+    startedProfilers += 1;
     startTime = (new Date()).getTime();
     return { "msg": "profiler started" }
   },
   onStopProfiler: function(aRequest) {
-    this._profiler.StopProfiler();
+    this.stopProfiler();
-    this._started = false;
     return { "msg": "profiler stopped" }
   },
   onGetProfileStr: function(aRequest) {

toolkit/devtools/server/tests/unit/test_profiler_activation.js

@@ -0,0 +1,66 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+const Profiler = Cc["@mozilla.org/tools/profiler;1"].getService(Ci.nsIProfiler);
+
+function connectClient(callback) {
+  let client = new DebuggerClient(DebuggerServer.connectPipe());
+  client.connect(function () {
+    client.listTabs(function(response) {
+      callback(client, response.profilerActor);
+    });
+  });
+}
+
+function run_test()
+{
+  // Ensure the profiler is not running when the test starts (it could
+  // happen if the MOZ_PROFILER_STARTUP environment variable is set)
+  Profiler.StopProfiler();
+
+  DebuggerServer.init(function () { return true; });
+  DebuggerServer.addBrowserActors();
+
+  connectClient((client1, actor1) => {
+    connectClient((client2, actor2) => {
+      activate_first(client1, actor1, client2, actor2);
+    });
+  })
+
+  do_test_pending();
+}
+
+function activate_first(client1, actor1, client2, actor2) {
+  // Start the profiler on the first connection....
+  client1.request({ to: actor1, type: "startProfiler", features: ['js']}, startResponse => {
+    // Profiler should be active now.
+    do_check_true(Profiler.IsActive());
+
+    // But on the next connection just make sure the actor has been
+    // instantiated.
+    client2.request({ to: actor2, type: "getFeatures" }, featureResponse => {
+
+      let connectionClosed = DebuggerServer._connectionClosed;
+      DebuggerServer._connectionClosed = function(conn) {
+        connectionClosed.call(this, conn);
+
+        // Client1 is the only actor that started the profiler,
+        // it shouldn't be active anymore.
+        do_check_false(Profiler.IsActive());
+
+        DebuggerServer._connectionClosed = function(conn) {
+          connectionClosed.call(this, conn);
+
+          // Now there are no open clients at all, it should *definitely*
+          // be deactivated by now.
+          do_check_false(Profiler.IsActive());
+          do_test_finished();
+        }
+        client2.close();
+      };
+      client1.close();
+    });
+  });
+}

toolkit/devtools/server/tests/unit/xpcshell.ini

@@ -124,6 +124,7 @@ skip-if = toolkit == "gonk"
 reason = bug 820380
 [test_breakpointstore.js]
 [test_profiler_actor.js]
+[test_profiler_activation.js]
 skip-if = toolkit == "gonk"
 reason = bug 820380
 [test_unsafeDereference.js]