Omnibox: Make URLs of Bookmarks Searchable - Try 2

chrome/browser/autocomplete/bookmark_provider_unittest.cc

Index: chrome/browser/autocomplete/bookmark_provider_unittest.cc
diff --git a/chrome/browser/autocomplete/bookmark_provider_unittest.cc b/chrome/browser/autocomplete/bookmark_provider_unittest.cc
index 6294b5697f8b44a99708168be93e2231c0c796f7..594d942ca07b5096af38edd3d3660f6429dd3f8f 100644
--- a/chrome/browser/autocomplete/bookmark_provider_unittest.cc
+++ b/chrome/browser/autocomplete/bookmark_provider_unittest.cc
@@ -33,8 +33,8 @@ struct BookmarksTestInfo {
   { "abc def", "http://www.catsanddogs.com/a" },
   { "abcde", "http://www.catsanddogs.com/b" },
   { "abcdef", "http://www.catsanddogs.com/c" },
-  { "a definition", "http://www.catsanddogs.com/d" },
-  { "carry carbon carefully", "http://www.catsanddogs.com/e" },
+  { "carry carbon carefully", "http://www.catsanddogs.com/d" },
+  { "a definition", "http://www.catsanddogs.com/e" },
   { "ghi jkl", "http://www.catsanddogs.com/f" },
   { "jkl ghi", "http://www.catsanddogs.com/g" },
   { "frankly frankly frank", "http://www.catsanddogs.com/h" },
@@ -49,21 +49,20 @@ struct BookmarksTestInfo {
   { "chrome://omnibox", "chrome://omnibox" },
   // For testing ranking with different URLs.
   {"achlorhydric featherheads resuscitates mockingbirds",

[Peter Kasting, 2014/08/27 19:36:45]

Nit: For consistency, put a space between { and " here and on subsequent lines

[Mark P, 2014/08/27 21:12:16]

Done.

-   "http://www.featherheads.com/a" },
+   "http://www.manylongwords.com/1a" },
   {"achlorhydric mockingbirds resuscitates featherhead",
-   "http://www.featherheads.com/b" },
+   "http://www.manylongwords.com/2b" },
   {"featherhead resuscitates achlorhydric mockingbirds",
-   "http://www.featherheads.com/c" },
+   "http://www.manylongwords.com/3c" },
   {"mockingbirds resuscitates featherheads achlorhydric",
-   "http://www.featherheads.com/d" },
-  // For testing URL boosting.
-  {"burning worms #1", "http://www.burned.com/" },
-  {"burning worms #2", "http://www.worms.com/" },
-  {"worming burns #10", "http://www.burned.com/" },
-  {"worming burns #20", "http://www.worms.com/" },
-  {"jive music", "http://www.worms.com/" },
+   "http://www.manylongwords.com/4d" },
+  // For testing URL boosting.  (URLs referenced multiple times are boosted.)
+  {"burning worms #1",  "http://www.burns.com/" },
+  {"burning worms #2",  "http://www.worms.com/" },
+  {"worming burns #10", "http://www.burns.com/" },
   // For testing strange spacing in bookmark titles.
   {" hello1  hello2  ", "http://whatever.com/" },
+  {"",                  "http://emptytitle.com/" },

[Mark P, 2014/08/26 21:33:35]

This has changed from the previous review.

 };
 
 class BookmarkProviderTest : public testing::Test {
@@ -83,7 +82,7 @@ class BookmarkProviderTest : public testing::Test {
 };
 
 BookmarkProviderTest::BookmarkProviderTest() {
-  model_ = client_.CreateModel(false);
+  model_ = client_.CreateModel();
 }
 
 void BookmarkProviderTest::SetUp() {
@@ -235,11 +234,12 @@ TEST_F(BookmarkProviderTest, Positions) {
                                   {{4, 7}, {0, 0}}}},
     {"ghi jkl",               2, {{{0, 3}, {4, 7}, {0, 0}},
                                   {{0, 3}, {4, 7}, {0, 0}}}},
-    // NB: GetBookmarksWithTitlesMatching(...) uses exact match for "a".
-    {"a",                     1, {{{0, 1}, {0, 0}}}},
+    // NB: GetBookmarksMatching(...) uses exact match for "a" in title or URL.
+    {"a",                     2, {{{0, 1}, {0, 0}},
+                                  {{0, 0}}}},
     {"a d",                   0, {{{0, 0}}}},
     {"carry carbon",          1, {{{0, 5}, {6, 12}, {0, 0}}}},
-    // NB: GetBookmarksWithTitlesMatching(...) sorts the match positions.
+    // NB: GetBookmarksMatching(...) sorts the match positions.
     {"carbon carry",          1, {{{0, 5}, {6, 12}, {0, 0}}}},
     {"arbon",                 0, {{{0, 0}}}},
     {"ar",                    0, {{{0, 0}}}},
@@ -257,6 +257,8 @@ TEST_F(BookmarkProviderTest, Positions) {
     {"foo foobar",            1, {{{0, 6}, {7, 13}, {0, 0}}}},
     // This one makes sure that leading whitespace in the title is removed.

[Peter Kasting, 2014/08/27 19:36:45]

Nit: one makes sure -> ensures (2 places)

[Mark P, 2014/08/27 21:12:16]

Done both.

     {"hello",                 1, {{{0, 5}, {7, 12}, {0, 0}}}},
+    // This one makes sure that empty titles yield empty classifications.
+    {"emptytitle",            1, {}},

[Mark P, 2014/08/26 21:33:35]

This has changed from the previous review.

   };
 
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(query_data); ++i) {
@@ -326,15 +328,18 @@ TEST_F(BookmarkProviderTest, Rankings) {
                   3, {"mockingbirds resuscitates featherheads achlorhydric",
                       "achlorhydric mockingbirds resuscitates featherhead",
                       "featherhead resuscitates achlorhydric mockingbirds"}},
-    // Ranking of exact-word matches with URL boost.
-    {"worms",     2, {"burning worms #2",    // boosted
-                      "burning worms #1",    // not boosted
+    // Ranking of exact-word matches with URL boosts.
+    {"worms",     2, {"burning worms #1",    // boosted
+                      "burning worms #2",    // not boosted
                       ""}},
-    // Ranking of prefix matches with URL boost. Note that a query of
-    // "worm burn" will have the same results.
-    {"burn worm", 3, {"burning worms #2",    // boosted
-                      "worming burns #20",   // boosted
-                      "burning worms #1"}},  // not boosted but shorter
+    // Ranking of prefix matches with URL boost.
+    {"burn worm", 3, {"burning worms #1",    // boosted
+                      "worming burns #10",   // boosted but longer title
+                      "burning worms #2"}},  // not boosted
+    // A query of "worm burn" will have the same results.
+    {"worm burn", 3, {"burning worms #1",    // boosted
+                      "worming burns #10",   // boosted but longer title
+                      "burning worms #2"}},  // not boosted
   };
 
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(query_data); ++i) {
@@ -436,10 +441,6 @@ TEST_F(BookmarkProviderTest, StripHttpAndAdjustOffsets) {
     { "versi",     "chrome://version",           "0:1,9:3,14:1"          }
   };
 
-  // Reload the bookmarks index with |index_urls| == true.
-  model_ = client_.CreateModel(true);
-  SetUp();
-
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(query_data); ++i) {
     std::string description = "for query=" + query_data[i].query;
     AutocompleteInput input(base::ASCIIToUTF16(query_data[i].query),