Bug 1439519 - fetch plugin information lazily in TelemetryEnvironment.jsm, r?chutten,florian draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Tue, 06 Mar 2018 16:31:24 +0000
changeset 764329 09c8afa4e337bca6ab60114adbfb3829a8f686a9
parent 763109 51200c0fdaddb2749549a82596da5323a4cbd499
push id101743
push usergijskruitbosch@gmail.com
push dateWed, 07 Mar 2018 17:57:54 +0000
reviewerschutten, florian
bugs1439519
milestone60.0a1
Bug 1439519 - fetch plugin information lazily in TelemetryEnvironment.jsm, r?chutten,florian MozReview-Commit-ID: 9eUwq3lMdZD
toolkit/components/telemetry/TelemetryEnvironment.jsm
toolkit/components/telemetry/docs/data/environment.rst
toolkit/mozapps/extensions/nsBlocklistService.js
toolkit/mozapps/extensions/nsBlocklistServiceContent.js
toolkit/mozapps/extensions/test/xpcshell/test_asyncBlocklistLoad.js
xpcom/system/nsIBlocklistService.idl
--- a/toolkit/components/telemetry/TelemetryEnvironment.jsm
+++ b/toolkit/components/telemetry/TelemetryEnvironment.jsm
@@ -279,16 +279,17 @@ const COMPOSITOR_CREATED_TOPIC = "compos
 const COMPOSITOR_PROCESS_ABORTED_TOPIC = "compositor:process-aborted";
 const DISTRIBUTION_CUSTOMIZATION_COMPLETE_TOPIC = "distribution-customization-complete";
 const EXPERIMENTS_CHANGED_TOPIC = "experiments-changed";
 const GFX_FEATURES_READY_TOPIC = "gfx-features-ready";
 const SEARCH_ENGINE_MODIFIED_TOPIC = "browser-search-engine-modified";
 const SEARCH_SERVICE_TOPIC = "browser-search-service";
 const SESSIONSTORE_WINDOWS_RESTORED_TOPIC = "sessionstore-windows-restored";
 const PREF_CHANGED_TOPIC = "nsPref:changed";
