Backed out 3 changesets (bug 1095236) for possibly spiking the frequency of VP(b-m) failures CLOSED TREE
authorWes Kocher <wkocher@mozilla.com>
Tue, 13 Oct 2015 10:08:11 -0700
changeset 300933 d2282c3bfb43af4ed72023b05a8d7756a768cdfb
parent 300932 80ce4acb7a4348e16b90253795415602d2c8b2b7
child 300934 c5e074f9eab48718d3d6a79281f761b9be1c6f01
push id5392
push userraliiev@mozilla.com
push dateMon, 14 Dec 2015 20:08:23 +0000
treeherdermozilla-beta@16ce8562a975 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1095236
milestone44.0a1
backs out11cb6379251ae9efd70bf3bc1f8fab9b66b3d964
856b7b90184f29a64093970e540193731b963f61
a179310161fb9240245995f86a31ef45cace38f6
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
Backed out 3 changesets (bug 1095236) for possibly spiking the frequency of VP(b-m) failures CLOSED TREE Backed out changeset 11cb6379251a (bug 1095236) Backed out changeset 856b7b90184f (bug 1095236) Backed out changeset a179310161fb (bug 1095236)
dom/tests/browser/browser.ini
dom/tests/browser/browser_test_new_window_from_content.js
dom/tests/browser/test_new_window_from_content_child.html
dom/tests/browser/test_new_window_from_content_child.js
embedding/components/windowwatcher/nsWindowWatcher.cpp
--- a/dom/tests/browser/browser.ini
+++ b/dom/tests/browser/browser.ini
@@ -21,16 +21,17 @@ skip-if= buildapp == 'mulet'
 [browser_focus_steal_from_chrome.js]
 [browser_focus_steal_from_chrome_during_mousedown.js]
 [browser_frame_elements.js]
 [browser_localStorage_privatestorageevent.js]
 [browser_test_new_window_from_content.js]
 skip-if = (toolkit == 'android' || buildapp == 'b2g' || buildapp == 'mulet')
 support-files =
   test_new_window_from_content_child.html
+  test_new_window_from_content_child.js
 [browser_webapps_permissions.js]
 # TODO: Re-enable permissions tests on Mac, bug 795334
 skip-if = buildapp != "b2g"
 support-files =
   test-webapp.webapp
   test-webapp-reinstall.webapp
   test-webapp-original.webapp
   test-webapps-permissions.html
--- a/dom/tests/browser/browser_test_new_window_from_content.js
+++ b/dom/tests/browser/browser_test_new_window_from_content.js
@@ -30,17 +30,20 @@
         2) Most things that open windows should behave according to browser.link.open_newwindow,
            _except_ for window.open calls with the "feature" parameter. This will open in a new
            window regardless of what browser.link.open_newwindow is set at. (default)
 
      This file attempts to test each window opening technique against all possible settings for
      each preference.
 */
 
+Cu.import("resource://gre/modules/Services.jsm");
+Cu.import("resource://gre/modules/Promise.jsm");
 Cu.import("resource://gre/modules/Task.jsm");
+Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
 const kXULNS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
 const kContentDoc = "http://www.example.com/browser/dom/tests/browser/test_new_window_from_content_child.html";
 const kContentScript = "chrome://mochitests/content/browser/dom/tests/browser/test_new_window_from_content_child.js";
 const kNewWindowPrefKey = "browser.link.open_newwindow";
 const kNewWindowRestrictionPrefKey = "browser.link.open_newwindow.restriction";
 const kSameTab = "same tab";
 const kNewWin = "new window";
