Clean up ShortcutsProvider and related classes.

chrome/browser/autocomplete/shortcuts_provider.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/shortcuts_provider.h"
 
 #include <algorithm>
 #include <cmath>
 #include <map>
 #include <vector>
@@ skipping 12 matching lines @@
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/guid.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "googleurl/src/url_parse.h"
 #include "googleurl/src/url_util.h"
 #include "net/base/escape.h"
 #include "net/base/net_util.h"
 
-using shortcuts_provider::Shortcut;
-using shortcuts_provider::ShortcutMap;
-
 namespace {
 
 class RemoveMatchPredicate {
  public:
   explicit RemoveMatchPredicate(const std::set<GURL>& urls)
       : urls_(urls) {
   }
   bool operator()(const AutocompleteMatch& match) {
     return urls_.find(match.destination_url) != urls_.end();
   }
@@ skipping 84 matching lines @@
        ++url)
     shortcuts_backend_->DeleteShortcutsWithUrl(*url);
 }
 
 void ShortcutsProvider::GetMatches(const AutocompleteInput& input) {
   // Get the URLs from the shortcuts database with keys that partially or
   // completely match the search term.
   string16 term_string(base::i18n::ToLower(input.text()));
   DCHECK(!term_string.empty());
 
-  for (ShortcutMap::const_iterator it = FindFirstMatch(term_string);
+  for (history::ShortcutsBackend::ShortcutMap::const_iterator it =
+       FindFirstMatch(term_string);
        it != shortcuts_backend_->shortcuts_map().end() &&
             StartsWith(it->first, term_string, true); ++it)
-    matches_.push_back(ShortcutToACMatch(input, term_string, it));
+    matches_.push_back(ShortcutToACMatch(input, term_string, it->second));
   std::partial_sort(matches_.begin(),
       matches_.begin() +
           std::min(AutocompleteProvider::kMaxMatches, matches_.size()),
       matches_.end(), &AutocompleteMatch::MoreRelevant);
   if (matches_.size() > AutocompleteProvider::kMaxMatches) {
     matches_.erase(matches_.begin() + AutocompleteProvider::kMaxMatches,
                    matches_.end());
   }
 }
 
 AutocompleteMatch ShortcutsProvider::ShortcutToACMatch(
     const AutocompleteInput& input,
     const string16& term_string,
-    ShortcutMap::const_iterator it) {
-  AutocompleteMatch match(this, CalculateScore(term_string, it->second),
+    const history::ShortcutsBackend::Shortcut& shortcut) {
+  AutocompleteMatch match(this, CalculateScore(term_string, shortcut),
                           true, AutocompleteMatch::HISTORY_TITLE);
-  match.destination_url = it->second.url;
+  match.destination_url = shortcut.url;
   DCHECK(match.destination_url.is_valid());
-  match.fill_into_edit = UTF8ToUTF16(it->second.url.spec());
+  match.fill_into_edit = UTF8ToUTF16(shortcut.url.spec());
 
-  match.contents = it->second.contents;
+  match.contents = shortcut.contents;
   match.contents_class = ClassifyAllMatchesInString(term_string,
                                                     match.contents,
-                                                    it->second.contents_class);
+                                                    shortcut.contents_class);
 
-  match.description = it->second.description;
+  match.description = shortcut.description;
   match.description_class = ClassifyAllMatchesInString(
-      term_string, match.description, it->second.description_class);
+      term_string, match.description, shortcut.description_class);
 
   return match;
 }
 
 // static
 ACMatchClassifications ShortcutsProvider::ClassifyAllMatchesInString(
     const string16& find_text,
     const string16& text,
     const ACMatchClassifications& original_class) {
   DCHECK(!find_text.empty());
@@ skipping 34 matching lines @@
       }
     }
 
     if (match_start >= match_end)
       break;
 
     // Collapse adjacent ranges into one.
     if (!match_class.empty() && match_class.back().offset == match_start)
       match_class.pop_back();
 
