Commit e23c56b8 authored by Tomasz Wiszkowski's avatar Tomasz Wiszkowski Committed by Commit Bot

Create a TILE_NAVSUGGEST suggestion.

This change introduces a TILE_NAVSUGGEST suggestion type for the
most-visited suggestions.
The TILE_NAVSUGGEST is built in a similar way to NAVSUGGEST and
TILE_SUGGEST elements, where a list of tiles is held as a vector of
elements arranged in the order these are scored.

The TILE_NAVSUGGEST is currently meant specifically for zero-prefix
most-visited URL suggestions and should be positioned below the
QueryTiles. These elements are currently not meant to be
cached as these URLs are used specifically when user is visiting a
website (ie. not a ZPS on a NTP).

The change does not create any logic wiring up TILE_NAVSUGGEST items;
instead, it simply creates a new suggestion type and adds
corresponding data to that object on both Native and Java sides.

Change-Id: Ia4c5141143d6a49f963bcf66fecae9b84d3d4b5e
Doc: http://doc/1IFsDKYhy_ARMOdT8jd1pMtk497xe03lb3hVnsaY-dOM
Slides: http://slides/1c1SzJLzEsTjLmqIVLTomC8l9EgslBgYaBN6I7t8mbDI#slide=id.g8b3736bab3_0_0
Figma: https://www.figma.com/file/h8qjD0s9tezRzHdCMxIhYJ/On-Focus-Suggestions?node-id=290%3A309
Bug: 1106109
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2422808
Commit-Queue: Tomasz Wiszkowski <ender@google.com>
Reviewed-by: default avatarJustin Donnelly <jdonnelly@chromium.org>
Reviewed-by: default avatarTed Choc <tedchoc@chromium.org>
Reviewed-by: default avatarFilip Gorski <fgorski@chromium.org>
Reviewed-by: default avatarTommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810285}
parent 669d4f82
......@@ -23,6 +23,7 @@ import org.chromium.chrome.browser.ntp.NewTabPage;
import org.chromium.chrome.browser.omnibox.OmniboxSuggestionType;
import org.chromium.chrome.browser.omnibox.suggestions.AutocompleteResult.GroupDetails;
import org.chromium.chrome.browser.omnibox.suggestions.OmniboxSuggestion.MatchClassification;
import org.chromium.chrome.browser.omnibox.suggestions.OmniboxSuggestion.NavsuggestTile;
import org.chromium.chrome.browser.omnibox.voice.VoiceRecognitionHandler.VoiceResult;
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.tab.Tab;
......@@ -353,7 +354,8 @@ public class AutocompleteController {
int[] descriptionClassificationStyles, SuggestionAnswer answer, String fillIntoEdit,
GURL url, GURL imageUrl, String imageDominantColor, boolean isStarred,
boolean isDeletable, String postContentType, byte[] postData, int groupId,
List<QueryTile> tiles, byte[] clipboardImageData, boolean hasTabMatch) {
List<QueryTile> tiles, byte[] clipboardImageData, boolean hasTabMatch,
List<NavsuggestTile> navsuggestTiles) {
assert contentClassificationOffsets.length == contentClassificationStyles.length;
List<MatchClassification> contentClassifications = new ArrayList<>();
for (int i = 0; i < contentClassificationOffsets.length; i++) {
......@@ -376,7 +378,19 @@ public class AutocompleteController {
return new OmniboxSuggestion(nativeType, subtypes, isSearchType, relevance, transition,
contents, contentClassifications, description, descriptionClassifications, answer,
fillIntoEdit, url, imageUrl, imageDominantColor, isStarred, isDeletable,
postContentType, postData, groupId, tiles, clipboardImageData, hasTabMatch);
postContentType, postData, groupId, tiles, clipboardImageData, hasTabMatch,
navsuggestTiles);
}
@CalledByNative
private static List<NavsuggestTile> buildOmniboxNavsuggestTileList(int capacity) {
return new ArrayList<>(capacity);
}
@CalledByNative
private static void addOmniboxNavsuggestTile(
List<NavsuggestTile> tiles, String title, GURL url) {
tiles.add(new NavsuggestTile(title, url));
}
/**
......
......@@ -185,7 +185,7 @@ public class CachedZeroSuggestionsManager {
OmniboxSuggestion suggestion = new OmniboxSuggestion(nativeType, subtypes, isSearchType,
0, 0, displayText, classifications, description, classifications, null, null,
url, GURL.emptyGURL(), null, isStarred, isDeletable, postContentType, postData,
groupId, null, null, false);
groupId, null, null, false, null);
suggestions.add(suggestion);
}
......@@ -280,7 +280,8 @@ public class CachedZeroSuggestionsManager {
return !suggestion.hasAnswer()
&& suggestion.getType() != OmniboxSuggestionType.CLIPBOARD_URL
&& suggestion.getType() != OmniboxSuggestionType.CLIPBOARD_TEXT
&& suggestion.getType() != OmniboxSuggestionType.CLIPBOARD_IMAGE;
&& suggestion.getType() != OmniboxSuggestionType.CLIPBOARD_IMAGE
&& suggestion.getType() != OmniboxSuggestionType.TILE_NAVSUGGEST;
}
/**
......
......@@ -27,6 +27,25 @@ public class OmniboxSuggestion {
public static final int INVALID_GROUP = -1;
public static final int INVALID_TYPE = -1;
/**
* Specifies an individual tile for TILE_NAVSUGGEST suggestions.
*/
public static class NavsuggestTile {
/**
* Title of the website the tile points to.
*/
public final String title;
/**
* URL of the website the tile points to.
*/
public final GURL url;
public NavsuggestTile(String title, GURL url) {
this.title = title;
this.url = url;
}
}
/**
* Specifies the style of portions of the suggestion text.
* <p>
......@@ -79,6 +98,7 @@ public class OmniboxSuggestion {
private final List<QueryTile> mQueryTiles;
private final byte[] mClipboardImageData;
private final boolean mHasTabMatch;
private final @Nullable List<NavsuggestTile> mNavsuggestTiles;
public OmniboxSuggestion(int nativeType, Set<Integer> subtypes, boolean isSearchType,
int relevance, int transition, String displayText,
......@@ -86,8 +106,8 @@ public class OmniboxSuggestion {
List<MatchClassification> descriptionClassifications, SuggestionAnswer answer,
String fillIntoEdit, GURL url, GURL imageUrl, String imageDominantColor,
boolean isStarred, boolean isDeletable, String postContentType, byte[] postData,
int groupId, List<QueryTile> queryTiles, byte[] clipboardImageData,
boolean hasTabMatch) {
int groupId, List<QueryTile> queryTiles, byte[] clipboardImageData, boolean hasTabMatch,
List<NavsuggestTile> navsuggestTiles) {
if (subtypes == null) {
subtypes = Collections.emptySet();
}
......@@ -115,6 +135,7 @@ public class OmniboxSuggestion {
mQueryTiles = queryTiles;
mClipboardImageData = clipboardImageData;
mHasTabMatch = hasTabMatch;
mNavsuggestTiles = navsuggestTiles;
}
public int getType() {
......@@ -263,6 +284,13 @@ public class OmniboxSuggestion {
return mGroupId;
}
/**
* @return List of tiles for TILE_NAVSUGGEST suggestion.
*/
public @Nullable List<NavsuggestTile> getNavsuggestTiles() {
return mNavsuggestTiles;
}
@Override
public String toString() {
List<String> pieces = Arrays.asList("mType=" + mType, "mSubtypes=" + mSubtypes.toString(),
......
......@@ -124,7 +124,7 @@ class VoiceSuggestionProvider {
suggestions.add(new OmniboxSuggestion(OmniboxSuggestionType.VOICE_SUGGEST, null, true, 0, 1,
result.getMatch(), classifications, null, classifications, null, null, voiceUrl,
GURL.emptyGURL(), null, false, false, null, null, OmniboxSuggestion.INVALID_GROUP,
null, null, false));
null, null, false, null));
}
private boolean doesVoiceResultHaveMatch(
......
......@@ -44,6 +44,7 @@ public class OmniboxSuggestionBuilderForTest {
private List<QueryTile> mQueryTiles;
private byte[] mClipboardImageData;
private boolean mHasTabMatch;
private List<OmniboxSuggestion.NavsuggestTile> mNavsuggestTiles;
/**
* Create a suggestion builder for a search suggestion.
......@@ -88,7 +89,7 @@ public class OmniboxSuggestionBuilderForTest {
mDisplayText, mDisplayTextClassifications, mDescription,
mDescriptionClassifications, mAnswer, mFillIntoEdit, mUrl, mImageUrl,
mImageDominantColor, mIsStarred, mIsDeletable, mPostContentType, mPostData,
mGroupId, mQueryTiles, mClipboardImageData, mHasTabMatch);
mGroupId, mQueryTiles, mClipboardImageData, mHasTabMatch, mNavsuggestTiles);
}
/**
......
......@@ -615,6 +615,8 @@ AutocompleteControllerAndroid::BuildOmniboxSuggestion(
ScopedJavaLocalRef<jobject> j_query_tiles =
query_tiles::TileConversionBridge::CreateJavaTiles(env,
match.query_tiles);
ScopedJavaLocalRef<jobject> j_navsuggest_tiles =
BuildNavsuggestTilesList(env, match.navsuggest_tiles);
BookmarkModel* bookmark_model =
BookmarkModelFactory::GetForBrowserContext(profile_);
......@@ -633,7 +635,27 @@ AutocompleteControllerAndroid::BuildOmniboxSuggestion(
match.suggestion_group_id.value_or(
SearchSuggestionParser::kNoSuggestionGroupId),
j_query_tiles, ToJavaByteArray(env, clipboard_image_data),
match.has_tab_match);
match.has_tab_match, j_navsuggest_tiles);
}
base::android::ScopedJavaLocalRef<jobject>
AutocompleteControllerAndroid::BuildNavsuggestTilesList(
JNIEnv* env,
const std::vector<AutocompleteMatch::NavsuggestTile>& tiles) {
if (tiles.empty())
return ScopedJavaLocalRef<jobject>();
ScopedJavaLocalRef<jobject> j_navsuggest_tiles =
Java_AutocompleteController_buildOmniboxNavsuggestTileList(env,
tiles.size());
for (const auto& tile : tiles) {
ScopedJavaLocalRef<jstring> title =
ConvertUTF16ToJavaString(env, tile.title);
ScopedJavaLocalRef<jobject> url =
url::GURLAndroid::FromNativeGURL(env, tile.url);
Java_AutocompleteController_addOmniboxNavsuggestTile(
env, j_navsuggest_tiles, title, url);
}
return j_navsuggest_tiles;
}
void AutocompleteControllerAndroid::PopulateOmniboxGroupsDetails(
......
......@@ -134,6 +134,11 @@ class AutocompleteControllerAndroid : public AutocompleteController::Observer,
base::android::ScopedJavaLocalRef<jobject> BuildOmniboxSuggestion(
JNIEnv* env, const AutocompleteMatch& match);
// Construct Java list of NavsuggestTile objects.
base::android::ScopedJavaLocalRef<jobject> BuildNavsuggestTilesList(
JNIEnv* env,
const std::vector<AutocompleteMatch::NavsuggestTile>& tiles);
// Construct Java GroupDetails map from supplied HeadersMap and expanded
// state.
void PopulateOmniboxGroupsDetails(
......
......@@ -116,6 +116,7 @@ const gfx::VectorIcon& TypeToVectorIcon(AutocompleteMatchType::Type type) {
case AutocompleteMatchType::EXTENSION_APP_DEPRECATED:
case AutocompleteMatchType::TILE_SUGGESTION:
case AutocompleteMatchType::TILE_NAVSUGGEST:
case AutocompleteMatchType::NUM_TYPES:
NOTREACHED();
break;
......@@ -231,6 +232,7 @@ ash::SearchResultType OmniboxResult::GetSearchResultType() const {
case AutocompleteMatchType::CLIPBOARD_IMAGE:
case AutocompleteMatchType::HISTORY_BODY:
case AutocompleteMatchType::TILE_SUGGESTION:
case AutocompleteMatchType::TILE_NAVSUGGEST:
case AutocompleteMatchType::NUM_TYPES:
// TODO(crbug.com/1028447): Add a NOTREACHED here once we are confident we
// know all possible types for this result.
......
......@@ -183,7 +183,8 @@ AutocompleteMatch::AutocompleteMatch(const AutocompleteMatch& match)
: nullptr),
additional_info(match.additional_info),
duplicate_matches(match.duplicate_matches),
query_tiles(match.query_tiles) {}
query_tiles(match.query_tiles),
navsuggest_tiles(match.navsuggest_tiles) {}
AutocompleteMatch::AutocompleteMatch(AutocompleteMatch&& match) noexcept =
default;
......@@ -243,6 +244,7 @@ AutocompleteMatch& AutocompleteMatch::operator=(
additional_info = match.additional_info;
duplicate_matches = match.duplicate_matches;
query_tiles = match.query_tiles;
navsuggest_tiles = match.navsuggest_tiles;
return *this;
}
......@@ -264,6 +266,7 @@ const gfx::VectorIcon& AutocompleteMatch::GetVectorIcon(
case Type::PHYSICAL_WEB_DEPRECATED:
case Type::PHYSICAL_WEB_OVERFLOW_DEPRECATED:
case Type::TAB_SEARCH_DEPRECATED:
case Type::TILE_NAVSUGGEST:
return omnibox::kPageIcon;
case Type::SEARCH_SUGGEST: {
......@@ -414,8 +417,9 @@ base::string16 AutocompleteMatch::GetWhyThisSuggestionText() const {
case Type::PHYSICAL_WEB_DEPRECATED:
case Type::PHYSICAL_WEB_OVERFLOW_DEPRECATED:
case Type::TAB_SEARCH_DEPRECATED:
case Type::NUM_TYPES:
case Type::TILE_SUGGESTION:
case Type::TILE_NAVSUGGEST:
case Type::NUM_TYPES:
NOTREACHED();
return base::string16();
}
......@@ -975,6 +979,8 @@ AutocompleteMatch::AsOmniboxEventResultType() const {
return OmniboxEventProto::Suggestion::CLIPBOARD_IMAGE;
case AutocompleteMatchType::TILE_SUGGESTION:
return OmniboxEventProto::Suggestion::TILE_SUGGESTION;
case AutocompleteMatchType::TILE_NAVSUGGEST:
return OmniboxEventProto::Suggestion::NAVSUGGEST;
case AutocompleteMatchType::VOICE_SUGGEST:
// VOICE_SUGGEST matches are only used in Java and are not logged,
// so we should never reach this case.
......
......@@ -106,6 +106,15 @@ struct AutocompleteMatch {
int style;
};
// NavsuggestTiles are used specifically with TILE_NAVSUGGEST matches.
// This structure should describe only the specific details for individual
// tiles; all other properties are considered as shared and should be
// extracted from the encompassing AutocompleteMatch object.
struct NavsuggestTile {
GURL url;
base::string16 title;
};
typedef std::vector<ACMatchClassification> ACMatchClassifications;
// Type used by providers to attach additional, optional information to
......@@ -642,6 +651,10 @@ struct AutocompleteMatch {
// A list of query tiles to be shown as part of this match.
std::vector<query_tiles::Tile> query_tiles;
// A list of navsuggest tiles to be shown as part of this match.
// This object is only populated for TILE_NAVSUGGEST AutocompleteMatches.
std::vector<NavsuggestTile> navsuggest_tiles;
// So users of AutocompleteMatch can use the same ellipsis that it uses.
static const char kEllipsis[];
......
......@@ -49,6 +49,7 @@ std::string AutocompleteMatchType::ToString(AutocompleteMatchType::Type type) {
"text-from-clipboard",
"image-from-clipboard",
"query-tiles",
"navsuggest-tiles",
};
// clang-format on
static_assert(base::size(strings) == AutocompleteMatchType::NUM_TYPES,
......@@ -128,6 +129,7 @@ base::string16 GetAccessibilityBaseLabel(const AutocompleteMatch& match,
IDS_ACC_AUTOCOMPLETE_CLIPBOARD_TEXT, // CLIPBOARD_TEXT
IDS_ACC_AUTOCOMPLETE_CLIPBOARD_IMAGE, // CLIPBOARD_IMAGE
0, // TILE_SUGGESTION
0, // TILE_NAVSUGGEST
};
static_assert(base::size(message_ids) == AutocompleteMatchType::NUM_TYPES,
"message_ids must have NUM_TYPES elements");
......
......@@ -73,6 +73,7 @@ struct AutocompleteMatchType {
CLIPBOARD_TEXT = 26, // Text based on the clipboard.
CLIPBOARD_IMAGE = 27, // An image based on the clipboard.
TILE_SUGGESTION = 28, // A suggestion containing query tiles.
TILE_NAVSUGGEST = 29, // A suggestion with navigation tiles.
NUM_TYPES,
};
// clang-format on
......
......@@ -91,6 +91,9 @@ void LogOmniboxZeroSuggestRequest(
// Relevance value to use if it was not set explicitly by the server.
const int kDefaultZeroSuggestRelevance = 100;
// The relevance score for navsuggest tiles.
// Navsuggest tiles should be positioned below the Query Tiles object.
const int kMostVisitedTilesRelevance = 1500;
// Used for testing whether zero suggest is ever available.
constexpr char kArbitraryInsecureUrlString[] = "http://www.google.com/";
......@@ -477,16 +480,34 @@ void ZeroSuggestProvider::ConvertResultsToAutocompleteMatches() {
return;
}
matches_.push_back(current_text_match_);
int relevance = 600;
const base::string16 current_query_string16(
base::ASCIIToUTF16(current_query_));
for (const auto& url : most_visited_urls_) {
SearchSuggestionParser::NavigationResult nav(
client()->GetSchemeClassifier(), url.url,
AutocompleteMatchType::NAVSUGGEST, {}, url.title, std::string(),
false, relevance, true, current_query_string16);
matches_.push_back(NavigationToMatch(nav));
--relevance;
// Short-circuit in case we have no MOST_VISITED urls to show.
if (most_visited_urls_.empty())
return;
if (base::FeatureList::IsEnabled(omnibox::kMostVisitedTiles)) {
AutocompleteMatch match =
NavigationToMatch(SearchSuggestionParser::NavigationResult(
client()->GetSchemeClassifier(), GURL::EmptyGURL(),
AutocompleteMatchType::TILE_NAVSUGGEST, {}, base::string16(),
std::string(), false, kMostVisitedTilesRelevance, true,
base::ASCIIToUTF16(current_query_)));
match.navsuggest_tiles.reserve(most_visited_urls_.size());
for (const auto& url : most_visited_urls_) {
match.navsuggest_tiles.push_back({url.url, url.title});
}
matches_.push_back(std::move(match));
} else {
int relevance = 600;
for (const auto& url : most_visited_urls_) {
SearchSuggestionParser::NavigationResult nav(
client()->GetSchemeClassifier(), url.url,
AutocompleteMatchType::NAVSUGGEST, {}, url.title, std::string(),
false, relevance, true, base::ASCIIToUTF16(current_query_));
matches_.push_back(NavigationToMatch(nav));
--relevance;
}
}
return;
}
......
......@@ -59,6 +59,7 @@ OmniboxSuggestionIconType GetOmniboxSuggestionIconTypeForAutocompleteMatchType(
return CALCULATOR;
case AutocompleteMatchType::EXTENSION_APP_DEPRECATED:
case AutocompleteMatchType::TILE_SUGGESTION:
case AutocompleteMatchType::TILE_NAVSUGGEST:
case AutocompleteMatchType::NUM_TYPES:
NOTREACHED();
return DEFAULT_FAVICON;
......
......@@ -82,6 +82,7 @@ OmniboxSuggestionIconType IconTypeFromMatchAndAnswerType(
return CALCULATOR;
case AutocompleteMatchType::EXTENSION_APP_DEPRECATED:
case AutocompleteMatchType::TILE_SUGGESTION:
case AutocompleteMatchType::TILE_NAVSUGGEST:
case AutocompleteMatchType::NUM_TYPES:
NOTREACHED();
return DEFAULT_FAVICON;
......
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