[omnibox] Pass provider-populated subtype_identifier into the aqs= param

Currently, we don't pass the provider-populated (generally
server-provided) subtype_identifier field into the aqs= CGI parameter.

We just statically make one in the AutocompleteController
AutocompleteMatchToAssistedQuery static method.

This more or less works, but in our ideal system, we would have these
qualities:

 1. We decentralize this provider-specific logic into the providers
    themselves, rather than having a centralized switch statement that
    handles all these per-provider cases.

 2. If the server passes us some special subtype_identifier, we can
    pass it along in our aqs= parameter without updating the Chrome
    binary.

This CL makes AutocompleteMatchToAssistedQuery start with the
provider-populated integer.

It also adds a test.

Ideally in the future, all of the logic within that static method
could be migrated into the individual provider classes.

Bug: 1064327
Change-Id: I8b56a6623f61520218f2f4001787171c300fb192
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2118221Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#754651}

components/omnibox/browser/autocomplete_controller.cc

@@ -59,35 +59,38 @@ namespace {
 // Converts the given match to a type (and possibly subtype) based on the AQS
 // specification. For more details, see
 // http://goto.google.com/binary-clients-logging.
-void AutocompleteMatchToAssistedQuery(
-    const AutocompleteMatch::Type& match,
-    const AutocompleteProvider* provider,
-    size_t* type,
-    size_t* subtype) {
+void AutocompleteMatchToAssistedQuery(const AutocompleteMatch& match,
+                                      size_t* type,
+                                      size_t* subtype) {
   // This type indicates a native chrome suggestion.
   *type = 69;
   // Default value, indicating no subtype.
   *subtype = base::string16::npos;
 
+  // If set, start with the AutocompletMatch::subtype_identifier field.
+  if (match.subtype_identifier != 0)
+    *subtype = match.subtype_identifier;
+
   // If provider is TYPE_ZERO_SUGGEST or TYPE_ON_DEVICE_HEAD, set the subtype
   // accordingly. Type will be set in the switch statement below where we'll
   // enter one of SEARCH_SUGGEST or NAVSUGGEST.
-  if (provider) {
-    if (provider->type() == AutocompleteProvider::TYPE_ZERO_SUGGEST &&
-        (match == AutocompleteMatchType::SEARCH_SUGGEST ||
-         match == AutocompleteMatchType::NAVSUGGEST)) {
+  if (match.provider) {
+    if (match.provider->type() == AutocompleteProvider::TYPE_ZERO_SUGGEST &&
+        (match.type == AutocompleteMatchType::SEARCH_SUGGEST ||
+         match.type == AutocompleteMatchType::NAVSUGGEST)) {
       // We abuse this subtype and use it to for zero-suggest suggestions that
       // aren't personalized by the server. That is, it indicates either
       // client-side most-likely URL suggestions or server-side suggestions
       // that depend only on the URL as context.
       *subtype = 66;
-    } else if (provider->type() == AutocompleteProvider::TYPE_ON_DEVICE_HEAD) {
+    } else if (match.provider->type() ==
+               AutocompleteProvider::TYPE_ON_DEVICE_HEAD) {
       // This subtype indicates a match from an on-device head provider.
       *subtype = 271;
     }
   }
 
-  switch (match) {
+  switch (match.type) {
     case AutocompleteMatchType::SEARCH_SUGGEST: {
       // Do not set subtype here; subtype may have been set above.
       *type = 0;
@@ -828,8 +831,7 @@ void AutocompleteController::UpdateAssistedQueryStats(
   for (auto match(result->begin()); match != result->end(); ++match) {
     size_t type = base::string16::npos;
     size_t subtype = base::string16::npos;
-    AutocompleteMatchToAssistedQuery(
-        match->type, match->provider, &type, &subtype);
+    AutocompleteMatchToAssistedQuery(*match, &type, &subtype);
     if (last_type != base::string16::npos &&
         (type != last_type || subtype != last_subtype)) {
       AppendAvailableAutocompletion(

components/omnibox/browser/autocomplete_provider_unittest.cc

@@ -250,6 +250,7 @@ class AutocompleteProviderTest : public testing::Test {
   struct AssistedQueryStatsTestData {
     const AutocompleteMatch::Type match_type;
     const std::string expected_aqs;
+    int subtype_identifier = 0;
   };
 
   // Registers a test TemplateURL under the given keyword.
@@ -549,6 +550,7 @@ void AutocompleteProviderTest::RunAssistedQueryStatsTest(
     match.keyword = base::ASCIIToUTF16(kTestTemplateURLKeyword);
     match.search_terms_args.reset(
         new TemplateURLRef::SearchTermsArgs(base::string16()));
+    match.subtype_identifier = aqs_test_data[i].subtype_identifier;
     matches.push_back(match);
   }
   result_.Reset();
@@ -836,6 +838,13 @@ TEST_F(AutocompleteProviderTest, UpdateAssistedQueryStats) {
     RunAssistedQueryStatsTest(test_data, base::size(test_data));
   }
 
+  {
+    AssistedQueryStatsTestData test_data[] = {
+        {AutocompleteMatchType::SEARCH_SUGGEST_ENTITY, "chrome.0.46i131", 131}};
+    SCOPED_TRACE("One match with provider populated subtype_identifier");
+    RunAssistedQueryStatsTest(test_data, base::size(test_data));
+  }
+
   {
     AssistedQueryStatsTestData test_data[] = {
         {AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED,