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

fix(chromium): associate navigation requests with navigations (#1724)

Browse source
This commit is contained in:
Pavel Feldman 2020-04-09 19:03:06 -07:00 committed by GitHub
parent 42beb3784f
commit 3584205086
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 31 additions and 16 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
src/chromium/crConnection.ts
View file

@ -121,6 +121,7 @@ export class CRSession extends EventEmitter {
private readonly _targetType: string; private readonly _targetType: string;
private readonly _sessionId: string; private readonly _sessionId: string;
private readonly _rootSessionId: string; private readonly _rootSessionId: string;
private _unhandledException: Error | undefined;
on: <T extends keyof Protocol.Events | symbol>(event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this; on: <T extends keyof Protocol.Events | symbol>(event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this;
addListener: <T extends keyof Protocol.Events | symbol>(event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this; addListener: <T extends keyof Protocol.Events | symbol>(event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this;
off: <T extends keyof Protocol.Events | symbol>(event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this; off: <T extends keyof Protocol.Events | symbol>(event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this;
@ -145,6 +146,8 @@ export class CRSession extends EventEmitter {
method: T, method: T,
params?: Protocol.CommandParameters[T] params?: Protocol.CommandParameters[T]
): Promise<Protocol.CommandReturnValues[T]> { ): Promise<Protocol.CommandReturnValues[T]> {
if (this._unhandledException)
throw this._unhandledException;
if (!this._connection) if (!this._connection)
throw new Error(`Protocol error (${method}): Session closed. Most likely the ${this._targetType} has been closed.`); throw new Error(`Protocol error (${method}): Session closed. Most likely the ${this._targetType} has been closed.`);
const id = this._connection._rawSend(this._sessionId, method, params); const id = this._connection._rawSend(this._sessionId, method, params);
@ -163,7 +166,7 @@ export class CRSession extends EventEmitter {
callback.resolve(object.result); callback.resolve(object.result);
} else { } else {
assert(!object.id); assert(!object.id);
Promise.resolve().then(() => this.emit(object.method!, object.params)); Promise.resolve().then(() => this.emit(object.method!, object.params)).catch(e => this._unhandledException = this._unhandledException || e);
} }
} }

2
src/chromium/crNetworkManager.ts
View file

@ -182,6 +182,8 @@ export class CRNetworkManager {
} }
const isNavigationRequest = event.requestId === event.loaderId && event.type === 'Document'; const isNavigationRequest = event.requestId === event.loaderId && event.type === 'Document';
const documentId = isNavigationRequest ? event.loaderId : undefined; const documentId = isNavigationRequest ? event.loaderId : undefined;
if (isNavigationRequest)
this._page._frameManager.frameUpdatedDocumentIdForNavigation(event.frameId!, documentId!);
const request = new InterceptableRequest(this._client, frame, interceptionId, documentId, this._userRequestInterceptionEnabled, event, redirectedFrom); const request = new InterceptableRequest(this._client, frame, interceptionId, documentId, this._userRequestInterceptionEnabled, event, redirectedFrom);
this._requestIdToRequest.set(event.requestId, request); this._requestIdToRequest.set(event.requestId, request);
this._page._frameManager.requestStarted(request.request); this._page._frameManager.requestStarted(request.request);

2
src/chromium/crPage.ts
View file

@ -482,7 +482,7 @@ class FrameSession {
} }
_onFrameRequestedNavigation(payload: Protocol.Page.frameRequestedNavigationPayload) { _onFrameRequestedNavigation(payload: Protocol.Page.frameRequestedNavigationPayload) {
this._page._frameManager.frameRequestedNavigation(payload.frameId); this._page._frameManager.frameRequestedNavigation(payload.frameId, '');
} }
_onFrameNavigatedWithinDocument(frameId: string, url: string) { _onFrameNavigatedWithinDocument(frameId: string, url: string) {

6
src/firefox/ffPage.ts
View file

@ -65,7 +65,7 @@ export class FFPage implements PageDelegate {
helper.addEventListener(this._session, 'Page.frameDetached', this._onFrameDetached.bind(this)), helper.addEventListener(this._session, 'Page.frameDetached', this._onFrameDetached.bind(this)),
helper.addEventListener(this._session, 'Page.navigationAborted', this._onNavigationAborted.bind(this)), helper.addEventListener(this._session, 'Page.navigationAborted', this._onNavigationAborted.bind(this)),
helper.addEventListener(this._session, 'Page.navigationCommitted', this._onNavigationCommitted.bind(this)), helper.addEventListener(this._session, 'Page.navigationCommitted', this._onNavigationCommitted.bind(this)),
helper.addEventListener(this._session, 'Page.navigationStarted', event => this._onNavigationStarted(event.frameId)), helper.addEventListener(this._session, 'Page.navigationStarted', this._onNavigationStarted.bind(this)),
helper.addEventListener(this._session, 'Page.sameDocumentNavigation', this._onSameDocumentNavigation.bind(this)), helper.addEventListener(this._session, 'Page.sameDocumentNavigation', this._onSameDocumentNavigation.bind(this)),
helper.addEventListener(this._session, 'Runtime.executionContextCreated', this._onExecutionContextCreated.bind(this)), helper.addEventListener(this._session, 'Runtime.executionContextCreated', this._onExecutionContextCreated.bind(this)),
helper.addEventListener(this._session, 'Runtime.executionContextDestroyed', this._onExecutionContextDestroyed.bind(this)), helper.addEventListener(this._session, 'Runtime.executionContextDestroyed', this._onExecutionContextDestroyed.bind(this)),
@ -136,8 +136,8 @@ export class FFPage implements PageDelegate {
this._page._frameManager.frameDidPotentiallyRequestNavigation(); this._page._frameManager.frameDidPotentiallyRequestNavigation();
} }
_onNavigationStarted(frameId: string) { _onNavigationStarted(params: Protocol.Page.navigationStartedPayload) {
this._page._frameManager.frameRequestedNavigation(frameId); this._page._frameManager.frameRequestedNavigation(params.frameId, params.navigationId);
} }
_onNavigationAborted(params: Protocol.Page.navigationAbortedPayload) { _onNavigationAborted(params: Protocol.Page.navigationAbortedPayload) {

18
src/frames.ts
View file

@ -127,12 +127,20 @@ export class FrameManager {
barrier.release(); barrier.release();
} }
frameRequestedNavigation(frameId: string) { frameRequestedNavigation(frameId: string, documentId: string) {
const frame = this._frames.get(frameId); const frame = this._frames.get(frameId);
if (!frame) if (!frame)
return; return;
for (const barrier of this._pendingNavigationBarriers) for (const barrier of this._pendingNavigationBarriers)
barrier.addFrame(frame); barrier.addFrame(frame);
frame._pendingDocumentId = documentId;
}
frameUpdatedDocumentIdForNavigation(frameId: string, documentId: string) {
const frame = this._frames.get(frameId);
if (!frame)
return;
frame._pendingDocumentId = documentId;
} }
frameCommittedNewDocumentNavigation(frameId: string, url: string, name: string, documentId: string, initial: boolean) { frameCommittedNewDocumentNavigation(frameId: string, url: string, name: string, documentId: string, initial: boolean) {
@ -140,7 +148,9 @@ export class FrameManager {
this.removeChildFramesRecursively(frame); this.removeChildFramesRecursively(frame);
frame._url = url; frame._url = url;
frame._name = name; frame._name = name;
assert(!frame._pendingDocumentId || frame._pendingDocumentId === documentId);
frame._lastDocumentId = documentId; frame._lastDocumentId = documentId;
frame._pendingDocumentId = '';
for (const task of frame._frameTasks) for (const task of frame._frameTasks)
task.onNewDocument(documentId); task.onNewDocument(documentId);
this.clearFrameLifecycle(frame); this.clearFrameLifecycle(frame);
@ -225,8 +235,9 @@ export class FrameManager {
requestFailed(request: network.Request, canceled: boolean) { requestFailed(request: network.Request, canceled: boolean) {
this._inflightRequestFinished(request); this._inflightRequestFinished(request);
if (request._documentId) { if (request._documentId) {
const isCurrentDocument = request.frame()._lastDocumentId === request._documentId; const isPendingDocument = request.frame()._pendingDocumentId === request._documentId;
if (!isCurrentDocument) { if (isPendingDocument) {
request.frame()._pendingDocumentId = '';
let errorText = request.failure()!.errorText; let errorText = request.failure()!.errorText;
if (canceled) if (canceled)
errorText += '; maybe frame was detached?'; errorText += '; maybe frame was detached?';
@ -319,6 +330,7 @@ export class Frame {
_id: string; _id: string;
readonly _firedLifecycleEvents: Set<types.LifecycleEvent>; readonly _firedLifecycleEvents: Set<types.LifecycleEvent>;
_lastDocumentId = ''; _lastDocumentId = '';
_pendingDocumentId = '';
_frameTasks = new Set<FrameTask>(); _frameTasks = new Set<FrameTask>();
readonly _page: Page; readonly _page: Page;
private _parentFrame: Frame | null; private _parentFrame: Frame | null;

4
src/webkit/wkPage.ts
View file

@ -329,7 +329,7 @@ export class WKPage implements PageDelegate {
} }
private _onFrameScheduledNavigation(frameId: string) { private _onFrameScheduledNavigation(frameId: string) {
this._page._frameManager.frameRequestedNavigation(frameId); this._page._frameManager.frameRequestedNavigation(frameId, '');
} }
private _onFrameStoppedLoading(frameId: string) { private _onFrameStoppedLoading(frameId: string) {
@ -761,6 +761,8 @@ export class WKPage implements PageDelegate {
// TODO(einbinder) this will fail if we are an XHR document request // TODO(einbinder) this will fail if we are an XHR document request
const isNavigationRequest = event.type === 'Document'; const isNavigationRequest = event.type === 'Document';
const documentId = isNavigationRequest ? event.loaderId : undefined; const documentId = isNavigationRequest ? event.loaderId : undefined;
if (isNavigationRequest)
this._page._frameManager.frameUpdatedDocumentIdForNavigation(event.frameId, documentId!);
const request = new WKInterceptableRequest(session, this._page._needsRequestInterception(), frame, event, redirectedFrom, documentId); const request = new WKInterceptableRequest(session, this._page._needsRequestInterception(), frame, event, redirectedFrom, documentId);
this._requestIdToRequest.set(event.requestId, request); this._requestIdToRequest.set(event.requestId, request);
this._page._frameManager.requestStarted(request.request); this._page._frameManager.requestStarted(request.request);

7
test/autowaiting.spec.js
View file

@ -103,7 +103,7 @@ describe('Auto waiting', () => {
]); ]);
expect(messages.join('|')).toBe('route|domcontentloaded|evaluate'); expect(messages.join('|')).toBe('route|domcontentloaded|evaluate');
}); });
it.fail(CHROMIUM)('should await navigation when assigning location twice', async({page, server}) => { it('should await navigation when assigning location twice', async({page, server}) => {
const messages = []; const messages = [];
server.setRoute('/empty.html?cancel', async (req, res) => { res.end('done'); }); server.setRoute('/empty.html?cancel', async (req, res) => { res.end('done'); });
server.setRoute('/empty.html?override', async (req, res) => { messages.push('routeoverride'); res.end('done'); }); server.setRoute('/empty.html?override', async (req, res) => { messages.push('routeoverride'); res.end('done'); });
@ -196,10 +196,7 @@ describe('Auto waiting should not hang when', () => {
setTimeout(() => window.stop(), 100); setTimeout(() => window.stop(), 100);
}, server.EMPTY_PAGE); }, server.EMPTY_PAGE);
}); });
it.fail(CHROMIUM)('calling window.stop sync', async({page, server, httpsServer}) => { it('calling window.stop sync', async({page, server, httpsServer}) => {
// Flaky, see https://github.com/microsoft/playwright/pull/1630/checks?check_run_id=553475173.
// We only get Page.frameStoppedLoading, but do not know that navigation was aborted or
// that navigation request was cancelled.
await page.evaluate((url) => { await page.evaluate((url) => {
window.location.href = url; window.location.href = url;
window.stop(); window.stop();

3
test/launcher.spec.js
View file

@ -316,8 +316,7 @@ describe('browserType.launchPersistentContext', function() {
await removeUserDataDir(userDataDir); await removeUserDataDir(userDataDir);
await removeUserDataDir(userDataDir2); await removeUserDataDir(userDataDir2);
}); });
// See https://github.com/microsoft/playwright/issues/717 it.slow()('userDataDir option should restore cookies', async({browserType, server}) => {
it.slow().fail(WIN && CHROMIUM)('userDataDir option should restore cookies', async({browserType, server}) => {
const userDataDir = await makeUserDataDir(); const userDataDir = await makeUserDataDir();
const browserContext = await browserType.launchPersistentContext(userDataDir, defaultBrowserOptions); const browserContext = await browserType.launchPersistentContext(userDataDir, defaultBrowserOptions);
const page = await browserContext.newPage(); const page = await browserContext.newPage();