From 101662765c26001e98edce9f20b2597c7f24ebb4 Mon Sep 17 00:00:00 2001
From: Ross Wollman <rwoll@users.noreply.github.com>
Date: Mon, 16 Aug 2021 12:49:10 -0700
Subject: [PATCH] api(cdp): newCDPSession accepts frames, too (#8157)

Without this, Playwright's CDP feature leaves unreachable
targets (namely OOPIFs).

This change allows for more advanced experimentation in user-land
without relying on out-of-band CDP connections and clients.

Now you can, for example, call `DOM.getDocument` on the
page OR main frame, observe there is an iframe node with
no `contentDocument` (i.e. OOPIF), make note of the referenced
`frameId`, and then iterate of page.frames() calling `Target.getInfo`
on each to link the Playwright Frame with the CDP `frameId` and
then recurse.

Relates #8113
---
 docs/src/api/class-browsercontext.md        |  9 +++---
 src/client/browserContext.ts                |  8 ++++--
 src/dispatchers/browserContextDispatcher.ts |  5 +++-
 src/protocol/channels.ts                    |  6 ++--
 src/protocol/protocol.yml                   |  3 +-
 src/protocol/validator.ts                   |  3 +-
 src/server/chromium/crBrowser.ts            | 17 ++++++++---
 tests/chromium/oopif.spec.ts                | 15 ++++++++++
 tests/chromium/session.spec.ts              | 31 +++++++++++++++++++--
 types/types.d.ts                            |  4 +--
 10 files changed, 81 insertions(+), 20 deletions(-)

diff --git a/docs/src/api/class-browsercontext.md b/docs/src/api/class-browsercontext.md
index 960d527374..97d29a1230 100644
--- a/docs/src/api/class-browsercontext.md
+++ b/docs/src/api/class-browsercontext.md
@@ -349,7 +349,7 @@ context.clear_permissions()
 ```csharp
 var context = await browser.NewContextAsync();
 await context.GrantPermissionsAsync(new[] { "clipboard-read" });
-// Alternatively, you can use the helper class ContextPermissions 
+// Alternatively, you can use the helper class ContextPermissions
 //  to specify the permissions...
 // do stuff ...
 await context.ClearPermissionsAsync();
@@ -589,7 +589,7 @@ await page.SetContentAsync("<script>\n" +
   "<div>Or click me</div>\n");
 
 await page.ClickAsync("div");
-// Note: it makes sense to await the result here, because otherwise, the context 
+// Note: it makes sense to await the result here, because otherwise, the context
 //  gets closed and the binding function will throw an exception.
 Assert.Equal("Click me", await result.Task);
 ```
@@ -834,9 +834,10 @@ CDP sessions are only supported on Chromium-based browsers.
 Returns the newly created session.
 
 ### param: BrowserContext.newCDPSession.page
-- `page` <[Page]>
+- `page` <[Page]|[Frame]>
 
-Page to create new session for.
+Target to create new session for. For backwards-compatability, this parameter is
+named `page`, but it can be a `Page` or `Frame` type.
 
 ## async method: BrowserContext.newPage
 - returns: <[Page]>
diff --git a/src/client/browserContext.ts b/src/client/browserContext.ts
index 5f477018b4..fd3366f891 100644
--- a/src/client/browserContext.ts
+++ b/src/client/browserContext.ts
@@ -16,6 +16,7 @@
  */
 
 import { Page, BindingCall } from './page';
+import { Frame } from './frame';
 import * as network from './network';
 import * as channels from '../protocol/channels';
 import fs from 'fs';
@@ -308,9 +309,12 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel,
     return [...this._serviceWorkers];
   }
 
