From 3e1c72ac5f608d02271aa9e8f757796196adc880 Mon Sep 17 00:00:00 2001
From: Dmitry Gozman <dgozman@gmail.com>
Date: Fri, 22 Jan 2021 15:58:53 -0800
Subject: [PATCH] fix(reload): do not throw when reload is racing with
 navigation (#5113)

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()`.
---
 src/server/page.ts        | 48 ++++++++++++++++++++---------
 test/page-history.spec.ts | 64 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 97 insertions(+), 15 deletions(-)

diff --git a/src/server/page.ts b/src/server/page.ts
index 3b2434d045..5296e35d62 100644
--- a/src/server/page.ts
+++ b/src/server/page.ts
@@ -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;
diff --git a/test/page-history.spec.ts b/test/page-history.spec.ts
index 38058fad0a..b10a9419e1 100644
--- a/test/page-history.spec.ts
+++ b/test/page-history.spec.ts
@@ -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');
+});