Bug 1088137 - Forget button can fail to clear cookies by running sanitizer too early. r=MattN, a=dolske
authorJustin Dolske <dolske@mozilla.com>
Sun, 26 Oct 2014 20:46:03 -0700
changeset 225837 4c8d686c690b
parent 225836 a42b0af72449
child 225838 4197f5318fd8
push id4033
push usergijskruitbosch@gmail.com
push date2014-10-27 22:59 +0000
treeherdermozilla-beta@4c8d686c690b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN, dolske
bugs1088137
milestone34.0
Bug 1088137 - Forget button can fail to clear cookies by running sanitizer too early. r=MattN, a=dolske
browser/base/content/sanitize.js
--- a/browser/base/content/sanitize.js
+++ b/browser/base/content/sanitize.js
@@ -68,21 +68,35 @@ Sanitizer.prototype = {
 
     // Ensure open windows get cleared first, if they're in our list, so that they don't stick
     // around in the recently closed windows list, and so we can cancel the whole thing
     // if the user selects to keep a window open from a beforeunload prompt.
     let openWindowsIndex = itemsToClear.indexOf("openWindows");
     if (openWindowsIndex != -1) {
       itemsToClear.splice(openWindowsIndex, 1);
       let item = this.items.openWindows;
-      if (!item.clear()) {
-        // When cancelled, reject the deferred and return the promise:
-        deferred.reject();
-        return deferred.promise;
+
+      function onWindowsCleaned() {
+        try {
+          let clearedPromise = this.sanitize(itemsToClear);
+          clearedPromise.then(deferred.resolve, deferred.reject);
+        } catch(e) {
+          let error = "Sanitizer threw after closing windows: " + e;
+          Cu.reportError(error);
+          deferred.reject(error);
+        }
       }
+
+      let ok = item.clear(onWindowsCleaned.bind(this));
+      // When cancelled, reject immediately
+      if (!ok) {
+        deferred.reject("Sanitizer canceled closing windows");
+      }
+
+      return deferred.promise;
     }
 
     // Cache the range of times to clear
     if (this.ignoreTimespan)
       var range = null;  // If we ignore timespan, clear everything
     else
       range = this.range || Sanitizer.getClearRange();
 
@@ -445,21 +459,25 @@ Sanitizer.prototype = {
         }
         return true;
       },
       _resetAllWindowClosures: function(aWindowList) {
         for (let win of aWindowList) {
           win.getInterface(Ci.nsIDocShell).contentViewer.resetCloseWindow();
         }
       },
-      clear: function()
+      clear: function(aCallback)
       {
         // NB: this closes all *browser* windows, not other windows like the library, about window,
         // browser console, etc.
 
+        if (!aCallback) {
+          throw "Sanitizer's openWindows clear() requires a callback.";
+        }
+
         // Keep track of the time in case we get stuck in la-la-land because of onbeforeunload
         // dialogs
         let existingWindow = Services.appShell.hiddenDOMWindow;
         let startDate = existingWindow.performance.now();
 
         // First check if all these windows are OK with being closed:
         let windowEnumerator = Services.wm.getEnumerator("navigator:browser");
         let windowList = [];
@@ -487,17 +505,49 @@ Sanitizer.prototype = {
         // First create a new window. We do this first so that on non-mac, we don't
         // accidentally close the app by closing all the windows.
         let handler = Cc["@mozilla.org/browser/clh;1"].getService(Ci.nsIBrowserHandler);
         let defaultArgs = handler.defaultArgs;
         let features = "chrome,all,dialog=no," + this.privateStateForNewWindow;
         let newWindow = existingWindow.openDialog("chrome://browser/content/", "_blank",
                                                   features, defaultArgs);
 
-        // Then close all those windows we checked:
+        // Window creation and destruction is asynchronous. We need to wait
+        // until all existing windows are fully closed, and the new window is
+        // fully open, before continuing. Otherwise the rest of the sanitizer
+        // could run too early (and miss new cookies being set when a page
+        // closes) and/or run too late (and not have a fully-formed window yet
+        // in existence). See bug 1088137.
+        let newWindowOpened = false;
+        function onWindowOpened(subject, topic, data) {
+          if (subject != newWindow)
+            return;
+
+          Services.obs.removeObserver(onWindowOpened, "browser-delayed-startup-finished");
+          newWindowOpened = true;
+          // If we're the last thing to happen, invoke callback.
+          if (numWindowsClosing == 0)
+            aCallback();
+        }
+
+        let numWindowsClosing = windowList.length;
+        function onWindowClosed() {
+          numWindowsClosing--;
+          if (numWindowsClosing == 0) {
+            Services.obs.removeObserver(onWindowClosed, "xul-window-destroyed");
+            // If we're the last thing to happen, invoke callback.
+            if (newWindowOpened)
+              aCallback();
+          }
+        }
+
+        Services.obs.addObserver(onWindowOpened, "browser-delayed-startup-finished", false);
+        Services.obs.addObserver(onWindowClosed, "xul-window-destroyed", false);
+
+        // Start the process of closing windows
         while (windowList.length) {
           windowList.pop().close();
         }
         newWindow.focus();
         return true;
       },
 
       get canClear()