Bug 1405491 Switch XPIDatabase over to DeferredTask r=kmag
authorAndrew Swan <aswan@mozilla.com>
Mon, 02 Oct 2017 13:51:27 -0700
changeset 444612 cfd90570ec7a01d0d1933a036ec1c16201a06dd1
parent 444611 cfce78c33f60e67a6c2068002e38721047eb3ba8
child 444613 9bc2b08cbd66d92307f05c735697ff916985c35a
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskmag
bugs1405491
milestone58.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1405491 Switch XPIDatabase over to DeferredTask r=kmag MozReview-Commit-ID: Kjfl2FIM5F8
toolkit/mozapps/extensions/internal/AddonTestUtils.jsm
toolkit/mozapps/extensions/internal/XPIProvider.jsm
toolkit/mozapps/extensions/internal/XPIProviderUtils.js
--- a/toolkit/mozapps/extensions/internal/AddonTestUtils.jsm
+++ b/toolkit/mozapps/extensions/internal/AddonTestUtils.jsm
@@ -611,17 +611,17 @@ var AddonTestUtils = {
         // Clear any crash report annotations
         this.appInfo.annotations = {};
 
         // Force the XPIProvider provider to reload to better
         // simulate real-world usage.
         let XPIscope = Cu.import("resource://gre/modules/addons/XPIProvider.jsm", {});
         // This would be cleaner if I could get it as the rejection reason from
         // the AddonManagerInternal.shutdown() promise
-        let shutdownError = XPIscope.XPIProvider._shutdownError;
+        let shutdownError = XPIscope.XPIDatabase._saveError;
 
         AddonManagerPrivate.unregisterProvider(XPIscope.XPIProvider);
         Cu.unload("resource://gre/modules/addons/XPIProvider.jsm");
         Cu.unload("resource://gre/modules/addons/XPIInstall.jsm");
 
         // We need to set this in order reset the startup service, which
         // is only possible when running in automation.
         Services.prefs.setBoolPref(PREF_DISABLE_SECURITY, true);
--- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ -2086,18 +2086,17 @@ this.XPIProvider = {
     try {
       AddonManagerPrivate.recordTimestamp("XPI_startup_begin");
 
       logger.debug("startup");
       this.runPhase = XPI_STARTING;
       this.installs = new Set();
       this.installLocations = [];
       this.installLocationsByName = {};
-      // Hook for tests to detect when saving database at shutdown time fails
-      this._shutdownError = null;
+
       // Clear this at startup for xpcshell test restarts
       this._telemetryDetails = {};
       // Register our details structure with AddonManager
       AddonManagerPrivate.setTelemetryDetails("XPI", this._telemetryDetails);
 
       let hasRegistry = ("nsIWindowsRegKey" in Ci);
 
       let enabledScopes = Services.prefs.getIntPref(PREF_EM_ENABLED_SCOPES,
@@ -2374,21 +2373,17 @@ this.XPIProvider = {
     this.installs = null;
     this.installLocations = null;
     this.installLocationsByName = null;
 
     // This is needed to allow xpcshell tests to simulate a restart
     this.extensionsActive = false;
     this._addonFileMap.clear();
 
-    try {
-      await XPIDatabase.shutdown();
-    } catch (err) {
-      this._shutdownError = err;
-    }
+    await XPIDatabase.shutdown();
   },
 
   cleanupTemporaryAddons() {
     let tempLocation = XPIStates.getLocation(TemporaryInstallLocation.name);
     if (tempLocation) {
       for (let [id, addon] of tempLocation.entries()) {
         tempLocation.delete(id);
 
--- a/toolkit/mozapps/extensions/internal/XPIProviderUtils.js
+++ b/toolkit/mozapps/extensions/internal/XPIProviderUtils.js
@@ -11,28 +11,27 @@
           flushChromeCaches, descriptorToPath */
 
 var Cc = Components.classes;
 var Ci = Components.interfaces;
 var Cr = Components.results;
 var Cu = Components.utils;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
-Cu.import("resource://gre/modules/Services.jsm");
-Cu.import("resource://gre/modules/AddonManager.jsm");
-/* globals AddonManagerPrivate*/
 
-XPCOMUtils.defineLazyModuleGetter(this, "AddonRepository",
-                                  "resource://gre/modules/addons/AddonRepository.jsm");
-XPCOMUtils.defineLazyModuleGetter(this, "FileUtils",
-                                  "resource://gre/modules/FileUtils.jsm");
-XPCOMUtils.defineLazyModuleGetter(this, "DeferredSave",
-                                  "resource://gre/modules/DeferredSave.jsm");
-XPCOMUtils.defineLazyModuleGetter(this, "OS",
-                                  "resource://gre/modules/osfile.jsm");
+XPCOMUtils.defineLazyModuleGetters(this, {
+  AddonManager: "resource://gre/modules/AddonManager.jsm",
+  AddonManagerPrivate: "resource://gre/modules/AddonManager.jsm",
+  AddonRepository: "resource://gre/modules/addons/AddonRepository.jsm",
+  DeferredTask: "resource://gre/modules/DeferredTask.jsm",
+  FileUtils: "resource://gre/modules/FileUtils.jsm",
+  OS: "resource://gre/modules/osfile.jsm",
+  Services: "resource://gre/modules/Services.jsm",
+});
+
 XPCOMUtils.defineLazyServiceGetter(this, "Blocklist",
                                    "@mozilla.org/extensions/blocklist;1",
                                    Ci.nsIBlocklistService);
 
 Cu.import("resource://gre/modules/Log.jsm");
 const LOGGER_ID = "addons.xpi-utils";
 
 const nsIFile = Components.Constructor("@mozilla.org/file/local;1", "nsIFile",
@@ -275,84 +274,86 @@ this.XPIDatabase = {
   initialized: false,
   // The database file
   jsonFile: FileUtils.getFile(KEY_PROFILEDIR, [FILE_JSON_DB], true),
   // Migration data loaded from an old version of the database.
   migrateData: null,
   // Active add-on directories loaded from extensions.ini and prefs at startup.
   activeBundles: null,
 
+  _saveTask: null,
+
   // Saved error object if we fail to read an existing database
   _loadError: null,
 
+  // Saved error object if we fail to save the database
+  _saveError: null,
+
   // Error reported by our most recent attempt to read or write the database, if any
   get lastError() {
     if (this._loadError)
       return this._loadError;
-    if (this._deferredSave)
-      return this._deferredSave.lastError;
+    if (this._saveError)
+      return this._saveError;
     return null;
   },
 
+  async _saveNow() {
+    try {
+      let json = JSON.stringify(this);
+      let path = this.jsonFile.path;
+      await OS.File.writeAtomic(path, json, {tmpPath: `${path}.tmp`});
+
+      if (!this._schemaVersionSet) {
+        // Update the XPIDB schema version preference the first time we
+        // successfully save the database.
+        logger.debug("XPI Database saved, setting schema version preference to " + DB_SCHEMA);
+        Services.prefs.setIntPref(PREF_DB_SCHEMA, DB_SCHEMA);
+        this._schemaVersionSet = true;
+
+        // Reading the DB worked once, so we don't need the load error
+        this._loadError = null;
+      }
+    } catch (error) {
+      logger.warn("Failed to save XPI database", error);
+      this._saveError = error;
+      throw error;
+    }
+  },
+
   /**
    * Mark the current stored data dirty, and schedule a flush to disk
    */
   saveChanges() {
     if (!this.initialized) {
       throw new Error("Attempt to use XPI database when it is not initialized");
     }
 
     if (XPIProvider._closing) {
       // use an Error here so we get a stack trace.
       let err = new Error("XPI database modified after shutdown began");
       logger.warn(err);
       AddonManagerPrivate.recordSimpleMeasure("XPIDB_late_stack", Log.stackTrace(err));
     }
 
-    if (!this._deferredSave) {
-      this._deferredSave = new DeferredSave(this.jsonFile.path,
-                                            () => JSON.stringify(this),
-                                            ASYNC_SAVE_DELAY_MS);
+    if (!this._saveTask) {
+      this._saveTask = new DeferredTask(() => this._saveNow(),
+                                        ASYNC_SAVE_DELAY_MS);
     }
 
-    let promise = this._deferredSave.saveChanges();
-    if (!this._schemaVersionSet) {
-      this._schemaVersionSet = true;
-      promise = promise.then(
-        count => {
-          // Update the XPIDB schema version preference the first time we successfully
-          // save the database.
-          logger.debug("XPI Database saved, setting schema version preference to " + DB_SCHEMA);
-          Services.prefs.setIntPref(PREF_DB_SCHEMA, DB_SCHEMA);
-          // Reading the DB worked once, so we don't need the load error
-          this._loadError = null;
-        },
-        error => {
-          // Need to try setting the schema version again later
-          this._schemaVersionSet = false;
-          // this._deferredSave.lastError has the most recent error so we don't
-          // need this any more
-          this._loadError = null;
+    this._saveTask.arm();
+  },
 
-          throw error;
-        });
+  async finalize() {
+    // handle the "in memory only" and "saveChanges never called" cases
+    if (!this._saveTask) {
+      return;
     }
 
-    promise.catch(error => {
-      logger.warn("Failed to save XPI database", error);
-    });
-  },
-
-  flush() {
-    // handle the "in memory only" and "saveChanges never called" cases
-    if (!this._deferredSave) {
-      return Promise.resolve(0);
-    }
-
-    return this._deferredSave.flush();
+    await this._saveTask.finalize();
   },
 
   /**
    * Converts the current internal state of the XPI addon database to
    * a JSON.stringify()-ready structure
    */
   toJSON() {
     if (!this.addonDB) {
@@ -653,49 +654,36 @@ this.XPIDatabase = {
     logger.debug("shutdown");
     if (this.initialized) {
       // If our last database I/O had an error, try one last time to save.
       if (this.lastError)
         this.saveChanges();
 
       this.initialized = false;
 
-      if (this._deferredSave) {
-        AddonManagerPrivate.recordSimpleMeasure(
-            "XPIDB_saves_total", this._deferredSave.totalSaves);
-        AddonManagerPrivate.recordSimpleMeasure(
-            "XPIDB_saves_overlapped", this._deferredSave.overlappedSaves);
-        AddonManagerPrivate.recordSimpleMeasure(
-            "XPIDB_saves_late", this._deferredSave.dirty ? 1 : 0);
-      }
-
       // If we're shutting down while still loading, finish loading
       // before everything else!
       if (this._dbPromise) {
         await this._dbPromise;
       }
 
-      // Await and pending DB writes and finish cleaning up.
-      try {
-        await this.flush();
-      } catch (error) {
-        logger.error("Flush of XPI database failed", error);
-        AddonManagerPrivate.recordSimpleMeasure("XPIDB_shutdownFlush_failed", 1);
+      // Await any pending DB writes and finish cleaning up.
+      await this.finalize();
+
+      if (this._saveError) {
         // If our last attempt to read or write the DB failed, force a new
         // extensions.ini to be written to disk on the next startup
         Services.prefs.setBoolPref(PREF_PENDING_OPERATIONS, true);
-
-        throw error;
       }
 
       // Clear out the cached addons data loaded from JSON
       delete this.addonDB;
       delete this._dbPromise;
       // same for the deferred save
-      delete this._deferredSave;
+      delete this._saveTask;
       // re-enable the schema version setter
       delete this._schemaVersionSet;
     }
   },
 
   /**
    * Asynchronously list all addons that match the filter function
    * @param  aFilter