Eliminate QueryOptions::most_recent_visit_only. All non-unittest consumers of this set it to true, so just turn it on unconditionally.

BUG=none
TEST=none
Review URL: http://codereview.chromium.org/341087

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

14 files changed, 35 insertions, 99 deletions

diff --git a/chrome/browser/autocomplete/history_contents_provider.cc b/chrome/browser/autocomplete/history_contents_provider.cc
index d4d264e..8a950ca 100644
--- a/chrome/browser/autocomplete/history_contents_provider.cc
+++ b/chrome/browser/autocomplete/history_contents_provider.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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.
 
@@ -118,7 +118,6 @@ void HistoryContentsProvider::Start(const AutocompleteInput& input,
 
       history::QueryOptions options;
       options.SetRecentDayRange(kDaysToSearch);
-      options.most_recent_visit_only = true;
       options.max_count = kMaxMatchCount;
       history->QueryHistory(input.text(), options, &request_consumer_,
           NewCallback(this, &HistoryContentsProvider::QueryComplete));
diff --git a/chrome/browser/dom_ui/history_ui.cc b/chrome/browser/dom_ui/history_ui.cc
index e852ee9..8d517f9 100644
--- a/chrome/browser/dom_ui/history_ui.cc
+++ b/chrome/browser/dom_ui/history_ui.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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.
 
@@ -148,9 +148,6 @@ void BrowsingHistoryHandler::HandleGetHistory(const Value* value) {
   options.end_time = base::Time::Now().LocalMidnight();
   options.end_time -= base::TimeDelta::FromDays(day - 1);
 
-  // As we're querying per-day, we can turn entry repeats off.
-  options.most_recent_visit_only = true;
-
   // Need to remember the query string for our results.
   search_text_ = std::wstring();
 
@@ -174,10 +171,8 @@ void BrowsingHistoryHandler::HandleSearchHistory(const Value* value) {
   // Set the query ranges for the given month.
   history::QueryOptions options = CreateMonthQueryOptions(month);
 
-  // When searching, limit the number of results returned and only show the
-  // most recent matches.
+  // When searching, limit the number of results returned.
   options.max_count = kMaxSearchResults;
-  options.most_recent_visit_only = true;
 
   // Need to remember the query string for our results.
   search_text_ = query;
diff --git a/chrome/browser/extensions/extension_history_api.cc b/chrome/browser/extensions/extension_history_api.cc
index 8437a70..ee856de 100644
--- a/chrome/browser/extensions/extension_history_api.cc
+++ b/chrome/browser/extensions/extension_history_api.cc
@@ -245,7 +245,6 @@ bool SearchHistoryFunction::RunAsyncImpl() {
   EXTENSION_FUNCTION_VALIDATE(json->GetString(keys::kSearchKey, &search_text));
 
   history::QueryOptions options;
-  options.most_recent_visit_only = true;
   options.SetRecentDayRange(1);
   options.max_count = 100;
 
diff --git a/chrome/browser/history/expire_history_backend_unittest.cc b/chrome/browser/history/expire_history_backend_unittest.cc
index 31eab71..17b67c4 100644
--- a/chrome/browser/history/expire_history_backend_unittest.cc
+++ b/chrome/browser/history/expire_history_backend_unittest.cc
@@ -274,10 +274,9 @@ int ExpireHistoryTest::CountTextMatchesForURL(const GURL& url) {
   if (!text_db_.get())
     return 0;
 
-  // "body" should match all pagesx in the example data.
+  // "body" should match all pages in the example data.
   std::vector<TextDatabase::Match> results;
   QueryOptions options;
-  options.most_recent_visit_only = false;
   Time first_time;
   text_db_->GetTextMatches(L"body", options, &results, &first_time);
 
diff --git a/chrome/browser/history/history_backend.cc b/chrome/browser/history/history_backend.cc
index d614a92..7b45b22 100644
--- a/chrome/browser/history/history_backend.cc
+++ b/chrome/browser/history/history_backend.cc
@@ -1056,7 +1056,6 @@ void HistoryBackend::QueryHistoryBasic(URLDatabase* url_db,
   // First get all visits.
   VisitVector visits;
   visit_db->GetVisibleVisitsInRange(options.begin_time, options.end_time,
-                                    options.most_recent_visit_only,
                                     options.max_count, &visits);
   DCHECK(options.max_count == 0 ||
          static_cast<int>(visits.size()) <= options.max_count);
diff --git a/chrome/browser/history/history_backend_unittest.cc b/chrome/browser/history/history_backend_unittest.cc
index dd2e2fc..a1cebc1 100644
--- a/chrome/browser/history/history_backend_unittest.cc
+++ b/chrome/browser/history/history_backend_unittest.cc
@@ -459,8 +459,8 @@ TEST_F(HistoryBackendTest, KeywordGenerated) {
 
   // But no visible visits.
   visits.clear();
-  backend_->db()->GetVisibleVisitsInRange(
-      base::Time(), base::Time(), false, 1, &visits);
+  backend_->db()->GetVisibleVisitsInRange(base::Time(), base::Time(), 1,
+                                          &visits);
   EXPECT_TRUE(visits.empty());
 
   // Expire the visits.
diff --git a/chrome/browser/history/history_querying_unittest.cc b/chrome/browser/history/history_querying_unittest.cc
index f7375d3..3740fab 100644
--- a/chrome/browser/history/history_querying_unittest.cc
+++ b/chrome/browser/history/history_querying_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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.
 
@@ -149,15 +149,13 @@ TEST_F(HistoryQueryTest, Basic) {
   QueryOptions options;
   QueryResults results;
 
-  // First query for all of them to make sure they are there and in
-  // chronological order, most recent first.
+  // Test duplicate collapsing.
   QueryHistory(std::wstring(), options, &results);
-  ASSERT_EQ(5U, results.size());
+  EXPECT_EQ(4U, results.size());
   EXPECT_TRUE(NthResultIs(results, 0, 4));
   EXPECT_TRUE(NthResultIs(results, 1, 2));
   EXPECT_TRUE(NthResultIs(results, 2, 3));
   EXPECT_TRUE(NthResultIs(results, 3, 1));
-  EXPECT_TRUE(NthResultIs(results, 4, 0));
 
   // Next query a time range. The beginning should be inclusive, the ending
   // should be exclusive.
@@ -184,24 +182,6 @@ TEST_F(HistoryQueryTest, BasicCount) {
   EXPECT_TRUE(NthResultIs(results, 1, 2));
 }
 
-// Tests duplicate collapsing and not in non-Full Text Search situations.
-TEST_F(HistoryQueryTest, BasicDupes) {
-  ASSERT_TRUE(history_.get());
-
-  QueryOptions options;
-  QueryResults results;
-
-  // We did the query for no collapsing in the "Basic" test above, so here we
-  // only test collapsing.
-  options.most_recent_visit_only = true;
-  QueryHistory(std::wstring(), options, &results);
-  EXPECT_EQ(4U, results.size());
-  EXPECT_TRUE(NthResultIs(results, 0, 4));
-  EXPECT_TRUE(NthResultIs(results, 1, 2));
-  EXPECT_TRUE(NthResultIs(results, 2, 3));
-  EXPECT_TRUE(NthResultIs(results, 3, 1));
-}
-
 TEST_F(HistoryQueryTest, ReachedBeginning) {
   ASSERT_TRUE(history_.get());
 
@@ -359,14 +339,6 @@ TEST_F(HistoryQueryTest, FTSDupes) {
   QueryOptions options;
   QueryResults results;
 
-  // First do the search with collapsing.
-  QueryHistory(std::wstring(L"Other"), options, &results);
-  EXPECT_EQ(2, results.urls().size());
-  EXPECT_TRUE(NthResultIs(results, 0, 4));
-  EXPECT_TRUE(NthResultIs(results, 1, 0));
-
-  // Now with collapsing.
-  options.most_recent_visit_only = true;
   QueryHistory(std::wstring(L"Other"), options, &results);
   EXPECT_EQ(1, results.urls().size());
   EXPECT_TRUE(NthResultIs(results, 0, 4));
diff --git a/chrome/browser/history/history_types.h b/chrome/browser/history/history_types.h
index 02023f1..b064ccf 100644
--- a/chrome/browser/history/history_types.h
+++ b/chrome/browser/history/history_types.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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.
 
@@ -467,17 +467,15 @@ class QueryResults {
 // QueryOptions ----------------------------------------------------------------
 
 struct QueryOptions {
-  QueryOptions()
-      : most_recent_visit_only(false),
-        max_count(0) {
-  }
+  QueryOptions() : max_count(0) {}
 
   // The time range to search for matches in.
   //
-  // For text search queries, this will match only the most recent visit of the
-  // URL. If the URL was visited in the given time period, but has also been
+  // This will match only the one recent visit of a URL.  For text search
+  // queries, if the URL was visited in the given time period, but has also been
   // visited more recently than that, it will not be returned. When the text
-  // query is empty, this will return all URLs visited in the time range.
+  // query is empty, this will return the most recent visit within the time
+  // range.
   //
   // As a special case, if both times are is_null(), then the entire database
   // will be searched. However, if you set one, you must set the other.
@@ -492,14 +490,6 @@ struct QueryOptions {
     begin_time = end_time - base::TimeDelta::FromDays(days_ago);
   }
 
-  // When set, only one visit for each URL will be returned, which will be the
-  // most recent one in the result set. When false, each URL may have multiple
-  // visit entries corresponding to each time the URL was visited in the given
-  // time range.
-  //
-  // Defaults to false (all visits).
-  bool most_recent_visit_only;
-
   // The maximum number of results to return. The results will be sorted with
   // the most recent first, so older results may not be returned if there is not
   // enough room. When 0, this will return everything (the default).
diff --git a/chrome/browser/history/text_database.cc b/chrome/browser/history/text_database.cc
index 27d8e6b..c92c054 100644
--- a/chrome/browser/history/text_database.cc
+++ b/chrome/browser/history/text_database.cc
@@ -326,11 +326,9 @@ void TextDatabase::GetTextMatches(const std::string& query,
     //   break;
 
     GURL url(statement.ColumnString(0));
-    if (options.most_recent_visit_only) {
-      URLSet::const_iterator found_url = found_urls->find(url);
-      if (found_url != found_urls->end())
-        continue;  // Don't add this duplicate when unique URLs are requested.
-    }
+    URLSet::const_iterator found_url = found_urls->find(url);
+    if (found_url != found_urls->end())
+      continue;  // Don't add this duplicate.
 
     // Fill the results into the vector (avoid copying the URL with Swap()).
     results->resize(results->size() + 1);
diff --git a/chrome/browser/history/text_database.h b/chrome/browser/history/text_database.h
index e15ac52..d1cb843 100644
--- a/chrome/browser/history/text_database.h
+++ b/chrome/browser/history/text_database.h
@@ -121,10 +121,9 @@ class TextDatabase {
   // time considered for the output is in |first_time_searched|
   // (see QueryResults for more).
   //
-  // When |options.most_recent_visit_only|, any URLs found will be added to
-  // |unique_urls|. If a URL is already in the set, additional results will not
-  // be added (giving the ability to uniquify URL results, with the most recent
-  // If |most_recent_visit_only| is not set, |unique_urls| will be untouched.
+  // Any URLs found will be added to |unique_urls|. If a URL is already in the
+  // set, additional results will not be added (giving the ability to uniquify
+  // URL results).
   //
   // Callers must run QueryParser on the user text and pass the results of the
   // QueryParser to this method as the query string.
diff --git a/chrome/browser/history/visit_database.cc b/chrome/browser/history/visit_database.cc
index 2f4b591..4f15f2f 100644
--- a/chrome/browser/history/visit_database.cc
+++ b/chrome/browser/history/visit_database.cc
@@ -256,7 +256,6 @@ void VisitDatabase::GetVisitsInRangeForTransition(
 
 void VisitDatabase::GetVisibleVisitsInRange(base::Time begin_time,
                                             base::Time end_time,
-                                            bool most_recent_visit_only,
                                             int max_count,
                                             VisitVector* visits) {
   visits->clear();
@@ -289,12 +288,10 @@ void VisitDatabase::GetVisibleVisitsInRange(base::Time begin_time,
   while (statement.Step()) {
     VisitRow visit;
     FillVisitRow(statement, &visit);
-    if (most_recent_visit_only) {
-      // Make sure the URL this visit corresponds to is unique if required.
-      if (found_urls.find(visit.url_id) != found_urls.end())
-        continue;
-      found_urls.insert(visit.url_id);
-    }
+    // Make sure the URL this visit corresponds to is unique.
+    if (found_urls.find(visit.url_id) != found_urls.end())
+      continue;
+    found_urls.insert(visit.url_id);
     visits->push_back(visit);
 
     if (max_count > 0 && static_cast<int>(visits->size()) >= max_count)
diff --git a/chrome/browser/history/visit_database.h b/chrome/browser/history/visit_database.h
index 8613ace..a6dbf3c 100644
--- a/chrome/browser/history/visit_database.h
+++ b/chrome/browser/history/visit_database.h
@@ -89,10 +89,9 @@ class VisitDatabase {
   // that, the most recent |max_count| will be returned. If 0, all visits in the
   // range will be computed.
   //
-  // When |most_recent_visit_only| is set, only one visit for each URL will be
-  // returned, and it will be the most recent one in the time range.
+  // Only one visit for each URL will be returned, and it will be the most
+  // recent one in the time range.
   void GetVisibleVisitsInRange(base::Time begin_time, base::Time end_time,
-                               bool most_recent_visit_only,
                                int max_count,
                                VisitVector* visits);
 
diff --git a/chrome/browser/history/visit_database_unittest.cc b/chrome/browser/history/visit_database_unittest.cc
index aedb9e9..ebc2e1b 100644
--- a/chrome/browser/history/visit_database_unittest.cc
+++ b/chrome/browser/history/visit_database_unittest.cc
@@ -207,33 +207,24 @@ TEST_F(VisitDatabaseTest, GetVisibleVisitsInRange) {
   visit_info5.visit_id = 5;
   EXPECT_TRUE(AddVisit(&visit_info5));
 
-  // Query the visits for all time, we should get the first 3 in descending
-  // order, but not the redirect & subframe ones later.
+  // Query the visits for all time, we should not get the first (duplicate of
+  // the second) or the redirect or subframe visits.
   VisitVector results;
-  GetVisibleVisitsInRange(Time(), Time(), false, 0, &results);
-  ASSERT_EQ(static_cast<size_t>(3), results.size());
-  EXPECT_TRUE(IsVisitInfoEqual(results[0], visit_info4) &&
-              IsVisitInfoEqual(results[1], visit_info2) &&
-              IsVisitInfoEqual(results[2], visit_info1));
-
-  // If we want only the most recent one, it should give us the same results
-  // minus the first (duplicate of the second) one.
-  GetVisibleVisitsInRange(Time(), Time(), true, 0, &results);
+  GetVisibleVisitsInRange(Time(), Time(), 0, &results);
   ASSERT_EQ(static_cast<size_t>(2), results.size());
   EXPECT_TRUE(IsVisitInfoEqual(results[0], visit_info4) &&
               IsVisitInfoEqual(results[1], visit_info2));
 
   // Query a time range and make sure beginning is inclusive and ending is
   // exclusive.
-  GetVisibleVisitsInRange(visit_info2.visit_time, visit_info4.visit_time,
-                          false, 0, &results);
+  GetVisibleVisitsInRange(visit_info2.visit_time, visit_info4.visit_time, 0,
+                          &results);
   ASSERT_EQ(static_cast<size_t>(1), results.size());
   EXPECT_TRUE(IsVisitInfoEqual(results[0], visit_info2));
 
   // Query for a max count and make sure we get only that number.
-  GetVisibleVisitsInRange(Time(), Time(), false, 2, &results);
-  ASSERT_EQ(static_cast<size_t>(2), results.size());
-  EXPECT_TRUE(IsVisitInfoEqual(results[0], visit_info4) &&
-              IsVisitInfoEqual(results[1], visit_info2));
+  GetVisibleVisitsInRange(Time(), Time(), 1, &results);
+  ASSERT_EQ(static_cast<size_t>(1), results.size());
+  EXPECT_TRUE(IsVisitInfoEqual(results[0], visit_info4));
 }
 }  // namespace history
diff --git a/chrome/browser/possible_url_model.cc b/chrome/browser/possible_url_model.cc
index 2f73ed9..7fb38f9 100644
--- a/chrome/browser/possible_url_model.cc
+++ b/chrome/browser/possible_url_model.cc
@@ -47,7 +47,6 @@ void PossibleURLModel::Reload(Profile *profile) {
     options.end_time = Time::Now();
     options.begin_time =
         options.end_time - TimeDelta::FromDays(kPossibleURLTimeScope);
-    options.most_recent_visit_only = true;
     options.max_count = 50;
 
     hs->QueryHistory(std::wstring(), options, &consumer_,