Commander: make FuzzyFinder a class instead of a free function

Right now, this just caches the folded input string, but it's in
anticipation of a new algorithm that will want to hold on to scratch
space in between invocations.

Why not just make one and pass it to all of the command sources:
There's not *that* many sources, and it will complicate the interface.
I'd rather revisit it later if it ends up being a problem than take
on the extra complexity now. Doing it per source, OTOH, is a clear
savings since some sources can have many candidates.

Bug: 1014639
Change-Id: I85124976e50f0d0bcc33b307567738e97bee63d6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2575857Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Leonard Grey <lgrey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#834257}

chrome/browser/ui/commander/apps_command_source.cc

@@ -32,11 +32,11 @@ CommandSource::CommandResults AppsCommandSource::GetCommands(
   };
 
   CommandSource::CommandResults results;
-  const base::string16& folded_input = base::i18n::FoldCase(input);
   std::vector<gfx::Range> ranges;
+  FuzzyFinder finder(input);
   for (const auto& command_spec : command_map) {
     base::string16 title = base::ASCIIToUTF16(command_spec.title);
-    double score = FuzzyFind(folded_input, title, &ranges);
+    double score = finder.Find(title, &ranges);
     if (score == 0)
       continue;
 

chrome/browser/ui/commander/bookmark_command_source.cc

@@ -49,10 +49,10 @@ CommandSource::CommandResults GetMatchingBookmarks(
   DCHECK(model && model->loaded());
   std::vector<bookmarks::UrlAndTitle> bookmarks;
   model->GetBookmarks(&bookmarks);
-  const base::string16& folded_input = base::i18n::FoldCase(input);
+  FuzzyFinder finder(input);
   std::vector<gfx::Range> ranges;
   for (bookmarks::UrlAndTitle& bookmark : bookmarks) {
-    double score = FuzzyFind(folded_input, bookmark.title, &ranges);
+    double score = finder.Find(bookmark.title, &ranges);
     if (score > 0) {
       auto item = CreateOpenBookmarkItem(bookmark, browser);
       item->score = score;
@@ -83,12 +83,12 @@ CommandSource::CommandResults BookmarkCommandSource::GetCommands(
     results = GetMatchingBookmarks(browser, input);
   }
 
-  const base::string16& folded_input = base::i18n::FoldCase(input);
+  FuzzyFinder finder(input);
   std::vector<gfx::Range> ranges;
   // TODO(lgrey): Temporarily using an untranslated string since it's not
   // yet clear which commands will ship.
   base::string16 open_title = base::ASCIIToUTF16("Open bookmark...");
-  double score = FuzzyFind(folded_input, open_title, &ranges);
+  double score = finder.Find(open_title, &ranges);
   if (score > 0) {
     auto verb = std::make_unique<CommandItem>();
     verb->title = open_title;

chrome/browser/ui/commander/fuzzy_finder.cc

@@ -138,15 +138,16 @@ double ConsecutiveMatchWithGaps(const base::string16& needle,
 
 namespace commander {
 
-double FuzzyFind(const base::string16& needle,
-                 const base::string16& haystack,
-                 std::vector<gfx::Range>* matched_ranges) {
-  if (needle.size() == 0)
-    return 0;
-  DCHECK(needle == base::i18n::FoldCase(needle));
+FuzzyFinder::FuzzyFinder(const base::string16& needle)
+    : needle_(base::i18n::FoldCase(needle)) {}
+
+double FuzzyFinder::Find(const base::string16& haystack,
+                         std::vector<gfx::Range>* matched_ranges) {
   matched_ranges->clear();
+  if (needle_.size() == 0)
+    return 0;
   const base::string16& folded = base::i18n::FoldCase(haystack);
-  size_t m = needle.size();
+  size_t m = needle_.size();
   size_t n = folded.size();
   // Special case 0: M > N. We don't allow skipping anything in |needle|, so
   // no match possible.
@@ -156,8 +157,8 @@ double FuzzyFind(const base::string16& needle,
   // Special case 1: M == N. It must be either an exact match,
   // or a non-match.
   if (m == n) {
-    if (folded == needle) {
-      matched_ranges->emplace_back(0, needle.length());
+    if (folded == needle_) {
+      matched_ranges->emplace_back(0, needle_.length());
       return kMaxScore;
     } else {
       return 0;
@@ -174,7 +175,7 @@ double FuzzyFind(const base::string16& needle,
   //      Scored based on how far into haystack needle is found, normalized by
   //      haystack length.
   if (m == 1) {
-    size_t substring_position = folded.find(needle);
+    size_t substring_position = folded.find(needle_);
     while (substring_position != std::string::npos) {
       if (substring_position == 0) {
         // Prefix match.
@@ -196,7 +197,7 @@ double FuzzyFind(const base::string16& needle,
                                        substring_position + 1);
         }
       }
-      substring_position = folded.find(needle, substring_position + 1);
+      substring_position = folded.find(needle_, substring_position + 1);
     }
     if (matched_ranges->empty()) {
       return 0;
@@ -214,7 +215,7 @@ double FuzzyFind(const base::string16& needle,
   //    full O(mn) matching algorithm.
   // ***TEMPORARY***: The full algorithm isn't implemented yet, so we will use
   // this unconditionally for now.
-  return ConsecutiveMatchWithGaps(needle, folded, matched_ranges);
+  return ConsecutiveMatchWithGaps(needle_, folded, matched_ranges);
 }
 
 }  // namespace commander

chrome/browser/ui/commander/fuzzy_finder.h

@@ -12,18 +12,27 @@
 
 namespace commander {
 
-// Returns a score from 0 to 1 based on how well |needle| matches |haystack|.
-// 0 means no match. |matched_ranges| will be filled with the ranges of
-// |haystack| that match |needle| so they can be highlighted in the UI; see
-// comment on commander::CommandItem |matched_ranges| for a worked example.
-// |needle| is expected to already be case folded (this is DCHECKED) to save
-// redundant processing, as one needle will be checked against many haystacks.
-// TODO(lgrey): This currently uses an algorithm which is not guaranteed to
-// return the optimal match. Update this to use a more comprehensive method
-// when inputs are small enough.
-double FuzzyFind(const base::string16& needle,
-                 const base::string16& haystack,
-                 std::vector<gfx::Range>* matched_ranges);
+class FuzzyFinder {
+ public:
+  explicit FuzzyFinder(const base::string16& needle);
+  ~FuzzyFinder() = default;
+  FuzzyFinder(const FuzzyFinder& other) = delete;
+  FuzzyFinder& operator=(const FuzzyFinder& other) = delete;
+
+  // Returns a score from 0 to 1 based on how well |needle_| matches |haystack|.
+  // 0 means no match. |matched_ranges| will be filled with the ranges of
+  // |haystack| that match |needle| so they can be highlighted in the UI; see
+  // comment on commander::CommandItem |matched_ranges| for a worked example.
+  // TODO(lgrey): This currently uses an algorithm which is not guaranteed to
+  // return the optimal match. Update this to use a more comprehensive method
+  // when inputs are small enough.
+  double Find(const base::string16& haystack,
+              std::vector<gfx::Range>* matched_ranges);
+
+ private:
+  // Case-folded input string.
+  base::string16 needle_;
+};
 
 }  // namespace commander
 

chrome/browser/ui/commander/fuzzy_finder_unittest.cc

@@ -9,6 +9,17 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace commander {
+
+namespace {
+// Convenience function to avoid visual noise from constructing FuzzyFinder
+// objects in-test.
+double FuzzyFind(const base::string16& needle,
+                 const base::string16& haystack,
+                 std::vector<gfx::Range>* matched_ranges) {
+  return FuzzyFinder(needle).Find(haystack, matched_ranges);
+}
+}  // namespace
+
 TEST(CommanderFuzzyFinder, NonmatchIsZero) {
   std::vector<gfx::Range> ranges;
   EXPECT_EQ(0, FuzzyFind(base::ASCIIToUTF16("orange"),
@@ -39,15 +50,14 @@ TEST(CommanderFuzzyFinder, NeedleHaystackSameLength) {
 // coverage rather than ensuring the path is taken).
 TEST(CommanderFuzzyFinder, SingleCharNeedle) {
   std::vector<gfx::Range> ranges;
+  FuzzyFinder finder(base::ASCIIToUTF16("o"));
 
-  double prefix_score =
-      FuzzyFind(base::ASCIIToUTF16("o"), base::ASCIIToUTF16("orange"), &ranges);
+  double prefix_score = finder.Find(base::ASCIIToUTF16("orange"), &ranges);
   EXPECT_EQ(ranges, std::vector<gfx::Range>({{0, 1}}));
-  double internal_score =
-      FuzzyFind(base::ASCIIToUTF16("o"), base::ASCIIToUTF16("phone"), &ranges);
+  double internal_score = finder.Find(base::ASCIIToUTF16("phone"), &ranges);
   EXPECT_EQ(ranges, std::vector<gfx::Range>({{2, 3}}));
-  double boundary_score = FuzzyFind(
-      base::ASCIIToUTF16("o"), base::ASCIIToUTF16("phone operator"), &ranges);
+  double boundary_score =
+      finder.Find(base::ASCIIToUTF16("phone operator"), &ranges);
   EXPECT_EQ(ranges, std::vector<gfx::Range>({{6, 7}}));
 
   // Expected ordering:
@@ -59,8 +69,7 @@ TEST(CommanderFuzzyFinder, SingleCharNeedle) {
   EXPECT_GT(boundary_score, internal_score);
 
   // ...and non-matches should have score = 0.
-  EXPECT_EQ(0, FuzzyFind(base::ASCIIToUTF16("o"),
-                         base::ASCIIToUTF16("aquarium"), &ranges));
+  EXPECT_EQ(0, finder.Find(base::ASCIIToUTF16("aquarium"), &ranges));
   EXPECT_TRUE(ranges.empty());
 }
 
@@ -73,12 +82,10 @@ TEST(CommanderFuzzyFinder, CaseInsensitive) {
 
 TEST(CommanderFuzzyFinder, PrefixRanksHigherThanInternal) {
   std::vector<gfx::Range> ranges;
-
-  double prefix_rank = FuzzyFind(base::ASCIIToUTF16("orange"),
-                                 base::ASCIIToUTF16("Orange juice"), &ranges);
+  FuzzyFinder finder(base::ASCIIToUTF16("orange"));
+  double prefix_rank = finder.Find(base::ASCIIToUTF16("Orange juice"), &ranges);
   double non_prefix_rank =
-      FuzzyFind(base::ASCIIToUTF16("orange"),
-                base::ASCIIToUTF16("William of Orange"), &ranges);
+      finder.Find(base::ASCIIToUTF16("William of Orange"), &ranges);
 
   EXPECT_GT(prefix_rank, 0);
   EXPECT_GT(non_prefix_rank, 0);

chrome/browser/ui/commander/simple_command_source.cc

@@ -66,14 +66,14 @@ CommandSource::CommandResults SimpleCommandSource::GetCommands(
       {IDC_DEV_TOOLS_TOGGLE, base::ASCIIToUTF16("Toggle developer tools")},
   };
   CommandSource::CommandResults results;
-  const base::string16& folded_input = base::i18n::FoldCase(input);
+  FuzzyFinder finder(input);
   std::vector<gfx::Range> ranges;
   for (const auto& command_spec : command_map) {
     if (!chrome::IsCommandEnabled(browser, command_spec.id))
       continue;
     base::string16 title = command_spec.title;
     base::Erase(title, '&');
-    double score = FuzzyFind(folded_input, title, &ranges);
+    double score = finder.Find(title, &ranges);
     if (score == 0)
       continue;