Omnibox metrics logging patch splitout, part 2: Remove the AutocompleteMatch NULL constructor. It's too easy to forget to set various members with this.

The changes from .resize() to .erase() are necessary because the compiler doesn't know resize() won't be enlarging the vector and thus needing to access the NULL constructor.  The changes to the HistoryContents shortcut code were similarly necessary to avoid a NULL construction, but in the end I think made the resulting code a bit clearer.
Review URL: http://codereview.chromium.org/10837

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@5271 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 23 insertions, 44 deletions

diff --git a/chrome/browser/autocomplete/autocomplete.cc b/chrome/browser/autocomplete/autocomplete.cc
index 4a49481..5e9b083 100644
--- a/chrome/browser/autocomplete/autocomplete.cc
+++ b/chrome/browser/autocomplete/autocomplete.cc
@@ -235,18 +235,6 @@ void AutocompleteInput::Clear() {
 
 // AutocompleteMatch ----------------------------------------------------------
 
-AutocompleteMatch::AutocompleteMatch()
-    : provider(NULL),
-      relevance(0),
-      deletable(false),
-      inline_autocomplete_offset(std::wstring::npos),
-      transition(PageTransition::TYPED),
-      is_history_what_you_typed_match(false),
-      type(URL),
-      template_url(NULL),
-      starred(false) {
-}
-
 AutocompleteMatch::AutocompleteMatch(AutocompleteProvider* provider,
                                      int relevance,
                                      bool deletable)
@@ -472,7 +460,7 @@ void AutocompleteResult::SortAndCull() {
   if (matches_.size() > max_matches()) {
     std::partial_sort(matches_.begin(), matches_.begin() + max_matches(),
                       matches_.end(), &AutocompleteMatch::MoreRelevant);
-    matches_.resize(max_matches());
+    matches_.erase(matches_.begin() + max_matches(), matches_.end());
   }
 
   // HistoryContentsProvider use a negative relevance as a way to avoid
@@ -696,11 +684,9 @@ void AutocompleteController::CommitResult() {
       Source<AutocompleteController>(this), NotificationService::NoDetails());
 }
 
-size_t AutocompleteController::CountMatchesNotInLatestResult(
-    const AutocompleteProvider* provider,
-    AutocompleteMatch* first_match) const {
+ACMatches AutocompleteController::GetMatchesNotInLatestResult(
+    const AutocompleteProvider* provider) const {
   DCHECK(provider);
-  DCHECK(first_match);
 
   // Determine the set of destination URLs.
   std::set<std::wstring> destination_urls;
@@ -708,19 +694,15 @@ size_t AutocompleteController::CountMatchesNotInLatestResult(
        i != latest_result_.end(); ++i)
     destination_urls.insert(i->destination_url);
 
+  ACMatches matches;
   const ACMatches& provider_matches = provider->matches();
-  bool found_first_unique_match = false;
-  size_t showing_count = 0;
   for (ACMatches::const_iterator i = provider_matches.begin();
        i != provider_matches.end(); ++i) {
-    if (destination_urls.find(i->destination_url) != destination_urls.end()) {
-      showing_count++;
-    } else if (!found_first_unique_match) {
-      found_first_unique_match = true;
-      *first_match = *i;
-    }
+    if (destination_urls.find(i->destination_url) == destination_urls.end())
+      matches.push_back(*i);
   }
-  return provider_matches.size() - showing_count;
+
+  return matches;
 }
 
 void AutocompleteController::AddHistoryContentsShortcut() {
@@ -736,17 +718,16 @@ void AutocompleteController::AddHistoryContentsShortcut() {
           (latest_result_.size() + 1)) ||
       (history_contents_provider_->db_match_count() == 1)) {
     // We only want to add a shortcut if we're not already showing the matches.
-    AutocompleteMatch first_unique_match;
-    size_t matches_not_shown = CountMatchesNotInLatestResult(
-        history_contents_provider_, &first_unique_match);
-    if (matches_not_shown == 0)
+    ACMatches matches(GetMatchesNotInLatestResult(history_contents_provider_));
+    if (matches.empty())
       return;
-    if (matches_not_shown == 1) {
+    if (matches.size() == 1) {
       // Only one match not shown, add it. The relevance may be negative,
       // which means we need to negate it to get the true relevance.
-      if (first_unique_match.relevance < 0)
-        first_unique_match.relevance = -first_unique_match.relevance;
-      latest_result_.AddMatch(first_unique_match);
+      AutocompleteMatch& match = matches.front();
+      if (match.relevance < 0)
+        match.relevance = -match.relevance;
+      latest_result_.AddMatch(match);
       return;
     } // else, fall through and add item.
   }
diff --git a/chrome/browser/autocomplete/autocomplete.h b/chrome/browser/autocomplete/autocomplete.h
index 78b03a2..ad4c138 100644
--- a/chrome/browser/autocomplete/autocomplete.h
+++ b/chrome/browser/autocomplete/autocomplete.h
@@ -297,7 +297,6 @@ struct AutocompleteMatch {
     HISTORY_SEARCH
   };
 
-  AutocompleteMatch();
   AutocompleteMatch(AutocompleteProvider* provider,
                     int relevance,
                     bool deletable);
@@ -735,11 +734,10 @@ class AutocompleteController : public ACProviderListener {
   // Copies |latest_result_| to |result_| and notifies observers of updates.
   void CommitResult();
 
-  // Returns the number of matches from provider whose destination urls are
-  // not in |latest_result_|. first_match is set to the first match whose
-  // destination url is NOT in the results.
-  size_t CountMatchesNotInLatestResult(const AutocompleteProvider* provider,
-                                       AutocompleteMatch* first_match) const;
+  // Returns the matches from |provider| whose destination urls are not in
+  // |latest_result_|.
+  ACMatches GetMatchesNotInLatestResult(
+      const AutocompleteProvider* provider) const;
 
   // If the HistoryContentsAutocomplete provider is done and there are more
   // matches in the database than currently shown, an entry is added to
diff --git a/chrome/browser/autocomplete/autocomplete_unittest.cc b/chrome/browser/autocomplete/autocomplete_unittest.cc
index 27d4ccb..386207c 100644
--- a/chrome/browser/autocomplete/autocomplete_unittest.cc
+++ b/chrome/browser/autocomplete/autocomplete_unittest.cc
@@ -257,8 +257,8 @@ TEST(AutocompleteMatch, MoreRelevant) {
     {  -5, -10, false },
   };
 
-  AutocompleteMatch m1;
-  AutocompleteMatch m2;
+  AutocompleteMatch m1(NULL, 0, false);
+  AutocompleteMatch m2(NULL, 0, false);
 
   for (int i = 0; i < arraysize(cases); ++i) {
     m1.relevance = cases[i].r1;
diff --git a/chrome/browser/autocomplete/search_provider.cc b/chrome/browser/autocomplete/search_provider.cc
index 96fd52f..2a2ec53 100644
--- a/chrome/browser/autocomplete/search_provider.cc
+++ b/chrome/browser/autocomplete/search_provider.cc
@@ -61,7 +61,7 @@ void SearchProvider::Start(const AutocompleteInput& input,
   if (input.text().empty()) {
     // User typed "?" alone.  Give them a placeholder result indicating what
     // this syntax does.
-    AutocompleteMatch match;
+    AutocompleteMatch match(this, 0, false);
     static const std::wstring kNoQueryInput(
         l10n_util::GetString(IDS_AUTOCOMPLETE_NO_QUERY));
     match.contents.assign(l10n_util::GetStringF(
@@ -373,7 +373,7 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() {
       matches_.begin() + std::min(max_total_matches, matches_.size()),
       matches_.end(), &AutocompleteMatch::MoreRelevant);
   if (matches_.size() > max_total_matches)
-    matches_.resize(max_total_matches);
+    matches_.erase(matches_.begin() + max_total_matches, matches_.end());
 
   UpdateStarredStateOfMatches();