Bug 830664, Disallow multiple profiles to run in the same time, r=robcee

browser/devtools/profiler/ProfilerPanel.jsm

@@ -27,10 +27,13 @@ XPCOMUtils.defineLazyGetter(this, "DebuggerServer", function () {
  * Its main function is to talk to the Cleopatra instance
  * inside of the iframe.
  *
- * ProfileUI is also an event emitter. Currently, it emits
- * only one event, 'ready', when Cleopatra is done loading.
- * You can also check 'isReady' property to see if a
- * particular instance has been loaded yet.
+ * ProfileUI is also an event emitter. It emits the following events:
+ *  - ready, when Cleopatra is done loading (you can also check the isReady
+ *    property to see if a particular instance has been loaded yet.
+ *  - disabled, when Cleopatra gets disabled. Happens when another ProfileUI
+ *    instance starts the profiler.
+ *  - enabled, when Cleopatra gets enabled. Happens when another ProfileUI
+ *    instance stops the profiler.
  *
  * @param number uid
  *   Unique ID for this profile.
@@ -63,27 +66,45 @@ function ProfileUI(uid, panel) {
       return;
     }
 
+    let label = doc.querySelector("li#profile-" + this.uid + " > h1");
     switch (event.data.status) {
       case "loaded":
+        if (this.panel._runningUid !== null) {
+          this.iframe.contentWindow.postMessage(JSON.stringify({
+            uid: this._runningUid,
+            isCurrent: this._runningUid === uid,
+            task: "onStarted"
+          }), "*");
+        }
+
         this.isReady = true;
         this.emit("ready");
         break;
       case "start":
-        // Start profiling and, once started, notify the
-        // underlying page so that it could update the UI.
+        // Start profiling and, once started, notify the underlying page
+        // so that it could update the UI. Also, once started, we add a
+        // star to the profile name to indicate which profile is currently
+        // running.
         this.panel.startProfiling(function onStart() {
-          var data = JSON.stringify({task: "onStarted"});
-          this.iframe.contentWindow.postMessage(data, "*");
+          this.panel.broadcast(this.uid, {task: "onStarted"});
+          label.textContent = label.textContent + " *";
         }.bind(this));
 
         break;
       case "stop":
-        // Stop profiling and, once stopped, notify the
-        // underlying page so that it could update the UI.
+        // Stop profiling and, once stopped, notify the underlying page so
+        // that it could update the UI and remove a star from the profile
+        // name.
         this.panel.stopProfiling(function onStop() {
-          var data = JSON.stringify({task: "onStopped"});
-          this.iframe.contentWindow.postMessage(data, "*");
+          this.panel.broadcast(this.uid, {task: "onStopped"});
+          label.textContent = label.textContent.replace(/\s\*$/, "");
         }.bind(this));
+        break;
+      case "disabled":
+        this.emit("disabled");
+        break;
+      case "enabled":
+        this.emit("enabled");
     }
   }.bind(this));
 }
@@ -188,15 +209,16 @@ function ProfilerPanel(frame, toolbox) {
 }
 
 ProfilerPanel.prototype = {
-  isReady:    null,
-  window:     null,
-  document:   null,
-  target:     null,
-  controller: null,
-  profiles:   null,
+  isReady:     null,
+  window:      null,
+  document:    null,
+  target:      null,
+  controller:  null,
+  profiles:    null,
 
-  _uid:       null,
-  _activeUid: null,
+  _uid:        null,
+  _activeUid:  null,
+  _runningUid: null,
 
   get activeProfile() {
     return this.profiles.get(this._activeUid);
@@ -207,8 +229,8 @@ ProfilerPanel.prototype = {
   },
 
   /**
-   * Open a debug connection and, on success, switch to
-   * the newly created profile.
+   * Open a debug connection and, on success, switch to the newly created
+   * profile.
    *
    * @return Promise
    */
@@ -359,6 +381,41 @@ ProfilerPanel.prototype = {
     }.bind(this));
   },
 
