Andrea Gomez (Gerrit)

unread,

May 26, 2026, 4:21:11 PM (6 days ago) May 26

to Justin DeWitt, Chromium IPC Reviews, Anurag Simgeker, Thorsten Kober, Chromium Metrics Reviews, Zewen Li, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, mfoltz+wa...@chromium.org, asvitkine...@chromium.org, chromium-a...@chromium.org, dewitt...@chromium.org, extension...@chromium.org

Attention needed from Chromium IPC Reviews and Justin DeWitt

Andrea Gomez voted and added 1 comment

Votes added by Andrea Gomez

Commit-Queue

+1

1 comment

Patchset-level comments

File-level comment, Patchset 12 (Latest):

Andrea Gomez . resolved

ptal!

Open in Gerrit

Related details

Attention is currently required from:

Chromium IPC Reviews
Justin DeWitt

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

gwsq (Gerrit)

unread,

May 26, 2026, 4:24:53 PM (6 days ago) May 26

to Chromium IPC Reviews, Daniel Cheng, Justin DeWitt, Anurag Simgeker, Thorsten Kober, Chromium Metrics Reviews, Zewen Li, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, mfoltz+wa...@chromium.org, asvitkine...@chromium.org, chromium-a...@chromium.org, dewitt...@chromium.org, extension...@chromium.org

Attention needed from Daniel Cheng and Justin DeWitt

Message from gwsq

From googleclient/chrome/chromium_gwsq/ipc/config.gwsq:
IPC: dch...@chromium.org

