From 20988775bf537c6343ec082c46a91c8eae741955 Mon Sep 17 00:00:00 2001
From: Andrey Lushnikov <aslushnikov@gmail.com>
Date: Thu, 11 Apr 2019 16:26:18 -0400
Subject: [PATCH] fix: gracefully handle multiple contexts for secondary DOM
 World (#4276)

In case of multiple sessions to the same target, there's a race between
sessions to create a secondary isolated world. As a result, we might
end up having 2 execution contexts created for the needs of the
secondary isolated world.

This patch starts handling this race gracefully: instead of crashing,
we can use either of the execution contexts and ignore the rest.

Notably, the same race condition might happen if page reloads itself
in-between the calls to `page.addEvaluateOnNewDocument` and
`page.createIsolatedWorld`.

Fixes #4197.
---
 lib/DOMWorld.js       |  7 +++++++
 lib/FrameManager.js   |  8 ++++++--
 test/launcher.spec.js | 13 +++++++++++++
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/lib/DOMWorld.js b/lib/DOMWorld.js
index e00d66621d5..1668fad0034 100644
--- a/lib/DOMWorld.js
+++ b/lib/DOMWorld.js
@@ -70,6 +70,13 @@ class DOMWorld {
     }
   }
 
+  /**
+   * @return {boolean}
+   */
+  _hasContext() {
+    return !this._contextResolveCallback;
+  }
+
   _detach() {
     this._detached = true;
     for (const waitTask of this._waitTasks)
diff --git a/lib/FrameManager.js b/lib/FrameManager.js
index 5f42e4f4d33..818bf37b450 100644
--- a/lib/FrameManager.js
+++ b/lib/FrameManager.js
@@ -306,10 +306,14 @@ class FrameManager extends EventEmitter {
     const frame = this._frames.get(frameId) || null;
     let world = null;
     if (frame) {
-      if (contextPayload.auxData && !!contextPayload.auxData['isDefault'])
+      if (contextPayload.auxData && !!contextPayload.auxData['isDefault']) {
         world = frame._mainWorld;
-      else if (contextPayload.name === UTILITY_WORLD_NAME)
+      } else if (contextPayload.name === UTILITY_WORLD_NAME && !frame._secondaryWorld._hasContext()) {
+        // In case of multiple sessions to the same target, there's a race between
+        // connections so we might end up creating multiple isolated worlds.
+        // We can use either.
         world = frame._secondaryWorld;
+      }
     }
     if (contextPayload.auxData && contextPayload.auxData['type'] === 'isolated')
       this._isolatedWorlds.add(contextPayload.name);
diff --git a/test/launcher.spec.js b/test/launcher.spec.js
index 8533f204448..b573dc7bc81 100644
--- a/test/launcher.spec.js
+++ b/test/launcher.spec.js
@@ -316,6 +316,19 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions, p
         expect(await restoredPage.evaluate(() => 7 * 8)).toBe(56);
         await browser.close();
       });
+      // @see https://github.com/GoogleChrome/puppeteer/issues/4197#issuecomment-481793410
+      it('should be able to connect to the same page simultaneously', async({server}) => {
+        const browserOne = await puppeteer.launch();
+        const browserTwo = await puppeteer.connect({ browserWSEndpoint: browserOne.wsEndpoint() });
+        const [page1, page2] = await Promise.all([
+          new Promise(x => browserOne.once('targetcreated', target => x(target.page()))),
+          browserTwo.newPage(),
+        ]);
+        expect(await page1.evaluate(() => 7 * 8)).toBe(56);
+        expect(await page2.evaluate(() => 7 * 6)).toBe(42);
+        await browserOne.close();
+      });
+
     });
     describe('Puppeteer.executablePath', function() {
       it('should work', async({server}) => {