Closed Bug 1706581 Opened 8 months ago Closed 2 months ago

Write WebSocket port and main browser target to "DevToolsActivePort" file in profile directory

Categories

(Remote Protocol :: CDP, enhancement, P3)

enhancement

Tracking

(firefox95 fixed)

RESOLVED FIXED
95 Branch
Tracking Status
firefox95 --- fixed

People

(Reporter: xmo, Assigned: whimboo)

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:88.0) Gecko/20100101 Firefox/88.0

Steps to reproduce:

We've got internal tooling which sadly uses CDP to interact with, well, Chrome. I was recently made aware of Firefox having (at least some) support for CDP and decided to take a look at it.

The issue here is retrieving the CDP port when using --remote-debugging-port 0, having to get it from the browser's stdout/stderr is complicated and unreliable. To get around this issue, Chrome creates a DevToolsActivePort file in the "user-data-dir".

Firefox doesn't seem to create such a file in the profile directory (which seems like the closest analogue to the user-data-dir, though more reliably isolated than Chrome's). It would be very useful for that to happen.

Thank you for filing this bug!

We weren't aware that Chrome is actually creating this file in case 0 is passed-in for this argument. It's indeed a good way to indicate the server's port and the browser endpoint. That way clients could easily pick-it up. But it's not created when Chrome DevTools is using the default or a hard-coded port.

The content looks like:

➜ cat ./chrome-profile/DevToolsActivePort
59053
/devtools/browser/637d546f-cb16-4cf1-a890-7a1f238c0d1f%

Also I don't see that Puppeteer is using this file. Mathias, do you know why? Maybe this decision has been made because the file doesn't always exist? If that could be changed and the file always written, the parsing of the WebSocket URL via stderr wouldn't be necessary.

Flags: needinfo?(mathias)

We weren't aware that Chrome is actually creating this file in case 0 is passed-in for this argument.

Note that I have no idea whether Chrome creates the file if a hard-coded port is provided, I never looked. It might have to do with other options e.g. https://stackoverflow.com/questions/50642308/webdriverexception-unknown-error-devtoolsactiveport-file-doesnt-exist-while-t mentions --no-sandbox, --disable-dev-shm-usage and --headless. Our runner did use all three before we added support for port 0.

Additionally I would recommend to use pipe in implementation fd 3 and 4 which chrome supports? I think using ip:port is kinda racy etc.

