fix: handle missing detach events for restored bfcache targets (#10967)

puppeteer · Sep 21, 2023 · 7bcdfcb · 7bcdfcb
1 parent 0b7f021
commit 7bcdfcb
Show file tree

Hide file tree

Showing 6 changed files with 54 additions and 9 deletions.
diff --git a/packages/puppeteer-core/src/cdp/ChromeTargetManager.ts b/packages/puppeteer-core/src/cdp/ChromeTargetManager.ts
@@ -24,6 +24,7 @@ import {debugError} from '../common/util.js';
 import {assert} from '../util/assert.js';
 import {Deferred} from '../util/Deferred.js';
 
+import {type CdpCDPSession} from './CDPSession.js';
 import {type Connection} from './Connection.js';
 import {CdpTarget, InitializationStatus} from './Target.js';
 import {
@@ -358,10 +359,7 @@ export class ChromeTargetManager
     // `this.#connection.isAutoAttached(targetInfo.targetId)`. In the future, we
     // should determine if a target is auto-attached or not with the help of
     // CDP.
-    if (
-      targetInfo.type === 'service_worker' &&
-      this.#connection.isAutoAttached(targetInfo.targetId)
-    ) {
+    if (targetInfo.type === 'service_worker') {
       this.#finishInitializationIfReady(targetInfo.targetId);
       await silentDetach();
       if (this.#attachedTargetsByTargetId.has(targetInfo.targetId)) {
@@ -393,18 +391,16 @@ export class ChromeTargetManager
       return;
     }
 
-    if (!isExistingTarget) {
-      target._initialize();
-    }
-
     this.#setupAttachmentListeners(session);
 
     if (isExistingTarget) {
+      (session as CdpCDPSession)._setTarget(target);
       this.#attachedTargetsBySessionId.set(
         session.id(),
         this.#attachedTargetsByTargetId.get(targetInfo.targetId)!
       );
     } else {
+      target._initialize();
       this.#attachedTargetsByTargetId.set(targetInfo.targetId, target);
       this.#attachedTargetsBySessionId.set(session.id(), target);
     }

diff --git a/test/TestExpectations.json b/test/TestExpectations.json
@@ -1397,6 +1397,12 @@
     "parameters": ["chrome", "webDriverBiDi"],
     "expectations": ["PASS"]
   },
+  {
+    "testIdPattern": "[bfcache.spec] BFCache can navigate to a BFCached page containing an OOPIF and a worker",
+    "platforms": ["darwin", "linux", "win32"],
+    "parameters": ["cdp", "firefox"],
+    "expectations": ["SKIP"]
+  },
   {
     "testIdPattern": "[browser.spec] Browser specs Browser.isConnected should set the browser connected state",
     "platforms": ["darwin", "linux", "win32"],

diff --git a/test/assets/cached/bfcache/worker-iframe-container.html b/test/assets/cached/bfcache/worker-iframe-container.html
@@ -0,0 +1,11 @@
+<body>BFCached<a href="target.html">next</a></body>
+<script>
+  window.addEventListener('DOMContentLoaded', () => {
+    const iframe = document.createElement('iframe');
+    const url = new URL(location.href);
+    url.hostname = url.hostname === 'localhost' ? '127.0.0.1' : 'localhost';
+    url.pathname = '/cached/bfcache/worker-iframe.html';
+    iframe.src = url.toString();
+    document.body.appendChild(iframe);
+  }, false);
+</script>
diff --git a/test/assets/cached/bfcache/worker-iframe.html b/test/assets/cached/bfcache/worker-iframe.html
@@ -0,0 +1,3 @@
+<script>
+  const worker = new Worker('worker.mjs', {type: 'module'})
+</script>
diff --git a/test/assets/cached/bfcache/worker.mjs b/test/assets/cached/bfcache/worker.mjs
@@ -0,0 +1 @@
+console.log('HELLO');
diff --git a/test/src/bfcache.spec.ts b/test/src/bfcache.spec.ts
@@ -15,8 +15,10 @@
  */
 
 import expect from 'expect';
+import {PageEvent} from 'puppeteer-core';
 
 import {launch} from './mocha-utils.js';
+import {waitEvent} from './utils.js';
 
 describe('BFCache', function () {
   it('can navigate to a BFCached page', async () => {
@@ -25,7 +27,7 @@ describe('BFCache', function () {
     });
 
     try {
-      page.setDefaultTimeout(2000);
+      page.setDefaultTimeout(3000);
 
       await page.goto(httpsServer.PREFIX + '/cached/bfcache/index.html');
 
@@ -44,4 +46,30 @@ describe('BFCache', function () {
       await close();
     }
   });
+
+  it('can navigate to a BFCached page containing an OOPIF and a worker', async () => {
+    const {httpsServer, page, close} = await launch({
+      ignoreHTTPSErrors: true,
+    });
+    try {
+      page.setDefaultTimeout(3000);
+      const [worker1] = await Promise.all([
+        waitEvent(page, PageEvent.WorkerCreated),
+        page.goto(
+          httpsServer.PREFIX + '/cached/bfcache/worker-iframe-container.html'
+        ),
+      ]);
+      expect(await worker1.evaluate('1 + 1')).toBe(2);
+      await Promise.all([page.waitForNavigation(), page.locator('a').click()]);
+
+      const [worker2] = await Promise.all([
+        waitEvent(page, PageEvent.WorkerCreated),
+        page.waitForNavigation(),
+        page.goBack(),
+      ]);
+      expect(await worker2.evaluate('1 + 1')).toBe(2);
+    } finally {
+      await close();
+    }
+  });
 });