📎 It looks like you’re making a possibly security-sensitive change! 📎 IPC security review isn’t a rubberstamp, so your friendly security reviewer will need a fair amount of context to review your CL effectively. Please review your CL description and code comments to make sure they provide context for someone unfamiliar with your project/area. Pay special attention to where data comes from and which processes it flows between (and their privilege levels). Feel free to point your security reviewer at design docs, bugs, or other links if you can’t reasonably make a self-contained CL description. (Also see https://cbea.ms/git-commit/).

IPC reviewer(s): dch...@chromium.org

Reviewer source(s):
dch...@chromium.org is from context(googleclient/chrome/chromium_gwsq/ipc/config.gwsq)

Open in Gerrit

Related details

Attention is currently required from:

Daniel Cheng
Justin DeWitt

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

Daniel Cheng (Gerrit)

unread,

May 26, 2026, 5:02:08 PM (6 days ago) May 26

to Andrea Gomez, Daniel Cheng, Chromium IPC Reviews, Justin DeWitt, Anurag Simgeker, Thorsten Kober, Chromium Metrics Reviews, Zewen Li, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, mfoltz+wa...@chromium.org, asvitkine...@chromium.org, chromium-a...@chromium.org, dewitt...@chromium.org, extension...@chromium.org

Attention needed from Andrea Gomez and Justin DeWitt

Daniel Cheng voted and added 1 comment

Votes added by Daniel Cheng

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 13 (Latest):

Daniel Cheng . resolved

LGTM

Open in Gerrit

Related details

Attention is currently required from:

Andrea Gomez
Justin DeWitt

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

Andrea Gomez (Gerrit)

unread,

May 26, 2026, 5:26:21 PM (6 days ago) May 26

to Toni Barzic, Daniel Cheng, Chromium IPC Reviews, Justin DeWitt, Anurag Simgeker, Thorsten Kober, Chromium Metrics Reviews, Zewen Li, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, mfoltz+wa...@chromium.org, asvitkine...@chromium.org, chromium-a...@chromium.org, dewitt...@chromium.org, extension...@chromium.org

Attention needed from Justin DeWitt and Toni Barzic

Andrea Gomez added 1 comment

Patchset-level comments

File-level comment, Patchset 14 (Latest):

Andrea Gomez . resolved

ptal!

Open in Gerrit

Related details

Attention is currently required from:

Justin DeWitt
Toni Barzic

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

Justin DeWitt (Gerrit)

unread,

May 27, 2026, 4:01:33 PM (5 days ago) May 27

to Andrea Gomez, Toni Barzic, Daniel Cheng, Chromium IPC Reviews, Anurag Simgeker, Thorsten Kober, Chromium Metrics Reviews, Zewen Li, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, mfoltz+wa...@chromium.org, asvitkine...@chromium.org, chromium-a...@chromium.org, dewitt...@chromium.org, extension...@chromium.org

Attention needed from Andrea Gomez, Daniel Cheng, Justin DeWitt and Toni Barzic

Justin DeWitt voted and added 2 comments

Votes added by Justin DeWitt

Code-Review

+1

2 comments

File chrome/browser/extensions/api/glic_private/glic_messaging_browsertest.cc

Line 188, Patchset 14: content::EvalJs(GetActiveWebContents(), script);

Justin DeWitt . unresolved

might be worth pulling into a helper?

File chrome/browser/extensions/api/glic_private/glic_private_api.cc

Line 433, Patchset 14: options.fre_override = glic::mojom::FreOverride::kTrustFirstInline;

Justin DeWitt . unresolved

are we sure we want this override in Chrome client code?

Open in Gerrit

Related details

Attention is currently required from:

Andrea Gomez
Daniel Cheng
Justin DeWitt
Toni Barzic

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

Andrea Gomez (Gerrit)

unread,

May 27, 2026, 4:53:36 PM (5 days ago) May 27

to Justin DeWitt, Toni Barzic, Daniel Cheng, Chromium IPC Reviews, Anurag Simgeker, Thorsten Kober, Chromium Metrics Reviews, Zewen Li, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, mfoltz+wa...@chromium.org, asvitkine...@chromium.org, chromium-a...@chromium.org, dewitt...@chromium.org, extension...@chromium.org

Attention needed from Daniel Cheng, Justin DeWitt and Toni Barzic

Andrea Gomez added 3 comments

Patchset-level comments

File-level comment, Patchset 18 (Latest):

Andrea Gomez . resolved

thank you ptal!

File chrome/browser/extensions/api/glic_private/glic_messaging_browsertest.cc

Line 188, Patchset 14: content::EvalJs(GetActiveWebContents(), script);

Justin DeWitt . resolved

might be worth pulling into a helper?

Andrea Gomez

Done

File chrome/browser/extensions/api/glic_private/glic_private_api.cc

Line 433, Patchset 14: options.fre_override = glic::mojom::FreOverride::kTrustFirstInline;

Justin DeWitt . resolved

are we sure we want this override in Chrome client code?

Andrea Gomez

True. it would be better and more flexibly handled web side. I also removed the tests because they weren't able to test the fre anyways and were redundant smoke tests.

Open in Gerrit

Related details

Attention is currently required from:

Daniel Cheng
Justin DeWitt
Toni Barzic

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

Daniel Cheng (Gerrit)

unread,

May 27, 2026, 6:13:01 PM (5 days ago) May 27

to Andrea Gomez, Daniel Cheng, Justin DeWitt, Toni Barzic, Chromium IPC Reviews, Anurag Simgeker, Thorsten Kober, Chromium Metrics Reviews, Zewen Li, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, mfoltz+wa...@chromium.org, asvitkine...@chromium.org, chromium-a...@chromium.org, dewitt...@chromium.org, extension...@chromium.org

Attention needed from Andrea Gomez, Justin DeWitt and Toni Barzic

Daniel Cheng voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Andrea Gomez
Justin DeWitt
Toni Barzic

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

Kelvin Jiang (Gerrit)

unread,

May 29, 2026, 4:47:25 PM (3 days ago) May 29

to Andrea Gomez, Daniel Cheng, Justin DeWitt, Chromium IPC Reviews, Anurag Simgeker, Thorsten Kober, Chromium Metrics Reviews, Zewen Li, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, mfoltz+wa...@chromium.org, asvitkine...@chromium.org, chromium-a...@chromium.org, dewitt...@chromium.org, extension...@chromium.org

Attention needed from Andrea Gomez and Justin DeWitt

Kelvin Jiang added 1 comment

Patchset-level comments

File-level comment, Patchset 21 (Latest):

Kelvin Jiang . resolved

hi Andrea which files do you want me to take a look at? I don't think I'm listed as an OWNER for anything here

Open in Gerrit

Related details

Attention is currently required from:

Andrea Gomez
Justin DeWitt

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

Tim (Gerrit)

unread,

May 29, 2026, 9:05:32 PM (3 days ago) May 29

to Andrea Gomez, Daniel Cheng, Justin DeWitt, Chromium IPC Reviews, Anurag Simgeker, Thorsten Kober, Chromium Metrics Reviews, Zewen Li, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, mfoltz+wa...@chromium.org, asvitkine...@chromium.org, chromium-a...@chromium.org, dewitt...@chromium.org, extension...@chromium.org

Attention needed from Andrea Gomez and Justin DeWitt

Tim voted and added 4 comments

Votes added by Tim

Code-Review

+1

4 comments

Patchset-level comments

File-level comment, Patchset 21 (Latest):

Tim . resolved

Thanks! LGTM with just a few nits for making things a bit easier to understand through the comments.

File chrome/browser/extensions/api/glic_private/glic_private_api.cc

Line 438, Patchset 21 (Latest): // fetching the prompt from the server and proceed directly.

Tim . unresolved

nit: add `, passing nullopt for the prompt.` or something similar.

Line 446, Patchset 21 (Latest): extensions::api::glic_private::ErrorCode::kNone, std::nullopt));

