Add a new entry to omnibox_event.proto to log specific type of contextual suggestions

chrome/browser/autocomplete/search_provider_unittest.cc

Index: chrome/browser/autocomplete/search_provider_unittest.cc
diff --git a/chrome/browser/autocomplete/search_provider_unittest.cc b/chrome/browser/autocomplete/search_provider_unittest.cc
index 275328f4c34bb1ecf762f3ed4977609ddaeb1cfc..02e53bbe8371890e62f4f3a38dd1d7dd7664d4e5 100644
--- a/chrome/browser/autocomplete/search_provider_unittest.cc
+++ b/chrome/browser/autocomplete/search_provider_unittest.cc
@@ -2507,6 +2507,63 @@ TEST_F(SearchProviderTest, FieldTrialTriggeredParsing) {
     EXPECT_EQ(0, providers_info[0].field_trial_triggered_in_session_size());
   }
 }
+// A basic test that verifies the specific type identifier parsing logic.
+TEST_F(SearchProviderTest, SpecificTypeIdentifierParsing) {
+  struct Match {
+    std::string contents;
+    int specific_type_identifier;
+  };
+
+  struct {
+    const std::string input_text;
+    const std::string provider_response_json;
+    const Match expected_matches[2];

[Mark P, 2017/03/25 22:16:48]

This needs a comment.  Until I read the test code in detail, I thought this
represented the order the expects should be returned in.  But you're not testing
that; you're just using this array more like a dictionary.

[gcomanici, 2017/03/29 02:12:28]

Done.

+  } cases[] = {
+      {// Check that the specific type is set to 0 when these values are not

[Mark P, 2017/03/25 22:16:48]

Please follow the formatting of other tests in this file:

// comment
{ blob }
  <- blank line (optional, depends on how dense the code looks)
// comment
{ blob ,
{ may other blob that tests similar thing that the comment applies to }
  <- blank line (optional)

etc.

(no
{ // comment
 continuation of blob

[gcomanici, 2017/03/29 02:12:28]

Done.

+       // proviede in the response.

[Mark P, 2017/03/25 22:16:48]

nit: provided

[gcomanici, 2017/03/29 02:12:28]

Fixed.

+       "a",
+       "[\"a\",[\"ab\",\"http://b.com\"],[],[], "
+       "{\"google:suggesttype\":[\"QUERY\", \"NAVIGATION\"]}]",
+       {{"ab", 0}, {"b.com", 0}}},
+      {// Check that the specific type works for zero-suggest suggestions.
+       "c",
+       "[\"c\",[\"cd\",\"http://d.com\"],[],[], "
+       "{\"google:suggesttype\":[\"QUERY\", "
+       "\"NAVIGATION\"],\"google:specifictypeid\":[1, 3]}]",

[Mark P, 2017/03/25 22:16:48]

please make sure that the dictionary keys (google:specifictypeid:) are always
the beginning of the line so they're easy to find.  For example,
{a: {b,
c}, d: {e, f}}
is hard to read.

[gcomanici, 2017/03/29 02:12:28]

Done.

+       {{"cd", 1}, {"d.com", 3}}},
+      {// Check that the specific type is set to zero when the number of
+       // suggestions is smaller than the number of typeid's provided.

[Mark P, 2017/03/25 22:16:48]

typeid's
->
ids

(to avoid the inappropriate apostrophe, and typeids looks odd)

ditto below

[gcomanici, 2017/03/29 02:12:28]

Done.

+       "foo",
+       "[\"foo\",[\"foo bar\", \"foo baz\"],[],[], "
+       "{\"google:suggesttype\":[\"QUERY\"],\"google:specifictypeid\":[1, 2, "

[Mark P, 2017/03/25 22:16:48]

google:suggesttype should have two entries here, not one.

[gcomanici, 2017/03/29 02:12:28]

Done.

+       "3]}]",
+       {{"foo bar", 0}, {"foo baz", 0}}},
+      {// Check that the specific type is set to zero when the number of
+       // suggestions is largerthan the number of typeid's provided.

[Mark P, 2017/03/25 22:16:48]

larger than

[gcomanici, 2017/03/29 02:12:28]

Fixed.

+       "bar",
+       "[\"bar\",[\"bar foo\", \"bar foz\"],[],[], "
+       "{\"google:suggesttype\":[\"QUERY\"],\"google:specifictypeid\":[1]}]",
+       {{"bar foo", 0}, {"bar foz", 0}}}};

[Mark P, 2017/03/25 22:16:48]

Please add a test for normal flow that also includes google:suggestrelevance in
such as way as to reorder the suggestions from their default order.  (i.e., the
ids should stick with their suggestions)

[gcomanici, 2017/03/29 02:12:28]

Done.

+
+  for (size_t i = 0; i < arraysize(cases); ++i) {
+    QueryForInputAndWaitForFetcherResponses(
+        ASCIIToUTF16(cases[i].input_text), false,
+        cases[i].provider_response_json, std::string());
+
+    // Check for the match and field trial triggered bits.
+    const ACMatches& matches = provider_->matches();
+    ASSERT_FALSE(matches.empty());
+    for (size_t j = 0; j < arraysize(cases[i].expected_matches); ++j) {
+      if (cases[i].expected_matches[j].contents == kNotApplicable)
+        continue;
+      AutocompleteMatch match;
+      EXPECT_TRUE(FindMatchWithContents(
+          ASCIIToUTF16(cases[i].expected_matches[j].contents), &match));
+      EXPECT_EQ(cases[i].expected_matches[j].specific_type_identifier,
+                match.specific_type_identifier);
+    }
+  }
+}
 
 // Verifies inline autocompletion of navigational results.
 TEST_F(SearchProviderTest, NavigationInline) {
@@ -2667,7 +2724,7 @@ TEST_F(SearchProviderTest, NavigationInline) {
     QueryForInput(ASCIIToUTF16(cases[i].input), false, false);
     SearchSuggestionParser::NavigationResult result(
         ChromeAutocompleteSchemeClassifier(&profile_), GURL(cases[i].url),
-        AutocompleteMatchType::NAVSUGGEST, base::string16(), std::string(),
+        AutocompleteMatchType::NAVSUGGEST, 0, base::string16(), std::string(),
         false, 0, false, ASCIIToUTF16(cases[i].input));
     result.set_received_after_last_keystroke(false);
     AutocompleteMatch match(provider_->NavigationToMatch(result));
@@ -2681,7 +2738,7 @@ TEST_F(SearchProviderTest, NavigationInline) {
     QueryForInput(ASCIIToUTF16(cases[i].input), true, false);
     SearchSuggestionParser::NavigationResult result_prevent_inline(
         ChromeAutocompleteSchemeClassifier(&profile_), GURL(cases[i].url),
-        AutocompleteMatchType::NAVSUGGEST, base::string16(), std::string(),
+        AutocompleteMatchType::NAVSUGGEST, 0, base::string16(), std::string(),
         false, 0, false, ASCIIToUTF16(cases[i].input));
     result_prevent_inline.set_received_after_last_keystroke(false);
     AutocompleteMatch match_prevent_inline(
@@ -2701,8 +2758,8 @@ TEST_F(SearchProviderTest, NavigationInlineSchemeSubstring) {
   const base::string16 url(ASCIIToUTF16("http://a.com"));
   SearchSuggestionParser::NavigationResult result(
       ChromeAutocompleteSchemeClassifier(&profile_), GURL(url),
-      AutocompleteMatchType::NAVSUGGEST,
-      base::string16(), std::string(), false, 0, false, input);
+      AutocompleteMatchType::NAVSUGGEST, 0, base::string16(), std::string(),
+      false, 0, false, input);
   result.set_received_after_last_keystroke(false);
 
   // Check the offset and strings when inline autocompletion is allowed.
@@ -2725,9 +2782,9 @@ TEST_F(SearchProviderTest, NavigationInlineSchemeSubstring) {
 TEST_F(SearchProviderTest, NavigationInlineDomainClassify) {
   QueryForInput(ASCIIToUTF16("w"), false, false);
   SearchSuggestionParser::NavigationResult result(
-      ChromeAutocompleteSchemeClassifier(&profile_),
-      GURL("http://www.wow.com"), AutocompleteMatchType::NAVSUGGEST,
-      base::string16(), std::string(), false, 0, false, ASCIIToUTF16("w"));
+      ChromeAutocompleteSchemeClassifier(&profile_), GURL("http://www.wow.com"),
+      AutocompleteMatchType::NAVSUGGEST, 0, base::string16(), std::string(),
+      false, 0, false, ASCIIToUTF16("w"));
   result.set_received_after_last_keystroke(false);
   AutocompleteMatch match(provider_->NavigationToMatch(result));
   EXPECT_EQ(ASCIIToUTF16("ow.com"), match.inline_autocompletion);
@@ -2857,71 +2914,70 @@ TEST_F(SearchProviderTest, PrefetchMetadataParsing) {
     const std::string keyword_provider_response_json;
     const Match matches[5];
   } cases[] = {
-    // Default provider response does not have prefetch details. Ensure that the
-    // suggestions are not marked as prefetch query.
-    { "a",
-      false,
-      "[\"a\",[\"b\", \"c\"],[],[],{\"google:suggestrelevance\":[1, 2]}]",
-      std::string(),
-      { { "a", false, AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, false },
-        { "c", false, AutocompleteMatchType::SEARCH_SUGGEST, false },
-        { "b", false, AutocompleteMatchType::SEARCH_SUGGEST, false },
-        kEmptyMatch,
-        kEmptyMatch
+      // Default provider response does not have prefetch details. Ensure that
+      // the suggestions are not marked as prefetch query.
+      {
+          "a",
+          false,
+          "[\"a\",[\"b\", \"c\"],[],[],{\"google:suggestrelevance\":[1, 2]}]",
+          std::string(),
+          {{"a", false, AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, false},
+           {"c", false, AutocompleteMatchType::SEARCH_SUGGEST, false},
+           {"b", false, AutocompleteMatchType::SEARCH_SUGGEST, false},
+           kEmptyMatch,
+           kEmptyMatch},
       },
-    },
-    // Ensure that default provider suggest response prefetch details are
-    // parsed and recorded in AutocompleteMatch.
-    { "ab",
-      false,
-      "[\"ab\",[\"abc\", \"http://b.com\", \"http://c.com\"],[],[],"
+      // Ensure that default provider suggest response prefetch details are
+      // parsed and recorded in AutocompleteMatch.
+      {
+          "ab",
+          false,
+          "[\"ab\",[\"abc\", \"http://b.com\", \"http://c.com\"],[],[],"
           "{\"google:clientdata\":{\"phi\": 0},"
           "\"google:suggesttype\":[\"QUERY\", \"NAVIGATION\", \"NAVIGATION\"],"
           "\"google:suggestrelevance\":[999, 12, 1]}]",
-      std::string(),
-      { { "ab",    false, AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, false },
-        { "abc",   true,  AutocompleteMatchType::SEARCH_SUGGEST, false },
-        { "b.com", false, AutocompleteMatchType::NAVSUGGEST, false },
-        { "c.com", false, AutocompleteMatchType::NAVSUGGEST, false },
-        kEmptyMatch
+          std::string(),
+          {{"ab", false, AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, false},
+           {"abc", true, AutocompleteMatchType::SEARCH_SUGGEST, false},
+           {"b.com", false, AutocompleteMatchType::NAVSUGGEST, false},
+           {"c.com", false, AutocompleteMatchType::NAVSUGGEST, false},
+           kEmptyMatch},
       },
-    },
-    // Default provider suggest response has prefetch details.
-    // SEARCH_WHAT_YOU_TYPE suggestion outranks SEARCH_SUGGEST suggestion for
-    // the same query string. Ensure that the prefetch details from
-    // SEARCH_SUGGEST match are set onto SEARCH_WHAT_YOU_TYPE match.
-    { "ab",
-      false,
-      "[\"ab\",[\"ab\", \"http://ab.com\"],[],[],"
+      // Default provider suggest response has prefetch details.
+      // SEARCH_WHAT_YOU_TYPE suggestion outranks SEARCH_SUGGEST suggestion for
+      // the same query string. Ensure that the prefetch details from
+      // SEARCH_SUGGEST match are set onto SEARCH_WHAT_YOU_TYPE match.
+      {
+          "ab",
+          false,
+          "[\"ab\",[\"ab\", \"http://ab.com\"],[],[],"
           "{\"google:clientdata\":{\"phi\": 0},"
           "\"google:suggesttype\":[\"QUERY\", \"NAVIGATION\"],"
           "\"google:suggestrelevance\":[99, 98]}]",
-      std::string(),
-      { {"ab", true, AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, false },
-        {"ab.com", false, AutocompleteMatchType::NAVSUGGEST, false },
-        kEmptyMatch,
-        kEmptyMatch,
-        kEmptyMatch
+          std::string(),
+          {{"ab", true, AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, false},
+           {"ab.com", false, AutocompleteMatchType::NAVSUGGEST, false},
+           kEmptyMatch,
+           kEmptyMatch,
+           kEmptyMatch},
       },
-    },
-    // Default provider response has prefetch details. We prefer keyword
-    // provider results. Ensure that prefetch bit for a suggestion from the
-    // default search provider does not get copied onto a higher-scoring match
-    // for the same query string from the keyword provider.
-    { "k a",
-      true,
-      "[\"k a\",[\"a\", \"ab\"],[],[], {\"google:clientdata\":{\"phi\": 0},"
+      // Default provider response has prefetch details. We prefer keyword
+      // provider results. Ensure that prefetch bit for a suggestion from the
+      // default search provider does not get copied onto a higher-scoring match
+      // for the same query string from the keyword provider.
+      {
+          "k a",
+          true,
+          "[\"k a\",[\"a\", \"ab\"],[],[], {\"google:clientdata\":{\"phi\": 0},"
           "\"google:suggesttype\":[\"QUERY\", \"QUERY\"],"
           "\"google:suggestrelevance\":[9, 12]}]",
-      "[\"a\",[\"b\", \"c\"],[],[],{\"google:suggestrelevance\":[1, 2]}]",
-      { { "a", false, AutocompleteMatchType::SEARCH_OTHER_ENGINE, true},
-        { "k a", false, AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, false },
-        { "ab", false, AutocompleteMatchType::SEARCH_SUGGEST, false },
-        { "c", false, AutocompleteMatchType::SEARCH_SUGGEST, true },
-        { "b", false, AutocompleteMatchType::SEARCH_SUGGEST, true }
-      },
-    }
-  };
+          "[\"a\",[\"b\", \"c\"],[],[],{\"google:suggestrelevance\":[1, 2]}]",
+          {{"a", false, AutocompleteMatchType::SEARCH_OTHER_ENGINE, true},
+           {"k a", false, AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, false},
+           {"ab", false, AutocompleteMatchType::SEARCH_SUGGEST, false},
+           {"c", false, AutocompleteMatchType::SEARCH_SUGGEST, true},
+           {"b", false, AutocompleteMatchType::SEARCH_SUGGEST, true}},
+      }};
 
   for (size_t i = 0; i < arraysize(cases); ++i) {
     QueryForInputAndWaitForFetcherResponses(
@@ -3474,7 +3530,7 @@ TEST_F(SearchProviderTest, AnswersCache) {
   // Inject a scored result, which will trigger answer retrieval.
   base::string16 query = base::ASCIIToUTF16("weather los angeles");
   SearchSuggestionParser::SuggestResult suggest_result(
-      query, AutocompleteMatchType::SEARCH_HISTORY, query, base::string16(),
+      query, AutocompleteMatchType::SEARCH_HISTORY, 0, query, base::string16(),
       base::string16(), base::string16(), base::string16(), nullptr,
       std::string(), std::string(), false, 1200, false, false, query);
   QueryForInput(ASCIIToUTF16("weather l"), false, false);