feat(chromium): connect to a browser over cdp #5207

JoelEinbinder · 2021-01-28T18:43:04Z

No description provided.

dgozman · 2021-01-28T19:07:21Z

src/server/chromium/chromium.ts

+        ...uiOptions,
+        name: 'chromium',
+        isChromium: true,
+        headful: true,


Why headful?

To ensure we have a default context/page.

I think we only need persistent for that to happen. Let's not lie to ourselves?

Oh I thought this was in the test. Yes I have no idea why this code is here.

dgozman · 2021-01-28T19:08:47Z

src/client/browserType.ts

-      });
+  async _connectCDP(params: ConnectOptions): Promise<Browser> {
+    if (this.name() !== 'chromium')
+      throw new Error('Connecting over CDP is onlly supported for the Chromium BrowserType');


'Connecting over CDP is only supported in Chromium'

I started with this text, but I realized that people reading it are unlikely to have chromium. Instead they have chrome, edge, webview etc. So I was trying to emphasize that they need to use playwright.chromium to connect.

dgozman · 2021-01-28T19:10:06Z

docs/src/api/class-browsertype.md

@@ -62,6 +62,11 @@ This methods attaches Playwright to an existing browser instance.
  - `logger` <[Logger]> Logger sink for Playwright logging. Optional.
  - `timeout` <[float]> Maximum time in milliseconds to wait for the connection to be established. Defaults to
    `30000` (30 seconds). Pass `0` to disable timeout.
+  - `protocol` <"playwright"|"cdp"> To use a Chrome DevTools Protocol endpoint, pass "cdp". Defaults to "playwright".


I'd call this cdp: boolean | undefined, so that you only use it if you know what you are doing. Otherwise you have some other legit option in there.

I am worried that we will want to put some other protocols in here in the future. WebDriver. Firefox cdp. WebInspector protocol for iPhones.

What do you all think about ChromiumBrowser.connectOverCDP(wsEndpoint, options)? Are we moving away from ChromiumBrowser it is ok?

We have ChromiumBrowserContext but not ChromiumBrowser. I'd rather have a small option than a big method.

docs/src/api/class-browsertype.md

dgozman · 2021-01-28T19:11:27Z

src/dispatchers/browserTypeDispatcher.ts

+    const browser = await this._object.cdpConnect(params.wsEndpoint, params, params.timeout);
+    return {
+      browser: new BrowserDispatcher(this._scope, browser),
+      defaultContext: new BrowserContextDispatcher(this._scope, browser._defaultContext!),


How are you sure there is a default context? If I launched with Playwright, there might be none.

dgozman · 2021-01-28T19:20:01Z

test/chromium/chromium.spec.ts

+
+  it('should connect to an existing cdp session', async ({browserType, testWorkerIndex, browserOptions, createUserDataDir }) => {
+    const port = 9339 + testWorkerIndex;
+    const spawnedProcess = spawn(


Why don't you use chromium.launch()? It will take care of killing the browser for you. Otherwise, you leave stray chrome when the test times out.

Our chromium.launch specifies --remote-debugging-pipe. You can't launch headful chrome with both pipe and port. In puppeteer we had code that switched to use a websocket if port was specified, but I didn't feel like reviving it for this test.

Passing both pipe and port works in headless. With the next roll, it should also work in headful. Let's do it.

dgozman · 2021-02-07T23:17:25Z

docs/src/api/class-browsertype.md

+
+### param: BrowserType.connectOverCDP.params
+- `params` <[Object]>
+  - `wsEndpoint` <[string]> A browser websocket endpoint to connect to.


nit: browser websocket endpoint -> cdp webscoket endpoint?

dgozman · 2021-02-07T23:19:41Z

src/dispatchers/browserTypeDispatcher.ts

+    const browser = await this._object.cdpConnect(params.wsEndpoint, params, params.timeout);
+    return {
+      browser: new BrowserDispatcher(this._scope, browser),
+      defaultContext: browser._defaultContext ? new BrowserContextDispatcher(this._scope, browser._defaultContext!) : undefined,


nit: no need for exclamation mark.

dgozman · 2021-02-07T23:20:16Z

src/protocol/protocol.yml

@@ -391,6 +391,14 @@ BrowserType:
      returns:
        context: BrowserContext

+    cdpConnect:


Let's call this connectOverCDP.

dgozman · 2021-02-07T23:31:22Z

test/chromium/chromium.spec.ts

+
+  it('should connect to an existing cdp session', async ({browserType, testWorkerIndex, browserOptions, createUserDataDir }) => {
+    const port = 9339 + testWorkerIndex;
+    const spawnedProcess = spawn(


Passing both pipe and port works in headless. With the next roll, it should also work in headful. Let's do it.

src/client/browserType.ts

dgozman · 2021-02-08T20:11:24Z

src/server/chromium/chromium.ts

+        ...uiOptions,
+        name: 'chromium',
+        isChromium: true,
+        headful: true,


I think we only need persistent for that to happen. Let's not lie to ourselves?

tjenkinson · 2022-10-14T09:24:55Z

👋 this might be a better place for my question in #18052 (comment)

I'm wondering what the difference is to the wsEndpoint in connect(). What protocol does connect() use?

Thanks

dgozman reviewed Jan 28, 2021

View reviewed changes

JoelEinbinder force-pushed the connect_cdp branch from 4f7018c to 4c5d821 Compare February 6, 2021 00:50

dgozman reviewed Feb 7, 2021

View reviewed changes

JoelEinbinder force-pushed the connect_cdp branch 3 times, most recently from 6b674a1 to 7161813 Compare February 8, 2021 16:55

dgozman approved these changes Feb 8, 2021

View reviewed changes

JoelEinbinder added 2 commits February 10, 2021 13:35

1

165574a

mark it as headless for now

225db2a

JoelEinbinder force-pushed the connect_cdp branch from 7161813 to 225db2a Compare February 10, 2021 21:35

JoelEinbinder merged commit dca70ab into microsoft:master Feb 10, 2021

dgozman mentioned this pull request Feb 11, 2021

[Question] Ability to connect to running chromium instance? #5317

Closed

mxschmitt mentioned this pull request Feb 17, 2021

feat: add support for using existing cdp websocket in launchServer #4344

Closed

YusukeIwaki mentioned this pull request Apr 18, 2021

support BrowserType#connectOverCDP YusukeIwaki/playwright-ruby-client#56

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(chromium): connect to a browser over cdp #5207

feat(chromium): connect to a browser over cdp #5207

JoelEinbinder commented Jan 28, 2021

dgozman Jan 28, 2021

JoelEinbinder Jan 28, 2021

dgozman Feb 8, 2021

JoelEinbinder Feb 8, 2021

dgozman Jan 28, 2021

JoelEinbinder Jan 28, 2021

dgozman Jan 28, 2021

JoelEinbinder Jan 28, 2021

pavelfeldman Jan 29, 2021 •

edited

Loading

JoelEinbinder Jan 29, 2021

dgozman Jan 28, 2021

dgozman Jan 28, 2021

JoelEinbinder Jan 28, 2021

dgozman Feb 7, 2021

dgozman Feb 7, 2021

dgozman Feb 7, 2021

dgozman Feb 7, 2021

dgozman Feb 7, 2021

dgozman Feb 8, 2021

tjenkinson commented Oct 14, 2022

feat(chromium): connect to a browser over cdp #5207

feat(chromium): connect to a browser over cdp #5207

Conversation

JoelEinbinder commented Jan 28, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pavelfeldman Jan 29, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjenkinson commented Oct 14, 2022

pavelfeldman Jan 29, 2021 •

edited

Loading