From 244c2f37b66e2e707536a34937a483fbc30cb84b Mon Sep 17 00:00:00 2001
From: Dmitry Gozman <dgozman@gmail.com>
Date: Fri, 14 Aug 2020 18:24:36 -0700
Subject: [PATCH] feat(rpc): make sure filechooser is only intercepted when
 needed (#3482)

So that user can choose a file manually in headful mode.
---
 src/rpc/client/page.ts            | 16 ++++++++++++++++
 src/rpc/server/pageDispatcher.ts  | 10 ++++++++--
 test/page-set-input-files.spec.ts | 32 ++++++++++++++++++++++++++++++-
 3 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/src/rpc/client/page.ts b/src/rpc/client/page.ts
index 646026fe4e..6b032e2be1 100644
--- a/src/rpc/client/page.ts
+++ b/src/rpc/client/page.ts
@@ -536,6 +536,22 @@ export class Page extends ChannelOwner<PageChannel, PageInitializer> {
     return this;
   }
 
+  addListener(event: string | symbol, listener: Listener): this {
+    if (event === Events.Page.FileChooser) {
+      if (!this.listenerCount(event))
+        this._channel.setFileChooserInterceptedNoReply({ intercepted: true });
+    }
+    super.addListener(event, listener);
+    return this;
+  }
+
+  off(event: string | symbol, listener: Listener): this {
+    super.off(event, listener);
+    if (event === Events.Page.FileChooser && !this.listenerCount(event))
+      this._channel.setFileChooserInterceptedNoReply({ intercepted: false });
+    return this;
+  }
+
   removeListener(event: string | symbol, listener: Listener): this {
     super.removeListener(event, listener);
     if (event === Events.Page.FileChooser && !this.listenerCount(event))
diff --git a/src/rpc/server/pageDispatcher.ts b/src/rpc/server/pageDispatcher.ts
index a488e6f91d..ca124bbde5 100644
--- a/src/rpc/server/pageDispatcher.ts
+++ b/src/rpc/server/pageDispatcher.ts
@@ -36,6 +36,7 @@ import { CRCoverage } from '../../chromium/crCoverage';
 
 export class PageDispatcher extends Dispatcher<Page, PageInitializer> implements PageChannel {
   private _page: Page;
+  private _onFileChooser: (fileChooser: FileChooser) => void;
 
   constructor(scope: DispatcherScope, page: Page) {
     // TODO: theoretically, there could be more than one frame already.
@@ -52,10 +53,11 @@ export class PageDispatcher extends Dispatcher<Page, PageInitializer> implements
     page.on(Events.Page.DOMContentLoaded, () => this._dispatchEvent('domcontentloaded'));
     page.on(Events.Page.Dialog, dialog => this._dispatchEvent('dialog', { dialog: new DialogDispatcher(this._scope, dialog) }));
     page.on(Events.Page.Download, dialog => this._dispatchEvent('download', { download: new DownloadDispatcher(this._scope, dialog) }));
-    page.on(Events.Page.FileChooser, (fileChooser: FileChooser) => this._dispatchEvent('fileChooser', {
+    // We add this listener lazily, to avoid intercepting file chooser when noone listens.
+    this._onFileChooser = fileChooser => this._dispatchEvent('fileChooser', {
       element: new ElementHandleDispatcher(this._scope, fileChooser.element()),
       isMultiple: fileChooser.isMultiple()
-    }));
+    });
     page.on(Events.Page.FrameAttached, frame => this._onFrameAttached(frame));
     page.on(Events.Page.FrameDetached, frame => this._onFrameDetached(frame));
     page.on(Events.Page.Load, () => this._dispatchEvent('load'));
@@ -141,6 +143,10 @@ export class PageDispatcher extends Dispatcher<Page, PageInitializer> implements
   }
 
   async setFileChooserInterceptedNoReply(params: { intercepted: boolean }) {
+    if (params.intercepted)
+      this._page.on(Events.Page.FileChooser, this._onFileChooser);
+    else
+      this._page.removeListener(Events.Page.FileChooser, this._onFileChooser);
   }
 
   async title() {
diff --git a/test/page-set-input-files.spec.ts b/test/page-set-input-files.spec.ts
index 440ec69676..511aac59bf 100644
--- a/test/page-set-input-files.spec.ts
+++ b/test/page-set-input-files.spec.ts
@@ -54,7 +54,7 @@ it('should set from memory', async({page}) => {
   expect(await page.$eval('input', input => input.files[0].name)).toBe('test.txt');
 });
 
-it('should emit event', async({page, server}) => {
+it('should emit event once', async({page, server}) => {
   await page.setContent(`<input type=file>`);
   const [chooser] = await Promise.all([
     new Promise(f => page.once('filechooser', f)),
@@ -63,6 +63,36 @@ it('should emit event', async({page, server}) => {
   expect(chooser).toBeTruthy();
 });
 
+it('should emit event on/off', async({page, server}) => {
+  await page.setContent(`<input type=file>`);
+  const [chooser] = await Promise.all([
+    new Promise(f => {
+      const listener = chooser => {
+        page.off('filechooser', listener);
+        f(chooser);
+      }
+      page.on('filechooser', listener);
+    }),
+    page.click('input'),
+  ]);
+  expect(chooser).toBeTruthy();
+});
+
+it('should emit event addListener/removeListener', async({page, server}) => {
+  await page.setContent(`<input type=file>`);
+  const [chooser] = await Promise.all([
+    new Promise(f => {
+      const listener = chooser => {
+        page.removeListener('filechooser', listener);
+        f(chooser);
+      }
+      page.addListener('filechooser', listener);
+    }),
+    page.click('input'),
+  ]);
+  expect(chooser).toBeTruthy();
+});
+
 it('should work when file input is attached to DOM', async({page, server}) => {
   await page.setContent(`<input type=file>`);
   const [chooser] = await Promise.all([