Rob Pitkin (Gerrit)

unread,

Feb 24, 2026, 5:11:30 PMFeb 24

to Matt Reynolds, Alvin Ji, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, mac-r...@chromium.org, mattreyno...@chromium.org

Attention needed from Alvin Ji and Matt Reynolds

Rob Pitkin added 3 comments

Patchset-level comments

File-level comment, Patchset 7 (Latest):

Rob Pitkin . resolved

Took another pass at these changes with a focus on the outcome of enumeration attempts on a per data fetcher basis rather than generic metrics. PTAL, thanks!

Commit Message

Line 14, Patchset 2:- Gamepad.ConnectionSource: Records the fetcher used when a new gamepad

Matt Reynolds . resolved

To better understand double-enumeration I think we want to always record the connection even if the gamepad isn't assigned a slot. If we wait until after a slot is assigned to record the metric then we'll ignore cases where double-enumeration is prevented by our static blocklist rules (or when kItemsLengthCap is reached and there are no available slots).

Rob Pitkin

Done

Line 16, Patchset 2:- Gamepad.DeduplicatedGamepadSource: Records when a fetcher ignores a

Matt Reynolds . resolved

Currently we're only measuring IsSupportedByGameController-based deduplication but there are other cases where we deduplicate gamepads in this data fetcher (IsNintendoController, kSteelSeriesProduct1420) and other data fetchers. I think it's fine if we only introduce logging for macOS data fetchers for now but it would be nice if this metric could represent different types of deduplication within the same data fetcher.
Instead of logging a GamepadSource enum, let's introduce a new enum to represent the deduplication rule that was applied when the gamepad was ignored. This could be expanded to support "ignore" rules that aren't meant for deduplication, like GamepadIsExcluded.

Rob Pitkin

Done

Open in Gerrit

Related details

Attention is currently required from:

Alvin Ji
Matt Reynolds

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

satisfied_requirement

unsatisfied_requirement

open

diffy

Rob Pitkin (Gerrit)

unread,

Feb 27, 2026, 3:57:17 PMFeb 27

to Code Review Nudger, Matt Reynolds, Alvin Ji, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, mac-r...@chromium.org, mattreyno...@chromium.org

Attention needed from Alvin Ji and Matt Reynolds

Rob Pitkin added 1 comment

File device/gamepad/xbox_data_fetcher_mac.cc

Line 188, Patchset 10 (Latest): RecordXboxMacOutcome(XboxMacOutcome::kOpenFailed);

Rob Pitkin . unresolved

@mattre...@chromium.org I removed the buggy `kOpenFailedExclusiveAccess` case since it was triggering too many times. Should I also remove these other failure enum cases? (e.g. only leave the successful case)

Open in Gerrit

Related details

Attention is currently required from:

Alvin Ji
Matt Reynolds

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

satisfied_requirement

unsatisfied_requirement

open

diffy

Matt Reynolds (Gerrit)

unread,

Mar 10, 2026, 2:43:30 PM (14 days ago) Mar 10

to Rob Pitkin, Code Review Nudger, Alvin Ji, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, mac-r...@chromium.org, mattreyno...@chromium.org

Attention needed from Alvin Ji and Rob Pitkin

Matt Reynolds added 2 comments

File device/gamepad/game_controller_data_fetcher_mac.mm

