fix(waitForSelector): retry when context is gone during node adoption (#6851)

There is a small window after finishing the "rerunnable task" where
we adopt the node to the main world and navigation could destroy the context.

src/server/chromium/crExecutionContext.ts

@@ -109,7 +109,7 @@ function rewriteError(error: Error): Protocol.Runtime.evaluateReturnValue {
   if (error.message.includes('Object couldn\'t be returned by value'))
     return {result: {type: 'undefined'}};
 
-  if (error.message.endsWith('Cannot find context with specified id') || error.message.endsWith('Inspected target navigated or closed') || error.message.endsWith('Execution context was destroyed.'))
+  if (js.isContextDestroyedError(error) || error.message.endsWith('Inspected target navigated or closed'))
     throw new Error('Execution context was destroyed, most likely because of a navigation.');
   if (error instanceof TypeError && error.message.startsWith('Converting circular structure to JSON'))
     rewriteErrorMessage(error, error.message + ' Are you passing a nested JSHandle?');

src/server/chromium/crPage.ts

@@ -1130,7 +1130,7 @@ class FrameSession {
       executionContextId: (to._delegate as CRExecutionContext)._contextId,
     });
     if (!result || result.object.subtype === 'null')
-      throw new Error('Unable to adopt element handle from a different document');
+      throw new Error(dom.kUnableToAdoptErrorMessage);
     return to.createHandle(result.object).asElement()!;
   }
 }

src/server/dom.ts

@@ -1023,3 +1023,5 @@ export function elementStateTask(selector: SelectorInfo, state: ElementStateWith
     });
   }, { parsed: selector.parsed, state });
 }
+
+export const kUnableToAdoptErrorMessage = 'Unable to adopt element handle from a different document';

src/server/firefox/ffExecutionContext.ts

@@ -111,7 +111,7 @@ function checkException(exceptionDetails?: Protocol.Runtime.ExceptionDetails) {
 function rewriteError(error: Error): (Protocol.Runtime.evaluateReturnValue | Protocol.Runtime.callFunctionReturnValue) {
   if (error.message.includes('cyclic object value') || error.message.includes('Object is not serializable'))
     return {result: {type: 'undefined', value: undefined}};
-  if (error.message.includes('Failed to find execution context with id') || error.message.includes('Execution context was destroyed!'))
+  if (js.isContextDestroyedError(error))
     throw new Error('Execution context was destroyed, most likely because of a navigation.');
   if (error instanceof TypeError && error.message.startsWith('Converting circular structure to JSON'))
     rewriteErrorMessage(error, error.message + ' Are you passing a nested JSHandle?');

src/server/firefox/ffPage.ts

@@ -528,7 +528,7 @@ export class FFPage implements PageDelegate {
       executionContextId: (to._delegate as FFExecutionContext)._executionContextId
     });
     if (!result.remoteObject)
-      throw new Error('Unable to adopt element handle from a different document');
+      throw new Error(dom.kUnableToAdoptErrorMessage);
     return to.createHandle(result.remoteObject) as dom.ElementHandle<T>;
   }
 

src/server/frames.ts

@@ -677,13 +677,26 @@ export class Frame extends SdkObject {
     const task = dom.waitForSelectorTask(info, state);
     return controller.run(async progress => {
       progress.log(`waiting for selector "${selector}"${state === 'attached' ? '' : ' to be ' + state}`);
+      while (progress.isRunning()) {
         const result = await this._scheduleRerunnableHandleTask(progress, info.world, task);
         if (!result.asElement()) {
           result.dispose();
           return null;
         }
+        if ((options as any).__testHookBeforeAdoptNode)
+          await (options as any).__testHookBeforeAdoptNode();
+        try {
           const handle = result.asElement() as dom.ElementHandle<Element>;
-      return handle._adoptTo(await this._mainContext());
+          const adopted = await handle._adoptTo(await this._mainContext());
+          return adopted;
+        } catch (e) {
+          // Navigated while trying to adopt the node.
+          if (!js.isContextDestroyedError(e) && !e.message.includes(dom.kUnableToAdoptErrorMessage))
+            throw e;
+          result.dispose();
+        }
+      }
+      return null;
     }, this._page._timeoutSettings.timeout(options));
   }
 
@@ -1263,16 +1276,9 @@ class RerunnableTask {
       this._contextData.rerunnableTasks.delete(this);
       this._resolve(result);
     } catch (e) {
-      // When the page is navigated, the promise is rejected.
       // We will try again in the new execution context.
-      if (e.message.includes('Execution context was destroyed'))
+      if (js.isContextDestroyedError(e))
         return;
-
-      // We could have tried to evaluate in a context which was already
-      // destroyed.
-      if (e.message.includes('Cannot find context with specified id'))
-        return;
-
       this._contextData.rerunnableTasks.delete(this);
       this._reject(e);
     }

