Keigo Oka (Gerrit)

unread,

May 28, 2026, 11:29:16 PM (3 days ago) May 28

to ben chin, Patrick Noland, manuk hovanesian, Chromium LUCI CQ, chromium...@chromium.org, christia...@chromium.org, ender...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org

Attention needed from Patrick Noland, ben chin and manuk hovanesian

Keigo Oka added 1 comment

Commit Message

Line 9, Patchset 2 (Latest):

- Suppress "Switch to Tab" button exclusively in @tabs site search context by removing the phone-only `!is_desktop()` restriction.

Keigo Oka . unresolved

Can we create a new CL for this if this is orthogonal to the crash fix?

Open in Gerrit

Related details

Attention is currently required from:

Patrick Noland
ben chin
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

ben chin (Gerrit)

unread,

May 28, 2026, 11:53:09 PM (3 days ago) May 28

to Keigo Oka, Patrick Noland, manuk hovanesian, Chromium LUCI CQ, chromium...@chromium.org, christia...@chromium.org, ender...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org

Attention needed from Keigo Oka, Patrick Noland and manuk hovanesian

ben chin voted and added 1 comment

Votes added by ben chin

Commit-Queue

+1

1 comment

Commit Message

Line 9, Patchset 2:

- Suppress "Switch to Tab" button exclusively in @tabs site search context by removing the phone-only `!is_desktop()` restriction.

Keigo Oka . resolved

Can we create a new CL for this if this is orthogonal to the crash fix?

ben chin

sure thing!

Open in Gerrit

Related details

Attention is currently required from:

Keigo Oka
Patrick Noland
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

Keigo Oka (Gerrit)

unread,

May 29, 2026, 12:32:10 AM (3 days ago) May 29

to ben chin, Patrick Noland, manuk hovanesian, Chromium LUCI CQ, chromium...@chromium.org, christia...@chromium.org, ender...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org

Attention needed from Patrick Noland, ben chin and manuk hovanesian

Keigo Oka added 1 comment

File components/omnibox/browser/autocomplete_result.cc

