Takashi Toyoshima (Gerrit)

unread,

Jul 7, 2026, 2:51:54 AM (5 days ago) Jul 7

to Jiacheng Guo, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, prerendering-reviews, asvitkine...@chromium.org, gavin...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, tburkar...@chromium.org

Attention needed from Jiacheng Guo and Moe Ahmadi

Takashi Toyoshima added 6 comments

File chrome/browser/preloading/android/java/src/org/chromium/chrome/browser/preloading/PreloadingFeatureMap.java

Line 51, Patchset 1 (Latest): return isEnabled("PrewarmZeroSuggestTrigger");

Takashi Toyoshima . unresolved

maybe `isEnabled("Prewarm") && isEnabled("PrewarmZeroSuggestTrigger");` just in case.

Line 58, Patchset 1 (Latest): return isEnabled("Prewarm");

Takashi Toyoshima . unresolved

`isEnabled("Prewarm") && !isEnabled("PrewarmZeroSuggestTrigger");`
Otherwise, both mode may be enabled together for the zero suggest settiong?
(but IIUC, these two methods are added to follow the guideline, but were not used at all?)

File chrome/browser/preloading/prerender/prerender_manager.cc

Line 528, Patchset 1 (Latest): base::FeatureList::IsEnabled(features::kPrewarmZeroSuggestTrigger)) {

Takashi Toyoshima . unresolved

I'm a bit confused on the original code. Do you remember why we need this parameter check here?
It seems this gate should be required always regardless of the trigger type.

File chrome/browser/ui/omnibox/chrome_omnibox_client.cc

Line 1019, Patchset 1 (Parent): CHECK(features::kPrewarmUserInteractionTrigger.Get());

Takashi Toyoshima . unresolved

We would keep this as `CHECK(!ase::FeatureList::IsEnabled(features::kPrewarmZeroSuggestTrigger));`?

File chrome/browser/ui/omnibox/omnibox_controller.cc

Line 92, Patchset 1 (Latest): if (base::FeatureList::IsEnabled(features::kPrewarmZeroSuggestTrigger)) {

Takashi Toyoshima . unresolved

Should we keep the base kPrewarm check too? Theoretically they may be configured differently, and need a check here to avoid CHECK failures.

Line 121, Patchset 1 (Latest): if (base::FeatureList::IsEnabled(features::kPrewarm)) {

Takashi Toyoshima . unresolved

Similarly !base... check too.

Open in Gerrit

Related details

Attention is currently required from:

Jiacheng Guo
Moe Ahmadi

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

Jiacheng Guo (Gerrit)

unread,

Jul 7, 2026, 3:37:27 AM (5 days ago) Jul 7

to Takashi Toyoshima, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, prerendering-reviews, asvitkine...@chromium.org, gavin...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, tburkar...@chromium.org

Attention needed from Moe Ahmadi and Takashi Toyoshima

Jiacheng Guo added 1 comment

File chrome/browser/preloading/prerender/prerender_manager.cc

Line 528, Patchset 1: base::FeatureList::IsEnabled(features::kPrewarmZeroSuggestTrigger)) {

Takashi Toyoshima . unresolved

I'm a bit confused on the original code. Do you remember why we need this parameter check here?
It seems this gate should be required always regardless of the trigger type.

Jiacheng Guo

Checked the code and it seems that we missed to add `kPrewarmUserInteractionTrigger` to the if clause when adding the feature parameter. Yes this should always be checked.

Open in Gerrit

Related details

Attention is currently required from:

Moe Ahmadi
Takashi Toyoshima

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

Jiacheng Guo (Gerrit)

unread,

Jul 7, 2026, 4:07:53 AM (5 days ago) Jul 7

to Takashi Toyoshima, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, prerendering-reviews, asvitkine...@chromium.org, gavin...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, tburkar...@chromium.org

Attention needed from Moe Ahmadi and Takashi Toyoshima

Jiacheng Guo added 5 comments

File chrome/browser/preloading/android/java/src/org/chromium/chrome/browser/preloading/PreloadingFeatureMap.java

Line 51, Patchset 1: return isEnabled("PrewarmZeroSuggestTrigger");

Takashi Toyoshima . resolved

maybe `isEnabled("Prewarm") && isEnabled("PrewarmZeroSuggestTrigger");` just in case.

Jiacheng Guo

Done

Line 58, Patchset 1: return isEnabled("Prewarm");

Takashi Toyoshima . resolved

`isEnabled("Prewarm") && !isEnabled("PrewarmZeroSuggestTrigger");`
Otherwise, both mode may be enabled together for the zero suggest settiong?
(but IIUC, these two methods are added to follow the guideline, but were not used at all?)

Jiacheng Guo

Done

File chrome/browser/preloading/prerender/prerender_manager.cc

Line 528, Patchset 1: base::FeatureList::IsEnabled(features::kPrewarmZeroSuggestTrigger)) {

Takashi Toyoshima . resolved

I'm a bit confused on the original code. Do you remember why we need this parameter check here?
It seems this gate should be required always regardless of the trigger type.

Jiacheng Guo

Checked the code and it seems that we missed to add `kPrewarmUserInteractionTrigger` to the if clause when adding the feature parameter. Yes this should always be checked.

Jiacheng Guo

Done

File chrome/browser/ui/omnibox/omnibox_controller.cc

Line 92, Patchset 1: if (base::FeatureList::IsEnabled(features::kPrewarmZeroSuggestTrigger)) {

Takashi Toyoshima . resolved

Should we keep the base kPrewarm check too? Theoretically they may be configured differently, and need a check here to avoid CHECK failures.

Jiacheng Guo

Done

Line 121, Patchset 1: if (base::FeatureList::IsEnabled(features::kPrewarm)) {

Takashi Toyoshima . resolved

Similarly !base... check too.

Jiacheng Guo

Done

Open in Gerrit

Related details

Attention is currently required from:

Moe Ahmadi
Takashi Toyoshima

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

Takashi Toyoshima (Gerrit)

unread,

Jul 7, 2026, 4:25:45 AM (5 days ago) Jul 7

to Jiacheng Guo, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, prerendering-reviews, asvitkine...@chromium.org, gavin...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, tburkar...@chromium.org

Attention needed from Jiacheng Guo and Moe Ahmadi

Takashi Toyoshima voted and added 2 comments

Votes added by Takashi Toyoshima

Code-Review

+1

2 comments

Patchset-level comments

File-level comment, Patchset 3 (Latest):

Takashi Toyoshima . resolved

LGTM!

File chrome/browser/ui/omnibox/chrome_omnibox_client.cc

Line 1019, Patchset 1 (Parent): CHECK(features::kPrewarmUserInteractionTrigger.Get());

Takashi Toyoshima . resolved

We would keep this as `CHECK(!ase::FeatureList::IsEnabled(features::kPrewarmZeroSuggestTrigger));`?

Takashi Toyoshima

Seems done.

Open in Gerrit

Related details

Attention is currently required from:

Jiacheng Guo
Moe Ahmadi

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

manuk hovanesian (Gerrit)

unread,

Jul 9, 2026, 5:02:47 PM (2 days ago) Jul 9

to Jiacheng Guo, Takashi Toyoshima, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, prerendering-reviews, asvitkine...@chromium.org, gavin...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, tburkar...@chromium.org

Attention needed from Jiacheng Guo

manuk hovanesian voted and added 4 comments

Votes added by manuk hovanesian

Code-Review

+1

4 comments

Patchset-level comments

File-level comment, Patchset 3 (Latest):

manuk hovanesian . resolved

lgtm

File chrome/browser/preloading/prerender/prerender_manager_unittest.cc

Line 354, Patchset 3 (Latest): // Set default search provider to match features::kPrewarmUrl.

manuk hovanesian . unresolved

nit, wrap comment symbols in ticks

    // ... `kPre`.

Line 358, Patchset 3 (Latest): TemplateURLData template_url_data;

manuk hovanesian . unresolved

missing include components/search_engines/template_url_data.h

File chrome/browser/preloading/scoped_prewarm_feature_list.cc

Line 11, Patchset 3 (Latest):ScopedPrewarmFeatureList::ScopedPrewarmFeatureList(PrewarmState state) {

manuk hovanesian . unresolved

optional nit, if this file is only for testing, consider adding some test_ or testing_ affix to the filename

Open in Gerrit

Related details

Attention is currently required from:

Jiacheng 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

Jiacheng Guo (Gerrit)

unread,

Jul 10, 2026, 12:47:15 AM (yesterday) Jul 10

to manuk hovanesian, Takashi Toyoshima, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, prerendering-reviews, asvitkine...@chromium.org, gavin...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, tburkar...@chromium.org

Attention needed from Takashi Toyoshima and manuk hovanesian

Jiacheng Guo added 1 comment

File chrome/browser/preloading/scoped_prewarm_feature_list.cc

Line 11, Patchset 3:ScopedPrewarmFeatureList::ScopedPrewarmFeatureList(PrewarmState state) {

manuk hovanesian . resolved

optional nit, if this file is only for testing, consider adding some test_ or testing_ affix to the filename

Jiacheng Guo

Added a TODO linking to a clean-up bug (crbug.com/533212496) to rename this file later.

Open in Gerrit

Related details

Attention is currently required from:

Takashi Toyoshima
manuk hovanesian

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

Jiacheng Guo (Gerrit)

unread,

Jul 10, 2026, 12:48:28 AM (yesterday) Jul 10

to manuk hovanesian, Takashi Toyoshima, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, prerendering-reviews, asvitkine...@chromium.org, gavin...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, tburkar...@chromium.org

Attention needed from Takashi Toyoshima and manuk hovanesian

Jiacheng Guo added 2 comments

File chrome/browser/preloading/prerender/prerender_manager_unittest.cc

Line 354, Patchset 3: // Set default search provider to match features::kPrewarmUrl.

manuk hovanesian . resolved

nit, wrap comment symbols in ticks
    // ... `kPre`.

Jiacheng Guo

Done

Line 358, Patchset 3: TemplateURLData template_url_data;

manuk hovanesian . resolved

missing include components/search_engines/template_url_data.h

Jiacheng Guo

Done

Open in Gerrit

Related details

Attention is currently required from:

Takashi Toyoshima
manuk hovanesian

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

Takashi Toyoshima (Gerrit)

unread,

Jul 10, 2026, 1:54:05 AM (yesterday) Jul 10

to Jiacheng Guo, manuk hovanesian, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, prerendering-reviews, asvitkine...@chromium.org, gavin...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, tburkar...@chromium.org

Attention needed from Jiacheng Guo and manuk hovanesian

Takashi Toyoshima voted and added 1 comment

Votes added by Takashi Toyoshima

Code-Review

+1

1 comment

File chrome/browser/preloading/prerender/prerender_manager.cc

Line 528, Patchset 1: base::FeatureList::IsEnabled(features::kPrewarmZeroSuggestTrigger)) {

Takashi Toyoshima . resolved

I'm a bit confused on the original code. Do you remember why we need this parameter check here?
It seems this gate should be required always regardless of the trigger type.

Jiacheng Guo

Checked the code and it seems that we missed to add `kPrewarmUserInteractionTrigger` to the if clause when adding the feature parameter. Yes this should always be checked.

Jiacheng Guo

Done

Takashi Toyoshima

thank you for double checking!

Open in Gerrit

Related details

Attention is currently required from:

Jiacheng Guo
manuk hovanesian

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

Jiacheng Guo (Gerrit)

unread,

Jul 10, 2026, 2:55:28 AM (yesterday) Jul 10

to Takashi Toyoshima, manuk hovanesian, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, prerendering-reviews, asvitkine...@chromium.org, gavin...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, tburkar...@chromium.org

Attention needed from manuk hovanesian

Jiacheng Guo voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention is currently required from:

manuk hovanesian

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

Jiacheng Guo (Gerrit)

unread,

Jul 10, 2026, 3:28:01 AM (yesterday) Jul 10

to Takashi Toyoshima, manuk hovanesian, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, prerendering-reviews, asvitkine...@chromium.org, gavin...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, tburkar...@chromium.org

Attention needed from manuk hovanesian

Jiacheng Guo voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention is currently required from:

manuk hovanesian

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

Jiacheng Guo (Gerrit)

unread,

Jul 10, 2026, 10:02:25 PM (19 hours ago) Jul 10

to Takashi Toyoshima, manuk hovanesian, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, prerendering-reviews, asvitkine...@chromium.org, gavin...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, tburkar...@chromium.org

satisfied_requirement

open

diffy

Jiacheng Guo (Gerrit)

unread,

10:46 AM (6 hours ago) 10:46 AM

to Takashi Toyoshima, manuk hovanesian, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, prerendering-reviews, asvitkine...@chromium.org, gavin...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, tburkar...@chromium.org

Attention needed from manuk hovanesian

Jiacheng Guo voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention is currently required from:

manuk hovanesian

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

Chromium LUCI CQ (Gerrit)

unread,

11:28 AM (6 hours ago) 11:28 AM

to Jiacheng Guo, Takashi Toyoshima, manuk hovanesian, Chromium Metrics Reviews, chromium...@chromium.org, prerendering-reviews, asvitkine...@chromium.org, gavin...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, tburkar...@chromium.org

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

4 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: chrome/browser/preloading/prerender/prerender_manager_unittest.cc
Insertions: 3, Deletions: 3.

The diff is too large to show. Please review the diff.
```

Change information

Commit message:

Split the kPrewarmZeroSuggestTrigger feature param into a separate feature.

The CL cleans up the prewarm trigger feature parameters.
The kPrewarmZeroSuggestTrigger feature param is moved into a separate
feature (PrewarmZeroSuggestTrigger) to test it in a follow-up
experiment. The kPrewarmUserInteractionTrigger feature param is removed.

The CL also cleans up the filed trial testing config of DSEPrewarm.

Bug: 531885903

Change-Id: I3145318cb35bbebd726f63226eac798dd7853535

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

Commit-Queue: Jiacheng Guo <g...@google.com>

Reviewed-by: Takashi Toyoshima <toyo...@chromium.org>

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

Files:

M chrome/browser/preloading/android/java/src/org/chromium/chrome/browser/preloading/PreloadingFeatureMap.java
M chrome/browser/preloading/android/preloading_feature_map.cc
M chrome/browser/preloading/preloading_features.cc
M chrome/browser/preloading/preloading_features.h
M chrome/browser/preloading/prerender/prerender_manager.cc
M chrome/browser/preloading/prerender/prerender_manager_unittest.cc
M chrome/browser/preloading/scoped_prewarm_feature_list.cc
M chrome/browser/preloading/scoped_prewarm_feature_list.h
M chrome/browser/ui/omnibox/chrome_omnibox_client.cc
M chrome/browser/ui/omnibox/omnibox_controller.cc
M testing/variations/fieldtrial_testing_config.json

Change size: M

Delta: 11 files changed, 83 insertions(+), 101 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Takashi Toyoshima

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

Split the kPrewarmZeroSuggestTrigger feature param into a separate feature. [chromium/src : main]

Takashi Toyoshima (Gerrit)

Takashi Toyoshima added 6 comments

Related details

Jiacheng Guo (Gerrit)

Jiacheng Guo added 1 comment

Related details

Jiacheng Guo (Gerrit)

Jiacheng Guo added 5 comments

Related details

Takashi Toyoshima (Gerrit)

Takashi Toyoshima voted and added 2 comments

Votes added by Takashi Toyoshima

2 comments

Related details

manuk hovanesian (Gerrit)

manuk hovanesian voted and added 4 comments

Votes added by manuk hovanesian

4 comments

Related details

Jiacheng Guo (Gerrit)

Jiacheng Guo added 1 comment

Related details

Jiacheng Guo (Gerrit)

Jiacheng Guo added 2 comments

Related details

Takashi Toyoshima (Gerrit)

Takashi Toyoshima voted and added 1 comment

Votes added by Takashi Toyoshima

1 comment

Related details

Jiacheng Guo (Gerrit)

Jiacheng Guo voted Commit-Queue+2

Related details

Jiacheng Guo (Gerrit)

Jiacheng Guo voted Commit-Queue+2

Related details

Jiacheng Guo (Gerrit)

Jiacheng Guo (Gerrit)

Jiacheng Guo voted Commit-Queue+2

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

Change information