fix(oopifs): account for various races between processes

src/server/chromium/crPage.ts

@@ -532,14 +532,25 @@ class FrameSession {
     if (frameSession && frameId !== this._targetId) {
       // This is a remote -> local frame transition.
       frameSession._swappedIn = true;
-      const frame = this._page._frameManager.frame(frameId)!;
-      this._page._frameManager.removeChildFramesRecursively(frame);
+      const frame = this._page._frameManager.frame(frameId);
+      // Frame or even a whole subtree may be already gone, because some ancestor did navigate.
+      if (frame)
+        this._page._frameManager.removeChildFramesRecursively(frame);
+      return;
+    }
+    if (parentFrameId && !this._page._frameManager.frame(parentFrameId)) {
+      // Parent frame may be gone already because some ancestor frame navigated and
+      // destroyed the whole subtree of some oopif, while oopif's process is still sending us events.
+      // Be careful to not confuse this with "main frame navigated cross-process" scenario
+      // where parentFrameId is null.
       return;
     }
     this._page._frameManager.frameAttached(frameId, parentFrameId);
   }
 
   _onFrameNavigated(framePayload: Protocol.Page.Frame, initial: boolean) {
+    if (!this._page._frameManager.frame(framePayload.id))
+      return; // Subtree may be already gone because some ancestor navigation destroyed the oopif.
     this._page._frameManager.frameCommittedNewDocumentNavigation(framePayload.id, framePayload.url + (framePayload.urlFragment || ''), framePayload.name || '', framePayload.loaderId, initial);
     if (!initial)
       this._firstNonInitialNavigationCommittedFulfill();
@@ -609,7 +620,9 @@ class FrameSession {
     if (event.targetInfo.type === 'iframe') {
       // Frame id equals target id.
       const targetId = event.targetInfo.targetId;
-      const frame = this._page._frameManager.frame(targetId)!;
+      const frame = this._page._frameManager.frame(targetId);
+      if (!frame)
+        return; // Subtree may be already gone due to renderer/browser race.
       this._page._frameManager.removeChildFramesRecursively(frame);
       const frameSession = new FrameSession(this._crPage, session, targetId, this);
       this._crPage._sessions.set(targetId, frameSession);
@@ -715,6 +728,8 @@ class FrameSession {
   }
 
   _onDialog(event: Protocol.Page.javascriptDialogOpeningPayload) {
+    if (!this._page._frameManager.frame(this._targetId))
+      return; // Our frame/subtree may be gone already.
     this._page.emit(Page.Events.Dialog, new dialog.Dialog(
         this._page,
         event.type,
@@ -749,10 +764,18 @@ class FrameSession {
   }
 
   async _onFileChooserOpened(event: Protocol.Page.fileChooserOpenedPayload) {
-    const frame = this._page._frameManager.frame(event.frameId)!;
-    const utilityContext = await frame._utilityContext();
-    const handle = await this._adoptBackendNodeId(event.backendNodeId, utilityContext);
-    this._page._onFileChooserOpened(handle);
+    const frame = this._page._frameManager.frame(event.frameId);
+    if (!frame)
+      return;
+    let handle;
+    try {
+      const utilityContext = await frame._utilityContext();
+      handle = await this._adoptBackendNodeId(event.backendNodeId, utilityContext);
+    } catch (e) {
+      // During async processing, frame/context may go away. We should not throw.
+      return;
+    }
+    await this._page._onFileChooserOpened(handle);
   }
 
   _onDownloadWillBegin(payload: Protocol.Page.downloadWillBeginPayload) {

src/server/page.ts

@@ -226,7 +226,13 @@ export class Page extends EventEmitter {
   }
 
   async _onFileChooserOpened(handle: dom.ElementHandle) {
-    const multiple = await handle.evaluate(element => !!(element as HTMLInputElement).multiple);
+    let multiple;
+    try {
+      multiple = await handle.evaluate(element => !!(element as HTMLInputElement).multiple);
+    } catch (e) {
+      // Frame/context may be gone during async processing. Do not throw.
+      return;
+    }
     if (!this.listenerCount(Page.Events.FileChooser)) {
       handle.dispose();
       return;

src/server/webkit/wkPage.ts

@@ -559,9 +559,15 @@ export class WKPage implements PageDelegate {
   }
 
   private async _onFileChooserOpened(event: {frameId: Protocol.Network.FrameId, element: Protocol.Runtime.RemoteObject}) {
-    const context = await this._page._frameManager.frame(event.frameId)!._mainContext();
-    const handle = context.createHandle(event.element).asElement()!;
-    this._page._onFileChooserOpened(handle);
+    let handle;
+    try {
+      const context = await this._page._frameManager.frame(event.frameId)!._mainContext();
+      handle = context.createHandle(event.element).asElement()!;
+    } catch (e) {
+      // During async processing, frame/context may go away. We should not throw.
+      return;
+    }
+    await this._page._onFileChooserOpened(handle);
   }
 
   private static async _setEmulateMedia(session: WKSession, mediaType: types.MediaType | null, colorScheme: types.ColorScheme | null): Promise<void> {

test/page-set-input-files.spec.ts

@@ -115,6 +115,50 @@ it('should work when file input is not attached to DOM', async ({page, server})
   expect(chooser).toBeTruthy();
 });
 
+it('should not throw when filechooser belongs to iframe', (test, { browserName }) => {
+  test.skip(browserName === 'firefox', 'Firefox ignores filechooser from child frame');
+}, async ({page, server}) => {
+  await page.goto(server.PREFIX + '/frames/one-frame.html');
+  const frame = page.mainFrame().childFrames()[0];
+  await frame.setContent(`
+    <div>Click me</div>
+    <script>
+      document.querySelector('div').addEventListener('click', () => {
+        const input = document.createElement('input');
+        input.type = 'file';
+        input.click();
+        window.parent.__done = true;
+      });
+    </script>
+  `);
+  await Promise.all([
+    page.waitForEvent('filechooser'),
+    frame.click('div')
+  ]);
+  await page.waitForFunction(() => (window as any).__done);
+});
+
+it('should not throw when frame is detached immediately', async ({page, server}) => {
+  await page.goto(server.PREFIX + '/frames/one-frame.html');
+  const frame = page.mainFrame().childFrames()[0];
+  await frame.setContent(`
+    <div>Click me</div>
+    <script>
+      document.querySelector('div').addEventListener('click', () => {
+        const input = document.createElement('input');
+        input.type = 'file';
+        input.click();
+        window.parent.__done = true;
+        const iframe = window.parent.document.querySelector('iframe');
+        iframe.remove();
+      });
+    </script>
+  `);
+  page.on('filechooser', () => {});  // To ensure we handle file choosers.
+  await frame.click('div');
+  await page.waitForFunction(() => (window as any).__done);
+});
+
 it('should work with CSP', async ({page, server}) => {
   server.setCSP('/empty.html', 'default-src "none"');
   await page.goto(server.EMPTY_PAGE);