A working implementation of AQS (Assisted Query Stats).

chrome/browser/autocomplete/autocomplete.cc

Index: chrome/browser/autocomplete/autocomplete.cc
diff --git a/chrome/browser/autocomplete/autocomplete.cc b/chrome/browser/autocomplete/autocomplete.cc
index 4f87500830bd40e23c106072b9e2452f9e475de3..821ce5c7a5a13c267aef5c1bf972f3ca895b58f6 100644
--- a/chrome/browser/autocomplete/autocomplete.cc
+++ b/chrome/browser/autocomplete/autocomplete.cc
@@ -14,6 +14,7 @@
 #include "base/metrics/histogram.h"
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
+#include "base/stringprintf.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/autocomplete/autocomplete_controller_delegate.h"
 #include "chrome/browser/autocomplete/autocomplete_match.h"
@@ -53,6 +54,48 @@
 
 using base::TimeDelta;
 
+namespace {

[Peter Kasting, 2012/06/16 03:14:17]

Nit: Because these functions are only used inside AutocompleteController, I
suggest putting this whole anonymous namespace just below the "//
AutocompleteController --" divider comment.

[Bart N, 2012/06/16 23:38:10]

Done.

+
+// Converts the given type to an integer based on the AQS specification.
+// For more details, See http://go/binary-clients-logging.

[Peter Kasting, 2012/06/16 03:14:17]

Nit: See->see; go->goto.google.com; I suggest a space before the trailing period
just to be clear.

Is it possible to eventually (maybe not before this lands) produce a sanitized
version of the doc that can be made public, and then link that here instead?

[Bart N, 2012/06/16 23:38:10]

Yes, we can have a sanitized version later on.
Done.
On 2012/06/16 03:14:17, Peter Kasting wrote:

+int AutocompleteMatchToAssistedQueryType(const AutocompleteMatch::Type& type) {
+  switch (type) {
+    case AutocompleteMatch::SEARCH_SUGGEST:        return 0;
+    case AutocompleteMatch::NAVSUGGEST:            return 5;
+    case AutocompleteMatch::SEARCH_WHAT_YOU_TYPED: return 57;
+    case AutocompleteMatch::URL_WHAT_YOU_TYPED:    return 58;
+    case AutocompleteMatch::SEARCH_HISTORY:        return 59;
+    case AutocompleteMatch::HISTORY_URL:           return 60;
+    case AutocompleteMatch::HISTORY_TITLE:         return 61;
+    case AutocompleteMatch::HISTORY_BODY:          return 62;
+    case AutocompleteMatch::HISTORY_KEYWORD:       return 63;
+    default:                                       return 64;
+  }
+}
+
+// Appends available autocompletion of the given type and number to the existing
+// available autocompletions string, encoding according to the spec.
+void AppendAvailableAutocompletion(int type, int count,

[Peter Kasting, 2012/06/16 03:14:17]

Nit: One arg per line (2 places)

[Bart N, 2012/06/16 23:38:10]

Done.

+                                   std::string* autocompletions) {
+  if (!autocompletions->empty())
+    autocompletions->append("j");
+  base::StringAppendF(autocompletions, "%d", type);
+  if (count > 1)
+    base::StringAppendF(autocompletions, "l%d", count);
+}
+
+// Constructs the final form of AQS param.
+string16 ConstructAssistedQueryStatsString(
+    const std::string& client, int query_index,
+    const std::string& available_autocompletions) {
+  std::string aqs(client);
+  aqs.append(".").append(base::StringPrintf("%d", query_index)).append(".")

[Peter Kasting, 2012/06/16 03:14:17]

Nit: Why not just something like

  return base::StringPrintf("%s.%d.%s", client, query_index,
available_autocompletions);

(Note, I don't remember exactly how to do string args, like if there's a format
type that means "std::string" or you have to pass .c_str() or whatever.)

At this point, you could then just inline this into the caller side.

[Bart N, 2012/06/16 23:38:10]

Done.

+      .append(available_autocompletions);
+  return ASCIIToUTF16(aqs);
+}
+
+}  // namespace
+
 // AutocompleteInput ----------------------------------------------------------
 
 AutocompleteInput::AutocompleteInput()
@@ -1044,6 +1087,7 @@ void AutocompleteController::UpdateResult(bool is_synchronous_pass) {
 
   UpdateKeywordDescriptions(&result_);
   UpdateAssociatedKeywords(&result_);
+  UpdateAssistedQueryStats(&result_);
 
   bool notify_default_match = is_synchronous_pass;
   if (!is_synchronous_pass) {
@@ -1100,6 +1144,46 @@ void AutocompleteController::UpdateAssociatedKeywords(
   }
 }
 
+void AutocompleteController::UpdateAssistedQueryStats(
+    AutocompleteResult* result) {
+  if (result->empty())
+    return;
+
+  // Build the impressions string (the AQS part after ".").
+  std::string autocompletions;
+  int count = 0;
+  int last_type = -1;
+  for (ACMatches::iterator match(result->begin()); match != result->end();
+       ++match) {
+    int type = AutocompleteMatchToAssistedQueryType(match->type);
+    if (last_type != -1 && type != last_type) {
+      AppendAvailableAutocompletion(last_type, count, &autocompletions);
+      count = 1;
+    } else {
+      count++;
+    }
+    last_type = type;
+  }
+  // It's OK to call it, because there is always at least one match.

[Peter Kasting, 2012/06/16 03:14:17]

Nit: I'd remove this comment (it seems clear enough from the top-of-function
check)

[Bart N, 2012/06/16 23:38:10]

It wasn't clear to another reviewer, so I added it. Removed.

+  AppendAvailableAutocompletion(last_type, count, &autocompletions);
+
+  // Go over all matches and set AQS if the match supports it.
+  int index = 0;
+  for (ACMatches::iterator match(result->begin()); match != result->end();

[Peter Kasting, 2012/06/16 03:14:17]

Nit: Since you need to use an index anyway, just omit the iterator and use
result->match_at(index).  (Note this will mean you should switch |index| and the
corresponding arg of ConstructAssistedQueryStatsString() to use size_t.)

[Bart N, 2012/06/16 23:38:10]

Done. Except size_t which is a pain to get proper % in printf. I also added
another access to Result, since match_at returns const&

+       ++match, ++index) {
+    if (match->provider != search_provider_)

[Peter Kasting, 2012/06/16 03:14:17]

Nit: Omit this check; there's no theoretical reason why only the search provider
can produce these.  Checking for |search_terms_args| is good enough.

[Bart N, 2012/06/16 23:38:10]

Done.

+      continue;
+    const TemplateURL* template_url = match->GetTemplateURL(profile_);
+    if (!template_url || !match->search_terms_args.get())
+      continue;
+    SearchTermsArgs& search_terms_args = *match->search_terms_args.get();

[Peter Kasting, 2012/06/16 03:14:17]

Nit: We generally avoid non-const refs; I suggest either accessing
match->search_terms_args directly or using a pointer as the temp.  (I'd do the
former.)

[Bart N, 2012/06/16 23:38:10]

Done.

+    search_terms_args.assisted_query_stats =
+        ConstructAssistedQueryStatsString("chrome", index, autocompletions);
+    match->destination_url = GURL(template_url->url_ref().ReplaceSearchTerms(
+        search_terms_args));
+  }
+}
+
 void AutocompleteController::UpdateKeywordDescriptions(
     AutocompleteResult* result) {
   string16 last_keyword;