Skip to content

Commit 284c4ee

Browse files
pavelfeldmanpavelfeldman
committed
feat(cr, wk): make clicks, input and evaluate await scheduled navigations
1 parent 3bedc60 commit 284c4ee

File tree

6 files changed

+215
-40
lines changed

6 files changed

+215
-40
lines changed

‎src/chromium/crPage.ts

+5
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ export class CRPage implements PageDelegate {
8080
helper.addEventListener(this._client, 'Page.frameAttached', event => this._onFrameAttached(event.frameId, event.parentFrameId)),
8181
helper.addEventListener(this._client, 'Page.frameDetached', event => this._onFrameDetached(event.frameId)),
8282
helper.addEventListener(this._client, 'Page.frameNavigated', event => this._onFrameNavigated(event.frame, false)),
83+
helper.addEventListener(this._client, 'Page.frameRequestedNavigation', event => this._onFrameRequestedNavigation(event)),
8384
helper.addEventListener(this._client, 'Page.frameStoppedLoading', event => this._onFrameStoppedLoading(event.frameId)),
8485
helper.addEventListener(this._client, 'Page.javascriptDialogOpening', event => this._onDialog(event)),
8586
helper.addEventListener(this._client, 'Page.lifecycleEvent', event => this._onLifecycleEvent(event)),
@@ -172,6 +173,10 @@ export class CRPage implements PageDelegate {
172173
this._page._frameManager.frameCommittedNewDocumentNavigation(framePayload.id, framePayload.url, framePayload.name || '', framePayload.loaderId, initial);
173174
}
174175

176+
_onFrameRequestedNavigation(payload: Protocol.Page.frameRequestedNavigationPayload) {
177+
this._page._frameManager.frameRequestedNavigation(payload.frameId);
178+
}
179+
175180
async _ensureIsolatedWorld(name: string) {
176181
if (this._isolatedWorlds.has(name))
177182
return;

‎src/dom.ts

+39-21
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,10 @@ export class FrameExecutionContext extends js.ExecutionContext {
5252

5353
if (!args.some(needsAdoption)) {
5454
// Only go through asynchronous calls if required.
55-
return this._delegate.evaluate(this, returnByValue, pageFunction, ...args);
55+
const result = await this.frame._page._frameManager.waitForNavigationsCreatedBy(async () => {
56+
return this._delegate.evaluate(this, returnByValue, pageFunction, ...args);
57+
});
58+
return result;
5659
}
5760

5861
const toDispose: Promise<ElementHandle>[] = [];
@@ -65,7 +68,9 @@ export class FrameExecutionContext extends js.ExecutionContext {
6568
}));
6669
let result;
6770
try {
68-
result = await this._delegate.evaluate(this, returnByValue, pageFunction, ...adopted);
71+
result = await this.frame._page._frameManager.waitForNavigationsCreatedBy(async () => {
72+
return this._delegate.evaluate(this, returnByValue, pageFunction, ...adopted);
73+
});
6974
} finally {
7075
toDispose.map(handlePromise => handlePromise.then(handle => handle.dispose()));
7176
}
@@ -248,12 +253,15 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
248253
const point = offset ? await this._offsetPoint(offset) : await this._clickablePoint();
249254
if (waitFor)
250255
await this._waitForHitTargetAt(point, options);
251-
let restoreModifiers: input.Modifier[] | undefined;
252-
if (options && options.modifiers)
253-
restoreModifiers = await this._page.keyboard._ensureModifiers(options.modifiers);
254-
await action(point);
255-
if (restoreModifiers)
256-
await this._page.keyboard._ensureModifiers(restoreModifiers);
256+
257+
await this._page._frameManager.waitForNavigationsCreatedBy(async () => {
258+
let restoreModifiers: input.Modifier[] | undefined;
259+
if (options && options.modifiers)
260+
restoreModifiers = await this._page.keyboard._ensureModifiers(options.modifiers);
261+
await action(point);
262+
if (restoreModifiers)
263+
await this._page.keyboard._ensureModifiers(restoreModifiers);
264+
});
257265
}
258266

259267
hover(options?: PointerActionOptions & types.WaitForOptions): Promise<void> {
@@ -284,18 +292,22 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
284292
if (option.index !== undefined)
285293
assert(helper.isNumber(option.index), 'Indices must be numbers. Found index "' + option.index + '" of type "' + (typeof option.index) + '"');
286294
}
287-
return this._evaluateInUtility((injected, node, ...optionsToSelect) => injected.selectOptions(node, optionsToSelect), ...options);
295+
return await this._page._frameManager.waitForNavigationsCreatedBy<string[]>(async () => {
296+
return this._evaluateInUtility((injected, node, ...optionsToSelect) => injected.selectOptions(node, optionsToSelect), ...options);
297+
});
288298
}
289299

