Bug 305085 - fix permitUnload usages in tab/window closing so we never show a permitUnload twice for the same page, r=jaws
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Thu, 04 Dec 2014 18:10:12 -0800
changeset 223826 e0c6c4e136eeec23f34541ded699abb86fdba642
parent 223825 9c9d441e616ec56542db6a5cd2d8f67c6954cec3
child 223827 458431092bd3cc158014d06834bb6d1e5a5bd04c
push id28109
push userryanvm@gmail.com
push dateWed, 14 Jan 2015 21:31:26 +0000
treeherdermozilla-central@c1f6345f2803 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws
bugs305085
milestone38.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 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
@@ -6445,18 +6445,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
@@ -2038,35 +2038,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");
 
@@ -2080,16 +2061,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();