Bug 566746 - various fixes to sanitize(). r=mak

browser/base/content/sanitize.js

@@ -8,6 +8,8 @@ XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils",
                                   "resource://gre/modules/PlacesUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "FormHistory",
                                   "resource://gre/modules/FormHistory.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "Promise",
+                                  "resource://gre/modules/commonjs/sdk/core/promise.js");
 
 function Sanitizer() {}
 Sanitizer.prototype = {
@@ -38,15 +40,18 @@ Sanitizer.prototype = {
   },
   
   /**
-   * Deletes privacy sensitive data in a batch, according to user preferences. Calls
-   * errorHandler with a list of errors on failure.
+   * 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.
    */
-  sanitize: function (errorHandler)
+  sanitize: function ()
   {
+    var deferred = Promise.defer();
     var psvc = Components.classes["@mozilla.org/preferences-service;1"]
                          .getService(Components.interfaces.nsIPrefService);
     var branch = psvc.getBranch(this.prefDomain);
-    var errors = null;
+    var seenError = false;
 
     // Cache the range of times to clear
     if (this.ignoreTimespan)
@@ -54,7 +59,12 @@ Sanitizer.prototype = {
     else
       range = this.range || Sanitizer.getClearRange();
 
-    let itemCount = this.items.length;
+    let itemCount = Object.keys(this.items).length;
+    let onItemComplete = function() {
+      if (!--itemCount) {
+        seenError ? deferred.reject() : deferred.resolve();
+      }
+    };
     for (var itemName in this.items) {
       let item = this.items[itemName];
       item.range = range;
@@ -70,25 +80,18 @@ Sanitizer.prototype = {
             if (aCanClear)
               item.clear();
           } catch(er) {
-            if (!errors)
-              errors = {};
-            errors[itemName] = er;
-            dump("Error sanitizing " + itemName + ": " + er + "\n");
-          }
-
-          // If this is the last item that needs to receive the callback, call the error handler
-          if (!--itemCount && errors) {
-            errorHandler(error);
+            seenError = true;
+            Cu.reportError("Error sanitizing " + itemName + ": " + er + "\n");
           }
+          onItemComplete();
         };
-
         this.canClearItem(itemName, clearCallback);
+      } else {
+        onItemComplete();
       }
     }
 
-    if (errors) {
-      errorHandler(error);
-    }
+    return deferred.promise;
   },
   
   // Time span only makes sense in certain cases.  Consumers who want
@@ -517,7 +520,8 @@ Sanitizer._checkAndSanitize = function()
     // this is a shutdown or a startup after an unclean exit
     var s = new Sanitizer();
     s.prefDomain = "privacy.clearOnShutdown.";
-    let errorHandler = function() prefs.setBoolPref(Sanitizer.prefDidShutdown, true);
-    s.sanitize(errorHandler);
+    s.sanitize().then(function() {
+      prefs.setBoolPref(Sanitizer.prefDidShutdown, true);
+    });
   }
 };

browser/base/content/sanitize.xul

@@ -27,7 +27,7 @@
             title="&sanitizeDialog2.title;"
             noneverythingtitle="&sanitizeDialog2.title;"
             style="width: &dialog.width2;;"
-            ondialogaccept="gSanitizePromptDialog.sanitize();">
+            ondialogaccept="return gSanitizePromptDialog.sanitize();">
 
   <prefpane id="SanitizeDialogPane" onpaneload="gSanitizePromptDialog.init();">
     <stringbundle id="bundleBrowser"

browser/base/content/sanitizeDialog.js

@@ -109,12 +109,23 @@ var gSanitizePromptDialog = {
     s.range = Sanitizer.getClearRange(this.selectedTimespan);
     s.ignoreTimespan = !s.range;
 
+    // As the sanitize is async, we disable the buttons, update the label on
+    // the 'accept' button to indicate things are happening and return false -
+    // once the async operation completes (either with or without errors)
+    // we close the window.
+    let docElt = document.documentElement;
+    let acceptButton = docElt.getButton("accept");
+    acceptButton.disabled = true;
+    acceptButton.setAttribute("label",
+                              this.bundleBrowser.getString("sanitizeButtonClearing"));
+    docElt.getButton("cancel").disabled = true;
     try {
-      s.sanitize();
+      s.sanitize().then(window.close, window.close);
     } catch (er) {
       Components.utils.reportError("Exception during sanitize: " + er);
+      return true; // We *do* want to close immediately on error.
     }
-    return true;
+    return false;
   },
 
   /**

browser/base/content/test/browser_bug409624.js

@@ -45,8 +45,9 @@ function test2()
   prefBranch.setBoolPref("siteSettings", false);
 
   // Sanitize now so we can test that canClear is correct. Formdata is cleared asynchronously.
-  s.sanitize();
+  s.sanitize().then(function() {
     s.canClearItem("formdata", clearDone1, s);
+  });
 }
 
 function clearDone1(aItemName, aResult, aSanitizer)
@@ -59,8 +60,9 @@ function clearDone1(aItemName, aResult, aSanitizer)
 function inputEntered(aItemName, aResult, aSanitizer)
 {
   ok(aResult, "formdata can be cleared after input");
-  aSanitizer.sanitize();
+  aSanitizer.sanitize().then(function() {
     aSanitizer.canClearItem("formdata", clearDone2);
+  });
 }
 
 function clearDone2(aItemName, aResult)

browser/locales/en-US/chrome/browser/browser.properties

@@ -142,6 +142,11 @@ pluginInfo.unknownPlugin=Unknown
 # changed to this.  See UI mockup and comment 11 at bug 480169 -->
 sanitizeDialog2.everything.title=Clear All History
 sanitizeButtonOK=Clear Now
+# LOCALIZATION NOTE (sanitizeButtonClearing): The label for the default
+# button between the user clicking it and the window closing.  Indicates the
+# items are being cleared.
+sanitizeButtonClearing=Clearing
+
 # LOCALIZATION NOTE (sanitizeEverythingWarning2): Warning that appears when
 # "Time range to clear" is set to "Everything" in Clear Recent History dialog,
 # provided that the user has not modified the default set of history items to clear.