Bug 305085 - fix permitUnload usages in tab/window closing so we never show a permitUnload twice for the same page, r=jaws
☠☠ backed out by ea461c27bb0f ☠ ☠
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Thu, 04 Dec 2014 18:10:12 -0800
changeset 221201 618a0ea01af9e711107b9256e44206b0ba209897
parent 221200 c8f1dab5c9e5e555243db1c41a4535e306f183c2
child 221202 ea461c27bb0f9bf02d61284b31ec6cd42067ab8c
push id10579
push usergijskruitbosch@gmail.com
push dateWed, 24 Dec 2014 13:28:02 +0000
treeherderfx-team@618a0ea01af9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws
bugs305085
milestone37.0a1
Bug 305085 - fix permitUnload usages in tab/window closing so we never show a permitUnload twice for the same page, r=jaws
browser/base/content/browser.js
browser/base/content/tabbrowser.xml
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -6438,18 +6438,28 @@ function WindowIsClosing()
     return false;
 
   // Bug 967873 - Proxy nsDocumentViewer::PermitUnload to the child process
   if (gMultiProcessBrowser)
     return true;
 
   for (let browser of gBrowser.browsers) {
     let ds = browser.docShell;
-    if (ds.contentViewer && !ds.contentViewer.permitUnload())
+    // Passing true to permitUnload indicates we plan on closing the window.
+    // This means that once unload is permitted, all further calls to
+    // permitUnload will be ignored. This avoids getting multiple prompts
+    // to unload the page.
+    if (ds.contentViewer && !ds.contentViewer.permitUnload(true)) {
+      // ... however, if the user aborts closing, we need to undo that,
+      // to ensure they get prompted again when we next try to close the window.
+      // We do this on the window's toplevel docshell instead of on the tab, so
+      // that all tabs we iterated before will get this reset.
+      window.getInterface(Ci.nsIDocShell).contentViewer.resetCloseWindow();
       return false;
+    }
   }
 
   return true;
 }
 
 /**
  * Checks if this is the last full *browser* window around. If it is, this will
  * be communicated like quitting. Otherwise, we warn about closing multiple tabs.
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -2036,35 +2036,16 @@
         <parameter name="aCloseWindowFastpath"/>
         <body>
           <![CDATA[
             if (aTab.closing ||
                 this._windowIsClosing)
               return false;
 
             var browser = this.getBrowserForTab(aTab);
-            if (!aTab._pendingPermitUnload && !aTabWillBeMoved) {
-              let ds = browser.docShell;
-              if (ds && ds.contentViewer) {
-                // We need to block while calling permitUnload() because it
-                // processes the event queue and may lead to another removeTab()
-                // call before permitUnload() even returned.
-                aTab._pendingPermitUnload = true;
-                let permitUnload = ds.contentViewer.permitUnload();
-                delete aTab._pendingPermitUnload;
-                // If we were closed during onbeforeunload, we return false now
-                // so we don't (try to) close the same tab again. Of course, we
-                // also stop if the unload was cancelled by the user:
-                if (aTab.closing || !permitUnload) {
-                  // NB: deliberately keep the _closedDuringPermitUnload set to
-                  // true so we keep exiting early in case of multiple calls.
-                  return false;
-                }
-              }
-            }
 
             var closeWindow = false;
             var newTab = false;
             if (this.tabs.length - this._removingTabs.length == 1) {
               closeWindow = aCloseWindowWithLastTab != null ? aCloseWindowWithLastTab :
                             !window.toolbar.visible ||
                               Services.prefs.getBoolPref("browser.tabs.closeWindowWithLastTab");
 
@@ -2078,16 +2059,36 @@
                 // cancels the operation.  We are finished here in both cases.
                 this._windowIsClosing = window.closeWindow(true, window.warnAboutClosingWindow);
                 return null;
               }
 
               newTab = true;
             }
 
+            if (!aTab._pendingPermitUnload && !aTabWillBeMoved) {
+              let ds = browser.docShell;
+              if (ds && ds.contentViewer) {
+                // We need to block while calling permitUnload() because it
+                // processes the event queue and may lead to another removeTab()
+                // call before permitUnload() returns.
+                aTab._pendingPermitUnload = true;
+                let permitUnload = ds.contentViewer.permitUnload();
+                delete aTab._pendingPermitUnload;
+                // If we were closed during onbeforeunload, we return false now
+                // so we don't (try to) close the same tab again. Of course, we
+                // also stop if the unload was cancelled by the user:
+                if (aTab.closing || !permitUnload) {
+                  // NB: deliberately keep the _closedDuringPermitUnload set to
+                  // true so we keep exiting early in case of multiple calls.
+                  return false;
+                }
+              }
+            }
+
             aTab.closing = true;
             this._removingTabs.push(aTab);
             this._visibleTabs = null; // invalidate cache
 
             // Invalidate hovered tab state tracking for this closing tab.
             if (this.tabContainer._hoveredTab == aTab)
               aTab._mouseleave();