Bug 1243549 - SessionFile.wipe() now waits until SessionFile has been properly initialized;r?mconley draft
authorDavid Rajchenbach-Teller <dteller@mozilla.com>
Tue, 02 Feb 2016 12:56:11 +0100
changeset 328905 c5800e9a9b1d41d9387c081b21702eaeff84031e
parent 326148 211a4c710fb6af2cad10102c4cabc7cb525998b8
child 328906 13a04097703ef6fec1b3842d32391bbceb91e48a
push id10430
push userdteller@mozilla.com
push dateThu, 04 Feb 2016 13:54:01 +0000
reviewersmconley
bugs1243549, 1089695
milestone47.0a1
Bug 1243549 - SessionFile.wipe() now waits until SessionFile has been properly initialized;r?mconley While investigating bug 1243549, we encountered several instances of the following error message during each startup: ************************* A coding exception was thrown and uncaught in a Task. Full message: TypeError: this.Paths is null Full stack: Agent.wipe@resource:///modules/sessionstore/SessionWorker.js:296:7 worker.dispatch@resource:///modules/sessionstore/SessionWorker.js:21:24 anonymous/AbstractWorker.prototype.handleMessage@resource://gre/modules/workers/PromiseWorker.js:122:16 @resource:///modules/sessionstore/SessionWorker.js:30:41 ************************* These messages can be explained as follows: * If sanitization has failed during shutdown, it attempts again to sanitize during startup. This happens more often than it used to, because of 1/ startup bug fixes in bug 1089695; 2/ new shutdown bugs most likely also added by or around bug 1089695. * Sanitization during startup doesn't wait until Session Restore has properly started to sanitize the session. So sanitization of Session Restore file fails. This has probably always been the case, except we never noticed. * For some reason I do not understand, it attempts to sanitize several times. * I suspect that this can cause problems during startup, as sanitization and Session Restore race to use/remove the files of Session Restore. This patch makes sure that SessionFile.wipe() waits until initialization of SessionFile is complete before proceeding.
browser/components/sessionstore/SessionFile.jsm
--- a/browser/components/sessionstore/SessionFile.jsm
+++ b/browser/components/sessionstore/SessionFile.jsm
@@ -34,16 +34,18 @@ Cu.import("resource://gre/modules/Servic
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/osfile.jsm");
 Cu.import("resource://gre/modules/Promise.jsm");
 Cu.import("resource://gre/modules/AsyncShutdown.jsm");
 Cu.import("resource://gre/modules/Preferences.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "console",
   "resource://gre/modules/Console.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "PromiseUtils",
+  "resource://gre/modules/PromiseUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "RunState",
   "resource:///modules/sessionstore/RunState.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "TelemetryStopwatch",
   "resource://gre/modules/TelemetryStopwatch.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Task",
   "resource://gre/modules/Task.jsm");
 XPCOMUtils.defineLazyServiceGetter(this, "Telemetry",
   "@mozilla.org/base/telemetry;1", "nsITelemetry");
@@ -174,16 +176,20 @@ var SessionFileInternal = {
       return order;
     },
   }),
 
   // `true` once `write` has succeeded at last once.
   // Used for error-reporting.
   _hasWriteEverSucceeded: false,
 
+  // Resolved once initialization is complete.
+  // The promise never rejects.
+  _deferredInitialized: PromiseUtils.defer(),
+
   // The ID of the latest version of Gecko for which we have an upgrade backup
   // or |undefined| if no upgrade backup was ever written.
   get latestUpgradeBackupID() {
     try {
       return Services.prefs.getCharPref(PREF_UPGRADE_BACKUP);
     } catch (ex) {
       return undefined;
     }
@@ -250,22 +256,24 @@ var SessionFileInternal = {
         parsed: null
       };
     }
 
     result.noFilesFound = noFilesFound;
 
     // Initialize the worker to let it handle backups and also
     // as a workaround for bug 964531.
-    SessionWorker.post("init", [result.origin, this.Paths, {
+    let initialized = SessionWorker.post("init", [result.origin, this.Paths, {
       maxUpgradeBackups: Preferences.get(PREF_MAX_UPGRADE_BACKUPS, 3),
       maxSerializeBack: Preferences.get(PREF_MAX_SERIALIZE_BACK, 10),
       maxSerializeForward: Preferences.get(PREF_MAX_SERIALIZE_FWD, -1)
     }]);
 
+    initialized.catch(Promise.reject).then(() => this._deferredInitialized.resolve());
+
     return result;
   }),
 
   write: function (aData) {
     if (RunState.isClosed) {
       return Promise.reject(new Error("SessionFile is closed"));
     }
 
@@ -276,17 +284,17 @@ var SessionFileInternal = {
       isFinalWrite = true;
       RunState.setClosed();
     }
 
     let performShutdownCleanup = isFinalWrite &&
       !sessionStartup.isAutomaticRestoreEnabled();
 
     let options = {isFinalWrite, performShutdownCleanup};
-    let promise = SessionWorker.post("write", [aData, options]);
+    let promise = this._deferredInitialized.promise.then(() => SessionWorker.post("write", [aData, options]));
 
     // Wait until the write is done.
     promise = promise.then(msg => {
       // Record how long the write took.
       this._recordTelemetry(msg.telemetry);
       this._hasWriteEverSucceeded = true;
       if (msg.result.upgradeBackup) {
         // We have just completed a backup-on-upgrade, store the information
@@ -323,17 +331,17 @@ var SessionFileInternal = {
 
       if (isFinalWrite) {
         Services.obs.notifyObservers(null, "sessionstore-final-state-write-complete", "");
       }
     });
   },
 
   wipe: function () {
-    return SessionWorker.post("wipe");
+    return this._deferredInitialized.promise.then(() => SessionWorker.post("wipe"));
   },
 
   _recordTelemetry: function(telemetry) {
     for (let id of Object.keys(telemetry)){
       let value = telemetry[id];
       let samples = [];
       if (Array.isArray(value)) {
         samples.push(...value);