+  /**
+   * Broadcast messages to all Cleopatra instances.
+   *
+   * @param number target
+   *   UID of the recepient profile. All profiles will receive the message
+   *   but the profile specified by 'target' will have a special property,
+   *   isCurrent, set to true.
+   * @param object data
+   *   An object with a property 'task' that will be sent over to Cleopatra.
+   */
+  broadcast: function PP_broadcast(target, data) {
+    if (!this.profiles) {
+      return;
+    }
+
+    if (data.task === "onStarted") {
+      this._runningUid = target;
+    } else {
+      this._runningUid = null;
+    }
+
+    let uid = this._uid;
+    while (uid >= 0) {
+      if (this.profiles.has(uid)) {
+        let iframe = this.profiles.get(uid).iframe;
+        iframe.contentWindow.postMessage(JSON.stringify({
+          uid: target,
+          isCurrent: target === uid,
+          task: data.task
+        }), "*");
+      }
+      uid -= 1;
+    }
+  },
+
   /**
    * Cleanup.
    */

browser/devtools/profiler/cleopatra/css/devtools.css

@@ -10,4 +10,9 @@
 
 #stopWrapper {
   display: none;
+}
+
+#profilerMessage {
+  color: #999;
+  display: none;
 }

browser/devtools/profiler/cleopatra/js/devtools.js

@@ -13,6 +13,8 @@ var gInstanceUID;
  *    - loaded, when page is loaded.
  *    - start, when user wants to start profiling.
  *    - stop, when user wants to stop profiling.
