Alexis Wu (Gerrit)

unread,

Mar 17, 2026, 5:35:53 PM (7 days ago) Mar 17

to Shuhei Takahashi, Chromium Sync Reviews, Carlos Knippschild, Chromium LUCI CQ, Hailey Wang, chromium...@chromium.org, devtools...@chromium.org

Attention needed from Carlos Knippschild and Hailey Wang

Alexis Wu added 4 comments

Patchset-level comments

File-level comment, Patchset 15:

Shuhei Takahashi . resolved

FYI: I'm a gardener and I'm reverting crrev.com/c/7655624 that this CL may depend on.

Alexis Wu

Acknowledged

File-level comment, Patchset 24 (Latest):

Alexis Wu . resolved

PTAL, thank you!

Commit Message

Line 9, Patchset 15:Bug: 489517728

Carlos Knippschild . resolved

nit: prefer b:XXX for internal issues.

Alexis Wu

Done

File chrome/browser/glic/glic_pref_names.cc

File-level comment, Patchset 15:

Carlos Knippschild . resolved

For the preferences being marked syncable in this file, IIUC this means they will be synced across all Chrome instances the user is signed in to. That is a significant behavior change and I'm not sure it is expected. Have this been agreed by Product and Privacy?

Alexis Wu

No, for now we are just trying to have the clank version follow the desktop settings. @haile...@google.com might know more on Product decisions.

Hailey Wang

Let's double check with Hena and others. If we are strictly following desktop settings, this does not seem to be syncable.

Carlos Knippschild

I'm not aware of any desktop settings that are account synced, and this CL seems to be changing that.

Alexis Wu

Reverted this change :)

Open in Gerrit

Related details

Attention is currently required from:

Carlos Knippschild
Hailey Wang

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

Carlos Knippschild (Gerrit)

unread,

Mar 17, 2026, 5:46:22 PM (6 days ago) Mar 17

to Alexis Wu, Carlos Knippschild, Shuhei Takahashi, Chromium Sync Reviews, Chromium LUCI CQ, Hailey Wang, chromium...@chromium.org, devtools...@chromium.org

Attention needed from Alexis Wu and Hailey Wang

Carlos Knippschild voted and added 1 comment

Votes added by Carlos Knippschild

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 24 (Latest):

Carlos Knippschild . resolved

RS LGTM

Open in Gerrit

Related details

Attention is currently required from:

Alexis Wu
Hailey Wang

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

Hailey Wang (Gerrit)

unread,

Mar 17, 2026, 5:54:39 PM (6 days ago) Mar 17

to Alexis Wu, Carlos Knippschild, Shuhei Takahashi, Chromium Sync Reviews, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org

Attention needed from Alexis Wu

Hailey Wang voted and added 1 comment

Votes added by Hailey Wang

Code-Review

+1

1 comment

File chrome/browser/glic/android/java/src/org/chromium/chrome/browser/glic/GlicSettings.java

Line 144, Patchset 25 (Latest): mSharedPreferencesManager.writeBoolean(sharedPreferenceKey, value);

Hailey Wang . unresolved

Is it safe to use the prefService here as the source of truth for writing to the share preference? If the prefService was never set, it would return the default value which would overwrite existing values in shared preference.

But maybe that scenario never happens...

Open in Gerrit

Related details

Attention is currently required from:

Alexis Wu

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

Alexis Wu (Gerrit)

unread,

Mar 18, 2026, 6:49:01 PM (5 days ago) Mar 18

to Hailey Wang, Carlos Knippschild, Shuhei Takahashi, Chromium Sync Reviews, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org

Attention needed from Hailey Wang

Alexis Wu added 1 comment

File chrome/browser/glic/android/java/src/org/chromium/chrome/browser/glic/GlicSettings.java

Line 144, Patchset 25 (Latest): mSharedPreferencesManager.writeBoolean(sharedPreferenceKey, value);

Hailey Wang . unresolved

