[AiS] Merge answers into highest-scoring result

components/omnibox/base_search_provider.cc

 // Copyright 2014 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 "components/omnibox/base_search_provider.h"
 
 #include "base/i18n/case_conversion.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/metrics/proto/omnibox_event.pb.h"
@@ skipping 402 matching lines @@
         // is sufficiently relevant that the user is likely to choose it.
         // Surely setting the prefetch bit on a match of even higher relevance
         // won't violate this assumption.
         should_prefetch |= ShouldPrefetch(i.first->second);
         i.first->second.RecordAdditionalInfo(kShouldPrefetchKey,
                                              should_prefetch ? kTrue : kFalse);
         if (should_prefetch)
           i.first->second.RecordAdditionalInfo(kSuggestMetadataKey, metadata);
       }
     }
+    // Copy over answer data from lower-ranking item, if necessary.
+    const AutocompleteMatch& less_relevant_match =
+        i.first->second.duplicate_matches.back();

[Mark P, 2014/09/19 18:56:38]

This makes me a little nervous because you're depending that duplicate-tracking
code above always appends the latest duplicate to the end of the list.  I'm not
wedded to this idea (so if you think this behavior is obvious, you don't have to
do anything), but I think you should either:
- comment somewhere about relying on this property
- simply add another variable, MatchMap::const_iterator latest_duplicate, that
you set appropriately in either branch above, then use here.

[groby-ooo-7-16, 2014/09/22 05:49:25]

Comment added. I think it's fairly clear, but calling it out doesn't hurt. Not
sure the iterator would make it clearer. 

+    if (!less_relevant_match.answer_type.empty()) {
+      DCHECK(i.first->second.answer_type.empty());

[Mark P, 2014/09/19 18:56:38]

Why is this true?  Why can't multiple equivalent suggestions have answers
(probably the same answers)?

[groby-ooo-7-16, 2014/09/22 05:49:25]

Because suggestions are only provided by the search engine, not Chrome - and it
will only deliver one answer. The DCHECK is here to flag that necessity -
merging answers when there are two of them will be an interesting problem :)

+      i.first->second.answer_type = less_relevant_match.answer_type;
+      i.first->second.answer_contents = less_relevant_match.answer_contents;
+    }
   }
 }
 
 bool BaseSearchProvider::ParseSuggestResults(
     const base::Value& root_val,
     int default_result_relevance,
     bool is_keyword_result,
     SearchSuggestionParser::Results* results) {
   if (!SearchSuggestionParser::ParseSuggestResults(
       root_val, GetInput(is_keyword_result),
@@ skipping 27 matching lines @@
 }
 
 void BaseSearchProvider::OnDeletionComplete(
     bool success, SuggestionDeletionHandler* handler) {
   RecordDeletionResult(success);
   SuggestionDeletionHandlers::iterator it = std::find(
       deletion_handlers_.begin(), deletion_handlers_.end(), handler);
   DCHECK(it != deletion_handlers_.end());
   deletion_handlers_.erase(it);
 }