Implement portal-based eye dropper for Linux [chromium/src : main]

[Thomas Anderson (Gerrit)]

Attention needed from Alex Gough, Lei Zhang and Rick Byers

Thomas Anderson added 1 comment

Patchset-level comments

File-level comment, Patchset 15 (Latest):

Thomas Anderson . resolved

reviewers ptal

```
ajgo:
  third_party/blink/common/renderer_preferences/renderer_preferences_mojom_traits.cc
  third_party/blink/public/common/renderer_preferences/renderer_preferences_mojom_traits.h
  third_party/blink/public/mojom/renderer_preferences.mojom
thestig:
  chrome/
rbyers:
  third_party/blink/public/common/renderer_preferences/renderer_preferences.h
  third_party/blink/renderer/core/exported/web_view_impl.cc
  third_party/blink/renderer/core/html/forms/color_chooser_popup_ui_controller.cc
  third_party/blink/renderer/core/page/page.cc
  third_party/blink/renderer/core/page/page.h
```

Open in Gerrit

Related details

Attention is currently required from:

• Alex Gough
• Lei Zhang
• Rick Byers

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I780a40e7722730ad0f7d6cb237bae8c82386f659

Gerrit-Change-Number: 7708809

Gerrit-PatchSet: 15

Gerrit-Owner: Thomas Anderson <thomasa...@chromium.org>

Gerrit-Reviewer: Alex Gough <aj...@chromium.org>

Gerrit-Reviewer: Lei Zhang <the...@chromium.org>

Gerrit-Reviewer: Rick Byers <rby...@chromium.org>

Gerrit-Reviewer: Thomas Anderson <thomasa...@chromium.org>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: android-bu...@system.gserviceaccount.com <android-bu...@system.gserviceaccount.com>

Gerrit-CC: chromium...@chromium.org

Gerrit-Attention: Lei Zhang <the...@chromium.org>

Gerrit-Attention: Rick Byers <rby...@chromium.org>

Gerrit-Attention: Alex Gough <aj...@chromium.org>

Gerrit-Comment-Date: Fri, 31 Jul 2026 22:54:58 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Lei Zhang (Gerrit)]

Attention needed from Alex Gough, Rick Byers and Thomas Anderson

Lei Zhang added 2 comments

Commit Message

Line 15, Patchset 15 (Latest):- Fixes a bug in ColorChooserPopupUIController where the eye dropper

Lei Zhang . unresolved

Is it possible to fix this bug separately to reduce CL size?

Line 22, Patchset 15 (Latest):- Adds unit tests for the new portal implementation using base::test::RunUntil.

Lei Zhang . unresolved

Wrap at 72 columns.

Open in Gerrit

Related details

Attention is currently required from:

• Alex Gough
• Rick Byers
• Thomas Anderson

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.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I780a40e7722730ad0f7d6cb237bae8c82386f659

Gerrit-Change-Number: 7708809

Gerrit-PatchSet: 15

Gerrit-Owner: Thomas Anderson <thomasa...@chromium.org>

Gerrit-Reviewer: Alex Gough <aj...@chromium.org>

Gerrit-Reviewer: Lei Zhang <the...@chromium.org>

Gerrit-Reviewer: Rick Byers <rby...@chromium.org>

Gerrit-Reviewer: Thomas Anderson <thomasa...@chromium.org>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: android-bu...@system.gserviceaccount.com <android-bu...@system.gserviceaccount.com>

Gerrit-CC: chromium...@chromium.org

Gerrit-Attention: Thomas Anderson <thomasa...@chromium.org>

Gerrit-Attention: Rick Byers <rby...@chromium.org>

Gerrit-Attention: Alex Gough <aj...@chromium.org>

Gerrit-Comment-Date: Sat, 01 Aug 2026 01:01:04 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Lei Zhang (Gerrit)]

Attention needed from Alex Gough, Rick Byers and Thomas Anderson

Lei Zhang added 9 comments

File chrome/browser/ui/views/eye_dropper/DEPS

Line 2, Patchset 15 (Latest): "+base/nix",

Lei Zhang . unresolved

Probably not needed?

File chrome/browser/ui/views/eye_dropper/eye_dropper_aura.cc

Line 40, Patchset 15 (Latest): std::unique_ptr<base::Environment> env = base::Environment::Create();

Lei Zhang . unresolved

BTW, can create `base::Environment` on the stack now.

File chrome/browser/ui/views/eye_dropper/eye_dropper_portal.h

Line 53, Patchset 15 (Latest): scoped_refptr<dbus::Bus> bus_;

Lei Zhang . unresolved

const

Line 52, Patchset 15 (Latest): raw_ptr<content::EyeDropperListener> listener_;

Lei Zhang . unresolved

const

Line 15, Patchset 15 (Latest):#include "content/public/browser/eye_dropper_listener.h"

Lei Zhang . unresolved

Can forward declare.

Line 13, Patchset 15 (Latest):#include "components/dbus/xdg/request.h"

Lei Zhang . unresolved

Can forward declare.

File chrome/browser/ui/views/eye_dropper/eye_dropper_portal.cc

Line 78, Patchset 15 (Latest): if (!bus_) {

Lei Zhang . unresolved

Can `bus_` ever be null in production? In tests?

Line 82, Patchset 15 (Latest): if (portal && portal->listener_) {

Lei Zhang . unresolved

Is `listener_` ever null? May not matter if this path is unreachable.

Line 121, Patchset 15 (Latest): dbus_xdg::Dictionary options;

Lei Zhang . unresolved

Inline onto line 124?

Gerrit-Comment-Date: Sat, 01 Aug 2026 01:14:59 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No