src/server/javascript.ts

@@ -277,3 +277,27 @@ export function normalizeEvaluationExpression(expression: string, isFunction: bo
     expression = '(' + expression + ')';
   return expression;
 }
+
+export const kSwappedOutErrorMessage = 'Target was swapped out.';
+
+export function isContextDestroyedError(e: any) {
+  if (!e || typeof e !== 'object' || typeof e.message !== 'string')
+    return false;
+
+  // Evaluating in a context which was already destroyed.
+  if (e.message.includes('Cannot find context with specified id')
+      || e.message.includes('Failed to find execution context with id')
+      || e.message.includes('Missing injected script for given')
+      || e.message.includes('Cannot find object with id'))
+    return true;
+
+  // Evaluation promise is rejected when context is gone.
+  if (e.message.includes('Execution context was destroyed'))
+    return true;
+
+  // WebKit target swap.
+  if (e.message.includes(kSwappedOutErrorMessage))
+    return true;
+
+  return false;
+}

src/server/webkit/wkConnection.ts

@@ -185,7 +185,3 @@ export function createProtocolError(error: Error, method: string, protocolError:
     message += ` ${JSON.stringify(protocolError.data)}`;
   return rewriteErrorMessage(error, message);
 }
-
-export function isSwappedOutError(e: Error) {
-  return e.message.includes('Target was swapped out.');
-}

src/server/webkit/wkExecutionContext.ts

@@ -15,7 +15,7 @@
  * limitations under the License.
  */
 
-import { WKSession, isSwappedOutError } from './wkConnection';
+import { WKSession } from './wkConnection';
 import { Protocol } from './protocol';
 import * as js from '../javascript';
 import { parseEvaluationResultValue } from '../common/utilityScriptSerializers';
@@ -143,7 +143,7 @@ function potentiallyUnserializableValue(remoteObject: Protocol.Runtime.RemoteObj
 }
 
 function rewriteError(error: Error): Error {
-  if (isSwappedOutError(error) || error.message.includes('Missing injected script for given'))
+  if (js.isContextDestroyedError(error))
     return new Error('Execution context was destroyed, most likely because of a navigation.');
   return error;
 }

src/server/webkit/wkPage.ts

@@ -26,7 +26,7 @@ import * as dialog from '../dialog';
 import * as dom from '../dom';
 import * as frames from '../frames';
 import { helper, RegisteredListener } from '../helper';
-import { JSHandle } from '../javascript';
+import { JSHandle, kSwappedOutErrorMessage } from '../javascript';
 import * as network from '../network';
 import { Page, PageBinding, PageDelegate } from '../page';
 import { Progress } from '../progress';
@@ -213,7 +213,7 @@ export class WKPage implements PageDelegate {
     assert(this._provisionalPage);
     assert(this._provisionalPage._session.sessionId === newTargetId, 'Unknown new target: ' + newTargetId);
     assert(this._session.sessionId === oldTargetId, 'Unknown old target: ' + oldTargetId);
-    this._session.errorText = 'Target was swapped out.';
+    this._session.errorText = kSwappedOutErrorMessage;
     const newSession = this._provisionalPage._session;
     this._provisionalPage.commit();
     this._provisionalPage.dispose();
@@ -896,7 +896,7 @@ export class WKPage implements PageDelegate {
       executionContextId: (to._delegate as WKExecutionContext)._contextId
     });
     if (!result || result.object.subtype === 'null')
-      throw new Error('Unable to adopt element handle from a different document');
+      throw new Error(dom.kUnableToAdoptErrorMessage);
     return to.createHandle(result.object) as dom.ElementHandle<T>;
   }
 

tests/page/page-wait-for-selector-2.spec.ts

@@ -268,3 +268,23 @@ it('should correctly handle hidden shadow host', async ({page, server}) => {
   expect(await page.textContent('div')).toBe('Find me');
   await page.waitForSelector('div', { state: 'hidden' });
 });
+
+it('should work when navigating before node adoption', async ({page, mode, server}) => {
+  it.skip(mode !== 'default');
+
+  await page.goto(server.EMPTY_PAGE);
+  await page.setContent(`<div>Hello</div>`);
+
+  let navigatedOnce = false;
+  const __testHookBeforeAdoptNode = async () => {
+    if (!navigatedOnce) {
+      navigatedOnce = true;
+      await page.goto(server.PREFIX + '/one-style.html');
+    }
+  };
+
+  const div = await page.waitForSelector('div', { __testHookBeforeAdoptNode } as any);
+
+  // This text is coming from /one-style.html
+  expect(await div.textContent()).toBe('hello, world!');
+});