Commit bde3d009 authored by Moe Ahmadi's avatar Moe Ahmadi Committed by Commit Bot

[PZPS] Adds the PZPS header text information to the Autocomplete model

The PZPS header information is expected in the following format in the
Chrome suggest response. Every suggestion with a header has a header group
id in its respective "google:suggestdetail" dictionary which maps to
a header text inside "google:headertexts>a".

...
"google:headertexts":{
  "a":{
    "40009":"Recommended for you"
  }
},
"google:suggestdetail":[
  ...
  {
    "zl":40009
  },
  {
    "zl":40009
  }
],
...

This CL parses that information and adds the respective header as a
|header| field on the first ACMatch in every group of adjacent matches
with the same header, unless:
1) Matches with the same header are not adjacent to one another, i.e.,
   are interleaved by matches with different headers or matches without
   one.
2) Matches without a header come after the ones with headers.

Note that the headers do not determine classification or ranking of
the matches. They are expected to be valid after the
AutocompleteResult is finalized. These error conditions, however, are
in place to safeguard the integrity of the displayed headers.

Bug: 1052519
Change-Id: Ibbd3a812690605cd13763e41f109197ef75c1727
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2079303Reviewed-by: default avatarTommy Li <tommycli@chromium.org>
Commit-Queue: Moe Ahmadi <mahmadi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746130}
parent 103eac84
...@@ -653,6 +653,7 @@ void AutocompleteController::UpdateResult( ...@@ -653,6 +653,7 @@ void AutocompleteController::UpdateResult(
result_); result_);
} }
UpdateHeaders(&result_);
UpdateKeywordDescriptions(&result_); UpdateKeywordDescriptions(&result_);
UpdateAssociatedKeywords(&result_); UpdateAssociatedKeywords(&result_);
UpdateAssistedQueryStats(&result_); UpdateAssistedQueryStats(&result_);
...@@ -741,6 +742,37 @@ void AutocompleteController::UpdateAssociatedKeywords( ...@@ -741,6 +742,37 @@ void AutocompleteController::UpdateAssociatedKeywords(
} }
} }
void AutocompleteController::UpdateHeaders(AutocompleteResult* result) {
auto clear_header = [](AutocompleteMatch& match) {
return match.header.clear();
};
base::string16 last_seen_header;
std::set<base::string16> all_seen_headers;
for (auto it(result->begin()); it != result->end(); ++it) {
const base::string16& current_match_header = it->header;
if (!current_match_header.empty()) {
if (current_match_header == last_seen_header) {
// This header is identical to the previous one. Clear it and move on.
clear_header(*it);
continue;
}
if (all_seen_headers.count(current_match_header)) {
// A header cannot be interleaved by other headers. Clear all headers.
std::for_each(result->begin(), result->end(), clear_header);
return;
}
// This header is seen for the first time. Record it and move on.
last_seen_header = current_match_header;
all_seen_headers.insert(current_match_header);
} else if (!all_seen_headers.empty()) {
// An empty header cannot follow a non-empty one. Clear all headers.
std::for_each(result->begin(), result->end(), clear_header);
return;
}
}
}
void AutocompleteController::UpdateKeywordDescriptions( void AutocompleteController::UpdateKeywordDescriptions(
AutocompleteResult* result) { AutocompleteResult* result) {
base::string16 last_keyword; base::string16 last_keyword;
......
...@@ -192,6 +192,19 @@ class AutocompleteController : public AutocompleteProviderListener, ...@@ -192,6 +192,19 @@ class AutocompleteController : public AutocompleteProviderListener,
// relevance before this is called. // relevance before this is called.
void UpdateAssociatedKeywords(AutocompleteResult* result); void UpdateAssociatedKeywords(AutocompleteResult* result);
// For each group of adjacent matches with the same header, keeps the header
// for the first match in that group and clears the rest of the headers. If
// any of the following error conditions occur, headers are deemed invalid and
// are entirely discarded:
// 1) Matches with the same header are not adjacent to one another, i.e., are
// interleaved by matches with different headers or matches without one.
// 2) Matches without a header come after the ones with headers.
// Note that the headers do not determine classification or ranking of the
// matches. They are expected to be valid after the AutocompleteResult is
// finalized. These error conditions, however, are in place to safeguard the
// integrity of the displayed headers.
void UpdateHeaders(AutocompleteResult* result);
// For each group of contiguous matches from the same TemplateURL, show the // For each group of contiguous matches from the same TemplateURL, show the
// provider name as a description on the first match in the group. // provider name as a description on the first match in the group.
void UpdateKeywordDescriptions(AutocompleteResult* result); void UpdateKeywordDescriptions(AutocompleteResult* result);
......
...@@ -141,6 +141,7 @@ AutocompleteMatch::AutocompleteMatch(const AutocompleteMatch& match) ...@@ -141,6 +141,7 @@ AutocompleteMatch::AutocompleteMatch(const AutocompleteMatch& match)
contents_class(match.contents_class), contents_class(match.contents_class),
description(match.description), description(match.description),
description_class(match.description_class), description_class(match.description_class),
header(match.header),
swap_contents_and_description(match.swap_contents_and_description), swap_contents_and_description(match.swap_contents_and_description),
answer(match.answer), answer(match.answer),
transition(match.transition), transition(match.transition),
...@@ -195,6 +196,7 @@ AutocompleteMatch& AutocompleteMatch::operator=( ...@@ -195,6 +196,7 @@ AutocompleteMatch& AutocompleteMatch::operator=(
contents_class = match.contents_class; contents_class = match.contents_class;
description = match.description; description = match.description;
description_class = match.description_class; description_class = match.description_class;
header = match.header;
swap_contents_and_description = match.swap_contents_and_description; swap_contents_and_description = match.swap_contents_and_description;
answer = match.answer; answer = match.answer;
transition = match.transition; transition = match.transition;
......
...@@ -549,6 +549,10 @@ struct AutocompleteMatch { ...@@ -549,6 +549,10 @@ struct AutocompleteMatch {
base::string16 description; base::string16 description;
ACMatchClassifications description_class; ACMatchClassifications description_class;
// An optional header text this match must appear under. Currently only
// zero-prefix matches may have a header.
base::string16 header;
// If true, UI-level code should swap the contents and description fields // If true, UI-level code should swap the contents and description fields
// before displaying. // before displaying.
// This field is set when matches are appended to autocomplete results via // This field is set when matches are appended to autocomplete results via
......
...@@ -241,6 +241,10 @@ class AutocompleteProviderTest : public testing::Test { ...@@ -241,6 +241,10 @@ class AutocompleteProviderTest : public testing::Test {
const base::string16 expected_associated_keyword; const base::string16 expected_associated_keyword;
}; };
struct HeaderTestData {
std::string header;
};
struct AssistedQueryStatsTestData { struct AssistedQueryStatsTestData {
const AutocompleteMatch::Type match_type; const AutocompleteMatch::Type match_type;
const std::string expected_aqs; const std::string expected_aqs;
...@@ -270,6 +274,9 @@ class AutocompleteProviderTest : public testing::Test { ...@@ -270,6 +274,9 @@ class AutocompleteProviderTest : public testing::Test {
const KeywordTestData* match_data, const KeywordTestData* match_data,
size_t size); size_t size);
void UpdateResultsWithHeaderTestData(const HeaderTestData* match_data,
size_t size);
void RunAssistedQueryStatsTest( void RunAssistedQueryStatsTest(
const AssistedQueryStatsTestData* aqs_test_data, const AssistedQueryStatsTestData* aqs_test_data,
size_t size); size_t size);
...@@ -508,6 +515,25 @@ void AutocompleteProviderTest::RunKeywordTest(const base::string16& input, ...@@ -508,6 +515,25 @@ void AutocompleteProviderTest::RunKeywordTest(const base::string16& input,
} }
} }
void AutocompleteProviderTest::UpdateResultsWithHeaderTestData(
const HeaderTestData* match_data,
size_t size) {
// Prepare input.
const size_t kMaxRelevance = 1000;
ACMatches matches;
for (size_t i = 0; i < size; ++i) {
AutocompleteMatch match(nullptr, kMaxRelevance - i, false,
AutocompleteMatchType::SEARCH_SUGGEST_PERSONALIZED);
match.header = base::ASCIIToUTF16(match_data[i].header);
matches.push_back(match);
}
result_.Reset();
result_.AppendMatches(AutocompleteInput(), matches);
// Update headers.
controller_->UpdateHeaders(&result_);
}
void AutocompleteProviderTest::RunAssistedQueryStatsTest( void AutocompleteProviderTest::RunAssistedQueryStatsTest(
const AssistedQueryStatsTestData* aqs_test_data, const AssistedQueryStatsTestData* aqs_test_data,
size_t size) { size_t size) {
...@@ -727,6 +753,68 @@ TEST_F(AutocompleteProviderTest, ExactMatchKeywords) { ...@@ -727,6 +753,68 @@ TEST_F(AutocompleteProviderTest, ExactMatchKeywords) {
} }
} }
// Tests that match headers are updated correctly.
TEST_F(AutocompleteProviderTest, Headers) {
using base::ASCIIToUTF16;
ResetControllerWithTestProviders(false, nullptr, nullptr);
const char kRecentSearchesHeader[] = "Recent Searches";
const char kRecommendedForYouHeader[] = "Recommended for you";
{
// The first header in every group of adjacent identical headers is kept.
HeaderTestData test_data[] = {{""},
{""},
{kRecentSearchesHeader},
{kRecentSearchesHeader},
{kRecommendedForYouHeader},
{kRecommendedForYouHeader}};
UpdateResultsWithHeaderTestData(test_data, base::size(test_data));
EXPECT_EQ(ASCIIToUTF16(""), result_.match_at(0)->header);
EXPECT_EQ(ASCIIToUTF16(""), result_.match_at(1)->header);
EXPECT_EQ(ASCIIToUTF16(kRecentSearchesHeader), result_.match_at(2)->header);
EXPECT_EQ(ASCIIToUTF16(""), result_.match_at(3)->header);
EXPECT_EQ(ASCIIToUTF16(kRecommendedForYouHeader),
result_.match_at(4)->header);
EXPECT_EQ(ASCIIToUTF16(""), result_.match_at(5)->header);
}
{
// All headers are cleared because identical headers cannot be interleaved
// by other headers.
HeaderTestData test_data[] = {{""},
{""},
{kRecommendedForYouHeader},
{kRecentSearchesHeader},
{kRecentSearchesHeader},
{kRecommendedForYouHeader}};
UpdateResultsWithHeaderTestData(test_data, base::size(test_data));
EXPECT_EQ(ASCIIToUTF16(""), result_.match_at(0)->header);
EXPECT_EQ(ASCIIToUTF16(""), result_.match_at(1)->header);
EXPECT_EQ(ASCIIToUTF16(""), result_.match_at(2)->header);
EXPECT_EQ(ASCIIToUTF16(""), result_.match_at(3)->header);
EXPECT_EQ(ASCIIToUTF16(""), result_.match_at(4)->header);
EXPECT_EQ(ASCIIToUTF16(""), result_.match_at(5)->header);
}
{
// All headers are cleared because an empty header cannot appear after
// a non-empty one.
HeaderTestData test_data[] = {{kRecentSearchesHeader},
{kRecentSearchesHeader},
{kRecommendedForYouHeader},
{kRecommendedForYouHeader},
{""},
{""}};
UpdateResultsWithHeaderTestData(test_data, base::size(test_data));
EXPECT_EQ(ASCIIToUTF16(""), result_.match_at(0)->header);
EXPECT_EQ(ASCIIToUTF16(""), result_.match_at(1)->header);
EXPECT_EQ(ASCIIToUTF16(""), result_.match_at(2)->header);
EXPECT_EQ(ASCIIToUTF16(""), result_.match_at(3)->header);
EXPECT_EQ(ASCIIToUTF16(""), result_.match_at(4)->header);
EXPECT_EQ(ASCIIToUTF16(""), result_.match_at(5)->header);
}
}
TEST_F(AutocompleteProviderTest, UpdateAssistedQueryStats) { TEST_F(AutocompleteProviderTest, UpdateAssistedQueryStats) {
ResetControllerWithTestProviders(false, nullptr, nullptr); ResetControllerWithTestProviders(false, nullptr, nullptr);
......
...@@ -287,6 +287,7 @@ AutocompleteMatch BaseSearchProvider::CreateSearchSuggestion( ...@@ -287,6 +287,7 @@ AutocompleteMatch BaseSearchProvider::CreateSearchSuggestion(
match.image_url = suggestion.image_url(); match.image_url = suggestion.image_url();
match.contents = suggestion.match_contents(); match.contents = suggestion.match_contents();
match.contents_class = suggestion.match_contents_class(); match.contents_class = suggestion.match_contents_class();
match.header = suggestion.header();
match.answer = suggestion.answer(); match.answer = suggestion.answer();
match.subtype_identifier = suggestion.subtype_identifier(); match.subtype_identifier = suggestion.subtype_identifier();
if (suggestion.type() == AutocompleteMatchType::SEARCH_SUGGEST_TAIL) { if (suggestion.type() == AutocompleteMatchType::SEARCH_SUGGEST_TAIL) {
......
...@@ -420,6 +420,7 @@ bool SearchSuggestionParser::ParseSuggestResults( ...@@ -420,6 +420,7 @@ bool SearchSuggestionParser::ParseSuggestResults(
const base::ListValue* relevances = nullptr; const base::ListValue* relevances = nullptr;
const base::ListValue* experiment_stats = nullptr; const base::ListValue* experiment_stats = nullptr;
const base::ListValue* suggestion_details = nullptr; const base::ListValue* suggestion_details = nullptr;
const base::DictionaryValue* headers = nullptr;
const base::ListValue* subtype_identifiers = nullptr; const base::ListValue* subtype_identifiers = nullptr;
const base::DictionaryValue* extras = nullptr; const base::DictionaryValue* extras = nullptr;
int prefetch_index = -1; int prefetch_index = -1;
...@@ -450,6 +451,12 @@ bool SearchSuggestionParser::ParseSuggestResults( ...@@ -450,6 +451,12 @@ bool SearchSuggestionParser::ParseSuggestResults(
} }
} }
const base::DictionaryValue* wrapper_dict = nullptr;
if (extras->GetDictionary("google:headertexts", &wrapper_dict) &&
wrapper_dict) {
wrapper_dict->GetDictionary("a", &headers);
}
const base::DictionaryValue* client_data = nullptr; const base::DictionaryValue* client_data = nullptr;
if (extras->GetDictionary("google:clientdata", &client_data) && client_data) if (extras->GetDictionary("google:clientdata", &client_data) && client_data)
client_data->GetInteger("phi", &prefetch_index); client_data->GetInteger("phi", &prefetch_index);
...@@ -545,6 +552,7 @@ bool SearchSuggestionParser::ParseSuggestResults( ...@@ -545,6 +552,7 @@ bool SearchSuggestionParser::ParseSuggestResults(
std::string image_dominant_color; std::string image_dominant_color;
std::string image_url; std::string image_url;
std::string additional_query_params; std::string additional_query_params;
base::string16 header;
if (suggestion_details) { if (suggestion_details) {
suggestion_details->GetDictionary(index, &suggestion_detail); suggestion_details->GetDictionary(index, &suggestion_detail);
...@@ -559,6 +567,16 @@ bool SearchSuggestionParser::ParseSuggestResults( ...@@ -559,6 +567,16 @@ bool SearchSuggestionParser::ParseSuggestResults(
suggestion_detail->GetString("i", &image_url); suggestion_detail->GetString("i", &image_url);
suggestion_detail->GetString("q", &additional_query_params); suggestion_detail->GetString("q", &additional_query_params);
// Header text.
if (headers) {
base::Optional<int> zero_suggest_group =
suggestion_detail->FindIntPath("zl");
if (zero_suggest_group) {
headers->GetString(base::NumberToString(*zero_suggest_group),
&header);
}
}
// Extract the Answer, if provided. // Extract the Answer, if provided.
const base::DictionaryValue* answer_json = nullptr; const base::DictionaryValue* answer_json = nullptr;
base::string16 answer_type; base::string16 answer_type;
...@@ -593,6 +611,7 @@ bool SearchSuggestionParser::ParseSuggestResults( ...@@ -593,6 +611,7 @@ bool SearchSuggestionParser::ParseSuggestResults(
relevances != nullptr, relevances != nullptr,
should_prefetch, should_prefetch,
trimmed_input)); trimmed_input));
results->suggest_results.back().set_header(header);
if (answer_parsed_successfully) if (answer_parsed_successfully)
results->suggest_results.back().SetAnswer(answer); results->suggest_results.back().SetAnswer(answer);
} }
......
...@@ -160,6 +160,9 @@ class SearchSuggestionParser { ...@@ -160,6 +160,9 @@ class SearchSuggestionParser {
return additional_query_params_; return additional_query_params_;
} }
void set_header(const base::string16& header) { header_ = header; }
const base::string16& header() const { return header_; }
void SetAnswer(const SuggestionAnswer& answer); void SetAnswer(const SuggestionAnswer& answer);
const base::Optional<SuggestionAnswer>& answer() const { return answer_; } const base::Optional<SuggestionAnswer>& answer() const { return answer_; }
...@@ -199,6 +202,10 @@ class SearchSuggestionParser { ...@@ -199,6 +202,10 @@ class SearchSuggestionParser {
// Optional additional parameters to be added to the search URL. // Optional additional parameters to be added to the search URL.
std::string additional_query_params_; std::string additional_query_params_;
// An optional header text this suggestion must appear under. Currently only
// zero-prefix suggestions may have a header.
base::string16 header_;
// Optional short answer to the input that produced this suggestion. // Optional short answer to the input that produced this suggestion.
base::Optional<SuggestionAnswer> answer_; base::Optional<SuggestionAnswer> answer_;
......
...@@ -234,3 +234,75 @@ TEST(SearchSuggestionParserTest, NavigationClassification) { ...@@ -234,3 +234,75 @@ TEST(SearchSuggestionParserTest, NavigationClassification) {
{0, AutocompleteMatch::ACMatchClassification::NONE}}; {0, AutocompleteMatch::ACMatchClassification::NONE}};
EXPECT_EQ(kNone, result.match_contents_class()); EXPECT_EQ(kNone, result.match_contents_class());
} }
TEST(SearchSuggestionParserTest, ParseHeaderTexts) {
std::string json_data = R"([
"",
["los angeles", "san diego", "las vegas", "san francisco"],
["history", "", "", ""],
[],
{
"google:clientdata": {
"bpc": false,
"tlw": false
},
"google:headertexts":{
"a":{
"40007":"Not recommended for you",
"40008":"Recommended for you"
}
},
"google:suggestdetail":[
{
},
{
"zl":40007
},
{
"zl":40008
},
{
"zl":40009
}
],
"google:suggestrelevance": [607, 606, 605, 604],
"google:suggesttype": ["PERSONALIZED_QUERY", "QUERY", "QUERY", "QUERY"]
}])";
base::Optional<base::Value> root_val = base::JSONReader::Read(json_data);
ASSERT_TRUE(root_val);
TestSchemeClassifier scheme_classifier;
AutocompleteInput input(base::ASCIIToUTF16(""),
metrics::OmniboxEventProto::NTP_REALBOX,
scheme_classifier);
SearchSuggestionParser::Results results;
ASSERT_TRUE(SearchSuggestionParser::ParseSuggestResults(
*root_val, input, scheme_classifier, /*default_result_relevance=*/400,
/*is_keyword_result=*/false, &results));
{
const auto& suggestion_result = results.suggest_results[0];
ASSERT_EQ(base::ASCIIToUTF16("los angeles"),
suggestion_result.suggestion());
// This suggestion has no header text.
ASSERT_EQ(base::ASCIIToUTF16(""), suggestion_result.header());
}
{
const auto& suggestion_result = results.suggest_results[1];
ASSERT_EQ(base::ASCIIToUTF16("san diego"), suggestion_result.suggestion());
ASSERT_EQ(base::ASCIIToUTF16("Not recommended for you"),
suggestion_result.header());
}
{
const auto& suggestion_result = results.suggest_results[2];
ASSERT_EQ(base::ASCIIToUTF16("las vegas"), suggestion_result.suggestion());
ASSERT_EQ(base::ASCIIToUTF16("Recommended for you"),
suggestion_result.header());
}
{
const auto& suggestion_result = results.suggest_results[3];
ASSERT_EQ(base::ASCIIToUTF16("san francisco"),
suggestion_result.suggestion());
// This suggestion has no header text.
ASSERT_EQ(base::ASCIIToUTF16(""), suggestion_result.header());
}
}
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