Omnibox SearchProvider Experiment Client Implementation.

chrome/browser/autocomplete/search_provider.cc

Index: chrome/browser/autocomplete/search_provider.cc
diff --git a/chrome/browser/autocomplete/search_provider.cc b/chrome/browser/autocomplete/search_provider.cc
index a8e45ee4e45f1923676a5e814c7c8b9ca07e00df..a88a471ffe4d66325d0cb21b5f898b9ce919939c 100644
--- a/chrome/browser/autocomplete/search_provider.cc
+++ b/chrome/browser/autocomplete/search_provider.cc
@@ -90,6 +90,8 @@ bool SearchProvider::query_suggest_immediately_ = false;
 SearchProvider::SearchProvider(ACProviderListener* listener, Profile* profile)
     : AutocompleteProvider(listener, profile, "Search"),
       providers_(profile),
+      has_verbatim_relevance_(false),
+      verbatim_relevance_(0),
       suggest_results_pending_(0),
       have_suggest_results_(false),
       instant_finalized_(false) {
@@ -147,6 +149,7 @@ void SearchProvider::FinalizeInstantQuery(const string16& input_text,
     }
   }
 
+  // TODO(msw): Ensure the experiments don't interfere with instant...
   // Add the new suggest result. We give it a rank higher than
   // SEARCH_WHAT_YOU_TYPED so that it gets autocompleted.
   int did_not_accept_default_suggestion = default_suggest_results_.empty() ?
@@ -234,6 +237,25 @@ void SearchProvider::Start(const AutocompleteInput& input,
   ConvertResultsToAutocompleteMatches();
 }
 