-    shortcuts_provider::AddLastMatchIfNeeded(&match_class, match_start,
-                                             ACMatchClassification::MATCH);
+    AutocompleteMatch::AddLastClassificationIfNecessary(&match_class,
+        match_start, ACMatchClassification::MATCH);

[GeorgeY, 2012/03/14 19:59:49]

May be move &match_class, to the same line as the rest of the parameters? Right
now it looks like two different styles are used.

[Peter Kasting, 2012/03/14 20:37:34]

This is explicitly OK: search for the "AndAnotherLongFunctionCall" example in
http://dev.chromium.org/developers/coding-style .

     if (match_end < text_lowercase.length()) {
-      shortcuts_provider::AddLastMatchIfNeeded(&match_class, match_end,
-                                               ACMatchClassification::NONE);
+      AutocompleteMatch::AddLastClassificationIfNecessary(&match_class,
+          match_end, ACMatchClassification::NONE);

[GeorgeY, 2012/03/14 19:59:49]

Here is well

     }
 
     last_position = match_end;
   }
 
   // Merge match-marking data with original classifications.
   if (match_class.empty())
     return original_class;
 
   ACMatchClassifications output;
   for (ACMatchClassifications::const_iterator i = original_class.begin(),
        j = match_class.begin(); i != original_class.end();) {
-    shortcuts_provider::AddLastMatchIfNeeded(&output,
-                                             std::max(i->offset, j->offset),
-                                             i->style | j->style);
+    AutocompleteMatch::AddLastClassificationIfNecessary(&output,
+        std::max(i->offset, j->offset), i->style | j->style);

[GeorgeY, 2012/03/14 19:59:49]

and here

     const size_t next_i_offset = (i + 1) == original_class.end() ?
         static_cast<size_t>(-1) : (i + 1)->offset;
     const size_t next_j_offset = (j + 1) == match_class.end() ?
         static_cast<size_t>(-1) : (j + 1)->offset;
     if (next_i_offset >= next_j_offset)
       ++j;
     if (next_j_offset >= next_i_offset)
       ++i;
   }
 
   return output;
 }
 
-ShortcutMap::const_iterator ShortcutsProvider::FindFirstMatch(
-    const string16& keyword) {
-  ShortcutMap::const_iterator it =
+history::ShortcutsBackend::ShortcutMap::const_iterator
+    ShortcutsProvider::FindFirstMatch(const string16& keyword) {
+  history::ShortcutsBackend::ShortcutMap::const_iterator it =
       shortcuts_backend_->shortcuts_map().lower_bound(keyword);
   // Lower bound not necessarily matches the keyword, check for item pointed by
   // the lower bound iterator to at least start with keyword.
   return ((it == shortcuts_backend_->shortcuts_map().end()) ||
     StartsWith(it->first, keyword, true)) ? it :
     shortcuts_backend_->shortcuts_map().end();
 }
 
 // static
-int ShortcutsProvider::CalculateScore(const string16& terms,
-                                      const Shortcut& shortcut) {
+int ShortcutsProvider::CalculateScore(
+    const string16& terms,
+    const history::ShortcutsBackend::Shortcut& shortcut) {
   DCHECK(!terms.empty());
   DCHECK_LE(terms.length(), shortcut.text.length());
 
   // The initial score is based on how much of the shortcut the user has typed.
   // Using the square root of the typed fraction boosts the base score rapidly
   // as characters are typed, compared with simply using the typed fraction
   // directly. This makes sense since the first characters typed are much more
   // important for determining how likely it is a user wants a particular
   // shortcut than are the remaining continued characters.
   double base_score = (AutocompleteResult::kLowestDefaultScore - 1) *
@@ skipping 21 matching lines @@
 }
 
 void ShortcutsProvider::set_shortcuts_backend(
     history::ShortcutsBackend* shortcuts_backend) {
   DCHECK(shortcuts_backend);
   shortcuts_backend_ = shortcuts_backend;
   shortcuts_backend_->AddObserver(this);
   if (shortcuts_backend_->initialized())
     initialized_ = true;
 }