+const BLOCKLIST_LOADED_TOPIC = "blocklist-loaded";
 
 /**
  * Enforces the parameter to a boolean value.
  * @param aValue The input value.
  * @return {Boolean|Object} If aValue is a boolean or a number, returns its truthfulness
  *         value. Otherwise, return null.
  */
 function enforceBoolean(aValue) {
@@ -485,16 +486,20 @@ function getWindowsVersionInfo() {
  */
 function EnvironmentAddonBuilder(environment) {
   this._environment = environment;
 
   // The pending task blocks addon manager shutdown. It can either be the initial load
   // or a change load.
   this._pendingTask = null;
 
+  // Have we added an observer to listen for blocklist changes that still needs to be
+  // removed:
+  this._blocklistObserverAdded = false;
+
   // Set to true once initial load is complete and we're watching for changes.
   this._loaded = false;
 }
 EnvironmentAddonBuilder.prototype = {
   /**
    * Get the initial set of addons.
    * @returns Promise<void> when the initial load is complete.
    */
@@ -566,17 +571,31 @@ EnvironmentAddonBuilder.prototype = {
   _onAddonChange() {
     this._environment._log.trace("_onAddonChange");
     this._checkForChanges("addons-changed");
   },
 
   // nsIObserver
   observe(aSubject, aTopic, aData) {
     this._environment._log.trace("observe - Topic " + aTopic);
-    this._checkForChanges("experiment-changed");
+    if (aTopic == "experiment-changed") {
+      this._checkForChanges("experiment-changed");
+    } else if (aTopic == BLOCKLIST_LOADED_TOPIC) {
+      Services.obs.removeObserver(this, BLOCKLIST_LOADED_TOPIC);
+      this._blocklistObserverAdded = false;
+      let plugins = this._getActivePlugins();
+      let gmpPluginsPromise = this._getActiveGMPlugins();
+      gmpPluginsPromise.then(gmpPlugins => {
+        let {addons} = this._environment._currentEnvironment;
+        addons.activePlugins = plugins;
+        addons.activeGMPlugins = gmpPlugins;
+      }, err => {
+        this._environment._log.error("blocklist observe: Error collecting plugins", err);
+      });
+    }
   },
 
   _checkForChanges(changeReason) {
     if (this._pendingTask) {
       this._environment._log.trace("_checkForChanges - task already pending, dropping change with reason " + changeReason);
       return;
     }
 
@@ -592,16 +611,19 @@ EnvironmentAddonBuilder.prototype = {
         this._environment._log.error("_checkForChanges: Error collecting addons", err);
       });
   },
 
   _shutdownBlocker() {
     if (this._loaded) {
       AddonManager.removeAddonListener(this);
       Services.obs.removeObserver(this, EXPERIMENTS_CHANGED_TOPIC);
+      if (this._blocklistObserverAdded) {
+        Services.obs.removeObserver(this, BLOCKLIST_LOADED_TOPIC);
+      }
     }
 
     // At startup, _pendingTask is set to a Promise that does not resolve
     // until the addons database has been read so complete details about
     // addons are available.  Returning it here will cause it to block
     // profileBeforeChange, guranteeing that full information will be
     // available by the time profileBeforeChangeTelemetry is fired.
     return this._pendingTask;
@@ -735,16 +757,30 @@ EnvironmentAddonBuilder.prototype = {
     return activeTheme;
   },
 
   /**
    * Get the plugins data in object form.
    * @return Object containing the plugins data.
    */
   _getActivePlugins() {
+    // If we haven't yet loaded the blocklist, pass back dummy data for now,
+    // and add an observer to update this data as soon as we get it.
+    if (!Services.blocklist.isLoaded) {
+      if (!this._blocklistObserverAdded) {
+        Services.obs.addObserver(this, BLOCKLIST_LOADED_TOPIC);
+        this._blocklistObserverAdded = true;
+      }
+      return [{
+        name: "dummy", version: "0.1", description: "Blocklist unavailable",
+        blocklisted: false, disabled: true, clicktoplay: false,
+        mimeTypes: ["text/there.is.only.blocklist"],
+        updateDay: Utils.millisecondsToDays(Date.now()),
+      }];
+    }
     let pluginTags =
       Cc["@mozilla.org/plugin/host;1"].getService(Ci.nsIPluginHost).getPluginTags({});
 
     let activePlugins = [];
     for (let tag of pluginTags) {
       // Skip plugins which are not active.
       if (tag.disabled) {
         continue;
@@ -776,16 +812,27 @@ EnvironmentAddonBuilder.prototype = {
   /**
    * Get the GMPlugins data in object form.
    * @return Object containing the GMPlugins data.
    *
    * This should only be called from _pendingTask; otherwise we risk
    * running this during addon manager shutdown.
    */
   async _getActiveGMPlugins() {
+    // If we haven't yet loaded the blocklist, pass back dummy data for now,
+    // and add an observer to update this data as soon as we get it.
+    if (!Services.blocklist.isLoaded) {
+      if (!this._blocklistObserverAdded) {
+        Services.obs.addObserver(this, BLOCKLIST_LOADED_TOPIC);
+        this._blocklistObserverAdded = true;
+      }
+      return {
+        "dummy-gmp": {version: "0.1", userDisabled: false, applyBackgroundUpdates: true}
+      };
+    }
     // Request plugins, asynchronously.
     let allPlugins = await AddonManager.getAddonsByTypes(["plugin"]);
 
     let activeGMPlugins = {};
     for (let plugin of allPlugins) {
       // Only get info for active GMplugins.
       if (!plugin.isGMPlugin || !plugin.isActive) {
         continue;
--- a/toolkit/components/telemetry/docs/data/environment.rst
+++ b/toolkit/components/telemetry/docs/data/environment.rst
@@ -398,13 +398,23 @@ This object contains operating system in
 addons
 ------
 
 activeAddons
 ~~~~~~~~~~~~
 
 Starting from Firefox 44, the length of the following string fields: ``name``, ``description`` and ``version`` is limited to 100 characters. The same limitation applies to the same fields in ``theme`` and ``activePlugins``.
 
-Some of the fields in the record for each add-on are not available during startup.  The fields that will always be present are ``id``, ``version``, ``type``, ``updateDate``, ``scope``, ``isSystem``, ``isWebExtension``, and ``multiprocessCompatible``.  All the other fields documented above become present shortly after the ``sessionstore-windows-restored`` event is dispatched.
+Some of the fields in the record for each add-on are not available during startup.  The fields that will always be present are ``id``, ``version``, ``type``, ``updateDate``, ``scope``, ``isSystem``, ``isWebExtension``, and ``multiprocessCompatible``.  All the other fields documented above become present shortly after the ``sessionstore-windows-restored`` observer topic is notified.
+
+activePlugins
+~~~~~~~~~~~~~
+
+Just like activeAddons, up-to-date information is not available immediately during startup. The field will be populated with dummy information until the blocklist is loaded. At the latest, this will happen just after the ``sessionstore-windows-restored`` observer topic is notified.
+
+activeGMPPlugins
+~~~~~~~~~~~~~~~~
+
+Just like activePlugins, this will report dummy values until the blocklist is loaded.
 
 experiments
 -----------
 For each experiment we collect the ``id`` and the ``branch`` the client is enrolled in. Both fields are truncated to 100 characters and a warning is printed when that happens. This section will eventually supersede ``addons/activeExperiment``.
--- a/toolkit/mozapps/extensions/nsBlocklistService.js
+++ b/toolkit/mozapps/extensions/nsBlocklistService.js
@@ -310,17 +310,17 @@ Blocklist.prototype = {
   /* See nsIBlocklistService */
   isAddonBlocklisted(addon, appVersion, toolkitVersion) {
     return this.getAddonBlocklistState(addon, appVersion, toolkitVersion) ==
                    Ci.nsIBlocklistService.STATE_BLOCKED;
   },
 
   /* See nsIBlocklistService */
   getAddonBlocklistState(addon, appVersion, toolkitVersion) {
-    if (!this._isBlocklistLoaded())
+    if (!this.isLoaded)
       this._loadBlocklist();
     return this._getAddonBlocklistState(addon, this._addonEntries,
                                         appVersion, toolkitVersion);
   },
 
   /**
    * Returns a matching blocklist entry for the given add-on, if one
    * exists.
@@ -366,17 +366,17 @@ Blocklist.prototype = {
           url: blItem.blockID && this._createBlocklistURL(blItem.blockID),
         };
       }
     }
     return null;
   },
 
   getAddonBlocklistEntry(addon, appVersion, toolkitVersion) {
-    if (!this._isBlocklistLoaded())
+    if (!this.isLoaded)
       this._loadBlocklist();
     return this._getAddonBlocklistEntry(addon, this._addonEntries,
                                         appVersion, toolkitVersion);
   },
 
   /**
    * Private version of getAddonBlocklistState that allows the caller to pass in
    * the add-on blocklist entries to compare against.
@@ -450,17 +450,17 @@ Blocklist.prototype = {
          return entry;
        }
      }
      return null;
   },
 
   /* See nsIBlocklistService */
   getAddonBlocklistURL(addon, appVersion, toolkitVersion) {
-    if (!this._isBlocklistLoaded())
+    if (!this.isLoaded)
       this._loadBlocklist();
 
     let entry = this._getAddonBlocklistEntry(addon, this._addonEntries);
     return entry && entry.url;
   },
 
   _createBlocklistURL(id) {
     let url = Services.urlFormatter.formatURLPref(PREF_BLOCKLIST_ITEM_URL);
@@ -579,17 +579,17 @@ Blocklist.prototype = {
     }
 
     request.addEventListener("error", event => this.onXMLError(event));
     request.addEventListener("load", event => this.onXMLLoad(event));
     request.send(null);
 
     // When the blocklist loads we need to compare it to the current copy so
     // make sure we have loaded it.
-    if (!this._isBlocklistLoaded())
+    if (!this.isLoaded)
       this._loadBlocklist();
 
     // If blocklist update via Kinto is enabled, poll for changes and sync.
     // Currently certificates blocklist relies on it by default.
     if (Services.prefs.getBoolPref(PREF_BLOCKLIST_UPDATE_ENABLED)) {
       BlocklistUpdater.checkVersions().catch(() => {
         // Bug 1254099 - Telemetry (success or errors) will be collected during this process.
       });
@@ -797,17 +797,17 @@ Blocklist.prototype = {
         } catch (ex) {}
       }
     }
 
     if (text)
       this._loadBlocklistFromString(text);
   },
 
-  _isBlocklistLoaded() {
+  get isLoaded() {
     return this._addonEntries != null && this._gfxEntries != null && this._pluginEntries != null;
   },
 
   _isBlocklistPreloaded() {
     return this._preloadedBlocklistContent != null;
   },
 
   /* Used for testing */
@@ -900,16 +900,22 @@ Blocklist.prototype = {
         }
       }
       if (this._gfxEntries.length > 0) {
         this._notifyObserversBlocklistGFX();
       }
     } catch (e) {
       LOG("Blocklist::_loadBlocklistFromXML: Error constructing blocklist " + e);
     }
+    // Dispatch to mainthread because consumers may try to construct nsIPluginHost
+    // again based on this notification, while we were called from nsIPluginHost
+    // anyway, leading to re-entrancy.
+    Services.tm.dispatchToMainThread(function() {
+      Services.obs.notifyObservers(null, "blocklist-loaded");
+    });
   },
 
   _processItemNodes(itemNodes, itemName, handler) {
     var result = [];
     for (var i = 0; i < itemNodes.length; ++i) {
       var blocklistElement = itemNodes.item(i);
       if (!(blocklistElement instanceof Ci.nsIDOMElement) ||
           blocklistElement.localName != itemName)
@@ -1084,17 +1090,17 @@ Blocklist.prototype = {
     result.push(blockEntry);
   },
 
   /* See nsIBlocklistService */
   getPluginBlocklistState(plugin, appVersion, toolkitVersion) {
     if (AppConstants.platform == "android") {
       return Ci.nsIBlocklistService.STATE_NOT_BLOCKED;
     }
-    if (!this._isBlocklistLoaded())
+    if (!this.isLoaded)
       this._loadBlocklist();
     return this._getPluginBlocklistState(plugin, this._pluginEntries,
                                          appVersion, toolkitVersion);
   },
 
   /**
    * Private helper to get the blocklist entry for a plugin given a set of
    * blocklist entries and versions.
@@ -1193,34 +1199,34 @@ Blocklist.prototype = {
         return Ci.nsIBlocklistService.STATE_VULNERABLE_NO_UPDATE;
       return Ci.nsIBlocklistService.STATE_OUTDATED;
     }
     return Ci.nsIBlocklistService.STATE_SOFTBLOCKED;
   },
 
   /* See nsIBlocklistService */
   getPluginBlocklistURL(plugin) {
-    if (!this._isBlocklistLoaded())
+    if (!this.isLoaded)
       this._loadBlocklist();
 
     let r = this._getPluginBlocklistEntry(plugin, this._pluginEntries);
     if (!r) {
       return null;
     }
     let {entry: blockEntry} = r;
     if (!blockEntry.blockID) {
       return null;
     }
 
     return this._createBlocklistURL(blockEntry.blockID);
   },
 
   /* See nsIBlocklistService */
   getPluginInfoURL(plugin) {
-    if (!this._isBlocklistLoaded())
+    if (!this.isLoaded)
       this._loadBlocklist();
 
     let r = this._getPluginBlocklistEntry(plugin, this._pluginEntries);
     if (!r) {
       return null;
     }
     let {entry: blockEntry} = r;
     if (!blockEntry.blockID) {
--- a/toolkit/mozapps/extensions/nsBlocklistServiceContent.js
+++ b/toolkit/mozapps/extensions/nsBlocklistServiceContent.js
@@ -79,16 +79,21 @@ Blocklist.prototype = {
   isAddonBlocklisted(aAddon, aAppVersion, aToolkitVersion) {
     return true;
   },
 
   getAddonBlocklistState(aAddon, aAppVersion, aToolkitVersion) {
     return Ci.nsIBlocklistService.STATE_BLOCKED;
   },
 
+  get isLoaded() {
+    // Lie until we fix bug 1443870.
+    return true;
+  },
+
   // There are a few callers in layout that rely on this.
   getPluginBlocklistState(aPluginTag, aAppVersion, aToolkitVersion) {
     return Services.cpmm.sendSyncMessage("Blocklist:getPluginBlocklistState", {
       addonData: this.flattenObject(aPluginTag),
       appVersion: aAppVersion,
       toolkitVersion: aToolkitVersion
     })[0];
   },
--- a/toolkit/mozapps/extensions/test/xpcshell/test_asyncBlocklistLoad.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_asyncBlocklistLoad.js
@@ -4,37 +4,37 @@
 
 add_task(async function() {
   let blocklist = AM_Cc["@mozilla.org/extensions/blocklist;1"].
                   getService().wrappedJSObject;
   let scope = ChromeUtils.import("resource://gre/modules/osfile.jsm", {});
 
   // sync -> async
   blocklist._loadBlocklist();
-  Assert.ok(blocklist._isBlocklistLoaded());
+  Assert.ok(blocklist.isLoaded);
   await blocklist._preloadBlocklist();
   Assert.ok(!blocklist._isBlocklistPreloaded());
   blocklist._clear();
 
   // async -> sync
   await blocklist._preloadBlocklist();
-  Assert.ok(!blocklist._isBlocklistLoaded());
+  Assert.ok(!blocklist.isLoaded);
   Assert.ok(blocklist._isBlocklistPreloaded());
   blocklist._loadBlocklist();
-  Assert.ok(blocklist._isBlocklistLoaded());
+  Assert.ok(blocklist.isLoaded);
   Assert.ok(!blocklist._isBlocklistPreloaded());
   blocklist._clear();
 
   // async -> sync -> async
   let read = scope.OS.File.read;
   scope.OS.File.read = function(...args) {
     return new Promise((resolve, reject) => {
       executeSoon(() => {
         blocklist._loadBlocklist();
         resolve(read(...args));
       });
     });
   };
 
   await blocklist._preloadBlocklist();
-  Assert.ok(blocklist._isBlocklistLoaded());
+  Assert.ok(blocklist.isLoaded);
   Assert.ok(!blocklist._isBlocklistPreloaded());
 });
--- a/xpcom/system/nsIBlocklistService.idl
+++ b/xpcom/system/nsIBlocklistService.idl
@@ -123,16 +123,21 @@ interface nsIBlocklistService : nsISuppo
   /**
    * Determine the blocklist infoURL of a plugin.
    * @param   plugin
    *          The blocked plugin that we are determining the infoURL for.
    * @returns The preferred URL to present the user, or |null| if
    *          it is not available.
    */
   AString getPluginInfoURL(in nsIPluginTag plugin);
+
+  /**
+   * Whether or not we've finished loading the blocklist.
+   */
+  readonly attribute boolean isLoaded;
 };
 
 /**
  * nsIBlocklistPrompt is used, if available, by the default implementation of 
  * nsIBlocklistService to display a confirmation UI to the user before blocking
  * extensions/plugins.
  */
 [scriptable, uuid(ba915921-b9c0-400d-8e4f-ca1b80c5699a)]