Is it safe to use the prefService here as the source of truth for writing to the share preference? If the prefService was never set, it would return the default value which would overwrite existing values in shared preference.
But maybe that scenario never happens...

Alexis Wu

I think if the user has set it different from default, it should already been saved to prefService?

Open in Gerrit

Related details

Attention is currently required from:

Hailey Wang

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

Hailey Wang (Gerrit)

unread,

Mar 18, 2026, 9:00:03 PM (5 days ago) Mar 18

to Alexis Wu, Carlos Knippschild, Shuhei Takahashi, Chromium Sync Reviews, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org

Attention needed from Alexis Wu

Hailey Wang added 1 comment

File chrome/browser/glic/android/java/src/org/chromium/chrome/browser/glic/GlicSettings.java

Line 144, Patchset 25 (Latest): mSharedPreferencesManager.writeBoolean(sharedPreferenceKey, value);

Hailey Wang . resolved

Is it safe to use the prefService here as the source of truth for writing to the share preference? If the prefService was never set, it would return the default value which would overwrite existing values in shared preference.
But maybe that scenario never happens...

Alexis Wu

I think if the user has set it different from default, it should already been saved to prefService?

Hailey Wang

Acknowledged

Open in Gerrit

Related details

Attention is currently required from:

Alexis Wu

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

Carlos Knippschild (Gerrit)

unread,

Mar 20, 2026, 11:36:49 AM (4 days ago) Mar 20

to Alexis Wu, Carlos Knippschild, Hailey Wang, Shuhei Takahashi, Chromium Sync Reviews, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org

Attention needed from Alexis Wu and Hailey Wang

Carlos Knippschild voted and added 1 comment

Votes added by Carlos Knippschild

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 24:

Carlos Knippschild . resolved

RS LGTM

Carlos Knippschild

Still LGTM

Open in Gerrit

Related details

Attention is currently required from:

Alexis Wu
Hailey Wang

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

Dan Harrington (Gerrit)

unread,

Mar 23, 2026, 12:09:05 PM (17 hours ago) Mar 23

to Alexis Wu, Dan Harrington, Carlos Knippschild, Hailey Wang, Shuhei Takahashi, Chromium Sync Reviews, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org

Attention needed from Alexis Wu

Dan Harrington added 1 comment

File chrome/browser/glic/android/java/src/org/chromium/chrome/browser/glic/GlicSettings.java

Line 137, Patchset 26 (Latest): private <T extends ChromeSwitchPreference> T setupSwitchPreference(

Dan Harrington . unresolved

a comment would be nice on this function. I'm not familiar with the java side prefs logic, and this does look complex.

Open in Gerrit

Related details

Attention is currently required from:

Alexis Wu

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

Alexis Wu (Gerrit)

unread,

Mar 23, 2026, 4:21:38 PM (13 hours ago) Mar 23

to Wenyu Fu, Dan Harrington, Carlos Knippschild, Shuhei Takahashi, Chromium Sync Reviews, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org

Attention needed from Carlos Knippschild, Dan Harrington and Wenyu Fu

Alexis Wu added 2 comments

Patchset-level comments

File-level comment, Patchset 27 (Latest):

Alexis Wu . resolved

Added descriptive function :) PTAL, thanks!

File chrome/browser/glic/android/java/src/org/chromium/chrome/browser/glic/GlicSettings.java

