From 74f7005c0286679955086437d440ad1166180de2 Mon Sep 17 00:00:00 2001
From: Pavel Feldman <pavel.feldman@gmail.com>
Date: Wed, 3 Aug 2022 16:14:28 -0700
Subject: [PATCH] chore: preserve window while reusing window (#16225)

---
 .../src/remote/playwrightServer.ts            |  9 +++-
 .../playwright-core/src/server/browser.ts     |  4 ++
 .../src/server/browserContext.ts              | 26 ++++++++++-
 .../src/server/chromium/crBrowser.ts          |  7 +++
 .../src/server/chromium/crPage.ts             | 43 +++++++++++++------
 .../src/server/firefox/ffBrowser.ts           |  4 ++
 packages/playwright-core/src/server/page.ts   | 11 ++---
 .../src/server/webkit/wkBrowser.ts            |  6 +++
 .../src/server/webkit/wkPage.ts               |  7 ++-
 .../playwright.ct-reuse.spec.ts               | 21 +++++++--
 10 files changed, 112 insertions(+), 26 deletions(-)

diff --git a/packages/playwright-core/src/remote/playwrightServer.ts b/packages/playwright-core/src/remote/playwrightServer.ts
index 01ca641aaa..cacf70dbf0 100644
--- a/packages/playwright-core/src/remote/playwrightServer.ts
+++ b/packages/playwright-core/src/remote/playwrightServer.ts
@@ -65,7 +65,14 @@ export class PlaywrightServer {
     if (this._mode === 'reuse-browser') {
       const callMetadata = serverSideCallMetadata();
       const browser = await this._preLaunchedPlaywright!.chromium.launch(callMetadata, { headless: false });
-      const { context } = await browser.newContextForReuse({ viewport: { width: 800, height: 600 } }, callMetadata);
+      const { context } = await browser.newContextForReuse({
+        viewport: {
+          width: 800,
+          height: 600
+        },
+        locale: 'en-US',
+        deviceScaleFactor: process.platform === 'darwin' ? 2 : 1
+      }, callMetadata);
       const page = await context.newPage(callMetadata);
       await page.mainFrame().setContent(callMetadata, `
         <style>
diff --git a/packages/playwright-core/src/server/browser.ts b/packages/playwright-core/src/server/browser.ts
index 2e4a5e77a3..0c7a12d1f0 100644
--- a/packages/playwright-core/src/server/browser.ts
+++ b/packages/playwright-core/src/server/browser.ts
@@ -94,6 +94,10 @@ export abstract class Browser extends SdkObject {
 
   async newContextForReuse(params: channels.BrowserNewContextForReuseParams, metadata: CallMetadata): Promise<{ context: BrowserContext, needsReset: boolean }> {
     const hash = BrowserContext.reusableContextHash(params);
+    for (const context of this.contexts()) {
+      if (context !== this._contextForReuse?.context)
+        await context.close(metadata);
+    }
     if (!this._contextForReuse || hash !== this._contextForReuse.hash || !this._contextForReuse.context.canResetForReuse()) {
       if (this._contextForReuse)
         await this._contextForReuse.context.close(metadata);
diff --git a/packages/playwright-core/src/server/browserContext.ts b/packages/playwright-core/src/server/browserContext.ts
index b5d9efde83..a14d31efc3 100644
--- a/packages/playwright-core/src/server/browserContext.ts
+++ b/packages/playwright-core/src/server/browserContext.ts
@@ -145,6 +145,13 @@ export abstract class BrowserContext extends SdkObject {
 
   static reusableContextHash(params: channels.BrowserNewContextForReuseParams): string {
     const paramsCopy = { ...params };
+
+    for (const k of Object.keys(paramsCopy)) {
+      const key = k as keyof channels.BrowserNewContextForReuseParams;
+      if (paramsCopy[key] === defaultNewContextParamValues[key])
+        delete paramsCopy[key];
+    }
+
     for (const key of paramsThatAllowContextReuse)
       delete paramsCopy[key];
     return JSON.stringify(paramsCopy);
@@ -159,6 +166,7 @@ export abstract class BrowserContext extends SdkObject {
 
     await this._cancelAllRoutesInFlight();
 
+    // Close extra pages early.
     let page: Page | undefined = this.pages()[0];
     const [, ...otherPages] = this.pages();
     for (const p of otherPages)
@@ -184,6 +192,7 @@ export abstract class BrowserContext extends SdkObject {
     await this.setExtraHTTPHeaders(this._options.extraHTTPHeaders || []);
     await this.setGeolocation(this._options.geolocation);
     await this.setOffline(!!this._options.offline);
+    await this.setUserAgent(this._options.userAgent);
     await this.clearCookies();
 
     await page?.resetForReuse(metadata);
@@ -218,6 +227,7 @@ export abstract class BrowserContext extends SdkObject {
   abstract clearCookies(): Promise<void>;
   abstract setGeolocation(geolocation?: types.Geolocation): Promise<void>;
   abstract setExtraHTTPHeaders(headers: types.HeadersArray): Promise<void>;
+  abstract setUserAgent(userAgent: string | undefined): Promise<void>;
   abstract setOffline(offline: boolean): Promise<void>;
   abstract cancelDownload(uuid: string): Promise<void>;
   protected abstract doGetCookies(urls: string[]): Promise<channels.NetworkCookie[]>;
@@ -637,5 +647,19 @@ const paramsThatAllowContextReuse: (keyof channels.BrowserNewContextForReusePara
   'forcedColors',
   'reducedMotion',
   'screen',
-  'viewport'
+  'userAgent',
+  'viewport',
 ];
+
+const defaultNewContextParamValues: channels.BrowserNewContextForReuseParams = {
+  noDefaultViewport: false,
+  ignoreHTTPSErrors: false,
+  javaScriptEnabled: true,
+  bypassCSP: false,
+  offline: false,
+  isMobile: false,
+  hasTouch: false,
+  acceptDownloads: true,
+  strictSelectors: false,
+  serviceWorkers: 'allow',
+};
diff --git a/packages/playwright-core/src/server/chromium/crBrowser.ts b/packages/playwright-core/src/server/chromium/crBrowser.ts
index 7ac3efd031..5647fd0a59 100644
--- a/packages/playwright-core/src/server/chromium/crBrowser.ts
+++ b/packages/playwright-core/src/server/chromium/crBrowser.ts
@@ -438,6 +438,13 @@ export class CRBrowserContext extends BrowserContext {
       await (sw as CRServiceWorker).updateExtraHTTPHeaders(false);
   }
 
+  async setUserAgent(userAgent: string | undefined): Promise<void> {
+    this._options.userAgent = userAgent;
+    for (const page of this.pages())
+      await (page._delegate as CRPage).updateUserAgent();
+    // TODO: service workers don't have Emulation domain?
+  }
+
   async setOffline(offline: boolean): Promise<void> {
     this._options.offline = offline;
     for (const page of this.pages())
diff --git a/packages/playwright-core/src/server/chromium/crPage.ts b/packages/playwright-core/src/server/chromium/crPage.ts
index 45ed62c021..cacb1e2613 100644
--- a/packages/playwright-core/src/server/chromium/crPage.ts
+++ b/packages/playwright-core/src/server/chromium/crPage.ts
@@ -199,8 +199,8 @@ export class CRPage implements PageDelegate {
     await this._forAllFrameSessions(frame => frame._updateHttpCredentials(false));
   }
 
-  async updateEmulatedViewportSize(): Promise<void> {
-    await this._mainFrameSession._updateViewport();
+  async updateEmulatedViewportSize(preserveWindowBoundaries?: boolean): Promise<void> {
+    await this._mainFrameSession._updateViewport(preserveWindowBoundaries);
   }
 
   async bringToFront(): Promise<void> {
@@ -211,6 +211,10 @@ export class CRPage implements PageDelegate {
     await this._forAllFrameSessions(frame => frame._updateEmulateMedia());
   }
 
+  async updateUserAgent(): Promise<void> {
+    await this._forAllFrameSessions(frame => frame._updateUserAgent());
+  }
+
   async updateRequestInterception(): Promise<void> {
     await this._forAllFrameSessions(frame => frame._updateRequestInterception());
   }
@@ -399,6 +403,7 @@ class FrameSession {
   private _screencastClients = new Set<any>();
   private _evaluateOnNewDocumentIdentifiers: string[] = [];
   private _exposedBindingNames: string[] = [];
+  private _metricsOverride: Protocol.Emulation.setDeviceMetricsOverrideParameters | undefined;
 
   constructor(crPage: CRPage, client: CRSession, targetId: string, parentSession: FrameSession | null) {
     this._client = client;
@@ -542,7 +547,7 @@ class FrameSession {
       if (options.javaScriptEnabled === false)
         promises.push(this._client.send('Emulation.setScriptExecutionDisabled', { value: true }));
       if (options.userAgent || options.locale)
-        promises.push(this._client.send('Emulation.setUserAgentOverride', { userAgent: options.userAgent || '', acceptLanguage: options.locale }));
+        promises.push(this._updateUserAgent());
       if (options.locale)
         promises.push(emulateLocale(this._client, options.locale));
       if (options.timezoneId)
@@ -995,7 +1000,7 @@ class FrameSession {
       await this._networkManager.authenticate(credentials);
   }
 
-  async _updateViewport(): Promise<void> {
+  async _updateViewport(preserveWindowBoundaries?: boolean): Promise<void> {
     if (this._crPage._browserContext._browser.isClank())
       return;
     assert(this._isMainFrame());
@@ -1006,18 +1011,22 @@ class FrameSession {
     const viewportSize = emulatedSize.viewport;
     const screenSize = emulatedSize.screen;
     const isLandscape = viewportSize.width > viewportSize.height;
+    const metricsOverride: Protocol.Emulation.setDeviceMetricsOverrideParameters = {
+      mobile: !!options.isMobile,
+      width: viewportSize.width,
+      height: viewportSize.height,
+      screenWidth: screenSize.width,
+      screenHeight: screenSize.height,
+      deviceScaleFactor: options.deviceScaleFactor || 1,
+      screenOrientation: isLandscape ? { angle: 90, type: 'landscapePrimary' } : { angle: 0, type: 'portraitPrimary' },
+      dontSetVisibleSize: preserveWindowBoundaries
+    };
+    if (JSON.stringify(this._metricsOverride) === JSON.stringify(metricsOverride))
+      return;
     const promises = [
-      this._client.send('Emulation.setDeviceMetricsOverride', {
-        mobile: !!options.isMobile,
-        width: viewportSize.width,
-        height: viewportSize.height,
-        screenWidth: screenSize.width,
-        screenHeight: screenSize.height,
-        deviceScaleFactor: options.deviceScaleFactor || 1,
-        screenOrientation: isLandscape ? { angle: 90, type: 'landscapePrimary' } : { angle: 0, type: 'portraitPrimary' },
-      }),
+      this._client.send('Emulation.setDeviceMetricsOverride', metricsOverride),
     ];
-    if (this._windowId) {
+    if (!preserveWindowBoundaries && this._windowId) {
       let insets = { width: 0, height: 0 };
       if (this._crPage._browserContext._browser.options.headful) {
         // TODO: popup windows have their own insets.
@@ -1041,6 +1050,7 @@ class FrameSession {
       }));
     }
     await Promise.all(promises);
+    this._metricsOverride = metricsOverride;
   }
 
   async windowBounds(): Promise<WindowBounds> {
@@ -1071,6 +1081,11 @@ class FrameSession {
     await this._client.send('Emulation.setEmulatedMedia', { media: emulatedMedia.media || '', features });
   }
 
+  async _updateUserAgent(): Promise<void> {
+    const options = this._crPage._browserContext._options;
+    await this._client.send('Emulation.setUserAgentOverride', { userAgent: options.userAgent || '', acceptLanguage: options.locale });
+  }
+
   private async _setDefaultFontFamilies(session: CRSession) {
     const fontFamilies = platformToFontFamilies[this._crPage._browserContext._browser._platform()];
     await session.send('Page.setFontFamilies', fontFamilies);
diff --git a/packages/playwright-core/src/server/firefox/ffBrowser.ts b/packages/playwright-core/src/server/firefox/ffBrowser.ts
index e48f61039f..c5ba027345 100644
--- a/packages/playwright-core/src/server/firefox/ffBrowser.ts
+++ b/packages/playwright-core/src/server/firefox/ffBrowser.ts
@@ -310,6 +310,10 @@ export class FFBrowserContext extends BrowserContext {
     await this._browser._connection.send('Browser.setExtraHTTPHeaders', { browserContextId: this._browserContextId, headers: allHeaders });
   }
 
+  async setUserAgent(userAgent: string | undefined): Promise<void> {
+    await this._browser._connection.send('Browser.setUserAgentOverride', { browserContextId: this._browserContextId, userAgent: userAgent || null });
+  }
+
   async setOffline(offline: boolean): Promise<void> {
     this._options.offline = offline;
     await this._browser._connection.send('Browser.setOnlineOverride', { browserContextId: this._browserContextId, override: offline ? 'offline' : 'online' });
diff --git a/packages/playwright-core/src/server/page.ts b/packages/playwright-core/src/server/page.ts
index 4fbdd25bec..2027938750 100644
--- a/packages/playwright-core/src/server/page.ts
+++ b/packages/playwright-core/src/server/page.ts
@@ -65,7 +65,7 @@ export interface PageDelegate {
   navigateFrame(frame: frames.Frame, url: string, referrer: string | undefined): Promise<frames.GotoResult>;
 
   updateExtraHTTPHeaders(): Promise<void>;
-  updateEmulatedViewportSize(): Promise<void>;
+  updateEmulatedViewportSize(preserveWindowBoundaries?: boolean): Promise<void>;
   updateEmulateMedia(): Promise<void>;
   updateRequestInterception(): Promise<void>;
   updateFileChooserInterception(): Promise<void>;
@@ -247,10 +247,11 @@ export class Page extends SdkObject {
     this._extraHTTPHeaders = undefined;
     this._interceptFileChooser = false;
 
-    await this._delegate.updateEmulatedViewportSize();
-    await this._delegate.updateEmulateMedia();
-    await this._delegate.updateExtraHTTPHeaders();
-    await this._delegate.updateFileChooserInterception();
+    await Promise.all([
+      this._delegate.updateEmulatedViewportSize(true),
+      this._delegate.updateEmulateMedia(),
+      this._delegate.updateFileChooserInterception(),
+    ]);
   }
 
   async _doSlowMo() {
diff --git a/packages/playwright-core/src/server/webkit/wkBrowser.ts b/packages/playwright-core/src/server/webkit/wkBrowser.ts
index 1556ffdce7..500505233b 100644
--- a/packages/playwright-core/src/server/webkit/wkBrowser.ts
+++ b/packages/playwright-core/src/server/webkit/wkBrowser.ts
@@ -292,6 +292,12 @@ export class WKBrowserContext extends BrowserContext {
       await (page._delegate as WKPage).updateExtraHTTPHeaders();
   }
 
+  async setUserAgent(userAgent: string | undefined): Promise<void> {
+    this._options.userAgent = userAgent;
+    for (const page of this.pages())
+      await (page._delegate as WKPage).updateUserAgent();
+  }
+
   async setOffline(offline: boolean): Promise<void> {
     this._options.offline = offline;
     for (const page of this.pages())
diff --git a/packages/playwright-core/src/server/webkit/wkPage.ts b/packages/playwright-core/src/server/webkit/wkPage.ts
index ecd4e4bf21..3aaa4d9d00 100644
--- a/packages/playwright-core/src/server/webkit/wkPage.ts
+++ b/packages/playwright-core/src/server/webkit/wkPage.ts
@@ -192,7 +192,7 @@ export class WKPage implements PageDelegate {
 
     const contextOptions = this._browserContext._options;
     if (contextOptions.userAgent)
-      promises.push(session.send('Page.overrideUserAgent', { value: contextOptions.userAgent }));
+      promises.push(this.updateUserAgent());
     const emulatedMedia = this._page.emulatedMedia();
     if (emulatedMedia.media || emulatedMedia.colorScheme || emulatedMedia.reducedMotion)
       promises.push(WKPage._setEmulateMedia(session, emulatedMedia.media, emulatedMedia.colorScheme, emulatedMedia.reducedMotion));
@@ -679,6 +679,11 @@ export class WKPage implements PageDelegate {
     await this._updateViewport();
   }
 
+  async updateUserAgent(): Promise<void> {
+    const contextOptions = this._browserContext._options;
+    this._updateState('Page.overrideUserAgent', { value: contextOptions.userAgent });
+  }
+
   async bringToFront(): Promise<void> {
     this._pageProxySession.send('Target.activate', {
       targetId: this._session.sessionId
diff --git a/tests/playwright-test/playwright.ct-reuse.spec.ts b/tests/playwright-test/playwright.ct-reuse.spec.ts
index 8b39584acc..9dabd12548 100644
--- a/tests/playwright-test/playwright.ct-reuse.spec.ts
+++ b/tests/playwright-test/playwright.ct-reuse.spec.ts
@@ -36,10 +36,23 @@ test('should reuse context', async ({ runInlineTest }) => {
         expect(context._guid).toBe(lastContextGuid);
       });
 
-      test.describe('Dark', () => {
-        test.use({ userAgent: 'dark' });
+      test.describe(() => {
+        test.use({ colorScheme: 'dark' });
+        test('dark', async ({ context }) => {
+          expect(context._guid).toBe(lastContextGuid);
+        });
+      });
 
-        test('three', async ({ context }) => {
+      test.describe(() => {
+        test.use({ userAgent: 'UA' });
+        test('UA', async ({ context }) => {
+          expect(context._guid).toBe(lastContextGuid);
+        });
+      });
+
+      test.describe(() => {
+        test.use({ timezoneId: 'Europe/Berlin' });
+        test('tz', async ({ context }) => {
           expect(context._guid).not.toBe(lastContextGuid);
         });
       });
@@ -47,7 +60,7 @@ test('should reuse context', async ({ runInlineTest }) => {
   }, { workers: 1 });
 
   expect(result.exitCode).toBe(0);
-  expect(result.passed).toBe(3);
+  expect(result.passed).toBe(5);
 });
 
 test('should not reuse context with video', async ({ runInlineTest }) => {