Bug 1025044 - CSS coverage actor should have started and stopped events; r=harth
authorJoe Walker <jwalker@mozilla.com>
Fri, 27 Jun 2014 08:31:20 +0100
changeset 191164 70ff1b3ae5512ee559644ea4856a2f0a1b993b90
parent 191163 2c5284563badef875e42d3b08b6830b5596ccba5
child 191165 f6cee8c74faf8736354367bc6337eccca3ea66a9
push id8436
push usercbook@mozilla.com
push dateFri, 27 Jun 2014 13:56:57 +0000
treeherderb2g-inbound@22ea396750e8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersharth
bugs1025044
milestone33.0a1
Bug 1025044 - CSS coverage actor should have started and stopped events; r=harth
browser/devtools/commandline/test/browser_cmd_csscoverage_oneshot.js
browser/devtools/commandline/test/browser_cmd_csscoverage_startstop.js
toolkit/devtools/gcli/commands/csscoverage.js
toolkit/devtools/server/actors/csscoverage.js
--- a/browser/devtools/commandline/test/browser_cmd_csscoverage_oneshot.js
+++ b/browser/devtools/commandline/test/browser_cmd_csscoverage_oneshot.js
@@ -28,23 +28,21 @@ let test = asyncTest(function*() {
   yield helpers.closeToolbar(options);
   yield helpers.closeTab(options);
 });
 
 /**
  * Just check current page
  */
 function* navigate(usage, options) {
-  let running = yield usage._testOnly_isRunning();
-  ok(!running, "csscoverage not is running");
+  ok(!usage.isRunning(), "csscoverage is not running");
 
   yield usage.oneshot();
 
-  running = yield usage._testOnly_isRunning();
-  ok(!running, "csscoverage not is running");
+  ok(!usage.isRunning(), "csscoverage is still not running");
 }
 
 /**
  * Check the expected pages have been visited
  */
 function* checkPages(usage) {
   let expectedVisited = [ PAGE_3 ];
   let actualVisited = yield usage._testOnly_visitedPages();
--- a/browser/devtools/commandline/test/browser_cmd_csscoverage_startstop.js
+++ b/browser/devtools/commandline/test/browser_cmd_csscoverage_startstop.js
@@ -30,32 +30,30 @@ let test = asyncTest(function*() {
 });
 
 /**
  * Visit all the pages in the test
  */
 function* navigate(usage, options) {
   yield usage.start();
 
-  let running = yield usage._testOnly_isRunning();
-  ok(running, "csscoverage is running");
+  ok(usage.isRunning(), "csscoverage is running");
 
   yield helpers.navigate(PAGE_1, options);
 
   // Wait for the test pages to auto-cycle
   let ev = yield helpers.listenOnce(options.browser, "load", true);
   is(ev.target.location.href, PAGE_1, "page 1 loaded");
 
   ev = yield helpers.listenOnce(options.browser, "load", true);
   is(ev.target.location.href, PAGE_3, "page 3 loaded");
 
   yield usage.stop();
 
-  running = yield usage._testOnly_isRunning();
-  ok(!running, "csscoverage not is running");
+  ok(!usage.isRunning(), "csscoverage not is running");
 }
 
 /**
  * Check the expected pages have been visited
  */
 function* checkPages(usage) {
   // 'load' event order. '' is for the initial location
   let expectedVisited = [ '', PAGE_2, PAGE_1, PAGE_3 ];
--- a/toolkit/devtools/gcli/commands/csscoverage.js
+++ b/toolkit/devtools/gcli/commands/csscoverage.js
@@ -66,30 +66,44 @@ exports.items = [
       yield usage.oneshot();
       yield gDevTools.showToolbox(target, "styleeditor");
     }
   },
   {
     name: "csscoverage toggle",
     hidden: true,
     description: l10n.lookup("csscoverageToggleDesc2"),
+    state: {
+      isChecked: function(target) {
+        return csscoverage.getUsage(target).then(usage => {
+          return usage.isRunning();
+        });
+      },
+      onChange: function(target, handler) {
+        csscoverage.getUsage(target).then(usage => {
+          this.handler = ev => { handler("state-change", ev); };
+          usage.on("state-change", this.handler);
+        });
+      },
+      offChange: function(target, handler) {
+        csscoverage.getUsage(target).then(usage => {
+          usage.off("state-change", this.handler);
+          this.handler = undefined;
+        });
+      },
+    },
     exec: function*(args, context) {
       let target = context.environment.target;
       let usage = yield csscoverage.getUsage(target);
       if (usage == null) {
         throw new Error(l10n.lookup("csscoverageNoRemoteError"));
       }
 
-      let running = yield usage.toggle();
-      if (running) {
-        return l10n.lookup("csscoverageRunningReply");
-      }
-
-      yield usage.stop();
-      yield gDevTools.showToolbox(target, "styleeditor");
+      yield usage.toggle(context.environment.chromeWindow,
+                         context.environment.target);
     }
   },
   {
     name: "csscoverage report",
     hidden: true,
     description: l10n.lookup("csscoverageReportDesc2"),
     exec: function*(args, context) {
       let usage = yield csscoverage.getUsage(context.environment.target);
--- a/toolkit/devtools/server/actors/csscoverage.js
+++ b/toolkit/devtools/server/actors/csscoverage.js
@@ -3,16 +3,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 const { Cc, Ci, Cu } = require("chrome");
 
 const Services = require("Services");
 
+const events = require("sdk/event/core");
 const protocol = require("devtools/server/protocol");
 const { method, custom, RetVal, Arg } = protocol;
 
 loader.lazyGetter(this, "gDevTools", () => {
   return require("resource:///modules/devtools/gDevTools.jsm").gDevTools;
 });
 loader.lazyGetter(this, "DOMUtils", () => {
   return Cc["@mozilla.org/inspector/dom-utils;1"].getService(Ci.inIDOMUtils)
@@ -70,16 +71,23 @@ const l10n = exports.l10n = {
  *         preLoadOn: Set([ "http://eg.com/page1.html" ]),
  *         isError: false,
  *       }, ...
  *     });
  */
 let UsageReportActor = protocol.ActorClass({
   typeName: "usageReport",
 
+  events: {
+    "state-change" : {
+      type: "stateChange",
+      stateChange: Arg(0, "json")
+    }
+  },
+
   initialize: function(conn, tabActor) {
     protocol.Actor.prototype.initialize.call(this, conn);
 
     this._tabActor = tabActor;
     this._running = false;
 
     this._onTabLoad = this._onTabLoad.bind(this);
     this._onChange = this._onChange.bind(this);
@@ -108,39 +116,38 @@ let UsageReportActor = protocol.ActorCla
     this._tooManyUnused = false;
 
     this._tabActor.browser.addEventListener("load", this._onTabLoad, true);
 
     this._observeMutations(this._tabActor.window.document);
 
     this._populateKnownRules(this._tabActor.window.document);
     this._updateUsage(this._tabActor.window.document, false);
+
+    events.emit(this, "state-change", { isRunning: true });
   }),
 
   /**
    * Cease recording usage data
    */
   stop: method(function() {
     if (!this._running) {
       throw new Error(l10n.lookup("csscoverageNotRunningError"));
     }
 
     this._tabActor.browser.removeEventListener("load", this._onTabLoad, true);
     this._running = false;
+    events.emit(this, "state-change", { isRunning: false });
   }),
 
   /**
    * Start/stop recording usage data depending on what we're currently doing.
    */
   toggle: method(function() {
-    return this._running ?
-        this.stop().then(() => false) :
-        this.start().then(() => true);
-  }, {
-    response: RetVal("boolean"),
+    return this._running ? this.stop() : this.start();
   }),
 
   /**
    * Running start() quickly followed by stop() does a bunch of unnecessary
    * work, so this cuts all that out
    */
   oneshot: method(function() {
     if (this._running) {
@@ -431,25 +438,16 @@ let UsageReportActor = protocol.ActorCla
       preload: preload,
       unused: unused
     };
   }, {
     response: RetVal("json")
   }),
 
   /**
-   * For testing only. Is css coverage running.
-   */
-  _testOnly_isRunning: method(function() {
-    return this._running;
-  }, {
-    response: { value: RetVal("boolean") }
-  }),
-
-  /**
    * For testing only. What pages did we visit.
    */
   _testOnly_visitedPages: method(function() {
     return [...this._visitedPages];
   }, {
     response: { value: RetVal("array:string") }
   }),
 });
@@ -693,69 +691,99 @@ const sheetToUrl = exports.sheetToUrl = 
     let index = sheets.indexOf(stylesheet.ownerNode);
     return getURL(document) + ' → <style> index ' + index;
   }
 
   throw new Error("Unknown sheet source");
 }
 
 /**
+ * Running more than one usage report at a time is probably bad for performance
+ * and it isn't particularly useful, and it's confusing from a notification POV
+ * so we only allow one.
+ */
+let isRunning = false;
+let notification;
+let target;
+let chromeWindow;
+
+/**
  * Front for UsageReportActor
  */
 const UsageReportFront = protocol.FrontClass(UsageReportActor, {
   initialize: function(client, form) {
     protocol.Front.prototype.initialize.call(this, client, form);
     this.actorID = form.usageReportActor;
     this.manage(this);
   },
 
-  /**
-   * Server-side start is above. Client-side start adds a notification box
-   */
-  start: custom(function(chromeWindow, target) {
-    if (chromeWindow != null) {
+  _onStateChange: protocol.preEvent("state-change", function(ev) {
+    isRunning = ev.isRunning;
+    ev.target = target;
+
+    if (isRunning) {
       let gnb = chromeWindow.document.getElementById("global-notificationbox");
-      this.notification = gnb.getNotificationWithValue("csscoverage-running");
+      notification = gnb.getNotificationWithValue("csscoverage-running");
 
-      if (this.notification == null) {
-        let notifyStop = ev => {
-          if (ev == "removed") {
+      if (notification == null) {
+        let notifyStop = reason => {
+          if (reason == "removed") {
             this.stop();
-            gDevTools.showToolbox(target, "styleeditor");
           }
         };
 
         let msg = l10n.lookup("csscoverageRunningReply");
-        this.notification = gnb.appendNotification(msg,
-                                                   "csscoverage-running",
-                                                   "", // i.e. no image
-                                                   gnb.PRIORITY_INFO_HIGH,
-                                                   null, // i.e. no buttons
-                                                   notifyStop);
+        notification = gnb.appendNotification(msg, "csscoverage-running",
+                                              "", // i.e. no image
+                                              gnb.PRIORITY_INFO_HIGH,
+                                              null, // i.e. no buttons
+                                              notifyStop);
       }
     }
+    else {
+      if (notification) {
+        notification.remove();
+        notification = undefined;
+      }
+
+      gDevTools.showToolbox(target, "styleeditor");
+      target = undefined;
+    }
+  }),
+
+  /**
+   * Server-side start is above. Client-side start adds a notification box
+   */
+  start: custom(function(newChromeWindow, newTarget) {
+    target = newTarget;
+    chromeWindow = newChromeWindow;
 
     return this._start();
   }, {
     impl: "_start"
   }),
 
   /**
-   * Client-side stop also removes the notification box
+   * Server-side start is above. Client-side start adds a notification box
    */
-  stop: custom(function() {
-    if (this.notification != null) {
-      this.notification.remove();
-      this.notification = undefined;
-    }
+  toggle: custom(function(newChromeWindow, newTarget) {
+    target = newTarget;
+    chromeWindow = newChromeWindow;
 
-    return this._stop();
+    return this._toggle();
   }, {
-    impl: "_stop"
+    impl: "_toggle"
   }),
+
+  /**
+   * We count STARTING and STOPPING as 'running'
+   */
+  isRunning: function() {
+    return isRunning;
+  }
 });
 
 exports.UsageReportFront = UsageReportFront;
 
 /**
  * Registration / De-registration
  */
 exports.register = function(handle) {