[headless] Add Emulation.setPrimaryScreen DevTools method [chromium/src : main]
0 views
Skip to first unread message
Peter Kvitek (Gerrit)
Feb 25, 2026, 1:14:14 PMFeb 25
to Colin Blundell, Andrey Kosyakov, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org, mac-r...@chromium.org, ozone-...@chromium.org, roblia...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, headless...@chromium.org
Attention needed from Andrey Kosyakov and Colin Blundell
New activity on the change![]( "Open in Gerrit")
Related details
Attention is currently required from:
- Andrey Kosyakov
- Colin Blundell
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrey Kosyakov (Gerrit)
Feb 25, 2026, 1:37:53 PMFeb 25
to Peter Kvitek, Colin Blundell, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org, mac-r...@chromium.org, ozone-...@chromium.org, roblia...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, headless...@chromium.org
Attention needed from Colin Blundell and Peter Kvitek
Andrey Kosyakov voted and added 2 comments![]( "Open in Gerrit")
Votes added by Andrey Kosyakov
| Code-Review | +1 |
2 comments
File ui/display/headless/headless_screen_manager.h
Line 58, Patchset 8 (Latest):
static void SetPrimaryDisplay(display::DisplayList& display_list,Andrey Kosyakov . unresolved
nit: extract these into a different class (follow-up is fine).
File ui/display/headless/headless_screen_manager.cc
Line 146, Patchset 8 (Latest):
if (!delegate_) {Andrey Kosyakov . unresolved
Just `CHECK(delegate_)` instead?
Related details
Attention is currently required from:
- Colin Blundell
- Peter Kvitek
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Peter Kvitek (Gerrit)
Feb 25, 2026, 1:45:23 PMFeb 25
to Andrey Kosyakov, Colin Blundell, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org, mac-r...@chromium.org, ozone-...@chromium.org, roblia...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, headless...@chromium.org
Attention needed from Andrey Kosyakov and Colin Blundell
Peter Kvitek added 2 comments![]( "Open in Gerrit")
File ui/display/headless/headless_screen_manager.h
Line 58, Patchset 8 (Latest):
static void SetPrimaryDisplay(display::DisplayList& display_list,Andrey Kosyakov . unresolved
nit: extract these into a different class (follow-up is fine).
Peter Kvitek
Yup, these belong to `//ui/display/headless/headless_screen_util.h/cc`
File ui/display/headless/headless_screen_manager.cc
Line 146, Patchset 8 (Latest):
if (!delegate_) {Andrey Kosyakov . unresolved
Just `CHECK(delegate_)` instead?
Peter Kvitek
Scary... technically the `HeadlessScreenManager` instance can outlive the `display::Screen` instances that implement the delegate interface. Just ignoring the stray calls to these methods may be enough though.
Related details
Attention is currently required from:
- Andrey Kosyakov
- Colin Blundell
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrey Kosyakov (Gerrit)
Feb 25, 2026, 4:21:07 PMFeb 25
to Peter Kvitek, Colin Blundell, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org, mac-r...@chromium.org, ozone-...@chromium.org, roblia...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, headless...@chromium.org
Attention needed from Colin Blundell and Peter Kvitek
Andrey Kosyakov added 1 comment![]( "Open in Gerrit")
File ui/display/headless/headless_screen_manager.cc
Line 146, Patchset 8 (Latest):
if (!delegate_) {Andrey Kosyakov . unresolved
Peter KvitekJust `CHECK(delegate_)` instead?
Scary... technically the `HeadlessScreenManager` instance can outlive the `display::Screen` instances that implement the delegate interface. Just ignoring the stray calls to these methods may be enough though.
Andrey Kosyakov
You're not ignoring it, NOTREACHED() is fatal! Besides, we don't put code (`return` in this case) after NOTREACHED().
Related details
Attention is currently required from:
- Colin Blundell
- Peter Kvitek
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Peter Kvitek (Gerrit)
Feb 25, 2026, 4:30:08 PMFeb 25
to Andrey Kosyakov, Colin Blundell, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org, mac-r...@chromium.org, ozone-...@chromium.org, roblia...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, headless...@chromium.org
Attention needed from Andrey Kosyakov and Colin Blundell
Peter Kvitek added 1 comment![]( "Open in Gerrit")
File ui/display/headless/headless_screen_manager.cc
Line 146, Patchset 8 (Latest):
if (!delegate_) {Andrey Kosyakov . unresolved
Peter KvitekJust `CHECK(delegate_)` instead?
Andrey KosyakovScary... technically the `HeadlessScreenManager` instance can outlive the `display::Screen` instances that implement the delegate interface. Just ignoring the stray calls to these methods may be enough though.
You're not ignoring it, NOTREACHED() is fatal! Besides, we don't put code (`return` in this case) after NOTREACHED().
Peter Kvitek
Yes, NOTREACHED() is fatal, but the code is using NOTIMPLEMENTED() which is not fatal, it's just logging. Even that may be an overkill though.
Related details
Attention is currently required from:
- Andrey Kosyakov
- Colin Blundell
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Peter Kvitek (Gerrit)
Feb 25, 2026, 7:48:16 PMFeb 25
to Andrey Kosyakov, Colin Blundell, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org, mac-r...@chromium.org, ozone-...@chromium.org, roblia...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, headless...@chromium.org
Attention needed from Andrey Kosyakov and Colin Blundell
Peter Kvitek added 1 comment![]( "Open in Gerrit")
File ui/display/headless/headless_screen_manager.h
Line 58, Patchset 8 (Latest):
static void SetPrimaryDisplay(display::DisplayList& display_list,Andrey Kosyakov . resolved
Peter Kviteknit: extract these into a different class (follow-up is fine).
Yup, these belong to `//ui/display/headless/headless_screen_util.h/cc`
Peter Kvitek
Peter Kvitek (Gerrit)
Feb 25, 2026, 7:48:34 PMFeb 25
to Andrey Kosyakov, Colin Blundell, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org, mac-r...@chromium.org, ozone-...@chromium.org, roblia...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, headless...@chromium.org
Attention needed from Andrey Kosyakov and Colin Blundell
Peter Kvitek added 1 comment![]( "Open in Gerrit")
File ui/display/headless/headless_screen_manager.cc
Line 146, Patchset 8 (Latest):
if (!delegate_) {Andrey Kosyakov . resolved
Peter KvitekJust `CHECK(delegate_)` instead?
Andrey KosyakovScary... technically the `HeadlessScreenManager` instance can outlive the `display::Screen` instances that implement the delegate interface. Just ignoring the stray calls to these methods may be enough though.
Peter KvitekYou're not ignoring it, NOTREACHED() is fatal! Besides, we don't put code (`return` in this case) after NOTREACHED().
Yes, NOTREACHED() is fatal, but the code is using NOTIMPLEMENTED() which is not fatal, it's just logging. Even that may be an overkill though.
Peter Kvitek
Done
Related details
Attention is currently required from:
- Andrey Kosyakov
- Colin Blundell
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Colin Blundell (Gerrit)
Feb 26, 2026, 3:05:53 AMFeb 26
to Peter Kvitek, Colin Blundell, Andrey Kosyakov, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org, mac-r...@chromium.org, ozone-...@chromium.org, roblia...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, headless...@chromium.org
Attention needed from Andrey Kosyakov and Peter Kvitek
Colin Blundell voted and added 1 comment![]( "Open in Gerrit")
Votes added by Colin Blundell
| Code-Review | +1 |
1 comment
Patchset-level comments
Related details
Attention is currently required from:
- Andrey Kosyakov
- Peter Kvitek
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Peter Kvitek (Gerrit)
Feb 26, 2026, 2:26:13 PMFeb 26
to Colin Blundell, Andrey Kosyakov, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org, mac-r...@chromium.org, ozone-...@chromium.org, roblia...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, headless...@chromium.org
Attention needed from Andrey Kosyakov
Peter Kvitek voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Attention is currently required from:
- Andrey Kosyakov
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Peter Kvitek (Gerrit)
Feb 26, 2026, 6:45:10 PMFeb 26
to Colin Blundell, Andrey Kosyakov, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org, mac-r...@chromium.org, ozone-...@chromium.org, roblia...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, headless...@chromium.org
Chromium LUCI CQ (Gerrit)
Feb 26, 2026, 6:47:46 PMFeb 26
to Peter Kvitek, Colin Blundell, Andrey Kosyakov, AyeAye, chromium...@chromium.org, devtools...@chromium.org, mac-r...@chromium.org, ozone-...@chromium.org, roblia...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, headless...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[headless] Add Emulation.setPrimaryScreen DevTools method
This CL introduces Emulation.setPrimaryScreen that allows browser
automaton users to change the primary screen. This is currently
supported by headless shell only. Support on other desktop platforms
will be added by follow up CLs.
Bug: 397350115, 404587568
Change-Id: I002996e88d326bb46f8d789a692bc1fe1849d6d2
Reviewed-by: Colin Blundell <blun...@chromium.org>
Commit-Queue: Peter Kvitek <kvi...@chromium.org>
Reviewed-by: Andrey Kosyakov <ca...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1591162}
Files:
- A components/headless/test/data/protocol/shared/set-primary-screen-expected.txt
- A components/headless/test/data/protocol/shared/set-primary-screen-scaled-expected.txt
- A components/headless/test/data/protocol/shared/set-primary-screen-scaled.js
- A components/headless/test/data/protocol/shared/set-primary-screen.js
- M headless/lib/browser/headless_screen.cc
- M headless/lib/browser/headless_screen.h
- M headless/lib/browser/protocol/emulation_handler.cc
- M headless/lib/browser/protocol/emulation_handler.h
- M headless/protocol_config.json
- M headless/test/headless_protocol_browsertest.cc
- M third_party/blink/public/devtools_protocol/domains/Emulation.pdl
- M ui/display/headless/headless_screen_manager.cc
- M ui/display/headless/headless_screen_manager.h
- M ui/display/headless/headless_screen_manager_unittest.cc
- M ui/display/mac/screen_mac_headless.h
- M ui/display/mac/screen_mac_headless.mm
- M ui/display/win/screen_win_headless.cc
- M ui/display/win/screen_win_headless.h
- M ui/ozone/platform/headless/headless_screen.cc
- M ui/ozone/platform/headless/headless_screen.h
Change size: L
Delta: 20 files changed, 514 insertions(+), 1 deletion(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Andrey Kosyakov, +1 by Colin Blundell
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages