Skip to content

Commit

Permalink
Bug 1792466: Track browser windows that are in the process of being o…
Browse files Browse the repository at this point in the history
…pened so we don't open multiple windows when trying to open many files/urls quickly. r=Gijs

Ideally we would use the window mediator to just find new browser windows that
are in the process of opening but while we can find the windows they just appear
as about:blank with no way to verify that they are browser windows.

This just takes the straightforward approach of forcing code that opens browser
windows to register them with the BrowserWindowTracker and provides a simple
shared API for opening browser windows that does this.

Differential Revision: https://phabricator.services.mozilla.com/D161076
  • Loading branch information
Mossop committed Nov 17, 2022
1 parent f70a1b1 commit f68a7e6
Show file tree
Hide file tree
Showing 3 changed files with 189 additions and 10 deletions.
25 changes: 15 additions & 10 deletions browser/components/BrowserContentHandler.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,6 @@ function openBrowserWindow(
postData = null,
forcePrivate = false
) {
let chromeURL = AppConstants.BROWSER_CHROME_URL;
const isStartup =
cmdLine && cmdLine.state == Ci.nsICommandLine.STATE_INITIAL_LAUNCH;

Expand Down Expand Up @@ -317,10 +316,11 @@ function openBrowserWindow(
}

let openTime = win.openTime;
win.location = chromeURL;
win.location = AppConstants.BROWSER_CHROME_URL;
win.arguments = args; // <-- needs to be a plain JS array here.

ChromeUtils.addProfilerMarker("earlyBlankWindowVisible", openTime);
lazy.BrowserWindowTracker.registerOpeningWindow(win, forcePrivate);
return win;
}
}
Expand Down Expand Up @@ -350,13 +350,11 @@ function openBrowserWindow(
args = array;
}

let features =
"chrome,dialog=no,all" + gBrowserContentHandler.getFeatures(cmdLine);
if (forcePrivate) {
features += ",private";
}

return Services.ww.openWindow(null, chromeURL, "_blank", features, args);
return lazy.BrowserWindowTracker.openWindow({
args,
features: gBrowserContentHandler.getFeatures(cmdLine),
private: forcePrivate,
});
}

