Index: chrome/browser/autocomplete/autocomplete.cc |
diff --git a/chrome/browser/autocomplete/autocomplete.cc b/chrome/browser/autocomplete/autocomplete.cc |
index 4f87500830bd40e23c106072b9e2452f9e475de3..7d57a3c7a2edd552dbb889257ad2d7d696372798 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" |
@@ -1044,6 +1045,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 +1102,86 @@ void AutocompleteController::UpdateAssociatedKeywords( |
} |
} |
+namespace { |
msw
2012/06/13 22:39:33
anon namespaces and contents belong at the top of
Bart N
2012/06/13 23:17:13
Done.
|
+ |
+// Converts the given type to an integer based on the AQS specification. |
msw
2012/06/13 22:39:33
Are these magic numbers really necessary? Why not
Bart N
2012/06/13 23:17:13
These values must map to existing enum on the serv
msw
2012/06/13 23:42:08
I wouldn't bother, but maybe add a vague statement
|
+int AutocompletMatchToAssistedQueryType(const AutocompleteMatch::Type& type) { |
msw
2012/06/13 22:39:33
Spelling: Autocomplet*e*MatchToAssistedQueryType
Bart N
2012/06/13 23:17:13
Done.
|
+ 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 query stats of the given type and number to the existing AQS string. |
+void AppendQueryStats(int type, int count, std::string* aqs) { |
+ if (!aqs->empty()) |
+ aqs->append("j"); |
+ base::StringAppendF(aqs, "%d", type); |
+ if (count > 1) |
+ base::StringAppendF(aqs, "l%d", count); |
+} |
+ |
+} // namespace |
+ |
+void AutocompleteController::UpdateAssistedQueryStats( |
+ AutocompleteResult* result) { |
+ if (result->empty()) |
+ return; |
+ |
+ // Build the impressions string (the AQS part after "."). |
+ std::string aqs; |
msw
2012/06/13 22:39:33
Use string16 if possible.
Bart N
2012/06/13 23:17:13
Hmmm.. a few methods broke when I switched to stri
msw
2012/06/13 23:42:08
Use + and += operators, IIRC. See base/string16.h
|
+ int count = 0; |
+ int last_type = -1; |
+ for (ACMatches::iterator match(result->begin()); match != result->end(); |
msw
2012/06/13 22:39:33
Is there a reason for combining consecutive matche
Bart N
2012/06/13 23:17:13
Yes, it's required by the spec. For instance, inst
msw
2012/06/13 23:42:08
Fair enough.
|
+ ++match) { |
+ int type = AutocompletMatchToAssistedQueryType(match->type); |
+ if (last_type != -1 && type != last_type) { |
+ AppendQueryStats(last_type, count, &aqs); |
+ count = 1; |
+ } else { |
+ count++; |
+ } |
+ last_type = type; |
+ } |
+ AppendQueryStats(last_type, count, &aqs); |
msw
2012/06/13 22:39:33
Should this be done even if there were no matches?
Bart N
2012/06/13 23:17:13
There is always at least one. See the check at the
msw
2012/06/13 23:42:08
Doh! Thanks for the heads up on the empty check.
|
+ |
+ // Go over all matches and set AQS if the match supports it. |
+ int index = 0; |
+ for (AutocompleteResult::iterator i = result->begin(); i != result->end(); |
msw
2012/06/13 22:39:33
nit: consistency is nice, consider matching your l
Bart N
2012/06/13 23:17:13
Ah yes, done.
|
+ ++i, ++index) { |
+ if (i->provider != search_provider_) |
msw
2012/06/13 22:39:33
It'd be nice if we could put this logic in SearchP
Bart N
2012/06/13 23:17:13
Currently, I rely on the fact that we check for Su
msw
2012/06/13 23:42:08
So, I'm not terribly familiar with AQS or what you
|
+ continue; |
+ const TemplateURL* template_url = i->GetTemplateURL(profile_); |
+ if (!template_url || !template_url->url_ref().SupportsAssistedQueryStats()) |
+ continue; |
+ std::string url(i->destination_url.spec()); |
+ if (url.find("&aqs=") != std::string::npos) |
+ continue; |
+ url.append(base::StringPrintf("&aqs=%d", index)).append(".").append(aqs); |
msw
2012/06/13 22:39:33
Whoa, we're sending the ordered list of match type
Bart N
2012/06/13 23:17:13
Yes. It was approved today, I can forward an email
msw
2012/06/13 23:42:08
You should link this CL to a chromium bug (set BUG
|
+ i->destination_url = GURL(url); |
+ |
+ VLOG(1) << "[" << i->provider->name() << "] URL = " << i->destination_url; |
msw
2012/06/13 22:39:33
Will this LOG help end users? if not, please remov
Bart N
2012/06/13 23:17:13
Removed.
|
+ } |
+} |
+ |
void AutocompleteController::UpdateKeywordDescriptions( |
AutocompleteResult* result) { |
string16 last_keyword; |