fix(reload): make sure reload() does not pick same-document navigaiton (#16504)

We lack `documentId` when doing a reload over browser protocols, so
`reload()` waits for the next navigation to finish. Sometimes, the page
might issue a same-document navigation while reload is in progress,
which confuses the reload command.

To fix the issue, just ignore same-document navigations for reload,
because it is always a new document.
This commit is contained in:
Dmitry Gozman 2022-08-12 13:48:47 -07:00 committed by GitHub
parent 0ff00e5978
commit 3ae50861a3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 39 additions and 4 deletions

View File

@ -715,7 +715,7 @@ export class Frame extends SdkObject {
return response;
}
async _waitForNavigation(progress: Progress, options: types.NavigateOptions): Promise<network.Response | null> {
async _waitForNavigation(progress: Progress, requiresNewDocument: boolean, options: types.NavigateOptions): Promise<network.Response | null> {
const waitUntil = verifyLifecycle('waitUntil', options.waitUntil === undefined ? 'load' : options.waitUntil);
progress.log(`waiting for navigation until "${waitUntil}"`);
@ -723,6 +723,8 @@ export class Frame extends SdkObject {
// Any failed navigation results in a rejection.
if (event.error)
return true;
if (requiresNewDocument && !event.newDocument)
return false;
progress.log(` navigated to "${this._url}"`);
return true;
}).promise;

View File

@ -372,7 +372,8 @@ export class Page extends SdkObject {
// Note: waitForNavigation may fail before we get response to reload(),
// so we should await it immediately.
const [response] = await Promise.all([
this.mainFrame()._waitForNavigation(progress, options),
// Reload must be a new document, and should not be confused with a stray pushState.
this.mainFrame()._waitForNavigation(progress, true /* requiresNewDocument */, options),
this._delegate.reload(),
]);
await this._doSlowMo();
@ -386,7 +387,7 @@ export class Page extends SdkObject {
// Note: waitForNavigation may fail before we get response to goBack,
// so we should catch it immediately.
let error: Error | undefined;
const waitPromise = this.mainFrame()._waitForNavigation(progress, options).catch(e => {
const waitPromise = this.mainFrame()._waitForNavigation(progress, false /* requiresNewDocument */, options).catch(e => {
error = e;
return null;
});
@ -407,7 +408,7 @@ export class Page extends SdkObject {
// Note: waitForNavigation may fail before we get response to goForward,
// so we should catch it immediately.
let error: Error | undefined;
const waitPromise = this.mainFrame()._waitForNavigation(progress, options).catch(e => {
const waitPromise = this.mainFrame()._waitForNavigation(progress, false /* requiresNewDocument */, options).catch(e => {
error = e;
return null;
});

View File

@ -114,6 +114,38 @@ it('page.reload during renderer-initiated navigation', async ({ page, server })
await page.waitForSelector('text=hello');
});
it('page.reload should not resolve with same-document navigation', async ({ page, server }) => {
await page.goto(server.EMPTY_PAGE);
// 1. Make sure execution contexts are ready for fast evaluate.
await page.evaluate('1');
// 2. Stall the reload request.
let response;
server.setRoute('/empty.html', (req, res) => { response = res; });
const requestPromise = server.waitForRequest('/empty.html');
// 3. Trigger push state that could resolve the reload.
page.evaluate(() => {
window.history.pushState({}, '');
}).catch(() => {});
// 4. Trigger the reload, it should not resolve.
const reloadPromise = page.reload();
// 5. Trigger push state again, for the good measure :)
page.evaluate(() => {
window.history.pushState({}, '');
}).catch(() => {});
// 5. Serve the request, it should resolve the reload.
await requestPromise;
response.end('hello');
// 6. Check the reload response.
const gotResponse = await reloadPromise;
expect(await gotResponse.text()).toBe('hello');
});
it('page.goBack during renderer-initiated navigation', async ({ page, server }) => {
await page.goto(server.PREFIX + '/one-style.html');
await page.goto(server.EMPTY_PAGE);