fix: gracefully handle multiple contexts for secondary DOM World (#4276)

In case of multiple sessions to the same target, there's a race between
sessions to create a secondary isolated world. As a result, we might
end up having 2 execution contexts created for the needs of the
secondary isolated world.

This patch starts handling this race gracefully: instead of crashing,
we can use either of the execution contexts and ignore the rest.

Notably, the same race condition might happen if page reloads itself
in-between the calls to `page.addEvaluateOnNewDocument` and
`page.createIsolatedWorld`.

Fixes #4197.
This commit is contained in:
Andrey Lushnikov 2019-04-11 16:26:18 -04:00 committed by GitHub
parent 2265974ce5
commit 20988775bf
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 26 additions and 2 deletions

View File

@ -70,6 +70,13 @@ class DOMWorld {
}
}
/**
* @return {boolean}
*/
_hasContext() {
return !this._contextResolveCallback;
}
_detach() {
this._detached = true;
for (const waitTask of this._waitTasks)

View File

@ -306,11 +306,15 @@ class FrameManager extends EventEmitter {
const frame = this._frames.get(frameId) || null;
let world = null;
if (frame) {
if (contextPayload.auxData && !!contextPayload.auxData['isDefault'])
if (contextPayload.auxData && !!contextPayload.auxData['isDefault']) {
world = frame._mainWorld;
else if (contextPayload.name === UTILITY_WORLD_NAME)
} else if (contextPayload.name === UTILITY_WORLD_NAME && !frame._secondaryWorld._hasContext()) {
// In case of multiple sessions to the same target, there's a race between
// connections so we might end up creating multiple isolated worlds.
// We can use either.
world = frame._secondaryWorld;
}
}
if (contextPayload.auxData && contextPayload.auxData['type'] === 'isolated')
this._isolatedWorlds.add(contextPayload.name);
/** @type {!ExecutionContext} */

View File

@ -316,6 +316,19 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions, p
expect(await restoredPage.evaluate(() => 7 * 8)).toBe(56);
await browser.close();
});
// @see https://github.com/GoogleChrome/puppeteer/issues/4197#issuecomment-481793410
it('should be able to connect to the same page simultaneously', async({server}) => {
const browserOne = await puppeteer.launch();
const browserTwo = await puppeteer.connect({ browserWSEndpoint: browserOne.wsEndpoint() });
const [page1, page2] = await Promise.all([
new Promise(x => browserOne.once('targetcreated', target => x(target.page()))),
browserTwo.newPage(),
]);
expect(await page1.evaluate(() => 7 * 8)).toBe(56);
expect(await page2.evaluate(() => 7 * 6)).toBe(42);
await browserOne.close();
});
});
describe('Puppeteer.executablePath', function() {
it('should work', async({server}) => {