Tim . unresolved

nit: a `/*prompt=*/` would make this call a lot clearer.

File chrome/common/extensions/api/glic_private.webidl

Line 45, Patchset 21 (Latest): // The prompt ID to lookup from Chrome.

Tim . unresolved

nit: add `, required unless called from the promotion page.`

Open in Gerrit

Related details

Attention is currently required from:

Andrea Gomez
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

satisfied_requirement

unsatisfied_requirement

open

diffy

Andrea Gomez (Gerrit)

unread,

12:22 PM (5 hours ago) 12:22 PM

to Tim, Daniel Cheng, Justin DeWitt, Chromium IPC Reviews, Anurag Simgeker, Thorsten Kober, Chromium Metrics Reviews, Zewen Li, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, mfoltz+wa...@chromium.org, asvitkine...@chromium.org, chromium-a...@chromium.org, dewitt...@chromium.org, extension...@chromium.org

Attention needed from Justin DeWitt

Andrea Gomez added 4 comments

Patchset-level comments

File-level comment, Patchset 22 (Latest):

Andrea Gomez . resolved

thanks!

File chrome/browser/extensions/api/glic_private/glic_private_api.cc

Line 438, Patchset 21: // fetching the prompt from the server and proceed directly.

Tim . resolved

nit: add `, passing nullopt for the prompt.` or something similar.

Andrea Gomez

Done

Line 446, Patchset 21: extensions::api::glic_private::ErrorCode::kNone, std::nullopt));

Tim . resolved

nit: a `/*prompt=*/` would make this call a lot clearer.

Andrea Gomez

Done

File chrome/common/extensions/api/glic_private.webidl

Line 45, Patchset 21: // The prompt ID to lookup from Chrome.

Tim . resolved

nit: add `, required unless called from the promotion page.`

Andrea Gomez

Done

Open in Gerrit

Related details

Attention is currently required from:

Justin DeWitt

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

Justin DeWitt (Gerrit)

unread,

12:41 PM (4 hours ago) 12:41 PM

to Andrea Gomez, Tim, Daniel Cheng, Chromium IPC Reviews, Anurag Simgeker, Thorsten Kober, Chromium Metrics Reviews, Zewen Li, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, mfoltz+wa...@chromium.org, asvitkine...@chromium.org, chromium-a...@chromium.org, dewitt...@chromium.org, extension...@chromium.org

Attention needed from Andrea Gomez

Justin DeWitt added 2 comments

File chrome/browser/extensions/api/glic_private/glic_private_api.cc

