Bug 1142937 - AddonWatcher now communicates through nsIObserverService;r?felipe draft
authorDavid Rajchenbach-Teller <dteller@mozilla.com>
Wed, 27 Jan 2016 13:34:58 +0100
changeset 328576 c288b1dfb2b22ea7c75469c8a8c0ca3015668dc5
parent 326148 211a4c710fb6af2cad10102c4cabc7cb525998b8
child 328577 127da9a4db2517a2f38ed94f2baf4a951ad25ff9
push id10375
push userdteller@mozilla.com
push dateWed, 03 Feb 2016 17:36:39 +0000
reviewersfelipe
bugs1142937
milestone47.0a1
Bug 1142937 - AddonWatcher now communicates through nsIObserverService;r?felipe The current API of AddonWatcher only supports a single callback. That's pretty unfriendly to testing, debugging, add-ons, etc. This patch replaces the mechanism with a notification through the nsIObserverService.
browser/components/nsBrowserGlue.js
toolkit/components/perfmonitoring/AddonWatcher.jsm
toolkit/components/perfmonitoring/tests/browser/browser_AddonWatcher.js
toolkit/components/perfmonitoring/tests/browser/browser_addonPerformanceAlerts.js
toolkit/components/perfmonitoring/tests/browser/browser_addonPerformanceAlerts_2.js
--- a/browser/components/nsBrowserGlue.js
+++ b/browser/components/nsBrowserGlue.js
@@ -443,16 +443,19 @@ BrowserGlue.prototype = {
         });
         break;
       case "autocomplete-did-enter-text":
         this._handleURLBarTelemetry(subject.QueryInterface(Ci.nsIAutoCompleteInput));
         break;
       case "test-initialize-sanitizer":
         this._sanitizer.onStartup();
         break;
