Bug 1258354 - Sanitize on shutdown adds 2 blockers, when only one is really needed. r=yoric
authorMarco Bonardo <mbonardo@mozilla.com>
Wed, 13 Apr 2016 14:08:18 +0200
changeset 331310 b7772a6288f8b1011e1dfc8629ced4f401e47c9c
parent 331309 529cff9c5663a3581dfb5e690727144f07e8e23c
child 331311 53c4ca2e1c43607bcce08089eaf60b4dd43b0235
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersyoric
bugs1258354
milestone48.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 1258354 - Sanitize on shutdown adds 2 blockers, when only one is really needed. r=yoric MozReview-Commit-ID: BbtZMIwT54N
browser/base/content/sanitize.js
--- a/browser/base/content/sanitize.js
+++ b/browser/base/content/sanitize.js
@@ -10,18 +10,16 @@ XPCOMUtils.defineLazyModuleGetter(this, 
 XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils",
                                   "resource://gre/modules/PlacesUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "FormHistory",
                                   "resource://gre/modules/FormHistory.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Downloads",
                                   "resource://gre/modules/Downloads.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Promise",
                                   "resource://gre/modules/Promise.jsm");
-XPCOMUtils.defineLazyModuleGetter(this, "PromiseUtils",
-                                  "resource://gre/modules/PromiseUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Task",
                                   "resource://gre/modules/Task.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "DownloadsCommon",
                                   "resource:///modules/DownloadsCommon.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "TelemetryStopwatch",
                                   "resource://gre/modules/TelemetryStopwatch.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "console",
                                   "resource://gre/modules/Console.jsm");
@@ -53,42 +51,45 @@ Sanitizer.prototype = {
   },
 
   /**
    * Deletes privacy sensitive data in a batch, according to user preferences.
    * Returns a promise which is resolved if no errors occurred.  If an error
    * occurs, a message is reported to the console and all other items are still
    * cleared before the promise is finally rejected.
    *
-   * If the consumer specifies the (optional) array parameter, only those
-   * items get cleared (irrespective of the preference settings)
+   * @param [optional] itemsToClear
+   *        Array of items to be cleared. if specified only those
+   *        items get cleared, irrespectively of the preference settings.
+   * @param [optional] options
+   *        Object whose properties are options for this sanitization.
+   *        TODO (bug 1167238) document options here.
    */
-  sanitize: Task.async(function*(aItemsToClear = null) {
-    let progress = {};
-    let promise = this._sanitize(aItemsToClear, progress);
+  sanitize: Task.async(function*(itemsToClear = null, options = {}) {
+    let progress = options.progress || {};
+    let promise = this._sanitize(itemsToClear, progress);
 
-    //
     // Depending on preferences, the sanitizer may perform asynchronous
     // work before it starts cleaning up the Places database (e.g. closing
     // windows). We need to make sure that the connection to that database
     // hasn't been closed by the time we use it.
-    //
-    let shutdownClient = Cc["@mozilla.org/browser/nav-history-service;1"]
-       .getService(Ci.nsPIPlacesDatabase)
-       .shutdownClient
-       .jsclient;
+    // Though, if this is a sanitize on shutdown, we already have a blocker.
+    if (!progress.isShutdown) {
+      let shutdownClient = Cc["@mozilla.org/browser/nav-history-service;1"]
+                             .getService(Ci.nsPIPlacesDatabase)
+                             .shutdownClient
+                             .jsclient;
+      shutdownClient.addBlocker("sanitize.js: Sanitize",
+        promise,
+        {
+          fetchState: () => ({ progress })
+        }
+      );
+    }
 
-    shutdownClient.addBlocker("sanitize.js: Sanitize",
-      promise,
-      {
-        fetchState: () => {
-          return { progress };
-        }
-      }
-    );
     try {
       yield promise;
     } finally {
       Services.obs.notifyObservers(null, "sanitizer-sanitization-complete", "");
     }
   }),
 
   _sanitize: Task.async(function*(aItemsToClear, progress = {}) {
@@ -828,56 +829,54 @@ Sanitizer.onStartup = Task.async(functio
   if (Preferences.has(Sanitizer.PREF_SANITIZE_DID_SHUTDOWN)) {
     // Reset the pref, so that if we crash before having a chance to
     // sanitize on shutdown, we will do at the next startup.
     // Flushing prefs has a cost, so do this only if necessary.
     Preferences.reset(Sanitizer.PREF_SANITIZE_DID_SHUTDOWN);
     Services.prefs.savePrefFile(null);
   }
 
-  // Make sure that we are triggered during shutdown, at the right time,
-  // and only once.
-  let placesClient = Cc["@mozilla.org/browser/nav-history-service;1"]
-                       .getService(Ci.nsPIPlacesDatabase)
-                       .shutdownClient
-                       .jsclient;
-
-  let deferredSanitization = PromiseUtils.defer();
-  let sanitizationInProgress = false;
-  let doSanitize = function() {
-    if (!sanitizationInProgress) {
-      sanitizationInProgress = true;
-      Sanitizer.onShutdown().catch(er => {Promise.reject(er) /* Do not return rejected promise */;}).then(() =>
-        deferredSanitization.resolve()
-      );
+  // Make sure that we are triggered during shutdown.
+  let shutdownClient = Cc["@mozilla.org/browser/nav-history-service;1"]
+                         .getService(Ci.nsPIPlacesDatabase)
+                         .shutdownClient
+                         .jsclient;
+  // We need to pass to sanitize() (through sanitizeOnShutdown) a state object
+  // that tracks the status of the shutdown blocker. This `progress` object
+  // will be updated during sanitization and reported with the crash in case of
+  // a shutdown timeout.
+  // We use the `options` argument to pass the `progress` object to sanitize().
+  let progress = { isShutdown: true };
+  shutdownClient.addBlocker("sanitize.js: Sanitize on shutdown",
+    () => sanitizeOnShutdown({ progress }),
+    {
+      fetchState: () => ({ progress })
     }
-    return deferredSanitization.promise;
-  }
-  placesClient.addBlocker("sanitize.js: Sanitize on shutdown", doSanitize);
+  );
 
   // Check if Firefox crashed during a sanitization.
   let lastInterruptedSanitization = Preferences.get(Sanitizer.PREF_SANITIZE_IN_PROGRESS, "");
   if (lastInterruptedSanitization) {
     let s = new Sanitizer();
     // If the json is invalid this will just throw and reject the Task.
     let itemsToClear = JSON.parse(lastInterruptedSanitization);
     yield s.sanitize(itemsToClear);
   } else if (shutownSanitizationWasInterrupted) {
     // Otherwise, could be we were supposed to sanitize on shutdown but we
     // didn't have a chance, due to an earlier crash.
     // In such a case, just redo a shutdown sanitize now, during startup.
-    yield Sanitizer.onShutdown();
+    yield sanitizeOnShutdown();
   }
 });
 
-Sanitizer.onShutdown = Task.async(function*() {
+var sanitizeOnShutdown = Task.async(function*(options = {}) {
   if (!Preferences.get(Sanitizer.PREF_SANITIZE_ON_SHUTDOWN)) {
     return;
   }
   // Need to sanitize upon shutdown
   let s = new Sanitizer();
   s.prefDomain = "privacy.clearOnShutdown.";
-  yield s.sanitize();
+  yield s.sanitize(null, options);
   // We didn't crash during shutdown sanitization, so annotate it to avoid
   // sanitizing again on startup.
   Preferences.set(Sanitizer.PREF_SANITIZE_DID_SHUTDOWN, true);
   Services.prefs.savePrefFile(null);
 });