Line 129, Patchset 10 (Latest):GameControllerMacOutcome IsSupported(GCController* controller) {

Matt Reynolds . unresolved

The "Is" prefix implies the method returns a boolean, but now it returns GameControllerMacOutcome. Please rename this method to reflect the new return type.

File device/gamepad/xbox_data_fetcher_mac.cc

Line 188, Patchset 10 (Latest): RecordXboxMacOutcome(XboxMacOutcome::kOpenFailed);

Rob Pitkin . unresolved

@mattre...@chromium.org I removed the buggy `kOpenFailedExclusiveAccess` case since it was triggering too many times. Should I also remove these other failure enum cases? (e.g. only leave the successful case)

Matt Reynolds

It's odd that kOpenFailed was recorded when switching between USB and Bluetooth. Does it happen when the USB device disconnects? If so, let's remove it since it would cause us to double-count.

Open in Gerrit

Related details

Attention is currently required from:

Alvin Ji
Rob Pitkin

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

satisfied_requirement

unsatisfied_requirement

open

diffy

Rob Pitkin (Gerrit)

unread,

Mar 12, 2026, 4:47:07 PM (12 days ago) Mar 12

to Code Review Nudger, Matt Reynolds, Alvin Ji, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, mac-r...@chromium.org, mattreyno...@chromium.org

Attention needed from Matt Reynolds

Rob Pitkin added 3 comments

Patchset-level comments

File-level comment, Patchset 11 (Latest):

Rob Pitkin . resolved

Tested manually and removed that other fail enum value. PTAL at the latest changes. Thanks!

File device/gamepad/game_controller_data_fetcher_mac.mm

Line 129, Patchset 10:GameControllerMacOutcome IsSupported(GCController* controller) {

Matt Reynolds . resolved

The "Is" prefix implies the method returns a boolean, but now it returns GameControllerMacOutcome. Please rename this method to reflect the new return type.

Rob Pitkin

Done

File device/gamepad/xbox_data_fetcher_mac.cc

Line 188, Patchset 10: RecordXboxMacOutcome(XboxMacOutcome::kOpenFailed);

Rob Pitkin . resolved

@mattre...@chromium.org I removed the buggy `kOpenFailedExclusiveAccess` case since it was triggering too many times. Should I also remove these other failure enum cases? (e.g. only leave the successful case)

Matt Reynolds

It's odd that kOpenFailed was recorded when switching between USB and Bluetooth. Does it happen when the USB device disconnects? If so, let's remove it since it would cause us to double-count.

Rob Pitkin

Yeah I tested this with monitoring mode in chrome://histograms and confirmed it gets logged when I unplug the gamepad over USB. Removed it's usage.

Open in Gerrit

Related details

Attention is currently required from:

Matt Reynolds

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

satisfied_requirement

unsatisfied_requirement

open

diffy

Matt Reynolds (Gerrit)

unread,

Mar 23, 2026, 7:20:40 PM (18 hours ago) Mar 23

to Rob Pitkin, Code Review Nudger, Alvin Ji, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, mac-r...@chromium.org, mattreyno...@chromium.org

Attention needed from Rob Pitkin

Matt Reynolds voted and added 1 comment

Votes added by Matt Reynolds

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 11 (Latest):

Matt Reynolds . resolved

lgtm

Open in Gerrit

Related details

Attention is currently required from:

Rob Pitkin

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

satisfied_requirement

unsatisfied_requirement

open

diffy

Rob Pitkin (Gerrit)

unread,

11:28 AM (1 hour ago) 11:28 AM

to Jonathan Ross, Matt Reynolds, Code Review Nudger, Alvin Ji, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, mac-r...@chromium.org, mattreyno...@chromium.org

Attention needed from Jonathan Ross

Rob Pitkin added 1 comment

Patchset-level comments

File-level comment, Patchset 11 (Latest):

Rob Pitkin . resolved

Hi Jonathan, I'm requesting your review as a code owner for `histograms/metadata/others/enums.xml` and `histogram/metadata/others/histograms.xml`. PTAL, thanks!

Open in Gerrit

Related details

Attention is currently required from:

Jonathan Ross

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

satisfied_requirement

unsatisfied_requirement

open

diffy

Reply all

Reply to author

Forward

[gamepad] Add UMA metrics for macOS data fetchers [chromium/src : main]

Rob Pitkin (Gerrit)

Rob Pitkin added 3 comments

Related details

Rob Pitkin (Gerrit)

Rob Pitkin added 1 comment

Related details

Matt Reynolds (Gerrit)

Matt Reynolds added 2 comments

Related details

Rob Pitkin (Gerrit)

Rob Pitkin added 3 comments

Related details

Matt Reynolds (Gerrit)

Matt Reynolds voted and added 1 comment

Votes added by Matt Reynolds

1 comment

Related details

Rob Pitkin (Gerrit)

Rob Pitkin added 1 comment

Related details