+      case AddonWatcher.TOPIC_SLOW_ADDON_DETECTED:
+        this._notifySlowAddon(data);
+        break;
     }
   },
 
   _handleURLBarTelemetry(input) {
     if (!input ||
         input.id != "urlbar" ||
         input.inPrivateContext ||
         input.popup.selectedIndex < 0) {
@@ -539,16 +542,21 @@ BrowserGlue.prototype = {
       os.addObserver(this, "keyword-search", false);
     }
     os.addObserver(this, "browser-search-engine-modified", false);
     os.addObserver(this, "restart-in-safe-mode", false);
     os.addObserver(this, "flash-plugin-hang", false);
     os.addObserver(this, "xpi-signature-changed", false);
     os.addObserver(this, "autocomplete-did-enter-text", false);
 
+    if (AppConstants.NIGHTLY_BUILD) {
+      AddonWatcher.init();
+      os.addObserver(this, AddonWatcher.TOPIC_SLOW_ADDON_DETECTED, false);
+    }
+
     ExtensionManagement.registerScript("chrome://browser/content/ext-utils.js");
     ExtensionManagement.registerScript("chrome://browser/content/ext-browserAction.js");
     ExtensionManagement.registerScript("chrome://browser/content/ext-pageAction.js");
     ExtensionManagement.registerScript("chrome://browser/content/ext-contextMenus.js");
     ExtensionManagement.registerScript("chrome://browser/content/ext-tabs.js");
     ExtensionManagement.registerScript("chrome://browser/content/ext-windows.js");
     ExtensionManagement.registerScript("chrome://browser/content/ext-bookmarks.js");
 
@@ -791,20 +799,16 @@ BrowserGlue.prototype = {
     }
 
     TabCrashHandler.init();
     if (AppConstants.MOZ_CRASHREPORTER) {
       PluginCrashReporter.init();
     }
 
     Services.obs.notifyObservers(null, "browser-ui-startup-complete", "");
-
-    if (AppConstants.NIGHTLY_BUILD) {
-      AddonWatcher.init(this._notifySlowAddon);
-    }
   },
 
   _checkForOldBuildUpdates: function () {
     // check for update if our build is old
     if (AppConstants.MOZ_UPDATER &&
         Services.prefs.getBoolPref("app.update.enabled") &&
         Services.prefs.getBoolPref("app.update.checkInstallTime")) {
 
--- a/toolkit/components/perfmonitoring/AddonWatcher.jsm
+++ b/toolkit/components/perfmonitoring/AddonWatcher.jsm
@@ -31,45 +31,43 @@ XPCOMUtils.defineLazyServiceGetter(this,
 /**
  * Don't notify observers of slow add-ons if at least `SUSPICIOUSLY_MANY_ADDONS`
  * show up at the same time. We assume that this indicates that the system itself
  * is busy, and that add-ons are not responsible.
  */
 let SUSPICIOUSLY_MANY_ADDONS = 5;
 
 this.AddonWatcher = {
-  init: function(callback) {
+  /**
+   * Watch this topic to be informed when a slow add-on is detected and should
+   * be reported to the user.
+   *
+   * If you need finer-grained control, use PerformanceWatcher.jsm.
+   */
+  TOPIC_SLOW_ADDON_DETECTED: "addon-watcher-detected-slow-addon",
+
+  init: function() {
     this._initializedTimeStamp = Cu.now();
 
-    this._callback = callback;
     try {
       this._ignoreList = new Set(JSON.parse(Preferences.get("browser.addon-watch.ignore", null)));
     } catch (ex) {
       // probably some malformed JSON, ignore and carry on
       this._ignoreList = new Set();
     }
 
     this._warmupPeriod = Preferences.get("browser.addon-watch.warmup-ms", 60 * 1000 /* 1 minute */);
     this._idleThreshold = Preferences.get("browser.addon-watch.deactivate-after-idle-ms", 3000);
     this.paused = false;
-    this.callback = callback;
   },
   uninit: function() {
     this.paused = true;
   },
   _initializedTimeStamp: 0,
 
-  set callback(callback) {
-    this._callback = callback;
-    if (this._callback == null) {
-      this.paused = true;
-    }
-  },
-  _callback: null,
-
   set paused(paused) {
     if (paused) {
       if (this._listener) {
         PerformanceWatcher.removePerformanceListener({addonId: "*"}, this._listener);
       }
       this._listener = null;
     } else {
       this._listener = this._onSlowAddons.bind(this);
@@ -191,22 +189,17 @@ this.AddonWatcher = {
             // Wait a little before displaying another one.
             continue;
           }
         }
 
         // Ok, time to inform the user.
         alerts.latestNotificationTimeStamp = now;
         alerts.occurrencesSinceLastNotification = 0;
-        try {
-          this._callback(addonId);
-        } catch (ex) {
-          Cu.reportError("Error in AddonWatcher callback " + ex);
-          Cu.reportError(Task.Debugging.generateReadableStack(ex.stack));
-        }
+        Services.obs.notifyObservers(null, this.TOPIC_SLOW_ADDON_DETECTED, addonId);
 
         highestNumberOfAddonsToReport--;
       }
     } catch (ex) {
       Cu.reportError("Error in AddonWatcher._onSlowAddons " + ex);
       Cu.reportError(Task.Debugging.generateReadableStack(ex.stack));
     }
   },
--- a/toolkit/components/perfmonitoring/tests/browser/browser_AddonWatcher.js
+++ b/toolkit/components/perfmonitoring/tests/browser/browser_AddonWatcher.js
@@ -10,18 +10,16 @@ requestLongerTimeout(2);
 Cu.import("resource://gre/modules/Promise.jsm", this);
 Cu.import("resource://gre/modules/AddonManager.jsm", this);
 Cu.import("resource://gre/modules/Services.jsm", this);
 
 const ADDON_URL = "http://example.com/browser/toolkit/components/perfmonitoring/tests/browser/browser_Addons_sample.xpi";
 const ADDON_ID = "addonwatcher-test@mozilla.com";
 
 add_task(function* init() {
-  AddonWatcher.uninit();
-
   info("Installing test add-on");
   let installer = yield new Promise(resolve => AddonManager.getInstallForURL(ADDON_URL, resolve, "application/x-xpinstall"));
   if (installer.error) {
     throw installer.error;
   }
   let ready = new Promise((resolve, reject) => installer.addListener({
     onInstallEnded: (_, addon) => resolve(addon),
     onInstallFailed: reject,
@@ -66,59 +64,60 @@ add_task(function* init() {
       "browser.addon-watch.deactivate-after-idle-ms"
     ]) {
       Preferences.reset(k);
     }
   });
 
   let oldCanRecord = Services.telemetry.canRecordExtended;
   Services.telemetry.canRecordExtended = true;
+  AddonWatcher.init();
+
   registerCleanupFunction(function () {
+    AddonWatcher.paused = true;
     Services.telemetry.canRecordExtended = oldCanRecord;
   });
 });
 
 // Utility function to burn some resource, trigger a reaction of the add-on watcher
 // and check both its notification and telemetry.
 let burn_rubber = Task.async(function*({histogramName, topic, expectedMinSum}) {
+  let detected = false;
+  let observer = (_, topic, id) => {
+    Assert.equal(id, ADDON_ID, "The add-on watcher has detected the misbehaving addon");
+    detected = true;
+  };
+
   try {
     info("Preparing add-on watcher");
 
-    let detected = false;
-    AddonWatcher.init(id => {
-      Assert.equal(id, ADDON_ID, "The add-on watcher has detected the misbehaving addon");
-      detected = true;
-    });
+    Services.obs.addObserver(observer, AddonWatcher.TOPIC_SLOW_ADDON_DETECTED, false);
 
     let histogram = Services.telemetry.getKeyedHistogramById(histogramName);
     histogram.clear();
     let snap1 = histogram.snapshot(ADDON_ID);
     Assert.equal(snap1.sum, 0, `Histogram ${histogramName} is initially empty for the add-on`);
 
     let histogramUpdated = false;
     do {
       info(`Burning some CPU with ${topic}. This should cause an add-on watcher notification`);
       yield new Promise(resolve => setTimeout(resolve, 100));
       Services.obs.notifyObservers(null, topic, "");
       yield new Promise(resolve => setTimeout(resolve, 100));
 
       let snap2 = histogram.snapshot(ADDON_ID);
       histogramUpdated = snap2.sum > 0;
       info(`For the moment, histogram ${histogramName} shows ${snap2.sum} => ${histogramUpdated}`);
-      info(`For the moment, we have ${detected?"":"NOT"}detected the slow add-on`);
+      info(`For the moment, we have ${detected?"":"NOT "}detected the slow add-on`);
     } while (!histogramUpdated || !detected);
 
     let snap3 = histogram.snapshot(ADDON_ID);
     Assert.ok(snap3.sum >= expectedMinSum, `Histogram ${histogramName} recorded a gravity of ${snap3.sum}, expecting at least ${expectedMinSum}.`);
   } finally {
-    if (typeof AddonWatcher != "undefined") {
-      // If the test fails, we may end up with `AddonWatcher` being undefined
-      // here. Let's not make logs harder to parse.
-      AddonWatcher.uninit();
-    }
+    Services.obs.removeObserver(observer, AddonWatcher.TOPIC_SLOW_ADDON_DETECTED);
   }
 });
 
 // Test that burning CPU will cause the add-on watcher to notice that
 // the add-on is misbehaving.
 add_task(function* test_burn_CPU() {
   yield burn_rubber({
     histogramName: "PERF_MONITORING_SLOW_ADDON_JANK_US",
--- a/toolkit/components/perfmonitoring/tests/browser/browser_addonPerformanceAlerts.js
+++ b/toolkit/components/perfmonitoring/tests/browser/browser_addonPerformanceAlerts.js
@@ -1,17 +1,15 @@
 "use strict";
 
 /**
  * Tests for PerformanceWatcher watching slow addons.
  */
 
 add_task(function* init() {
-  AddonWatcher.uninit();
-
   // Get rid of buffering.
   let service = Cc["@mozilla.org/toolkit/performance-stats-service;1"].getService(
     Ci.nsIPerformanceStatsService);
   let oldDelay = service.jankAlertBufferingDelay;
 
   service.jankAlertBufferingDelay = 0 /* ms */;
   registerCleanupFunction(() => {
     info("Cleanup");
--- a/toolkit/components/perfmonitoring/tests/browser/browser_addonPerformanceAlerts_2.js
+++ b/toolkit/components/perfmonitoring/tests/browser/browser_addonPerformanceAlerts_2.js
@@ -1,18 +1,14 @@
 "use strict";
 
 /**
  * Tests for PerformanceWatcher watching slow addons.
  */
 
-add_task(function* init() {
-  AddonWatcher.uninit();
-});
-
 add_task(function* test_watch_addon_then_install_it() {
   for (let topic of ["burnCPU", "promiseBurnContentCPU", "promiseBurnCPOW"]) {
     let addonId = "addon:test_watch_addons_before_installing" + Math.random();
     let realListener = new AddonListener(addonId, (group, details) => {
       if (group.addonId == addonId) {
         return details.highestJank;
       }
       throw new Error(`I shouldn't have been called with addon ${group.addonId}`);