Line 407, Patchset 22 (Latest): if (is_valid_source &&

Justin DeWitt . unresolved

this name is a little bit odd - should it be source_requires_google_webpage_access or similar? I am worried about adding all sorts of little corner cases to the logic for ACL type of changes.

Should we put this into the switch directly?

File chrome/browser/resources/glic/extension/background.ts

Line 44, Patchset 22 (Latest):

        if (!details.promptId &&
            details.invocationSource !== 'promotion-page') {
          throw new Error('missing promptId');
        }

Justin DeWitt . unresolved

I don't think we should duplicate this logic in the extension and in c++, we should pick a layer to enforce this and just do it there.

Open in Gerrit

Related details

Attention is currently required from:

Andrea Gomez

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

Justin DeWitt (Gerrit)

unread,

2:30 PM (2 hours ago) 2:30 PM

to Andrea Gomez, Tim, Daniel Cheng, Chromium IPC Reviews, Anurag Simgeker, Thorsten Kober, Chromium Metrics Reviews, Zewen Li, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, mfoltz+wa...@chromium.org, asvitkine...@chromium.org, chromium-a...@chromium.org, dewitt...@chromium.org, extension...@chromium.org

Attention needed from Andrea Gomez

Justin DeWitt voted and added 2 comments

Votes added by Justin DeWitt

Code-Review	+1
Commit-Queue	+1

2 comments

File chrome/browser/extensions/api/glic_private/glic_private_api.cc

Line 407, Patchset 22 (Latest): if (is_valid_source &&

Justin DeWitt . resolved

this name is a little bit odd - should it be source_requires_google_webpage_access or similar? I am worried about adding all sorts of little corner cases to the logic for ACL type of changes.
Should we put this into the switch directly?

Justin DeWitt

This is ok as a followup.

File chrome/browser/resources/glic/extension/background.ts

Line 44, Patchset 22 (Latest):

        if (!details.promptId &&
            details.invocationSource !== 'promotion-page') {
          throw new Error('missing promptId');
        }

Justin DeWitt . resolved

I don't think we should duplicate this logic in the extension and in c++, we should pick a layer to enforce this and just do it there.

Justin DeWitt

This is okay as a followup.

Open in Gerrit

Related details

Attention is currently required from:

Andrea Gomez

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

Andrea Gomez (Gerrit)

unread,

2:38 PM (2 hours ago) 2:38 PM

to Justin DeWitt, Tim, Daniel Cheng, Chromium IPC Reviews, Anurag Simgeker, Thorsten Kober, Chromium Metrics Reviews, Zewen Li, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, mfoltz+wa...@chromium.org, asvitkine...@chromium.org, chromium-a...@chromium.org, dewitt...@chromium.org, extension...@chromium.org

Andrea Gomez voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention set is empty

satisfied_requirement

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

2:43 PM (2 hours ago) 2:43 PM

to Andrea Gomez, Justin DeWitt, Tim, Daniel Cheng, Chromium IPC Reviews, Anurag Simgeker, Thorsten Kober, Chromium Metrics Reviews, Zewen Li, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, mfoltz+wa...@chromium.org, asvitkine...@chromium.org, chromium-a...@chromium.org, dewitt...@chromium.org, extension...@chromium.org

Chromium LUCI CQ submitted the change

Change information

Commit message:

Add promotion-page invocation source to glicPrivate API

This change adds the "promotion-page" invocation source to the
glicPrivate API.

Guarded by the ApiGlicAccessFromGoogleWebpage feature flag.

Bug: b:516850705

Change-Id: I0cab488a5b154d5ba867a2493cd616b196d07852

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

Reviewed-by: Justin DeWitt <dew...@chromium.org>

Reviewed-by: Daniel Cheng <dch...@chromium.org>

Commit-Queue: Andrea Gomez <andr...@google.com>

Reviewed-by: Tim <tjud...@chromium.org>

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

Files:

M chrome/browser/extensions/api/glic_private/glic_messaging_browsertest.cc
M chrome/browser/extensions/api/glic_private/glic_private_api.cc
M chrome/browser/glic/host/glic.mojom
M chrome/browser/glic/service/metrics/metrics_types.cc
M chrome/browser/glic/service/metrics/metrics_types.h
M chrome/browser/resources/glic/extension/background.ts
M chrome/browser/resources/glic/extension/glic_private.d.ts
M chrome/browser/resources/glic/glic_api/glic_api.ts
M chrome/common/extensions/api/glic_private.webidl
M chrome/common/glic_enums.mojom
M tools/metrics/histograms/metadata/glic/enums.xml

Change size: M

Delta: 11 files changed, 140 insertions(+), 44 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Tim, +1 by Daniel Cheng, +1 by Justin DeWitt

Open in Gerrit

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

Gerrit

open

diffy

satisfied_requirement

Reply all

Reply to author

Forward

Add promotion-page invocation source to glicPrivate API [chromium/src : main]

Andrea Gomez (Gerrit)

Andrea Gomez voted and added 1 comment

Votes added by Andrea Gomez

1 comment

Related details

gwsq (Gerrit)

Message from gwsq

Related details

Daniel Cheng (Gerrit)

Daniel Cheng voted and added 1 comment

Votes added by Daniel Cheng

1 comment

Related details

Andrea Gomez (Gerrit)

Andrea Gomez added 1 comment

Related details

Justin DeWitt (Gerrit)

Justin DeWitt voted and added 2 comments

Votes added by Justin DeWitt

2 comments

Related details

Andrea Gomez (Gerrit)

Andrea Gomez added 3 comments

Related details

Daniel Cheng (Gerrit)

Daniel Cheng voted Code-Review+1

Related details

Kelvin Jiang (Gerrit)

Kelvin Jiang added 1 comment

Related details

Tim (Gerrit)

Tim voted and added 4 comments

Votes added by Tim

4 comments

Related details

Andrea Gomez (Gerrit)

Andrea Gomez added 4 comments

Related details

Justin DeWitt (Gerrit)

Justin DeWitt added 2 comments

Related details

Justin DeWitt (Gerrit)

Justin DeWitt voted and added 2 comments

Votes added by Justin DeWitt

2 comments

Related details

Andrea Gomez (Gerrit)

Andrea Gomez voted Commit-Queue+2

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change

Change information