+ *    - disabled, when the profiler was disabled
+ *    - enabled, when the profiler was enabled
  */
 function notifyParent(status) {
   if (!gInstanceUID) {
@@ -45,18 +47,34 @@ function notifyParent(status) {
 function onParentMessage(event) {
   var start = document.getElementById("startWrapper");
   var stop = document.getElementById("stopWrapper");
+  var profilerMessage = document.getElementById("profilerMessage");
   var msg = JSON.parse(event.data);
 
   switch (msg.task) {
     case "onStarted":
-      start.style.display = "none";
-      start.querySelector("button").removeAttribute("disabled");
-      stop.style.display = "inline";
+      if (msg.isCurrent) {
+        start.style.display = "none";
+        start.querySelector("button").removeAttribute("disabled");
+        stop.style.display = "inline";
+      } else {
+        start.querySelector("button").setAttribute("disabled", true);
+        var text = gStrings.getFormatStr("profiler.alreadyRunning", [msg.uid]);
+        profilerMessage.textContent = text;
+        profilerMessage.style.display = "block";
+        notifyParent("disabled");
+      }
       break;
     case "onStopped":
-      stop.style.display = "none";
-      stop.querySelector("button").removeAttribute("disabled");
-      start.style.display = "inline";
+      if (msg.isCurrent) {
+        stop.style.display = "none";
+        stop.querySelector("button").removeAttribute("disabled");
+        start.style.display = "inline";
+      } else {
+        start.querySelector("button").removeAttribute("disabled");
+        profilerMessage.textContent = "";
+        profilerMessage.style.display = "none";
+        notifyParent("enabled");
+      }
       break;
     case "receiveProfileData":
       loadProfile(JSON.stringify(msg.rawProfile));
@@ -107,7 +125,8 @@ function initUI() {
   controlPane.className = "controlPane";
   controlPane.innerHTML =
     "<p id='startWrapper'>" + startProfiling + "</p>" +
-    "<p id='stopWrapper'>" + stopProfiling + "</p>";
+    "<p id='stopWrapper'>" + stopProfiling + "</p>" +
+    "<p id='profilerMessage'></p>";
 
   controlPane.querySelector("#startWrapper > span.btn").appendChild(startButton);
   controlPane.querySelector("#stopWrapper > span.btn").appendChild(stopButton);

browser/devtools/profiler/test/Makefile.in

@@ -15,6 +15,7 @@ MOCHITEST_BROWSER_FILES = \
 		browser_profiler_controller.js \
 		browser_profiler_profiles.js \
 		browser_profiler_remote.js \
+		browser_profiler_bug_830664_multiple_profiles.js \
 		head.js \
 
 include $(topsrcdir)/config/rules.mk

browser/devtools/profiler/test/browser_profiler_bug_830664_multiple_profiles.js

@@ -0,0 +1,83 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+const URL = "data:text/html;charset=utf8,<p>JavaScript Profiler test</p>";
+
+let gTab, gPanel, gUid;
+
+function test() {
+  waitForExplicitFinish();
+
+  setUp(URL, function onSetUp(tab, browser, panel) {
+    gTab = tab;
+    gPanel = panel;
+
+    gPanel.once("profileCreated", function (_, uid) {
+      gUid = uid;
+      let profile = gPanel.profiles.get(uid);
+
+      if (profile.isReady) {
+        startProfiling();
+      } else {
+        profile.once("ready", startProfiling);
+      }
+    });
+    gPanel.createProfile();
+  });
+}
+
+function getCleoControls(doc) {
+  return [
+    doc.querySelector("#startWrapper button"),
+    doc.querySelector("#profilerMessage")
+  ];
+}
+
+function sendFromActiveProfile(msg) {
+  let [win, doc] = getProfileInternals();
+
+  win.parent.postMessage({
+    uid: gPanel.activeProfile.uid,
+    status: msg
+  }, "*");
+}
+
+function startProfiling() {
+  gPanel.profiles.get(gUid).once("disabled", stopProfiling);
+  sendFromActiveProfile("start");
+}
+
+function stopProfiling() {
+  let [win, doc] = getProfileInternals(gUid);
+  let [btn, msg] = getCleoControls(doc);
+
+  ok(msg.textContent.match("Profile 1") !== null, "Message is visible");
+  ok(btn.hasAttribute("disabled"), "Button is disabled");
+
+  is(gPanel.document.querySelector("li#profile-1 > h1").textContent,
+    "Profile 1 *", "Profile 1 has a star next to it.");
+  is(gPanel.document.querySelector("li#profile-2 > h1").textContent,
+    "Profile 2", "Profile 2 doesn't have a star next to it.");
+
+  gPanel.profiles.get(gUid).once("enabled", confirmAndFinish);
+  sendFromActiveProfile("stop");
+}
+
+function confirmAndFinish() {
+  let [win, doc] = getProfileInternals(gUid);
+  let [btn, msg] = getCleoControls(doc);
+
+  ok(msg.style.display === "none", "Message is hidden");
+  ok(!btn.hasAttribute("disabled"), "Button is enabled");
+
+  is(gPanel.document.querySelector("li#profile-1 > h1").textContent,
+    "Profile 1", "Profile 1 doesn't have a star next to it.");
+  is(gPanel.document.querySelector("li#profile-2 > h1").textContent,
+    "Profile 2", "Profile 2 doesn't have a star next to it.");  
+
+  tearDown(gTab, function onTearDown() {
+    gPanel = null;
+    gTab = null;
+    gUid = null;
+  });
+}

browser/devtools/profiler/test/browser_profiler_run.js

@@ -33,13 +33,6 @@ function attemptTearDown() {
   });
 }
 
-function getProfileInternals() {
-  let win = gPanel.activeProfile.iframe.contentWindow;
-  let doc = win.document;
-
-  return [win, doc];
-}
-
 function testUI() {
   ok(gPanel, "Profiler panel exists");
   ok(gPanel.activeProfile, "Active profile exists");

browser/devtools/profiler/test/head.js

@@ -20,6 +20,14 @@ registerCleanupFunction(function () {
   DebuggerServer.destroy();
 });
 
+function getProfileInternals(uid) {
+  let profile = (uid != null) ? gPanel.profiles.get(uid) : gPanel.activeProfile;
+  let win = profile.iframe.contentWindow;
+  let doc = win.document;
+
+  return [win, doc];
+}
+
 function loadTab(url, callback) {
   let tab = gBrowser.addTab();
   gBrowser.selectedTab = tab;

browser/locales/en-US/chrome/browser/devtools/profiler.properties

@@ -76,4 +76,9 @@ profiler.stop=Stop
 # LOCALIZATION NOTE (profiler.loading)
 # This string is displayed above the progress bar when the profiler
 # is busy loading and parsing the report.
-profiler.loading=Loading profile…
+profiler.loading=Loading profile…
+
+# LOCALIZATION NOTE (profiler.alreadyRunning)
+# This string is displayed in the profiler whenever there is already
+# another running profile. Users can run only one profile at a time.
+profiler.alreadyRunning=Profiler is already running. If you want to run this profile stop Profile %S first.