summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authordubroy@chromium.org <dubroy@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-02-20 15:37:59 +0000
committerdubroy@chromium.org <dubroy@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-02-20 15:37:59 +0000
commitf7647c59b0baa285226b21075ee11039614782ea (patch)
tree2d24a2644d39bb28e341daad19a550f0fb75f709
parentba26a4440ea5dbd51953dd0a061e39b5861ab777 (diff)
downloadchromium_src-f7647c59b0baa285226b21075ee11039614782ea.zip
chromium_src-f7647c59b0baa285226b21075ee11039614782ea.tar.gz
chromium_src-f7647c59b0baa285226b21075ee11039614782ea.tar.bz2
History: Eliminate query cursor, use end_time argument instead.
The query cursor was introduced in http://crrev.com/171498 to allow querying for results a page at a time. For history sync, it's necessary to truncate the list of local results when they are merged with the results from the server. When results are truncated, the cursor becomes useless. This CL removes the cursor in favor of an end_time argument that specifies the end of the query range. BUG=176812 Review URL: https://chromiumcodereview.appspot.com/12218147 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@183547 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--chrome/browser/history/history_backend.cc10
-rw-r--r--chrome/browser/history/history_querying_unittest.cc82
-rw-r--r--chrome/browser/history/history_types.cc44
-rw-r--r--chrome/browser/history/history_types.h77
-rw-r--r--chrome/browser/history/text_database.cc60
-rw-r--r--chrome/browser/history/text_database.h3
-rw-r--r--chrome/browser/history/visit_database.cc42
-rw-r--r--chrome/browser/history/web_history_service.h2
-rw-r--r--chrome/browser/resources/history/history.js18
-rw-r--r--chrome/browser/ui/webui/history_ui.cc17
-rw-r--r--chrome/browser/ui/webui/history_ui.h11
-rw-r--r--chrome/test/data/webui/history_browsertest.js18
12 files changed, 121 insertions, 263 deletions
diff --git a/chrome/browser/history/history_backend.cc b/chrome/browser/history/history_backend.cc
index 934673c..9e8083d 100644
--- a/chrome/browser/history/history_backend.cc
+++ b/chrome/browser/history/history_backend.cc
@@ -1366,7 +1366,6 @@ void HistoryBackend::QueryHistoryBasic(URLDatabase* url_db,
// Now add them and the URL rows to the results.
URLResult url_result;
- QueryCursor cursor;
for (size_t i = 0; i < visits.size(); i++) {
const VisitRow visit = visits[i];
@@ -1399,11 +1398,7 @@ void HistoryBackend::QueryHistoryBasic(URLDatabase* url_db,
// We don't set any of the query-specific parts of the URLResult, since
// snippets and stuff don't apply to basic querying.
result->AppendURLBySwapping(&url_result);
-
- cursor.rowid_ = visit.visit_id;
- cursor.time_ = visit.visit_time;
}
- result->set_cursor(cursor);
if (!has_more_results && options.begin_time <= first_recorded_time_)
result->set_reached_beginning(true);
@@ -1425,7 +1420,6 @@ void HistoryBackend::QueryHistoryFTS(const string16& text_query,
// Now get the row and visit information for each one.
URLResult url_result; // Declare outside loop to prevent re-construction.
- QueryCursor cursor;
for (size_t i = 0; i < fts_matches.size(); i++) {
if (options.max_count != 0 &&
static_cast<int>(result->size()) >= options.max_count)
@@ -1457,11 +1451,7 @@ void HistoryBackend::QueryHistoryFTS(const string16& text_query,
// Add it to the vector, this will clear our |url_row| object as a
// result of the swap.
result->AppendURLBySwapping(&url_result);
-
- cursor.rowid_ = fts_matches[i].rowid;
- cursor.time_ = fts_matches[i].time;
}
- result->set_cursor(cursor);
if (first_time_searched <= first_recorded_time_)
result->set_reached_beginning(true);
diff --git a/chrome/browser/history/history_querying_unittest.cc b/chrome/browser/history/history_querying_unittest.cc
index 413a4d3..d1cf16b 100644
--- a/chrome/browser/history/history_querying_unittest.cc
+++ b/chrome/browser/history/history_querying_unittest.cc
@@ -46,13 +46,9 @@ struct TestEntry {
// A more recent visit of the first one.
{"http://example.com/", "Other", 6, "Other"},
- {"http://www.google.com/6", "Title 6", 12,
- "I'm the second oldest"},
-
- {"http://www.google.com/4", "Title 4", 11,
- "duplicate timestamps"},
- {"http://www.google.com/5", "Title 5", 11,
- "duplicate timestamps"},
+ {"http://www.google.com/6", "Title 6", 13, "I'm the second oldest"},
+ {"http://www.google.com/4", "Title 4", 12, "four"},
+ {"http://www.google.com/5", "Title 5", 11, "five"},
};
// Returns true if the nth result in the given results set matches. It will
@@ -78,7 +74,7 @@ bool NthResultIs(const QueryResults& results,
class HistoryQueryTest : public testing::Test {
public:
- HistoryQueryTest() {
+ HistoryQueryTest() : page_id_(0) {
}
// Acts like a synchronous call to history's QueryHistory.
@@ -93,12 +89,12 @@ class HistoryQueryTest : public testing::Test {
results->Swap(&last_query_results_);
}
- // Test paging through results using a cursor.
+ // Test paging through results, with a fixed number of results per page.
// Defined here so code can be shared for the FTS version and the non-FTS
// version.
- void TestWithCursor(const std::string& query_text,
- int* expected_results,
- int results_length) {
+ void TestPaging(const std::string& query_text,
+ const int* expected_results,
+ int results_length) {
ASSERT_TRUE(history_.get());
QueryOptions options;
@@ -110,27 +106,59 @@ class HistoryQueryTest : public testing::Test {
QueryHistory(query_text, options, &results);
ASSERT_EQ(1U, results.size());
EXPECT_TRUE(NthResultIs(results, 0, expected_results[i]));
- options.cursor = results.cursor();
+ options.end_time = results.back().visit_time();
}
QueryHistory(query_text, options, &results);
EXPECT_EQ(0U, results.size());
- // Try using a cursor with a max_count > 1.
+ // Try with a max_count > 1.
options.max_count = 2;
- options.cursor.Clear();
+ options.end_time = base::Time();
for (int i = 0; i < results_length / 2; i++) {
SCOPED_TRACE(testing::Message() << "i = " << i);
QueryHistory(query_text, options, &results);
ASSERT_EQ(2U, results.size());
EXPECT_TRUE(NthResultIs(results, 0, expected_results[i * 2]));
EXPECT_TRUE(NthResultIs(results, 1, expected_results[i * 2 + 1]));
- options.cursor = results.cursor();
+ options.end_time = results.back().visit_time();
}
+
+ // Add a couple of entries with duplicate timestamps. Use |query_text| as
+ // the body of both entries so that they match a full-text query.
+ TestEntry duplicates[] = {
+ { "http://www.google.com/x", "", 1, query_text.c_str() },
+ { "http://www.google.com/y", "", 1, query_text.c_str() }
+ };
+ AddEntryToHistory(duplicates[0]);
+ AddEntryToHistory(duplicates[1]);
+
+ // Make sure that paging proceeds even if there are duplicate timestamps.
+ options.end_time = base::Time();
+ do {
+ QueryHistory(query_text, options, &results);
+ ASSERT_NE(options.end_time, results.back().visit_time());
+ options.end_time = results.back().visit_time();
+ } while (!results.reached_beginning());
}
protected:
scoped_ptr<HistoryService> history_;
+ // Counter used to generate a unique ID for each page added to the history.
+ int32 page_id_;
+
+ void AddEntryToHistory(const TestEntry& entry) {
+ // We need the ID scope and page ID so that the visit tracker can find it.
+ const void* id_scope = reinterpret_cast<void*>(1);
+ GURL url(entry.url);
+
+ history_->AddPage(url, entry.time, id_scope, page_id_++, GURL(),
+ history::RedirectList(), content::PAGE_TRANSITION_LINK,
+ history::SOURCE_BROWSED, false);
+ history_->SetPageTitle(url, UTF8ToUTF16(entry.title));
+ history_->SetPageContents(url, UTF8ToUTF16(entry.body));
+ }
+
private:
virtual void SetUp() {
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
@@ -148,17 +176,7 @@ class HistoryQueryTest : public testing::Test {
for (size_t i = 0; i < arraysize(test_entries); i++) {
test_entries[i].time =
now - (test_entries[i].days_ago * TimeDelta::FromDays(1));
-
- // We need the ID scope and page ID so that the visit tracker can find it.
- const void* id_scope = reinterpret_cast<void*>(1);
- int32 page_id = i;
- GURL url(test_entries[i].url);
-
- history_->AddPage(url, test_entries[i].time, id_scope, page_id, GURL(),
- history::RedirectList(), content::PAGE_TRANSITION_LINK,
- history::SOURCE_BROWSED, false);
- history_->SetPageTitle(url, UTF8ToUTF16(test_entries[i].title));
- history_->SetPageContents(url, UTF8ToUTF16(test_entries[i].body));
+ AddEntryToHistory(test_entries[i]);
}
}
@@ -445,20 +463,20 @@ TEST_F(HistoryQueryTest, FTSDupes) {
}
*/
-// Test iterating over pages of results using a cursor.
-TEST_F(HistoryQueryTest, Cursor) {
+// Test iterating over pages of results.
+TEST_F(HistoryQueryTest, Paging) {
// Since results are fetched 1 and 2 at a time, entry #0 and #6 will not
// be de-duplicated.
int expected_results[] = { 4, 2, 3, 1, 7, 6, 5, 0 };
- TestWithCursor("", expected_results, arraysize(expected_results));
+ TestPaging("", expected_results, arraysize(expected_results));
}
-TEST_F(HistoryQueryTest, FTSCursor) {
+TEST_F(HistoryQueryTest, FTSPaging) {
// Since results are fetched 1 and 2 at a time, entry #0 and #6 will not
// be de-duplicated. Entry #4 does not contain the text "title", so it
// shouldn't appear.
int expected_results[] = { 2, 3, 1, 7, 6, 5 };
- TestWithCursor("title", expected_results, arraysize(expected_results));
+ TestPaging("title", expected_results, arraysize(expected_results));
}
} // namespace history
diff --git a/chrome/browser/history/history_types.cc b/chrome/browser/history/history_types.cc
index 70e0e2b..2133761 100644
--- a/chrome/browser/history/history_types.cc
+++ b/chrome/browser/history/history_types.cc
@@ -8,8 +8,6 @@
#include "base/logging.h"
#include "base/stl_util.h"
-#include "base/strings/string_number_conversions.h"
-#include "base/values.h"
#include "chrome/browser/history/page_usage_data.h"
namespace history {
@@ -118,38 +116,6 @@ void URLResult::SwapResult(URLResult* other) {
title_match_positions_.swap(other->title_match_positions_);
}
-// QueryCursor -----------------------------------------------------------------
-
-QueryCursor::QueryCursor() : rowid_(0) {
-}
-
-QueryCursor::~QueryCursor() {
-}
-
-Value* QueryCursor::ToValue() const {
- ListValue* value = new ListValue;
- value->AppendString(base::Int64ToString(time_.ToInternalValue()));
- value->AppendString(base::Int64ToString(rowid_));
- return value;
-}
-
-bool QueryCursor::FromValue(const Value* value, QueryCursor* cursor) {
- if (value->GetType() == Value::TYPE_LIST) {
- const ListValue* list_value = static_cast<const ListValue*>(value);
- string16 string_value;
- int64 timestamp;
-
- if (list_value->GetString(0, &string_value) &&
- base::StringToInt64(string_value, &timestamp) &&
- list_value->GetString(1, &string_value) &&
- base::StringToInt64(string_value, &cursor->rowid_)) {
- cursor->time_ = base::Time::FromInternalValue(timestamp);
- return true;
- }
- }
- return false;
-}
-
// QueryResults ----------------------------------------------------------------
QueryResults::QueryResults() : reached_beginning_(false) {
@@ -180,7 +146,6 @@ const size_t* QueryResults::MatchesForURL(const GURL& url,
void QueryResults::Swap(QueryResults* other) {
std::swap(first_time_searched_, other->first_time_searched_);
std::swap(reached_beginning_, other->reached_beginning_);
- std::swap(cursor_, other->cursor_);
results_.swap(other->results_);
url_to_results_.swap(other->url_to_results_);
}
@@ -288,15 +253,8 @@ int64 QueryOptions::EffectiveBeginTime() const {
}
int64 QueryOptions::EffectiveEndTime() const {
- int64 end = end_time.is_null() ?
+ return end_time.is_null() ?
std::numeric_limits<int64>::max() : end_time.ToInternalValue();
-
- // If the cursor is specified, it provides the true end time.
- if (!cursor.empty()) {
- DCHECK(cursor.time_.ToInternalValue() <= end);
- return cursor.time_.ToInternalValue();
- }
- return end;
}
int QueryOptions::EffectiveMaxCount() const {
diff --git a/chrome/browser/history/history_types.h b/chrome/browser/history/history_types.h
index fdb435b..b62838d 100644
--- a/chrome/browser/history/history_types.h
+++ b/chrome/browser/history/history_types.h
@@ -16,7 +16,6 @@
#include "base/memory/ref_counted_memory.h"
#include "base/string16.h"
#include "base/time.h"
-#include "base/values.h"
#include "chrome/browser/history/snippet.h"
#include "chrome/browser/search_engines/template_url_id.h"
#include "chrome/common/ref_counted_util.h"
@@ -32,9 +31,7 @@ namespace history {
// Forward declaration for friend statements.
class HistoryBackend;
-class TextDatabase;
class URLDatabase;
-class VisitDatabase;
// Structure to hold redirect lists for URLs. For a redirect chain
// A -> B -> C, and entry in the map would look like "A => {B -> C}".
@@ -324,42 +321,6 @@ class URLResult : public URLRow {
// We support the implicit copy constructor and operator=.
};
-// QueryCursor -----------------------------------------------------------------
-
-// Represents the point at which a QueryResult ended. This should be treated as
-// an opaque token by clients of the history service.
-class QueryCursor {
- public:
- QueryCursor();
- ~QueryCursor();
-
- // Returns a newly-allocated Value object representing the cursor. The caller
- // takes ownership of the value.
- Value* ToValue() const;
-
- // Opposite of ToValue() -- converts a Value object to a QueryCursor.
- // Returns true if the conversion was successful.
- static bool FromValue(const Value* value, QueryCursor* cursor);
-
- bool empty() const {
- return time_.is_null() && rowid_ == 0;
- }
-
- void Clear() {
- time_ = base::Time();
- rowid_ = 0;
- }
-
- private:
- friend class HistoryBackend;
- friend struct QueryOptions;
- friend class TextDatabase;
- friend class VisitDatabase;
-
- int64 rowid_;
- base::Time time_;
-};
-
// QueryResults ----------------------------------------------------------------
// Encapsulates the results of a history query. It supports an ordered list of
@@ -391,12 +352,12 @@ class QueryResults {
void set_reached_beginning(bool reached) { reached_beginning_ = reached; }
bool reached_beginning() { return reached_beginning_; }
- void set_cursor(const QueryCursor& cursor) { cursor_ = cursor; }
- QueryCursor cursor() { return cursor_; }
-
size_t size() const { return results_.size(); }
bool empty() const { return results_.empty(); }
+ URLResult& back() { return *results_.back(); }
+ const URLResult& back() const { return *results_.back(); }
+
URLResult& operator[](size_t i) { return *results_[i]; }
const URLResult& operator[](size_t i) const { return *results_[i]; }
@@ -460,23 +421,6 @@ class QueryResults {
// Maps URLs to entries in results_.
URLToResultIndices url_to_results_;
- // An opaque value representing the point at which the QueryResult begins.
- // This value can be passed to a future query through QueryOptions::cursor,
- // in order to fetch a set of results contiguous to, but strictly older than,
- // the results in this object. (Just using timestamps doesn't work because
- // multiple visits can have the same timestamp.)
- //
- // For example, to fetch all results, 100 at a time:
- //
- // QueryOptions options;
- // QueryResults results;
- // options.max_count = 100;
- // do {
- // QueryHistory(query_text, options, &results);
- // options.cursor = results.cursor();
- // } while (results.size() > 0);
- QueryCursor cursor_;
-
DISALLOW_COPY_AND_ASSIGN(QueryResults);
};
@@ -486,8 +430,7 @@ struct QueryOptions {
QueryOptions();
// The time range to search for matches in. The beginning is inclusive and
- // the ending is exclusive. Either one (or both) may be null. If |cursor| is
- // set, it takes precedence over end_time.
+ // the ending is exclusive. Either one (or both) may be null.
//
// 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
@@ -509,13 +452,6 @@ struct QueryOptions {
// including url and time. Defaults to false.
bool body_only;
- // If set, the cursor provides an alternate way to specify the end of the
- // query range. The value should come from QueryResults::cursor() from a
- // previous query, which means that the new query should only return results
- // older than the ones in the previous query. Due to the possiblity of
- // duplicate timestamps, |end_time| is not sufficient for that purpose.
- QueryCursor cursor;
-
enum DuplicateHandling {
// Omit visits for which there is a more recent visit to the same URL.
// Each URL in the results will appear only once.
@@ -535,11 +471,6 @@ struct QueryOptions {
// "unspecified".
int EffectiveMaxCount() const;
int64 EffectiveBeginTime() const;
-
- // The effective end time can be determined by either |end_time| or |cursor|.
- // If cursor is set, it takes precedence. This allows consecutive queries to
- // re-use the same QueryOptions to fetch consecutive pages of results by
- // simply copying the cursor from the QueryResults of the previous query.
int64 EffectiveEndTime() const;
};
diff --git a/chrome/browser/history/text_database.cc b/chrome/browser/history/text_database.cc
index 2fd1e27..fd62904 100644
--- a/chrome/browser/history/text_database.cc
+++ b/chrome/browser/history/text_database.cc
@@ -287,43 +287,24 @@ bool TextDatabase::GetTextMatches(const std::string& query,
const QueryOptions& options,
std::vector<Match>* results,
URLSet* found_urls) {
- std::string sql =
- "SELECT info.rowid, url, title, time, offsets(pages), body FROM pages "
- "LEFT OUTER JOIN info ON pages.rowid = info.rowid WHERE ";
+ std::string sql = "SELECT url, title, time, offsets(pages), body FROM pages "
+ "LEFT OUTER JOIN info ON pages.rowid = info.rowid WHERE ";
sql += options.body_only ? "body " : "pages ";
- sql += "MATCH ? AND time >= ? AND (time < ?";
- if (!options.cursor.empty())
- sql += " OR (time = ? AND info.rowid < ?)";
+ sql += "MATCH ? AND time >= ? AND time < ? ";
// Times may not be unique, so also sort by rowid to ensure a stable order.
- sql += ") ORDER BY time DESC, info.rowid DESC";
+ sql += "ORDER BY time DESC, info.rowid DESC";
- // Generate unique IDs for the different variations of the statement,
+ // Generate unique IDs for the two possible variations of the statement,
// so they don't share the same cached prepared statement.
sql::StatementID body_only_id = SQL_FROM_HERE;
sql::StatementID pages_id = SQL_FROM_HERE;
- sql::StatementID pages_with_cursor_id = SQL_FROM_HERE;
-
- // Ensure that cursor and body_only aren't both specified, because that
- // combination is not covered here.
- DCHECK(!options.body_only || options.cursor.empty());
-
- // Choose the correct statement ID based on the options.
- sql::StatementID statement_id = pages_id;
- if (options.body_only)
- statement_id = body_only_id;
- else if (!options.cursor.empty())
- statement_id = pages_with_cursor_id;
-
- sql::Statement statement(db_.GetCachedStatement(statement_id, sql.c_str()));
-
- int i = 0;
- statement.BindString(i++, query);
- statement.BindInt64(i++, options.EffectiveBeginTime());
- statement.BindInt64(i++, options.EffectiveEndTime());
- if (!options.cursor.empty()) {
- statement.BindInt64(i++, options.EffectiveEndTime());
- statement.BindInt64(i++, options.cursor.rowid_);
- }
+
+ sql::Statement statement(db_.GetCachedStatement(
+ (options.body_only ? body_only_id : pages_id), sql.c_str()));
+
+ statement.BindString(0, query);
+ statement.BindInt64(1, options.EffectiveBeginTime());
+ statement.BindInt64(2, options.EffectiveEndTime());
// |results| may not be initially empty, so keep track of how many were added
// by this call.
@@ -334,28 +315,27 @@ bool TextDatabase::GetTextMatches(const std::string& query,
// if (canceled_or_something)
// break;
- GURL url(statement.ColumnString(1));
+ GURL url(statement.ColumnString(0));
URLSet::const_iterator found_url = found_urls->find(url);
if (found_url != found_urls->end())
continue; // Don't add this duplicate.
- if (options.max_count > 0 && ++result_count > options.max_count)
+ if (++result_count > options.EffectiveMaxCount())
break;
// Fill the results into the vector (avoid copying the URL with Swap()).
results->resize(results->size() + 1);
Match& match = results->at(results->size() - 1);
- match.rowid = statement.ColumnInt64(0);
match.url.Swap(&url);
- match.title = statement.ColumnString16(2);
- match.time = base::Time::FromInternalValue(statement.ColumnInt64(3));
+ match.title = statement.ColumnString16(1);
+ match.time = base::Time::FromInternalValue(statement.ColumnInt64(2));
// Extract any matches in the title.
- std::string offsets_str = statement.ColumnString(4);
+ std::string offsets_str = statement.ColumnString(3);
Snippet::ExtractMatchPositions(offsets_str, kTitleColumnIndex,
&match.title_match_positions);
- Snippet::ConvertMatchPositionsToWide(statement.ColumnString(2),
+ Snippet::ConvertMatchPositionsToWide(statement.ColumnString(1),
&match.title_match_positions);
// Extract the matches in the body.
@@ -364,11 +344,11 @@ bool TextDatabase::GetTextMatches(const std::string& query,
&match_positions);
// Compute the snippet based on those matches.
- std::string body = statement.ColumnString(5);
+ std::string body = statement.ColumnString(4);
match.snippet.ComputeSnippet(match_positions, body);
}
statement.Reset(true);
- return result_count > options.max_count;
+ return result_count > options.EffectiveMaxCount();
}
} // namespace history
diff --git a/chrome/browser/history/text_database.h b/chrome/browser/history/text_database.h
index 7be7870..99ccff0 100644
--- a/chrome/browser/history/text_database.h
+++ b/chrome/browser/history/text_database.h
@@ -30,9 +30,6 @@ class TextDatabase {
Match();
~Match();
- // The database rowid of the match.
- int64 rowid;
-
// URL of the match.
GURL url;
diff --git a/chrome/browser/history/visit_database.cc b/chrome/browser/history/visit_database.cc
index 939415a..289ce9d 100644
--- a/chrome/browser/history/visit_database.cc
+++ b/chrome/browser/history/visit_database.cc
@@ -320,39 +320,23 @@ bool VisitDatabase::GetVisitsInRangeForTransition(
bool VisitDatabase::GetVisibleVisitsInRange(const QueryOptions& options,
VisitVector* visits) {
visits->clear();
- std::string sql = "SELECT" HISTORY_VISIT_ROW_FIELDS "FROM visits "
- "WHERE visit_time >= ? AND (visit_time < ? ";
- if (!options.cursor.empty())
- sql += " OR (visit_time = ? and id < ?)";
-
// The visit_time values can be duplicated in a redirect chain, so we sort
// by id too, to ensure a consistent ordering just in case.
- sql += ") AND (transition & ?) != 0 " // CHAIN_END
+ sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
+ "SELECT" HISTORY_VISIT_ROW_FIELDS "FROM visits "
+ "WHERE visit_time >= ? AND visit_time < ? "
+ "AND (transition & ?) != 0 " // CHAIN_END
"AND (transition & ?) NOT IN (?, ?, ?) " // NO SUBFRAME or
// KEYWORD_GENERATED
- "ORDER BY visit_time DESC, id DESC";
-
- // Generate unique IDs for the different variations of the statement,
- // so they don't share the same cached prepared statement.
- sql::StatementID id_with_cursor = SQL_FROM_HERE;
- sql::StatementID id_without_cursor = SQL_FROM_HERE;
-
- sql::Statement statement(GetDB().GetCachedStatement(
- options.cursor.empty() ? id_without_cursor : id_with_cursor,
- sql.c_str()));
-
- int i = 0;
- statement.BindInt64(i++, options.EffectiveBeginTime());
- statement.BindInt64(i++, options.EffectiveEndTime());
- if (!options.cursor.empty()) {
- statement.BindInt64(i++, options.EffectiveEndTime());
- statement.BindInt64(i++, options.cursor.rowid_);
- }
- statement.BindInt(i++, content::PAGE_TRANSITION_CHAIN_END);
- statement.BindInt(i++, content::PAGE_TRANSITION_CORE_MASK);
- statement.BindInt(i++, content::PAGE_TRANSITION_AUTO_SUBFRAME);
- statement.BindInt(i++, content::PAGE_TRANSITION_MANUAL_SUBFRAME);
- statement.BindInt(i++, content::PAGE_TRANSITION_KEYWORD_GENERATED);
+ "ORDER BY visit_time DESC, id DESC"));
+
+ statement.BindInt64(0, options.EffectiveBeginTime());
+ statement.BindInt64(1, options.EffectiveEndTime());
+ statement.BindInt(2, content::PAGE_TRANSITION_CHAIN_END);
+ statement.BindInt(3, content::PAGE_TRANSITION_CORE_MASK);
+ statement.BindInt(4, content::PAGE_TRANSITION_AUTO_SUBFRAME);
+ statement.BindInt(5, content::PAGE_TRANSITION_MANUAL_SUBFRAME);
+ statement.BindInt(6, content::PAGE_TRANSITION_KEYWORD_GENERATED);
std::set<URLID> found_urls;
diff --git a/chrome/browser/history/web_history_service.h b/chrome/browser/history/web_history_service.h
index 039ab14..577eb15 100644
--- a/chrome/browser/history/web_history_service.h
+++ b/chrome/browser/history/web_history_service.h
@@ -38,7 +38,7 @@ class WebHistoryService : public ProfileKeyedService {
// Callback with the result of a call to QueryHistory(). Currently, the
// DictionaryValue is just the parsed JSON response from the server.
// TODO(dubroy): Extract the DictionaryValue into a structured results object.
- typedef base::Callback<void(Request*, const DictionaryValue*)>
+ typedef base::Callback<void(Request*, const base::DictionaryValue*)>
QueryWebHistoryCallback;
typedef base::Callback<void(Request*, bool success)>
diff --git a/chrome/browser/resources/history/history.js b/chrome/browser/resources/history/history.js
index b49b443..557f80b 100644
--- a/chrome/browser/resources/history/history.js
+++ b/chrome/browser/resources/history/history.js
@@ -405,7 +405,6 @@ HistoryModel.prototype.addResults = function(info, results) {
$('loading-spinner').hidden = true;
this.inFlight_ = false;
this.isQueryFinished_ = info.finished;
- this.queryCursor_ = info.cursor;
this.queryStartTime = info.queryStartTime;
this.queryEndTime = info.queryEndTime;
@@ -517,10 +516,6 @@ HistoryModel.prototype.clearModel_ = function() {
// currently held in |this.visits_|.
this.isQueryFinished_ = false;
- // An opaque value that is returned with the query results. This is used to
- // fetch the next page of results for a query.
- this.queryCursor_ = null;
-
if (this.view_)
this.view_.clear_();
};
@@ -539,9 +534,8 @@ HistoryModel.prototype.updateSearch_ = function() {
// Try to fetch more results if more results can arrive and the page is not
// full.
- if (!doneLoading && !this.inFlight_) {
+ if (!doneLoading && !this.inFlight_)
this.queryHistory_();
- }
// Show the result or a message if no results were returned.
this.view_.onModelReady(doneLoading);
@@ -552,14 +546,18 @@ HistoryModel.prototype.updateSearch_ = function() {
* @private
*/
HistoryModel.prototype.queryHistory_ = function() {
- var max_results =
+ var maxResults =
(this.rangeInDays_ == HistoryModel.Range.ALL_TIME) ? RESULTS_PER_PAGE : 0;
+ // If there are already some visits, pick up the previous query where it
+ // left off.
+ var lastVisit = this.visits_.slice(-1)[0];
+ var endTime = lastVisit ? lastVisit.date.getTime() : 0;
+
$('loading-spinner').hidden = false;
this.inFlight_ = true;
chrome.send('queryHistory',
- [this.searchText_, this.offset_, this.rangeInDays_, this.queryCursor_,
- max_results]);
+ [this.searchText_, this.offset_, this.rangeInDays_, endTime, maxResults]);
};
/**
diff --git a/chrome/browser/ui/webui/history_ui.cc b/chrome/browser/ui/webui/history_ui.cc
index 97fae46..04bc547 100644
--- a/chrome/browser/ui/webui/history_ui.cc
+++ b/chrome/browser/ui/webui/history_ui.cc
@@ -310,9 +310,8 @@ void BrowsingHistoryHandler::HandleQueryHistory(const ListValue* args) {
// month, set by the next argument).
// - the range (BrowsingHistoryHandler::Range) Enum value that sets the range
// of the query.
- // - the search cursor, an opaque value from a previous query result, which
- // allows this query to pick up where the previous one left off. May be
- // null or undefined.
+ // - the end time for the query. Only results older than this time will be
+ // returned.
// - the maximum number of results to return (may be 0, meaning that there
// is no maximum).
string16 search_text = ExtractStringValue(args);
@@ -332,15 +331,13 @@ void BrowsingHistoryHandler::HandleQueryHistory(const ListValue* args) {
else if (range == BrowsingHistoryHandler::WEEK)
SetQueryTimeInWeeks(offset, &options);
- const Value* cursor_value;
-
- // Get the cursor. It must be either null, or a list.
- if (!args->Get(3, &cursor_value) ||
- (!cursor_value->IsType(Value::TYPE_NULL) &&
- !history::QueryCursor::FromValue(cursor_value, &options.cursor))) {
+ double end_time;
+ if (!args->GetDouble(3, &end_time)) {
NOTREACHED() << "Failed to convert argument 3. ";
return;
}
+ if (end_time)
+ options.end_time = base::Time::FromJsTime(end_time);
if (!ExtractIntegerValueAtIndex(args, 4, &options.max_count)) {
NOTREACHED() << "Failed to convert argument 4.";
@@ -535,7 +532,7 @@ void BrowsingHistoryHandler::QueryComplete(
results_info_value_.SetString("term", search_text);
results_info_value_.SetBoolean("finished", results->reached_beginning());
- results_info_value_.Set("cursor", results->cursor().ToValue());
+
// Add the specific dates that were searched to display them.
// TODO(sergiu): Put today if the start is in the future.
results_info_value_.SetString("queryStartTime",
diff --git a/chrome/browser/ui/webui/history_ui.h b/chrome/browser/ui/webui/history_ui.h
index 608b383..b068b47 100644
--- a/chrome/browser/ui/webui/history_ui.h
+++ b/chrome/browser/ui/webui/history_ui.h
@@ -9,6 +9,7 @@
#include "base/string16.h"
#include "base/timer.h"
+#include "base/values.h"
#include "chrome/browser/common/cancelable_request.h"
#include "chrome/browser/history/history_service.h"
#include "chrome/browser/history/web_history_service.h"
@@ -48,7 +49,7 @@ class BrowsingHistoryHandler : public content::WebUIMessageHandler,
// Removes duplicate visits from the given list of query results, only
// retaining the most recent visit to a URL on a particular day. |results|
// must already be sorted by visit time, most recent first.
- static void RemoveDuplicateResults(ListValue* results);
+ static void RemoveDuplicateResults(base::ListValue* results);
private:
// The range for which to return results:
@@ -65,7 +66,7 @@ class BrowsingHistoryHandler : public content::WebUIMessageHandler,
void QueryHistory(string16 search_text, const history::QueryOptions& options);
// Creates a history query result value.
- DictionaryValue* CreateQueryResultValue(
+ base::DictionaryValue* CreateQueryResultValue(
const GURL& url, const string16& title, base::Time visit_time,
bool is_search_result, const string16& snippet);
@@ -90,7 +91,7 @@ class BrowsingHistoryHandler : public content::WebUIMessageHandler,
void WebHistoryQueryComplete(const string16& search_text,
const history::QueryOptions& options,
history::WebHistoryService::Request* request,
- const DictionaryValue* results_value);
+ const base::DictionaryValue* results_value);
// Callback from the history system when visits were deleted.
void RemoveComplete();
@@ -128,10 +129,10 @@ class BrowsingHistoryHandler : public content::WebUIMessageHandler,
std::set<GURL> urls_to_be_deleted_;
// The info value that is returned to the front end with the query results.
- DictionaryValue results_info_value_;
+ base::DictionaryValue results_info_value_;
// The list of query results that is returned to the front end.
- ListValue results_value_;
+ base::ListValue results_value_;
// Timer used to implement a timeout on a Web History response.
base::OneShotTimer<BrowsingHistoryHandler> web_history_timer_;
diff --git a/chrome/test/data/webui/history_browsertest.js b/chrome/test/data/webui/history_browsertest.js
index c2dc308..815f27d 100644
--- a/chrome/test/data/webui/history_browsertest.js
+++ b/chrome/test/data/webui/history_browsertest.js
@@ -114,7 +114,7 @@ BaseHistoryWebUITest.prototype = {
*/
queryHistoryStub_: function(args) {
callFrontendAsync(
- 'historyResult', { term: args[0], finished: true, cursor: 0 }, []);
+ 'historyResult', { term: args[0], finished: true }, []);
},
};
@@ -156,19 +156,23 @@ HistoryWebUITest.prototype = {
var searchText = args[0];
var offset = args[1];
var range = args[2];
- var cursor = args[3];
+ var endTime = args[3] || Number.MAX_VALUE;
var maxCount = args[4];
- var resultCount = Math.min(maxCount, this.fakeHistory_.length - cursor);
- var results = this.fakeHistory_.slice(cursor, cursor + resultCount);
- var newCursor = cursor + results.length;
+ // Advance past all entries newer than the specified end time.
+ var i = 0;
+ while (this.fakeHistory_[i] && this.fakeHistory_[i].time >= endTime)
+ ++i;
+
+ var results = this.fakeHistory_.slice(i);
+ if (maxCount)
+ results = results.slice(0, maxCount);
callFrontendAsync(
'historyResult',
{
term: searchText,
- finished: newCursor >= this.fakeHistory_.length,
- cursor: newCursor,
+ finished: (this.fakeHistory_.length <= i + results.length)
},
results);
},