[glic-ob-metrics] Add OptinImpression metric to Glic Web UI [chromium/src : main]

[Trevor Perrier (Gerrit)]

Attention needed from Justin DeWitt

Trevor Perrier voted and added 3 comments

Votes added by Trevor Perrier

Code-Review

+1

3 comments

Patchset-level comments

File-level comment, Patchset 1 (Latest):

Trevor Perrier . resolved

LGTM with one suggestion to make it less slightly simple since I think the current flow is over optimized for just the opt-in impression case and wouldn't work for the two others I know about. (And I suspect we'll have other impressions that will also need their own bespoke logic)

Commit Message

Line 9, Patchset 1 (Latest):This CL adds OnOptinImpression to GlicBrowserHostMetrics and handles it by
recording the Glic.Onboarding.OptInImpression user action.

Trevor Perrier . unresolved

nit: Click format in the message edit box to wrap this cleanly.

File chrome/browser/glic/service/metrics/glic_instance_metrics.cc

Line 133, Patchset 1 (Latest): !is_client_ready_ || !visibility_tracker_->state()) {

Trevor Perrier . unresolved

Not blocking but the method name and use of a `pending_impressions_` vector implies that other impressions will be logged through `FlushPendingImpression`. However this check and whole method is very specific to the `opt-in` impression and makes it so other impressions can't use this method.

• Invoke should be recorded on `!visibility_tracker_->state()` regardless of `is_client_ready_`.
• WebAppImpression should be recorded everytime visibility_tracker_->state() is true and `is_client_ready_` is true. (e.g. we don't want to clear that from the vector.

Given that, it seems like: `MaybeRecordOptInImpression()` is a better method name and have a `is_opt_in_pending_` bool instead of the vector.

Open in Gerrit

Related details

Attention is currently required from:

• Justin DeWitt

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: Idad7370432dfab1182aac351c6398195f940efeb

Gerrit-Change-Number: 7832943

Gerrit-PatchSet: 1

Gerrit-Owner: Justin DeWitt <dew...@chromium.org>

Gerrit-Reviewer: Justin DeWitt <dew...@chromium.org>

Gerrit-Reviewer: Trevor Perrier <per...@chromium.org>

Gerrit-CC: Anurag Simgeker <anurags...@google.com>

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

Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>

Gerrit-CC: Thorsten Kober <thor...@google.com>

Gerrit-CC: Zewen Li <zew...@google.com>

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

Gerrit-Attention: Justin DeWitt <dew...@chromium.org>

Gerrit-Comment-Date: Fri, 08 May 2026 23:56:13 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: Yes

[Basia Zimirska (Gerrit)]

Attention needed from Justin DeWitt

Basia Zimirska voted and added 1 comment

Votes added by Basia Zimirska

Code-Review

+1

1 comment

File chrome/browser/glic/public/features.h

Line 98, Patchset 1 (Latest):BASE_DECLARE_FEATURE(kGlicRecordImpressionMetrics);

Basia Zimirska . unresolved

nit: maybe rename to OptInImpressionMetrics or do you think we'll keep merging new impressions under the same flag?

Open in Gerrit

Related details

Attention is currently required from:

• Justin DeWitt

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: Idad7370432dfab1182aac351c6398195f940efeb

Gerrit-Change-Number: 7832943

Gerrit-PatchSet: 1

Gerrit-Owner: Justin DeWitt <dew...@chromium.org>

Gerrit-Reviewer: Basia Zimirska <bas...@google.com>

Gerrit-Reviewer: Justin DeWitt <dew...@chromium.org>

Gerrit-Reviewer: Trevor Perrier <per...@chromium.org>

Gerrit-CC: Anurag Simgeker <anurags...@google.com>

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

Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>

Gerrit-CC: Thorsten Kober <thor...@google.com>

Gerrit-CC: Zewen Li <zew...@google.com>

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

Gerrit-Attention: Justin DeWitt <dew...@chromium.org>

Gerrit-Comment-Date: Sat, 09 May 2026 00:18:34 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: Yes

[Daniel Cheng (Gerrit)]

Attention needed from Justin DeWitt

Daniel Cheng voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

• Justin DeWitt

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: Idad7370432dfab1182aac351c6398195f940efeb

Gerrit-Change-Number: 7832943

Gerrit-PatchSet: 1

Gerrit-Owner: Justin DeWitt <dew...@chromium.org>

Gerrit-Reviewer: Basia Zimirska <bas...@google.com>

Gerrit-Reviewer: Daniel Cheng <dch...@chromium.org>

Gerrit-Reviewer: Justin DeWitt <dew...@chromium.org>

Gerrit-Reviewer: Trevor Perrier <per...@chromium.org>

Gerrit-CC: Anurag Simgeker <anurags...@google.com>

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

Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>

Gerrit-CC: Thorsten Kober <thor...@google.com>

Gerrit-CC: Zewen Li <zew...@google.com>

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

Gerrit-Attention: Justin DeWitt <dew...@chromium.org>

Gerrit-Comment-Date: Sat, 09 May 2026 01:25:35 +0000

Gerrit-HasComments: No

Gerrit-Has-Labels: Yes

[Justin DeWitt (Gerrit)]

Justin DeWitt voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention set is empty

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: Idad7370432dfab1182aac351c6398195f940efeb

Gerrit-Change-Number: 7832943

Gerrit-PatchSet: 3

Gerrit-Owner: Justin DeWitt <dew...@chromium.org>

Gerrit-Reviewer: Basia Zimirska <bas...@google.com>

Gerrit-Reviewer: Daniel Cheng <dch...@chromium.org>

Gerrit-Reviewer: Justin DeWitt <dew...@chromium.org>

Gerrit-Reviewer: Trevor Perrier <per...@chromium.org>

Gerrit-CC: Anurag Simgeker <anurags...@google.com>

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

Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>

Gerrit-CC: Thorsten Kober <thor...@google.com>

Gerrit-CC: Zewen Li <zew...@google.com>

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

Gerrit-Comment-Date: Sun, 10 May 2026 15:12:15 +0000

Gerrit-HasComments: No

Gerrit-Has-Labels: Yes

[Justin DeWitt (Gerrit)]

Justin DeWitt added 3 comments

Commit Message

Line 9, Patchset 1:This CL adds OnOptinImpression to GlicBrowserHostMetrics and handles it by


recording the Glic.Onboarding.OptInImpression user action.

Trevor Perrier . resolved

nit: Click format in the message edit box to wrap this cleanly.

Justin DeWitt

Done

File chrome/browser/glic/public/features.h

Line 98, Patchset 1:BASE_DECLARE_FEATURE(kGlicRecordImpressionMetrics);

Basia Zimirska . resolved

nit: maybe rename to OptInImpressionMetrics or do you think we'll keep merging new impressions under the same flag?

Justin DeWitt

Done

File chrome/browser/glic/service/metrics/glic_instance_metrics.cc

Line 133, Patchset 1: !is_client_ready_ || !visibility_tracker_->state()) {

Trevor Perrier . resolved

Not blocking but the method name and use of a `pending_impressions_` vector implies that other impressions will be logged through `FlushPendingImpression`. However this check and whole method is very specific to the `opt-in` impression and makes it so other impressions can't use this method.

• Invoke should be recorded on `!visibility_tracker_->state()` regardless of `is_client_ready_`.
• WebAppImpression should be recorded everytime visibility_tracker_->state() is true and `is_client_ready_` is true. (e.g. we don't want to clear that from the vector.

Given that, it seems like: `MaybeRecordOptInImpression()` is a better method name and have a `is_opt_in_pending_` bool instead of the vector.

Justin DeWitt

Done

Open in Gerrit

Related details

Attention set is empty

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: Idad7370432dfab1182aac351c6398195f940efeb

Gerrit-Change-Number: 7832943

Gerrit-PatchSet: 3

Gerrit-Owner: Justin DeWitt <dew...@chromium.org>

Gerrit-Reviewer: Basia Zimirska <bas...@google.com>

Gerrit-Reviewer: Daniel Cheng <dch...@chromium.org>

Gerrit-Reviewer: Justin DeWitt <dew...@chromium.org>

Gerrit-Reviewer: Trevor Perrier <per...@chromium.org>

Gerrit-CC: Anurag Simgeker <anurags...@google.com>

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

Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>

Gerrit-CC: Thorsten Kober <thor...@google.com>

Gerrit-CC: Zewen Li <zew...@google.com>

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

Gerrit-Comment-Date: Sun, 10 May 2026 15:40:26 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: Trevor Perrier <per...@chromium.org>

Comment-In-Reply-To: Basia Zimirska <bas...@google.com>

[Justin DeWitt (Gerrit)]

Justin DeWitt voted Commit-Queue+2

Commit-Queue

+2

Gerrit-Comment-Date: Sun, 10 May 2026 15:40:31 +0000

Gerrit-HasComments: No

Gerrit-Has-Labels: Yes