290300
async fill(value: string): Promise<void> {
291301
assert(helper.isString(value), 'Value must be string. Found value "' + value + '" of type "' + (typeof value) + '"');
292-
const error = await this._evaluateInUtility((injected, node, value) => injected.fill(node, value), value);
293-
if (error)
294-
throw new Error(error);
295-
if (value)
296-
await this._page.keyboard.sendCharacters(value);
297-
else
298-
await this._page.keyboard.press('Delete');
302+
await this._page._frameManager.waitForNavigationsCreatedBy(async () => {
303+
const error = await this._evaluateInUtility((injected, node, value) => injected.fill(node, value), value);
304+
if (error)
305+
throw new Error(error);
306+
if (value)
307+
await this._page.keyboard.sendCharacters(value);
308+
else
309+
await this._page.keyboard.press('Delete');
310+
});
299311
}
300312

301313
async setInputFiles(...files: (string | types.FilePayload)[]) {
@@ -317,7 +329,9 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
317329
}
318330
return item;
319331
}));
320-
await this._page._delegate.setInputFiles(this as any as ElementHandle<HTMLInputElement>, filePayloads);
332+
await this._page._frameManager.waitForNavigationsCreatedBy(async () => {
333+
await this._page._delegate.setInputFiles(this as any as ElementHandle<HTMLInputElement>, filePayloads);
334+
});
321335
}
322336

323337
async focus() {
@@ -332,13 +346,17 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
332346
}
333347

334348
async type(text: string, options?: { delay?: number }) {
335-
await this.focus();
336-
await this._page.keyboard.type(text, options);
349+
await this._page._frameManager.waitForNavigationsCreatedBy(async () => {
350+
await this.focus();
351+
await this._page.keyboard.type(text, options);
352+
});
337353
}
338354

339355
async press(key: string, options?: { delay?: number, text?: string }) {
340-
await this.focus();
341-
await this._page.keyboard.press(key, options);
356+
await this._page._frameManager.waitForNavigationsCreatedBy(async () => {
357+
await this.focus();
358+
await this._page.keyboard.press(key, options);
359+
});
342360
}
343361

