Bug 1130411 - deal with opening dialogs when the previous dialog is not unloaded yet, r=jaws

MozReview-Commit-ID: IY0W3UfYTdc

--HG--
rename : browser/components/preferences/in-content/tests/subdialog.xul => browser/components/preferences/in-content/tests/subdialog2.xul
extra : rebase_source : b843e77866d9c50cfb91263d3a2d1439977b3e40

browser/components/preferences/in-content/subdialogs.js

@@ -41,6 +41,21 @@ var gSubDialog = {
   },
 
   open: function(aURL, aFeatures = null, aParams = null, aClosingCallback = null) {
+    // If we're already open/opening on this URL, do nothing.
+    if (this._openedURL == aURL && !this._isClosing) {
+      return;
+    }
+    // If we're open on some (other) URL or we're closing, open when closing has finished.
+    if (this._openedURL || this._isClosing) {
+      if (!this._isClosing) {
+        this.close();
+      }
+      let args = Array.from(arguments);
+      this._closingPromise.then(() => {
+        this.open.apply(this, args);
+      });
+      return;
+    }
     this._addDialogEventListeners();
 
     let features = (!!aFeatures ? aFeatures + "," : "") + "resizable,dialog=no,centerscreen";
@@ -58,7 +73,6 @@ var gSubDialog = {
     this._box.setAttribute("resizable", featureParams.has("resizable") &&
                                         featureParams.get("resizable") != "no" &&
                                         featureParams.get("resizable") != "0");
-    return dialog;
   },
 
   close: function(aEvent = null) {
@@ -66,6 +80,9 @@ var gSubDialog = {
       return;
     }
     this._isClosing = true;
+    this._closingPromise = new Promise(resolve => {
+      this._resolveClosePromise = resolve;
+    });
 
     if (this._closingCallback) {
       try {
@@ -90,6 +107,16 @@ var gSubDialog = {
     setTimeout(() => {
       // Unload the dialog after the event listeners run so that the load of about:blank isn't
       // cancelled by the ESC <key>.
+      let onBlankLoad = e => {
+        if (this._frame.contentWindow.location.href == "about:blank") {
+          this._frame.removeEventListener("load", onBlankLoad);
+          // We're now officially done closing, so update the state to reflect that.
+          delete this._openedURL;
+          this._isClosing = false;
+          this._resolveClosePromise();
+        }
+      };
+      this._frame.addEventListener("load", onBlankLoad);
       this._frame.loadURI("about:blank");
     }, 0);
   },
@@ -132,8 +159,9 @@ var gSubDialog = {
   },
 
   _onContentLoaded: function(aEvent) {
-    if (aEvent.target != this._frame || aEvent.target.contentWindow.location == "about:blank")
+    if (aEvent.target != this._frame || aEvent.target.contentWindow.location == "about:blank") {
       return;
+    }
 
     for (let styleSheetURL of this._injectedStyleSheets) {
       this.injectXMLStylesheet(styleSheetURL);
@@ -169,8 +197,9 @@ var gSubDialog = {
   },
 
   _onLoad: function(aEvent) {
-    if (aEvent.target.contentWindow.location == "about:blank")
+    if (aEvent.target.contentWindow.location == "about:blank") {
       return;
+    }
 
     // Do this on load to wait for the CSS to load and apply before calculating the size.
     let docEl = this._frame.contentDocument.documentElement;
@@ -260,8 +289,9 @@ var gSubDialog = {
 
     let docEl = frame.contentDocument.documentElement;
     let persistedAttributes = docEl.getAttribute("persist");
-    if (!persistedAttributes.contains("width") &&
-        !persistedAttributes.contains("height")) {
+    if (!persistedAttributes ||
+        (!persistedAttributes.contains("width") &&
+         !persistedAttributes.contains("height"))) {
       return;
     }
 

browser/components/preferences/in-content/tests/browser.ini

@@ -32,7 +32,9 @@ skip-if = true || !healthreport # Bug 1185403 for the "true"
 [browser_sanitizeOnShutdown_prefLocked.js]
 [browser_searchsuggestions.js]
 [browser_subdialogs.js]
-support-files = subdialog.xul
+support-files =
+  subdialog.xul
+  subdialog2.xul
 [browser_telemetry.js]
 # Skip this test on Android and B2G as FHR and Telemetry are separate systems there.
 skip-if = !healthreport || !telemetry || (os == 'linux' && debug) || (os == 'android') || (os == 'b2g')

browser/components/preferences/in-content/tests/browser_subdialogs.js

@@ -8,17 +8,18 @@
  */
 
 const gDialogURL = getRootDirectory(gTestPath) + "subdialog.xul";
+const gDialogURL2 = getRootDirectory(gTestPath) + "subdialog2.xul";
 
-function* open_subdialog_and_test_generic_start_state(browser, domcontentloadedFn) {
+function* open_subdialog_and_test_generic_start_state(browser, domcontentloadedFn, url = gDialogURL) {
   let domcontentloadedFnStr = domcontentloadedFn ?
     "(" + domcontentloadedFn.toString() + ")()" :
     "";
-  return ContentTask.spawn(browser, {gDialogURL, domcontentloadedFnStr}, function*(args) {
-    let {gDialogURL, domcontentloadedFnStr} = args;
+  return ContentTask.spawn(browser, {url, domcontentloadedFnStr}, function*(args) {
+    let {url, domcontentloadedFnStr} = args;
     let rv = { acceptCount: 0 };
     let win = content.window;
     let subdialog = win.gSubDialog;
-    subdialog.open(gDialogURL, null, rv);
+    subdialog.open(url, null, rv);
 
     info("waiting for subdialog DOMFrameContentLoaded");
     yield ContentTaskUtils.waitForEvent(win, "DOMFrameContentLoaded", true);
@@ -128,6 +129,28 @@ add_task(function* check_canceling_dialog() {
     "cancel", 0);
 });
 
+add_task(function* check_reopening_dialog() {
+  yield open_subdialog_and_test_generic_start_state(tab.linkedBrowser);
+  info("opening another dialog which will close the first");
+  yield open_subdialog_and_test_generic_start_state(tab.linkedBrowser, "", gDialogURL2);
+  info("closing as normal");
+  yield close_subdialog_and_test_generic_end_state(tab.linkedBrowser,
+    function() { content.window.gSubDialog._frame.contentDocument.documentElement.acceptDialog(); },
+    "accept", 1);
+});
+
+add_task(function* check_opening_while_closing() {
+  yield open_subdialog_and_test_generic_start_state(tab.linkedBrowser);
+  info("closing");
+  content.window.gSubDialog.close();
+  info("reopening immediately after calling .close()");
+  yield open_subdialog_and_test_generic_start_state(tab.linkedBrowser);
+  yield close_subdialog_and_test_generic_end_state(tab.linkedBrowser,
+    function() { content.window.gSubDialog._frame.contentDocument.documentElement.acceptDialog(); },
+    "accept", 1);
+
+});
+
 add_task(function* window_close_on_dialog() {
   yield open_subdialog_and_test_generic_start_state(tab.linkedBrowser);
 

browser/components/preferences/in-content/tests/subdialog2.xul

@@ -0,0 +1,27 @@
+<?xml version="1.0"?>
+
+<!-- Any copyright is dedicated to the Public Domain.
+   - http://creativecommons.org/publicdomain/zero/1.0/ -->
+
+<?xml-stylesheet href="chrome://global/skin/" type="text/css"?>
+
+<dialog id="subDialog"
+        xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
+        title="Sample sub-dialog #2" style="width: 32em; height: 5em;"
+        onload="document.getElementById('textbox').focus();"
+        ondialogaccept="acceptSubdialog();">
+  <script>
+    function acceptSubdialog() {
+      window.arguments[0].acceptCount++;
+    }
+  </script>
+
+  <description id="desc">A sample sub-dialog for testing</description>
+
+  <textbox id="textbox" value="Default text" />
+
+  <separator class="thin"/>
+
+  <button oncommand="close();" icon="close" label="Close" />
+
+</dialog>