@@ -83,120 +86,249 @@ const kTargetBlank = {
 var originalNewWindowPref = Services.prefs.getIntPref(kNewWindowPrefKey);
 var originalNewWindowRestrictionPref =
   Services.prefs.getIntPref(kNewWindowRestrictionPrefKey);
 
 registerCleanupFunction(function() {
   Services.prefs.setIntPref(kNewWindowPrefKey, originalNewWindowPref);
   Services.prefs.setIntPref(kNewWindowRestrictionPrefKey,
                             originalNewWindowRestrictionPref);
+  // If there are any content tabs leftover, make sure they're not going to
+  // block exiting with onbeforeunload.
+  for (let tab of gBrowser.tabs) {
+    let browser = gBrowser.getBrowserForTab(tab);
+    if (browser.contentDocument.location == kContentDoc) {
+      closeTab(tab);
+    }
+  }
 });
 
 /**
+ * WindowOpenListener is a very simple nsIWindowMediatorListener that
+ * listens for a new window opening to aExpectedURI. It has two Promises
+ * attached to it - openPromise and closePromise. As you'd expect,
+ * openPromise resolves when the window is opened, and closePromise
+ * resolves if and when a window with the same URI closes. There is
+ * no attempt to make sure that it's the same window opening and
+ * closing - we just use the URI.
+ *
+ * @param aExpectedURI the URI to watch for in a new window.
+ * @return nsIWindowMediatorListener
+ */
+function WindowOpenListener(aExpectedURI) {
+  this._openDeferred = Promise.defer();
+  this._closeDeferred = Promise.defer();
+  this._expectedURI = aExpectedURI;
+}
+
+WindowOpenListener.prototype = {
+  get openPromise() {
+    return this._openDeferred.promise;
+  },
+
+  get closePromise() {
+    return this._closeDeferred.promise;
+  },
+
+  onOpenWindow: function(aXULWindow) {
+    let domWindow = aXULWindow.QueryInterface(Ci.nsIInterfaceRequestor)
+                              .getInterface(Ci.nsIDOMWindow);
+    let location = domWindow.document.location;
+    if (location == this._expectedURI) {
+      let deferred = this._openDeferred;
+      domWindow.addEventListener("load", function onWindowLoad() {
+        domWindow.removeEventListener("load", onWindowLoad);
+        deferred.resolve(domWindow);
+      })
+    }
+  },
+
+  onCloseWindow: function(aXULWindow) {
+    this._closeDeferred.resolve();
+  },
+  onWindowTitleChange: function(aXULWindow, aNewTitle) {},
+
+  QueryInterface: XPCOMUtils.generateQI([Ci.nsIWindowMediatorListener])
+};
+
+/**
+ * Adds the testing tab, and injects our frame script which
+ * allows us to send click events to links and other things.
+ *
+ * @return a Promise that resolves once the tab is loaded and ready.
+ */
+function loadAndSelectTestTab() {
+  let tab = gBrowser.addTab(kContentDoc);
+  gBrowser.selectedTab = tab;
+
+  let browser = gBrowser.getBrowserForTab(tab);
+  browser.messageManager.loadFrameScript(kContentScript, false);
+
+  let deferred = Promise.defer();
+  browser.addEventListener("DOMContentLoaded", function onBrowserLoad(aEvent) {
+    browser.removeEventListener("DOMContentLoaded", onBrowserLoad);
+    deferred.resolve(tab);
+  });
+
+  return deferred.promise;
+}
+
+/**
+ * Clears the onbeforeunload event handler from the testing tab,
+ * and then closes the tab.
+ *
+ * @param aTab the testing tab to close.
+ * @param a Promise that resolves once the tab has been closed.
+ */
+function closeTab(aTab) {
+  let deferred = Promise.defer();
+  let browserMM = gBrowser.getBrowserForTab(aTab).messageManager;
+  browserMM.sendAsyncMessage("TEST:allow-unload");
+  browserMM.addMessageListener("TEST:allow-unload:done", function(aMessage) {
+    gBrowser.removeTab(aTab);
+    deferred.resolve();
+  })
+  return deferred.promise;
+}
+
+/**
+ * Sends a click event on some item into a tab.
+ *
+ * @param aTab the tab to send the click event to
+ * @param aItemId the item within the tab content to click.
+ */
+function clickInTab(aTab, aItemId) {
+  let browserMM = gBrowser.getBrowserForTab(aTab).messageManager;
+  browserMM.sendAsyncMessage("TEST:click-item", {
+    details: aItemId,
+  });
+}
+
+/**
  * For some expectation when a link is clicked, creates and
  * returns a Promise that resolves when that expectation is
  * fulfilled. For example, aExpectation might be kSameTab, which
  * will cause this function to return a Promise that resolves when
  * the current tab attempts to browse to about:blank.
  *
  * This function also takes care of cleaning up once the result has
  * occurred - for example, if a new window was opened, this function
  * closes it before resolving.
  *
- * @param aBrowser the <xul:browser> with the test document
+ * @param aTab the tab with the test document
  * @param aExpectation one of kSameTab, kNewWin, or kNewTab.
  * @return a Promise that resolves when the expectation is fulfilled,
  *         and cleaned up after.
  */
-function prepareForResult(aBrowser, aExpectation) {
+function prepareForResult(aTab, aExpectation) {
+  let deferred = Promise.defer();
+  let browser = gBrowser.getBrowserForTab(aTab);
+
   switch(aExpectation) {
     case kSameTab:
-      return Task.spawn(function*() {
-        yield BrowserTestUtils.browserLoaded(aBrowser);
-        is(aBrowser.currentURI.spec, "about:robots", "Should be at about:robots");
-        // Now put the browser back where it came from
-        yield BrowserTestUtils.loadURI(aBrowser, kContentDoc);
-        yield BrowserTestUtils.browserLoaded(aBrowser);
-      });
+      // We expect about:blank to be loaded in the current tab. In order
+      // to prevent us needing to reload the document and content script
+      // after browsing away, we'll detect the attempt by using onbeforeunload,
+      // and then cancel the unload. It's a hack, but it's also a pretty
+      // cheap way of detecting when we're browsing away in the test tab.
+      // The onbeforeunload event handler is set in the content script automatically.
+      browser.addEventListener("DOMWillOpenModalDialog", function onModalDialog() {
+        browser.removeEventListener("DOMWillOpenModalDialog", onModalDialog, true);
+        executeSoon(() => {
+          let stack = browser.parentNode;
+          let dialogs = stack.getElementsByTagNameNS(kXULNS, "tabmodalprompt");
+          dialogs[0].ui.button1.click()
+          deferred.resolve();
+        })
+      }, true);
       break;
     case kNewWin:
-      return Task.spawn(function*() {
-        let newWin = yield BrowserTestUtils.waitForNewWindow();
-        let newBrowser = newWin.gBrowser.selectedBrowser;
-        yield BrowserTestUtils.browserLoaded(newBrowser);
-        is(newBrowser.currentURI.spec, "about:robots", "Should be at about:robots");
-        yield BrowserTestUtils.closeWindow(newWin);
+      let listener = new WindowOpenListener("about:blank");
+      Services.wm.addListener(listener);
+
+      info("Waiting for a new about:blank window");
+      listener.openPromise.then(function(aWindow) {
+        info("Got the new about:blank window - closing it.");
+        executeSoon(() => {
+          aWindow.close();
+        });
+        listener.closePromise.then(() => {
+          info("New about:blank window closed!");
+          Services.wm.removeListener(listener);
+          deferred.resolve();
+        });
       });
       break;
     case kNewTab:
-      return Task.spawn(function*() {
-        let newTab = yield BrowserTestUtils.waitForNewTab(gBrowser);
-        is(newTab.linkedBrowser.currentURI.spec, "about:robots",
-           "Should be at about:robots");
-        yield BrowserTestUtils.removeTab(newTab);
-      });
+      gBrowser.tabContainer.addEventListener("TabOpen", function onTabOpen(aEvent) {
+        let newTab = aEvent.target;
+        let newBrowser = gBrowser.getBrowserForTab(newTab);
+        if (newBrowser.contentDocument.location.href == "about:blank") {
+          gBrowser.tabContainer.removeEventListener("TabOpen", onTabOpen);
+          executeSoon(() => {
+            gBrowser.removeTab(newTab);
+            deferred.resolve();
+          })
+        }
+      })
       break;
     default:
       ok(false, "prepareForResult can't handle an expectation of " + aExpectation)
       return;
   }
 
   return deferred.promise;
 }
 
 /**
  * Ensure that clicks on a link with ID aLinkID cause us to
  * perform as specified in the supplied aMatrix (kWinOpenDefault,
  * for example).
  *
- * @param aLinkSelector a selector for the link within the testing page to click.
+ * @param aLinkId the id of the link within the testing page to test.
  * @param aMatrix a testing matrix for the
  *        browser.link.open_newwindow and browser.link.open_newwindow.restriction
  *        prefs to test against. See kWinOpenDefault for an example.
  */
-function testLinkWithMatrix(aLinkSelector, aMatrix) {
-  return BrowserTestUtils.withNewTab({
-    gBrowser,
-    url: kContentDoc,
-  }, function*(browser) {
+function testLinkWithMatrix(aLinkId, aMatrix) {
+  return Task.spawn(function* () {
+    let tab = yield loadAndSelectTestTab();
     // This nested for-loop is unravelling the matrix const
     // we set up, and gives us three things through each tick
     // of the inner loop:
     // 1) newWindowPref: a browser.link.open_newwindow pref to try
     // 2) newWindowRestPref: a browser.link.open_newwindow.restriction pref to try
     // 3) expectation: what we expect the click outcome on this link to be,
     //                 which will either be kSameTab, kNewWin or kNewTab.
+
     for (let newWindowPref in aMatrix) {
       let expectations = aMatrix[newWindowPref];
       for (let i = 0; i < expectations.length; ++i) {
         let newWindowRestPref = i;
         let expectation = expectations[i];
 
         Services.prefs.setIntPref("browser.link.open_newwindow", newWindowPref);
         Services.prefs.setIntPref("browser.link.open_newwindow.restriction", newWindowRestPref);
-        info("Clicking on " + aLinkSelector);
+        info("Clicking on " + aLinkId);
         info("Testing with browser.link.open_newwindow = " + newWindowPref + " and " +
              "browser.link.open_newwindow.restriction = " + newWindowRestPref);
         info("Expecting: " + expectation);
-        let resultPromise = prepareForResult(browser, expectation);
-        BrowserTestUtils.synthesizeMouseAtCenter(aLinkSelector, {}, browser);
+        let resultPromise = prepareForResult(tab, expectation);
+        clickInTab(tab, aLinkId);
         yield resultPromise;
-        info("Got expectation: " + expectation);
+        ok(true, "Got expectation: " + expectation);
       }
     }
+    yield closeTab(tab);
   });
 }
 
 add_task(function* test_window_open_with_defaults() {
-  yield testLinkWithMatrix("#winOpenDefault", kWinOpenDefault);
+  yield testLinkWithMatrix("winOpenDefault", kWinOpenDefault);
 });
 
 add_task(function* test_window_open_with_non_defaults() {
-  yield testLinkWithMatrix("#winOpenNonDefault", kWinOpenNonDefault);
-});
-
-add_task(function* test_window_open_dialog() {
-  yield testLinkWithMatrix("#winOpenDialog", kWinOpenNonDefault);
+  yield testLinkWithMatrix("winOpenNonDefault", kWinOpenNonDefault);
 });
 
 add_task(function* test_target__blank() {
-  yield testLinkWithMatrix("#targetBlank", kTargetBlank);
+  yield testLinkWithMatrix("targetBlank", kTargetBlank);
 });
--- a/dom/tests/browser/test_new_window_from_content_child.html
+++ b/dom/tests/browser/test_new_window_from_content_child.html
@@ -1,19 +1,23 @@
 <!DOCTYPE html>
 <head>
   <meta charset="utf-8">
   <title>Test popup window opening behaviour</title>
 </head>
 <body>
   <p><a id="winOpenDefault" href="#" onclick="return openWindow();">Open a new window via window.open with default features.</a></p>
   <p><a id="winOpenNonDefault" href="#" onclick="return openWindow('resizable=no, toolbar=no, scrollbars=no, menubar=no, status=no, directories=no, height=100, width=500');">Open a new window via window.open with non-default features.</a></p>
-  <p><a id="winOpenDialog" href="#" onclick="return openWindow('dialog=yes');">Open a new window via window.open with dialog=1.</a></p>
-  <p><a id="targetBlank" href="about:robots" target="_blank">Open a new window via target="_blank".</a></p>
+  <p><a id="targetBlank" href="about:blank" target="_blank">Open a new window via target="_blank".</a></p>
 </body>
 </html>
 
 <script>
 function openWindow(aFeatures="") {
-  window.open("about:robots", "_blank", aFeatures);
+  window.open("about:blank", "_blank", aFeatures);
   return false;
 }
+
+window.onbeforeunload = function(aEvent) {
+  return "We should not browse away from this document.";
+}
+
 </script>
\ No newline at end of file
new file mode 100644
--- /dev/null
+++ b/dom/tests/browser/test_new_window_from_content_child.js
@@ -0,0 +1,19 @@
+// A hacky mechanism for catching and detecting that we're attempting
+// to browse away is by setting the onbeforeunload event handler. We
+// detect this dialog opening in the parent test script, and dismiss
+// it.
+
+function handleClickItem(aMessage) {
+  let itemId = aMessage.data.details;
+  content.console.log("Getting item with ID: " + itemId);
+  let item = content.document.getElementById(itemId);
+  item.click();
+}
+
+function handleAllowUnload(aMessage) {
+  content.onbeforeunload = null;
+  sendSyncMessage("TEST:allow-unload:done");
+}
+
+addMessageListener("TEST:click-item", handleClickItem);
+addMessageListener("TEST:allow-unload", handleAllowUnload);
\ No newline at end of file
--- a/embedding/components/windowwatcher/nsWindowWatcher.cpp
+++ b/embedding/components/windowwatcher/nsWindowWatcher.cpp
@@ -1646,19 +1646,19 @@ nsWindowWatcher::CalculateChromeFlags(ns
   nsCOMPtr<nsIPrefBranch> branch = do_QueryInterface(prefs);
   branch->GetBoolPref("dom.disable_window_open_dialog_feature",
                       &disableDialogFeature);
 
   bool isFullScreen = false;
   if (aParent) {
     aParent->GetFullScreen(&isFullScreen);
   }
-  if (openedFromContentScript) {
-    // If the caller context is content, we do not support the
-    // dialog feature. See bug 1095236.
+  if (isFullScreen && openedFromContentScript) {
+    // If the parent window is in fullscreen & the caller context is content,
+    // dialog feature is disabled. (see bug 803675)
     disableDialogFeature = true;
   }
 
   if (!disableDialogFeature) {
     chromeFlags |= WinHasOption(aFeatures, "dialog", 0, nullptr) ?
       nsIWebBrowserChrome::CHROME_OPENAS_DIALOG : 0;
   }