344362
async check(options?: types.WaitForOptions) {

‎src/frames.ts

+33
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ export class FrameManager {
5959
private _mainFrame: Frame;
6060
readonly _lifecycleWatchers = new Set<() => void>();
6161
readonly _consoleMessageTags = new Map<string, ConsoleTagHandler>();
62+
private _navigationRequestCollectors = new Set<Set<string>>();
6263

6364
constructor(page: Page) {
6465
this._page = page;
@@ -107,7 +108,34 @@ export class FrameManager {
107108
}
108109
}
109110

111+
async waitForNavigationsCreatedBy<T>(action: () => Promise<T>): Promise<T> {
112+
const frameIds = new Set<string>();
113+
this._navigationRequestCollectors.add(frameIds);
114+
try {
115+
const result = await action();
116+
if (!frameIds.size)
117+
return result;
118+
const frames = Array.from(frameIds.values()).map(frameId => this._frames.get(frameId));
119+
await Promise.all(frames.map(frame => frame!.waitForNavigation({ waitUntil: []}))).catch(e => {});
120+
await new Promise(platform.makeWaitForNextTask());
121+
return result;
122+
} finally {
123+
this._navigationRequestCollectors.delete(frameIds);
124+
}
125+
}
126+
127+
frameRequestedNavigation(frameId: string) {
128+
for (const frameIds of this._navigationRequestCollectors)
129+
frameIds.add(frameId);
130+
}
131+
132+
_cancelFrameRequestedNavigation(frameId: string) {
133+
for (const frameIds of this._navigationRequestCollectors)
134+
frameIds.delete(frameId);
135+
}
136+
110137
frameCommittedNewDocumentNavigation(frameId: string, url: string, name: string, documentId: string, initial: boolean) {
138+
this._cancelFrameRequestedNavigation(frameId);
111139
const frame = this._frames.get(frameId)!;
112140
for (const child of frame.childFrames())
113141
this._removeFramesRecursively(child);
@@ -122,6 +150,7 @@ export class FrameManager {
122150
}
123151

124152
frameCommittedSameDocumentNavigation(frameId: string, url: string) {
153+
this._cancelFrameRequestedNavigation(frameId);
125154
const frame = this._frames.get(frameId);
126155
if (!frame)
127156
return;
@@ -206,6 +235,7 @@ export class FrameManager {
206235
if (request._documentId && frame) {
207236
const isCurrentDocument = frame._lastDocumentId === request._documentId;
208237
if (!isCurrentDocument) {
238+
this._cancelFrameRequestedNavigation(frame._id);
209239
let errorText = request.failure()!.errorText;
210240
if (canceled)
211241
errorText += '; maybe frame was detached?';
@@ -218,11 +248,13 @@ export class FrameManager {
218248
}
219249

220250
provisionalLoadFailed(frame: Frame, documentId: string, error: string) {
251+
this._cancelFrameRequestedNavigation(frame._id);
221252
for (const watcher of frame._documentWatchers)
222253
watcher(documentId, new Error(error));
223254
}
224255

225256
private _removeFramesRecursively(frame: Frame) {
257+
this._cancelFrameRequestedNavigation(frame._id);
226258
for (const child of frame.childFrames())
227259
this._removeFramesRecursively(child);
228260
frame._onDetached();
@@ -406,6 +438,7 @@ export class Frame {
406438
disposer.dispose();
407439
if (error)
408440
throw error;
441+
409442
return request ? request._finalRequest._waitForResponse() : null;
410443
}
411444

‎src/webkit/wkPage.ts

+5
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,7 @@ export class WKPage implements PageDelegate {
201201
helper.addEventListener(this._session, 'Page.navigatedWithinDocument', event => this._onFrameNavigatedWithinDocument(event.frameId, event.url)),
202202
helper.addEventListener(this._session, 'Page.frameAttached', event => this._onFrameAttached(event.frameId, event.parentFrameId)),
203203
helper.addEventListener(this._session, 'Page.frameDetached', event => this._onFrameDetached(event.frameId)),
204+
helper.addEventListener(this._session, 'Page.frameScheduledNavigation', event => this._onFrameScheduledNavigation(event.frameId)),
204205
helper.addEventListener(this._session, 'Page.frameStoppedLoading', event => this._onFrameStoppedLoading(event.frameId)),
205206
helper.addEventListener(this._session, 'Page.loadEventFired', event => this._onLifecycleEvent(event.frameId, 'load')),
206207
helper.addEventListener(this._session, 'Page.domContentEventFired', event => this._onLifecycleEvent(event.frameId, 'domcontentloaded')),
@@ -234,6 +235,10 @@ export class WKPage implements PageDelegate {
234235
await Promise.all(sessions.map(session => callback(session).catch(debugError)));
235236
}
236237

238+
private _onFrameScheduledNavigation(frameId: string) {
239+
this._page._frameManager.frameRequestedNavigation(frameId);
240+
}
241+
237242
private _onFrameStoppedLoading(frameId: string) {
238243
this._page._frameManager.frameStoppedLoading(frameId);
239244
}

‎test/navigation.spec.js

+130-6
Original file line numberDiff line numberDiff line change
@@ -800,6 +800,130 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF
800800
});
801801
});
802802

803+
describe.fail(FFOX)('Page.automaticWaiting', () => {
804+
it('clicking anchor should await navigation', async({page, server}) => {
805+
const messages = [];
806+
server.setRoute('/empty.html', async (req, res) => {
807+
messages.push('route');
808+
res.end('done');
809+
});
810+
811+
await page.setContent(`<a href="${server.EMPTY_PAGE}">empty.html</a>`);
812+
813+
await Promise.all([
814+
page.click('a').then(() => messages.push('click')),
815+
page.waitForNavigation({ waitUntil: [] }).then(() => messages.push('waitForNavigation'))
816+
]);
817+
expect(messages.join('|')).toBe('route|waitForNavigation|click');
818+
});
819+
it('clicking anchor should await cross-process navigation', async({page, server}) => {
820+
const messages = [];
821+
server.setRoute('/empty.html', async (req, res) => {
822+
messages.push('route');
823+
res.end('done');
824+
});
825+
826+
await page.setContent(`<a href="${server.CROSS_PROCESS_PREFIX + '/empty.html'}">empty.html</a>`);
827+
828+
await Promise.all([
829+
page.click('a').then(() => messages.push('click')),
830+
page.waitForNavigation({ waitUntil: [] }).then(() => messages.push('waitForNavigation'))
831+
]);
832+
expect(messages.join('|')).toBe('route|waitForNavigation|click');
833+
});
834+
it.fail(CHROMIUM)('should await form-get on click', async({page, server}) => {
835+
const messages = [];
836+
server.setRoute('/empty.html?foo=bar', async (req, res) => {
837+
messages.push('route');
838+
res.end('done');
839+
});
840+
841+
await page.setContent(`
842+
<form action="${server.EMPTY_PAGE}" method="get">
843+
<input name="foo" value="bar">
844+
<input type="submit" value="Submit">
845+
</form>`);
846+
847+
await Promise.all([
848+
page.click('input[type=submit]').then(() => messages.push('click')),
849+
page.waitForNavigation({ waitUntil: [] }).then(() => messages.push('waitForNavigation'))
850+
]);
851+
expect(messages.join('|')).toBe('route|waitForNavigation|click');
852+
});
853+
it.fail(CHROMIUM)('should await form-post on click', async({page, server}) => {
854+
const messages = [];
855+
server.setRoute('/empty.html', async (req, res) => {
856+
messages.push('route');
857+
res.end('done');
858+
});
859+
860+
await page.setContent(`
861+
<form action="${server.EMPTY_PAGE}" method="post">
862+
<input name="foo" value="bar">
863+
<input type="submit" value="Submit">
864+
</form>`);
865+
866+
await Promise.all([
867+
page.click('input[type=submit]').then(() => messages.push('click')),
868+
page.waitForNavigation({ waitUntil: [] }).then(() => messages.push('waitForNavigation'))
869+
]);
870+
expect(messages.join('|')).toBe('route|waitForNavigation|click');
871+
});
872+
it('assigning to location should await navigation', async({page, server}) => {
873+
const messages = [];
874+
server.setRoute('/empty.html', async (req, res) => {
875+
messages.push('route');
876+
res.end('done');
877+
});
878+
await Promise.all([
879+
page.evaluate(`window.location.href = "${server.EMPTY_PAGE}"`).then(() => messages.push('evaluate')),
880+
page.waitForNavigation({ waitUntil: [] }).then(() => messages.push('waitForNavigation')),
881+
]);
882+
expect(messages.join('|')).toBe('route|waitForNavigation|evaluate');
883+
});
884+
it('should await navigating specified target', async({page, server, httpsServer}) => {
885+
const messages = [];
886+
server.setRoute('/empty.html', async (req, res) => {
887+
messages.push('route');
888+
res.end('done');
889+
});
890+
891+
await page.setContent(`
892+
<a href="${server.EMPTY_PAGE}" target=target>empty.html</a>
893+
<iframe name=target></iframe>
894+
`);
895+
const frame = page.frame({ name: 'target' });
896+
await Promise.all([
897+
page.click('a').then(() => messages.push('click')),
898+
frame.waitForNavigation({ waitUntil: [] }).then(() => messages.push('waitForNavigation'))
899+
]);
900+
expect(frame.url()).toBe(server.EMPTY_PAGE);
901+
expect(messages.join('|')).toBe('route|waitForNavigation|click');
902+
});
903+
});
904+
905+
describe('Page.automaticWaiting should not hang', () => {
906+
it('should not throw when clicking on links which do not commit navigation', async({page, server, httpsServer}) => {
907+
await page.goto(server.EMPTY_PAGE);
908+
await page.setContent(`<a href='${httpsServer.EMPTY_PAGE}'>foobar</a>`);
909+
await page.click('a');
910+
});
911+
it('should not throw when clicking on download link', async({page, server, httpsServer}) => {
912+
await page.setContent(`<a href="${server.PREFIX}/wasm/table2.wasm" download=true>table2.wasm</a>`);
913+
await page.click('a');
914+
});
915+
it('should not hang on window.stop', async({page, server, httpsServer}) => {
916+
server.setRoute('/empty.html', async (req, res) => {});
917+
await Promise.all([
918+
page.waitForNavigation().catch(e => {}),
919+
page.evaluate((url) => {
920+
window.location.href = url;
921+
setTimeout(() => window.stop(), 100);
922+
}, server.EMPTY_PAGE)
923+
]);
924+
});
925+
});
926+
803927
describe('Page.waitForLoadState', () => {
804928
it('should pick up ongoing navigation', async({page, server}) => {
805929
let response = null;
@@ -950,13 +1074,13 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF
9501074

9511075
server.setRoute('/empty.html', () => {});
9521076
let error = null;
953-
const navigationPromise = frame.waitForNavigation().catch(e => error = e);
9541077
await Promise.all([
955-
server.waitForRequest('/empty.html'),
956-
frame.evaluate(() => window.location = '/empty.html')
957-
]);
958-
await page.$eval('iframe', frame => frame.remove());
959-
await navigationPromise;
1078+
frame.waitForNavigation().catch(e => error = e),
1079+
server.waitForRequest('/empty.html').then(() => {
1080+
page.$eval('iframe', frame => frame.remove());
1081+
}),
1082+
frame.evaluate(() => window.location = '/empty.html'),
1083+
]).catch(e => error = e);
9601084
expect(error.message).toContain('frame was detached');
9611085
});
9621086
});

0 commit comments

Comments
 (0)