fix: let adoptContext and Connect To Browser index contexts like New Context

atest/test/01_Browser_Management/persistent_state.robot

@@ -45,7 +45,7 @@ Switching Between Two Persistent Contexts Works
 New Context Fails With Persistent Context
     New Persistent Context
     Run Keyword And Expect Error
-    ...    Error: Trying to create a new context when a persistentContext is active
+    ...    Error: Cannot create a new context in chromium, because its context is persistent or adopted. Open a new browser first.
     ...    New Context
 
 New Persistent Context Can Use An URL

atest/test/05_JS_Tests/adopt.js

@@ -1,13 +1,12 @@
 let adoptedContextClosed = false;
 let onCloseCalled = false;
 
-async function openAdoptedPersistentContext(userDataDir, url, headless, playwright, adoptContext, defaultTimeout) {
+async function openAdoptedPersistentContext(userDataDir, url, headless, playwright, adoptContext) {
     const context = await playwright.chromium.launchPersistentContext(userDataDir, {
         headless: String(headless).toLowerCase() !== 'false',
     });
     context.on('close', () => (adoptedContextClosed = true));
     await context.pages()[0].goto(url);
-    context.setDefaultTimeout(defaultTimeout);
     return adoptContext(context, {
         onClose: async () => {
             onCloseCalled = true;
@@ -49,10 +48,8 @@ async function openAdoptedPersistentContextWithRecording(userDataDir, url, headl
         tracesDir,
         ...contextOptions,
     });
-    const page = context.pages()[0];
-    await page.goto(url);
-    const adopted = await adoptContext(context, { tracing, contextOptions });
-    return { ...adopted, videoPath: await page.video().path() };
+    await context.pages()[0].goto(url);
+    return adoptContext(context, { tracing, contextOptions });
 }
 
 exports.__esModule = true;

atest/test/05_JS_Tests/adopt_context.robot

@@ -17,7 +17,7 @@ Browser Keywords Work On A Context Adopted By A JavaScript Extension
     ${on_close_called} =    Adopted Context On Close Was Called
     Should Be True    ${on_close_called}
 
-An Extension Gives An Adopted Context The Library Timeout
+An Adopted Context Gets The Library Timeout
     Set Browser Timeout    1s    scope=Test
     Open Adopted Persistent Context    ${OUTPUT_DIR}/adopted-profile-timeout    ${LOGIN_URL}    ${HEADLESS}
     ${message} =    Message Of A Failed Wait

node/playwright-wrapper/__tests__/playwright-state.test.ts

@@ -19,6 +19,7 @@ import {
     closeContext,
     extensionKeywordCall,
     locatorCache,
+    newContext,
     newPage,
     PlaywrightState,
     removeFailedPage,
@@ -484,7 +485,7 @@ describe('removeFailedPage', () => {
 });
 
 function makeMockPage() {
-    return { on: jest.fn() } as any;
+    return { on: jest.fn(), video: () => null } as any;
 }
 
 function makeMockContext(pages: any[], close = jest.fn().mockResolvedValue(undefined)) {
@@ -574,6 +575,33 @@ describe('adoptContext', () => {
         expect(adopted.pageId).toBe(state.activeBrowser?.page?.id);
     });
 
+    it('returns the video path of the active page', async () => {
+        const state = new PlaywrightState();
+        const first = { on: jest.fn(), video: () => ({ path: async () => '/videos/first.webm' }) } as any;
+        const second = { on: jest.fn(), video: () => ({ path: async () => '/videos/second.webm' }) } as any;
+
+        const adopted = await state.adoptContext(makeMockContext([first, second]));
+
+        expect(adopted.videoPath).toBe('/videos/first.webm');
+    });
+
+    it('returns no video path when the context records no video', async () => {
+        const state = new PlaywrightState();
+        const page = { on: jest.fn(), video: () => null } as any;
+
+        const adopted = await state.adoptContext(makeMockContext([page]));
+
+        expect(adopted.videoPath).toBeNull();
+    });
+
+    it('returns no video path when the context has no pages', async () => {
+        const state = new PlaywrightState();
+
+        const adopted = await state.adoptContext(makeMockContext([]));
+
+        expect(adopted.videoPath).toBeNull();
+    });
+
     it('keeps an existing browser state with a null browser separate', async () => {
         const state = new PlaywrightState();
 
@@ -638,6 +666,50 @@ describe('adoptContext', () => {
     });
 });
 
+describe('newContext', () => {
+    it('explains that an adopted context cannot get a sibling context', async () => {
+        const state = new PlaywrightState();
+        await state.adoptContext(makeMockContext([makeMockPage()]), { name: 'electron' });
+
+        await expect(newContext({ rawOptions: '{}', defaultTimeout: 0, traceFile: '' }, state)).rejects.toThrow(
+            'Cannot create a new context in electron, because its context is persistent or adopted. ' +
+                'Open a new browser first.',
+        );
+    });
+});
+
+describe('addBrowser', () => {
+    beforeEach(() => {
+        jest.clearAllMocks();
+    });
+
+    it('removes a page that existed when the browser was connected once it closes', async () => {
+        const listeners: Record<string, () => void> = {};
+        const page = { on: (event: string, listener: () => void) => (listeners[event] = listener), video: () => null };
+        const context = makeMockContext([page]);
+        const browser = { version: () => '1', contexts: () => [context] } as any;
+        const state = new PlaywrightState();
+
+        await state.addBrowser({ browser, browserType: 'chromium', headless: true });
+        listeners['close']?.();
+
+        expect(state.activeBrowser?.context?.pageStack).toEqual([]);
+    });
+
+    it('indexes a page once when the context reports it while the browser is being added', async () => {
+        const page = makeMockPage();
+        const context = makeMockContext([page]);
+        context.on = (event: string, listener: (page: unknown) => void) => event === 'page' && listener(page);
+        const browser = { version: () => '1', contexts: () => [context] } as any;
+        const state = new PlaywrightState();
+
+        await state.addBrowser({ browser, browserType: 'chromium', headless: true });
+        await new Promise((resolve) => setTimeout(resolve, 0));
+
+        expect(state.activeBrowser?.context?.pageStack.map((indexed) => indexed.p)).toEqual([page]);
+    });
+});
+
 describe('extensionKeywordCall', () => {
     beforeEach(() => {
         jest.clearAllMocks();
@@ -646,11 +718,7 @@ describe('extensionKeywordCall', () => {
     it('lets an extension function adopt a context it created', async () => {
         const state = new PlaywrightState();
         const context = makeMockContext([makeMockPage()]);
-        async function openOwnContext(
-            defaultTimeout: number,
-            adoptContext: (c: unknown, options?: unknown) => Promise<unknown>,
-        ) {
-            context.setDefaultTimeout(defaultTimeout);
+        async function openOwnContext(adoptContext: (c: unknown, options?: unknown) => Promise<unknown>) {
             return adoptContext(context, { name: 'own' });
         }
         state.extensions.push({ openOwnContext } as any);
@@ -660,10 +728,7 @@ describe('extensionKeywordCall', () => {
             {
                 name: 'openOwnContext',
                 arguments: JSON.stringify({
-                    arguments: [
-                        ['defaultTimeout', 5000],
-                        ['adoptContext', 'RESERVED'],
-                    ],
+                    arguments: [['adoptContext', 'RESERVED']],
                 }),
             },
             call,
@@ -674,6 +739,30 @@ describe('extensionKeywordCall', () => {
         expect(adopted.browserId).toBe(state.activeBrowser?.id);
         expect(state.activeBrowser?.context?.c).toBe(context);
         expect(state.activeBrowser?.name).toBe('own');
+    });
+
+    it('gives an adopted context the default timeout of the call', async () => {
+        const state = new PlaywrightState();
+        const context = makeMockContext([makeMockPage()]);
+        async function openOwnContext(adoptContext: (c: unknown) => Promise<unknown>) {
+            return adoptContext(context);
+        }
+        state.extensions.push({ openOwnContext } as any);
+
+        await extensionKeywordCall(
+            {
+                name: 'openOwnContext',
+                arguments: JSON.stringify({
+                    arguments: [
+                        ['adoptContext', 'RESERVED'],
+                        ['defaultTimeout', 5000],
+                    ],
+                }),
+            },
+            { write: jest.fn() } as any,
+            state,
+        );
+
         expect(context.setDefaultTimeout).toHaveBeenCalledWith(5000);
     });
 

node/playwright-wrapper/playwright-state.ts

@@ -174,7 +174,7 @@ export async function extensionKeywordCall(
     apiArguments.set('logger', (msg: string) => call.write(jsonResponse('', msg)));
     apiArguments.set('playwright', playwright);
     apiArguments.set('adoptContext', (context: BrowserContext, options?: AdoptContextOptions) =>
-        state.adoptContext(context, options),
+        state.adoptContext(context, options, namedArguments.defaultTimeout as number | undefined),
     );
     // Injected values can be null, for example the browser of an adopted context.
     const functionArguments = argNames.map((argName) =>
@@ -525,7 +525,7 @@ export class PlaywrightState {
         );
     }
 
-    public addBrowser(browserAndConfs: BrowserAndConfs): BrowserState {
+    public async addBrowser(browserAndConfs: BrowserAndConfs): Promise<BrowserState> {
         const adding_browser = browserAndConfs.browser;
         logger.info(`Adding browser to stack: ${browserAndConfs.browserType}, version: ${adding_browser?.version()}`);
         let browserState = this.browserStack.find((b) => b.browser === adding_browser);
@@ -538,26 +538,25 @@ export class PlaywrightState {
         if (browserContexts !== undefined) {
             logger.info(`Adding ${browserContexts.length} contexts to browser`);
             for (const context of browserContexts) {
-                const indexedPages = context.pages().map((page) => indexedPage(page));
-                logger.info(`Adding ${indexedPages.length} pages to context`);
-                const indexedContext = {
-                    id: `context=${uuidv4()}`,
-                    c: context,
-                    pageStack: indexedPages,
-                    options: {},
-                    traceFile: '',
-                };
-                indexedContext.c.on('page', async (page) => {
-                    indexedContext.pageStack.unshift(await _newPage(indexedContext, page));
-                });
-                browserState?.pushContext(indexedContext);
+                const indexedContext = await _createIndexedContext(context, undefined, '', {});
+                for (const page of context.pages()) {
+                    if (!indexedContext.pageStack.some((indexed) => indexed.p === page)) {
+                        indexedContext.pageStack.push(await _newPage(indexedContext, page));
+                    }
+                }
+                logger.info(`Adding ${indexedContext.pageStack.length} pages to context`);
+                browserState.pushContext(indexedContext);
             }
         }
         this.browserStack.push(browserState);
         return browserState;
     }
 
-    public async adoptContext(context: BrowserContext, options: AdoptContextOptions = {}): Promise<AdoptedContext> {
+    public async adoptContext(
+        context: BrowserContext,
+        options: AdoptContextOptions = {},
+        defaultTimeout?: number,
+    ): Promise<AdoptedContext> {
         const browserState = new BrowserState({
             browser: null,
             name: options.name ?? 'adopted',
@@ -566,13 +565,19 @@ export class PlaywrightState {
         browserState.onClose = options.onClose;
         const indexedContext = await _indexContextWithPages(
             context,
-            undefined,
+            defaultTimeout,
             options.tracing ?? '',
             options.contextOptions,
         );
         browserState.pushContext(indexedContext);
         this.browserStack.push(browserState);
-        return { browserId: browserState.id, contextId: indexedContext.id, pageId: browserState.page?.id };
+        const page = browserState.page;
+        return {
+            browserId: browserState.id,
+            contextId: indexedContext.id,
+            pageId: page?.id,
+            videoPath: (await page?.p.video()?.path()) ?? null,
+        };
     }
 
     public addBrowserServer = (browserServer: BrowserServer): void => {
@@ -694,6 +699,7 @@ type AdoptedContext = {
     browserId: Uuid;
     contextId: Uuid;
     pageId: Uuid | undefined;
+    videoPath: string | null;
 };
 
 type FailedPage = {
@@ -962,7 +968,10 @@ export async function newContext(
     logger.info(`Trace file: ${traceFile}`);
 
     if (browserState.browser.browser === null) {
-        throw new Error('Trying to create a new context when a persistentContext is active');
+        throw new Error(
+            `Cannot create a new context in ${browserState.browser.name}, because its context is persistent or ` +
+                'adopted. Open a new browser first.',
+        );
     }
     const indexedContext = await _createIndexedContext(
         await browserState.browser.browser.newContext(options),
@@ -1003,7 +1012,7 @@ export async function newBrowser(request: Request_Browser, openBrowsers: Playwri
         options['timeout'] as number,
         options,
     );
-    const browserState = openBrowsers.addBrowser(browserAndConfs);
+    const browserState = await openBrowsers.addBrowser(browserAndConfs);
     return stringResponse(browserState.id, 'Successfully created browser with options: ' + JSON.stringify(options));
 }
 
@@ -1044,7 +1053,7 @@ export async function newPersistentContext(
         browser: null,
         headless: options?.headless || true,
     } as BrowserAndConfs;
-    openBrowsers.addBrowser(browserAndConfs);
+    await openBrowsers.addBrowser(browserAndConfs);
     const browserState = await openBrowsers.getOrCreateActiveBrowser(null, timeout);
     if (browserState.newBrowser === true) {
         throw new Error('A new browser was created in error while trying to create a persistent context');
@@ -1073,7 +1082,7 @@ export async function connectToBrowser(
     const connectCDP = request.connectCDP;
     const timeout = request.timeout;
     const browserAndConfs = await _connectBrowser(browserType, url, connectCDP, timeout);
-    const browserState = openBrowsers.addBrowser(browserAndConfs);
+    const browserState = await openBrowsers.addBrowser(browserAndConfs);
     return stringResponse(browserState.id, 'Successfully connected to browser');
 }