Bug 1340987 - WIP Crash fix
I think the crash is simply because the openDialog code path assumes[1] that the name being passed to openDialog from chrome callers is of a frame that is ready to use but now that we dynamically append a frame it's no longer ready. A simple way to know if the <browser> binding is attached is to wait for the about:blank load event after appending. This seems to work fine and you can confirm that it's not just working now because of the Promise itself by switching to a Promise.resolve() instead... it will still crash.
If possible we should switch `open` to an async function and await on the about:blank load inside it. I'm not sure if that will cause other test failures or not though... it seems like opening is async anyways so it's probably better that way.
[1] http://searchfox.org/mozilla-central/rev/507743376d1ba753d14ab6b9305b7c6358570be8/dom/base/nsGlobalWindow.cpp#12802-12803,12831
MozReview-Commit-ID: 5tZgFwcQgzD
--- a/browser/components/preferences/in-content/subdialogs.js
+++ b/browser/components/preferences/in-content/subdialogs.js
@@ -21,25 +21,36 @@ function SubDialog({template, parentElem
this._frame = this._overlay.querySelector(".dialogFrame");
this._box = this._overlay.querySelector(".dialogBox");
this._closeButton = this._overlay.querySelector(".dialogClose");
this._titleElement = this._overlay.querySelector(".dialogTitle");
this._overlay.id = `dialogOverlay-${id}`;
this._frame.setAttribute("name", `dialogFrame-${id}`);
+ //this._frameCreated = Promise.resolve();
+ this._frameCreated = new Promise(resolve => {
+ // Wait for the about:blank load to know that the frame is ready to use
+ // (the <browser> binding is attached).
+ this._frame.addEventListener("load", () => {
+ console.log(`load event for dialogFrame-${id}`); // TODO
+ resolve();
+ });
+ });
+
parentElement.appendChild(this._overlay);
this._overlay.hidden = false;
}
SubDialog.prototype = {
_closingCallback: null,
_closingEvent: null,
_isClosing: false,
_frame: null,
+ _frameCreated: null,
_overlay: null,
_box: null,
_openedURL: null,
_injectedStyleSheets: [
"chrome://browser/skin/preferences/preferences.css",
"chrome://global/skin/in-content/common.css",
"chrome://browser/skin/preferences/in-content/preferences.css",
"chrome://browser/skin/preferences/in-content/dialog.css",
@@ -61,44 +72,49 @@ SubDialog.prototype = {
"xml-stylesheet",
'href="' + aStylesheetURL + '" type="text/css"'
);
this._frame.contentDocument.insertBefore(contentStylesheet,
this._frame.contentDocument.documentElement);
},
open(aURL, aFeatures = null, aParams = null, aClosingCallback = null) {
+ this._frameCreated.then(() => {
// 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";
+ let overlay = document.getElementById(`dialogOverlay-${this._id}`);
+ let b = overlay.querySelector(".dialogFrame");
+ console.log("foobar", this._id, overlay, b, b.contentDocument); // TODO
let dialog = window.openDialog(aURL, `dialogFrame-${this._id}`, features, aParams);
if (aClosingCallback) {
this._closingCallback = aClosingCallback.bind(dialog);
}
this._closingEvent = null;
this._isClosing = false;
this._openedURL = aURL;
features = features.replace(/,/g, "&");
let featureParams = new URLSearchParams(features.toLowerCase());
this._box.setAttribute("resizable", featureParams.has("resizable") &&
featureParams.get("resizable") != "no" &&
featureParams.get("resizable") != "0");
+ });
},
close(aEvent = null) {
if (this._isClosing) {
return;
}
this._isClosing = true;
this._closingPromise = new Promise(resolve => {
--- a/browser/components/preferences/in-content/tests/head.js
+++ b/browser/components/preferences/in-content/tests/head.js
@@ -34,22 +34,17 @@ function open_preferences(aCallback) {
let newTabBrowser = gBrowser.getBrowserForTab(gBrowser.selectedTab);
newTabBrowser.addEventListener("Initialized", function() {
aCallback(gBrowser.contentWindow);
}, {capture: true, once: true});
}
function openAndLoadSubDialog(aURL, aFeatures = null, aParams = null, aClosingCallback = null) {
let promise = promiseLoadSubDialog(aURL);
- // XXX: Bug 1340987 introduced a change to enable multiple subdialogs, but it
- // also causes a timing related crash here. Wrapping it within executeSoon
- // is a workaround. Detail of the crash is documented in the bug above.
- executeSoon(() => {
- content.gSubDialog.open(aURL, aFeatures, aParams, aClosingCallback);
- });
+ content.gSubDialog.open(aURL, aFeatures, aParams, aClosingCallback);
return promise;
}
function promiseLoadSubDialog(aURL) {
return new Promise((resolve, reject) => {
content.gSubDialog._dialogStack.addEventListener("dialogopen", function dialogopen(aEvent) {
if (aEvent.detail.dialog._frame.contentWindow.location == "about:blank")
return;