Line 137, Patchset 26: private <T extends ChromeSwitchPreference> T setupSwitchPreference(

Dan Harrington . resolved

a comment would be nice on this function. I'm not familiar with the java side prefs logic, and this does look complex.

Alexis Wu

Done

Open in Gerrit

Related details

Attention is currently required from:

Carlos Knippschild
Dan Harrington
Wenyu Fu

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

Dan Harrington (Gerrit)

unread,

Mar 23, 2026, 4:25:30 PM (13 hours ago) Mar 23

to Alexis Wu, Dan Harrington, Wenyu Fu, Carlos Knippschild, Shuhei Takahashi, Chromium Sync Reviews, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org

Attention needed from Alexis Wu, Carlos Knippschild and Wenyu Fu

Dan Harrington voted and added 1 comment

Votes added by Dan Harrington

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 27 (Latest):

Dan Harrington . resolved

thanks!

Open in Gerrit

Related details

Attention is currently required from:

Alexis Wu
Carlos Knippschild
Wenyu Fu

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

Carlos Knippschild (Gerrit)

unread,

Mar 23, 2026, 5:39:23 PM (12 hours ago) Mar 23

to Alexis Wu, Carlos Knippschild, Dan Harrington, Wenyu Fu, Shuhei Takahashi, Chromium Sync Reviews, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org

Attention needed from Alexis Wu and Wenyu Fu

Carlos Knippschild voted and added 1 comment

Votes added by Carlos Knippschild

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 24:

Carlos Knippschild . resolved

RS LGTM

Carlos Knippschild

Still LGTM

Carlos Knippschild

And yes still LGTM

Open in Gerrit

Related details

Attention is currently required from:

Alexis Wu
Wenyu Fu

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

Alexis Wu (Gerrit)

unread,

Mar 23, 2026, 5:49:42 PM (12 hours ago) Mar 23

to Carlos Knippschild, Dan Harrington, Wenyu Fu, Shuhei Takahashi, Chromium Sync Reviews, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org

Attention needed from Wenyu Fu

Alexis Wu voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention is currently required from:

Wenyu Fu

satisfied_requirement

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

Mar 23, 2026, 6:30:00 PM (11 hours ago) Mar 23

to Alexis Wu, Carlos Knippschild, Dan Harrington, Wenyu Fu, Shuhei Takahashi, Chromium Sync Reviews, chromium...@chromium.org, devtools...@chromium.org

Chromium LUCI CQ submitted the change

Change information

Commit message:

[GlicSettings] Add Native Pref

Link the glic settings with C++ native preferences.

Fixed: b:489517728

Change-Id: Iaeb1ad08ab12ab08b0b46ab12a5180df6b4dab4a

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

Reviewed-by: Dan Harrington <harri...@chromium.org>

Reviewed-by: Carlos Knippschild <car...@chromium.org>

Commit-Queue: Alexis Wu <alex...@google.com>

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

Files:

M chrome/browser/glic/BUILD.gn
M chrome/browser/glic/android/BUILD.gn
M chrome/browser/glic/android/java/src/org/chromium/chrome/browser/glic/GlicSettings.java
M chrome/browser/glic/android/java/src/org/chromium/chrome/browser/glic/GlicSettingsUnitTest.java

Change size: M

Delta: 4 files changed, 116 insertions(+), 30 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Carlos Knippschild, +1 by Dan Harrington

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

[GlicSettings] Add Native Pref [chromium/src : main]

Alexis Wu (Gerrit)

Alexis Wu added 4 comments

Related details

Carlos Knippschild (Gerrit)

Carlos Knippschild voted and added 1 comment

Votes added by Carlos Knippschild

1 comment

Related details

Hailey Wang (Gerrit)

Hailey Wang voted and added 1 comment

Votes added by Hailey Wang

1 comment

Related details

Alexis Wu (Gerrit)

Alexis Wu added 1 comment

Related details

Hailey Wang (Gerrit)

Hailey Wang added 1 comment

Related details

Carlos Knippschild (Gerrit)

Carlos Knippschild voted and added 1 comment

Votes added by Carlos Knippschild

1 comment

Related details

Dan Harrington (Gerrit)

Dan Harrington added 1 comment

Related details

Alexis Wu (Gerrit)

Alexis Wu added 2 comments

Related details

Dan Harrington (Gerrit)

Dan Harrington voted and added 1 comment

Votes added by Dan Harrington

1 comment

Related details

Carlos Knippschild (Gerrit)

Carlos Knippschild voted and added 1 comment

Votes added by Carlos Knippschild

1 comment

Related details

Alexis Wu (Gerrit)

Alexis Wu voted Commit-Queue+2

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change

Change information