Filip/playwright
1
0
Fork 0
Code Issues Pull requests Actions Packages Projects Releases Wiki Activity

fix(PageEvent): properly wait for initial navigation in chromium and webkit (#1412)

Browse source
This commit is contained in:
Dmitry Gozman 2020-03-18 17:14:18 -07:00 committed by GitHub
parent b0749e3a6d
commit 7bd924673a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 154 additions and 16 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
package.json
View file

@ -10,7 +10,7 @@
"playwright": {
"chromium_revision": "750417",
"firefox_revision": "1043",
"webkit_revision": "1179"
"webkit_revision": "1180"
},
"scripts": {
"ctest": "cross-env BROWSER=chromium node test/test.js",

23
src/chromium/crBrowser.ts
View file

@ -40,6 +40,7 @@ export class CRBrowser extends platform.EventEmitter implements Browser {
_targets = new Map<string, CRTarget>();
readonly _firstPagePromise: Promise<void>;
private _firstPageCallback = () => {};
private _popupOpeners: string[] = [];
private _tracingRecording = false;
private _tracingPath: string | null = '';
@ -104,6 +105,15 @@ export class CRBrowser extends platform.EventEmitter implements Browser {
return createPageInNewContext(this, options);
}
_onWindowOpen(openerId: string, payload: Protocol.Page.windowOpenPayload) {
// window.open($url) with noopener forces a new browser-initiated window, not
// a renderer-initiated one. When url is about:blank, we only get an
// initial navigation, and no real navigation to $url.
if (payload.windowFeatures.includes('noopener') && payload.url.startsWith('about:blank'))
return;
this._popupOpeners.push(openerId);
}
_onAttachedToTarget({targetInfo, sessionId, waitingForDebugger}: Protocol.Target.attachedToTargetPayload) {
const session = this._connection.session(sessionId)!;
if (!CRTarget.isPageType(targetInfo.type)) {
@ -148,7 +158,18 @@ export class CRBrowser extends platform.EventEmitter implements Browser {
private _createTarget(targetInfo: Protocol.Target.TargetInfo, session: CRSession | null) {
const {browserContextId} = targetInfo;
const context = (browserContextId && this._contexts.has(browserContextId)) ? this._contexts.get(browserContextId)! : this._defaultContext;
const target = new CRTarget(this, targetInfo, context, session, () => this._connection.createSession(targetInfo));
let hasInitialAboutBlank = false;
if (CRTarget.isPageType(targetInfo.type) && targetInfo.openerId) {
const openerIndex = this._popupOpeners.indexOf(targetInfo.openerId);
if (openerIndex !== -1) {
this._popupOpeners.splice(openerIndex, 1);
// When this page is a result of window.open($url) call, we should have it's opener
// in the list of popup openers. In this case we know there is an initial
// about:blank navigation, followed by a navigation to $url.
hasInitialAboutBlank = true;
}
}
const target = new CRTarget(this, targetInfo, context, session, () => this._connection.createSession(targetInfo), hasInitialAboutBlank);
assert(!this._targets.has(targetInfo.targetId), 'Target should not exist before targetCreated');
this._targets.set(targetInfo.targetId, target);
return { context, target };

9
src/chromium/crPage.ts
View file

@ -53,6 +53,8 @@ export class CRPage implements PageDelegate {
private readonly _pdf: CRPDF;
private readonly _coverage: CRCoverage;
private readonly _browserContext: CRBrowserContext;
private _firstNonInitialNavigationCommittedPromise: Promise<void>;
private _firstNonInitialNavigationCommittedCallback = () => {};
constructor(client: CRSession, browser: CRBrowser, browserContext: CRBrowserContext) {
this._client = client;
@ -64,9 +66,10 @@ export class CRPage implements PageDelegate {
this._browserContext = browserContext;
this._page = new Page(this, browserContext);
this._networkManager = new CRNetworkManager(client, this._page);
this._firstNonInitialNavigationCommittedPromise = new Promise(f => this._firstNonInitialNavigationCommittedCallback = f);
}
async initialize() {
async initialize(hasInitialAboutBlank: boolean) {
const promises: Promise<any>[] = [
this._client.send('Page.enable'),
this._client.send('Page.getFrameTree').then(({frameTree}) => {
@ -142,6 +145,8 @@ export class CRPage implements PageDelegate {
for (const source of this._browserContext._evaluateOnNewDocumentSources)
promises.push(this.evaluateOnNewDocument(source));
promises.push(this._client.send('Runtime.runIfWaitingForDebugger'));
if (hasInitialAboutBlank)
promises.push(this._firstNonInitialNavigationCommittedPromise);
await Promise.all(promises);
}
@ -189,6 +194,8 @@ export class CRPage implements PageDelegate {
_onFrameNavigated(framePayload: Protocol.Page.Frame, initial: boolean) {
this._page._frameManager.frameCommittedNewDocumentNavigation(framePayload.id, framePayload.url, framePayload.name || '', framePayload.loaderId, initial);
if (!initial)
this._firstNonInitialNavigationCommittedCallback();
}
_onFrameRequestedNavigation(payload: Protocol.Page.frameRequestedNavigationPayload) {

8
src/chromium/crTarget.ts
View file

@ -19,7 +19,7 @@ import { CRBrowser, CRBrowserContext } from './crBrowser';
import { CRSession, CRSessionEvents } from './crConnection';
import { Page, Worker } from '../page';
import { Protocol } from './protocol';
import { debugError, assert } from '../helper';
import { debugError, assert, helper } from '../helper';
import { CRPage } from './crPage';
import { CRExecutionContext } from './crExecutionContext';
@ -49,7 +49,8 @@ export class CRTarget {
targetInfo: Protocol.Target.TargetInfo,
browserContext: CRBrowserContext,
session: CRSession | null,
sessionFactory: () => Promise<CRSession>) {
sessionFactory: () => Promise<CRSession>,
hasInitialAboutBlank: boolean) {
this._targetInfo = targetInfo;
this._browser = browser;
this._browserContext = browserContext;
@ -58,10 +59,11 @@ export class CRTarget {
if (CRTarget.isPageType(targetInfo.type)) {
assert(session, 'Page target must be created with existing session');
this._crPage = new CRPage(session, this._browser, this._browserContext);
helper.addEventListener(session, 'Page.windowOpen', event => browser._onWindowOpen(targetInfo.targetId, event));
const page = this._crPage.page();
(page as any)[targetSymbol] = this;
session.once(CRSessionEvents.Disconnected, () => page._didDisconnect());
this._pagePromise = this._crPage.initialize().then(() => this._initializedPage = page).catch(e => e);
this._pagePromise = this._crPage.initialize(hasInitialAboutBlank).then(() => this._initializedPage = page).catch(e => e);
}
}

19
src/webkit/wkBrowser.ts
View file

@ -38,6 +38,7 @@ export class WKBrowser extends platform.EventEmitter implements Browser {
readonly _contexts = new Map<string, WKBrowserContext>();
readonly _wkPages = new Map<string, WKPage>();
private readonly _eventListeners: RegisteredListener[];
private _popupOpeners: string[] = [];
private _firstPageCallback: () => void = () => {};
private readonly _firstPagePromise: Promise<void>;
@ -59,6 +60,7 @@ export class WKBrowser extends platform.EventEmitter implements Browser {
helper.addEventListener(this._browserSession, 'Playwright.pageProxyCreated', this._onPageProxyCreated.bind(this)),
helper.addEventListener(this._browserSession, 'Playwright.pageProxyDestroyed', this._onPageProxyDestroyed.bind(this)),
helper.addEventListener(this._browserSession, 'Playwright.provisionalLoadFailed', event => this._onProvisionalLoadFailed(event)),
helper.addEventListener(this._browserSession, 'Playwright.windowOpen', this._onWindowOpen.bind(this)),
helper.addEventListener(this._browserSession, kPageProxyMessageReceived, this._onPageProxyMessageReceived.bind(this)),
];
@ -98,6 +100,10 @@ export class WKBrowser extends platform.EventEmitter implements Browser {
return this._firstPagePromise;
}
_onWindowOpen(payload: Protocol.Playwright.windowOpenPayload) {
this._popupOpeners.push(payload.pageProxyId);
}
_onPageProxyCreated(event: Protocol.Playwright.pageProxyCreatedPayload) {
const { pageProxyInfo } = event;
const pageProxyId = pageProxyInfo.pageProxyId;
@ -117,7 +123,18 @@ export class WKBrowser extends platform.EventEmitter implements Browser {
this._connection.rawSend({ ...message, pageProxyId });
});
const opener = pageProxyInfo.openerId ? this._wkPages.get(pageProxyInfo.openerId) : undefined;
const wkPage = new WKPage(context, pageProxySession, opener || null);
let hasInitialAboutBlank = false;
if (pageProxyInfo.openerId) {
const openerIndex = this._popupOpeners.indexOf(pageProxyInfo.openerId);
if (openerIndex !== -1) {
this._popupOpeners.splice(openerIndex, 1);
// When this page is a result of window.open($url) call, we should have it's opener
// in the list of popup openers. In this case we know there is an initial
// about:blank navigation, followed by a navigation to $url.
hasInitialAboutBlank = true;
}
}
const wkPage = new WKPage(context, pageProxySession, opener || null, hasInitialAboutBlank);
this._wkPages.set(pageProxyId, wkPage);
const pageEvent = new PageEvent(context, wkPage.pageOrError());

13
src/webkit/wkPage.ts
View file

@ -57,10 +57,14 @@ export class WKPage implements PageDelegate {
private readonly _evaluateOnNewDocumentSources: string[] = [];
readonly _browserContext: WKBrowserContext;
private _initialized = false;
private _hasInitialAboutBlank: boolean;
private _firstNonInitialNavigationCommittedPromise: Promise<void>;
private _firstNonInitialNavigationCommittedCallback = () => {};
constructor(browserContext: WKBrowserContext, pageProxySession: WKSession, opener: WKPage | null) {
constructor(browserContext: WKBrowserContext, pageProxySession: WKSession, opener: WKPage | null, hasInitialAboutBlank: boolean) {
this._pageProxySession = pageProxySession;
this._opener = opener;
this._hasInitialAboutBlank = hasInitialAboutBlank;
this.rawKeyboard = new RawKeyboardImpl(pageProxySession);
this.rawMouse = new RawMouseImpl(pageProxySession);
this._contextIdToContext = new Map();
@ -76,6 +80,7 @@ export class WKPage implements PageDelegate {
helper.addEventListener(this._pageProxySession, 'Target.didCommitProvisionalTarget', this._onDidCommitProvisionalTarget.bind(this)),
];
this._pagePromise = new Promise(f => this._pagePromiseCallback = f);
this._firstNonInitialNavigationCommittedPromise = new Promise(f => this._firstNonInitialNavigationCommittedCallback = f);
}
_initializedPage(): Page | null {
@ -251,6 +256,8 @@ export class WKPage implements PageDelegate {
}
if (targetInfo.isPaused)
this._pageProxySession.send('Target.resume', { targetId: targetInfo.targetId }).catch(debugError);
if (this._hasInitialAboutBlank)
await this._firstNonInitialNavigationCommittedPromise;
this._initialized = true;
this._pagePromiseCallback(pageOrError);
} else {
@ -330,6 +337,8 @@ export class WKPage implements PageDelegate {
private _handleFrameTree(frameTree: Protocol.Page.FrameResourceTree) {
this._onFrameAttached(frameTree.frame.id, frameTree.frame.parentId || null);
this._onFrameNavigated(frameTree.frame, true);
this._page._frameManager.frameLifecycleEvent(frameTree.frame.id, 'domcontentloaded');
this._page._frameManager.frameLifecycleEvent(frameTree.frame.id, 'load');
if (!frameTree.childFrames)
return;
@ -348,6 +357,8 @@ export class WKPage implements PageDelegate {
if (!framePayload.parentId)
this._workers.clear();
this._page._frameManager.frameCommittedNewDocumentNavigation(framePayload.id, framePayload.url, framePayload.name || '', framePayload.loaderId, initial);
if (!initial)
this._firstNonInitialNavigationCommittedCallback();
}
private _onFrameNavigatedWithinDocument(frameId: string, url: string) {

28
test/browsercontext.spec.js
View file

@ -480,7 +480,7 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FF
});
describe('Events.BrowserContext.PageEvent', function() {
it.fail(true)('should have url with nowait', async({browser, server}) => {
it.fail(FFOX)('should have url with nowait', async({browser, server}) => {
const context = await browser.newContext();
const page = await context.newPage();
const [otherPage] = await Promise.all([
@ -490,7 +490,7 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FF
expect(otherPage.url()).toBe(server.EMPTY_PAGE);
await context.close();
});
it.fail(CHROMIUM)('should have url with domcontentloaded', async({browser, server}) => {
it('should have url with domcontentloaded', async({browser, server}) => {
const context = await browser.newContext();
const page = await context.newPage();
const [otherPage] = await Promise.all([
@ -500,6 +500,26 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FF
expect(otherPage.url()).toBe(server.EMPTY_PAGE);
await context.close();
});
it('should have about:blank url with domcontentloaded', async({browser, server}) => {
const context = await browser.newContext();
const page = await context.newPage();
const [otherPage] = await Promise.all([
context.waitForEvent('page').then(event => event.page({ waitUntil: 'domcontentloaded' })),
page.evaluate(url => window.open(url), 'about:blank')
]);
expect(otherPage.url()).toBe('about:blank');
await context.close();
});
it.fail(FFOX)('should have about:blank for empty url with domcontentloaded', async({browser, server}) => {
const context = await browser.newContext();
const page = await context.newPage();
const [otherPage] = await Promise.all([
context.waitForEvent('page').then(event => event.page({ waitUntil: 'domcontentloaded' })),
page.evaluate(() => window.open())
]);
expect(otherPage.url()).toBe('about:blank');
await context.close();
});
it('should report when a new page is created and closed', async({browser, server}) => {
const context = await browser.newContext();
const page = await context.newPage();
@ -527,7 +547,7 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FF
});
it('should report initialized pages', async({browser, server}) => {
const context = await browser.newContext();
const pagePromise = context.waitForEvent('page').then(e => e.page({ waitUntil: 'nowait' }));
const pagePromise = context.waitForEvent('page').then(e => e.page());
context.newPage();
const newPage = await pagePromise;
expect(newPage.url()).toBe('about:blank');
@ -576,7 +596,7 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FF
const context = await browser.newContext();
const events = [];
context.on('page', async event => {
const page = await event.page({ waitUntil: 'nowait' });
const page = await event.page();
events.push('CREATED: ' + page.url());
page.on('close', () => events.push('DESTROYED: ' + page.url()));
});

5
test/page.spec.js
View file

@ -224,13 +224,14 @@ module.exports.describe = function({testRunner, expect, headless, playwright, FF
});
});
// @see https://github.com/GoogleChrome/puppeteer/issues/3865
it('should not throw when there are console messages in detached iframes', async({page, server}) => {
it.fail(FFOX)('should not throw when there are console messages in detached iframes', async({page, server}) => {
await page.goto(server.EMPTY_PAGE);
const [popup] = await Promise.all([
page.waitForEvent('popup').then(e => e.page()),
page.evaluate(async() => {
// 1. Create a popup that Playwright is not connected to.
const win = window.open(window.location.href);
const win = window.open('');
window._popup = win;
if (window.document.readyState !== 'complete')
await new Promise(f => window.addEventListener('load', f));
// 2. In this popup, create an iframe that console.logs a message.

63
test/popup.spec.js
View file

@ -292,12 +292,23 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, WE
await evaluatePromise;
await context.close();
});
it('should work with empty url', async({browser}) => {
it.fail(FFOX)('should work with empty url', async({browser}) => {
const context = await browser.newContext();
const page = await context.newPage();
const [popup] = await Promise.all([
page.waitForEvent('popup').then(e => e.page()),
page.evaluate(() => window.__popup = window.open('')),
]);
expect(await page.evaluate(() => !!window.opener)).toBe(false);
expect(await popup.evaluate(() => !!window.opener)).toBe(true);
await context.close();
});
it('should work with empty url and nowait', async({browser}) => {
const context = await browser.newContext();
const page = await context.newPage();
const [popup] = await Promise.all([
page.waitForEvent('popup').then(e => e.page({ waitUntil: 'nowait' })),
page.evaluate(() => window.__popup = window.open('')),
page.evaluate(() => window.__popup = window.open()),
]);
expect(await page.evaluate(() => !!window.opener)).toBe(false);
expect(await popup.evaluate(() => !!window.opener)).toBe(true);
@ -314,6 +325,41 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, WE
expect(await popup.evaluate(() => !!window.opener)).toBe(false);
await context.close();
});
it('should work with noopener and nowait', async({browser}) => {
const context = await browser.newContext();
const page = await context.newPage();
const [popup] = await Promise.all([
page.waitForEvent('popup').then(e => e.page({ waitUntil: 'nowait' })),
page.evaluate(() => window.__popup = window.open('about:blank', null, 'noopener')),
]);
expect(await page.evaluate(() => !!window.opener)).toBe(false);
expect(await popup.evaluate(() => !!window.opener)).toBe(false);
await context.close();
});
it('should work with noopener and url', async({browser, server}) => {
const context = await browser.newContext();
const page = await context.newPage();
await page.goto(server.EMPTY_PAGE);
const [popup] = await Promise.all([
page.waitForEvent('popup').then(e => e.page()),
page.evaluate(url => window.__popup = window.open(url, null, 'noopener'), server.EMPTY_PAGE),
]);
expect(await page.evaluate(() => !!window.opener)).toBe(false);
expect(await popup.evaluate(() => !!window.opener)).toBe(false);
await context.close();
});
it('should work with noopener and url and nowait', async({browser, server}) => {
const context = await browser.newContext();
const page = await context.newPage();
await page.goto(server.EMPTY_PAGE);
const [popup] = await Promise.all([
page.waitForEvent('popup').then(e => e.page({ waitUntil: 'nowait' })),
page.evaluate(url => window.__popup = window.open(url, null, 'noopener'), server.EMPTY_PAGE),
]);
expect(await page.evaluate(() => !!window.opener)).toBe(false);
expect(await popup.evaluate(() => !!window.opener)).toBe(false);
await context.close();
});
it('should work with clicking target=_blank', async({browser, server}) => {
const context = await browser.newContext();
const page = await context.newPage();
@ -327,6 +373,19 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, WE
expect(await popup.evaluate(() => !!window.opener)).toBe(true);
await context.close();
});
it('should work with clicking target=_blank and nowait', async({browser, server}) => {
const context = await browser.newContext();
const page = await context.newPage();
await page.goto(server.EMPTY_PAGE);
await page.setContent('<a target=_blank rel="opener" href="/one-style.html">yo</a>');
const [popup] = await Promise.all([
page.waitForEvent('popup').then(e => e.page({ waitUntil: 'nowait' })),
page.click('a'),
]);
expect(await page.evaluate(() => !!window.opener)).toBe(false);
expect(await popup.evaluate(() => !!window.opener)).toBe(true);
await context.close();
});
it('should work with fake-clicking target=_blank and rel=noopener', async({browser, server}) => {
const context = await browser.newContext();
const page = await context.newPage();