Mikel Astiz (Gerrit)

unread,

Apr 13, 2026, 5:00:15 AM (4 days ago) Apr 13

to Wei Guo, Tommy Nyquist, Marke Hallowell, Chromium LUCI CQ, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, derinel+wat...@google.com, webauthn...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, jz...@chromium.org, mar...@chromium.org, devtools...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, feature-me...@chromium.org, fgal...@chromium.org, chrome-intelligence-te...@google.com, dewitt...@chromium.org, knollr+wat...@chromium.org

Attention needed from Marke Hallowell, Tommy Nyquist and Wei Guo

Mikel Astiz added 9 comments

Patchset-level comments

File-level comment, Patchset 26 (Latest):

Mikel Astiz . resolved

Thanks!

File chrome/browser/sharing/glic_experimental_triggering/glic_experimental_triggering_message_handler.h

Line 27, Patchset 26 (Latest): raw_ptr<Profile> profile_;

Mikel Astiz . unresolved

Nit: this member could be marked as const.

File chrome/browser/sharing/glic_experimental_triggering/glic_experimental_triggering_message_handler.cc

Line 29, Patchset 26 (Latest): if (!base::FeatureList::IsEnabled(features::kGlicExperimentalTriggering)) {

Mikel Astiz . unresolved

Can this be a CHECK instead? With the feature disabled this code should be unreachable.

Line 35, Patchset 26 (Latest): DCHECK(message.has_glic_experimental_triggering());

Mikel Astiz . unresolved

For new code CHECK should be favored over DCHECK, see https://chromium.googlesource.com/chromium/src/+/HEAD/styleguide/c++/checks.md#invariant_verification-mechanisms

Line 40, Patchset 26 (Latest): GetLastActiveBrowserWindowInterfaceWithAnyProfile();

Mikel Astiz . unresolved

How come any profile? I think you'd want to narrow this down to `profile_`?

If there are good reasons for this, please add a comment explaining why this makes sense.

Line 43, Patchset 26 (Latest): << "No active browser window found for GlicExperimentalTriggering";

Mikel Astiz . unresolved

Wouldn't you want to open a window in this case?

Line 50, Patchset 26 (Latest): if (!glic_service) {

Mikel Astiz . unresolved

This one and the one above should ideally be CHECK failures too. Otherwise, I fear this device may be advertising a capability that it doesn't actually have. Unfortunately I am not familiar with the details of how capabilities are evaluated and whether there would be a way to achieve this nicely, but a drastic approach would be to not even register the message handler in such a case?

File chrome/browser/sharing/glic_experimental_triggering/glic_experimental_triggering_message_handler_unittest.cc

Line 33, Patchset 26 (Latest): base::test::ScopedFeatureList feature_list_;

Mikel Astiz . unresolved

Nit: you could do `feature_list_{features::kGlicExperimentalTriggering}` here and keep the constructor simpler.

Line 35, Patchset 26 (Latest): std::unique_ptr<GlicExperimentalTriggeringMessageHandler> handler_;

Mikel Astiz . unresolved

Optional: does this need to be a unique_ptr, or could it be: `GlicExperimentalTriggeringMessageHandler handler_(&profile_)`?

Open in Gerrit

Related details

Attention is currently required from:

Marke Hallowell
Tommy Nyquist
Wei Guo

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

Wei Guo (Gerrit)

unread,

Apr 13, 2026, 2:26:16 PM (4 days ago) Apr 13

to Tommy Nyquist, Marke Hallowell, Mikel Astiz, chromiu...@luci-project-accounts.iam.gserviceaccount.com, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, derinel+wat...@google.com, webauthn...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, jz...@chromium.org, mar...@chromium.org, devtools...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, feature-me...@chromium.org, fgal...@chromium.org, chrome-intelligence-te...@google.com, dewitt...@chromium.org, knollr+wat...@chromium.org

Attention needed from Marke Hallowell, Mikel Astiz and Tommy Nyquist

Wei Guo added 8 comments

File chrome/browser/sharing/glic_experimental_triggering/glic_experimental_triggering_message_handler.h

Line 27, Patchset 26: raw_ptr<Profile> profile_;

Mikel Astiz . resolved

Nit: this member could be marked as const.

Wei Guo

Done

File chrome/browser/sharing/glic_experimental_triggering/glic_experimental_triggering_message_handler.cc

Line 29, Patchset 26: if (!base::FeatureList::IsEnabled(features::kGlicExperimentalTriggering)) {

Mikel Astiz . resolved

Can this be a CHECK instead? With the feature disabled this code should be unreachable.

Wei Guo

Done

Line 35, Patchset 26: DCHECK(message.has_glic_experimental_triggering());

Mikel Astiz . resolved

For new code CHECK should be favored over DCHECK, see https://chromium.googlesource.com/chromium/src/+/HEAD/styleguide/c++/checks.md#invariant_verification-mechanisms

Wei Guo

Done

Line 40, Patchset 26: GetLastActiveBrowserWindowInterfaceWithAnyProfile();

Mikel Astiz . resolved

How come any profile? I think you'd want to narrow this down to `profile_`?
If there are good reasons for this, please add a comment explaining why this makes sense.

Wei Guo

Done

I narrowed down to this profile_.
However, this is not the final behavior, we will likely open a new background tab for this.

Line 43, Patchset 26: << "No active browser window found for GlicExperimentalTriggering";

Mikel Astiz . resolved

Wouldn't you want to open a window in this case?

Wei Guo

Sergio is working on opening a background window/tab for that.

Line 50, Patchset 26: if (!glic_service) {

Mikel Astiz . resolved

This one and the one above should ideally be CHECK failures too. Otherwise, I fear this device may be advertising a capability that it doesn't actually have. Unfortunately I am not familiar with the details of how capabilities are evaluated and whether there would be a way to achieve this nicely, but a drastic approach would be to not even register the message handler in such a case?

Wei Guo

Done

I added the glic_server presence test to the registration site.
I used CHECK here.
But I didn't change to use CHECK(browser) since I am concerned it might be a little too aggressive for some transient cases when browser is null for other reasons. Let me know if you think otherwise.

File chrome/browser/sharing/glic_experimental_triggering/glic_experimental_triggering_message_handler_unittest.cc

Line 33, Patchset 26: base::test::ScopedFeatureList feature_list_;

Mikel Astiz . resolved

Nit: you could do `feature_list_{features::kGlicExperimentalTriggering}` here and keep the constructor simpler.

Wei Guo

Done

Line 35, Patchset 26: std::unique_ptr<GlicExperimentalTriggeringMessageHandler> handler_;

Mikel Astiz . resolved

Optional: does this need to be a unique_ptr, or could it be: `GlicExperimentalTriggeringMessageHandler handler_(&profile_)`?

Wei Guo

Done
Much cleaner!

Open in Gerrit

Related details

Attention is currently required from:

Marke Hallowell
Mikel Astiz
Tommy Nyquist

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

Mikel Astiz (Gerrit)

unread,

Apr 13, 2026, 3:59:39 PM (4 days ago) Apr 13

to Wei Guo, Tommy Nyquist, Marke Hallowell, chromiu...@luci-project-accounts.iam.gserviceaccount.com, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, derinel+wat...@google.com, webauthn...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, jz...@chromium.org, mar...@chromium.org, devtools...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, feature-me...@chromium.org, fgal...@chromium.org, chrome-intelligence-te...@google.com, dewitt...@chromium.org, knollr+wat...@chromium.org

Attention needed from Marke Hallowell, Tommy Nyquist and Wei Guo

Mikel Astiz added 6 comments

File chrome/browser/sharing/glic_experimental_triggering/glic_experimental_triggering_message_handler.cc

Line 21, Patchset 28 (Latest): : profile_(profile) {}

Mikel Astiz . unresolved

Nit: CHECK(profile_), assuming this is legit.

Line 45, Patchset 28 (Latest): LOG(ERROR) << "No active browser window found for Profile for "

Mikel Astiz . unresolved

Optional: I don't know about the latest guidance on this, but LOG traces increase the binary size and are extremely unlikely to become useful for production builds. I would rather add DLOG, DVLOG and/or histograms.

Line 56, Patchset 28 (Latest): glic::GlicInvokeOptions options{glic::mojom::InvocationSource::kUnsupported};

Mikel Astiz . unresolved

Optional: the code could read better if the construction of `GlicInvokeOptions` could be refactored to a helper function that returns this object given a request.

Line 74, Patchset 28 (Latest): std::move(done_callback).Run(nullptr);

Mikel Astiz . unresolved

Can this move below and be the very last step?

It seems more logical to trigger completion once the operation has actually triggered.

File chrome/browser/sharing/glic_experimental_triggering/glic_experimental_triggering_message_handler_unittest.cc

Line 40, Patchset 28 (Latest): EXPECT_CALL(done_callback, Run(testing::_)).Times(1);

Mikel Astiz . unresolved

Can you please avoid fully qualifying `testing::_` via using declarations? Also, related, please wrap the entire file within an unnamed namespace if possible (which makes it obvious that these tests don't have declared friendships).

File chrome/browser/sharing/sharing_handler_registry_impl.cc

Line 107, Patchset 28 (Latest): /*create=*/false)) {

Mikel Astiz . unresolved

This needs to be /*create=*/true, otherwise this logic fulfill its intended purpose.

If this is problematic, and the CHECK can be hit, we may need to make extensions to `SharingHandlerRegistry` to support dynamic capabilities.

Open in Gerrit

Related details

Attention is currently required from:

Marke Hallowell
Tommy Nyquist
Wei Guo

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

Marke Hallowell (Gerrit)

unread,

Apr 13, 2026, 4:24:37 PM (4 days ago) Apr 13

to Wei Guo, Tommy Nyquist, Mikel Astiz, chromiu...@luci-project-accounts.iam.gserviceaccount.com, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, derinel+wat...@google.com, webauthn...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, jz...@chromium.org, mar...@chromium.org, devtools...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, feature-me...@chromium.org, fgal...@chromium.org, chrome-intelligence-te...@google.com, dewitt...@chromium.org, knollr+wat...@chromium.org

Attention needed from Tommy Nyquist and Wei Guo

Marke Hallowell voted and added 1 comment

Votes added by Marke Hallowell

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 29 (Latest):

Marke Hallowell . resolved

+1 for glic_passkeys.h change.

Open in Gerrit

Related details

Attention is currently required from:

Tommy Nyquist
Wei Guo

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

Wei Guo (Gerrit)

unread,

Apr 13, 2026, 4:51:36 PM (4 days ago) Apr 13

to Marke Hallowell, Tommy Nyquist, Mikel Astiz, chromiu...@luci-project-accounts.iam.gserviceaccount.com, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, derinel+wat...@google.com, webauthn...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, jz...@chromium.org, mar...@chromium.org, devtools...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, feature-me...@chromium.org, fgal...@chromium.org, chrome-intelligence-te...@google.com, dewitt...@chromium.org, knollr+wat...@chromium.org

Attention needed from Marke Hallowell, Mikel Astiz and Tommy Nyquist

Wei Guo added 6 comments

File chrome/browser/sharing/glic_experimental_triggering/glic_experimental_triggering_message_handler.cc

Line 21, Patchset 28: : profile_(profile) {}

Mikel Astiz . resolved

Nit: CHECK(profile_), assuming this is legit.

Wei Guo

Done

Line 45, Patchset 28: LOG(ERROR) << "No active browser window found for Profile for "

Mikel Astiz . resolved

Optional: I don't know about the latest guidance on this, but LOG traces increase the binary size and are extremely unlikely to become useful for production builds. I would rather add DLOG, DVLOG and/or histograms.

Wei Guo

Done

Line 56, Patchset 28: glic::GlicInvokeOptions options{glic::mojom::InvocationSource::kUnsupported};

Mikel Astiz . resolved

Optional: the code could read better if the construction of `GlicInvokeOptions` could be refactored to a helper function that returns this object given a request.

Wei Guo

Done

Line 74, Patchset 28: std::move(done_callback).Run(nullptr);

Mikel Astiz . resolved

Can this move below and be the very last step?
It seems more logical to trigger completion once the operation has actually triggered.

Wei Guo

Done

File chrome/browser/sharing/glic_experimental_triggering/glic_experimental_triggering_message_handler_unittest.cc

Line 40, Patchset 28: EXPECT_CALL(done_callback, Run(testing::_)).Times(1);

Mikel Astiz . resolved

Can you please avoid fully qualifying `testing::_` via using declarations? Also, related, please wrap the entire file within an unnamed namespace if possible (which makes it obvious that these tests don't have declared friendships).

Wei Guo

Done

File chrome/browser/sharing/sharing_handler_registry_impl.cc

Line 107, Patchset 28: /*create=*/false)) {

Mikel Astiz . resolved

This needs to be /*create=*/true, otherwise this logic fulfill its intended purpose.
If this is problematic, and the CHECK can be hit, we may need to make extensions to `SharingHandlerRegistry` to support dynamic capabilities.

Wei Guo

Done
Changed to true.

Open in Gerrit

Related details

Attention is currently required from:

Marke Hallowell
Mikel Astiz
Tommy Nyquist

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

Mikel Astiz (Gerrit)

unread,

Apr 14, 2026, 5:24:24 AM (3 days ago) Apr 14

to Wei Guo, Marke Hallowell, Tommy Nyquist, chromiu...@luci-project-accounts.iam.gserviceaccount.com, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, derinel+wat...@google.com, webauthn...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, jz...@chromium.org, mar...@chromium.org, devtools...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, feature-me...@chromium.org, fgal...@chromium.org, chrome-intelligence-te...@google.com, dewitt...@chromium.org, knollr+wat...@chromium.org

Attention needed from Marke Hallowell, Tommy Nyquist and Wei Guo

Mikel Astiz voted and added 4 comments

Votes added by Mikel Astiz

Code-Review

+1

4 comments

Patchset-level comments

File-level comment, Patchset 32 (Latest):

Mikel Astiz . resolved

Thanks! A few tiny nits only, LGTM.

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

Line 12, Patchset 32 (Latest):class GlicExperimentalTriggeringMessageHandler;

Mikel Astiz . unresolved

Nit: sort alphabetically.

File chrome/browser/sharing/glic_experimental_triggering/glic_experimental_triggering_message_handler.cc

Line 28, Patchset 32 (Latest):namespace {

Mikel Astiz . unresolved

Optional: I would recommend moving this whole block up before any other functions (in this case constructor/destructor). Depending on your team's preferences.

Line 85, Patchset 32 (Latest): glic::GlicInvokeOptions options = CreateInvokeOptions(request);

Mikel Astiz . unresolved

Optional: I think you could also avoid this variable and inline the call below, which also means you can avoid a `std::move()`.

Open in Gerrit

Related details

Attention is currently required from:

Marke Hallowell
Tommy Nyquist
Wei Guo

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

Wei Guo (Gerrit)

unread,

Apr 14, 2026, 12:15:15 PM (3 days ago) Apr 14

to Mikel Astiz, Marke Hallowell, Tommy Nyquist, chromiu...@luci-project-accounts.iam.gserviceaccount.com, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, derinel+wat...@google.com, webauthn...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, jz...@chromium.org, mar...@chromium.org, devtools...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, feature-me...@chromium.org, fgal...@chromium.org, chrome-intelligence-te...@google.com, dewitt...@chromium.org, knollr+wat...@chromium.org

Attention needed from Marke Hallowell, Mikel Astiz and Tommy Nyquist

Wei Guo added 3 comments

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

Line 12, Patchset 32:class GlicExperimentalTriggeringMessageHandler;

Mikel Astiz . resolved

Nit: sort alphabetically.

Wei Guo

Done

File chrome/browser/sharing/glic_experimental_triggering/glic_experimental_triggering_message_handler.cc

Line 28, Patchset 32:namespace {

Mikel Astiz . resolved

Optional: I would recommend moving this whole block up before any other functions (in this case constructor/destructor). Depending on your team's preferences.

Wei Guo

Done

Line 85, Patchset 32: glic::GlicInvokeOptions options = CreateInvokeOptions(request);

Mikel Astiz . resolved

Optional: I think you could also avoid this variable and inline the call below, which also means you can avoid a `std::move()`.

Wei Guo

Done

Open in Gerrit

Related details

Attention is currently required from:

Marke Hallowell
Mikel Astiz
Tommy Nyquist

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

Marke Hallowell (Gerrit)

unread,

Apr 14, 2026, 12:26:41 PM (3 days ago) Apr 14

to Wei Guo, Mikel Astiz, Tommy Nyquist, chromiu...@luci-project-accounts.iam.gserviceaccount.com, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, derinel+wat...@google.com, webauthn...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, jz...@chromium.org, mar...@chromium.org, devtools...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, feature-me...@chromium.org, fgal...@chromium.org, chrome-intelligence-te...@google.com, dewitt...@chromium.org, knollr+wat...@chromium.org

Attention needed from Mikel Astiz, Tommy Nyquist and Wei Guo

Marke Hallowell voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Mikel Astiz
Tommy Nyquist
Wei Guo

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

Mikel Astiz (Gerrit)

unread,

Apr 14, 2026, 1:02:39 PM (3 days ago) Apr 14

to Wei Guo, Marke Hallowell, Tommy Nyquist, chromiu...@luci-project-accounts.iam.gserviceaccount.com, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, derinel+wat...@google.com, webauthn...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, jz...@chromium.org, mar...@chromium.org, devtools...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, feature-me...@chromium.org, fgal...@chromium.org, chrome-intelligence-te...@google.com, dewitt...@chromium.org, knollr+wat...@chromium.org

Attention needed from Marke Hallowell, Tommy Nyquist and Wei Guo

Mikel Astiz voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Marke Hallowell
Tommy Nyquist
Wei Guo

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

Min Qin (Gerrit)

unread,

Apr 14, 2026, 4:41:34 PM (3 days ago) Apr 14

to Wei Guo, Mikel Astiz, Marke Hallowell, Tommy Nyquist, chromiu...@luci-project-accounts.iam.gserviceaccount.com, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, derinel+wat...@google.com, webauthn...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, jz...@chromium.org, mar...@chromium.org, devtools...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, feature-me...@chromium.org, fgal...@chromium.org, chrome-intelligence-te...@google.com, dewitt...@chromium.org, knollr+wat...@chromium.org

Attention needed from Marke Hallowell, Tommy Nyquist and Wei Guo

Min Qin voted and added 2 comments

Votes added by Min Qin

Code-Review

+1

2 comments

Patchset-level comments

File-level comment, Patchset 34 (Latest):

Min Qin . resolved

lgtm % comment

File chrome/browser/sharing/glic_experimental_triggering/BUILD.gn

Line 31, Patchset 34 (Latest): ":glic_experimental_triggering",

Min Qin . unresolved

missing ":impl" here

Open in Gerrit

Related details

Attention is currently required from:

Marke Hallowell
Tommy Nyquist
Wei Guo

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

Wei Guo (Gerrit)

unread,

Apr 14, 2026, 4:56:50 PM (3 days ago) Apr 14

to Min Qin, Mikel Astiz, Marke Hallowell, Tommy Nyquist, chromiu...@luci-project-accounts.iam.gserviceaccount.com, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, derinel+wat...@google.com, webauthn...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, jz...@chromium.org, mar...@chromium.org, devtools...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, feature-me...@chromium.org, fgal...@chromium.org, chrome-intelligence-te...@google.com, dewitt...@chromium.org, knollr+wat...@chromium.org

Attention needed from Marke Hallowell, Mikel Astiz, Min Qin and Tommy Nyquist

Wei Guo added 1 comment

File chrome/browser/sharing/glic_experimental_triggering/BUILD.gn

Line 31, Patchset 34: ":glic_experimental_triggering",

Min Qin . resolved

missing ":impl" here

Wei Guo

Done

Open in Gerrit

Related details

Attention is currently required from:

Marke Hallowell
Mikel Astiz
Min Qin
Tommy Nyquist

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

Min Qin (Gerrit)

unread,

Apr 14, 2026, 6:07:21 PM (2 days ago) Apr 14

to Wei Guo, Mikel Astiz, Marke Hallowell, Tommy Nyquist, chromiu...@luci-project-accounts.iam.gserviceaccount.com, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, derinel+wat...@google.com, webauthn...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, jz...@chromium.org, mar...@chromium.org, devtools...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, feature-me...@chromium.org, fgal...@chromium.org, chrome-intelligence-te...@google.com, dewitt...@chromium.org, knollr+wat...@chromium.org

Attention needed from Marke Hallowell, Mikel Astiz, Tommy Nyquist and Wei Guo

Min Qin voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Marke Hallowell
Mikel Astiz
Tommy Nyquist
Wei Guo

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

Marke Hallowell (Gerrit)

unread,

Apr 14, 2026, 6:07:50 PM (2 days ago) Apr 14

to Wei Guo, Min Qin, Mikel Astiz, Tommy Nyquist, chromiu...@luci-project-accounts.iam.gserviceaccount.com, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, derinel+wat...@google.com, webauthn...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, jz...@chromium.org, mar...@chromium.org, devtools...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, feature-me...@chromium.org, fgal...@chromium.org, chrome-intelligence-te...@google.com, dewitt...@chromium.org, knollr+wat...@chromium.org

Attention needed from Mikel Astiz, Tommy Nyquist and Wei Guo

Marke Hallowell voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Mikel Astiz
Tommy Nyquist
Wei Guo

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

open

diffy

Wei Guo (Gerrit)

unread,

Apr 14, 2026, 6:08:46 PM (2 days ago) Apr 14

to Marke Hallowell, Min Qin, Mikel Astiz, Tommy Nyquist, chromiu...@luci-project-accounts.iam.gserviceaccount.com, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, derinel+wat...@google.com, webauthn...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, jz...@chromium.org, mar...@chromium.org, devtools...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, feature-me...@chromium.org, fgal...@chromium.org, chrome-intelligence-te...@google.com, dewitt...@chromium.org, knollr+wat...@chromium.org

Attention needed from Mikel Astiz and Tommy Nyquist

Wei Guo voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention is currently required from:

Mikel Astiz
Tommy Nyquist

satisfied_requirement

open

diffy

chromium-scoped@luci-project-accounts.iam.gserviceaccount.com (Gerrit)

unread,

Apr 14, 2026, 8:08:16 PM (2 days ago) Apr 14

to Wei Guo, Marke Hallowell, Min Qin, Mikel Astiz, Tommy Nyquist, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, derinel+wat...@google.com, webauthn...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, jz...@chromium.org, mar...@chromium.org, devtools...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, feature-me...@chromium.org, fgal...@chromium.org, chrome-intelligence-te...@google.com, dewitt...@chromium.org, knollr+wat...@chromium.org

chromiu...@luci-project-accounts.iam.gserviceaccount.com submitted the change

Change information

Commit message:

Basic scaffolding of GlicExperimentalTriggeringMessageHandler.

Exclude Android out, since Android build doesn't support
glic_service->InvokeWithAutoSubmit().

Change-Id: Ib2a662fce0fbbeaa314b361533cd23f2bf3df646

Bug: 500811549

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7745650

Reviewed-by: Min Qin <qin...@chromium.org>

Reviewed-by: Marke Hallowell <w...@chromium.org>

Commit-Queue: Wei Guo <wei...@google.com>

Cr-Commit-Position: refs/heads/main@{#1614785}

Files:

M chrome/browser/BUILD.gn
M chrome/browser/glic/public/glic_passkeys.h
M chrome/browser/sharing/BUILD.gn
A chrome/browser/sharing/glic_experimental_triggering/BUILD.gn
A chrome/browser/sharing/glic_experimental_triggering/glic_experimental_triggering_message_handler.cc
A chrome/browser/sharing/glic_experimental_triggering/glic_experimental_triggering_message_handler.h
A chrome/browser/sharing/glic_experimental_triggering/glic_experimental_triggering_message_handler_unittest.cc
M chrome/browser/sharing/sharing_handler_registry_impl.cc
M chrome/browser/sharing/sharing_handler_registry_impl_unittest.cc
M chrome/test/BUILD.gn

Change size: L

Delta: 10 files changed, 253 insertions(+), 0 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Marke Hallowell, +1 by Min Qin

Open in Gerrit

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

Gerrit

open

diffy

satisfied_requirement

Tommy Nyquist (Gerrit)

unread,

Apr 15, 2026, 1:11:42 AM (2 days ago) Apr 15

to Wei Guo, chromiu...@luci-project-accounts.iam.gserviceaccount.com, Marke Hallowell, Min Qin, Mikel Astiz, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, derinel+wat...@google.com, webauthn...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, jz...@chromium.org, mar...@chromium.org, devtools...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, feature-me...@chromium.org, fgal...@chromium.org, chrome-intelligence-te...@google.com, dewitt...@chromium.org, knollr+wat...@chromium.org

Attention needed from Wei Guo

Tommy Nyquist voted and added 2 comments

Votes added by Tommy Nyquist

Code-Review

+1

2 comments

Patchset-level comments

File-level comment, Patchset 36 (Latest):

Tommy Nyquist . resolved

lgtm

File chrome/browser/sharing/sharing_handler_registry_impl.cc

Line 31, Patchset 36 (Latest):#include "base/feature_list.h"

Tommy Nyquist . unresolved

Nit: This and `chrome/common/chrome_features.h` includes are in the wrong place.

Open in Gerrit

Related details

Attention is currently required from:

Wei Guo

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

open

diffy

Wei Guo (Gerrit)

unread,

Apr 15, 2026, 6:17:06 PM (2 days ago) Apr 15

to Chromium LUCI CQ, Tommy Nyquist, Marke Hallowell, Min Qin, Mikel Astiz, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, derinel+wat...@google.com, webauthn...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, jz...@chromium.org, mar...@chromium.org, devtools...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, feature-me...@chromium.org, fgal...@chromium.org, chrome-intelligence-te...@google.com, dewitt...@chromium.org, knollr+wat...@chromium.org

Wei Guo added 1 comment

File chrome/browser/sharing/sharing_handler_registry_impl.cc

Line 31, Patchset 36 (Latest):#include "base/feature_list.h"

Tommy Nyquist . resolved

Nit: This and `chrome/common/chrome_features.h` includes are in the wrong place.

Wei Guo

Done in a follow up cl 7766470

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

satisfied_requirement

open

diffy

Reply all

Reply to author

Forward

Basic scaffolding of GlicExperimentalTriggeringMessageHandler. [chromium/src : main]

Mikel Astiz (Gerrit)

Mikel Astiz added 9 comments

Related details

Wei Guo (Gerrit)

Wei Guo added 8 comments

Related details

Mikel Astiz (Gerrit)

Mikel Astiz added 6 comments

Related details

Marke Hallowell (Gerrit)

Marke Hallowell voted and added 1 comment

Votes added by Marke Hallowell

1 comment

Related details

Wei Guo (Gerrit)

Wei Guo added 6 comments

Related details

Mikel Astiz (Gerrit)

Mikel Astiz voted and added 4 comments

Votes added by Mikel Astiz

4 comments

Related details

Wei Guo (Gerrit)

Wei Guo added 3 comments

Related details

Marke Hallowell (Gerrit)

Marke Hallowell voted Code-Review+1

Related details

Mikel Astiz (Gerrit)

Mikel Astiz voted Code-Review+1

Related details

Min Qin (Gerrit)

Min Qin voted and added 2 comments

Votes added by Min Qin

2 comments

Related details

Wei Guo (Gerrit)

Wei Guo added 1 comment

Related details

Min Qin (Gerrit)

Min Qin voted Code-Review+1

Related details

Marke Hallowell (Gerrit)

Marke Hallowell voted Code-Review+1

Related details

Wei Guo (Gerrit)

Wei Guo voted Commit-Queue+2

Related details

chromium-scoped@luci-project-accounts.iam.gserviceaccount.com (Gerrit)

chromiu...@luci-project-accounts.iam.gserviceaccount.com submitted the change

Change information

Tommy Nyquist (Gerrit)

Tommy Nyquist voted and added 2 comments

Votes added by Tommy Nyquist

2 comments

Related details

Wei Guo (Gerrit)

Wei Guo added 1 comment

Related details