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

fix(reload): do not throw when reload is racing with navigation (#5113)

Browse source 
When `page.reload()` is racing against the renderer-initiated
navigation, we might end up with `waitForNavigation()` being rejected
before the reload implementation is able to catch it.

To avoid that, carefully use Promise.all and await `waitForNavigation`
from the get go.

Same happens to `page.goForward()` and `page.goBack()`.
This commit is contained in:
Dmitry Gozman 2021-01-22 15:58:53 -08:00 committed by GitHub
parent b88afe5877
commit 3e1c72ac5f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 97 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

48
src/server/page.ts
View file

@ -293,9 +293,13 @@ export class Page extends EventEmitter {
async reload(controller: ProgressController, options: types.NavigateOptions): Promise<network.Response | null> {
this.mainFrame().setupNavigationProgressController(controller);
const response = await controller.run(async progress => {
const waitPromise = this.mainFrame()._waitForNavigation(progress, options);
await this._delegate.reload();
return waitPromise;
// 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),
this._delegate.reload(),
]);
return response;
}, this._timeoutSettings.navigationTimeout(options));
await this._doSlowMo();
return response;
@ -304,13 +308,20 @@ export class Page extends EventEmitter {
async goBack(controller: ProgressController, options: types.NavigateOptions): Promise<network.Response | null> {
this.mainFrame().setupNavigationProgressController(controller);
const response = await controller.run(async progress => {
const waitPromise = this.mainFrame()._waitForNavigation(progress, options);
const result = await this._delegate.goBack();
if (!result) {
waitPromise.catch(() => {});
// 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 => {
error = e;
return null;
}
return waitPromise;
});
const result = await this._delegate.goBack();
if (!result)
return null;
const response = await waitPromise;
if (error)
throw error;
return response;
}, this._timeoutSettings.navigationTimeout(options));
await this._doSlowMo();
return response;
@ -319,13 +330,20 @@ export class Page extends EventEmitter {
async goForward(controller: ProgressController, options: types.NavigateOptions): Promise<network.Response | null> {
this.mainFrame().setupNavigationProgressController(controller);
const response = await controller.run(async progress => {
const waitPromise = this.mainFrame()._waitForNavigation(progress, options);
const result = await this._delegate.goForward();
if (!result) {
waitPromise.catch(() => {});
// 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 => {
error = e;
return null;
}
return waitPromise;
});
const result = await this._delegate.goForward();
if (!result)
return null;
const response = await waitPromise;
if (error)
throw error;
return response;
}, this._timeoutSettings.navigationTimeout(options));
await this._doSlowMo();
return response;

64
test/page-history.spec.ts
View file

@ -92,3 +92,67 @@ it('page.reload should work with data url', async ({page, server}) => {
expect(await page.reload()).toBe(null);
expect(await page.content()).toContain('hello');
});
it('page.reload during renderer-initiated navigation', async ({page, server}) => {
await page.goto(server.PREFIX + '/one-style.html');
await page.setContent(`<form method='POST' action='/post'>Form is here<input type='submit'></form>`);
server.setRoute('/post', (req, res) => {});
let callback;
const reloadFailedPromise = new Promise(f => callback = f);
page.once('request', async () => {
await page.reload().catch(e => {});
callback();
});
const clickPromise = page.click('input[type=submit]').catch(e => {});
await reloadFailedPromise;
await clickPromise;
// Form submit should be canceled, and reload should eventually arrive
// to the original one-style.html.
await page.waitForSelector('text=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);
await page.setContent(`<form method='POST' action='/post'>Form is here<input type='submit'></form>`);
server.setRoute('/post', (req, res) => {});
let callback;
const reloadFailedPromise = new Promise(f => callback = f);
page.once('request', async () => {
await page.goBack().catch(e => {});
callback();
});
const clickPromise = page.click('input[type=submit]').catch(e => {});
await reloadFailedPromise;
await clickPromise;
// Form submit should be canceled, and goBack should eventually arrive
// to the original one-style.html.
await page.waitForSelector('text=hello');
});
it('page.goForward during renderer-initiated navigation', async ({page, server}) => {
await page.goto(server.EMPTY_PAGE);
await page.goto(server.PREFIX + '/one-style.html');
await page.goBack();
await page.setContent(`<form method='POST' action='/post'>Form is here<input type='submit'></form>`);
server.setRoute('/post', (req, res) => {});
let callback;
const reloadFailedPromise = new Promise(f => callback = f);
page.once('request', async () => {
await page.goForward().catch(e => {});
callback();
});
const clickPromise = page.click('input[type=submit]').catch(e => {});
await reloadFailedPromise;
await clickPromise;
// Form submit should be canceled, and goForward should eventually arrive
// to the original one-style.html.
await page.waitForSelector('text=hello');
});