function openPreferences(cmdLine, extraArgs) {
Expand Down Expand Up @@ -931,7 +929,7 @@ nsBrowserContentHandler.prototype = {
};
var gBrowserContentHandler = new nsBrowserContentHandler();

function handURIToExistingBrowser(
async function handURIToExistingBrowser(
uri,
location,
cmdLine,
Expand All @@ -949,6 +947,13 @@ function handURIToExistingBrowser(
var navWin = lazy.BrowserWindowTracker.getTopWindow({
private: allowPrivate,
});

if (!navWin) {
navWin = await lazy.BrowserWindowTracker.getPendingWindow({
private: allowPrivate,
});
}

if (!navWin) {
// if we couldn't load it in an existing window, open a new one
openBrowserWindow(
Expand Down
99 changes: 99 additions & 0 deletions browser/modules/BrowserWindowTracker.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,16 @@

var EXPORTED_SYMBOLS = ["BrowserWindowTracker"];

const { AppConstants } = ChromeUtils.importESModule(
"resource://gre/modules/AppConstants.sys.mjs"
);

const lazy = {};

// Lazy getters
ChromeUtils.defineESModuleGetters(lazy, {
PrivateBrowsingUtils: "resource://gre/modules/PrivateBrowsingUtils.sys.mjs",
PromiseUtils: "resource://gre/modules/PromiseUtils.sys.mjs",
});

// Constants
Expand Down Expand Up @@ -145,6 +150,8 @@ var WindowHelper = {
};

const BrowserWindowTracker = {
pendingWindows: new Map(),

/**
* Get the most recent browser window.
*
Expand All @@ -169,6 +176,90 @@ const BrowserWindowTracker = {
return null;
},

/**
* Get a window that is in the process of loading. Only supports windows
* opened via the `openWindow` function in this module or that have been
* registered with the `registerOpeningWindow` function.
*
* @param {Object} options
* Options for the search.
* @param {boolean} [options.private]
* true to restrict the search to private windows only, false to restrict
* the search to non-private only. Omit the property to search in both
* groups.
*
* @returns {Promise<Window> | null}
*/
getPendingWindow(options = {}) {
for (let pending of this.pendingWindows.values()) {
if (
!("private" in options) ||
lazy.PrivateBrowsingUtils.permanentPrivateBrowsing ||
pending.isPrivate == options.private
) {
return pending.deferred.promise;
}
}
return null;
},

/**
* Registers a browser window that is in the process of opening. Normally it
* would be preferable to use the standard method for opening the window from
* this module.
*
* @param {Window} window
* The opening window.
* @param {boolean} isPrivate
* Whether the opening window is a private browsing window.
*/
registerOpeningWindow(window, isPrivate) {
let deferred = lazy.PromiseUtils.defer();

this.pendingWindows.set(window, {
isPrivate,
deferred,
});
},

/**
* A standard function for opening a new browser window.
*
* @param {Object} [options]
* Options for the new window.
* @param {boolean} [options.private]
* True to make the window a private browsing window.
* @param {String} [options.features]
* Additional window features to give the new window.
* @param {nsIArray | nsISupportsString} [options.args]
* Arguments to pass to the new window.
*
* @returns {Window}
*/
openWindow({
private: isPrivate = false,
features = undefined,
args = null,
} = {}) {
let windowFeatures = "chrome,dialog=no,all";
if (features) {
windowFeatures += `,${features}`;
}
if (isPrivate) {
windowFeatures += ",private";
}

let win = Services.ww.openWindow(
null,
AppConstants.BROWSER_CHROME_URL,
"_blank",
windowFeatures,
args
);
this.registerOpeningWindow(win, isPrivate);
return win;
},

windowCreated(browser) {
if (browser === browser.ownerGlobal.gBrowser.selectedBrowser) {
_updateCurrentBrowsingContextID(browser);
Expand Down Expand Up @@ -207,6 +298,14 @@ const BrowserWindowTracker = {
},

track(window) {
let pending = this.pendingWindows.get(window);
if (pending) {
this.pendingWindows.delete(window);
// Waiting for delayed startup to complete ensures that this new window
// has started loading its initial urls.
window.delayedStartupPromise.then(() => pending.deferred.resolve(window));
}

return WindowHelper.addWindow(window);
},

Expand Down
75 changes: 75 additions & 0 deletions browser/modules/test/browser/browser_BrowserWindowTracker.js
Original file line number Diff line number Diff line change
Expand Up @@ -164,3 +164,78 @@ add_task(async function test_orderedWindows() {
);
});
});

add_task(async function test_pendingWindows() {
Assert.equal(
BrowserWindowTracker.windowCount,
1,
"Number of tracked windows, including the test window"
);

let pending = BrowserWindowTracker.getPendingWindow();
Assert.equal(pending, null, "Should be no pending window");

let expectedWin = BrowserWindowTracker.openWindow();
pending = BrowserWindowTracker.getPendingWindow();
Assert.ok(pending, "Should be a pending window now.");
Assert.ok(
!BrowserWindowTracker.getPendingWindow({ private: true }),
"Should not be a pending private window"
);
Assert.equal(
pending,
BrowserWindowTracker.getPendingWindow({ private: false }),
"Should be the same non-private window pending"
);

let foundWin = await pending;
Assert.equal(foundWin, expectedWin, "Should have found the right window");
Assert.ok(
!BrowserWindowTracker.getPendingWindow(),
"Should be no pending window now."
);

await BrowserTestUtils.closeWindow(foundWin);

expectedWin = BrowserWindowTracker.openWindow({ private: true });
pending = BrowserWindowTracker.getPendingWindow();
Assert.ok(pending, "Should be a pending window now.");
Assert.ok(
!BrowserWindowTracker.getPendingWindow({ private: false }),
"Should not be a pending non-private window"
);
Assert.equal(
pending,
BrowserWindowTracker.getPendingWindow({ private: true }),
"Should be the same private window pending"
);

foundWin = await pending;
Assert.equal(foundWin, expectedWin, "Should have found the right window");
Assert.ok(
!BrowserWindowTracker.getPendingWindow(),
"Should be no pending window now."
);

await BrowserTestUtils.closeWindow(foundWin);

expectedWin = Services.ww.openWindow(
null,
AppConstants.BROWSER_CHROME_URL,
"_blank",
"chrome,dialog=no,all",
null
);
BrowserWindowTracker.registerOpeningWindow(expectedWin, false);
pending = BrowserWindowTracker.getPendingWindow();
Assert.ok(pending, "Should be a pending window now.");

foundWin = await pending;
Assert.equal(foundWin, expectedWin, "Should have found the right window");
Assert.ok(
!BrowserWindowTracker.getPendingWindow(),
"Should be no pending window now."
);

await BrowserTestUtils.closeWindow(foundWin);
});

0 comments on commit f68a7e6

Please sign in to comment.