From 4dfdca1b990604ec2a76f86d3111a3c0e00b95e6 Mon Sep 17 00:00:00 2001
From: Pavel Feldman <pavel.feldman@gmail.com>
Date: Fri, 7 Jan 2022 13:04:19 -0800
Subject: [PATCH] =?UTF-8?q?cherry-pick(#11228):=20fix(click):=20don't=20fa?=
 =?UTF-8?q?il=20on=20stale=20context=20while=20cl=E2=80=A6=20(#11231)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 packages/playwright-core/src/server/dom.ts    | 13 ++++++--
 packages/playwright-core/src/server/frames.ts | 16 +++++++---
 .../page/page-click-during-navigation.spec.ts | 30 +++++++++++++++++++
 3 files changed, 52 insertions(+), 7 deletions(-)
 create mode 100644 tests/page/page-click-during-navigation.spec.ts

diff --git a/packages/playwright-core/src/server/dom.ts b/packages/playwright-core/src/server/dom.ts
index 9a261d50a1..95cbe4ac05 100644
--- a/packages/playwright-core/src/server/dom.ts
+++ b/packages/playwright-core/src/server/dom.ts
@@ -29,6 +29,13 @@ import * as types from './types';
 
 type SetInputFilesFiles = channels.ElementHandleSetInputFilesParams['files'];
 
+export class NonRecoverableDOMError extends Error {
+}
+
+export function isNonRecoverableDOMError(error: Error) {
+  return error instanceof NonRecoverableDOMError;
+}
+
 export class FrameExecutionContext extends js.ExecutionContext {
   readonly frame: frames.Frame;
   private _injectedScriptPromise?: Promise<js.JSHandle>;
@@ -356,13 +363,13 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
       ++retry;
       if (result === 'error:notvisible') {
         if (options.force)
-          throw new Error('Element is not visible');
+          throw new NonRecoverableDOMError('Element is not visible');
         progress.log('  element is not visible');
         continue;
       }
       if (result === 'error:notinviewport') {
         if (options.force)
-          throw new Error('Element is outside of the viewport');
+          throw new NonRecoverableDOMError('Element is outside of the viewport');
         progress.log('  element is outside of the viewport');
         continue;
       }
@@ -740,7 +747,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
     if (options.trial)
       return 'done';
     if (await isChecked() !== state)
-      throw new Error('Clicking the checkbox did not change its state');
+      throw new NonRecoverableDOMError('Clicking the checkbox did not change its state');
     return 'done';
   }
 
diff --git a/packages/playwright-core/src/server/frames.ts b/packages/playwright-core/src/server/frames.ts
index 84709484aa..8c3f368be5 100644
--- a/packages/playwright-core/src/server/frames.ts
+++ b/packages/playwright-core/src/server/frames.ts
@@ -997,10 +997,18 @@ export class Frame extends SdkObject {
         // Always fail on JavaScript errors or when the main connection is closed.
         if (js.isJavaScriptErrorInEvaluate(e) || isSessionClosedError(e))
           throw e;
-        // If error has happened in the detached inner frame, ignore it, keep polling.
-        if (selectorInFrame?.frame !== this && selectorInFrame?.frame.isDetached())
-          continue;
-        throw e;
+        // Certain error opt-out of the retries, throw.
+        if (dom.isNonRecoverableDOMError(e))
+          throw e;
+        // If the call is made on the detached frame - throw.
+        if (this.isDetached())
+          throw e;
+        // If there is scope, and scope is within the frame we use to select, assume context is destroyed and
+        // operation is not recoverable.
+        if (scope && scope._context.frame === selectorInFrame?.frame)
+          throw e;
+        // Retry upon all other errors.
+        continue;
       }
     }
     progress.throwIfAborted();
diff --git a/tests/page/page-click-during-navigation.spec.ts b/tests/page/page-click-during-navigation.spec.ts
new file mode 100644
index 0000000000..f3550fbb49
--- /dev/null
+++ b/tests/page/page-click-during-navigation.spec.ts
@@ -0,0 +1,30 @@
+/**
+ * Copyright 2018 Google Inc. All rights reserved.
+ * Modifications copyright (c) Microsoft Corporation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { test as it } from './pageTest';
+
+it('should not fail with internal error upon navigation', async ({ page, server }) => {
+  await page.goto(server.PREFIX + '/input/button.html');
+  let triggered = false;
+  const __testHookAfterStable = () => {
+    if (triggered)
+      return;
+    triggered = true;
+    return page.goto(server.PREFIX + '/input/button.html');
+  };
+  await page.click('button', { __testHookAfterStable } as any);
+});