Commit c37e0ef2 authored by Tomasz Wiszkowski's avatar Tomasz Wiszkowski Committed by Ender

Do not include specialized suggestions in grouping.

This change replicates an update in native SortAndCull method to
skip past specialized suggestions when partially grouping suggestions
by search vs url.

The specialized suggestion types that are intentionally skipped are:
- default match
- query tiles
- clipboard

Corresponding native change: https://crrev.com/c/2266012

Bug: 1091098
Change-Id: If9446f60561f013948876a7ec38ff87e0cd8deed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2268228Reviewed-by: default avatarFilip Gorski <fgorski@chromium.org>
Reviewed-by: default avatarTed Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#785752}
parent aa990afd
......@@ -19,6 +19,7 @@ import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.image_fetcher.ImageFetcher;
import org.chromium.chrome.browser.image_fetcher.ImageFetcherConfig;
import org.chromium.chrome.browser.image_fetcher.ImageFetcherFactory;
import org.chromium.chrome.browser.omnibox.OmniboxSuggestionType;
import org.chromium.chrome.browser.omnibox.UrlBarEditingTextStateProvider;
import org.chromium.chrome.browser.omnibox.suggestions.answer.AnswerSuggestionProcessor;
import org.chromium.chrome.browser.omnibox.suggestions.basic.BasicSuggestionProcessor;
......@@ -341,6 +342,46 @@ class DropdownItemViewInfoListBuilder {
void groupSuggestionsBySearchVsURL(
List<Pair<OmniboxSuggestion, SuggestionProcessor>> suggestionsPairedWithProcessors,
int numVisibleSuggestions) {
final int firstIndexWithHeader = findFirstIndexWithHeader(suggestionsPairedWithProcessors);
final int firstIndexForGrouping =
findFirstIndexForGrouping(suggestionsPairedWithProcessors);
// Check if we have any suggestions we can group.
if (firstIndexWithHeader <= firstIndexForGrouping) return;
// Compute the index of first concealed element as a start of the second group.
// This addresses the situation, where all visible and some concealed suggestions are
// specialized (eg. visible default match, query tiles and concealed clipboard suggestion).
final int firstIndexInConcealedGroup = Math.max(
Math.min(numVisibleSuggestions, firstIndexWithHeader), firstIndexForGrouping);
// Comparator addressing the suggestion grouping.
final Comparator<Pair<OmniboxSuggestion, SuggestionProcessor>> comparator =
(pair1, pair2) -> {
if (pair1.first.isSearchSuggestion() != pair2.first.isSearchSuggestion()) {
return pair1.first.isSearchSuggestion() ? -1 : 1;
}
return pair2.first.getRelevance() - pair1.first.getRelevance();
};
// Sort visible part of suggestions list.
if (firstIndexForGrouping < firstIndexInConcealedGroup) {
Collections.sort(suggestionsPairedWithProcessors.subList(
firstIndexForGrouping, firstIndexInConcealedGroup),
comparator);
}
// Sort the concealed part of suggestions list.
if (firstIndexInConcealedGroup < firstIndexWithHeader) {
Collections.sort(suggestionsPairedWithProcessors.subList(
firstIndexInConcealedGroup, firstIndexWithHeader),
comparator);
}
}
/** @return Index of the first suggestion decorated with a suggestion header. */
private int findFirstIndexWithHeader(
List<Pair<OmniboxSuggestion, SuggestionProcessor>> suggestionsPairedWithProcessors) {
// Native counterpart ensures that suggestion with group headers always end up at the
// end of the list. This guarantees that these suggestions are both grouped at the end
// of the list and that there's nothing more we should do about them. See
......@@ -354,28 +395,32 @@ class DropdownItemViewInfoListBuilder {
break;
}
}
return firstIndexWithHeader;
}
// Make sure we do not accidentally rearrange grouped suggestions.
if (numVisibleSuggestions > firstIndexWithHeader) {
numVisibleSuggestions = firstIndexWithHeader;
}
if (numVisibleSuggestions == 0) return;
final Comparator<Pair<OmniboxSuggestion, SuggestionProcessor>> comparator =
(pair1, pair2) -> {
if (pair1.first.isSearchSuggestion() != pair2.first.isSearchSuggestion()) {
return pair1.first.isSearchSuggestion() ? -1 : 1;
/**
* @return Index of the first element that should be used to group suggestions by
* search vs URL.
*/
private int findFirstIndexForGrouping(
List<Pair<OmniboxSuggestion, SuggestionProcessor>> suggestionsPairedWithProcessors) {
int firstIndexForGrouping;
// Find the first suggestion that will be the subject for grouping by search vs url.
// Note that the first suggestion is the default match and we never change it.
for (firstIndexForGrouping = 1;
firstIndexForGrouping < suggestionsPairedWithProcessors.size();
firstIndexForGrouping++) {
final @OmniboxSuggestionType int type =
suggestionsPairedWithProcessors.get(firstIndexForGrouping).first.getType();
if (type != OmniboxSuggestionType.TILE_SUGGESTION
&& type != OmniboxSuggestionType.CLIPBOARD_TEXT
&& type != OmniboxSuggestionType.CLIPBOARD_URL
&& type != OmniboxSuggestionType.CLIPBOARD_IMAGE) {
break;
}
return pair2.first.getRelevance() - pair1.first.getRelevance();
};
// Note: the first match is always the default match. We do not want to sort it.
Collections.sort(
suggestionsPairedWithProcessors.subList(1, numVisibleSuggestions), comparator);
Collections.sort(suggestionsPairedWithProcessors.subList(
numVisibleSuggestions, firstIndexWithHeader),
comparator);
}
return firstIndexForGrouping;
}
/**
......
......@@ -503,4 +503,55 @@ public class DropdownItemViewInfoListBuilderUnitTest {
mBuilder.buildDropdownViewInfoList(result);
Assert.assertFalse(mBuilder.hasFullyConcealedElements());
}
@NativeJavaTestFeatures.Enable(ChromeFeatureList.OMNIBOX_ADAPTIVE_SUGGESTIONS_COUNT)
@CalledByNativeJavaTest
public void grouping_verifySpecializedSuggestionsAreNotIncludedInGrouping() {
mBuilder.onNativeInitialized();
final int viewHeight = 10;
final OmniboxSuggestionBuilderForTest builder =
OmniboxSuggestionBuilderForTest
.searchWithType(OmniboxSuggestionType.SEARCH_WHAT_YOU_TYPED)
.setIsSearch(false) // Pretend all specialized suggestions are URLs so that
// these would get demoted.
.setRelevance(1);
final OmniboxSuggestion defaultSuggestion = builder.build();
final OmniboxSuggestion tileSuggestion =
builder.setType(OmniboxSuggestionType.TILE_SUGGESTION).build();
final OmniboxSuggestion clipboardTextSuggestion =
builder.setType(OmniboxSuggestionType.CLIPBOARD_TEXT).build();
final OmniboxSuggestion clipboardImageSuggestion =
builder.setType(OmniboxSuggestionType.CLIPBOARD_IMAGE).build();
final OmniboxSuggestion clipboardUrlSuggestion =
builder.setType(OmniboxSuggestionType.CLIPBOARD_URL).build();
final OmniboxSuggestion searchSuggestion =
builder.setRelevance(100)
.setIsSearch(true)
.setType(OmniboxSuggestionType.SEARCH_SUGGEST)
.build();
final OmniboxSuggestion urlSuggestion =
builder.setRelevance(100).setIsSearch(false).build();
final List<Pair<OmniboxSuggestion, SuggestionProcessor>> pairs = Arrays.asList(
new Pair(defaultSuggestion, null), // Default match, never participates in grouping.
new Pair(clipboardUrlSuggestion, null), // Clipboard, specialized suggestion.
new Pair(tileSuggestion, null), // Query tiles, specialized suggestion.
new Pair(clipboardTextSuggestion, null), // Clipboard, specialized suggestion.
new Pair(clipboardImageSuggestion, null), // Clipboard, specialized suggestion.
new Pair(searchSuggestion, null), new Pair(searchSuggestion, null),
new Pair(urlSuggestion, null), new Pair(urlSuggestion, null),
new Pair(searchSuggestion, null), new Pair(searchSuggestion, null));
final List<Pair<OmniboxSuggestion, SuggestionProcessor>> expected = Arrays.asList(
// Specialized suggestions are in the same order as received.
pairs.get(0), pairs.get(1), pairs.get(2), pairs.get(3), pairs.get(4),
// Other suggestions get grouped.
pairs.get(5), pairs.get(6), pairs.get(9), pairs.get(10), pairs.get(7),
pairs.get(8));
mBuilder.groupSuggestionsBySearchVsURL(pairs, pairs.size());
verifyListsMatch(expected, pairs);
}
}
......@@ -211,4 +211,13 @@ public class OmniboxSuggestionBuilderForTest {
mRelevance = relevance;
return this;
}
/**
* @param type Suggestion type.
* @return Omnibox suggestion builder.
*/
public OmniboxSuggestionBuilderForTest setType(@OmniboxSuggestionType int type) {
mType = type;
return this;
}
}
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