Bug 1358907 Part 3 Avoid reading XPI database at startup r=Dexter

Switch telemetry and experiments from AddonManager.getAddonsByTypes()
to AddonManager.getActiveAddons() which gives us less detailed information
in the environment during startup but also means we don't need to load the
extensions database until startup is complete.

MozReview-Commit-ID: 4SxdPHSPovB

--HG--
extra : rebase_source : c4810207bd08c426bb2f5b63b93dfc38e0b0245a
extra : source : c391410c07ed5a9462ca7dec8c01037c9ab14466

browser/experiments/Experiments.jsm

@@ -164,14 +164,30 @@ function addonInstallForURL(url, hash) {
 // Returns a promise that is resolved with an Array<Addon> of the installed
 // experiment addons.
 function installedExperimentAddons() {
-  return AddonManager.getAddonsByTypes(["experiment"]).then(addons => {
+  return AddonManager.getActiveAddons(["experiment"]).then(addons => {
     return addons.filter(a => !a.appDisabled);
   });
 }
 
 // Takes an Array<Addon> and returns a promise that is resolved when the
 // addons are uninstalled.
-function uninstallAddons(addons) {
+async function uninstallAddons(addons) {
+  if (!AddonManagerPrivate.isDBLoaded()) {
+    await new Promise(resolve => {
+      Services.obs.addObserver({
+        observe(subject, topic, data) {
+          Services.obs.removeObserver(this, "xpi-database-loaded");
+          resolve();
+        },
+      }, "xpi-database-loaded");
+    });
+
+    // This function was called during startup so the addons that were
+    // passed in were partial addon objects.  Now that the full addons
+    // database is loaded, get proper Addon objects.
+    addons = await AddonManager.getAddonsByIDs(addons.map(a => a.id));
+  }
+
   let ids = new Set(addons.map(addon => addon.id));
   return new Promise(resolve => {
 
@@ -191,10 +207,6 @@ function uninstallAddons(addons) {
     AddonManager.addAddonListener(listener);
 
     for (let addon of addons) {
-      // Disabling the add-on before uninstalling is necessary to cause tests to
-      // pass. This might be indicative of a bug in XPIProvider.
-      // TODO follow up in bug 992396.
-      addon.userDisabled = true;
       addon.uninstall();
     }
 

browser/experiments/test/xpcshell/head.js

@@ -155,6 +155,7 @@ function startAddonManagerOnly() {
                        .getService(Ci.nsIObserver)
                        .QueryInterface(Ci.nsITimerCallback);
   addonManager.observe(null, "addons-startup", null);
+  Services.obs.notifyObservers(null, "sessionstore-windows-restored");
 }
 
 function getExperimentAddons(previous = false) {

toolkit/components/telemetry/TelemetryEnvironment.jsm

@@ -22,11 +22,12 @@ Cu.import("resource://gre/modules/AppConstants.jsm");
 
 const Utils = TelemetryUtils;
 
+const { AddonManager, AddonManagerPrivate } = Cu.import("resource://gre/modules/AddonManager.jsm", {});
+
 XPCOMUtils.defineLazyModuleGetter(this, "AttributionCode",
                                   "resource:///modules/AttributionCode.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "ctypes",
                                   "resource://gre/modules/ctypes.jsm");
-Cu.import("resource://gre/modules/AddonManager.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "LightweightThemeManager",
                                   "resource://gre/modules/LightweightThemeManager.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "ProfileAge",
@@ -482,13 +483,34 @@ EnvironmentAddonBuilder.prototype = {
       return Promise.reject(err);
     }
 
-    this._pendingTask = this._updateAddons().then(
+    this._pendingTask = (async () => {
-      () => { this._pendingTask = null; },
+      try {
-      (err) => {
+        // Gather initial addons details
+        await this._updateAddons();
+
+        if (!AddonManagerPrivate.isDBLoaded()) {
+          // The addon database has not been loaded, so listen for the event
+          // triggered by the AddonManager when it is loaded so we can
+          // immediately gather full data at that time.
+          await new Promise(resolve => {
+            const ADDON_LOAD_NOTIFICATION = "xpi-database-loaded";
+            Services.obs.addObserver({
+              observe(subject, topic, data) {
+                Services.obs.removeObserver(this, ADDON_LOAD_NOTIFICATION);
+                resolve();
+              },
+            }, ADDON_LOAD_NOTIFICATION);
+          });
+
+          // Now gather complete addons details.
+          await this._updateAddons();
+        }
+      } catch (err) {
         this._environment._log.error("init - Exception in _updateAddons", err);
+      } finally {
         this._pendingTask = null;
       }
-    );
+    })();
 
     return this._pendingTask;
   },
@@ -551,6 +573,12 @@ EnvironmentAddonBuilder.prototype = {
       AddonManager.removeAddonListener(this);
       Services.obs.removeObserver(this, EXPERIMENTS_CHANGED_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;
   },
 
@@ -601,44 +629,43 @@ EnvironmentAddonBuilder.prototype = {
    */
   async _getActiveAddons() {
     // Request addons, asynchronously.
-    let allAddons = await AddonManager.getAddonsByTypes(["extension", "service"]);
+    let allAddons = await AddonManager.getActiveAddons(["extension", "service"]);
 
+    let isDBLoaded = AddonManagerPrivate.isDBLoaded();
     let activeAddons = {};
     for (let addon of allAddons) {
-      // Skip addons which are not active.
-      if (!addon.isActive) {
-        continue;
-      }
-
       // Weird addon data in the wild can lead to exceptions while collecting
       // the data.
       try {
         // Make sure to have valid dates.
-        let installDate = new Date(Math.max(0, addon.installDate));
         let updateDate = new Date(Math.max(0, addon.updateDate));
 
         activeAddons[addon.id] = {
-          blocklisted: (addon.blocklistState !== Ci.nsIBlocklistService.STATE_NOT_BLOCKED),
-          description: limitStringToLength(addon.description, MAX_ADDON_STRING_LENGTH),
-          name: limitStringToLength(addon.name, MAX_ADDON_STRING_LENGTH),
-          userDisabled: enforceBoolean(addon.userDisabled),
-          appDisabled: addon.appDisabled,
           version: limitStringToLength(addon.version, MAX_ADDON_STRING_LENGTH),
           scope: addon.scope,
           type: addon.type,
-          foreignInstall: enforceBoolean(addon.foreignInstall),
-          hasBinaryComponents: addon.hasBinaryComponents,
-          installDay: Utils.millisecondsToDays(installDate.getTime()),
           updateDay: Utils.millisecondsToDays(updateDate.getTime()),
-          signedState: addon.signedState,
           isSystem: addon.isSystem,
           isWebExtension: addon.isWebExtension,
           multiprocessCompatible: Boolean(addon.multiprocessCompatible),
         };
 
-        if (addon.signedState !== undefined)
+        // getActiveAddons() gives limited data during startup and full
-          activeAddons[addon.id].signedState = addon.signedState;
+        // data after the addons database is loaded.
-
+        if (isDBLoaded) {
+          let installDate = new Date(Math.max(0, addon.installDate));
+          Object.assign(activeAddons[addon.id], {
+            blocklisted: (addon.blocklistState !== Ci.nsIBlocklistService.STATE_NOT_BLOCKED),
+            description: limitStringToLength(addon.description, MAX_ADDON_STRING_LENGTH),
+            name: limitStringToLength(addon.name, MAX_ADDON_STRING_LENGTH),
+            userDisabled: enforceBoolean(addon.userDisabled),
+            appDisabled: addon.appDisabled,
+            foreignInstall: enforceBoolean(addon.foreignInstall),
+            hasBinaryComponents: addon.hasBinaryComponents,
+            installDay: Utils.millisecondsToDays(installDate.getTime()),
+            signedState: addon.signedState,
+          });
+        }
       } catch (ex) {
         this._environment._log.error("_getActiveAddons - An addon was discarded due to an error", ex);
         continue;
@@ -654,7 +681,7 @@ EnvironmentAddonBuilder.prototype = {
    */
   async _getActiveTheme() {
     // Request themes, asynchronously.
-    let themes = await AddonManager.getAddonsByTypes(["theme"]);
+    let themes = await AddonManager.getActiveAddons(["theme"]);
 
     let activeTheme = {};
     // We only store information about the active theme.

toolkit/components/telemetry/docs/data/environment.rst

@@ -215,7 +215,7 @@ Structure:
             scope: <integer>,
             type: <string>, // "extension", "service", ...
             foreignInstall: <bool>,
-            hasBinaryComponents: <bool>
+            hasBinaryComponents: <bool>,
             installDay: <number>, // days since UNIX epoch, 0 on failure
             updateDay: <number>, // days since UNIX epoch, 0 on failure
             signedState: <integer>, // whether the add-on is signed by AMO, only present for extensions
@@ -395,6 +395,8 @@ 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 addon 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.
+
 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``.

toolkit/components/telemetry/tests/unit/head.js

@@ -179,6 +179,10 @@ function loadAddonManager(...args) {
   return AddonTestUtils.promiseStartupManager();
 }
 
+function finishAddonManagerStartup() {
+  Services.obs.notifyObservers(null, "test-load-xpi-database");
+}
+
 var gAppInfo = null;
 
 function createAppInfo(ID = "xpcshell@tests.mozilla.org", name = "XPCShell",

toolkit/components/telemetry/tests/unit/test_ChildEvents.js

@@ -66,6 +66,7 @@ add_task(async function() {
   // Setup.
   do_get_profile(true);
   loadAddonManager(APP_ID, APP_NAME, APP_VERSION, PLATFORM_VERSION);
+  finishAddonManagerStartup();
   Services.prefs.setBoolPref(PREF_TELEMETRY_ENABLED, true);
   await TelemetryController.testSetup();
   // Make sure we don't generate unexpected pings due to pref changes.

toolkit/components/telemetry/tests/unit/test_ChildHistograms.js

@@ -91,6 +91,7 @@ add_task(async function() {
   // Setup.
   do_get_profile(true);
   loadAddonManager(APP_ID, APP_NAME, APP_VERSION, PLATFORM_VERSION);
+  finishAddonManagerStartup();
   Services.prefs.setBoolPref(PREF_TELEMETRY_ENABLED, true);
   await TelemetryController.testSetup();
   if (runningInParent) {

toolkit/components/telemetry/tests/unit/test_ChildScalars.js

@@ -140,6 +140,7 @@ add_task(async function() {
   // Setup.
   do_get_profile(true);
   loadAddonManager(APP_ID, APP_NAME, APP_VERSION, PLATFORM_VERSION);
+  finishAddonManagerStartup();
   Services.prefs.setBoolPref(PREF_TELEMETRY_ENABLED, true);
   await TelemetryController.testSetup();
   if (runningInParent) {

toolkit/components/telemetry/tests/unit/test_SubsessionChaining.js

@@ -88,6 +88,7 @@ add_task(async function test_setup() {
   // Addon manager needs a profile directory
   do_get_profile();
   loadAddonManager("xpcshell@tests.mozilla.org", "XPCShell", "1", "1.9.2");
+  finishAddonManagerStartup();
   // Make sure we don't generate unexpected pings due to pref changes.
   await setEmptyPrefWatchlist();
 

toolkit/components/telemetry/tests/unit/test_TelemetryController.js

@@ -98,6 +98,7 @@ add_task(async function test_setup() {
   // Addon manager needs a profile directory
   do_get_profile();
   loadAddonManager("xpcshell@tests.mozilla.org", "XPCShell", "1", "1.9.2");
+  finishAddonManagerStartup();
   // Make sure we don't generate unexpected pings due to pref changes.
   await setEmptyPrefWatchlist();
 

toolkit/components/telemetry/tests/unit/test_TelemetryControllerShutdown.js

@@ -29,6 +29,7 @@ add_task(async function test_setup() {
   // Addon manager needs a profile directory
   do_get_profile();
   loadAddonManager("xpcshell@tests.mozilla.org", "XPCShell", "1", "1.9.2");
+  finishAddonManagerStartup();
   // Make sure we don't generate unexpected pings due to pref changes.
   await setEmptyPrefWatchlist();
 

toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js

@@ -1,10 +1,12 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
-Cu.import("resource://gre/modules/AddonManager.jsm");
+const {AddonManager, AddonManagerPrivate} = Cu.import("resource://gre/modules/AddonManager.jsm", {});
 Cu.import("resource://gre/modules/TelemetryEnvironment.jsm", this);
+Cu.import("resource://gre/modules/ObjectUtils.jsm");
 Cu.import("resource://gre/modules/Preferences.jsm", this);
 Cu.import("resource://gre/modules/PromiseUtils.jsm", this);
+Cu.import("resource://gre/modules/Timer.jsm", this);
 Cu.import("resource://gre/modules/XPCOMUtils.jsm", this);
 Cu.import("resource://testing-common/AddonManagerTesting.jsm");
 Cu.import("resource://testing-common/httpd.js");
@@ -657,38 +659,48 @@ function checkSystemSection(data) {
   } catch (e) {}
 }
 
-function checkActiveAddon(data) {
+function checkActiveAddon(data, partialRecord) {
   let signedState = mozinfo.addon_signing ? "number" : "undefined";
   // system add-ons have an undefined signState
   if (data.isSystem)
     signedState = "undefined";
 
   const EXPECTED_ADDON_FIELDS_TYPES = {
-    blocklisted: "boolean",
-    name: "string",
-    userDisabled: "boolean",
-    appDisabled: "boolean",
     version: "string",
     scope: "number",
     type: "string",
-    foreignInstall: "boolean",
-    hasBinaryComponents: "boolean",
-    installDay: "number",
     updateDay: "number",
-    signedState,
     isSystem: "boolean",
     isWebExtension: "boolean",
     multiprocessCompatible: "boolean",
   };
 
-  for (let f in EXPECTED_ADDON_FIELDS_TYPES) {
+  const FULL_ADDON_FIELD_TYPES = {
-    Assert.ok(f in data, f + " must be available.");
+    blocklisted: "boolean",
-    Assert.equal(typeof data[f], EXPECTED_ADDON_FIELDS_TYPES[f],
+    name: "string",
-                 f + " must have the correct type.");
+    userDisabled: "boolean",
+    appDisabled: "boolean",
+    foreignInstall: "boolean",
+    hasBinaryComponents: "boolean",
+    installDay: "number",
+    signedState,
+  };
+
+  let fields = EXPECTED_ADDON_FIELDS_TYPES;
+  if (!partialRecord) {
+    fields = Object.assign({}, fields, FULL_ADDON_FIELD_TYPES);
   }
 
+  for (let [name, type] of Object.entries(fields)) {
+    Assert.ok(name in data, name + " must be available.");
+    Assert.equal(typeof data[name], type,
+                 name + " must have the correct type.");
+  }
+
+  if (!partialRecord) {
     // We check "description" separately, as it can be null.
     Assert.ok(checkNullOrString(data.description));
+  }
 }
 
 function checkPlugin(data) {
@@ -748,7 +760,7 @@ function checkActiveGMPlugin(data) {
   Assert.equal(typeof data.applyBackgroundUpdates, "number");
 }
 
-function checkAddonsSection(data, expectBrokenAddons) {
+function checkAddonsSection(data, expectBrokenAddons, partialAddonsRecords) {
   const EXPECTED_FIELDS = [
     "activeAddons", "theme", "activePlugins", "activeGMPlugins", "activeExperiment",
     "persona",
@@ -763,7 +775,7 @@ function checkAddonsSection(data, expectBrokenAddons) {
   if (!expectBrokenAddons) {
     let activeAddons = data.addons.activeAddons;
     for (let addon in activeAddons) {
-      checkActiveAddon(activeAddons[addon]);
+      checkActiveAddon(activeAddons[addon], partialAddonsRecords);
     }
   }
 
@@ -815,7 +827,12 @@ function checkExperimentsSection(data) {
   }
 }
 
-function checkEnvironmentData(data, isInitial = false, expectBrokenAddons = false) {
+function checkEnvironmentData(data, options = {}) {
+  const {
+    isInitial = false,
+    expectBrokenAddons = false,
+  } = options;
+
   checkBuildSection(data);
   checkSettingsSection(data);
   checkProfileSection(data);
@@ -843,6 +860,14 @@ add_task(async function setup() {
   // Spoof the persona ID.
   LightweightThemeManager.currentTheme =
     spoofTheme(PERSONA_ID, PERSONA_NAME, PERSONA_DESCRIPTION);
+
+  // The test runs in a fresh profile so starting the AddonManager causes
+  // the addons database to be created (as does setting new theme).
+  // For test_addonsStartup below, we want to test a "warm" startup where
+  // there is already a database on disk.  Simulate that here by just
+  // restarting the AddonManager.
+  await AddonTestUtils.promiseRestartManager();
+
   // Register a fake plugin host for consistent flash version data.
   registerFakePluginHost();
 
@@ -873,8 +898,19 @@ add_task(async function setup() {
 });
 
 add_task(async function test_checkEnvironment() {
-  let environmentData = await TelemetryEnvironment.onInitialized();
+  // During startup we have partial addon records.
-  checkEnvironmentData(environmentData, true);
+  // First make sure we haven't yet read the addons DB, then test that
+  // we have some partial addons data.
+  Assert.equal(AddonManagerPrivate.isDBLoaded(), false,
+               "addons database is not loaded");
+
+  checkAddonsSection(TelemetryEnvironment.currentEnvironment, false, true);
+
+  // Now continue with startup.
+  let initPromise = TelemetryEnvironment.onInitialized();
+  finishAddonManagerStartup();
+  let environmentData = await initPromise;
+  checkEnvironmentData(environmentData, {isInitial: true});
 
   spoofPartnerInfo();
   Services.obs.notifyObservers(null, DISTRIBUTION_CUSTOMIZATION_COMPLETE_TOPIC);
@@ -1412,7 +1448,7 @@ add_task(async function test_collectionWithbrokenAddonData() {
   // Check that the new environment contains the Social addon installed with the broken
   // manifest and the rest of the data.
   let data = TelemetryEnvironment.currentEnvironment;
-  checkEnvironmentData(data, false, true /* expect broken addons*/);
+  checkEnvironmentData(data, {expectBrokenAddons: true});
 
   let activeAddons = data.addons.activeAddons;
   Assert.ok(BROKEN_ADDON_ID in activeAddons,
@@ -1434,7 +1470,8 @@ add_task(async function test_collectionWithbrokenAddonData() {
 add_task(async function test_defaultSearchEngine() {
   // Check that no default engine is in the environment before the search service is
   // initialized.
-  let data = TelemetryEnvironment.currentEnvironment;
+
+  let data = await TelemetryEnvironment.testCleanRestart().onInitialized();
   checkEnvironmentData(data);
   Assert.ok(!("defaultSearchEngine" in data.settings));
   Assert.ok(!("defaultSearchEngineData" in data.settings));

toolkit/components/telemetry/tests/unit/test_TelemetryReportingPolicy.js

@@ -60,6 +60,7 @@ add_task(async function test_setup() {
   // Addon manager needs a profile directory
   do_get_profile(true);
   loadAddonManager("xpcshell@tests.mozilla.org", "XPCShell", "1", "1.9.2");
+  finishAddonManagerStartup();
 
   // Make sure we don't generate unexpected pings due to pref changes.
   await setEmptyPrefWatchlist();

toolkit/components/telemetry/tests/unit/test_TelemetrySendOldPings.js

@@ -146,6 +146,7 @@ add_task(async function test_setup() {
   PingServer.registerPingHandler(pingHandler);
   do_get_profile();
   loadAddonManager("xpcshell@tests.mozilla.org", "XPCShell", "1", "1.9.2");
+  finishAddonManagerStartup();
   // Make sure we don't generate unexpected pings due to pref changes.
   await setEmptyPrefWatchlist();
 

toolkit/components/telemetry/tests/unit/test_TelemetrySession.js

@@ -480,6 +480,7 @@ add_task(async function test_setup() {
   // Addon manager needs a profile directory
   do_get_profile();
   loadAddonManager(APP_ID, APP_NAME, APP_VERSION, PLATFORM_VERSION);
+  finishAddonManagerStartup();
   // Make sure we don't generate unexpected pings due to pref changes.
   await setEmptyPrefWatchlist();
 

toolkit/components/telemetry/tests/unit/test_TelemetrySession_activeTicks.js

@@ -10,6 +10,7 @@ add_task(async function test_setup() {
   // Addon manager needs a profile directory
   do_get_profile();
   loadAddonManager("xpcshell@tests.mozilla.org", "XPCShell", "1", "1.9.2");
+  finishAddonManagerStartup();
   // Make sure we don't generate unexpected pings due to pref changes.
   await setEmptyPrefWatchlist();
 

toolkit/components/telemetry/tests/unit/test_TelemetryTimestamps.js

@@ -24,6 +24,7 @@ function getSimpleMeasurementsFromTelemetryController() {
 add_task(async function test_setup() {
   // Telemetry needs the AddonManager.
   loadAddonManager();
+  finishAddonManagerStartup();
   // Make profile available for |TelemetryController.testShutdown()|.
   do_get_profile();
 

toolkit/mozapps/extensions/internal/XPIProvider.jsm

@@ -2325,16 +2325,25 @@ this.XPIProvider = {
       // XPI database so that the telemetry environment can be populated
       // with detailed addon information.
       if (!this.isDBLoaded) {
-        Services.obs.addObserver({
+        // The test-load-xpi-database event is only triggered from
+        // tests, and only as a temporary workaround for bug 1372845.
+        // This can be cleaned up when that bug is resolved.
+        const EVENTS = [ "sessionstore-windows-restored", "test-load-xpi-database" ];
+        let observer = {
           observe(subject, topic, data) {
-            Services.obs.removeObserver(this, "sessionstore-windows-restored");
+            for (let event of EVENTS) {
+              Services.obs.removeObserver(this, event);
+            }
 
             // It would be nice to defer some of the work here until we
             // have idle time but we can't yet use requestIdleCallback()
             // from chrome.  See bug 1358476.
             XPIDatabase.asyncLoadDB();
           },
-        }, "sessionstore-windows-restored");
+        };
+        for (let event of EVENTS) {
+          Services.obs.addObserver(observer, event);
+        }
       }
 
       AddonManagerPrivate.recordTimestamp("XPI_startup_end");