From 97fa2518e9f5ba8b097524d84d4cc29a72565d6a Mon Sep 17 00:00:00 2001
From: Pavel Feldman <pavel.feldman@gmail.com>
Date: Thu, 4 Aug 2022 16:39:18 -0700
Subject: [PATCH] fix(reuse): clear storage after stopping all scripts (#16275)

---
 examples/todomvc/tests/integration.spec.ts    | 22 +++++++++----------
 .../src/server/browserContext.ts              | 14 +++---------
 packages/playwright-core/src/server/page.ts   |  6 +----
 .../playwright.ct-reuse.spec.ts               | 12 ++++++----
 4 files changed, 23 insertions(+), 31 deletions(-)

diff --git a/examples/todomvc/tests/integration.spec.ts b/examples/todomvc/tests/integration.spec.ts
index 47778cd765..0faf4da1e6 100644
--- a/examples/todomvc/tests/integration.spec.ts
+++ b/examples/todomvc/tests/integration.spec.ts
@@ -383,20 +383,20 @@ async function createDefaultTodos(page: Page) {
   }
 }
 
-async function checkNumberOfTodosInLocalStorage(page: Page, expected: number) {
-  return await page.waitForFunction(e => {
-    return JSON.parse(localStorage['react-todos']).length === e;
-  }, expected);
+async function checkNumberOfCompletedTodosInLocalStorage(page: Page, expected: number) {
+  await expect.poll(() => {
+    return page.evaluate(() => JSON.parse(localStorage['react-todos']).filter(i => i.completed).length);
+  }).toBe(expected);
 }
 
-async function checkNumberOfCompletedTodosInLocalStorage(page: Page, expected: number) {
-  return await page.waitForFunction(e => {
-    return JSON.parse(localStorage['react-todos']).filter(i => i.completed).length === e;
-  }, expected);
+async function checkNumberOfTodosInLocalStorage(page: Page, expected: number) {
+  await expect.poll(() => {
+    return page.evaluate(() => JSON.parse(localStorage['react-todos']).length);
+  }).toBe(expected);
 }
 
 async function checkTodosInLocalStorage(page: Page, title: string) {
-  return await page.waitForFunction(t => {
-    return JSON.parse(localStorage['react-todos']).map(i => i.title).includes(t);
-  }, title);
+  await expect.poll(() => {
+    return page.evaluate(() => JSON.parse(localStorage['react-todos']).map(i => i.title));
+  }).toContain(title);
 }
diff --git a/packages/playwright-core/src/server/browserContext.ts b/packages/playwright-core/src/server/browserContext.ts
index 5fd8bf7146..8510569f89 100644
--- a/packages/playwright-core/src/server/browserContext.ts
+++ b/packages/playwright-core/src/server/browserContext.ts
@@ -178,10 +178,9 @@ export abstract class BrowserContext extends SdkObject {
 
     // Unless I do this early, setting extra http headers below does not respond.
     await page?._frameManager.closeOpenDialogs();
-    // This should be before the navigation to about:blank so that we could save on
-    // a navigation as we clear local storage.
-    await this._clearStorage();
+    // Navigate to about:blank first to ensure no page scripts are running after this point.
     await page?.mainFrame().goto(metadata, 'about:blank', { timeout: 0 });
+    await this._clearStorage();
     await this._removeExposedBindings();
     await this._removeInitScripts();
     // TODO: following can be optimized to not perform noops.
@@ -475,15 +474,7 @@ export abstract class BrowserContext extends SdkObject {
     if (!this._origins.size)
       return;
     let page = this.pages()[0];
-    const originArray = [...this._origins];
 
-    // Fast path.
-    if (page && originArray.length === 1 && page.mainFrame().url().startsWith(originArray[0])) {
-      await page.mainFrame().clearStorageForCurrentOriginBestEffort();
-      return;
-    }
-
-    // Slow path.
     const internalMetadata = serverSideCallMetadata();
     page = page || await this.newPage(internalMetadata);
     await page._setServerRequestInterceptor(handler => {
@@ -495,6 +486,7 @@ export abstract class BrowserContext extends SdkObject {
       await frame.clearStorageForCurrentOriginBestEffort();
     }
     await page._setServerRequestInterceptor(undefined);
+    // It is safe to not restore the URL to about:blank since we are doing it in Page::resetForReuse.
   }
 
   isSettingStorageState(): boolean {
diff --git a/packages/playwright-core/src/server/page.ts b/packages/playwright-core/src/server/page.ts
index 2027938750..f8d6a90344 100644
--- a/packages/playwright-core/src/server/page.ts
+++ b/packages/playwright-core/src/server/page.ts
@@ -231,16 +231,12 @@ export class Page extends SdkObject {
     this.setDefaultNavigationTimeout(undefined);
     this.setDefaultTimeout(undefined);
 
-    // Do this first in order to unfreeze evaluates.
-    await this._frameManager.closeOpenDialogs();
-
     await this._removeExposedBindings();
     await this._removeInitScripts();
-
-    // TODO: handle pending routes.
     await this.setClientRequestInterceptor(undefined);
     await this._setServerRequestInterceptor(undefined);
     await this.setFileChooserIntercepted(false);
+    // Re-navigate once init scripts are gone.
     await this.mainFrame().goto(metadata, 'about:blank');
     this._emulatedSize = undefined;
     this._emulatedMedia = {};
diff --git a/tests/playwright-test/playwright.ct-reuse.spec.ts b/tests/playwright-test/playwright.ct-reuse.spec.ts
index d5863c40b9..6dbdbc9a4a 100644
--- a/tests/playwright-test/playwright.ct-reuse.spec.ts
+++ b/tests/playwright-test/playwright.ct-reuse.spec.ts
@@ -184,10 +184,14 @@ test('should clean storage', async ({ runInlineTest }) => {
       test('one', async ({ context, page }) => {
         lastContextGuid = context._guid;
 
-        await page.evaluate(async () => {
-          localStorage.foo = 'bar';
-          sessionStorage.foo = 'bar';
-        });
+        // Spam local storage.
+        page.evaluate(async () => {
+          while (true) {
+            localStorage.foo = 'bar';
+            sessionStorage.foo = 'bar';
+            await new Promise(f => setTimeout(f, 0));
+          }
+        }).catch(() => {});
 
         const local = await page.evaluate('localStorage.foo');
         const session = await page.evaluate('sessionStorage.foo');