+SearchProvider::SuggestResult::SuggestResult(const string16& suggestion,
+                                             bool has_suggested_relevance,
+                                             int relevance)
+    : suggestion(suggestion),
+      has_suggested_relevance(has_suggested_relevance),
+      relevance(relevance) {
+}
+
+SearchProvider::NavigationResult::NavigationResult(
+    const GURL& url,
+    const string16& site_name,
+    bool has_suggested_relevance,
+    int relevance)
+    : url(url),
+      site_name(site_name),
+      has_suggested_relevance(has_suggested_relevance),
+      relevance(relevance) {
+}
+
 class SearchProvider::CompareScoredTerms {
  public:
   bool operator()(const ScoredTerm& a, const ScoredTerm& b) {
@@ -356,8 +378,11 @@ void SearchProvider::StartOrStopSuggestQuery(bool minimal_changes) {
   // likely to end up throwing away anyway.
   const int kQueryDelayMs = 200;
 
+  // TODO(msw): Synchronously remove irrelevant inline-autocomplete *results*...
+
   if (!IsQuerySuitableForSuggest()) {
     StopSuggest();
+    ClearResults();
     return;
   }
 
@@ -449,6 +474,9 @@ void SearchProvider::StopSuggest() {
   // Stop any in-progress URL fetches.
   keyword_fetcher_.reset();
   default_fetcher_.reset();
+}
+
+void SearchProvider::ClearResults() {
   keyword_suggest_results_.clear();
   default_suggest_results_.clear();
   keyword_navigation_results_.clear();
@@ -479,51 +507,47 @@ bool SearchProvider::ParseSuggestResults(Value* root_val,
     return false;
   ListValue* root_list = static_cast<ListValue*>(root_val);
 
-  Value* query_val;
   string16 query_str;
-  Value* result_val;
-  if ((root_list->GetSize() < 2) || !root_list->Get(0, &query_val) ||
-      !query_val->GetAsString(&query_str) ||
-      (query_str != input_text) ||
-      !root_list->Get(1, &result_val) || !result_val->IsType(Value::TYPE_LIST))
+  ListValue* result_list = NULL;
+  if ((root_list->GetSize() < 2) || !root_list->GetString(0, &query_str) ||
+      (query_str != input_text) || !root_list->GetList(1, &result_list))
     return false;
 
+  // 3rd element: Description list.
   ListValue* description_list = NULL;
-  if (root_list->GetSize() > 2) {
-    // 3rd element: Description list.
-    Value* description_val;
-    if (root_list->Get(2, &description_val) &&
-        description_val->IsType(Value::TYPE_LIST))
-      description_list = static_cast<ListValue*>(description_val);
-  }
+  if (root_list->GetSize() > 2)
+    root_list->GetList(2, &description_list);
 
-  // We don't care about the query URL list (the fourth element in the
-  // response) for now.
+  // 4th element: Disregard the query URL list for now.
 
-  // Parse optional data in the results from the Suggest server if any.
+  // 5th element: Optional key-value pairs from the Suggest server.
+  DictionaryValue* dict_val = NULL;
   ListValue* type_list = NULL;
-  // 5th argument: Optional key-value pairs.
-  // TODO: We may iterate the 5th+ arguments of the root_list if any other
-  // optional data are defined.
-  if (root_list->GetSize() > 4) {
-    Value* optional_val;
-    if (root_list->Get(4, &optional_val) &&
-        optional_val->IsType(Value::TYPE_DICTIONARY)) {
-      DictionaryValue* dict_val = static_cast<DictionaryValue*>(optional_val);
-
-      // Parse Google Suggest specific type extension.
-      const std::string kGoogleSuggestType("google:suggesttype");
-      if (dict_val->HasKey(kGoogleSuggestType))
-        dict_val->GetList(kGoogleSuggestType, &type_list);
-    }
+  ListValue* relevance_list = NULL;
+  has_verbatim_relevance_ = false;
+  verbatim_relevance_ = 0;
+  if (root_list->GetSize() > 4 && root_list->GetDictionary(4, &dict_val)) {
+    // Parse Google Suggest specific type extension.
+    const std::string kGoogleSuggestType("google:suggesttype");
+    dict_val->GetList(kGoogleSuggestType, &type_list);
+
+    // Parse Google Suggest specific relevance extension.
+    const std::string kGoogleSuggestRelevance("google:suggestrelevance");
+    dict_val->GetList(kGoogleSuggestRelevance, &relevance_list);
+
+    // Parse Google Suggest specific verbatim relevance extension.
+    const std::string kGoogleVerbatimRelevance("google:verbatimrelevance");
+    if (dict_val->GetInteger(kGoogleVerbatimRelevance, &verbatim_relevance_))
+      has_verbatim_relevance_ = true;
   }
 
-  ListValue* result_list = static_cast<ListValue*>(result_val);
+  // Clear the previous results if new results are available.
+  if (result_list->GetSize() > 0)
+    ClearResults();
+
   for (size_t i = 0; i < result_list->GetSize(); ++i) {
-    Value* suggestion_val;
     string16 suggestion_str;
-    if (!result_list->Get(i, &suggestion_val) ||
-        !suggestion_val->GetAsString(&suggestion_str))
+    if (!result_list->GetString(i, &suggestion_str))
       return false;
 
     // Google search may return empty suggestions for weird input characters,
@@ -532,31 +556,36 @@ bool SearchProvider::ParseSuggestResults(Value* root_val,
     if (!suggestion_str.length())
       continue;
 
-    Value* type_val;
-    std::string type_str;
-    if (type_list && type_list->Get(i, &type_val) &&
-        type_val->GetAsString(&type_str) && (type_str == "NAVIGATION")) {
-      Value* site_val;
+    bool has_suggested_relevance = false;
+    int relevance = 0;
+    if (relevance_list && relevance_list->GetInteger(i, &relevance))
+      has_suggested_relevance = true;
+
+    std::string type;
+    if (type_list && type_list->GetString(i, &type) && (type == "NAVIGATION")) {
       string16 site_name;
       NavigationResults& navigation_results =
           is_keyword ? keyword_navigation_results_ :
                        default_navigation_results_;
       if ((navigation_results.size() < kMaxMatches) &&
-          description_list && description_list->Get(i, &site_val) &&
-          site_val->IsType(Value::TYPE_STRING) &&
-          site_val->GetAsString(&site_name)) {
+          description_list && description_list->GetString(i, &site_name)) {
         // We can't blindly trust the URL coming from the server to be valid.
         GURL result_url(URLFixerUpper::FixupURL(UTF16ToUTF8(suggestion_str),
                                                 std::string()));
         if (result_url.is_valid()) {
-          navigation_results.push_back(NavigationResult(result_url, site_name));
+          navigation_results.push_back(NavigationResult(result_url, site_name,
+                                                        has_suggested_relevance,
+                                                        relevance));
         }
       }
     } else {
       // TODO(kochi): Currently we treat a calculator result as a query, but it
       // is better to have better presentation for caluculator results.
-      if (suggest_results->size() < kMaxMatches)
-        suggest_results->push_back(suggestion_str);
+      if (suggest_results->size() < kMaxMatches) {
+        suggest_results->push_back(SuggestResult(suggestion_str,
+                                                 has_suggested_relevance,
+                                                 relevance));
+      }
     }
   }
 
@@ -573,18 +602,21 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() {
       TemplateURLRef::NO_SUGGESTION_CHOSEN;
   // Keyword what you typed results are handled by the KeywordProvider.
 
+  // TODO(msw): Use normal scoring, disable experiments for instant?
+  int verbatim_relevance = CalculateRelevanceForWhatYouTyped();
+  bool add_verbatim_result = verbatim_relevance > 0;
+
   int did_not_accept_default_suggestion = default_suggest_results_.empty() ?
-        TemplateURLRef::NO_SUGGESTIONS_AVAILABLE :
-        TemplateURLRef::NO_SUGGESTION_CHOSEN;
-  if (providers_.valid_default_provider()) {
-    AddMatchToMap(input_.text(), input_.text(),
-                  CalculateRelevanceForWhatYouTyped(),
+      TemplateURLRef::NO_SUGGESTIONS_AVAILABLE :
+      TemplateURLRef::NO_SUGGESTION_CHOSEN;
+  if (providers_.valid_default_provider() && add_verbatim_result) {
+    AddMatchToMap(input_.text(), input_.text(), verbatim_relevance,
                   AutocompleteMatch::SEARCH_WHAT_YOU_TYPED,
                   did_not_accept_default_suggestion, false,
                   input_.prevent_inline_autocomplete(), &map);
     if (!default_provider_suggest_text_.empty()) {
       AddMatchToMap(input_.text() + default_provider_suggest_text_,
-                    input_.text(), CalculateRelevanceForWhatYouTyped() + 1,
+                    input_.text(), verbatim_relevance + 1,
                     AutocompleteMatch::SEARCH_SUGGEST,
                     did_not_accept_default_suggestion, false,
                     input_.prevent_inline_autocomplete(), &map);
@@ -609,7 +641,8 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() {
   AddNavigationResultsToMatches(keyword_navigation_results_, true);
   AddNavigationResultsToMatches(default_navigation_results_, false);
 
-  const size_t max_total_matches = kMaxMatches + 1;  // 1 for "what you typed"
+  // Allow an additional match for "what you typed", if it's used.
+  const size_t max_total_matches = kMaxMatches + (add_verbatim_result ? 0 : 1);

[Mark P, 2012/05/04 20:31:38]

Are the 1s and 0s backward here?

[msw, 2012/05/04 22:13:39]

Done.

   std::partial_sort(matches_.begin(),
       matches_.begin() + std::min(max_total_matches, matches_.size()),
       matches_.end(), &AutocompleteMatch::MoreRelevant);
@@ -617,7 +650,6 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() {
     matches_.erase(matches_.begin() + max_total_matches, matches_.end());
 
   UpdateStarredStateOfMatches();
-
   UpdateDone();
 }
 
@@ -628,10 +660,8 @@ void SearchProvider::AddNavigationResultsToMatches(
     // TODO(kochi): http://b/1170574  We add only one results for navigational
     // suggestions. If we can get more useful information about the score,
     // consider adding more results.
-    const size_t num_results = is_keyword ?
-        keyword_navigation_results_.size() : default_navigation_results_.size();
     matches_.push_back(NavigationToMatch(navigation_results.front(),
-        CalculateRelevanceForNavigation(num_results, 0, is_keyword),
+        CalculateRelevanceForNavigation(navigation_results, 0, is_keyword),
         is_keyword));
   }
 }
@@ -741,9 +771,9 @@ void SearchProvider::AddSuggestResultsToMap(
     int did_not_accept_suggestion,
     MatchMap* map) {
   for (size_t i = 0; i < suggest_results.size(); ++i) {
-    AddMatchToMap(suggest_results[i],
+    AddMatchToMap(suggest_results[i].suggestion,
                   is_keyword ? keyword_input_text_ : input_.text(),
-                  CalculateRelevanceForSuggestion(suggest_results.size(), i,
+                  CalculateRelevanceForSuggestion(suggest_results, i,
                                                   is_keyword),
                   AutocompleteMatch::SEARCH_SUGGEST,
                   static_cast<int>(i), is_keyword,
@@ -752,6 +782,10 @@ void SearchProvider::AddSuggestResultsToMap(
 }
 
 int SearchProvider::CalculateRelevanceForWhatYouTyped() const {
+  // TODO(msw): Effect SEARCH_WHAT_YOU_TYPED match (line 624) and instant??
+  if (has_verbatim_relevance_ && !input_.prevent_inline_autocomplete())
+    return verbatim_relevance_;
+
   if (providers_.valid_keyword_provider())
     return 250;
 
@@ -808,27 +842,33 @@ int SearchProvider::CalculateRelevanceForHistory(
   return std::max(0, base_score - score_discount);
 }
 
-int SearchProvider::CalculateRelevanceForSuggestion(size_t num_results,
-                                                    size_t result_number,
-                                                    bool is_keyword) const {
-  DCHECK(result_number < num_results);
+int SearchProvider::CalculateRelevanceForSuggestion(
+    const SuggestResults& results,
+    size_t result_number,
+    bool is_keyword) const {
+  DCHECK(result_number < results.size());
+  if (results[result_number].has_suggested_relevance)
+    return results[result_number].relevance;
+
   int base_score;
   if (!providers_.is_primary_provider(is_keyword))
     base_score = 100;
   else
     base_score = (input_.type() == AutocompleteInput::URL) ? 300 : 600;
   return base_score +
-      static_cast<int>(num_results - 1 - result_number);
+      static_cast<int>(results.size() - 1 - result_number);
 }
 
-int SearchProvider::CalculateRelevanceForNavigation(size_t num_results,
-                                                    size_t result_number,
-                                                    bool is_keyword) const {
-  DCHECK(result_number < num_results);
-  // TODO(kochi): http://b/784900  Use relevance score from the NavSuggest
-  // server if possible.
+int SearchProvider::CalculateRelevanceForNavigation(
+    const NavigationResults& results,
+    size_t result_number,
+    bool is_keyword) const {
+  DCHECK(result_number < results.size());
+  if (results[result_number].has_suggested_relevance)
+    return results[result_number].relevance;
+
   return (providers_.is_primary_provider(is_keyword) ? 800 : 150) +
-      static_cast<int>(num_results - 1 - result_number);
+      static_cast<int>(results.size() - 1 - result_number);
 }
 
 void SearchProvider::AddMatchToMap(const string16& query_string,
@@ -959,8 +999,13 @@ AutocompleteMatch SearchProvider::NavigationToMatch(
   match.fill_into_edit.append(
       AutocompleteInput::FormattedStringWithEquivalentMeaning(navigation.url,
                                                               match.contents));
-  // TODO(pkasting): http://b/1112879 These should perhaps be
-  // inline-autocompletable?
+  // Inline-autocomplete NavigationResults; exclude AutocompleteInput::URL.
+  if (input_.type() != AutocompleteInput::URL &&
+      !input_.prevent_inline_autocomplete()) {
+    size_t start = match.fill_into_edit.find(input_text);
+    if (start != string16::npos)
+      match.inline_autocomplete_offset = start + input_text.length();
+  }
 
   return match;
 }