Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-10-09 19:35:51 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1528456. Fix browser_domainPolicy.js to not be racy. r=Gijs

Browse Source 
The old complicated code here was working around a fundamental race in the
test.  The test was trying to do the following:

1)  Open a new tab.  This sends an IPC message to do that and to start loading
about:blank in the tab.

2)  Add a "load" listener.  This sends an IPC message to add the listener.

3)  Start loading the URL we really want to load.  This sends an IPC message to
start the load.

There are two races here: the message from step 2 can be received before or
after the about:blank load is done, and the message from step 3 can be received
before or after the about:blank load is done.  If step 2 wins the race (we add
the listener before the about:blank load is done) but step 3 loses the race
(new load starts only after the about:blank load is done), then we would get a
load event for the about:blank, not for the url we care about.

The test worked around this by having step 2 add a listener for the
"DOMDocElementInserted" event, which about:blank did not fire.  Then when that
happened (indicating that the load from step 3 is now in progress), it added a
"load" listener.  This fixed the race.

Bug 1528146 fixed about:blank to also fire "DOMDocElementInserted", so the test
became racy again.  Now if the "DOMDocElementInserted" listener got added
_after_ the about:blank load had fired that event (which is what usually
happened), then the test passed.  But if the IPC message from step 2 to add the
"DOMDocElementInserted" listener won the race against the about:blank load
firing "DOMDocElementInserted", then we added the "load" listener too early and
got the "load" event for the about:blank, not the document we cared about.

The fix is to just use the (now) existing openNewForegroundTab function which
lets us pass in a url, then wait until that URL is loaded, instead of trying to
reinvent that wheel.

There is an additional ride-along change to make openNewForegroundTab always
create a new process in this test, which is the original intent of the test but
wasn't necessarily guaranteed by the old code.

Differential Revision: https://phabricator.services.mozilla.com/D20544

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Boris Zbarsky 2019-02-20 20:12:16 +00:00
parent 5321bbb16b
commit f8e0094981
1 changed files with 10 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
dom/ipc/tests/browser_domainPolicy.js
View File

@ -27,29 +27,10 @@ async function test_domainPolicy() {
// Init test
function initProcess() {
tab = BrowserTestUtils.addTab(gBrowser);
gBrowser.selectedTab = tab;
let initPromise = ContentTask.spawn(tab.linkedBrowser, null, function() {
const {PromiseUtils} = ChromeUtils.import("resource://gre/modules/PromiseUtils.jsm");
function loadBase() {
let deferred = PromiseUtils.defer();
let listener = (event) => {
removeEventListener("DOMDocElementInserted", listener, true);
let listener2 = (event2) => {
content.removeEventListener("load", listener2);
deferred.resolve();
};
content.addEventListener("load", listener2);
};
addEventListener("DOMDocElementInserted", listener, true);
return deferred.promise;
}
return loadBase();
});
BrowserTestUtils.loadURI(tab.linkedBrowser, "http://mochi.test:8888/browser/dom/ipc/tests/file_domainPolicy_base.html");
return initPromise;
return BrowserTestUtils.openNewForegroundTab(
{gBrowser,
opening: "http://mochi.test:8888/browser/dom/ipc/tests/file_domainPolicy_base.html",
forceNewProcess: true});
}
// We use ContentTask for the tests, but we also want to pass some data and some helper functions too.
@ -113,8 +94,7 @@ async function test_domainPolicy() {
info("Testing simple blocklist policy");
info("Creating child process first, activating domainPolicy after");
currentTask = initProcess();
await currentTask;
tab = await initProcess();
activateDomainPolicy();
var bl = policy.blocklist;
bl.add(Services.io.newURI("http://example.com"));
@ -125,8 +105,7 @@ async function test_domainPolicy() {
activateDomainPolicy();
bl = policy.blocklist;
bl.add(Services.io.newURI("http://example.com"));
currentTask = initProcess();
await currentTask;
tab = await initProcess();
currentTask = runTest(testDomain("http://example.com"));
checkAndCleanup(await currentTask);
@ -185,8 +164,7 @@ async function test_domainPolicy() {
info("Testing Blocklist-style Domain Policy");
info("Activating domainPolicy first, creating child process after");
activate(true, testPolicy.exceptions, testPolicy.superExceptions);
currentTask = initProcess();
await currentTask;
tab = await initProcess();
let results = [];
currentTask = runTest(testList(true, testPolicy.notExempt));
results = results.concat(await currentTask);
@ -195,8 +173,7 @@ async function test_domainPolicy() {
checkAndCleanup(results);
info("Creating child process first, activating domainPolicy after");
currentTask = initProcess();
await currentTask;
tab = await initProcess();
activate(true, testPolicy.exceptions, testPolicy.superExceptions);
results = [];
currentTask = runTest(testList(true, testPolicy.notExempt));
@ -213,8 +190,7 @@ async function test_domainPolicy() {
info("Activating domainPolicy first, creating child process after");
activate(false, testPolicy.exceptions, testPolicy.superExceptions);
currentTask = initProcess();
await currentTask;
tab = await initProcess();
results = [];
currentTask = runTest(testList(false, testPolicy.notExempt));
results = results.concat(await currentTask);
@ -223,8 +199,7 @@ async function test_domainPolicy() {
checkAndCleanup(results);
info("Creating child process first, activating domainPolicy after");
currentTask = initProcess();
await currentTask;
tab = await initProcess();
activate(false, testPolicy.exceptions, testPolicy.superExceptions);
results = [];
currentTask = runTest(testList(false, testPolicy.notExempt));