Line 1099, Patchset 4 (Latest):

        if (!match.from_keyword ||
            match.type != AutocompleteMatchType::OPEN_TAB) {

Keigo Oka . unresolved

It looks a little weird to me because the condition for this `if` is the same as the condition for the `else if` below. Why should it be structured like so?

Open in Gerrit

Related details

Attention is currently required from:

Patrick Noland
ben chin
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

ben chin (Gerrit)

unread,

May 29, 2026, 1:38:31 AM (3 days ago) May 29

to Keigo Oka, Patrick Noland, manuk hovanesian, Chromium LUCI CQ, chromium...@chromium.org, christia...@chromium.org, ender...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org

Attention needed from Keigo Oka, Patrick Noland and manuk hovanesian

ben chin voted and added 1 comment

Votes added by ben chin

Commit-Queue

+1

1 comment

File components/omnibox/browser/autocomplete_result.cc

Line 1099, Patchset 4:

        if (!match.from_keyword ||
            match.type != AutocompleteMatchType::OPEN_TAB) {

Keigo Oka . resolved

It looks a little weird to me because the condition for this `if` is the same as the condition for the `else if` below. Why should it be structured like so?

ben chin

Thanks! I eliminates the duplication logic

Open in Gerrit

Related details

Attention is currently required from:

Keigo Oka
Patrick Noland
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

Patrick Noland (Gerrit)

unread,

May 29, 2026, 4:53:33 PM (2 days ago) May 29

to ben chin, Keigo Oka, manuk hovanesian, Chromium LUCI CQ, chromium...@chromium.org, christia...@chromium.org, ender...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org

Attention needed from Keigo Oka, ben chin and manuk hovanesian

Patrick Noland added 1 comment

File components/omnibox/browser/autocomplete_result.cc

Line 1099, Patchset 5 (Latest):

        if constexpr (is_android) {
#if BUILDFLAG(IS_ANDROID)

Patrick Noland . unresolved

Do we need both the if constexpr(is_android) and the BUILDFLAG check? Can we put the      
```


if (!match.from_keyword ||
          match.type != AutocompleteMatchType::OPEN_TAB) {

```

as part of the if constexpr (is_android) to reduce nesting/make this easier to follow?

Open in Gerrit

Related details

Attention is currently required from:

Keigo Oka
ben chin
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

ben chin (Gerrit)

unread,

May 31, 2026, 10:30:29 PM (2 hours ago) May 31

to Keigo Oka, Patrick Noland, manuk hovanesian, Chromium LUCI CQ, chromium...@chromium.org, christia...@chromium.org, ender...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org

Attention needed from Keigo Oka, Patrick Noland and manuk hovanesian

ben chin voted and added 1 comment

Votes added by ben chin

Commit-Queue

+1

1 comment

File components/omnibox/browser/autocomplete_result.cc

Line 1099, Patchset 5:

        if constexpr (is_android) {
#if BUILDFLAG(IS_ANDROID)

Patrick Noland . resolved

Do we need both the if constexpr(is_android) and the BUILDFLAG check? Can we put the      
```
if (!match.from_keyword ||
          match.type != AutocompleteMatchType::OPEN_TAB) {
```

as part of the if constexpr (is_android) to reduce nesting/make this easier to follow?

ben chin

thanks! the `BUILDFLAG(IS_ANDROID)` is redundant. Regarding putting the keyword/type check inside if constexpr : The @tabs keyword mode suppression of the action button is needed for both Android and Desktop (so that we don't show the redundant "Switch to Tab" button on either platform in keyword mode).

Open in Gerrit

Related details

Attention is currently required from:

Keigo Oka
Patrick Noland
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

ben chin (Gerrit)

unread,

May 31, 2026, 10:45:31 PM (2 hours ago) May 31

to Keigo Oka, Patrick Noland, manuk hovanesian, Chromium LUCI CQ, chromium...@chromium.org, christia...@chromium.org, ender...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org

Attention needed from Keigo Oka, Patrick Noland and manuk hovanesian

ben chin voted and added 1 comment

Votes added by ben chin

Commit-Queue

+1

1 comment

File components/omnibox/browser/autocomplete_result.cc

Line 1099, Patchset 5:

        if constexpr (is_android) {
#if BUILDFLAG(IS_ANDROID)

Patrick Noland . resolved

Do we need both the if constexpr(is_android) and the BUILDFLAG check? Can we put the      
```
if (!match.from_keyword ||
          match.type != AutocompleteMatchType::OPEN_TAB) {
```

as part of the if constexpr (is_android) to reduce nesting/make this easier to follow?

ben chin

thanks! the `BUILDFLAG(IS_ANDROID)` is redundant. Regarding putting the keyword/type check inside if constexpr : The @tabs keyword mode suppression of the action button is needed for both Android and Desktop (so that we don't show the redundant "Switch to Tab" button on either platform in keyword mode).

ben chin

Ah ... looks like even though if constexpr (is_android) discards the execution branch on desktop, the compiler still type-checks the discarded branch on non-Android platforms so we still need it

Open in Gerrit

Related details

Attention is currently required from:

Keigo Oka
Patrick Noland
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

Reply all

Reply to author

Forward

Fix Clank @tabs tab-switch button and BaseSuggestionView crash [chromium/src : main]

Keigo Oka (Gerrit)

Keigo Oka added 1 comment

Related details

ben chin (Gerrit)

ben chin voted and added 1 comment

Votes added by ben chin

1 comment

Related details

Keigo Oka (Gerrit)

Keigo Oka added 1 comment

Related details

ben chin (Gerrit)

ben chin voted and added 1 comment

Votes added by ben chin

1 comment

Related details

Patrick Noland (Gerrit)

Patrick Noland added 1 comment

Related details

ben chin (Gerrit)

ben chin voted and added 1 comment

Votes added by ben chin

1 comment

Related details

ben chin (Gerrit)

ben chin voted and added 1 comment

Votes added by ben chin

1 comment

Related details