Bug 1162860 - ensure closing tabs really don't get closed twice, r=ttaubert,a=lizzard
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Mon, 11 May 2015 14:13:08 +0100
changeset 266110 b0b76d0add0b
parent 266109 b671922dedff
child 266111 3ff03c918bdc
push id4757
push usergijskruitbosch@gmail.com
push date2015-05-26 13:11 +0000
treeherdermozilla-beta@b0b76d0add0b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersttaubert, lizzard
bugs1162860
milestone39.0
Bug 1162860 - ensure closing tabs really don't get closed twice, r=ttaubert,a=lizzard
browser/base/content/tabbrowser.xml
browser/base/content/test/general/browser.ini
browser/base/content/test/general/browser_tabs_close_beforeunload.js
browser/base/content/test/general/close_beforeunload.html
browser/base/content/test/general/close_beforeunload_opens_second_tab.html
testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -5519,17 +5519,23 @@
         </getter>
         <setter>
           this._lastAccessed = val;
         </setter>
       </property>
 
       <field name="mOverCloseButton">false</field>
       <field name="mCorrespondingMenuitem">null</field>
+
+      <!--
+      While it would make sense to track this in a field, the field will get nuked
+      once the node is gone from the DOM, which causes us to think the tab is not
+      closed, which causes us to make wrong decisions. So we use an expando instead.
       <field name="closing">false</field>
+      -->
 
       <method name="_mouseenter">
         <body><![CDATA[
           if (this.hidden || this.closing)
             return;
 
           let tabContainer = this.parentNode;
           let visibleTabs = tabContainer.tabbrowser.visibleTabs;
--- a/browser/base/content/test/general/browser.ini
+++ b/browser/base/content/test/general/browser.ini
@@ -427,16 +427,20 @@ skip-if = buildapp == 'mulet'
 skip-if = buildapp == 'mulet'
 [browser_tabbar_big_widgets.js]
 skip-if = os == "linux" || os == "mac" # No tabs in titlebar on linux
                                        # Disabled on OS X because of bug 967917
 [browser_tabfocus.js]
 [browser_tabkeynavigation.js]
 skip-if = e10s
 [browser_tabopen_reflows.js]
+[browser_tabs_close_beforeunload.js]
+support-files =
+  close_beforeunload_opens_second_tab.html
+  close_beforeunload.html
 [browser_tabs_isActive.js]
 skip-if = e10s # Bug 1100664 - test relies on linkedBrowser.docShell
 [browser_tabs_owner.js]
 [browser_testOpenNewRemoteTabsFromNonRemoteBrowsers.js]
 run-if = e10s
 [browser_trackingUI.js]
 support-files =
   trackingPage.html
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/general/browser_tabs_close_beforeunload.js
@@ -0,0 +1,59 @@
+"use strict";
+
+const FIRST_TAB = getRootDirectory(gTestPath) + "close_beforeunload_opens_second_tab.html";
+const SECOND_TAB = getRootDirectory(gTestPath) + "close_beforeunload.html";
+
+add_task(function*() {
+  let firstTab = yield BrowserTestUtils.openNewForegroundTab(gBrowser, FIRST_TAB);
+  let newTabPromise = waitForNewTabEvent(gBrowser);
+  ContentTask.spawn(firstTab.linkedBrowser, "", function*() {
+    content.document.getElementsByTagName("a")[0].click();
+  });
+  let tabOpenEvent = yield newTabPromise;
+  let secondTab = tabOpenEvent.target;
+  yield ContentTask.spawn(secondTab.linkedBrowser, SECOND_TAB, function*(expectedURL) {
+    if (content.window.location.href == expectedURL &&
+        content.document.readyState === "complete") {
+      return Promise.resolve();
+    }
+    return new Promise(function(resolve, reject) {
+      content.window.addEventListener("load", function() {
+        if (content.window.location.href == expectedURL) {
+          resolve();
+        }
+      }, false);
+    });
+  });
+
+  let closeBtn = document.getAnonymousElementByAttribute(secondTab, "anonid", "close-button");
+  let mutationObserver;
+  let browserToBeRemoved = secondTab.linkedBrowser;
+  let mutationObserverPromiseFn = (resolve, reject) => {
+    mutationObserver = new MutationObserver(function() {
+      if (!browserToBeRemoved.parentNode) {
+        mutationObserver.disconnect();
+        mutationObserver = null;
+        resolve();
+      }
+    });
+    mutationObserver.observe(browserToBeRemoved.parentNode, {childList: true});
+  };
+  let closePromise = new Promise(mutationObserverPromiseFn);
+  info("closing second tab (which will self-close in beforeunload)");
+  closeBtn.click();
+  ok(secondTab.closing, "Second tab should be marked as closing synchronously.");
+  yield closePromise;
+
+  ok(secondTab.closing, "Second tab should still be marked as closing");
+  ok(!secondTab.linkedBrowser, "Second tab's browser should be dead");
+  ok(!firstTab.closing, "First tab should not be closing");
+  ok(firstTab.linkedBrowser, "First tab's browser should be alive");
+  info("closing first tab");
+  browserToBeRemoved = firstTab.linkedBrowser;
+  closePromise = new Promise(mutationObserverPromiseFn);
+  gBrowser.removeTab(firstTab);
+  yield closePromise;
+
+  ok(firstTab.closing, "First tab should be marked as closing");
+  ok(!firstTab.linkedBrowser, "First tab's browser should be dead");
+});
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/general/close_beforeunload.html
@@ -0,0 +1,8 @@
+<body>
+  <p>I will close myself if you close me.</p>
+  <script>
+    window.onbeforeunload = function() {
+      window.close();
+    };
+  </script>
+</body>
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/general/close_beforeunload_opens_second_tab.html
@@ -0,0 +1,3 @@
+<body>
+  <a href="#" onclick="window.open('close_beforeunload.html', '_blank')">Open second tab</a>
+</body>
--- a/testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm
+++ b/testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm
@@ -335,31 +335,9 @@ this.BrowserTestUtils = {
    *  Version of synthesizeMouse that uses a client point within the child
    *  window instead of a target as the offset. Otherwise, the arguments and
    *  return value are the same as synthesizeMouse.
    */
   synthesizeMouseAtPoint(offsetX, offsetY, event, browser)
   {
     return BrowserTestUtils.synthesizeMouse(null, offsetX, offsetY, event, browser);
   },
-
-  /**
-   * Removes the given tab from its parent tabbrowser and
-   * waits until its final message has reached the parent.
-   */
-  removeTab(tab, options = {}) {
-    let dontRemove = options && options.dontRemove;
-
-    return new Promise(resolve => {
-      let {messageManager: mm, frameLoader} = tab.linkedBrowser;
-      mm.addMessageListener("SessionStore:update", function onMessage(msg) {
-        if (msg.targetFrameLoader == frameLoader && msg.data.isFinal) {
-          mm.removeMessageListener("SessionStore:update", onMessage);
-          resolve();
-        }
-      }, true);
-
-      if (!dontRemove && !tab.closing) {
-        tab.ownerDocument.defaultView.gBrowser.removeTab(tab);
-      }
-    });
-  }
 };