From d5a2781e8015dd905b04fe72e37c614e5557ccad Mon Sep 17 00:00:00 2001
From: Dmitry Gozman <dgozman@gmail.com>
Date: Tue, 10 Mar 2020 10:06:17 -0700
Subject: [PATCH] fix(chromium): do not await extra promises in initialize() to
 attach early enough (#1311)

---
 src/chromium/crPage.ts | 29 ++++++++++-------------------
 src/firefox/ffPage.ts  |  1 +
 test/popup.spec.js     | 18 ++++++++++++++++++
 3 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/src/chromium/crPage.ts b/src/chromium/crPage.ts
index 20b33a038e..e4c51e335b 100644
--- a/src/chromium/crPage.ts
+++ b/src/chromium/crPage.ts
@@ -46,7 +46,6 @@ export class CRPage implements PageDelegate {
   private readonly _page: Page;
   readonly _networkManager: CRNetworkManager;
   private _contextIdToContext = new Map<number, dom.FrameExecutionContext>();
-  private _isolatedWorlds = new Set<string>();
   private _eventListeners: RegisteredListener[] = [];
   rawMouse: RawMouseImpl;
   rawKeyboard: RawKeyboardImpl;
@@ -93,10 +92,19 @@ export class CRPage implements PageDelegate {
           helper.addEventListener(this._client, 'Target.attachedToTarget', event => this._onAttachedToTarget(event)),
           helper.addEventListener(this._client, 'Target.detachedFromTarget', event => this._onDetachedFromTarget(event)),
         ];
+        this._page.frames().map(frame => this._client.send('Page.createIsolatedWorld', {
+          frameId: frame._id,
+          grantUniveralAccess: true,
+          worldName: UTILITY_WORLD_NAME,
+        }).catch(debugError)); // frames might be removed before we send this.
       }),
       this._client.send('Log.enable', {}),
       this._client.send('Page.setLifecycleEventsEnabled', { enabled: true }),
-      this._client.send('Runtime.enable', {}).then(() => this._ensureIsolatedWorld(UTILITY_WORLD_NAME)),
+      this._client.send('Runtime.enable', {}),
+      this._client.send('Page.addScriptToEvaluateOnNewDocument', {
+        source: `//# sourceURL=${EVALUATION_SCRIPT_URL}`,
+        worldName: UTILITY_WORLD_NAME,
+      }),
       this._networkManager.initialize(),
       this._client.send('Target.setAutoAttach', { autoAttach: true, waitForDebuggerOnStart: true, flatten: true }),
       this._client.send('Emulation.setFocusEmulationEnabled', { enabled: true }),
@@ -182,21 +190,6 @@ export class CRPage implements PageDelegate {
     this._page._frameManager.frameRequestedNavigation(payload.frameId);
   }
 
-  async _ensureIsolatedWorld(name: string) {
-    if (this._isolatedWorlds.has(name))
-      return;
-    this._isolatedWorlds.add(name);
-    await this._client.send('Page.addScriptToEvaluateOnNewDocument', {
-      source: `//# sourceURL=${EVALUATION_SCRIPT_URL}`,
-      worldName: name,
-    });
-    await Promise.all(this._page.frames().map(frame => this._client.send('Page.createIsolatedWorld', {
-      frameId: frame._id,
-      grantUniveralAccess: true,
-      worldName: name,
-    }).catch(debugError))); // frames might be removed before we send this
-  }
-
   _onFrameNavigatedWithinDocument(frameId: string, url: string) {
     this._page._frameManager.frameCommittedSameDocumentNavigation(frameId, url);
   }
@@ -209,8 +202,6 @@ export class CRPage implements PageDelegate {
     const frame = contextPayload.auxData ? this._page._frameManager.frame(contextPayload.auxData.frameId) : null;
     if (!frame)
       return;
-    if (contextPayload.auxData && contextPayload.auxData.type === 'isolated')
-      this._isolatedWorlds.add(contextPayload.name);
     const delegate = new CRExecutionContext(this._client, contextPayload);
     const context = new dom.FrameExecutionContext(delegate, frame);
     if (contextPayload.auxData && !!contextPayload.auxData.isDefault)
diff --git a/src/firefox/ffPage.ts b/src/firefox/ffPage.ts
index abfb6c168b..215ebac946 100644
--- a/src/firefox/ffPage.ts
+++ b/src/firefox/ffPage.ts
@@ -88,6 +88,7 @@ export class FFPage implements PageDelegate {
   async _initialize() {
     try {
       await Promise.all([
+        // TODO: we should get rid of this call to resolve before any early events arrive, e.g. dialogs.
         this._session.send('Page.addScriptToEvaluateOnNewDocument', {
           script: '',
           worldName: UTILITY_WORLD_NAME,
diff --git a/test/popup.spec.js b/test/popup.spec.js
index 0a971a60e9..1bc21d406f 100644
--- a/test/popup.spec.js
+++ b/test/popup.spec.js
@@ -204,6 +204,24 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, WE
       expect(popupEvent).toBeTruthy();
       await context.close();
     });
+    it.fail(FFOX)('should be able to capture alert', async({browser}) => {
+      // Firefox:
+      // - immediately closes dialog by itself, without protocol call;
+      // - waits for Page.addScriptToEvaluateOnNewDocument before resolving page(), which is too late.
+      const context = await browser.newContext();
+      const page = await context.newPage();
+      const evaluatePromise = page.evaluate(() => {
+        const win = window.open('about:blank');
+        win.alert('hello');
+      });
+      const popupEvent = await page.waitForEvent('popup');
+      const popup = await popupEvent.page();
+      const dialog = await popup.waitForEvent('dialog');
+      expect(dialog.message()).toBe('hello');
+      await dialog.dismiss();
+      await evaluatePromise;
+      await context.close();
+    });
     it('should work with empty url', async({browser}) => {
       const context = await browser.newContext();
       const page = await context.newPage();