diff options
| -rw-r--r-- | chrome/browser/history/history_backend.cc | 10 | ||||
| -rw-r--r-- | chrome/browser/history/history_querying_unittest.cc | 82 | ||||
| -rw-r--r-- | chrome/browser/history/history_types.cc | 44 | ||||
| -rw-r--r-- | chrome/browser/history/history_types.h | 77 | ||||
| -rw-r--r-- | chrome/browser/history/text_database.cc | 60 | ||||
| -rw-r--r-- | chrome/browser/history/text_database.h | 3 | ||||
| -rw-r--r-- | chrome/browser/history/visit_database.cc | 42 | ||||
| -rw-r--r-- | chrome/browser/history/web_history_service.h | 2 | ||||
| -rw-r--r-- | chrome/browser/resources/history/history.js | 18 | ||||
| -rw-r--r-- | chrome/browser/ui/webui/history_ui.cc | 17 | ||||
| -rw-r--r-- | chrome/browser/ui/webui/history_ui.h | 11 | ||||
| -rw-r--r-- | chrome/test/data/webui/history_browsertest.js | 18 |
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, ×tamp) && - 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); }, |