(In reply to shirshak55 from comment #3)

Additionally I would recommend to use pipe in implementation fd 3 and 4 which chrome supports? I think using ip:port is kinda racy etc.

Using --remote-debugging-port 0 specifically avoids race conditions, Chrome asks the OS for a free ephemeral port then the client can retrieve said port from DevToolsActivePort.

And while --remote-debugging-pipe uses less resources (does not need a local socket) it also requires an exposed fork/join model or preexec facility e.g. this is the horror show to use it in Rust, where --remote-debugging-port is just "start chrome, read port from file, pass port to your websocket library".

Actually that is not horror if we try to do same thing with ip:port there will lot of unsafe in websocket/http crates right? --remote-debugging-port 0 still has some issue. As this is already discussed issue here https://groups.google.com/a/chromium.org/g/headless-dev/c/n1cLc4qSfrM I still think pipe is best way to connect.

Google Chrome implementation patch: https://groups.google.com/a/chromium.org/g/devtools-reviews/c/wnCpqPbWqiU/m/1T95Os2sBAAJ
Node CDP client issue: https://github.com/cyrus-and/chrome-remote-interface/issues/381

The bug concerning pipe support is https://bugzilla.mozilla.org/show_bug.cgi?id=1704552; let's keep this one focused on the DevToolsActivePort issue.

(In reply to xmo from comment #0)

The issue here is retrieving the CDP port when using --remote-debugging-port 0, having to get it from the browser's stdout/stderr is complicated and unreliable. To get around this issue, Chrome creates a DevToolsActivePort file in the "user-data-dir".

Could I ask how you retrieve the path of the profile, especially if it's one that Puppeteer created temporarily? The BrowserRunner only has a "private" property for it. Or are you using a fixed profile path that you push into the launcher? Thanks.

Flags: needinfo?(mathias) → needinfo?(xmo)

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #7)

(In reply to xmo from comment #0)
Could I ask how you retrieve the path of the profile, especially if it's one that Puppeteer created temporarily? The BrowserRunner only has a "private" property for it. Or are you using a fixed profile path that you push into the launcher? Thanks.

The latter, we actually drive chrome "by hand", so part of the process is to mkdtemp a directory and that as Chrome's user-data-dir, this is necessary to ensure our instances are properly isolated and "clean" but a positive side-effect is that's where we get access to DevToolsActivePort.

Checking the log, this specific use is credited to this comment on the chromium bug tracker.

Flags: needinfo?(xmo)

and pass that as

Thanks. So I had a brief look into the Chromium code and that seems to be the code that is triggering the write of this file:

https://source.chromium.org/chromium/chromium/src/+/main:content/browser/devtools/devtools_http_handler.cc;l=283-294;drc=0df7f36ee31a800b865d2498e02e4f52ed03f217

The contents of this file looks like:

63392
/devtools/browser/fb8745fe-473a-4b52-a00a-8f41fdc215b1

It means on our side the required code needs to end-up after this line:
https://searchfox.org/mozilla-central/rev/01adc17c9a41d9f7975de170acc78634bd743609/remote/cdp/CDP.jsm#86

Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3
Summary: DevToolsActivePort file missing → Write WebSocket port and main browser target to "DevToolsActivePort" file in profile directory
Version: Firefox 88 → unspecified
Assignee: nobody → hskupin
Status: NEW → ASSIGNED

xmo, could you please check with a Ci build if it's working for you? To download it click the Artifacts tab at the bottom of the page, and then find the file target.tar.bz2. Thanks!

Flags: needinfo?(xmo)

Henrik, seems to work perfectly, after fixing one of the routes (we were relying on /json as an alias to json/list and Firefox apparently only has the latter -- which is fine) and providing an alternate slate of CLI options, the test harness launches.

Only trouble I had is I figured "safe mode" would be a good idea to ensure I had a clean slate and apparently the CDT stuff is an extension so it got disabled but there is no message so I got a bit confused there.

Flags: needinfo?(xmo)

(In reply to xmo from comment #13)

Henrik, seems to work perfectly, after fixing one of the routes (we were relying on /json as an alias to json/list and Firefox apparently only has the latter -- which is fine) and providing an alternate slate of CLI options, the test harness launches.

That's great to hear! So we can get it reviewed and landed. Regarding /json versus /json/list I can see difference here when opening the URLs in Chrome. But that's a different topic, which we might not add or change.

Only trouble I had is I figured "safe mode" would be a good idea to ensure I had a clean slate and apparently the CDT stuff is an extension so it got disabled but there is no message so I got a bit confused there.

What do you mean with CDT? I'm missing the context here. Also safe mode you should be able to use by specifying --safe-mode as argument to Firefox.

That's great to hear! So we can get it reviewed and landed. Regarding /json versus /json/list I can see difference here when opening the URLs in Chrome. But that's a different topic, which we might not add or change.

It's really not an issue (for me at least), I just changed our integration to use /json/list, if there are differences between the two they're probably not things we rely on.

What do you mean with CDT? I'm missing the context here. Also safe mode you should be able to use by specifying --safe-mode as argument to Firefox.

I meant that if I use --safe-mode then the devtools listening doesn't happen, no endpoint information gets printed and the file doesn't get written. I assume that's because RDP / CDP (sorry, mistype, not CDT) is treated as an extension.

Ah, yes. Thanks for mentioning that. Actually we haven't ported that code yet from Marionette. I just filed bug 1734619. If you are interested to work on it I'm happy to mentor. Shouldn't be too hard to get implemented.

Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bf2267c9d02b
[remote] Write CDP connection details to DevToolsActivePort file. r=webdriver-reviewers,jdescottes
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 95 Branch
You need to log in before you can comment on or make changes to this bug.