-  async newCDPSession(page: Page): Promise<api.CDPSession> {
+  async newCDPSession(page: Page | Frame): Promise<api.CDPSession> {
+    // channelOwner.ts's validation messages don't handle the pseudo-union type, so we're explicit here
+    if (!(page instanceof Page) && !(page instanceof Frame))
+      throw new Error('page: expected Page or Frame');
     return this._wrapApiCall(async (channel: channels.BrowserContextChannel) => {
-      const result = await channel.newCDPSession({ page: page._channel });
+      const result = await channel.newCDPSession(page instanceof Page ? { page: page._channel } : { frame: page._channel });
       return CDPSession.from(result.session);
     });
   }
diff --git a/src/dispatchers/browserContextDispatcher.ts b/src/dispatchers/browserContextDispatcher.ts
index 535e15c6c4..650969c452 100644
--- a/src/dispatchers/browserContextDispatcher.ts
+++ b/src/dispatchers/browserContextDispatcher.ts
@@ -17,6 +17,7 @@
 import { BrowserContext } from '../server/browserContext';
 import { Dispatcher, DispatcherScope, lookupDispatcher } from './dispatcher';
 import { PageDispatcher, BindingCallDispatcher, WorkerDispatcher } from './pageDispatcher';
+import { FrameDispatcher } from './frameDispatcher';
 import * as channels from '../protocol/channels';
 import { RouteDispatcher, RequestDispatcher, ResponseDispatcher } from './networkDispatchers';
 import { CRBrowserContext } from '../server/chromium/crBrowser';
@@ -176,8 +177,10 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
   async newCDPSession(params: channels.BrowserContextNewCDPSessionParams): Promise<channels.BrowserContextNewCDPSessionResult> {
     if (!this._object._browser.options.isChromium)
       throw new Error(`CDP session is only available in Chromium`);
+    if (!params.page && !params.frame || params.page && params.frame)
+      throw new Error(`CDP session must be initiated with either Page or Frame, not none or both`);
     const crBrowserContext = this._object as CRBrowserContext;
-    return { session: new CDPSessionDispatcher(this._scope, await crBrowserContext.newCDPSession((params.page as PageDispatcher)._object)) };
+    return { session: new CDPSessionDispatcher(this._scope, await crBrowserContext.newCDPSession((params.page ? params.page as PageDispatcher : params.frame as FrameDispatcher)._object)) };
   }
 
   async tracingStart(params: channels.BrowserContextTracingStartParams): Promise<channels.BrowserContextTracingStartResult> {
diff --git a/src/protocol/channels.ts b/src/protocol/channels.ts
index 4af0bbb277..1d2d2b5a50 100644
--- a/src/protocol/channels.ts
+++ b/src/protocol/channels.ts
@@ -845,10 +845,12 @@ export type BrowserContextRecorderSupplementEnableOptions = {
 };
 export type BrowserContextRecorderSupplementEnableResult = void;
 export type BrowserContextNewCDPSessionParams = {
-  page: PageChannel,
+  page?: PageChannel,
+  frame?: FrameChannel,
 };
 export type BrowserContextNewCDPSessionOptions = {
-
+  page?: PageChannel,
+  frame?: FrameChannel,
 };
 export type BrowserContextNewCDPSessionResult = {
   session: CDPSessionChannel,
diff --git a/src/protocol/protocol.yml b/src/protocol/protocol.yml
index d84a70c815..063000673f 100644
--- a/src/protocol/protocol.yml
+++ b/src/protocol/protocol.yml
@@ -629,7 +629,8 @@ BrowserContext:
 
     newCDPSession:
       parameters:
-        page: Page
+        page: Page?
+        frame: Frame?
       returns:
         session: CDPSession
 
diff --git a/src/protocol/validator.ts b/src/protocol/validator.ts
index 941e5da705..ed92a9d3c2 100644
--- a/src/protocol/validator.ts
+++ b/src/protocol/validator.ts
@@ -406,7 +406,8 @@ export function createScheme(tChannel: (name: string) => Validator): Scheme {
     outputFile: tOptional(tString),
   });
   scheme.BrowserContextNewCDPSessionParams = tObject({
-    page: tChannel('Page'),
+    page: tOptional(tChannel('Page')),
+    frame: tOptional(tChannel('Frame')),
   });
   scheme.BrowserContextTracingStartParams = tObject({
     name: tOptional(tString),
diff --git a/src/server/chromium/crBrowser.ts b/src/server/chromium/crBrowser.ts
index d9ed6cb29e..4797b948b4 100644
--- a/src/server/chromium/crBrowser.ts
+++ b/src/server/chromium/crBrowser.ts
@@ -20,6 +20,7 @@ import { assertBrowserContextIsNotOwned, BrowserContext, validateBrowserContextO
 import { assert } from '../../utils/utils';
 import * as network from '../network';
 import { Page, PageBinding, PageDelegate, Worker } from '../page';
+import { Frame } from '../frames';
 import { ConnectionTransport } from '../transport';
 import * as types from '../types';
 import { ConnectionEvents, CRConnection, CRSession } from './crConnection';
@@ -503,10 +504,18 @@ export class CRBrowserContext extends BrowserContext {
     return Array.from(this._browser._serviceWorkers.values()).filter(serviceWorker => serviceWorker._browserContext === this);
   }
 
-  async newCDPSession(page: Page): Promise<CRSession> {
-    if (!(page instanceof Page))
-      throw new Error('page: expected Page');
-    const targetId = (page._delegate as CRPage)._targetId;
+  async newCDPSession(page: Page | Frame): Promise<CRSession> {
+    let targetId: string | null = null;
+    if (page instanceof Page) {
+      targetId = (page._delegate as CRPage)._targetId;
+    } else if (page instanceof Frame) {
+      const session = (page._page._delegate as CRPage)._sessions.get(page._id);
+      if (!session) throw new Error(`This frame does not have a separate CDP session, it is a part of the parent frame's session`);
+      targetId = session._targetId;
+    } else {
+      throw new Error('page: expected Page or Frame');
+    }
+
     const rootSession = await this._browser._clientRootSession();
     const { sessionId } = await rootSession.send('Target.attachToTarget', { targetId, flatten: true });
     return this._browser._connection.session(sessionId)!;
diff --git a/tests/chromium/oopif.spec.ts b/tests/chromium/oopif.spec.ts
index 9b1b23a2b9..697a2f058e 100644
--- a/tests/chromium/oopif.spec.ts
+++ b/tests/chromium/oopif.spec.ts
@@ -292,6 +292,21 @@ it('should click', async function({page, browser, server}) {
   expect(await handle1.evaluate(() => window['_clicked'])).toBe(true);
 });
 
+it('should allow cdp sessions on oopifs', async function({page, browser, server}) {
+  await page.goto(server.PREFIX + '/dynamic-oopif.html');
+  expect(await countOOPIFs(browser)).toBe(1);
+  expect(page.frames().length).toBe(2);
+  expect(await page.frames()[1].evaluate(() => '' + location.href)).toBe(server.CROSS_PROCESS_PREFIX + '/grid.html');
+
+  const parentCDP = await page.context().newCDPSession(page.frames()[0]);
+  const parent = await parentCDP.send('DOM.getDocument', { pierce: true, depth: -1 });
+  expect(JSON.stringify(parent)).not.toContain('./digits/1.png');
+
+  const oopifCDP = await page.context().newCDPSession(page.frames()[1]);
+  const oopif = await oopifCDP.send('DOM.getDocument', { pierce: true, depth: -1});
+  expect(JSON.stringify(oopif)).toContain('./digits/1.png');
+});
+
 async function countOOPIFs(browser) {
   const browserSession = await browser.newBrowserCDPSession();
   const oopifs = [];
diff --git a/tests/chromium/session.spec.ts b/tests/chromium/session.spec.ts
index e9da7cbdc3..ab01e23323 100644
--- a/tests/chromium/session.spec.ts
+++ b/tests/chromium/session.spec.ts
@@ -37,10 +37,15 @@ it('should send events', async function({page, server}) {
   expect(events.length).toBe(1);
 });
 
-it('should only accept a page', async function({page}) {
-  // @ts-expect-error newCDPSession expects a Page
+it('should only accept a page or frame', async function({page}) {
+  // @ts-expect-error newCDPSession expects a Page or Frame
   const error = await page.context().newCDPSession(page.context()).catch(e => e);
-  expect(error.message).toContain('page: expected Page');
+  expect(error.message).toContain('page: expected Page or Frame');
+
+  // non-channelable types hit validation at a different layer
+  // @ts-expect-error newCDPSession expects a Page or Frame
+  const errorAlt = await page.context().newCDPSession({}).catch(e => e);
+  expect(errorAlt.message).toContain('page: expected Page or Frame');
 });
 
 it('should enable and disable domains independently', async function({page}) {
@@ -88,6 +93,26 @@ it('should throw nice errors', async function({page}) {
   }
 });
 
+it('should work with main frame', async function({page}) {
+  const client = await page.context().newCDPSession(page.mainFrame());
+
+  await Promise.all([
+    client.send('Runtime.enable'),
+    client.send('Runtime.evaluate', { expression: 'window.foo = "bar"' })
+  ]);
+  const foo = await page.evaluate(() => window['foo']);
+  expect(foo).toBe('bar');
+});
+
+it('should throw if target is part of main', async function({server, page}){
+  await page.goto(server.PREFIX + '/frames/one-frame.html');
+  expect(page.frames()[0].url()).toContain('/frames/one-frame.html');
+  expect(page.frames()[1].url()).toContain('/frames/frame.html');
+
+  const error = await page.context().newCDPSession(page.frames()[1]).catch(e => e);
+  expect(error.message).toContain(`This frame does not have a separate CDP session, it is a part of the parent frame's session`);
+});
+
 browserTest('should not break page.close()', async function({browser}) {
   const context = await browser.newContext();
   const page = await context.newPage();
diff --git a/types/types.d.ts b/types/types.d.ts
index dadb2d5707..48a20a9db9 100644
--- a/types/types.d.ts
+++ b/types/types.d.ts
@@ -5482,9 +5482,9 @@ export interface BrowserContext {
    * > NOTE: CDP sessions are only supported on Chromium-based browsers.
    *
    * Returns the newly created session.
-   * @param page Page to create new session for.
+   * @param page Target to create new session for. For backwards-compatability, this parameter is named `page`, but it can be a `Page` or `Frame` type.
    */
-  newCDPSession(page: Page): Promise<CDPSession>;
+  newCDPSession(page: Page|Frame): Promise<CDPSession>;
 
   /**
    * Creates a new page in the browser context.