Commit c88936c2 authored by Tommy Li's avatar Tommy Li Committed by Commit Bot

[omnibox] Stop hardcoding ChromeOS launcher to have no default match

It seems that in the past, we've hardcoded the ChromeOS launcher
page context to have no default match.

But that seems unnecessary, since nothing in chrome/browser/ui/app_list
even looks at the default_match() member.

I'm removing this because we want to keep the Autocomplete controller
as generic as possible without any special cases.

ChromeOS launcher can simply ignore the default match if it wants to,
and indeed it seems to.

The goal of this CL is to unblock removing the default_match_ member
variable entirely, since it's always equal to results_.begin() if that
first match has |allowed_to_be_default_match| set to true.

Bug: 1016845, 363656
Change-Id: Ifd9f72c71c20b3e49bc4fa006353d980f7ba55a3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1913559Reviewed-by: default avatarJenny Zhang <jennyz@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#715521}
parent 11279702
...@@ -271,21 +271,15 @@ void AutocompleteResult::SortAndCull( ...@@ -271,21 +271,15 @@ void AutocompleteResult::SortAndCull(
// 1. There are no matches. // 1. There are no matches.
// 2. The first match doesn't have |allowed_to_be_default_match| as true. // 2. The first match doesn't have |allowed_to_be_default_match| as true.
// This implies that NONE of the matches were allowed to be the default. // This implies that NONE of the matches were allowed to be the default.
// 3. Hardcoded for ChromeOS Launcher empty-textfield on-focus suggestions. if (matches_.empty() || !matches_.begin()->allowed_to_be_default_match) {
// TODO(tommycli): We should remove the ChromeOS launcher special case.
// Instead, ensure none of the launcher matches are allowed to be default.
if (matches_.empty() || !matches_.begin()->allowed_to_be_default_match ||
(input.text().empty() &&
(input.current_page_classification() ==
metrics::OmniboxEventProto::CHROMEOS_APP_LIST))) {
default_match_ = end(); default_match_ = end();
return; return;
} }
// Since we didn't early exit, the first match must be the default match. // Since we didn't early exit, the first match must be the default match.
// TODO(tommycli): Once we eliminate the ChromeOS Launcher hardcoding above, // TODO(tommycli): We can delete |default_match_|, since if matches.begin()
// we can delete |default_match_|, since if matches.begin() has a true // has a true |allowed_to_be_default_match|, it will always be the default
// |allowed_to_be_default_match|, it will always be the default match. // match.
default_match_ = matches_.begin(); default_match_ = matches_.begin();
// Almost all matches are "navigable": they have a valid |destination_url|. // Almost all matches are "navigable": they have a valid |destination_url|.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment