chore(bidi): add support for context locators

Author: hbenl

packages/playwright-core/src/server/bidi/bidiBrowser.ts

@@ -135,10 +135,12 @@ export class BidiBrowser extends Browser {
         if (!parentFrame)
           continue;
         page._session.addFrameBrowsingContext(event.context);
-        page._page.frameManager.frameAttached(event.context, parentFrameId);
-        const frame = page._page.frameManager.frame(event.context);
-        if (frame)
-          frame._url = event.url;
+        const frame = page._page.frameManager.frameAttached(event.context, parentFrameId);
+        frame._url = event.url;
+        page._getFrameNode(frame).then(node => {
+          const attributes = node?.value?.attributes;
+          frame._name = attributes?.name ?? attributes?.id ?? '';
+        });
         return;
       }
       return;

packages/playwright-core/src/server/bidi/bidiExecutionContext.ts

@@ -141,11 +141,11 @@ export class BidiExecutionContext implements js.ExecutionContextDelegate {
     };
   }
 
-  async remoteObjectForNodeId(context: dom.FrameExecutionContext, nodeId: bidi.Script.SharedReference): Promise<js.JSHandle> {
+  async remoteObjectForNodeId(context: dom.FrameExecutionContext, nodeId: bidi.Script.SharedReference): Promise<dom.ElementHandle> {
     const result = await this._remoteValueForReference(nodeId, true);
     if (!('handle' in result))
       throw new Error('Can\'t get remote object for nodeId');
-    return createHandle(context, result);
+    return createHandle(context, result) as dom.ElementHandle;
   }
 
   async contentFrameIdForFrame(handle: dom.ElementHandle) {

packages/playwright-core/src/server/bidi/bidiPage.ts

@@ -582,24 +582,24 @@ export class BidiPage implements PageDelegate {
     const parent = frame.parentFrame();
     if (!parent)
       throw new Error('Frame has been detached.');
-    const parentContext = await parent._mainContext();
-    const list = await parentContext.evaluateHandle(() => { return [...document.querySelectorAll('iframe,frame')]; });
-    const length = await list.evaluate(list => list.length);
-    let foundElement = null;
-    for (let i = 0; i < length; i++) {
-      const element = await list.evaluateHandle((list, i) => list[i], i);
-      const candidate = await element.contentFrame();
-      if (frame === candidate) {
-        foundElement = element;
-        break;
-      } else {
-        element.dispose();
-      }
-    }
-    list.dispose();
-    if (!foundElement)
+    const node = await this._getFrameNode(frame);
+    if (!node?.sharedId)
       throw new Error('Frame has been detached.');
-    return foundElement;
+    const parentFrameExecutionContext = await parent._mainContext();
+    return await toBidiExecutionContext(parentFrameExecutionContext).remoteObjectForNodeId(parentFrameExecutionContext, { sharedId: node.sharedId });
+  }
+
+  async _getFrameNode(frame: frames.Frame): Promise<bidi.Script.NodeRemoteValue | undefined> {
+    const parent = frame.parentFrame();
+    if (!parent)
+      return undefined;
+
+    const result = await this._session.send('browsingContext.locateNodes', {
+      context: parent._id,
+      locator: { type: 'context', value: { context: frame._id } },
+    });
+    const node = result.nodes[0];
+    return node;
   }
 
   shouldToggleStyleSheetToSyncAnimations(): boolean {

packages/playwright-core/src/server/frames.ts

@@ -213,7 +213,7 @@ export class FrameManager {
     this.removeChildFramesRecursively(frame);
     this.clearWebSockets(frame);
     frame._url = url;
-    frame._name = name;
+    frame._name = name || frame._name;
 
     let keepPending: DocumentInfo | undefined;
     const pendingDocument = frame.pendingDocument();

tests/config/utils.ts

@@ -43,6 +43,14 @@ export async function detachFrame(page: Page, frameId: string) {
   }, frameId);
 }
 
+export async function findFrameByName(page: Page, name: string) {
+  for (const frame of page.frames()) {
+    if (frame.parentFrame() && await (await frame.frameElement()).getAttribute('name') === name)
+      return frame;
+  }
+  return null;
+}
+
 export async function verifyViewport(page: Page, width: number, height: number) {
   // `expect` may clash in test runner tests if imported eagerly.
   const { expect } = require('@playwright/test');

tests/library/hit-target.spec.ts

@@ -15,6 +15,7 @@
  */
 
 import { contextTest as it, expect } from '../config/browserTest';
+import { findFrameByName } from '../config/utils';
 import type { ElementHandle } from 'playwright-core';
 
 declare const renderComponent;
@@ -290,7 +291,7 @@ it('should click into frame inside closed shadow root', async ({ page, server })
     </script>
   `);
 
-  const frame = page.frame({ name: 'myframe' });
+  const frame = await findFrameByName(page, 'myframe');
   await frame.locator('text=click me').click();
   expect(await page.evaluate('window.__clicked')).toBe(true);
 });

tests/page/elementhandle-bounding-box.spec.ts

@@ -30,8 +30,8 @@ it('should work', async ({ page, server, browserName, headless, isLinux }) => {
 it('should handle nested frames', async ({ page, server }) => {
   await page.setViewportSize({ width: 616, height: 500 });
   await page.goto(server.PREFIX + '/frames/nested-frames.html');
-  const nestedFrame = page.frames().find(frame => frame.name() === 'dos');
-  const elementHandle = await nestedFrame.$('div');
+  const nestedFrame = page.frameLocator('[name="2frames"]').frameLocator('[name=dos]');
+  const elementHandle = await nestedFrame.locator('div').elementHandle();
   const box = await elementHandle.boundingBox();
   expect(box).toEqual({ x: 24, y: 224, width: 268, height: 18 });
 });

tests/page/frame-frame-element.spec.ts

@@ -16,7 +16,7 @@
  */
 
 import { test as it, expect } from './pageTest';
-import { attachFrame } from '../config/utils';
+import { attachFrame, findFrameByName } from '../config/utils';
 
 it('should work @smoke', async ({ page, server }) => {
   await page.goto(server.EMPTY_PAGE);
@@ -71,7 +71,7 @@ it('should work inside closed shadow root', async ({ page, server, browserName }
     </script>
   `);
 
-  const frame = page.frame({ name: 'myframe' });
+  const frame = await findFrameByName(page, 'myframe');
   const element = await frame.frameElement();
   expect(await element.getAttribute('name')).toBe('myframe');
 });
@@ -87,7 +87,7 @@ it('should work inside declarative shadow root', async ({ page, server, browserN
       <span>footer</span>
     </div>
   `);
-  const frame = page.frame({ name: 'myframe' });
+  const frame = await findFrameByName(page, 'myframe');
   const element = await frame.frameElement();
   expect(await element.getAttribute('name')).toBe('myframe');
 });

tests/page/frame-hierarchy.spec.ts

@@ -35,8 +35,10 @@ function dumpFrames(frame: Frame, indentation: string = ''): string[] {
   return result;
 }
 
-it('should handle nested frames @smoke', async ({ page, server, isAndroid }) => {
+it('should handle nested frames @smoke', async ({ page, server, isAndroid, browserName, channel }) => {
   it.skip(isAndroid, 'No cross-process on Android');
+  it.skip(browserName === 'firefox' && channel?.startsWith('moz-firefox'), 'frame.name() is racy with BiDi');
+  it.skip(channel?.startsWith('bidi-chrom'), 'frame.name() is racy with BiDi');
 
   await page.goto(server.PREFIX + '/frames/nested-frames.html');
   expect(dumpFrames(page.mainFrame())).toEqual([
@@ -154,7 +156,10 @@ it('should report frame from-inside shadow DOM', async ({ page, server }) => {
   expect(page.frames()[1].url()).toBe(server.EMPTY_PAGE);
 });
 
-it('should report frame.name()', async ({ page, server }) => {
+it('should report frame.name()', async ({ page, server, browserName, channel }) => {
+  it.skip(browserName === 'firefox' && channel?.startsWith('moz-firefox'), 'frame.name() is racy with BiDi');
+  it.skip(channel?.startsWith('bidi-chrom'), 'frame.name() is racy with BiDi');
+
   await attachFrame(page, 'theFrameId', server.EMPTY_PAGE);
   await page.evaluate(url => {
     const frame = document.createElement('iframe');

tests/page/page-basic.spec.ts

@@ -94,7 +94,10 @@ it('page.close should work with window.close', async function({ page }) {
   await closedPromise;
 });
 
-it('page.frame should respect name', async function({ page }) {
+it('page.frame should respect name', async function({ page, browserName, channel }) {
+  it.skip(browserName === 'firefox' && channel?.startsWith('moz-firefox'), 'page.frame({ name }) is racy with BiDi');
+  it.skip(channel?.startsWith('bidi-chrom'), 'page.frame({ name }) is racy with BiDi');
+
   await page.setContent(`<iframe name=target></iframe>`);
   expect(page.frame({ name: 'bogus' })).toBe(null);
   const frame = page.frame({ name: 'target' });