Skip to content

Commit e382bb3

Browse files
dgozmandgozman
authored
api: remove 'commit' phase, actions to wait until 'domcontentloaded' by default (#1358)
1 parent 7c59f9c commit e382bb3

File tree

7 files changed

+156
-151
lines changed

7 files changed

+156
-151
lines changed

‎docs/api.md

+75-110
Large diffs are not rendered by default.

‎src/frames.ts

+3-5
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,10 @@ export class FrameManager {
9898
}
9999
}
100100

101-
async waitForNavigationsCreatedBy<T>(action: () => Promise<T>, options?: types.NavigatingActionWaitOptions, input?: boolean): Promise<T> {
102-
if (options && options.waitUntil === 'nowait')
101+
async waitForNavigationsCreatedBy<T>(action: () => Promise<T>, options: types.NavigatingActionWaitOptions = {}, input?: boolean): Promise<T> {
102+
if (options.waitUntil === 'nowait')
103103
return action();
104-
const barrier = new PendingNavigationBarrier(options);
104+
const barrier = new PendingNavigationBarrier({ waitUntil: 'domcontentloaded', ...options });
105105
this._pendingNavigationBarriers.add(barrier);
106106
try {
107107
const result = await action();
@@ -170,7 +170,6 @@ export class FrameManager {
170170
return;
171171
const hasDOMContentLoaded = frame._firedLifecycleEvents.has('domcontentloaded');
172172
const hasLoad = frame._firedLifecycleEvents.has('load');
173-
frame._firedLifecycleEvents.add('commit');
174173
frame._firedLifecycleEvents.add('domcontentloaded');
175174
frame._firedLifecycleEvents.add('load');
176175
for (const watcher of this._lifecycleWatchers)
@@ -196,7 +195,6 @@ export class FrameManager {
196195

197196
clearFrameLifecycle(frame: Frame) {
198197
frame._firedLifecycleEvents.clear();
199-
frame._firedLifecycleEvents.add('commit');
200198
// Keep the current navigation request if any.
201199
frame._inflightRequests = new Set(Array.from(frame._inflightRequests).filter(request => request._documentId === frame._lastDocumentId));
202200
this._stopNetworkIdleTimer(frame, 'networkidle0');

‎src/page.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -115,12 +115,12 @@ export class PageEvent {
115115
return page;
116116
}
117117

118-
async page(options: types.NavigateOptions = {}): Promise<Page> {
118+
async page(options: types.TimeoutOptions & { waitUntil?: types.LifecycleEvent | 'nowait' } = {}): Promise<Page> {
119119
const {
120120
timeout = this._browserContext._timeoutSettings.navigationTimeout(),
121121
waitUntil = 'load',
122122
} = options;
123-
const lifecyclePromise = this._lifecyclePromises.get(waitUntil);
123+
const lifecyclePromise = waitUntil === 'nowait' ? this._pageOrError : this._lifecyclePromises.get(waitUntil);
124124
if (!lifecyclePromise)
125125
throw new Error(`Unsupported waitUntil option ${String(waitUntil)}`);
126126
const pageOrError = await helper.waitWithTimeout(lifecyclePromise, `"${waitUntil}"`, timeout);

‎src/types.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@ export type WaitForElementOptions = TimeoutOptions & { waitFor?: 'attached' | 'd
4545
export type Polling = 'raf' | 'mutation' | number;
4646
export type WaitForFunctionOptions = TimeoutOptions & { polling?: Polling };
4747

48-
export type LifecycleEvent = 'commit' | 'load' | 'domcontentloaded' | 'networkidle0' | 'networkidle2';
49-
export const kLifecycleEvents: Set<LifecycleEvent> = new Set(['commit', 'load', 'domcontentloaded', 'networkidle0', 'networkidle2']);
48+
export type LifecycleEvent = 'load' | 'domcontentloaded' | 'networkidle0' | 'networkidle2';
49+
export const kLifecycleEvents: Set<LifecycleEvent> = new Set(['load', 'domcontentloaded', 'networkidle0', 'networkidle2']);
5050

5151
export type NavigateOptions = TimeoutOptions & {
5252
waitUntil?: LifecycleEvent,

‎test/browsercontext.spec.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -506,7 +506,7 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FF
506506
});
507507
it('should report initialized pages', async({browser, server}) => {
508508
const context = await browser.newContext();
509-
const pagePromise = context.waitForEvent('page').then(e => e.page({ waitUntil: 'commit' }));
509+
const pagePromise = context.waitForEvent('page').then(e => e.page({ waitUntil: 'nowait' }));
510510
context.newPage();
511511
const newPage = await pagePromise;
512512
expect(newPage.url()).toBe('about:blank');
@@ -555,7 +555,7 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FF
555555
const context = await browser.newContext();
556556
const events = [];
557557
context.on('page', async event => {
558-
const page = await event.page({ waitUntil: 'commit' });
558+
const page = await event.page({ waitUntil: 'nowait' });
559559
events.push('CREATED: ' + page.url());
560560
page.on('close', () => events.push('DESTROYED: ' + page.url()));
561561
});

‎test/navigation.spec.js

+71-29
Original file line numberDiff line numberDiff line change
@@ -792,7 +792,7 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF
792792
});
793793
it('should work for cross-process navigations', async({page, server}) => {
794794
await page.goto(server.EMPTY_PAGE);
795-
const waitPromise = page.waitForNavigation({waitUntil: 'commit'});
795+
const waitPromise = page.waitForNavigation({waitUntil: 'domcontentloaded'});
796796
const url = server.CROSS_PROCESS_PREFIX + '/empty.html';
797797
const gotoPromise = page.goto(url);
798798
const response = await waitPromise;
@@ -808,37 +808,42 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF
808808
const messages = [];
809809
server.setRoute('/empty.html', async (req, res) => {
810810
messages.push('route');
811-
res.end('done');
811+
res.setHeader('Content-Type', 'text/html');
812+
res.end(`<link rel='stylesheet' href='./one-style.css'>`);
812813
});
813814

814815
await page.setContent(`<a href="${server.EMPTY_PAGE}">empty.html</a>`);
815816

816817
await Promise.all([
817818
page.click('a').then(() => messages.push('click')),
818-
page.waitForNavigation({ waitUntil: 'commit' }).then(() => messages.push('waitForNavigation'))
819+
page.waitForNavigation({ waitUntil: 'domcontentloaded' }).then(() => messages.push('domcontentloaded')),
820+
page.waitForNavigation({ waitUntil: 'load' }).then(() => messages.push('load')),
819821
]);
820-
expect(messages.join('|')).toBe('route|waitForNavigation|click');
822+
expect(messages.join('|')).toBe('route|domcontentloaded|click|load');
821823
});
822824
it('clicking anchor should await cross-process navigation', async({page, server}) => {
823825
const messages = [];
824826
server.setRoute('/empty.html', async (req, res) => {
825827
messages.push('route');
826-
res.end('done');
828+
res.setHeader('Content-Type', 'text/html');
829+
res.end(`<link rel='stylesheet' href='./one-style.css'>`);
827830
});
828831

829832
await page.setContent(`<a href="${server.CROSS_PROCESS_PREFIX + '/empty.html'}">empty.html</a>`);
830833

831834
await Promise.all([
832835
page.click('a').then(() => messages.push('click')),
833-
page.waitForNavigation({ waitUntil: 'commit' }).then(() => messages.push('waitForNavigation'))
836+
page.waitForNavigation({ waitUntil: 'domcontentloaded' }).then(() => messages.push('domcontentloaded')),
837+
page.waitForNavigation({ waitUntil: 'load' }).then(() => messages.push('load')),
834838
]);
835-
expect(messages.join('|')).toBe('route|waitForNavigation|click');
839+
expect(messages.join('|')).toBe('route|domcontentloaded|click|load');
836840
});
837841
it('should await form-get on click', async({page, server}) => {
838842
const messages = [];
839843
server.setRoute('/empty.html?foo=bar', async (req, res) => {
840844
messages.push('route');
841-
res.end('done');
845+
res.setHeader('Content-Type', 'text/html');
846+
res.end(`<link rel='stylesheet' href='./one-style.css'>`);
842847
});
843848

844849
await page.setContent(`
@@ -849,15 +854,17 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF
849854

850855
await Promise.all([
851856
page.click('input[type=submit]').then(() => messages.push('click')),
852-
page.waitForNavigation({ waitUntil: 'commit' }).then(() => messages.push('waitForNavigation'))
857+
page.waitForNavigation({ waitUntil: 'domcontentloaded' }).then(() => messages.push('domcontentloaded')),
858+
page.waitForNavigation({ waitUntil: 'load' }).then(() => messages.push('load')),
853859
]);
854-
expect(messages.join('|')).toBe('route|waitForNavigation|click');
860+
expect(messages.join('|')).toBe('route|domcontentloaded|click|load');
855861
});
856862
it('should await form-post on click', async({page, server}) => {
857863
const messages = [];
858864
server.setRoute('/empty.html', async (req, res) => {
859865
messages.push('route');
860-
res.end('done');
866+
res.setHeader('Content-Type', 'text/html');
867+
res.end(`<link rel='stylesheet' href='./one-style.css'>`);
861868
});
862869

863870
await page.setContent(`
@@ -868,23 +875,26 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF
868875

869876
await Promise.all([
870877
page.click('input[type=submit]').then(() => messages.push('click')),
871-
page.waitForNavigation({ waitUntil: 'commit' }).then(() => messages.push('waitForNavigation'))
878+
page.waitForNavigation({ waitUntil: 'domcontentloaded' }).then(() => messages.push('domcontentloaded')),
879+
page.waitForNavigation({ waitUntil: 'load' }).then(() => messages.push('load')),
872880
]);
873-
expect(messages.join('|')).toBe('route|waitForNavigation|click');
881+
expect(messages.join('|')).toBe('route|domcontentloaded|click|load');
874882
});
875883
it('assigning location should await navigation', async({page, server}) => {
876884
const messages = [];
877885
server.setRoute('/empty.html', async (req, res) => {
878886
messages.push('route');
879-
res.end('done');
887+
res.setHeader('Content-Type', 'text/html');
888+
res.end(`<link rel='stylesheet' href='./one-style.css'>`);
880889
});
881890
await Promise.all([
882891
page.evaluate(`window.location.href = "${server.EMPTY_PAGE}"`).then(() => messages.push('evaluate')),
883-
page.waitForNavigation({ waitUntil: 'commit' }).then(() => messages.push('waitForNavigation')),
892+
page.waitForNavigation({ waitUntil: 'domcontentloaded' }).then(() => messages.push('domcontentloaded')),
893+
page.waitForNavigation({ waitUntil: 'load' }).then(() => messages.push('load')),
884894
]);
885-
expect(messages.join('|')).toBe('route|waitForNavigation|evaluate');
895+
expect(messages.join('|')).toBe('route|domcontentloaded|evaluate|load');
886896
});
887-
it.skip(CHROMIUM)('assigning location twice should await navigation', async({page, server}) => {
897+
it.fail(CHROMIUM)('assigning location twice should await navigation', async({page, server}) => {
888898
const messages = [];
889899
server.setRoute('/empty.html?cancel', async (req, res) => { res.end('done'); });
890900
server.setRoute('/empty.html?override', async (req, res) => { messages.push('routeoverride'); res.end('done'); });
@@ -899,17 +909,26 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF
899909
it('evaluating reload should await navigation', async({page, server}) => {
900910
const messages = [];
901911
await page.goto(server.EMPTY_PAGE);
902-
server.setRoute('/empty.html', async (req, res) => { messages.push('route'); res.end('done'); });
912+
server.setRoute('/empty.html', async (req, res) => {
913+
messages.push('route');
914+
res.setHeader('Content-Type', 'text/html');
915+
res.end(`<link rel='stylesheet' href='./one-style.css'>`);
916+
});
903917

904918
await Promise.all([
905919
page.evaluate(`window.location.reload()`).then(() => messages.push('evaluate')),
906-
page.waitForNavigation({ waitUntil: 'commit' }).then(() => messages.push('waitForNavigation')),
920+
page.waitForNavigation({ waitUntil: 'domcontentloaded' }).then(() => messages.push('domcontentloaded')),
921+
page.waitForNavigation({ waitUntil: 'load' }).then(() => messages.push('load')),
907922
]);
908-
expect(messages.join('|')).toBe('route|waitForNavigation|evaluate');
923+
expect(messages.join('|')).toBe('route|domcontentloaded|evaluate|load');
909924
});
910925
it('should await navigating specified target', async({page, server}) => {
911926
const messages = [];
912-
server.setRoute('/empty.html', async (req, res) => { messages.push('route'); res.end('done'); });
927+
server.setRoute('/empty.html', async (req, res) => {
928+
messages.push('route');
929+
res.setHeader('Content-Type', 'text/html');
930+
res.end(`<link rel='stylesheet' href='./one-style.css'>`);
931+
});
913932

914933
await page.setContent(`
915934
<a href="${server.EMPTY_PAGE}" target=target>empty.html</a>
@@ -918,19 +937,42 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF
918937
const frame = page.frame({ name: 'target' });
919938
await Promise.all([
920939
page.click('a').then(() => messages.push('click')),
921-
frame.waitForNavigation({ waitUntil: 'commit' }).then(() => messages.push('waitForNavigation'))
940+
frame.waitForNavigation({ waitUntil: 'domcontentloaded' }).then(() => messages.push('domcontentloaded')),
941+
frame.waitForNavigation({ waitUntil: 'load' }).then(() => messages.push('load')),
922942
]);
923943
expect(frame.url()).toBe(server.EMPTY_PAGE);
924-
expect(messages.join('|')).toBe('route|waitForNavigation|click');
944+
expect(messages.join('|')).toBe('route|domcontentloaded|click|load');
925945
});
926-
it('nowait', async({page, server}) => {
946+
it('waitUntil: nowait', async({page, server}) => {
927947
const messages = [];
948+
server.setRoute('/empty.html', async (req, res) => {
949+
res.setHeader('Content-Type', 'text/html');
950+
res.end(`<link rel='stylesheet' href='./one-style.css'>`);
951+
});
952+
928953
await page.setContent(`<a href="${server.EMPTY_PAGE}">empty.html</a>`);
929954
await Promise.all([
930955
page.click('a', { waitUntil: 'nowait' }).then(() => messages.push('click')),
931-
page.waitForNavigation({ waitUntil: 'commit' }).then(() => messages.push('waitForNavigation'))
956+
page.waitForNavigation({ waitUntil: 'domcontentloaded' }).then(() => messages.push('domcontentloaded')),
957+
page.waitForNavigation({ waitUntil: 'load' }).then(() => messages.push('load')),
958+
]);
959+
expect(messages.join('|')).toBe('click|domcontentloaded|load');
960+
});
961+
it('waitUntil: load', async({page, server}) => {
962+
const messages = [];
963+
server.setRoute('/empty.html', async (req, res) => {
964+
messages.push('route');
965+
res.setHeader('Content-Type', 'text/html');
966+
res.end(`<link rel='stylesheet' href='./one-style.css'>`);
967+
});
968+
969+
await page.setContent(`<a href="${server.EMPTY_PAGE}">empty.html</a>`);
970+
await Promise.all([
971+
page.click('a', { waitUntil: 'load' }).then(() => messages.push('click')),
972+
page.waitForNavigation({ waitUntil: 'domcontentloaded' }).then(() => messages.push('domcontentloaded')),
973+
page.waitForNavigation({ waitUntil: 'load' }).then(() => messages.push('load')),
932974
]);
933-
expect(messages.join('|')).toBe('click|waitForNavigation');
975+
expect(messages.join('|')).toBe('route|domcontentloaded|load|click');
934976
});
935977
});
936978

@@ -975,7 +1017,7 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF
9751017
server.setRoute('/one-style.css', (req, res) => response = res);
9761018
await Promise.all([
9771019
server.waitForRequest('/one-style.css'),
978-
page.goto(server.PREFIX + '/one-style.html', {waitUntil: 'commit'}),
1020+
page.goto(server.PREFIX + '/one-style.html', {waitUntil: 'domcontentloaded'}),
9791021
]);
9801022
const waitPromise = page.mainFrame()._waitForLoadState();
9811023
response.statusCode = 404;
@@ -984,7 +1026,7 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF
9841026
});
9851027
it('should respect timeout', async({page, server}) => {
9861028
server.setRoute('/one-style.css', (req, res) => response = res);
987-
await page.goto(server.PREFIX + '/one-style.html', {waitUntil: 'commit'});
1029+
await page.goto(server.PREFIX + '/one-style.html', {waitUntil: 'domcontentloaded'});
9881030
const error = await page.mainFrame()._waitForLoadState({ timeout: 1 }).catch(e => e);
9891031
expect(error.message).toBe('Navigation timeout of 1 ms exceeded');
9901032
});
@@ -995,7 +1037,7 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF
9951037
it('should resolve immediately if load state matches', async({page, server}) => {
9961038
await page.goto(server.EMPTY_PAGE);
9971039
server.setRoute('/one-style.css', (req, res) => response = res);
998-
await page.goto(server.PREFIX + '/one-style.html', {waitUntil: 'commit'});
1040+
await page.goto(server.PREFIX + '/one-style.html', {waitUntil: 'domcontentloaded'});
9991041
await page.mainFrame()._waitForLoadState({ waitUntil: 'domcontentloaded' });
10001042
});
10011043
it('should work with pages that have loaded before being connected to', async({page, context, server}) => {

‎test/popup.spec.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, WE
283283
const context = await browser.newContext();
284284
const page = await context.newPage();
285285
const [popup] = await Promise.all([
286-
page.waitForEvent('popup').then(e => e.page({ waitUntil: 'commit' })),
286+
page.waitForEvent('popup').then(e => e.page({ waitUntil: 'nowait' })),
287287
page.evaluate(() => window.__popup = window.open('')),
288288
]);
289289
expect(await page.evaluate(() => !!window.opener)).toBe(false);

0 commit comments

Comments
 (0)