A working implementation of AQS (Assisted Query Stats).

chrome/browser/autocomplete/autocomplete.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/autocomplete/autocomplete.h"
 
 #include <algorithm>
 #include <iterator>
 #include <set>
 
 #include "base/basictypes.h"
 #include "base/command_line.h"
 #include "base/i18n/number_formatting.h"
 #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"
 #include "chrome/browser/autocomplete/builtin_provider.h"
 #include "chrome/browser/autocomplete/extension_app_provider.h"
 #include "chrome/browser/autocomplete/history_contents_provider.h"
 #include "chrome/browser/autocomplete/history_quick_provider.h"
 #include "chrome/browser/autocomplete/history_url_provider.h"
 #include "chrome/browser/autocomplete/keyword_provider.h"
 #include "chrome/browser/autocomplete/search_provider.h"
@@ skipping 19 matching lines @@
 #include "googleurl/src/url_util.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
 #include "net/base/net_util.h"
 #include "net/base/registry_controlled_domain.h"
 #include "net/url_request/url_request.h"
 #include "ui/base/l10n/l10n_util.h"
 
 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()
   : type_(INVALID),
     prevent_inline_autocomplete_(false),
     prefer_keyword_(false),
     allow_exact_keyword_match_(true),
     matches_requested_(ALL_MATCHES) {
 }
 
@@ skipping 971 matching lines @@
 #endif
 
   if (!done_) {
     // This conditional needs to match the conditional in Start that invokes
     // StartExpireTimer.
     result_.CopyOldMatches(input_, last_result);
   }
 
   UpdateKeywordDescriptions(&result_);
   UpdateAssociatedKeywords(&result_);
+  UpdateAssistedQueryStats(&result_);
 
   bool notify_default_match = is_synchronous_pass;
   if (!is_synchronous_pass) {
     const bool last_default_was_valid =
         last_result.default_match() != last_result.end();
     const bool default_is_valid = result_.default_match() != result_.end();
     // We've gotten async results. Send notification that the default match
     // updated if fill_into_edit differs or associated_keyword differ.  (The
     // latter can change if we've just started Chrome and the keyword database
     // finishes loading while processing this request.) We don't check the URL
@@ skipping 36 matching lines @@
           match->associated_keyword.reset(new AutocompleteMatch(
               keyword_provider_->CreateAutocompleteMatch(match->fill_into_edit,
                   keyword, input_)));
       } else {
         match->associated_keyword.reset();
       }
     }
   }
 }
 
+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;
   for (AutocompleteResult::iterator i = result->begin(); i != result->end();
        ++i) {
     if (((i->provider == keyword_provider_) && !i->keyword.empty()) ||
         ((i->provider == search_provider_) &&
          (i->type == AutocompleteMatch::SEARCH_WHAT_YOU_TYPED ||
           i->type == AutocompleteMatch::SEARCH_HISTORY ||
           i->type == AutocompleteMatch::SEARCH_SUGGEST))) {
@@ skipping 66 matching lines @@
       current_page_classification(current_page_classification),
       elapsed_time_since_user_first_modified_omnibox(
           elapsed_time_since_user_first_modified_omnibox),
       inline_autocompleted_length(inline_autocompleted_length),
       result(result),
       providers_info() {
 }
 
 AutocompleteLog::~AutocompleteLog() {
 }