diff options
Diffstat (limited to 'chrome/browser/history')
60 files changed, 2971 insertions, 772 deletions
diff --git a/chrome/browser/history/archived_database.cc b/chrome/browser/history/archived_database.cc index 1b9e010..72fa998 100644 --- a/chrome/browser/history/archived_database.cc +++ b/chrome/browser/history/archived_database.cc @@ -5,7 +5,6 @@ #include <algorithm> #include <string> -#include "app/sql/statement.h" #include "app/sql/transaction.h" #include "base/string_util.h" #include "chrome/browser/history/archived_database.h" @@ -14,7 +13,7 @@ namespace history { namespace { -static const int kCurrentVersionNumber = 2; +static const int kCurrentVersionNumber = 3; static const int kCompatibleVersionNumber = 2; } // namespace @@ -111,6 +110,12 @@ sql::InitStatus ArchivedDatabase::EnsureCurrentVersion() { std::min(cur_version, kCompatibleVersionNumber)); } + if (cur_version == 2) { + // This is the version prior to adding visit_source table. + ++cur_version; + meta_table_.SetVersionNumber(cur_version); + } + // Put future migration cases here. // When the version is too old, we just try to continue anyway, there should diff --git a/chrome/browser/history/archived_database.h b/chrome/browser/history/archived_database.h index c9d8757..9568aa5 100644 --- a/chrome/browser/history/archived_database.h +++ b/chrome/browser/history/archived_database.h @@ -4,6 +4,7 @@ #ifndef CHROME_BROWSER_HISTORY_ARCHIVED_DATABASE_H_ #define CHROME_BROWSER_HISTORY_ARCHIVED_DATABASE_H_ +#pragma once #include "app/sql/connection.h" #include "app/sql/init_status.h" diff --git a/chrome/browser/history/download_database.cc b/chrome/browser/history/download_database.cc index aa3dbde..f101e39 100644 --- a/chrome/browser/history/download_database.cc +++ b/chrome/browser/history/download_database.cc @@ -7,13 +7,12 @@ #include <limits> #include <vector> -#include "app/sql/connection.h" #include "app/sql/statement.h" #include "base/file_path.h" #include "base/utf_string_conversions.h" #include "build/build_config.h" #include "chrome/browser/download/download_item.h" -#include "chrome/browser/history/download_types.h" +#include "chrome/browser/history/download_create_info.h" // Download schema: // @@ -197,22 +196,4 @@ void DownloadDatabase::RemoveDownloadsBetween(base::Time delete_begin, statement.Run(); } -void DownloadDatabase::SearchDownloads(std::vector<int64>* results, - const string16& search_text) { - sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE, - "SELECT id FROM downloads WHERE url LIKE ? " - "OR full_path LIKE ? ORDER BY id")); - if (!statement) - return; - - std::string text("%"); - text.append(UTF16ToUTF8(search_text)); - text.push_back('%'); - statement.BindString(0, text); - statement.BindString(1, text); - - while (statement.Step()) - results->push_back(statement.ColumnInt64(0)); -} - } // namespace history diff --git a/chrome/browser/history/download_database.h b/chrome/browser/history/download_database.h index 11adf31..56f650a 100644 --- a/chrome/browser/history/download_database.h +++ b/chrome/browser/history/download_database.h @@ -4,6 +4,7 @@ #ifndef CHROME_BROWSER_HISTORY_DOWNLOAD_DATABASE_H_ #define CHROME_BROWSER_HISTORY_DOWNLOAD_DATABASE_H_ +#pragma once #include "chrome/browser/history/history_types.h" @@ -50,10 +51,6 @@ class DownloadDatabase { // all downloads that are in progress or are waiting to be cancelled. void RemoveDownloadsBetween(base::Time remove_begin, base::Time remove_end); - // Search for downloads matching the search text. - void SearchDownloads(std::vector<int64>* results, - const string16& search_text); - protected: // Returns the database for the functions in this interface. virtual sql::Connection& GetDB() = 0; diff --git a/chrome/browser/history/download_types.h b/chrome/browser/history/download_types.h deleted file mode 100644 index 642ac5b..0000000 --- a/chrome/browser/history/download_types.h +++ /dev/null @@ -1,104 +0,0 @@ -// Copyright (c) 2006-2008 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. -// -// Download creation struct used for querying the history service. - -#ifndef CHROME_BROWSER_HISTORY_DOWNLOAD_TYPES_H_ -#define CHROME_BROWSER_HISTORY_DOWNLOAD_TYPES_H_ - -#include <string> - -#include "base/basictypes.h" -#include "base/file_path.h" -#include "base/time.h" -#include "chrome/browser/download/download_file.h" -#include "googleurl/src/gurl.h" - -// Used for informing the download database of a new download, where we don't -// want to pass DownloadItems between threads. The history service also uses a -// vector of these structs for passing us the state of all downloads at -// initialization time (see DownloadQueryInfo below). -struct DownloadCreateInfo { - DownloadCreateInfo(const FilePath& path, - const GURL& url, - base::Time start_time, - int64 received_bytes, - int64 total_bytes, - int32 state, - int32 download_id) - : path(path), - url(url), - path_uniquifier(0), - start_time(start_time), - received_bytes(received_bytes), - total_bytes(total_bytes), - state(state), - download_id(download_id), - child_id(-1), - render_view_id(-1), - request_id(-1), - db_handle(0), - prompt_user_for_save_location(false), - is_dangerous(false), - is_extension_install(false) { - } - - DownloadCreateInfo() - : path_uniquifier(0), - received_bytes(0), - total_bytes(0), - state(-1), - download_id(-1), - child_id(-1), - render_view_id(-1), - request_id(-1), - db_handle(0), - prompt_user_for_save_location(false), - is_dangerous(false), - is_extension_install(false) { - } - - // DownloadItem fields - FilePath path; - GURL url; - GURL referrer_url; - FilePath suggested_path; - // A number that should be added to the suggested path to make it unique. - // 0 means no number should be appended. Not actually stored in the db. - int path_uniquifier; - base::Time start_time; - int64 received_bytes; - int64 total_bytes; - int32 state; - int32 download_id; - int child_id; - int render_view_id; - int request_id; - int64 db_handle; - std::string content_disposition; - std::string mime_type; - // The value of the content type header sent with the downloaded item. It - // may be different from |mime_type|, which may be set based on heuristics - // which may look at the file extension and first few bytes of the file. - std::string original_mime_type; - - // True if we should display the 'save as...' UI and prompt the user - // for the download location. - // False if the UI should be supressed and the download performed to the - // default location. - bool prompt_user_for_save_location; - // Whether this download is potentially dangerous (ex: exe, dll, ...). - bool is_dangerous; - // The original name for a dangerous download. - FilePath original_name; - // Whether this download is for extension install or not. - bool is_extension_install; - // The charset of the referring page where the download request comes from. - // It's used to construct a suggested filename. - std::string referrer_charset; - // The download file save info. - DownloadSaveInfo save_info; -}; - -#endif // CHROME_BROWSER_HISTORY_DOWNLOAD_TYPES_H_ diff --git a/chrome/browser/history/expire_history_backend.cc b/chrome/browser/history/expire_history_backend.cc index bd471ad..294c0e6 100644 --- a/chrome/browser/history/expire_history_backend.cc +++ b/chrome/browser/history/expire_history_backend.cc @@ -512,7 +512,7 @@ void ExpireHistoryBackend::ExpireURLsForVisits( void ExpireHistoryBackend::ArchiveURLsAndVisits( const VisitVector& visits, DeleteDependencies* dependencies) { - if (!archived_db_) + if (!archived_db_ || !main_db_) return; // Make sure all unique URL rows are added to the dependency list and the @@ -539,6 +539,12 @@ void ExpireHistoryBackend::ArchiveURLsAndVisits( } } + // Retrieve the sources for all the archived visits before archiving. + // The returned visit_sources vector should contain the source for each visit + // from visits at the same index. + VisitSourceMap visit_sources; + main_db_->GetVisitsSource(visits, &visit_sources); + // Now archive the visits since we know the URL ID to make them reference. // The source visit list should still reference the visits in the main DB, but // we will update it to reflect only the visits that were successfully @@ -550,7 +556,9 @@ void ExpireHistoryBackend::ArchiveURLsAndVisits( VisitRow cur_visit(visits[i]); cur_visit.url_id = main_id_to_archived_id[cur_visit.url_id]; cur_visit.referring_visit = 0; - archived_db_->AddVisit(&cur_visit); + VisitSourceMap::iterator iter = visit_sources.find(visits[i].visit_id); + archived_db_->AddVisit(&cur_visit, + iter == visit_sources.end() ? SOURCE_BROWSED : iter->second); // Ignore failures, we will delete it from the main DB no matter what. } } diff --git a/chrome/browser/history/expire_history_backend.h b/chrome/browser/history/expire_history_backend.h index 9f060ed..96e2e9a 100644 --- a/chrome/browser/history/expire_history_backend.h +++ b/chrome/browser/history/expire_history_backend.h @@ -4,6 +4,7 @@ #ifndef CHROME_BROWSER_HISTORY_EXPIRE_HISTORY_BACKEND_H_ #define CHROME_BROWSER_HISTORY_EXPIRE_HISTORY_BACKEND_H_ +#pragma once #include <queue> #include <set> @@ -105,6 +106,7 @@ class ExpireHistoryBackend { FRIEND_TEST_ALL_PREFIXES(ExpireHistoryTest, DeleteFaviconsIfPossible); FRIEND_TEST_ALL_PREFIXES(ExpireHistoryTest, ArchiveSomeOldHistory); FRIEND_TEST_ALL_PREFIXES(ExpireHistoryTest, ExpiringVisitsReader); + FRIEND_TEST_ALL_PREFIXES(ExpireHistoryTest, ArchiveSomeOldHistoryWithSource); friend class ::TestingProfile; struct DeleteDependencies; diff --git a/chrome/browser/history/expire_history_backend_unittest.cc b/chrome/browser/history/expire_history_backend_unittest.cc index ca822bc..91129ed 100644 --- a/chrome/browser/history/expire_history_backend_unittest.cc +++ b/chrome/browser/history/expire_history_backend_unittest.cc @@ -8,6 +8,7 @@ #include "base/file_util.h" #include "base/path_service.h" #include "base/scoped_ptr.h" +#include "base/string16.h" #include "base/utf_string_conversions.h" #include "chrome/browser/bookmarks/bookmark_model.h" #include "chrome/browser/history/archived_database.h" @@ -16,8 +17,10 @@ #include "chrome/browser/history/history_notifications.h" #include "chrome/browser/history/text_database_manager.h" #include "chrome/browser/history/thumbnail_database.h" +#include "chrome/browser/history/top_sites.h" #include "chrome/common/notification_service.h" #include "chrome/common/thumbnail_score.h" +#include "chrome/test/testing_profile.h" #include "chrome/tools/profiles/thumbnail-inl.h" #include "gfx/codec/jpeg_codec.h" #include "testing/gtest/include/gtest/gtest.h" @@ -53,6 +56,8 @@ class ExpireHistoryTest : public testing::Test, protected: // Called by individual tests when they want data populated. void AddExampleData(URLID url_ids[3], Time visit_times[4]); + // Add visits with source information. + void AddExampleSourceData(const GURL& url, URLID* id); // Returns true if the given favicon/thumanil has an entry in the DB. bool HasFavIcon(FavIconID favicon_id); @@ -75,7 +80,7 @@ class ExpireHistoryTest : public testing::Test, void StarURL(const GURL& url) { bookmark_model_.AddURL( - bookmark_model_.GetBookmarkBarNode(), 0, std::wstring(), url); + bookmark_model_.GetBookmarkBarNode(), 0, string16(), url); } static bool IsStringInFile(const FilePath& filename, const char* str); @@ -90,6 +95,8 @@ class ExpireHistoryTest : public testing::Test, scoped_ptr<ArchivedDatabase> archived_db_; scoped_ptr<ThumbnailDatabase> thumb_db_; scoped_ptr<TextDatabaseManager> text_db_; + TestingProfile profile_; + scoped_refptr<TopSites> top_sites_; // Time at the beginning of the test, so everybody agrees what "now" is. const Time now_; @@ -134,6 +141,7 @@ class ExpireHistoryTest : public testing::Test, expirer_.SetDatabases(main_db_.get(), archived_db_.get(), thumb_db_.get(), text_db_.get()); + top_sites_ = profile_.GetTopSites(); } void TearDown() { @@ -145,6 +153,7 @@ class ExpireHistoryTest : public testing::Test, archived_db_.reset(); thumb_db_.reset(); text_db_.reset(); + TopSites::DeleteTopSites(top_sites_); file_util::Delete(dir_, true); } @@ -211,35 +220,35 @@ void ExpireHistoryTest::AddExampleData(URLID url_ids[3], Time visit_times[4]) { Time time; GURL gurl; - thumb_db_->SetPageThumbnail(gurl, url_ids[0], *thumbnail, score, time); - thumb_db_->SetPageThumbnail(gurl, url_ids[1], *thumbnail, score, time); - thumb_db_->SetPageThumbnail(gurl, url_ids[2], *thumbnail, score, time); + top_sites_->SetPageThumbnail(url_row1.url(), *thumbnail, score); + top_sites_->SetPageThumbnail(url_row2.url(), *thumbnail, score); + top_sites_->SetPageThumbnail(url_row3.url(), *thumbnail, score); // Four visits. VisitRow visit_row1; visit_row1.url_id = url_ids[0]; visit_row1.visit_time = visit_times[0]; visit_row1.is_indexed = true; - main_db_->AddVisit(&visit_row1); + main_db_->AddVisit(&visit_row1, SOURCE_BROWSED); VisitRow visit_row2; visit_row2.url_id = url_ids[1]; visit_row2.visit_time = visit_times[1]; visit_row2.is_indexed = true; - main_db_->AddVisit(&visit_row2); + main_db_->AddVisit(&visit_row2, SOURCE_BROWSED); VisitRow visit_row3; visit_row3.url_id = url_ids[1]; visit_row3.visit_time = visit_times[2]; visit_row3.is_indexed = true; visit_row3.transition = PageTransition::TYPED; - main_db_->AddVisit(&visit_row3); + main_db_->AddVisit(&visit_row3, SOURCE_BROWSED); VisitRow visit_row4; visit_row4.url_id = url_ids[2]; visit_row4.visit_time = visit_times[3]; visit_row4.is_indexed = true; - main_db_->AddVisit(&visit_row4); + main_db_->AddVisit(&visit_row4, SOURCE_BROWSED); // Full text index for each visit. text_db_->AddPageData(url_row1.url(), visit_row1.url_id, visit_row1.visit_id, @@ -260,6 +269,35 @@ void ExpireHistoryTest::AddExampleData(URLID url_ids[3], Time visit_times[4]) { UTF8ToUTF16("goats body")); } +void ExpireHistoryTest::AddExampleSourceData(const GURL& url, URLID* id) { + if (!main_db_.get()) + return; + + Time last_visit_time = Time::Now(); + // Add one URL. + URLRow url_row1(url); + url_row1.set_last_visit(last_visit_time); + url_row1.set_visit_count(4); + URLID url_id = main_db_->AddURL(url_row1); + *id = url_id; + + // Four times for each visit. + VisitRow visit_row1(url_id, last_visit_time - TimeDelta::FromDays(4), 0, + PageTransition::TYPED, 0); + main_db_->AddVisit(&visit_row1, SOURCE_SYNCED); + + VisitRow visit_row2(url_id, last_visit_time - TimeDelta::FromDays(3), 0, + PageTransition::TYPED, 0); + main_db_->AddVisit(&visit_row2, SOURCE_BROWSED); + + VisitRow visit_row3(url_id, last_visit_time - TimeDelta::FromDays(2), 0, + PageTransition::TYPED, 0); + main_db_->AddVisit(&visit_row3, SOURCE_EXTENSION); + + VisitRow visit_row4(url_id, last_visit_time, 0, PageTransition::TYPED, 0); + main_db_->AddVisit(&visit_row4, SOURCE_FIREFOX_IMPORTED); +} + bool ExpireHistoryTest::HasFavIcon(FavIconID favicon_id) { if (!thumb_db_.get()) return false; @@ -271,8 +309,12 @@ bool ExpireHistoryTest::HasFavIcon(FavIconID favicon_id) { } bool ExpireHistoryTest::HasThumbnail(URLID url_id) { - std::vector<unsigned char> temp_data; - return thumb_db_->GetPageThumbnail(url_id, &temp_data); + URLRow info; + if (!main_db_->GetURLRow(url_id, &info)) + return false; + GURL url = info.url(); + RefCountedBytes *data; + return top_sites_->GetPageThumbnail(url, &data); } int ExpireHistoryTest::CountTextMatchesForURL(const GURL& url) { @@ -507,7 +549,7 @@ TEST_F(ExpireHistoryTest, DontDeleteStarredURL) { ASSERT_TRUE(HasThumbnail(url_row.id())); // Unstar the URL and delete again. - bookmark_model_.SetURLStarred(url, std::wstring(), false); + bookmark_model_.SetURLStarred(url, string16(), false); expirer_.DeleteURL(url); // Now it should be completely deleted. @@ -804,8 +846,52 @@ TEST_F(ExpireHistoryTest, ExpiringVisitsReader) { EXPECT_EQ(1U, visits.size()); } +// Tests how ArchiveSomeOldHistory treats source information. +TEST_F(ExpireHistoryTest, ArchiveSomeOldHistoryWithSource) { + const GURL url("www.testsource.com"); + URLID url_id; + AddExampleSourceData(url, &url_id); + const ExpiringVisitsReader* reader = expirer_.GetAllVisitsReader(); + + // Archiving all the visits we added. + ASSERT_FALSE(expirer_.ArchiveSomeOldHistory(Time::Now(), reader, 10)); + + URLRow archived_row; + ASSERT_TRUE(archived_db_->GetRowForURL(url, &archived_row)); + VisitVector archived_visits; + archived_db_->GetVisitsForURL(archived_row.id(), &archived_visits); + ASSERT_EQ(4U, archived_visits.size()); + VisitSourceMap sources; + archived_db_->GetVisitsSource(archived_visits, &sources); + ASSERT_EQ(3U, sources.size()); + int result = 0; + VisitSourceMap::iterator iter; + for (int i = 0; i < 4; i++) { + iter = sources.find(archived_visits[i].visit_id); + if (iter == sources.end()) + continue; + switch (iter->second) { + case history::SOURCE_EXTENSION: + result |= 0x1; + break; + case history::SOURCE_FIREFOX_IMPORTED: + result |= 0x2; + break; + case history::SOURCE_SYNCED: + result |= 0x4; + default: + break; + } + } + EXPECT_EQ(0x7, result); + main_db_->GetVisitsSource(archived_visits, &sources); + EXPECT_EQ(0U, sources.size()); + main_db_->GetVisitsForURL(url_id, &archived_visits); + EXPECT_EQ(0U, archived_visits.size()); +} + // TODO(brettw) add some visits with no URL to make sure everything is updated -// properly. Have the visits also refer to nonexistant FTS rows. +// properly. Have the visits also refer to nonexistent FTS rows. // // Maybe also refer to invalid favicons. diff --git a/chrome/browser/history/history.cc b/chrome/browser/history/history.cc index 0b92b6f..d70fd85 100644 --- a/chrome/browser/history/history.cc +++ b/chrome/browser/history/history.cc @@ -24,18 +24,18 @@ #include "chrome/browser/history/history.h" -#include "app/l10n_util.h" #include "base/callback.h" #include "base/message_loop.h" #include "base/path_service.h" #include "base/ref_counted.h" +#include "base/string_util.h" #include "base/task.h" #include "chrome/browser/autocomplete/history_url_provider.h" #include "chrome/browser/browser_list.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/browser_window.h" #include "chrome/browser/chrome_thread.h" -#include "chrome/browser/history/download_types.h" +#include "chrome/browser/history/download_create_info.h" #include "chrome/browser/history/history_backend.h" #include "chrome/browser/history/history_notifications.h" #include "chrome/browser/history/history_types.h" @@ -222,6 +222,16 @@ history::URLDatabase* HistoryService::InMemoryDatabase() { return NULL; } +history::InMemoryURLIndex* HistoryService::InMemoryIndex() { + // NOTE: See comments in BackendLoaded() as to why we call + // LoadBackendIfNecessary() here even though it won't affect the return value + // for this call. + LoadBackendIfNecessary(); + if (in_memory_backend_.get()) + return in_memory_backend_->index(); + return NULL; +} + void HistoryService::SetSegmentPresentationIndex(int64 segment_id, int index) { ScheduleAndForget(PRIORITY_UI, &HistoryBackend::SetSegmentPresentationIndex, @@ -229,7 +239,7 @@ void HistoryService::SetSegmentPresentationIndex(int64 segment_id, int index) { } void HistoryService::SetKeywordSearchTermsForURL(const GURL& url, - TemplateURL::IDType keyword_id, + TemplateURLID keyword_id, const string16& term) { ScheduleAndForget(PRIORITY_UI, &HistoryBackend::SetKeywordSearchTermsForURL, @@ -237,14 +247,14 @@ void HistoryService::SetKeywordSearchTermsForURL(const GURL& url, } void HistoryService::DeleteAllSearchTermsForKeyword( - TemplateURL::IDType keyword_id) { + TemplateURLID keyword_id) { ScheduleAndForget(PRIORITY_UI, &HistoryBackend::DeleteAllSearchTermsForKeyword, keyword_id); } HistoryService::Handle HistoryService::GetMostRecentKeywordSearchTerms( - TemplateURL::IDType keyword_id, + TemplateURLID keyword_id, const string16& prefix, int max_count, CancelableRequestConsumerBase* consumer, @@ -291,9 +301,10 @@ void HistoryService::AddPage(const GURL& url, const GURL& referrer, PageTransition::Type transition, const history::RedirectList& redirects, + history::VisitSource visit_source, bool did_replace_entry) { AddPage(url, Time::Now(), id_scope, page_id, referrer, transition, redirects, - did_replace_entry); + visit_source, did_replace_entry); } void HistoryService::AddPage(const GURL& url, @@ -303,37 +314,45 @@ void HistoryService::AddPage(const GURL& url, const GURL& referrer, PageTransition::Type transition, const history::RedirectList& redirects, + history::VisitSource visit_source, bool did_replace_entry) { + scoped_refptr<history::HistoryAddPageArgs> request( + new history::HistoryAddPageArgs(url, time, id_scope, page_id, referrer, + redirects, transition, visit_source, + did_replace_entry)); + AddPage(*request); +} + +void HistoryService::AddPage(const history::HistoryAddPageArgs& add_page_args) { DCHECK(thread_) << "History service being called after cleanup"; // Filter out unwanted URLs. We don't add auto-subframe URLs. They are a // large part of history (think iframes for ads) and we never display them in // history UI. We will still add manual subframes, which are ones the user // has clicked on to get. - if (!CanAddURL(url)) + if (!CanAddURL(add_page_args.url)) return; // Add link & all redirects to visited link list. VisitedLinkMaster* visited_links; if (profile_ && (visited_links = profile_->GetVisitedLinkMaster())) { - visited_links->AddURL(url); + visited_links->AddURL(add_page_args.url); - if (!redirects.empty()) { + if (!add_page_args.redirects.empty()) { // We should not be asked to add a page in the middle of a redirect chain. - DCHECK(redirects[redirects.size() - 1] == url); + DCHECK_EQ(add_page_args.url, + add_page_args.redirects[add_page_args.redirects.size() - 1]); // We need the !redirects.empty() condition above since size_t is unsigned // and will wrap around when we subtract one from a 0 size. - for (size_t i = 0; i < redirects.size() - 1; i++) - visited_links->AddURL(redirects[i]); + for (size_t i = 0; i < add_page_args.redirects.size() - 1; i++) + visited_links->AddURL(add_page_args.redirects[i]); } } - scoped_refptr<history::HistoryAddPageArgs> request( - new history::HistoryAddPageArgs(url, time, id_scope, page_id, - referrer, redirects, transition, - did_replace_entry)); - ScheduleAndForget(PRIORITY_NORMAL, &HistoryBackend::AddPage, request); + ScheduleAndForget(PRIORITY_NORMAL, &HistoryBackend::AddPage, + scoped_refptr<history::HistoryAddPageArgs>( + add_page_args.Clone())); } void HistoryService::SetPageTitle(const GURL& url, @@ -346,7 +365,8 @@ void HistoryService::AddPageWithDetails(const GURL& url, int visit_count, int typed_count, Time last_visit, - bool hidden) { + bool hidden, + history::VisitSource visit_source) { // Filter out unwanted URLs. if (!CanAddURL(url)) return; @@ -367,11 +387,12 @@ void HistoryService::AddPageWithDetails(const GURL& url, rows.push_back(row); ScheduleAndForget(PRIORITY_NORMAL, - &HistoryBackend::AddPagesWithDetails, rows); + &HistoryBackend::AddPagesWithDetails, rows, visit_source); } void HistoryService::AddPagesWithDetails( - const std::vector<history::URLRow>& info) { + const std::vector<history::URLRow>& info, + history::VisitSource visit_source) { // Add to the visited links system. VisitedLinkMaster* visited_links; @@ -387,7 +408,7 @@ void HistoryService::AddPagesWithDetails( } ScheduleAndForget(PRIORITY_NORMAL, - &HistoryBackend::AddPagesWithDetails, info); + &HistoryBackend::AddPagesWithDetails, info, visit_source); } void HistoryService::SetPageContents(const GURL& url, @@ -529,14 +550,6 @@ void HistoryService::RemoveDownloadsBetween(Time remove_begin, remove_end); } -HistoryService::Handle HistoryService::SearchDownloads( - const string16& search_text, - CancelableRequestConsumerBase* consumer, - DownloadSearchCallback* callback) { - return Schedule(PRIORITY_NORMAL, &HistoryBackend::SearchDownloads, consumer, - new history::DownloadSearchRequest(callback), search_text); -} - HistoryService::Handle HistoryService::QueryHistory( const string16& text_query, const history::QueryOptions& options, @@ -744,7 +757,6 @@ void HistoryService::LoadBackendIfNecessary() { } void HistoryService::OnDBLoaded() { - LOG(INFO) << "History backend finished loading"; backend_loaded_ = true; NotificationService::current()->Notify(NotificationType::HISTORY_LOADED, Source<Profile>(profile_), diff --git a/chrome/browser/history/history.h b/chrome/browser/history/history.h index 9548f65..3d24981 100644 --- a/chrome/browser/history/history.h +++ b/chrome/browser/history/history.h @@ -4,8 +4,8 @@ #ifndef CHROME_BROWSER_HISTORY_HISTORY_H_ #define CHROME_BROWSER_HISTORY_HISTORY_H_ +#pragma once -#include <string> #include <vector> #include "base/basictypes.h" @@ -18,7 +18,7 @@ #include "chrome/browser/cancelable_request.h" #include "chrome/browser/favicon_service.h" #include "chrome/browser/history/history_types.h" -#include "chrome/browser/search_engines/template_url.h" +#include "chrome/browser/search_engines/template_url_id.h" #include "chrome/common/notification_registrar.h" #include "chrome/common/page_transition_types.h" #include "chrome/common/ref_counted_util.h" @@ -48,14 +48,14 @@ class TypedUrlDataTypeController; } namespace history { - class InMemoryHistoryBackend; +class InMemoryURLIndex; +class HistoryAddPageArgs; class HistoryBackend; class HistoryDatabase; struct HistoryDetails; class HistoryQueryTest; class URLDatabase; - } // namespace history @@ -150,6 +150,8 @@ class HistoryService : public CancelableRequestProvider, // TODO(brettw) this should return the InMemoryHistoryBackend. history::URLDatabase* InMemoryDatabase(); + history::InMemoryURLIndex* InMemoryIndex(); + // Navigation ---------------------------------------------------------------- // Adds the given canonical URL to history with the current time as the visit @@ -182,6 +184,7 @@ class HistoryService : public CancelableRequestProvider, const GURL& referrer, PageTransition::Type transition, const history::RedirectList& redirects, + history::VisitSource visit_source, bool did_replace_entry); // For adding pages to history with a specific time. This is for testing @@ -193,14 +196,18 @@ class HistoryService : public CancelableRequestProvider, const GURL& referrer, PageTransition::Type transition, const history::RedirectList& redirects, + history::VisitSource visit_source, bool did_replace_entry); // For adding pages to history where no tracking information can be done. - void AddPage(const GURL& url) { - AddPage(url, NULL, 0, GURL(), PageTransition::LINK, history::RedirectList(), - false); + void AddPage(const GURL& url, history::VisitSource visit_source) { + AddPage(url, NULL, 0, GURL(), PageTransition::LINK, + history::RedirectList(), visit_source, false); } + // All AddPage variants end up here. + void AddPage(const history::HistoryAddPageArgs& add_page_args); + // Sets the title for the given page. The page should be in history. If it // is not, this operation is ignored. This call will not update the full // text index. The last title set when the page is indexed will be the @@ -442,15 +449,6 @@ class HistoryService : public CancelableRequestProvider, // both directions. void RemoveDownloadsBetween(base::Time remove_begin, base::Time remove_end); - // Implemented by the caller of 'SearchDownloads' below, and is called when - // the history system has retrieved the search results. - typedef Callback2<Handle, std::vector<int64>*>::Type DownloadSearchCallback; - - // Search for downloads that match the search text. - Handle SearchDownloads(const string16& search_text, - CancelableRequestConsumerBase* consumer, - DownloadSearchCallback* callback); - // Visit Segments ------------------------------------------------------------ typedef Callback2<Handle, std::vector<PageUsageData*>*>::Type @@ -482,11 +480,11 @@ class HistoryService : public CancelableRequestProvider, // Sets the search terms for the specified url and keyword. url_id gives the // id of the url, keyword_id the id of the keyword and term the search term. void SetKeywordSearchTermsForURL(const GURL& url, - TemplateURL::IDType keyword_id, + TemplateURLID keyword_id, const string16& term); // Deletes all search terms for the specified keyword. - void DeleteAllSearchTermsForKeyword(TemplateURL::IDType keyword_id); + void DeleteAllSearchTermsForKeyword(TemplateURLID keyword_id); typedef Callback2<Handle, std::vector<history::KeywordSearchTermVisit>*>::Type GetMostRecentKeywordSearchTermsCallback; @@ -496,7 +494,7 @@ class HistoryService : public CancelableRequestProvider, // in descending order up to |max_count| with the most recent search term // first. Handle GetMostRecentKeywordSearchTerms( - TemplateURL::IDType keyword_id, + TemplateURLID keyword_id, const string16& prefix, int max_count, CancelableRequestConsumerBase* consumer, @@ -541,10 +539,12 @@ class HistoryService : public CancelableRequestProvider, int visit_count, int typed_count, base::Time last_visit, - bool hidden); + bool hidden, + history::VisitSource visit_source); // The same as AddPageWithDetails() but takes a vector. - void AddPagesWithDetails(const std::vector<history::URLRow>& info); + void AddPagesWithDetails(const std::vector<history::URLRow>& info, + history::VisitSource visit_source); // Starts the TopSites migration in the HistoryThread. Called by the // BackendDelegate. diff --git a/chrome/browser/history/history_backend.cc b/chrome/browser/history/history_backend.cc index 0f512db..237b14b 100644 --- a/chrome/browser/history/history_backend.cc +++ b/chrome/browser/history/history_backend.cc @@ -17,11 +17,12 @@ #include "base/time.h" #include "chrome/browser/autocomplete/history_url_provider.h" #include "chrome/browser/bookmarks/bookmark_service.h" -#include "chrome/browser/history/download_types.h" +#include "chrome/browser/history/download_create_info.h" #include "chrome/browser/history/history_notifications.h" #include "chrome/browser/history/history_publisher.h" #include "chrome/browser/history/in_memory_history_backend.h" #include "chrome/browser/history/page_usage_data.h" +#include "chrome/browser/history/top_sites.h" #include "chrome/common/chrome_constants.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/notification_type.h" @@ -362,8 +363,6 @@ SegmentID HistoryBackend::UpdateSegments(const GURL& url, } void HistoryBackend::AddPage(scoped_refptr<HistoryAddPageArgs> request) { - DLOG(INFO) << "Adding page " << request->url.possibly_invalid_spec(); - if (!db_.get()) return; @@ -405,7 +404,7 @@ void HistoryBackend::AddPage(scoped_refptr<HistoryAddPageArgs> request) { // No redirect case (one element means just the page itself). last_ids = AddPageVisit(request->url, last_recorded_time_, - last_ids.second, t); + last_ids.second, t, request->visit_source); // Update the segment for this visit. KEYWORD_GENERATED visits should not // result in changing most visited, so we don't update segments (most @@ -471,7 +470,8 @@ void HistoryBackend::AddPage(scoped_refptr<HistoryAddPageArgs> request) { // them anyway, and if we ever decide to, we can reconstruct their order // from the redirect chain. last_ids = AddPageVisit(request->redirects[redirect_index], - last_recorded_time_, last_ids.second, t); + last_recorded_time_, last_ids.second, + t, request->visit_source); if (t & PageTransition::CHAIN_START) { // Update the segment for this visit. UpdateSegments(request->redirects[redirect_index], @@ -548,7 +548,7 @@ void HistoryBackend::InitImpl() { // Fill the in-memory database and send it back to the history service on the // main thread. InMemoryHistoryBackend* mem_backend = new InMemoryHistoryBackend; - if (mem_backend->Init(history_name)) + if (mem_backend->Init(history_name, db_.get())) delegate_->SetInMemoryBackend(mem_backend); // Takes ownership of pointer. else delete mem_backend; // Error case, run without the in-memory DB. @@ -581,11 +581,12 @@ void HistoryBackend::InitImpl() { // Thumbnail database. thumbnail_db_.reset(new ThumbnailDatabase()); - if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kTopSites)) { - if (!db_->needs_version_18_migration()) { - // No convertion needed - use new filename right away. - thumbnail_name = GetFaviconsFileName(); - } + if (history::TopSites::IsEnabled()) { + // TODO(sky): once we reenable top sites this needs to be fixed. + // if (!db_->needs_version_18_migration()) { + // No convertion needed - use new filename right away. + // thumbnail_name = GetFaviconsFileName(); + // } } if (thumbnail_db_->Init(thumbnail_name, history_publisher_.get()) != sql::INIT_OK) { @@ -598,11 +599,12 @@ void HistoryBackend::InitImpl() { thumbnail_db_.reset(); } - if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kTopSites)) { - if (db_->needs_version_18_migration()) { - LOG(INFO) << "Starting TopSites migration"; - delegate_->StartTopSitesMigration(); - } + if (history::TopSites::IsEnabled()) { + // TODO(sky): fix when reenabling top sites migration. + // if (db_->needs_version_18_migration()) { + // LOG(INFO) << "Starting TopSites migration"; + // delegate_->StartTopSitesMigration(); + // } } // Archived database. @@ -651,7 +653,8 @@ std::pair<URLID, VisitID> HistoryBackend::AddPageVisit( const GURL& url, Time time, VisitID referring_visit, - PageTransition::Type transition) { + PageTransition::Type transition, + VisitSource visit_source) { // Top-level frame navigations are visible, everything else is hidden bool new_hidden = !PageTransition::IsMainFrame(transition); @@ -708,7 +711,7 @@ std::pair<URLID, VisitID> HistoryBackend::AddPageVisit( // Add the visit with the time to the database. VisitRow visit_info(url_id, time, referring_visit, transition, 0); - VisitID visit_id = db_->AddVisit(&visit_info); + VisitID visit_id = db_->AddVisit(&visit_info, visit_source); if (visit_info.visit_time < first_recorded_time_) first_recorded_time_ = visit_info.visit_time; @@ -727,7 +730,8 @@ std::pair<URLID, VisitID> HistoryBackend::AddPageVisit( return std::make_pair(url_id, visit_id); } -void HistoryBackend::AddPagesWithDetails(const std::vector<URLRow>& urls) { +void HistoryBackend::AddPagesWithDetails(const std::vector<URLRow>& urls, + VisitSource visit_source) { if (!db_.get()) return; @@ -789,7 +793,7 @@ void HistoryBackend::AddPagesWithDetails(const std::vector<URLRow>& urls) { PageTransition::LINK | PageTransition::CHAIN_START | PageTransition::CHAIN_END, 0); visit_info.is_indexed = has_indexed; - if (!visit_database->AddVisit(&visit_info)) { + if (!visit_database->AddVisit(&visit_info, visit_source)) { NOTREACHED() << "Adding visit failed."; return; } @@ -906,11 +910,12 @@ bool HistoryBackend::UpdateURL(URLID id, const history::URLRow& url) { } bool HistoryBackend::AddVisits(const GURL& url, - const std::vector<base::Time>& visits) { + const std::vector<base::Time>& visits, + VisitSource visit_source) { if (db_.get()) { for (std::vector<base::Time>::const_iterator visit = visits.begin(); visit != visits.end(); ++visit) { - if (!AddPageVisit(url, *visit, 0, 0).first) { + if (!AddPageVisit(url, *visit, 0, 0, visit_source).first) { return false; } } @@ -1022,7 +1027,7 @@ void HistoryBackend::QuerySegmentUsage( // Keyword visits -------------------------------------------------------------- void HistoryBackend::SetKeywordSearchTermsForURL(const GURL& url, - TemplateURL::IDType keyword_id, + TemplateURLID keyword_id, const string16& term) { if (!db_.get()) return; @@ -1040,7 +1045,7 @@ void HistoryBackend::SetKeywordSearchTermsForURL(const GURL& url, } void HistoryBackend::DeleteAllSearchTermsForKeyword( - TemplateURL::IDType keyword_id) { + TemplateURLID keyword_id) { if (!db_.get()) return; @@ -1051,7 +1056,7 @@ void HistoryBackend::DeleteAllSearchTermsForKeyword( void HistoryBackend::GetMostRecentKeywordSearchTerms( scoped_refptr<GetMostRecentKeywordSearchTermsRequest> request, - TemplateURL::IDType keyword_id, + TemplateURLID keyword_id, const string16& prefix, int max_count) { if (request->canceled()) @@ -1126,17 +1131,6 @@ void HistoryBackend::RemoveDownloadsBetween(const Time remove_begin, db_->RemoveDownloadsBetween(remove_begin, remove_end); } -void HistoryBackend::SearchDownloads( - scoped_refptr<DownloadSearchRequest> request, - const string16& search_text) { - if (request->canceled()) - return; - if (db_.get()) - db_->SearchDownloads(&request->value, search_text); - request->ForwardResult(DownloadSearchRequest::TupleType(request->handle(), - &request->value)); -} - void HistoryBackend::QueryHistory(scoped_refptr<QueryHistoryRequest> request, const string16& text_query, const QueryOptions& options) { @@ -2156,9 +2150,16 @@ BookmarkService* HistoryBackend::GetBookmarkService() { } void HistoryBackend::MigrateThumbnailsDatabase() { - thumbnail_db_->RenameAndDropThumbnails(GetThumbnailFileName(), - GetFaviconsFileName()); - db_->MigrationToTopSitesDone(); + // If there is no History DB, we can't record that the migration was done. + // It will be recorded on the next run. + if (db_.get()) { + // If there is no thumbnail DB, we can still record a successful migration. + if (thumbnail_db_.get()) { + thumbnail_db_->RenameAndDropThumbnails(GetThumbnailFileName(), + GetFaviconsFileName()); + } + db_->MigrationToTopSitesDone(); + } } } // namespace history diff --git a/chrome/browser/history/history_backend.h b/chrome/browser/history/history_backend.h index cbda2e9..f3e76cf 100644 --- a/chrome/browser/history/history_backend.h +++ b/chrome/browser/history/history_backend.h @@ -4,6 +4,7 @@ #ifndef CHROME_BROWSER_HISTORY_HISTORY_BACKEND_H_ #define CHROME_BROWSER_HISTORY_HISTORY_BACKEND_H_ +#pragma once #include <utility> @@ -11,7 +12,6 @@ #include "base/gtest_prod_util.h" #include "base/scoped_ptr.h" #include "chrome/browser/history/archived_database.h" -#include "chrome/browser/history/download_types.h" #include "chrome/browser/history/expire_history_backend.h" #include "chrome/browser/history/history_database.h" #include "chrome/browser/history/history_marshaling.h" @@ -19,9 +19,11 @@ #include "chrome/browser/history/text_database_manager.h" #include "chrome/browser/history/thumbnail_database.h" #include "chrome/browser/history/visit_tracker.h" +#include "chrome/browser/search_engines/template_url_id.h" #include "chrome/common/mru_cache.h" class BookmarkService; +struct DownloadCreateInfo; class TestingProfile; struct ThumbnailScore; @@ -224,8 +226,6 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, void RemoveDownloadsBetween(const base::Time remove_begin, const base::Time remove_end); void RemoveDownloads(const base::Time remove_end); - void SearchDownloads(scoped_refptr<DownloadSearchRequest>, - const string16& search_text); // Segment usage ------------------------------------------------------------- @@ -238,14 +238,14 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, // Keyword search terms ------------------------------------------------------ void SetKeywordSearchTermsForURL(const GURL& url, - TemplateURL::IDType keyword_id, + TemplateURLID keyword_id, const string16& term); - void DeleteAllSearchTermsForKeyword(TemplateURL::IDType keyword_id); + void DeleteAllSearchTermsForKeyword(TemplateURLID keyword_id); void GetMostRecentKeywordSearchTerms( scoped_refptr<GetMostRecentKeywordSearchTermsRequest> request, - TemplateURL::IDType keyword_id, + TemplateURLID keyword_id, const string16& prefix, int max_count); @@ -259,8 +259,10 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, virtual bool UpdateURL(URLID id, const history::URLRow& url); + // While adding visits in batch, the source needs to be provided. virtual bool AddVisits(const GURL& url, - const std::vector<base::Time>& visits); + const std::vector<base::Time>& visits, + VisitSource visit_source); virtual bool RemoveVisits(const VisitVector& visits); @@ -293,7 +295,9 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, // Adds the given rows to the database if it doesn't exist. A visit will be // added for each given URL at the last visit time in the URLRow. - void AddPagesWithDetails(const std::vector<URLRow>& info); + // Each visit will have the visit_source type set. + void AddPagesWithDetails(const std::vector<URLRow>& info, + VisitSource visit_source); #if defined(UNIT_TEST) HistoryDatabase* db() const { return db_.get(); } @@ -313,6 +317,11 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, URLsNoLongerBookmarked); FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, StripUsernamePasswordTest); FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, DeleteThumbnailsDatabaseTest); + FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, AddPageVisitSource); + FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, AddPageArgsSource); + FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, AddVisitsSource); + FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, RemoveVisitsSource); + FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, MigrationVisitSource); friend class ::TestingProfile; // Computes the name of the specified database on disk. @@ -340,7 +349,8 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, std::pair<URLID, VisitID> AddPageVisit(const GURL& url, base::Time time, VisitID referring_visit, - PageTransition::Type transition); + PageTransition::Type transition, + VisitSource visit_source); // Returns a redirect chain in |redirects| for the VisitID // |cur_visit|. |cur_visit| is assumed to be valid. Assumes that diff --git a/chrome/browser/history/history_backend_unittest.cc b/chrome/browser/history/history_backend_unittest.cc index 24cc1cd..907b7b6 100644 --- a/chrome/browser/history/history_backend_unittest.cc +++ b/chrome/browser/history/history_backend_unittest.cc @@ -4,15 +4,21 @@ #include "base/file_path.h" #include "base/file_util.h" +#include "base/command_line.h" #include "base/path_service.h" #include "base/ref_counted.h" #include "base/scoped_ptr.h" +#include "base/string16.h" #include "base/utf_string_conversions.h" #include "chrome/browser/bookmarks/bookmark_model.h" #include "chrome/browser/history/history_backend.h" #include "chrome/browser/history/history_notifications.h" #include "chrome/browser/history/in_memory_history_backend.h" #include "chrome/browser/history/in_memory_database.h" +#include "chrome/browser/history/top_sites.h" +#include "chrome/common/chrome_constants.h" +#include "chrome/common/chrome_paths.h" +#include "chrome/common/chrome_switches.h" #include "chrome/common/notification_service.h" #include "chrome/common/thumbnail_score.h" #include "chrome/tools/profiles/thumbnail-inl.h" @@ -72,7 +78,7 @@ class HistoryBackendTest : public testing::Test { scoped_refptr<history::HistoryAddPageArgs> request( new history::HistoryAddPageArgs( redirects.back(), Time::Now(), scope, page_id, GURL(), - redirects, PageTransition::LINK, true)); + redirects, PageTransition::LINK, history::SOURCE_BROWSED, true)); backend_->AddPage(request); } @@ -92,7 +98,8 @@ class HistoryBackendTest : public testing::Test { redirects.push_back(url2); scoped_refptr<HistoryAddPageArgs> request( new HistoryAddPageArgs(url2, base::Time(), dummy_scope, 0, url1, - redirects, PageTransition::CLIENT_REDIRECT, did_replace)); + redirects, PageTransition::CLIENT_REDIRECT, + history::SOURCE_BROWSED, did_replace)); backend_->AddPage(request); *transition1 = getTransition(url1); @@ -109,6 +116,10 @@ class HistoryBackendTest : public testing::Test { return visits[0].transition; } + FilePath getTestDir() { + return test_dir_; + } + BookmarkModel bookmark_model_; protected: @@ -212,7 +223,7 @@ TEST_F(HistoryBackendTest, DeleteAll) { std::vector<URLRow> rows; rows.push_back(row2); // Reversed order for the same reason as favicons. rows.push_back(row1); - backend_->AddPagesWithDetails(rows); + backend_->AddPagesWithDetails(rows, history::SOURCE_BROWSED); URLID row1_id = backend_->db_->GetRowForURL(row1.url(), NULL); URLID row2_id = backend_->db_->GetRowForURL(row2.url(), NULL); @@ -250,7 +261,7 @@ TEST_F(HistoryBackendTest, DeleteAll) { // Star row1. bookmark_model_.AddURL( - bookmark_model_.GetBookmarkBarNode(), 0, std::wstring(), row1.url()); + bookmark_model_.GetBookmarkBarNode(), 0, string16(), row1.url()); // Set full text index for each one. backend_->text_database_->AddPageData(row1.url(), row1_id, visit1_id, @@ -342,14 +353,14 @@ TEST_F(HistoryBackendTest, URLsNoLongerBookmarked) { std::vector<URLRow> rows; rows.push_back(row2); // Reversed order for the same reason as favicons. rows.push_back(row1); - backend_->AddPagesWithDetails(rows); + backend_->AddPagesWithDetails(rows, history::SOURCE_BROWSED); URLID row1_id = backend_->db_->GetRowForURL(row1.url(), NULL); URLID row2_id = backend_->db_->GetRowForURL(row2.url(), NULL); // Star the two URLs. - bookmark_model_.SetURLStarred(row1.url(), std::wstring(), true); - bookmark_model_.SetURLStarred(row2.url(), std::wstring(), true); + bookmark_model_.SetURLStarred(row1.url(), string16(), true); + bookmark_model_.SetURLStarred(row2.url(), string16(), true); // Delete url 2. Because url 2 is starred this won't delete the URL, only // the visits. @@ -366,7 +377,7 @@ TEST_F(HistoryBackendTest, URLsNoLongerBookmarked) { backend_->thumbnail_db_->GetFavIconIDForFavIconURL(favicon_url2)); // Unstar row2. - bookmark_model_.SetURLStarred(row2.url(), std::wstring(), false); + bookmark_model_.SetURLStarred(row2.url(), string16(), false); // Tell the backend it was unstarred. We have to explicitly do this as // BookmarkModel isn't wired up to the backend during testing. std::set<GURL> unstarred_urls; @@ -380,7 +391,7 @@ TEST_F(HistoryBackendTest, URLsNoLongerBookmarked) { backend_->thumbnail_db_->GetFavIconIDForFavIconURL(favicon_url2)); // Unstar row 1. - bookmark_model_.SetURLStarred(row1.url(), std::wstring(), false); + bookmark_model_.SetURLStarred(row1.url(), string16(), false); // Tell the backend it was unstarred. We have to explicitly do this as // BookmarkModel isn't wired up to the backend during testing. unstarred_urls.clear(); @@ -402,6 +413,8 @@ TEST_F(HistoryBackendTest, URLsNoLongerBookmarked) { TEST_F(HistoryBackendTest, GetPageThumbnailAfterRedirects) { ASSERT_TRUE(backend_.get()); + if (history::TopSites::IsEnabled()) + return; const char* base_url = "http://mail"; const char* thumbnail_url = "http://mail.google.com"; @@ -448,7 +461,8 @@ TEST_F(HistoryBackendTest, KeywordGenerated) { scoped_refptr<HistoryAddPageArgs> request( new HistoryAddPageArgs(url, visit_time, NULL, 0, GURL(), history::RedirectList(), - PageTransition::KEYWORD_GENERATED, false)); + PageTransition::KEYWORD_GENERATED, + history::SOURCE_BROWSED, false)); backend_->AddPage(request); // A row should have been added for the url. @@ -531,7 +545,7 @@ TEST_F(HistoryBackendTest, ImportedFaviconsTest) { std::vector<URLRow> rows; rows.push_back(row1); rows.push_back(row2); - backend_->AddPagesWithDetails(rows); + backend_->AddPagesWithDetails(rows, history::SOURCE_BROWSED); URLRow url_row1, url_row2; EXPECT_FALSE(backend_->db_->GetRowForURL(row1.url(), &url_row1) == 0); EXPECT_FALSE(backend_->db_->GetRowForURL(row2.url(), &url_row2) == 0); @@ -568,8 +582,8 @@ TEST_F(HistoryBackendTest, ImportedFaviconsTest) { EXPECT_TRUE(backend_->db_->GetRowForURL(url3, &url_row3) == 0); // If the URL is bookmarked, it should get added to history with 0 visits. - bookmark_model_.AddURL(bookmark_model_.GetBookmarkBarNode(), 0, - std::wstring(), url3); + bookmark_model_.AddURL(bookmark_model_.GetBookmarkBarNode(), 0, string16(), + url3); backend_->SetImportedFavicons(favicons); EXPECT_FALSE(backend_->db_->GetRowForURL(url3, &url_row3) == 0); EXPECT_TRUE(url_row3.visit_count() == 0); @@ -586,7 +600,8 @@ TEST_F(HistoryBackendTest, StripUsernamePasswordTest) { // Visit the url with username, password. backend_->AddPageVisit(url, base::Time::Now(), 0, - PageTransition::GetQualifier(PageTransition::TYPED)); + PageTransition::GetQualifier(PageTransition::TYPED), + history::SOURCE_BROWSED); // Fetch the row information about stripped url from history db. VisitVector visits; @@ -598,9 +613,222 @@ TEST_F(HistoryBackendTest, StripUsernamePasswordTest) { } TEST_F(HistoryBackendTest, DeleteThumbnailsDatabaseTest) { + if (history::TopSites::IsEnabled()) + return; + EXPECT_TRUE(backend_->thumbnail_db_->NeedsMigrationToTopSites()); backend_->delegate_->StartTopSitesMigration(); EXPECT_FALSE(backend_->thumbnail_db_->NeedsMigrationToTopSites()); } +TEST_F(HistoryBackendTest, AddPageVisitSource) { + ASSERT_TRUE(backend_.get()); + + GURL url("http://www.google.com"); + + // Clear all history. + backend_->DeleteAllHistory(); + + // Assume visiting the url from an externsion. + backend_->AddPageVisit(url, base::Time::Now(), 0, PageTransition::TYPED, + history::SOURCE_EXTENSION); + // Assume the url is imported from Firefox. + backend_->AddPageVisit(url, base::Time::Now(), 0, PageTransition::TYPED, + history::SOURCE_FIREFOX_IMPORTED); + // Assume this url is also synced. + backend_->AddPageVisit(url, base::Time::Now(), 0, PageTransition::TYPED, + history::SOURCE_SYNCED); + + // Fetch the row information about the url from history db. + VisitVector visits; + URLID row_id = backend_->db_->GetRowForURL(url, NULL); + backend_->db_->GetVisitsForURL(row_id, &visits); + + // Check if all the visits to the url are stored in database. + ASSERT_EQ(3U, visits.size()); + VisitSourceMap visit_sources; + backend_->db_->GetVisitsSource(visits, &visit_sources); + ASSERT_EQ(3U, visit_sources.size()); + int sources = 0; + for (int i = 0; i < 3; i++) { + switch (visit_sources[visits[i].visit_id]) { + case history::SOURCE_EXTENSION: + sources |= 0x1; + break; + case history::SOURCE_FIREFOX_IMPORTED: + sources |= 0x2; + break; + case history::SOURCE_SYNCED: + sources |= 0x4; + default: + break; + } + } + EXPECT_EQ(0x7, sources); +} + +TEST_F(HistoryBackendTest, AddPageArgsSource) { + ASSERT_TRUE(backend_.get()); + + GURL url("http://testpageargs.com"); + + // Assume this page is browsed by user. + scoped_refptr<HistoryAddPageArgs> request1( + new HistoryAddPageArgs(url, base::Time::Now(), NULL, 0, GURL(), + history::RedirectList(), + PageTransition::KEYWORD_GENERATED, + history::SOURCE_BROWSED, false)); + backend_->AddPage(request1); + // Assume this page is synced. + scoped_refptr<HistoryAddPageArgs> request2( + new HistoryAddPageArgs(url, base::Time::Now(), NULL, 0, GURL(), + history::RedirectList(), + PageTransition::LINK, + history::SOURCE_SYNCED, false)); + backend_->AddPage(request2); + // Assume this page is browsed again. + scoped_refptr<HistoryAddPageArgs> request3( + new HistoryAddPageArgs(url, base::Time::Now(), NULL, 0, GURL(), + history::RedirectList(), + PageTransition::TYPED, + history::SOURCE_BROWSED, false)); + backend_->AddPage(request3); + + // Three visits should be added with proper sources. + VisitVector visits; + URLRow row; + URLID id = backend_->db()->GetRowForURL(url, &row); + ASSERT_TRUE(backend_->db()->GetVisitsForURL(id, &visits)); + ASSERT_EQ(3U, visits.size()); + VisitSourceMap visit_sources; + backend_->db_->GetVisitsSource(visits, &visit_sources); + ASSERT_EQ(1U, visit_sources.size()); + EXPECT_EQ(history::SOURCE_SYNCED, visit_sources.begin()->second); +} + +TEST_F(HistoryBackendTest, AddVisitsSource) { + ASSERT_TRUE(backend_.get()); + + GURL url1("http://www.cnn.com"); + std::vector<base::Time> visits1; + visits1.push_back(Time::Now() - base::TimeDelta::FromDays(5)); + visits1.push_back(Time::Now() - base::TimeDelta::FromDays(1)); + visits1.push_back(Time::Now()); + + GURL url2("http://www.example.com"); + std::vector<base::Time> visits2; + visits2.push_back(Time::Now() - base::TimeDelta::FromDays(10)); + visits2.push_back(Time::Now()); + + // Clear all history. + backend_->DeleteAllHistory(); + + // Add the visits. + backend_->AddVisits(url1, visits1, history::SOURCE_IE_IMPORTED); + backend_->AddVisits(url2, visits2, history::SOURCE_SYNCED); + + // Verify the visits were added with their sources. + VisitVector visits; + URLRow row; + URLID id = backend_->db()->GetRowForURL(url1, &row); + ASSERT_TRUE(backend_->db()->GetVisitsForURL(id, &visits)); + ASSERT_EQ(3U, visits.size()); + VisitSourceMap visit_sources; + backend_->db_->GetVisitsSource(visits, &visit_sources); + ASSERT_EQ(3U, visit_sources.size()); + for (int i = 0; i < 3; i++) + EXPECT_EQ(history::SOURCE_IE_IMPORTED, visit_sources[visits[i].visit_id]); + id = backend_->db()->GetRowForURL(url2, &row); + ASSERT_TRUE(backend_->db()->GetVisitsForURL(id, &visits)); + ASSERT_EQ(2U, visits.size()); + backend_->db_->GetVisitsSource(visits, &visit_sources); + ASSERT_EQ(2U, visit_sources.size()); + for (int i = 0; i < 2; i++) + EXPECT_EQ(history::SOURCE_SYNCED, visit_sources[visits[i].visit_id]); +} + +TEST_F(HistoryBackendTest, RemoveVisitsSource) { + ASSERT_TRUE(backend_.get()); + + GURL url1("http://www.cnn.com"); + std::vector<base::Time> visits1; + visits1.push_back(Time::Now() - base::TimeDelta::FromDays(5)); + visits1.push_back(Time::Now()); + + GURL url2("http://www.example.com"); + std::vector<base::Time> visits2; + visits2.push_back(Time::Now() - base::TimeDelta::FromDays(10)); + visits2.push_back(Time::Now()); + + // Clear all history. + backend_->DeleteAllHistory(); + + // Add the visits. + backend_->AddVisits(url1, visits1, history::SOURCE_IE_IMPORTED); + backend_->AddVisits(url2, visits2, history::SOURCE_SYNCED); + + // Verify the visits of url1 were added. + VisitVector visits; + URLRow row; + URLID id = backend_->db()->GetRowForURL(url1, &row); + ASSERT_TRUE(backend_->db()->GetVisitsForURL(id, &visits)); + ASSERT_EQ(2U, visits.size()); + // Remove these visits. + ASSERT_TRUE(backend_->RemoveVisits(visits)); + + // Now check only url2's source in visit_source table. + VisitSourceMap visit_sources; + backend_->db_->GetVisitsSource(visits, &visit_sources); + ASSERT_EQ(0U, visit_sources.size()); + id = backend_->db()->GetRowForURL(url2, &row); + ASSERT_TRUE(backend_->db()->GetVisitsForURL(id, &visits)); + ASSERT_EQ(2U, visits.size()); + backend_->db_->GetVisitsSource(visits, &visit_sources); + ASSERT_EQ(2U, visit_sources.size()); + for (int i = 0; i < 2; i++) + EXPECT_EQ(history::SOURCE_SYNCED, visit_sources[visits[i].visit_id]); +} + +// Test for migration of adding visit_source table. +TEST_F(HistoryBackendTest, MigrationVisitSource) { + ASSERT_TRUE(backend_.get()); + + FilePath old_history_path; + ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &old_history_path)); + old_history_path = old_history_path.AppendASCII("History"); + old_history_path = old_history_path.AppendASCII("HistoryNoSource"); + + // Copy history database file to current directory so that it will be deleted + // in Teardown. + FilePath new_history_path(getTestDir()); + file_util::Delete(new_history_path, true); + file_util::CreateDirectory(new_history_path); + FilePath new_history_file = new_history_path.Append(chrome::kHistoryFilename); + ASSERT_TRUE(file_util::CopyFile(old_history_path, new_history_file)); + + backend_->Closing(); + backend_ = new HistoryBackend(new_history_path, + new HistoryBackendTestDelegate(this), + &bookmark_model_); + backend_->Init(false); + + // Now the database should already be migrated. + // Check version first. + int cur_version = HistoryDatabase::GetCurrentVersion(); + sql::Connection db; + ASSERT_TRUE(db.Open(new_history_file)); + sql::Statement s(db.GetUniqueStatement( + "SELECT value FROM meta WHERE key = 'version'")); + ASSERT_TRUE(s.Step()); + int file_version = s.ColumnInt(0); + EXPECT_EQ(cur_version, file_version); + + // Check visit_source table is created and empty. + s.Assign(db.GetUniqueStatement( + "SELECT name FROM sqlite_master WHERE name=\"visit_source\"")); + ASSERT_TRUE(s.Step()); + s.Assign(db.GetUniqueStatement("SELECT * FROM visit_source LIMIT 10")); + EXPECT_FALSE(s.Step()); +} + } // namespace history diff --git a/chrome/browser/history/history_database.cc b/chrome/browser/history/history_database.cc index 26bb81a..4661814 100644 --- a/chrome/browser/history/history_database.cc +++ b/chrome/browser/history/history_database.cc @@ -26,7 +26,7 @@ namespace { // Current version number. We write databases at the "current" version number, // but any previous version that can read the "compatible" one can make do with // or database without *too* many bad effects. -static const int kCurrentVersionNumber = 18; +static const int kCurrentVersionNumber = 19; static const int kCompatibleVersionNumber = 16; static const char kEarlyExpirationThresholdKey[] = "early_expiration_threshold"; @@ -56,8 +56,7 @@ void ComputeDatabaseMetrics(const FilePath& history_name, } // namespace HistoryDatabase::HistoryDatabase() - : needs_version_17_migration_(false), - needs_version_18_migration_(false) { + : needs_version_17_migration_(false) { } HistoryDatabase::~HistoryDatabase() { @@ -134,14 +133,7 @@ void HistoryDatabase::BeginExclusiveMode() { // static int HistoryDatabase::GetCurrentVersion() { - // Temporary solution while TopSites is behind a flag. If there is - // no flag, we are still using the Thumbnails file, i.e. we are at - // version 17. - if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kTopSites)) { - return kCurrentVersionNumber; - } else { - return kCurrentVersionNumber - 1; - } + return kCurrentVersionNumber; } void HistoryDatabase::BeginTransaction() { @@ -283,21 +275,24 @@ sql::InitStatus HistoryDatabase::EnsureCurrentVersion( meta_table_.SetVersionNumber(cur_version); } - if (cur_version == 17) - needs_version_18_migration_ = true; + if (cur_version == 17) { + // Version 17 was for thumbnails to top sites migration. We ended up + // disabling it though, so 17->18 does nothing. + ++cur_version; + meta_table_.SetVersionNumber(cur_version); + } - if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kTopSites) && - cur_version == 18) { - // Set DB version back to pre-top sites. - cur_version = 17; + if (cur_version == 18) { + // This is the version prior to adding url_source column. We need to + // migrate the database. + cur_version = 19; meta_table_.SetVersionNumber(cur_version); } // When the version is too old, we just try to continue anyway, there should // not be a released product that makes a database too old for us to handle. - LOG_IF(WARNING, (cur_version < GetCurrentVersion() && - !needs_version_18_migration_)) << - "History database version " << cur_version << " is too old to handle."; + LOG_IF(WARNING, cur_version < GetCurrentVersion()) << + "History database version " << cur_version << " is too old to handle."; return sql::INIT_OK; } @@ -328,10 +323,7 @@ void HistoryDatabase::MigrateTimeEpoch() { #endif void HistoryDatabase::MigrationToTopSitesDone() { - // We should be migrating from 17 to 18. - DCHECK_EQ(17, meta_table_.GetVersionNumber()); - meta_table_.SetVersionNumber(18); - needs_version_18_migration_ = false; + // TODO(sky): implement me. } } // namespace history diff --git a/chrome/browser/history/history_database.h b/chrome/browser/history/history_database.h index 3b3414a..fe74e89 100644 --- a/chrome/browser/history/history_database.h +++ b/chrome/browser/history/history_database.h @@ -4,6 +4,7 @@ #ifndef CHROME_BROWSER_HISTORY_HISTORY_DATABASE_H_ #define CHROME_BROWSER_HISTORY_HISTORY_DATABASE_H_ +#pragma once #include "app/sql/connection.h" #include "app/sql/init_status.h" @@ -122,14 +123,6 @@ class HistoryDatabase : public DownloadDatabase, return needs_version_17_migration_; } - // Returns true if the Thumbnails database should be renamed to - // Favicons database. 17 -> 18 is migration to TopSites. ThumbnailsDatabase - // doesn't store the thumbnails any more, only the favicons. Hence, its file - // is renamed from Thumbnails to Favicons. - bool needs_version_18_migration() const { - return needs_version_18_migration_; - } - // Update the database version after the TopSites migration. void MigrationToTopSitesDone(); @@ -178,7 +171,6 @@ class HistoryDatabase : public DownloadDatabase, // See the getters above. bool needs_version_17_migration_; - bool needs_version_18_migration_; DISALLOW_COPY_AND_ASSIGN(HistoryDatabase); }; diff --git a/chrome/browser/history/history_marshaling.h b/chrome/browser/history/history_marshaling.h index 39b8983..1efd670 100644 --- a/chrome/browser/history/history_marshaling.h +++ b/chrome/browser/history/history_marshaling.h @@ -7,6 +7,7 @@ #ifndef CHROME_BROWSER_HISTORY_HISTORY_MARSHALING_H__ #define CHROME_BROWSER_HISTORY_HISTORY_MARSHALING_H__ +#pragma once #include "base/scoped_vector.h" #include "chrome/browser/cancelable_request.h" @@ -16,49 +17,6 @@ namespace history { -// Navigation ----------------------------------------------------------------- - -// Marshalling structure for AddPage. -class HistoryAddPageArgs - : public base::RefCountedThreadSafe<HistoryAddPageArgs> { - public: - HistoryAddPageArgs(const GURL& arg_url, - base::Time arg_time, - const void* arg_id_scope, - int32 arg_page_id, - const GURL& arg_referrer, - const history::RedirectList& arg_redirects, - PageTransition::Type arg_transition, - bool arg_did_replace_entry) - : url(arg_url), - time(arg_time), - id_scope(arg_id_scope), - page_id(arg_page_id), - referrer(arg_referrer), - redirects(arg_redirects), - transition(arg_transition), - did_replace_entry(arg_did_replace_entry) { - } - - GURL url; - base::Time time; - - const void* id_scope; - int32 page_id; - - GURL referrer; - history::RedirectList redirects; - PageTransition::Type transition; - bool did_replace_entry; - - private: - friend class base::RefCountedThreadSafe<HistoryAddPageArgs>; - - ~HistoryAddPageArgs() {} - - DISALLOW_COPY_AND_ASSIGN(HistoryAddPageArgs); -}; - // Querying ------------------------------------------------------------------- typedef CancelableRequest1<HistoryService::QueryURLCallback, @@ -104,10 +62,6 @@ typedef CancelableRequest1<HistoryService::DownloadQueryCallback, typedef CancelableRequest<HistoryService::DownloadCreateCallback> DownloadCreateRequest; -typedef CancelableRequest1<HistoryService::DownloadSearchCallback, - std::vector<int64> > - DownloadSearchRequest; - // Deletion -------------------------------------------------------------------- typedef CancelableRequest<HistoryService::ExpireHistoryCallback> diff --git a/chrome/browser/history/history_notifications.h b/chrome/browser/history/history_notifications.h index 80fc9d5..3957041 100644 --- a/chrome/browser/history/history_notifications.h +++ b/chrome/browser/history/history_notifications.h @@ -6,6 +6,7 @@ #ifndef CHROME_BROWSER_HISTORY_HISTORY_NOTIFICATIONS_H__ #define CHROME_BROWSER_HISTORY_HISTORY_NOTIFICATIONS_H__ +#pragma once #include <set> #include <vector> diff --git a/chrome/browser/history/history_publisher.h b/chrome/browser/history/history_publisher.h index 5fafc3e..fdf94b7 100644 --- a/chrome/browser/history/history_publisher.h +++ b/chrome/browser/history/history_publisher.h @@ -4,9 +4,9 @@ #ifndef CHROME_BROWSER_HISTORY_HISTORY_PUBLISHER_H_ #define CHROME_BROWSER_HISTORY_HISTORY_PUBLISHER_H_ +#pragma once #include <vector> -#include <string> #include "base/basictypes.h" #include "base/string16.h" diff --git a/chrome/browser/history/history_publisher_win.cc b/chrome/browser/history/history_publisher_win.cc index cbde619..3a7a548 100644 --- a/chrome/browser/history/history_publisher_win.cc +++ b/chrome/browser/history/history_publisher_win.cc @@ -15,6 +15,7 @@ #include "base/scoped_variant_win.h" #include "base/string_util.h" #include "base/time.h" +#include "base/utf_string_conversions.h" #include "googleurl/src/gurl.h" namespace { @@ -119,7 +120,10 @@ void HistoryPublisher::PublishDataToIndexers(const PageData& page_data) ScopedBstr url(ASCIIToWide(page_data.url.spec()).c_str()); ScopedBstr html(page_data.html); ScopedBstr title(page_data.title); - ScopedBstr format(ASCIIToWide(page_data.thumbnail_format).c_str()); + // Don't send a NULL string through ASCIIToWide. + ScopedBstr format(page_data.thumbnail_format ? + ASCIIToWide(page_data.thumbnail_format).c_str() : + NULL); ScopedVariant psa(thumbnail_arr.m_psa); for (size_t i = 0; i < indexers_.size(); ++i) { indexers_[i]->SendPageData(time, url, html, title, format, psa); diff --git a/chrome/browser/history/history_querying_unittest.cc b/chrome/browser/history/history_querying_unittest.cc index 7512786..c752d3f 100644 --- a/chrome/browser/history/history_querying_unittest.cc +++ b/chrome/browser/history/history_querying_unittest.cc @@ -7,6 +7,7 @@ #include "base/file_path.h" #include "base/file_util.h" #include "base/path_service.h" +#include "base/scoped_temp_dir.h" #include "base/utf_string_conversions.h" #include "chrome/browser/history/history.h" #include "testing/gtest/include/gtest/gtest.h" @@ -86,11 +87,9 @@ class HistoryQueryTest : public testing::Test { private: virtual void SetUp() { - FilePath temp_dir; - PathService::Get(base::DIR_TEMP, &temp_dir); - history_dir_ = temp_dir.AppendASCII("HistoryTest"); - file_util::Delete(history_dir_, true); - file_util::CreateDirectory(history_dir_); + ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); + history_dir_ = temp_dir_.path().AppendASCII("HistoryTest"); + ASSERT_TRUE(file_util::CreateDirectory(history_dir_)); history_ = new HistoryService; if (!history_->Init(history_dir_, NULL)) { @@ -111,7 +110,7 @@ class HistoryQueryTest : public testing::Test { history_->AddPage(url, test_entries[i].time, id_scope, page_id, GURL(), PageTransition::LINK, history::RedirectList(), - false); + history::SOURCE_BROWSED, false); history_->SetPageTitle(url, UTF8ToUTF16(test_entries[i].title)); history_->SetPageContents(url, UTF8ToUTF16(test_entries[i].body)); } @@ -124,7 +123,6 @@ class HistoryQueryTest : public testing::Test { history_ = NULL; MessageLoop::current()->Run(); // Wait for the other thread. } - file_util::Delete(history_dir_, true); } void QueryHistoryComplete(HistoryService::Handle, QueryResults* results) { @@ -132,6 +130,8 @@ class HistoryQueryTest : public testing::Test { MessageLoop::current()->Quit(); // Will return out to QueryHistory. } + ScopedTempDir temp_dir_; + MessageLoop message_loop_; FilePath history_dir_; @@ -314,7 +314,7 @@ TEST_F(HistoryQueryTest, FTSArchived) { row2.set_last_visit(Time::Now()); urls_to_add.push_back(row2); - history_->AddPagesWithDetails(urls_to_add); + history_->AddPagesWithDetails(urls_to_add, history::SOURCE_BROWSED); QueryOptions options; QueryResults results; diff --git a/chrome/browser/history/history_types.cc b/chrome/browser/history/history_types.cc index 50395aa..8a8ce6d 100644 --- a/chrome/browser/history/history_types.cc +++ b/chrome/browser/history/history_types.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. @@ -10,11 +10,43 @@ #include "base/stl_util-inl.h" using base::Time; +using base::TimeDelta; namespace history { // URLRow ---------------------------------------------------------------------- +URLRow::URLRow() { + Initialize(); +} + +URLRow::URLRow(const GURL& url) : url_(url) { + // Initialize will not set the URL, so our initialization above will stay. + Initialize(); +} + +URLRow::URLRow(const GURL& url, URLID id) : url_(url) { + // Initialize will not set the URL, so our initialization above will stay. + Initialize(); + // Initialize will zero the id_, so set it here. + id_ = id; +} + +URLRow::~URLRow() { +} + +URLRow& URLRow::operator=(const URLRow& other) { + id_ = other.id_; + url_ = other.url_; + title_ = other.title_; + visit_count_ = other.visit_count_; + typed_count_ = other.typed_count_; + last_visit_ = other.last_visit_; + hidden_ = other.hidden_; + favicon_id_ = other.favicon_id_; + return *this; +} + void URLRow::Swap(URLRow* other) { std::swap(id_, other->id_); url_.Swap(&other->url_); @@ -60,6 +92,9 @@ VisitRow::VisitRow(URLID arg_url_id, is_indexed(false) { } +VisitRow::~VisitRow() { +} + // StarredEntry ---------------------------------------------------------------- StarredEntry::StarredEntry() @@ -71,6 +106,9 @@ StarredEntry::StarredEntry() url_id(0) { } +StarredEntry::~StarredEntry() { +} + void StarredEntry::Swap(StarredEntry* other) { std::swap(id, other->id); title.swap(other->title); @@ -86,6 +124,23 @@ void StarredEntry::Swap(StarredEntry* other) { // URLResult ------------------------------------------------------------------- +URLResult::URLResult() { +} + +URLResult::URLResult(const GURL& url, base::Time visit_time) + : URLRow(url), + visit_time_(visit_time) { +} + +URLResult::URLResult(const GURL& url, + const Snippet::MatchPositions& title_matches) + : URLRow(url) { + title_match_positions_ = title_matches; +} + +URLResult::~URLResult() { +} + void URLResult::Swap(URLResult* other) { URLRow::Swap(other); std::swap(visit_time_, other->visit_time_); @@ -237,4 +292,34 @@ void QueryResults::AdjustResultMap(size_t begin, size_t end, ptrdiff_t delta) { } } +HistoryAddPageArgs::HistoryAddPageArgs( + const GURL& arg_url, + base::Time arg_time, + const void* arg_id_scope, + int32 arg_page_id, + const GURL& arg_referrer, + const history::RedirectList& arg_redirects, + PageTransition::Type arg_transition, + VisitSource arg_source, + bool arg_did_replace_entry) + : url(arg_url), + time(arg_time), + id_scope(arg_id_scope), + page_id(arg_page_id), + referrer(arg_referrer), + redirects(arg_redirects), + transition(arg_transition), + visit_source(arg_source), + did_replace_entry(arg_did_replace_entry) { +} + +HistoryAddPageArgs::~HistoryAddPageArgs() { +} + +HistoryAddPageArgs* HistoryAddPageArgs::Clone() const { + return new HistoryAddPageArgs( + url, time, id_scope, page_id, referrer, redirects, transition, + visit_source, did_replace_entry); +} + } // namespace history diff --git a/chrome/browser/history/history_types.h b/chrome/browser/history/history_types.h index f7bc7fb..8fab602 100644 --- a/chrome/browser/history/history_types.h +++ b/chrome/browser/history/history_types.h @@ -4,19 +4,23 @@ #ifndef CHROME_BROWSER_HISTORY_HISTORY_TYPES_H_ #define CHROME_BROWSER_HISTORY_HISTORY_TYPES_H_ +#pragma once +#include <deque> #include <map> #include <set> #include <string> #include <vector> #include "base/basictypes.h" +#include "base/ref_counted_memory.h" #include "base/stack_container.h" #include "base/string16.h" #include "base/time.h" #include "chrome/browser/history/snippet.h" #include "chrome/common/page_transition_types.h" #include "chrome/common/ref_counted_util.h" +#include "chrome/common/thumbnail_score.h" #include "googleurl/src/gurl.h" namespace history { @@ -56,25 +60,16 @@ typedef int64 URLID; // dirty bits will not be in sync for these copies. class URLRow { public: - URLRow() { - Initialize(); - } + URLRow(); - explicit URLRow(const GURL& url) : url_(url) { - // Initialize will not set the URL, so our initialization above will stay. - Initialize(); - } + explicit URLRow(const GURL& url); // We need to be able to set the id of a URLRow that's being passed through // an IPC message. This constructor should probably not be used otherwise. - URLRow(const GURL& url, URLID id) : url_(url) { - // Initialize will not set the URL, so our initialization above will stay. - Initialize(); - // Initialize will zero the id_, so set it here. - id_ = id; - } + URLRow(const GURL& url, URLID id); - virtual ~URLRow() {} + virtual ~URLRow(); + URLRow& operator=(const URLRow& other); URLID id() const { return id_; } const GURL& url() const { return url_; } @@ -172,9 +167,27 @@ class URLRow { // We support the implicit copy constuctor and operator=. }; -// VisitRow ------------------------------------------------------------------- +// The enumeration of all possible sources of visits is listed below. +// The source will be propogated along with a URL or a visit item +// and eventually be stored in the history database, +// visit_source table specifically. +// Different from page transition types, they describe the origins of visits. +// (Warning): Please don't change any existing values while it is ok to add +// new values when needed. +typedef enum { + SOURCE_SYNCED = 0, // Synchronized from somewhere else. + SOURCE_BROWSED = 1, // User browsed. + SOURCE_EXTENSION = 2, // Added by an externsion. + SOURCE_FIREFOX_IMPORTED = 3, + SOURCE_IE_IMPORTED = 4, + SOURCE_SAFARI_IMPORTED = 5, +} VisitSource; typedef int64 VisitID; +// Structure to hold the mapping between each visit's id and its source. +typedef std::map<VisitID, VisitSource> VisitSourceMap; + +// VisitRow ------------------------------------------------------------------- // Holds all information associated with a specific visit. A visit holds time // and referrer information for one time a URL is visited. @@ -186,6 +199,7 @@ class VisitRow { VisitID arg_referring_visit, PageTransition::Type arg_transition, SegmentID arg_segment_id); + ~VisitRow(); // ID of this row (visit ID, used a a referrer for other visits). VisitID visit_id; @@ -277,6 +291,7 @@ struct StarredEntry { }; StarredEntry(); + ~StarredEntry(); void Swap(StarredEntry* other); @@ -310,7 +325,7 @@ struct StarredEntry { // If type == URL, this is the ID of the URL of the primary page that was // starred. - history::URLID url_id; + URLID url_id; // Time the entry was last modified. This is only used for groups and // indicates the last time a URL was added as a child to the group. @@ -321,17 +336,12 @@ struct StarredEntry { class URLResult : public URLRow { public: - URLResult() {} - URLResult(const GURL& url, base::Time visit_time) - : URLRow(url), - visit_time_(visit_time) { - } + URLResult(); + URLResult(const GURL& url, base::Time visit_time); // Constructor that create a URLResult from the specified URL and title match // positions from title_matches. - URLResult(const GURL& url, const Snippet::MatchPositions& title_matches) - : URLRow(url) { - title_match_positions_ = title_matches; - } + URLResult(const GURL& url, const Snippet::MatchPositions& title_matches); + ~URLResult(); base::Time visit_time() const { return visit_time_; } void set_visit_time(base::Time visit_time) { visit_time_ = visit_time; } @@ -525,8 +535,57 @@ struct MostVisitedURL { } }; +// Used by TopSites to store the thumbnails. +struct Images { + scoped_refptr<RefCountedBytes> thumbnail; + ThumbnailScore thumbnail_score; + + // TODO(brettw): this will eventually store the favicon. + // scoped_refptr<RefCountedBytes> favicon; +}; + typedef std::vector<MostVisitedURL> MostVisitedURLList; -} // history +// Navigation ----------------------------------------------------------------- + +// Marshalling structure for AddPage. +class HistoryAddPageArgs + : public base::RefCountedThreadSafe<HistoryAddPageArgs> { + public: + HistoryAddPageArgs(const GURL& arg_url, + base::Time arg_time, + const void* arg_id_scope, + int32 arg_page_id, + const GURL& arg_referrer, + const history::RedirectList& arg_redirects, + PageTransition::Type arg_transition, + VisitSource arg_source, + bool arg_did_replace_entry); + + // Returns a new HistoryAddPageArgs that is a copy of this (ref count is + // of course reset). Ownership of returned object passes to caller. + HistoryAddPageArgs* Clone() const; + + GURL url; + base::Time time; + + const void* id_scope; + int32 page_id; + + GURL referrer; + history::RedirectList redirects; + PageTransition::Type transition; + VisitSource visit_source; + bool did_replace_entry; + + private: + friend class base::RefCountedThreadSafe<HistoryAddPageArgs>; + + ~HistoryAddPageArgs(); + + DISALLOW_COPY_AND_ASSIGN(HistoryAddPageArgs); +}; + +} // namespace history #endif // CHROME_BROWSER_HISTORY_HISTORY_TYPES_H_ diff --git a/chrome/browser/history/history_unittest.cc b/chrome/browser/history/history_unittest.cc index c8db05a..53d91c6 100644 --- a/chrome/browser/history/history_unittest.cc +++ b/chrome/browser/history/history_unittest.cc @@ -24,15 +24,19 @@ #include "app/sql/statement.h" #include "base/basictypes.h" #include "base/callback.h" +#include "base/command_line.h" #include "base/file_path.h" #include "base/file_util.h" #include "base/message_loop.h" #include "base/path_service.h" +#include "base/scoped_temp_dir.h" #include "base/scoped_vector.h" #include "base/string_util.h" #include "base/task.h" +#include "base/utf_string_conversions.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/download/download_item.h" +#include "chrome/browser/history/download_create_info.h" #include "chrome/browser/history/history.h" #include "chrome/browser/history/history_backend.h" #include "chrome/browser/history/history_database.h" @@ -40,7 +44,9 @@ #include "chrome/browser/history/in_memory_database.h" #include "chrome/browser/history/in_memory_history_backend.h" #include "chrome/browser/history/page_usage_data.h" +#include "chrome/browser/history/top_sites.h" #include "chrome/common/chrome_paths.h" +#include "chrome/common/chrome_switches.h" #include "chrome/common/notification_service.h" #include "chrome/common/thumbnail_score.h" #include "chrome/tools/profiles/thumbnail-inl.h" @@ -163,11 +169,9 @@ class HistoryTest : public testing::Test { // testing::Test virtual void SetUp() { - FilePath temp_dir; - PathService::Get(base::DIR_TEMP, &temp_dir); - history_dir_ = temp_dir.AppendASCII("HistoryTest"); - file_util::Delete(history_dir_, true); - file_util::CreateDirectory(history_dir_); + ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); + history_dir_ = temp_dir_.path().AppendASCII("HistoryTest"); + ASSERT_TRUE(file_util::CreateDirectory(history_dir_)); } void DeleteBackend() { @@ -183,9 +187,6 @@ class HistoryTest : public testing::Test { if (history_service_) CleanupHistoryService(); - // Try to clean up the database file. - file_util::Delete(history_dir_, true); - // Make sure we don't have any event pending that could disrupt the next // test. MessageLoop::current()->PostTask(FROM_HERE, new MessageLoop::QuitTask); @@ -261,6 +262,8 @@ class HistoryTest : public testing::Test { MessageLoop::current()->Quit(); } + ScopedTempDir temp_dir_; + MessageLoopForUI message_loop_; // PageUsageData vector to test segments. @@ -413,7 +416,8 @@ TEST_F(HistoryTest, AddPage) { const GURL test_url("http://www.google.com/"); history->AddPage(test_url, NULL, 0, GURL(), PageTransition::MANUAL_SUBFRAME, - history::RedirectList(), false); + history::RedirectList(), + history::SOURCE_BROWSED, false); EXPECT_TRUE(QueryURL(history, test_url)); EXPECT_EQ(1, query_url_row_.visit_count()); EXPECT_EQ(0, query_url_row_.typed_count()); @@ -421,7 +425,8 @@ TEST_F(HistoryTest, AddPage) { // Add the page once from the main frame (should unhide it). history->AddPage(test_url, NULL, 0, GURL(), PageTransition::LINK, - history::RedirectList(), false); + history::RedirectList(), + history::SOURCE_BROWSED, false); EXPECT_TRUE(QueryURL(history, test_url)); EXPECT_EQ(2, query_url_row_.visit_count()); // Added twice. EXPECT_EQ(0, query_url_row_.typed_count()); // Never typed. @@ -444,14 +449,16 @@ TEST_F(HistoryTest, AddPageSameTimes) { // additions have different timestamps. history->AddPage(test_urls[0], now, NULL, 0, GURL(), PageTransition::LINK, - history::RedirectList(), false); + history::RedirectList(), + history::SOURCE_BROWSED, false); EXPECT_TRUE(QueryURL(history, test_urls[0])); EXPECT_EQ(1, query_url_row_.visit_count()); EXPECT_TRUE(now == query_url_row_.last_visit()); // gtest doesn't like Time history->AddPage(test_urls[1], now, NULL, 0, GURL(), PageTransition::LINK, - history::RedirectList(), false); + history::RedirectList(), + history::SOURCE_BROWSED, false); EXPECT_TRUE(QueryURL(history, test_urls[1])); EXPECT_EQ(1, query_url_row_.visit_count()); EXPECT_TRUE(now + TimeDelta::FromMicroseconds(1) == @@ -461,7 +468,8 @@ TEST_F(HistoryTest, AddPageSameTimes) { history->AddPage(test_urls[2], now + TimeDelta::FromMinutes(1), NULL, 0, GURL(), PageTransition::LINK, - history::RedirectList(), false); + history::RedirectList(), + history::SOURCE_BROWSED, false); EXPECT_TRUE(QueryURL(history, test_urls[2])); EXPECT_EQ(1, query_url_row_.visit_count()); EXPECT_TRUE(now + TimeDelta::FromMinutes(1) == @@ -484,7 +492,8 @@ TEST_F(HistoryTest, AddRedirect) { // Add the sequence of pages as a server with no referrer. Note that we need // to have a non-NULL page ID scope. history->AddPage(first_redirects.back(), MakeFakeHost(1), 0, GURL(), - PageTransition::LINK, first_redirects, true); + PageTransition::LINK, first_redirects, + history::SOURCE_BROWSED, true); // The first page should be added once with a link visit type (because we set // LINK when we added the original URL, and a referrer of nowhere (0). @@ -522,7 +531,7 @@ TEST_F(HistoryTest, AddRedirect) { second_redirects[0], static_cast<PageTransition::Type>(PageTransition::LINK | PageTransition::CLIENT_REDIRECT), - second_redirects, true); + second_redirects, history::SOURCE_BROWSED, true); // The last page (source of the client redirect) should NOT have an // additional visit added, because it was a client redirect (normally it @@ -547,7 +556,8 @@ TEST_F(HistoryTest, Typed) { // Add the page once as typed. const GURL test_url("http://www.google.com/"); history->AddPage(test_url, NULL, 0, GURL(), PageTransition::TYPED, - history::RedirectList(), false); + history::RedirectList(), + history::SOURCE_BROWSED, false); EXPECT_TRUE(QueryURL(history, test_url)); // We should have the same typed & visit count. @@ -556,7 +566,8 @@ TEST_F(HistoryTest, Typed) { // Add the page again not typed. history->AddPage(test_url, NULL, 0, GURL(), PageTransition::LINK, - history::RedirectList(), false); + history::RedirectList(), + history::SOURCE_BROWSED, false); EXPECT_TRUE(QueryURL(history, test_url)); // The second time should not have updated the typed count. @@ -566,7 +577,7 @@ TEST_F(HistoryTest, Typed) { // Add the page again as a generated URL. history->AddPage(test_url, NULL, 0, GURL(), PageTransition::GENERATED, history::RedirectList(), - false); + history::SOURCE_BROWSED, false); EXPECT_TRUE(QueryURL(history, test_url)); // This should have worked like a link click. @@ -576,7 +587,7 @@ TEST_F(HistoryTest, Typed) { // Add the page again as a reload. history->AddPage(test_url, NULL, 0, GURL(), PageTransition::RELOAD, history::RedirectList(), - false); + history::SOURCE_BROWSED, false); EXPECT_TRUE(QueryURL(history, test_url)); // This should not have incremented any visit counts. @@ -591,7 +602,7 @@ TEST_F(HistoryTest, SetTitle) { // Add a URL. const GURL existing_url("http://www.google.com/"); - history->AddPage(existing_url); + history->AddPage(existing_url, history::SOURCE_BROWSED); // Set some title. const string16 existing_title = UTF8ToUTF16("Google"); @@ -626,7 +637,7 @@ TEST_F(HistoryTest, Segments) { const GURL existing_url("http://www.google.com/"); history->AddPage(existing_url, scope, 0, GURL(), PageTransition::TYPED, history::RedirectList(), - false); + history::SOURCE_BROWSED, false); // Make sure a segment was created. history->QuerySegmentUsageSince( @@ -645,7 +656,7 @@ TEST_F(HistoryTest, Segments) { const GURL link_url("http://yahoo.com/"); history->AddPage(link_url, scope, 0, GURL(), PageTransition::LINK, history::RedirectList(), - false); + history::SOURCE_BROWSED, false); // Query again history->QuerySegmentUsageSince( @@ -663,7 +674,7 @@ TEST_F(HistoryTest, Segments) { // Add a page linked from existing_url. history->AddPage(GURL("http://www.google.com/foo"), scope, 3, existing_url, PageTransition::LINK, history::RedirectList(), - false); + history::SOURCE_BROWSED, false); // Query again history->QuerySegmentUsageSince( @@ -685,6 +696,9 @@ TEST_F(HistoryTest, Segments) { // This just tests history system -> thumbnail database integration, the actual // thumbnail tests are in its own file. TEST_F(HistoryTest, Thumbnails) { + if (history::TopSites::IsEnabled()) + return; // TopSitesTest replaces this. + scoped_refptr<HistoryService> history(new HistoryService); history_service_ = history; ASSERT_TRUE(history->Init(history_dir_, NULL)); @@ -694,7 +708,8 @@ TEST_F(HistoryTest, Thumbnails) { static const double boringness = 0.25; const GURL url("http://www.google.com/thumbnail_test/"); - history->AddPage(url); // Must be visited before adding a thumbnail. + // Must be visited before adding a thumbnail. + history->AddPage(url, history::SOURCE_BROWSED); history->SetPageThumbnail(url, *thumbnail, ThumbnailScore(boringness, true, true)); @@ -761,10 +776,10 @@ TEST_F(HistoryTest, MostVisitedURLs) { // Add two pages. history->AddPage(url0, scope, 0, GURL(), PageTransition::TYPED, history::RedirectList(), - false); + history::SOURCE_BROWSED, false); history->AddPage(url1, scope, 0, GURL(), PageTransition::TYPED, history::RedirectList(), - false); + history::SOURCE_BROWSED, false); history->QueryMostVisitedURLs(20, 90, &consumer_, NewCallback(static_cast<HistoryTest*>(this), &HistoryTest::OnMostVisitedURLsAvailable)); @@ -777,7 +792,7 @@ TEST_F(HistoryTest, MostVisitedURLs) { // Add another page. history->AddPage(url2, scope, 0, GURL(), PageTransition::TYPED, history::RedirectList(), - false); + history::SOURCE_BROWSED, false); history->QueryMostVisitedURLs(20, 90, &consumer_, NewCallback(static_cast<HistoryTest*>(this), &HistoryTest::OnMostVisitedURLsAvailable)); @@ -791,7 +806,7 @@ TEST_F(HistoryTest, MostVisitedURLs) { // Revisit url2, making it the top URL. history->AddPage(url2, scope, 0, GURL(), PageTransition::TYPED, history::RedirectList(), - false); + history::SOURCE_BROWSED, false); history->QueryMostVisitedURLs(20, 90, &consumer_, NewCallback(static_cast<HistoryTest*>(this), &HistoryTest::OnMostVisitedURLsAvailable)); @@ -805,7 +820,7 @@ TEST_F(HistoryTest, MostVisitedURLs) { // Revisit url1, making it the top URL. history->AddPage(url1, scope, 0, GURL(), PageTransition::TYPED, history::RedirectList(), - false); + history::SOURCE_BROWSED, false); history->QueryMostVisitedURLs(20, 90, &consumer_, NewCallback(static_cast<HistoryTest*>(this), &HistoryTest::OnMostVisitedURLsAvailable)); @@ -824,7 +839,7 @@ TEST_F(HistoryTest, MostVisitedURLs) { // Visit url4 using redirects. history->AddPage(url4, scope, 0, GURL(), PageTransition::TYPED, redirects, - false); + history::SOURCE_BROWSED, false); history->QueryMostVisitedURLs(20, 90, &consumer_, NewCallback(static_cast<HistoryTest*>(this), &HistoryTest::OnMostVisitedURLsAvailable)); @@ -882,7 +897,8 @@ HistoryAddPageArgs* MakeAddArgs(const GURL& url) { 0, GURL(), history::RedirectList(), - PageTransition::TYPED, false); + PageTransition::TYPED, + history::SOURCE_BROWSED, false); } // Convenience version of the above to convert a char string. diff --git a/chrome/browser/history/in_memory_database.h b/chrome/browser/history/in_memory_database.h index 62460dd..27e1091 100644 --- a/chrome/browser/history/in_memory_database.h +++ b/chrome/browser/history/in_memory_database.h @@ -1,11 +1,10 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. #ifndef CHROME_BROWSER_HISTORY_IN_MEMORY_DATABASE_H_ #define CHROME_BROWSER_HISTORY_IN_MEMORY_DATABASE_H_ - -#include <string> +#pragma once #include "app/sql/connection.h" #include "base/basictypes.h" diff --git a/chrome/browser/history/in_memory_history_backend.cc b/chrome/browser/history/in_memory_history_backend.cc index 9f3e7be..b3d5da9 100644 --- a/chrome/browser/history/in_memory_history_backend.cc +++ b/chrome/browser/history/in_memory_history_backend.cc @@ -5,13 +5,19 @@ #include "chrome/browser/history/in_memory_history_backend.h" #include "base/command_line.h" +#include "base/histogram.h" +#include "base/time.h" +#include "base/utf_string_conversions.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/history/history_notifications.h" #include "chrome/browser/history/in_memory_database.h" #include "chrome/browser/history/in_memory_url_index.h" +#include "chrome/browser/history/url_database.h" +#include "chrome/browser/prefs/pref_service.h" #include "chrome/browser/profile.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/notification_service.h" +#include "chrome/common/pref_names.h" namespace history { @@ -26,14 +32,19 @@ InMemoryHistoryBackend::InMemoryHistoryBackend() InMemoryHistoryBackend::~InMemoryHistoryBackend() { } -bool InMemoryHistoryBackend::Init(const FilePath& history_filename) { +bool InMemoryHistoryBackend::Init(const FilePath& history_filename, + URLDatabase* db) { db_.reset(new InMemoryDatabase); bool success = db_->InitFromDisk(history_filename); if (CommandLine::ForCurrentProcess()->HasSwitch( switches::kEnableInMemoryURLIndex)) { - index_.reset(new InMemoryURLIndex); - // TODO(rohitrao): Load index. + index_.reset(new InMemoryURLIndex()); + base::TimeTicks beginning_time = base::TimeTicks::Now(); + // TODO(mrossetti): Provide languages when profile is available. + index_->Init(db, std::string()); + UMA_HISTOGRAM_TIMES("Autocomplete.HistoryDatabaseIndexingTime", + base::TimeTicks::Now() - beginning_time); } return success; diff --git a/chrome/browser/history/in_memory_history_backend.h b/chrome/browser/history/in_memory_history_backend.h index 30026b5..bfa51d8 100644 --- a/chrome/browser/history/in_memory_history_backend.h +++ b/chrome/browser/history/in_memory_history_backend.h @@ -12,12 +12,12 @@ #ifndef CHROME_BROWSER_HISTORY_IN_MEMORY_HISTORY_BACKEND_H_ #define CHROME_BROWSER_HISTORY_IN_MEMORY_HISTORY_BACKEND_H_ - -#include <string> +#pragma once #include "base/basictypes.h" #include "base/gtest_prod_util.h" #include "base/scoped_ptr.h" +#include "chrome/common/notification_observer.h" #include "chrome/common/notification_registrar.h" class FilePath; @@ -28,6 +28,7 @@ namespace history { class InMemoryDatabase; class InMemoryURLIndex; +class URLDatabase; struct URLsDeletedDetails; struct URLsModifiedDetails; @@ -37,7 +38,7 @@ class InMemoryHistoryBackend : public NotificationObserver { ~InMemoryHistoryBackend(); // Initializes with data from the given history database. - bool Init(const FilePath& history_filename); + bool Init(const FilePath& history_filename, URLDatabase* db); // Does initialization work when this object is attached to the history // system on the main thread. The argument is the profile with which the diff --git a/chrome/browser/history/in_memory_url_index.cc b/chrome/browser/history/in_memory_url_index.cc index 83c401f..95afa57 100644 --- a/chrome/browser/history/in_memory_url_index.cc +++ b/chrome/browser/history/in_memory_url_index.cc @@ -4,10 +4,495 @@ #include "chrome/browser/history/in_memory_url_index.h" +#include <algorithm> +#include <limits> + +#include "app/l10n_util.h" +#include "base/i18n/word_iterator.h" +#include "base/string_util.h" +#include "base/time.h" +#include "base/utf_string_conversions.h" +#include "chrome/browser/autocomplete/history_provider_util.h" +#include "chrome/browser/history/url_database.h" +#include "net/base/escape.h" +#include "net/base/net_util.h" + +using base::Time; +using base::TimeDelta; + namespace history { -InMemoryURLIndex::InMemoryURLIndex() {} +ScoredHistoryMatch::ScoredHistoryMatch() : raw_score(0) {} + +ScoredHistoryMatch::ScoredHistoryMatch(const URLRow& url_info, + size_t input_location, + bool match_in_scheme, + bool innermost_match, + int score) + : HistoryMatch(url_info, input_location, match_in_scheme, innermost_match), + raw_score(score) { +} + +InMemoryURLIndex::InMemoryURLIndex() : history_item_count_(0) {} InMemoryURLIndex::~InMemoryURLIndex() {} +static const int32_t kURLToLowerBufferSize = 2048; + +// Indexing + +bool InMemoryURLIndex::Init(history::URLDatabase* history_db, + const std::string& languages) { + // TODO(mrossetti): Register for profile/language change notifications. + languages_ = languages; + // Reset our indexes. + char_word_map_.clear(); + word_id_history_map_.clear(); + if (!history_db) + return false; + URLDatabase::URLEnumerator history_enum; + if (history_db->InitURLEnumeratorForEverything(&history_enum)) { + URLRow row; + Time recent_threshold = InMemoryURLIndex::RecentThreshold(); + while (history_enum.GetNextURL(&row)) { + // Do some filtering so that we only get history items which could + // possibly pass the HistoryURLProvider::CullPoorMatches filter later. + if ((row.typed_count() > kLowQualityMatchTypedLimit) || + (row.visit_count() > kLowQualityMatchVisitLimit) || + (row.last_visit() >= recent_threshold)) { + if (!IndexRow(row)) + return false; + } + } + } + return true; +} + +bool InMemoryURLIndex::IndexRow(URLRow row) { + const GURL& gurl(row.url()); + string16 url(net::FormatUrl(gurl, languages_, + net::kFormatUrlOmitUsernamePassword, + UnescapeRule::SPACES | UnescapeRule::URL_SPECIAL_CHARS, + NULL, NULL, NULL)); + + // TODO(mrossetti): Detect row_id > std::numeric_limits<HistoryID>::max(). + HistoryID history_id = static_cast<HistoryID>(row.id()); + + // Add the row for quick lookup in the history info store. + url = l10n_util::ToLower(url); + URLRow new_row(GURL(url), row.id()); + new_row.set_visit_count(row.visit_count()); + new_row.set_typed_count(row.typed_count()); + new_row.set_last_visit(row.last_visit()); + new_row.set_title(row.title()); + history_info_map_.insert(std::make_pair(history_id, new_row)); + + // Split into individual, unique words. + String16Set words = WordSetFromString16(url); + + // For each word, add a new entry into the word index referring to the + // associated history item. + for (String16Set::iterator iter = words.begin(); + iter != words.end(); ++iter) { + String16Set::value_type uni_word = *iter; + AddWordToIndex(uni_word, history_id); + } + ++history_item_count_; + return true; +} + +// Searching + +ScoredHistoryMatches InMemoryURLIndex::HistoryItemsForTerms( + const String16Vector& terms) { + ScoredHistoryMatches scored_items; + if (!terms.empty()) { + // Reset used_ flags for term_char_word_set_cache_. We use a basic mark- + // and-sweep approach. + ResetTermCharWordSetCache(); + + // Lowercase the terms. + // TODO(mrossetti): Another opportunity for a transform algorithm. + String16Vector lower_terms; + for (String16Vector::const_iterator term_iter = terms.begin(); + term_iter != terms.end(); ++term_iter) { + String16Vector::value_type lower_string(*term_iter); + std::transform(lower_string.begin(), + lower_string.end(), + lower_string.begin(), + tolower); + lower_terms.push_back(lower_string); + } + + String16Vector::value_type all_terms(JoinString(lower_terms, ' ')); + HistoryIDSet history_id_set = HistoryIDSetFromWords(all_terms); + + // Pass over all of the candidates filtering out any without a proper + // substring match, inserting those which pass in order by score. + scored_items = std::for_each(history_id_set.begin(), history_id_set.end(), + AddHistoryMatch(*this, + lower_terms)).ScoredMatches(); + } + + // Remove any stale TermCharWordSet's. + term_char_word_set_cache_.erase( + std::remove_if(term_char_word_set_cache_.begin(), + term_char_word_set_cache_.end(), + std::mem_fun_ref(&TermCharWordSet::IsNotUsed)), + term_char_word_set_cache_.end()); + return scored_items; +} + +void InMemoryURLIndex::ResetTermCharWordSetCache() { + // TODO(mrossetti): Consider keeping more of the cache around for possible + // repeat searches, except a 'shortcuts' approach might be better for that. + // TODO(mrossetti): Change TermCharWordSet to a class and use for_each. + for (TermCharWordSetVector::iterator iter = + term_char_word_set_cache_.begin(); + iter != term_char_word_set_cache_.end(); ++iter) + iter->used_ = false; +} + +InMemoryURLIndex::HistoryIDSet InMemoryURLIndex::HistoryIDSetFromWords( + const string16& uni_string) { + // Break the terms down into individual terms (words), get the candidate + // set for each term, and intersect each to get a final candidate list. + // Note that a single 'term' from the user's perspective might be + // a string like "http://www.somewebsite.com" which, from our perspective, + // is four words: 'http', 'www', 'somewebsite', and 'com'. + HistoryIDSet history_id_set; + String16Set words = WordSetFromString16(uni_string); + bool first_word = true; + for (String16Set::iterator iter = words.begin(); + iter != words.end(); ++iter) { + String16Set::value_type uni_word = *iter; + HistoryIDSet term_history_id_set = + InMemoryURLIndex::HistoryIDsForTerm(uni_word); + if (first_word) { + history_id_set.swap(term_history_id_set); + first_word = false; + } else { + HistoryIDSet old_history_id_set(history_id_set); + history_id_set.clear(); + std::set_intersection(old_history_id_set.begin(), + old_history_id_set.end(), + term_history_id_set.begin(), + term_history_id_set.end(), + std::inserter(history_id_set, + history_id_set.begin())); + } + } + return history_id_set; +} + +InMemoryURLIndex::HistoryIDSet InMemoryURLIndex::HistoryIDsForTerm( + const string16& uni_word) { + InMemoryURLIndex::HistoryIDSet history_id_set; + + // For each character in the word, get the char/word_id map entry and + // intersect with the set in an incremental manner. + Char16Set uni_chars = CharactersFromString16(uni_word); + WordIDSet word_id_set(WordIDSetForTermChars(uni_chars)); + + // If any words resulted then we can compose a set of history IDs by unioning + // the sets from each word. + if (!word_id_set.empty()) { + for (WordIDSet::iterator word_id_iter = word_id_set.begin(); + word_id_iter != word_id_set.end(); ++word_id_iter) { + WordID word_id = *word_id_iter; + WordIDHistoryMap::iterator word_iter = word_id_history_map_.find(word_id); + if (word_iter != word_id_history_map_.end()) { + HistoryIDSet& word_history_id_set(word_iter->second); + history_id_set.insert(word_history_id_set.begin(), + word_history_id_set.end()); + } + } + } + + return history_id_set; +} + +// Utility Functions + +// static +InMemoryURLIndex::String16Set InMemoryURLIndex::WordSetFromString16( + const string16& uni_string) { + String16Set words; + WordIterator iter(&uni_string, WordIterator::BREAK_WORD); + if (iter.Init()) { + while (iter.Advance()) { + if (iter.IsWord()) + words.insert(iter.GetWord()); + } + } + return words; +} + +// static +InMemoryURLIndex::Char16Set InMemoryURLIndex::CharactersFromString16( + const string16& uni_word) { + Char16Set characters; + for (string16::const_iterator iter = uni_word.begin(); + iter != uni_word.end(); ++iter) + characters.insert(*iter); + return characters; +} + +void InMemoryURLIndex::AddWordToIndex(const string16& uni_word, + HistoryID history_id) { + WordMap::iterator word_pos = word_map_.find(uni_word); + if (word_pos != word_map_.end()) + UpdateWordHistory(word_pos->second, history_id); + else + AddWordHistory(uni_word, history_id); +} + +void InMemoryURLIndex::UpdateWordHistory(WordID word_id, HistoryID history_id) { + WordIDHistoryMap::iterator history_pos = word_id_history_map_.find(word_id); + DCHECK(history_pos != word_id_history_map_.end()); + HistoryIDSet& history_id_set(history_pos->second); + history_id_set.insert(history_id); +} + +// Add a new word to the word list and the word map, and then create a +// new entry in the word/history map. +void InMemoryURLIndex::AddWordHistory(const string16& uni_word, + HistoryID history_id) { + word_list_.push_back(uni_word); + WordID word_id = word_list_.size() - 1; + word_map_.insert(std::make_pair(uni_word, word_id)); + HistoryIDSet history_id_set; + history_id_set.insert(history_id); + word_id_history_map_.insert(std::make_pair(word_id, history_id_set)); + // For each character in the newly added word (i.e. a word that is not + // already in the word index), add the word to the character index. + Char16Set characters = CharactersFromString16(uni_word); + for (Char16Set::iterator uni_char_iter = characters.begin(); + uni_char_iter != characters.end(); ++uni_char_iter) { + Char16Set::value_type uni_string = *uni_char_iter; + CharWordIDMap::iterator char_iter = char_word_map_.find(uni_string); + if (char_iter != char_word_map_.end()) { + // Update existing entry in the char/word index. + WordIDSet& word_id_set(char_iter->second); + word_id_set.insert(word_id); + } else { + // Create a new entry in the char/word index. + WordIDSet word_id_set; + word_id_set.insert(word_id); + char_word_map_.insert(std::make_pair(uni_string, word_id_set)); + } + } +} + +InMemoryURLIndex::WordIDSet InMemoryURLIndex::WordIDSetForTermChars( + InMemoryURLIndex::Char16Set const& uni_chars) { + TermCharWordSet* best_term_char_word_set = NULL; + bool set_found = false; + size_t outstanding_count = 0; + Char16Set outstanding_chars; + for (TermCharWordSetVector::iterator iter = term_char_word_set_cache_.begin(); + iter != term_char_word_set_cache_.end(); ++iter) { + TermCharWordSetVector::value_type& term_char_word_set(*iter); + Char16Set& char_set(term_char_word_set.char_set_); + Char16Set exclusions; + std::set_difference(char_set.begin(), char_set.end(), + uni_chars.begin(), uni_chars.end(), + std::inserter(exclusions, exclusions.begin())); + if (exclusions.empty()) { + // Do a reverse difference so that we know which characters remain + // to be indexed. Then decide if this is a better match than any + // previous cached set. + std::set_difference(uni_chars.begin(), uni_chars.end(), + char_set.begin(), char_set.end(), + std::inserter(exclusions, exclusions.begin())); + if (!set_found || exclusions.size() < outstanding_count) { + outstanding_chars = exclusions; + best_term_char_word_set = &*iter; + outstanding_count = exclusions.size(); + set_found = true; + } + } + } + + WordIDSet word_id_set; + if (set_found && outstanding_count == 0) { + // If there were no oustanding characters then we can use the cached one. + best_term_char_word_set->used_ = true; + word_id_set = best_term_char_word_set->word_id_set_; + } else { + // Some or all of the characters remain to be indexed. + bool first_character = true; + if (set_found) { + first_character = false; + word_id_set = best_term_char_word_set->word_id_set_; + } else { + outstanding_chars = uni_chars; + } + + for (Char16Set::iterator uni_char_iter = outstanding_chars.begin(); + uni_char_iter != outstanding_chars.end(); ++uni_char_iter) { + Char16Set::value_type uni_char = *uni_char_iter; + CharWordIDMap::iterator char_iter = char_word_map_.find(uni_char); + if (char_iter == char_word_map_.end()) { + // The character was not found so bail. + word_id_set.clear(); + break; + } + WordIDSet& char_word_id_set(char_iter->second); + if (first_character) { + word_id_set = char_word_id_set; + first_character = false; + } else { + WordIDSet old_word_id_set(word_id_set); + word_id_set.clear(); + std::set_intersection(old_word_id_set.begin(), + old_word_id_set.end(), + char_word_id_set.begin(), + char_word_id_set.end(), + std::inserter(word_id_set, + word_id_set.begin())); + } + } + // Update the cache. + if (!set_found || outstanding_count) { + term_char_word_set_cache_.push_back(TermCharWordSet(uni_chars, + word_id_set, true)); + } + } + return word_id_set; +} + +// static +int InMemoryURLIndex::RawScoreForURL(const URLRow& row, + const String16Vector& terms, + size_t* first_term_location) { + GURL gurl = row.url(); + if (!gurl.is_valid()) + return 0; + + string16 url = UTF8ToUTF16(gurl.spec()); + + // Collect all term start positions so we can see if they appear in order. + std::vector<size_t> term_locations; + int out_of_order = 0; // Count the terms which are out of order. + size_t start_location_total = 0; + size_t term_length_total = 0; + for (String16Vector::const_iterator iter = terms.begin(); iter != terms.end(); + ++iter) { + string16 term = *iter; + size_t term_location = url.find(term); + if (term_location == string16::npos) + return 0; // A term was not found. This should not happen. + if (iter != terms.begin()) { + // See if this term is out-of-order. + for (std::vector<size_t>::const_iterator order_iter = + term_locations.begin(); order_iter != term_locations.end(); + ++order_iter) { + if (term_location <= *order_iter) + ++out_of_order; + } + } else { + *first_term_location = term_location; + } + term_locations.push_back(term_location); + start_location_total += term_location; + term_length_total += term.size(); + } + + // Calculate a raw score. + // TODO(mrossetti): This is good enough for now but must be fine-tuned. + const float kOrderMaxValue = 10.0; + float order_value = 10.0; + if (terms.size() > 1) { + int max_possible_out_of_order = (terms.size() * (terms.size() - 1)) / 2; + order_value = + (static_cast<float>(max_possible_out_of_order - out_of_order) / + max_possible_out_of_order) * kOrderMaxValue; + } + + const float kStartMaxValue = 10.0; + const size_t kMaxSignificantStart = 20; + float start_value = + (static_cast<float>(kMaxSignificantStart - + std::min(kMaxSignificantStart, start_location_total / terms.size()))) / + static_cast<float>(kMaxSignificantStart) * kStartMaxValue; + + const float kCompleteMaxValue = 10.0; + float complete_value = + (static_cast<float>(term_length_total) / static_cast<float>(url.size())) * + kStartMaxValue; + + const float kLastVisitMaxValue = 10.0; + const base::TimeDelta kMaxSignificantDay = base::TimeDelta::FromDays(30); + int64 delta_time = (kMaxSignificantDay - + std::min((base::Time::Now() - row.last_visit()), + kMaxSignificantDay)).ToInternalValue(); + float last_visit_value = + (static_cast<float>(delta_time) / + static_cast<float>(kMaxSignificantDay.ToInternalValue())) * + kLastVisitMaxValue; + + const float kVisitCountMaxValue = 10.0; + const int kMaxSignificantVisits = 10; + float visit_count_value = + (static_cast<float>(std::min(row.visit_count(), + kMaxSignificantVisits))) / static_cast<float>(kMaxSignificantVisits) * + kVisitCountMaxValue; + float raw_score = order_value + start_value + complete_value + + last_visit_value + visit_count_value; + + // Normalize the score. + const float kMaxNormalizedRawScore = 1000.0; + raw_score = + (raw_score / (kOrderMaxValue + kStartMaxValue + kCompleteMaxValue + + kLastVisitMaxValue + kVisitCountMaxValue)) * + kMaxNormalizedRawScore; + return static_cast<int>(raw_score); +} + +// static +Time InMemoryURLIndex::RecentThreshold() { + return Time::Now() - TimeDelta::FromDays(kLowQualityMatchAgeLimitInDays); +} + +void InMemoryURLIndex::AddHistoryMatch::operator()( + const InMemoryURLIndex::HistoryID history_id) { + HistoryInfoMap::const_iterator hist_pos = + index_.history_info_map_.find(history_id); + if (hist_pos != index_.history_info_map_.end()) { + const URLRow& hist_item = hist_pos->second; + // TODO(mrossetti): Accommodate multiple term highlighting. + size_t input_location = 0; + int score = InMemoryURLIndex::RawScoreForURL( + hist_item, lower_terms_, &input_location); + if (score != 0) { + // We only retain the top 10 highest scoring results so + // see if this one fits into the top 10 and, if so, where. + ScoredHistoryMatches::iterator scored_iter = scored_matches_.begin(); + while (scored_iter != scored_matches_.end() && + (*scored_iter).raw_score > score) + ++scored_iter; + if ((scored_matches_.size() < 10) || + (scored_iter != scored_matches_.end())) { + // Create and insert the new item. + // TODO(mrossetti): Properly set |match_in_scheme| and + // |innermost_match|. + bool match_in_scheme = false; + bool innermost_match = true; + ScoredHistoryMatch match(hist_item, input_location, + match_in_scheme, innermost_match, score); + if (!scored_matches_.empty()) + scored_matches_.insert(scored_iter, match); + else + scored_matches_.push_back(match); + // Trim any entries beyond 10. + if (scored_matches_.size() > 10) { + scored_matches_.erase(scored_matches_.begin() + 10, + scored_matches_.end()); + } + } + } + } +} + } // namespace history diff --git a/chrome/browser/history/in_memory_url_index.h b/chrome/browser/history/in_memory_url_index.h index 7b57a4a..b5b3373 100644 --- a/chrome/browser/history/in_memory_url_index.h +++ b/chrome/browser/history/in_memory_url_index.h @@ -4,18 +4,238 @@ #ifndef CHROME_BROWSER_HISTORY_IN_MEMORY_URL_INDEX_H_ #define CHROME_BROWSER_HISTORY_IN_MEMORY_URL_INDEX_H_ +#pragma once + +#include <functional> +#include <map> +#include <set> +#include <string> +#include <vector> + +#include "app/sql/connection.h" +#include "base/basictypes.h" +#include "base/linked_ptr.h" +#include "base/scoped_ptr.h" +#include "base/string16.h" +#include "chrome/browser/autocomplete/history_provider_util.h" +#include "chrome/browser/history/history_types.h" +#include "testing/gtest/include/gtest/gtest_prod.h" + +namespace base { +class Time; +} namespace history { +class URLDatabase; + +// Used for intermediate history result operations. +struct ScoredHistoryMatch : public HistoryMatch { + // Required for STL, we don't use this directly. + ScoredHistoryMatch(); + + ScoredHistoryMatch(const URLRow& url_info, + size_t input_location, + bool match_in_scheme, + bool innermost_match, + int score); + + // An interim score taking into consideration location and completeness + // of the match. + int raw_score; +}; +typedef std::vector<ScoredHistoryMatch> ScoredHistoryMatches; + // The URL history source. // Holds portions of the URL database in memory in an indexed form. Used to // quickly look up matching URLs for a given query string. Used by // the HistoryURLProvider for inline autocomplete and to provide URL // matches to the omnibox. +// +// Note about multi-byte codepoints and the data structures in the +// InMemoryURLIndex class: One will quickly notice that no effort is made to +// insure that multi-byte character boundaries are detected when indexing the +// words and characters in the URL history database except when converting +// URL strings to lowercase. Multi-byte-edness makes no difference when +// indexing or when searching the index as the final filtering of results +// is dependent on the comparison of a string of bytes, not individual +// characters. While the lookup of those bytes during a search in the +// |char_word_map_| could serve up words in which the individual char16 +// occurs as a portion of a composite character the next filtering step +// will eliminate such words except in the case where a single character +// is being searched on and which character occurs as the second char16 of a +// multi-char16 instance. class InMemoryURLIndex { public: InMemoryURLIndex(); ~InMemoryURLIndex(); + + // Convenience types + typedef std::vector<string16> String16Vector; + + // Open and index the URL history database. + bool Init(URLDatabase* history_db, const std::string& languages); + + // Reset the history index. + void Reset(); + + // Given a vector containing one or more words as string16s, scan the + // history index and return a vector with all scored, matching history items. + // Each term must occur somewhere in the history item for the item to + // qualify; however, the terms do not necessarily have to be adjacent. + // Results are sorted with higher scoring items first. + ScoredHistoryMatches HistoryItemsForTerms(const String16Vector& terms); + + // Returns the date threshold for considering an history item as significant. + static base::Time RecentThreshold(); + + private: + friend class AddHistoryMatch; + friend class InMemoryURLIndexTest; + FRIEND_TEST(InMemoryURLIndexTest, Initialization); + + // Convenience types. + typedef std::set<string16> String16Set; + typedef std::set<char16> Char16Set; + + // An index into list of all of the words we have indexed. + typedef int16 WordID; + + // A map allowing a WordID to be determined given a word. + typedef std::map<string16, WordID> WordMap; + + // A map from character to word_ids. + typedef std::set<WordID> WordIDSet; // An index into the WordList. + typedef std::map<char16, WordIDSet> CharWordIDMap; + + // A map from word_id to history item. + // TODO(mrossetti): URLID is 64 bit: a memory bloat and performance hit. + // Consider using a smaller type. + typedef URLID HistoryID; + typedef std::set<HistoryID> HistoryIDSet; + typedef std::map<WordID, HistoryIDSet> WordIDHistoryMap; + + // Support caching of term character intersections so that we can optimize + // searches which build upon a previous search. + struct TermCharWordSet { + TermCharWordSet(Char16Set char_set, WordIDSet word_id_set, bool used) + : char_set_(char_set), + word_id_set_(word_id_set), + used_(used) {} + + bool IsNotUsed() const { return !used_; } + + Char16Set char_set_; + WordIDSet word_id_set_; + bool used_; // true if this set has been used for the current term search. + }; + typedef std::vector<TermCharWordSet> TermCharWordSetVector; + + // TODO(rohitrao): Probably replace this with QueryResults. + typedef std::vector<URLRow> URLRowVector; + + // A map from history_id to the history's URL and title. + typedef std::map<HistoryID, URLRow> HistoryInfoMap; + + // A helper class which performs the final filter on each candidate + // history URL match, inserting accepted matches into |scored_matches_| + // and trimming the maximum number of matches to 10. + class AddHistoryMatch : std::unary_function<HistoryID, void> { + public: + AddHistoryMatch(const InMemoryURLIndex& index, + const String16Vector& lower_terms) + : index_(index), + lower_terms_(lower_terms) {} + + void operator()(const HistoryID history_id); + + ScoredHistoryMatches ScoredMatches() const { return scored_matches_; } + + private: + const InMemoryURLIndex& index_; + ScoredHistoryMatches scored_matches_; + const String16Vector& lower_terms_; + }; + + // Break a string down into individual words. + static String16Set WordSetFromString16(const string16& uni_string); + + // Given a set of Char16s, find words containing those characters. If any + // existing, cached set is a proper subset then start with that cached + // set. Update the cache. + WordIDSet WordIDSetForTermChars(const Char16Set& uni_chars); + + // URL History indexing support functions. + + // Index one URL history item. + bool IndexRow(URLRow row); + + // Break a string down into its individual characters. + // Note that this is temporarily intended to work on a single word, but + // _will_ work on a string of words, perhaps with unexpected results. + // TODO(mrossetti): Lots of optimizations possible here for not restarting + // a search if the user is just typing along. Also, change this to uniString + // and properly handle substring matches, scoring and sorting the results + // by score. Also, provide the metrics for where the matches occur so that + // the UI can highlight the matched sections. + Char16Set CharactersFromString16(const string16& uni_word); + + // Given a single word, add a reference to the containing history item + // to the index. + void AddWordToIndex(const string16& uni_word, HistoryID history_id); + + // Update an existing entry in the word/history index by adding the + // |history_id| to set for |word_id| in the word_id_history_map_. + void UpdateWordHistory(WordID word_id, HistoryID history_id); + + // Create a new entry in the word/history map for |word_id| and add + // |history_id| as the initial element of the word's set. + void AddWordHistory(const string16& uni_word, HistoryID history_id); + + // Clear the search term cache. This cache holds on to the intermediate + // word results for each previously typed character to eliminate the need + // to re-perform set operations for previously typed characters. + void ResetTermCharWordSetCache(); + + // Compose a set of history item IDs by intersecting the set for each word + // in |uni_string|. + HistoryIDSet HistoryIDSetFromWords(const string16& uni_string); + + // Helper function to HistoryIDSetFromWords which composes a set of history + // ids for the given term given in |uni_word|. + HistoryIDSet HistoryIDsForTerm(const string16& uni_word); + + // Calculate a raw score for this history item by first determining + // if all of the terms in |terms_vector| occur in |row| and, if so, + // calculating a raw score based on 1) starting position of each term + // in the user input, 2) completeness of each term's match, 3) ordering + // of the occurrence of each term (i.e. they appear in order), 4) last + // visit time, and 5) number of visits. + // This raw score allows the results to be ordered and can be used + // to influence the final score calculated by the client of this + // index. Return the starting location of the first term in + // |first_term_location|. + static int RawScoreForURL(const URLRow& row, + const String16Vector& terms_vector, + size_t* first_term_location); + + // A list of all of indexed words. The index of a word in this list is the + // ID of the word in the word_map_. It reduces the memory overhead by + // replacing a potentially long and repeated string with a simple index. + // NOTE: A word will _never_ be removed from this vector thus insuring + // the immutability of the word_id throughout the session, reducing + // maintenance complexity. + String16Vector word_list_; + + uint64 history_item_count_; + WordMap word_map_; + CharWordIDMap char_word_map_; + WordIDHistoryMap word_id_history_map_; + TermCharWordSetVector term_char_word_set_cache_; + HistoryInfoMap history_info_map_; + std::string languages_; + + DISALLOW_COPY_AND_ASSIGN(InMemoryURLIndex); }; } // namespace history diff --git a/chrome/browser/history/in_memory_url_index_unittest.cc b/chrome/browser/history/in_memory_url_index_unittest.cc index f5932ef..9306ec5 100644 --- a/chrome/browser/history/in_memory_url_index_unittest.cc +++ b/chrome/browser/history/in_memory_url_index_unittest.cc @@ -2,15 +2,105 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "chrome/browser/history/in_memory_url_index.h" +#include <stdio.h> + +#include <fstream> +#include <string> +#include <vector> +#include "app/sql/connection.h" +#include "app/sql/statement.h" +#include "app/sql/transaction.h" +#include "base/file_util.h" +#include "base/path_service.h" #include "base/scoped_ptr.h" +#include "base/time.h" +#include "base/utf_string_conversions.h" +#include "chrome/browser/history/in_memory_url_index.h" +#include "chrome/browser/history/in_memory_database.h" +#include "chrome/common/chrome_paths.h" #include "testing/gtest/include/gtest/gtest.h" +// The test version of the history url database table ('url') is contained in +// a database file created from a text file('url_history_provider_test.db.txt'). +// The only difference between this table and a live 'urls' table from a +// profile is that the last_visit_time column in the test table contains a +// number specifying the number of days relative to 'today' to which the +// absolute time should be set during the test setup stage. +// +// The format of the test database text file is of a SQLite .dump file. +// Note that only lines whose first character is an upper-case letter are +// processed when creating the test database. + +using base::Time; +using base::TimeDelta; + namespace history { -class InMemoryURLIndexTest : public testing::Test { +class InMemoryURLIndexTest : public testing::Test, + public InMemoryDatabase { + public: + InMemoryURLIndexTest() { InitFromScratch(); } + protected: + // Test setup. + virtual void SetUp() { + // Create and populate a working copy of the URL history database. + FilePath history_proto_path; + PathService::Get(chrome::DIR_TEST_DATA, &history_proto_path); + history_proto_path = history_proto_path.Append( + FILE_PATH_LITERAL("History")); + history_proto_path = history_proto_path.Append( + FILE_PATH_LITERAL("url_history_provider_test.db.txt")); + EXPECT_TRUE(file_util::PathExists(history_proto_path)); + + std::ifstream proto_file(history_proto_path.value().c_str()); + static const size_t kCommandBufferMaxSize = 2048; + char sql_cmd_line[kCommandBufferMaxSize]; + + sql::Connection& db(GetDB()); + { + sql::Transaction transaction(&db); + transaction.Begin(); + while (!proto_file.eof()) { + proto_file.getline(sql_cmd_line, kCommandBufferMaxSize); + if (!proto_file.eof()) { + // We only process lines which begin with a upper-case letter. + // TODO(mrossetti): Can iswupper() be used here? + if (sql_cmd_line[0] >= 'A' && sql_cmd_line[0] <= 'Z') { + std::string sql_cmd(sql_cmd_line); + sql::Statement sql_stmt(db.GetUniqueStatement(sql_cmd_line)); + EXPECT_TRUE(sql_stmt.Run()); + } + } + } + transaction.Commit(); + } + proto_file.close(); + + // Update the last_visit_time table column + // such that it represents a time relative to 'now'. + sql::Statement statement(db.GetUniqueStatement( + "SELECT" HISTORY_URL_ROW_FIELDS "FROM urls;")); + EXPECT_TRUE(statement); + Time time_right_now = Time::NowFromSystemTime(); + TimeDelta day_delta = TimeDelta::FromDays(1); + { + sql::Transaction transaction(&db); + transaction.Begin(); + while (statement.Step()) { + URLRow row; + FillURLRow(statement, &row); + Time last_visit = time_right_now; + for (int64 i = row.last_visit().ToInternalValue(); i > 0; --i) + last_visit -= day_delta; + row.set_last_visit(last_visit); + UpdateURLRow(row.id(), row); + } + transaction.Commit(); + } + } + scoped_ptr<InMemoryURLIndex> url_index_; }; @@ -19,4 +109,76 @@ TEST_F(InMemoryURLIndexTest, Construction) { EXPECT_TRUE(url_index_.get()); } +// TODO(mrossetti): Write python script to calculate the validation criteria. +TEST_F(InMemoryURLIndexTest, Initialization) { + // Verify that the database contains the expected number of items, which + // is the pre-filtered count, i.e. all of the items. + sql::Statement statement(GetDB().GetUniqueStatement("SELECT * FROM urls;")); + EXPECT_TRUE(statement); + uint64 row_count = 0; + while (statement.Step()) ++row_count; + EXPECT_EQ(33U, row_count); + url_index_.reset(new InMemoryURLIndex); + url_index_->Init(this, "en,ja,hi,zh"); + + // There should have been 27 of the 31 urls accepted during filtering. + EXPECT_EQ(28U, url_index_->history_item_count_); + + // history_info_map_ should have the same number of items as were filtered. + EXPECT_EQ(28U, url_index_->history_info_map_.size()); + + // The resulting indexes should account for: + // 37 characters + // 88 words + EXPECT_EQ(37U, url_index_->char_word_map_.size()); + EXPECT_EQ(91U, url_index_->word_map_.size()); +} + +TEST_F(InMemoryURLIndexTest, Retrieval) { + url_index_.reset(new InMemoryURLIndex); + std::string languages("en,ja,hi,zh"); + url_index_->Init(this, languages); + InMemoryURLIndex::String16Vector terms; + // The term will be lowercased by the search. + + // See if a very specific term gives a single result. + string16 term = ASCIIToUTF16("DrudgeReport"); + terms.push_back(term); + ScoredHistoryMatches matches = url_index_->HistoryItemsForTerms(terms); + EXPECT_EQ(1U, matches.size()); + + // Search which should result in multiple results. + terms.clear(); + term = ASCIIToUTF16("drudge"); + terms.push_back(term); + matches = url_index_->HistoryItemsForTerms(terms); + ASSERT_EQ(2U, matches.size()); + // The results should be in descending score order. + EXPECT_GT(matches[0].raw_score, matches[1].raw_score); + + // Search which should result in nearly perfect result. + terms.clear(); + term = ASCIIToUTF16("http"); + terms.push_back(term); + term = ASCIIToUTF16("NearlyPerfectResult"); + terms.push_back(term); + matches = url_index_->HistoryItemsForTerms(terms); + ASSERT_EQ(1U, matches.size()); + // The results should have a very high score. + EXPECT_GT(matches[0].raw_score, 900); + + // Search which should result in very poor result. + terms.clear(); + term = ASCIIToUTF16("z"); + terms.push_back(term); + term = ASCIIToUTF16("y"); + terms.push_back(term); + term = ASCIIToUTF16("x"); + terms.push_back(term); + matches = url_index_->HistoryItemsForTerms(terms); + ASSERT_EQ(1U, matches.size()); + // The results should have a poor score. + EXPECT_LT(matches[0].raw_score, 200); +} + } // namespace history diff --git a/chrome/browser/history/multipart_uitest.cc b/chrome/browser/history/multipart_uitest.cc index a8fcf4c..036cd96 100644 --- a/chrome/browser/history/multipart_uitest.cc +++ b/chrome/browser/history/multipart_uitest.cc @@ -6,32 +6,39 @@ #include "app/sql/connection.h" #include "app/sql/statement.h" +#include "base/file_util.h" #include "chrome/test/automation/tab_proxy.h" #include "chrome/test/automation/browser_proxy.h" -#include "net/url_request/url_request_unittest.h" +#include "net/test/test_server.h" namespace { class MultipartResponseUITest : public UITest { }; +// http://crbug.com/50060 +#if defined(OS_MACOSX) +#define MAYBE_SingleVisit DISABLED_SingleVisit +#else +#define MAYBE_SingleVisit SingleVisit +#endif + #if defined(NDEBUG) // http://code.google.com/p/chromium/issues/detail?id=37746 // Running this test only for release builds as it fails in debug test // runs -TEST_F(MultipartResponseUITest, SingleVisit) { +TEST_F(MultipartResponseUITest, MAYBE_SingleVisit) { // Make sure that visiting a multipart/x-mixed-replace site only // creates one entry in the visits table. - const wchar_t kDocRoot[] = L"chrome/test/data"; - scoped_refptr<HTTPTestServer> server = - HTTPTestServer::CreateServer(kDocRoot, NULL); - ASSERT_TRUE(NULL != server.get()); + net::TestServer test_server(net::TestServer::TYPE_HTTP, + FilePath(FILE_PATH_LITERAL("chrome/test/data"))); + ASSERT_TRUE(test_server.Start()); scoped_refptr<BrowserProxy> browser_proxy(automation()->GetBrowserWindow(0)); ASSERT_TRUE(browser_proxy.get()); scoped_refptr<TabProxy> tab_proxy(browser_proxy->GetActiveTab()); ASSERT_TRUE(tab_proxy.get()); - NavigateToURL(server->TestServerPage("multipart")); + NavigateToURL(test_server.GetURL("multipart")); std::wstring title; EXPECT_TRUE(tab_proxy->GetTabTitle(&title)); EXPECT_EQ(L"page 9", title); @@ -47,7 +54,8 @@ TEST_F(MultipartResponseUITest, SingleVisit) { ASSERT_TRUE(db.Open(history)); std::string query( "SELECT COUNT(1) FROM visits, urls WHERE visits.url = urls.id" - " AND urls.url LIKE 'http://localhost:%/multipart'"); + " AND urls.url LIKE 'http://" + + test_server.host_port_pair().HostForURL() + ":%/multipart'"); { sql::Statement statement(db.GetUniqueStatement(query.c_str())); EXPECT_TRUE(statement); diff --git a/chrome/browser/history/page_usage_data.h b/chrome/browser/history/page_usage_data.h index 66a63e2..b7a689a 100644 --- a/chrome/browser/history/page_usage_data.h +++ b/chrome/browser/history/page_usage_data.h @@ -4,6 +4,7 @@ #ifndef CHROME_BROWSER_HISTORY_PAGE_USAGE_DATA_H__ #define CHROME_BROWSER_HISTORY_PAGE_USAGE_DATA_H__ +#pragma once #include "base/string16.h" #include "chrome/browser/history/history_types.h" diff --git a/chrome/browser/history/query_parser.h b/chrome/browser/history/query_parser.h index 8399abf..4b44c91 100644 --- a/chrome/browser/history/query_parser.h +++ b/chrome/browser/history/query_parser.h @@ -7,6 +7,7 @@ #ifndef CHROME_BROWSER_HISTORY_QUERY_PARSER_H_ #define CHROME_BROWSER_HISTORY_QUERY_PARSER_H_ +#pragma once #include <vector> diff --git a/chrome/browser/history/redirect_uitest.cc b/chrome/browser/history/redirect_uitest.cc index f7a1669..458c8e0 100644 --- a/chrome/browser/history/redirect_uitest.cc +++ b/chrome/browser/history/redirect_uitest.cc @@ -12,25 +12,35 @@ #include "base/scoped_ptr.h" #include "base/string_util.h" #include "base/string16.h" +#include "base/utf_string_conversions.h" +#include "chrome/browser/view_ids.h" +#include "chrome/test/automation/browser_proxy.h" #include "chrome/test/automation/tab_proxy.h" +#include "chrome/test/automation/window_proxy.h" #include "chrome/test/ui/ui_test.h" #include "net/base/net_util.h" -#include "net/url_request/url_request_unittest.h" +#include "net/test/test_server.h" +#include "views/event.h" namespace { -const wchar_t kDocRoot[] = L"chrome/test/data"; +class RedirectTest : public UITest { + public: + RedirectTest() + : test_server_(net::TestServer::TYPE_HTTP, + FilePath(FILE_PATH_LITERAL("chrome/test/data"))) { + } -typedef UITest RedirectTest; + protected: + net::TestServer test_server_; +}; // Tests a single server redirect TEST_F(RedirectTest, Server) { - scoped_refptr<HTTPTestServer> server = - HTTPTestServer::CreateServer(kDocRoot, NULL); - ASSERT_TRUE(NULL != server.get()); + ASSERT_TRUE(test_server_.Start()); - GURL final_url = server->TestServerPage(std::string()); - GURL first_url = server->TestServerPage( + GURL final_url = test_server_.GetURL(std::string()); + GURL first_url = test_server_.GetURL( "server-redirect?" + final_url.spec()); NavigateToURL(first_url); @@ -46,13 +56,12 @@ TEST_F(RedirectTest, Server) { } // Tests a single client redirect. -TEST_F(RedirectTest, Client) { - scoped_refptr<HTTPTestServer> server = - HTTPTestServer::CreateServer(kDocRoot, NULL); - ASSERT_TRUE(NULL != server.get()); +// Flaky: see crbug.com/55380 +TEST_F(RedirectTest, FLAKY_Client) { + ASSERT_TRUE(test_server_.Start()); - GURL final_url = server->TestServerPage(std::string()); - GURL first_url = server->TestServerPage( + GURL final_url = test_server_.GetURL(std::string()); + GURL first_url = test_server_.GetURL( "client-redirect?" + final_url.spec()); // The client redirect appears as two page visits in the browser. @@ -81,11 +90,9 @@ TEST_F(RedirectTest, Client) { } TEST_F(RedirectTest, ClientEmptyReferer) { - scoped_refptr<HTTPTestServer> server = - HTTPTestServer::CreateServer(kDocRoot, NULL); - ASSERT_TRUE(NULL != server.get()); + ASSERT_TRUE(test_server_.Start()); - GURL final_url = server->TestServerPage(std::string()); + GURL final_url = test_server_.GetURL(std::string()); FilePath test_file(test_data_directory_); test_file = test_file.AppendASCII("file_client_redirect.html"); GURL first_url = net::FilePathToFileURL(test_file); @@ -103,7 +110,17 @@ TEST_F(RedirectTest, ClientEmptyReferer) { // Tests to make sure a location change when a pending redirect exists isn't // flagged as a redirect. -TEST_F(RedirectTest, ClientCancelled) { +#if defined(OS_MACOSX) +// SimulateOSClick is broken on the Mac: http://crbug.com/45162 +#define MAYBE_ClientCancelled DISABLED_ClientCancelled +#elif defined(OS_WIN) +// http://crbug.com/53091 +#define MAYBE_ClientCancelled FAILS_ClientCancelled +#else +#define MAYBE_ClientCancelled ClientCancelled +#endif + +TEST_F(RedirectTest, MAYBE_ClientCancelled) { FilePath first_path(test_data_directory_); first_path = first_path.AppendASCII("cancelled_redirect_test.html"); ASSERT_TRUE(file_util::AbsolutePath(&first_path)); @@ -111,10 +128,26 @@ TEST_F(RedirectTest, ClientCancelled) { NavigateToURLBlockUntilNavigationsComplete(first_url, 1); - NavigateToURL(GURL("javascript:click()")); // User initiated location change. - + scoped_refptr<BrowserProxy> browser = automation()->GetBrowserWindow(0); + ASSERT_TRUE(browser.get()); + scoped_refptr<WindowProxy> window = browser->GetWindow(); + ASSERT_TRUE(window.get()); scoped_refptr<TabProxy> tab_proxy(GetActiveTab()); ASSERT_TRUE(tab_proxy.get()); + int64 last_nav_time = 0; + EXPECT_TRUE(tab_proxy->GetLastNavigationTime(&last_nav_time)); + // Simulate a click to force to make a user-initiated location change; + // otherwise, a non user-initiated in-page location change will be treated + // as client redirect and the redirect will be recoreded, which can cause + // this test failed. + gfx::Rect tab_view_bounds; + ASSERT_TRUE(browser->BringToFront()); + ASSERT_TRUE(window->GetViewBounds(VIEW_ID_TAB_CONTAINER, &tab_view_bounds, + true)); + ASSERT_TRUE( + window->SimulateOSClick(tab_view_bounds.CenterPoint(), + views::Event::EF_LEFT_BUTTON_DOWN)); + EXPECT_TRUE(tab_proxy->WaitForNavigation(last_nav_time)); std::vector<GURL> redirects; ASSERT_TRUE(tab_proxy->GetRedirectsFrom(first_url, &redirects)); @@ -141,16 +174,14 @@ TEST_F(RedirectTest, ClientCancelled) { // Tests a client->server->server redirect TEST_F(RedirectTest, ClientServerServer) { - scoped_refptr<HTTPTestServer> server = - HTTPTestServer::CreateServer(kDocRoot, NULL); - ASSERT_TRUE(NULL != server.get()); + ASSERT_TRUE(test_server_.Start()); - GURL final_url = server->TestServerPage(std::string()); - GURL next_to_last = server->TestServerPage( + GURL final_url = test_server_.GetURL(std::string()); + GURL next_to_last = test_server_.GetURL( "server-redirect?" + final_url.spec()); - GURL second_url = server->TestServerPage( + GURL second_url = test_server_.GetURL( "server-redirect?" + next_to_last.spec()); - GURL first_url = server->TestServerPage( + GURL first_url = test_server_.GetURL( "client-redirect?" + second_url.spec()); std::vector<GURL> redirects; @@ -175,14 +206,12 @@ TEST_F(RedirectTest, ClientServerServer) { // Tests that the "#reference" gets preserved across server redirects. TEST_F(RedirectTest, ServerReference) { - scoped_refptr<HTTPTestServer> server = - HTTPTestServer::CreateServer(kDocRoot, NULL); - ASSERT_TRUE(NULL != server.get()); + ASSERT_TRUE(test_server_.Start()); const std::string ref("reference"); - GURL final_url = server->TestServerPage(std::string()); - GURL initial_url = server->TestServerPage( + GURL final_url = test_server_.GetURL(std::string()); + GURL initial_url = test_server_.GetURL( "server-redirect?" + final_url.spec() + "#" + ref); NavigateToURL(initial_url); @@ -195,14 +224,12 @@ TEST_F(RedirectTest, ServerReference) { // A) does not crash the browser or confuse the redirect chain, see bug 1080873 // B) does not take place. TEST_F(RedirectTest, NoHttpToFile) { - scoped_refptr<HTTPTestServer> server = - HTTPTestServer::CreateServer(kDocRoot, NULL); - ASSERT_TRUE(NULL != server.get()); + ASSERT_TRUE(test_server_.Start()); FilePath test_file(test_data_directory_); test_file = test_file.AppendASCII("http_to_file.html"); GURL file_url = net::FilePathToFileURL(test_file); - GURL initial_url = server->TestServerPage( + GURL initial_url = test_server_.GetURL( "client-redirect?" + file_url.spec()); NavigateToURL(initial_url); @@ -218,9 +245,7 @@ TEST_F(RedirectTest, NoHttpToFile) { // Ensures that non-user initiated location changes (within page) are // flagged as client redirects. See bug 1139823. TEST_F(RedirectTest, ClientFragments) { - scoped_refptr<HTTPTestServer> server = - HTTPTestServer::CreateServer(kDocRoot, NULL); - ASSERT_TRUE(NULL != server.get()); + ASSERT_TRUE(test_server_.Start()); FilePath test_file(test_data_directory_); test_file = test_file.AppendASCII("ref_redirect.html"); @@ -252,13 +277,11 @@ TEST_F(RedirectTest, // which causes it to start a provisional load, and while it is waiting // for the response (which means it hasn't committed the load for the client // redirect destination page yet), we issue a new navigation request. - scoped_refptr<HTTPTestServer> server = - HTTPTestServer::CreateServer(kDocRoot, NULL); - ASSERT_TRUE(NULL != server.get()); + ASSERT_TRUE(test_server_.Start()); - GURL final_url = server->TestServerPage("files/title2.html"); - GURL slow = server->TestServerPage("slow?60"); - GURL first_url = server->TestServerPage( + GURL final_url = test_server_.GetURL("files/title2.html"); + GURL slow = test_server_.GetURL("slow?60"); + GURL first_url = test_server_.GetURL( "client-redirect?" + slow.spec()); std::vector<GURL> redirects; diff --git a/chrome/browser/history/snippet.cc b/chrome/browser/history/snippet.cc index cb96e16..6e3e93c 100644 --- a/chrome/browser/history/snippet.cc +++ b/chrome/browser/history/snippet.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. @@ -8,6 +8,7 @@ #include "base/logging.h" #include "base/scoped_ptr.h" +#include "base/string_split.h" #include "base/string_util.h" #include "base/utf_string_conversions.h" #include "unicode/brkiter.h" diff --git a/chrome/browser/history/snippet.h b/chrome/browser/history/snippet.h index 9e92893..ffa8fa1 100644 --- a/chrome/browser/history/snippet.h +++ b/chrome/browser/history/snippet.h @@ -7,6 +7,7 @@ #ifndef CHROME_BROWSER_HISTORY_SNIPPET_H__ #define CHROME_BROWSER_HISTORY_SNIPPET_H__ +#pragma once #include <vector> diff --git a/chrome/browser/history/snippet_unittest.cc b/chrome/browser/history/snippet_unittest.cc index 5bc8a3b..75f627a 100644 --- a/chrome/browser/history/snippet_unittest.cc +++ b/chrome/browser/history/snippet_unittest.cc @@ -6,6 +6,7 @@ #include <algorithm> +#include "base/string_split.h" #include "base/string_util.h" #include "base/utf_string_conversions.h" #include "testing/gtest/include/gtest/gtest.h" @@ -162,11 +163,7 @@ TEST(Snippets, UTF8) { UTF16ToUTF8(BuildSnippet(kSampleDocument, "relationship"))); } -// Bug: 1274923 -// TODO(jungshik): Move this bug report to crbugs.com -// Fails consistently. From the report, "Broken by latest ICU. Need new expected -// results." -TEST(Snippets, FAILS_ThaiUTF8) { +TEST(Snippets, ThaiUTF8) { // There are 3 instances of '\u0E43\u0E2B\u0E49' // (\xE0\xB9\x83\xE0\xB8\xAB\xE0\xB9\x89) in kThaiSample. // The 1st is more than |kSniipetContext| graphemes away from the @@ -174,53 +171,54 @@ TEST(Snippets, FAILS_ThaiUTF8) { // the 2nd match added, the snippet goes over the size limit so that // the snippet ends right before the 3rd match. ASSERT_EQ(" ... " - " \xE0\xB8\x82\xE0\xB9\x89\xE0\xB8\xAD\xE0\xB8\xA1\xE0\xB8\xB9" - "\xE0\xB8\xA5\xE0\xB8\xAA\xE0\xB9\x88\xE0\xB8\xA7\xE0\xB8\x99" - "\xE0\xB8\x9A\xE0\xB8\xB8\xE0\xB8\x84\xE0\xB8\x84\xE0\xB8\xA5 " - "\xE0\xB9\x80\xE0\xB8\xA1\xE0\xB8\xB7\xE0\xB9\x88\xE0\xB8\xAD" - "\xE0\xB8\x84\xE0\xB8\xB8\xE0\xB8\x93\xE0\xB8\xA5\xE0\xB8\x87" - "\xE0\xB8\x97\xE0\xB8\xB0\xE0\xB9\x80\xE0\xB8\x9A\xE0\xB8\xB5" - "\xE0\xB8\xA2\xE0\xB8\x99\xE0\xB9\x80\xE0\xB8\x9E\xE0\xB8\xB7" - "\xE0\xB9\x88\xE0\xB8\xAD\xE0\xB9\x83\xE0\xB8\x8A\xE0\xB9\x89" - "\xE0\xB8\x9A\xE0\xB8\xA3\xE0\xB8\xB4\xE0\xB8\x81\xE0\xB8\xB2" - "\xE0\xB8\xA3\xE0\xB8\x82\xE0\xB8\xAD\xE0\xB8\x87 Google " - "\xE0\xB8\xAB\xE0\xB8\xA3\xE0\xB8\xB7\xE0\xB8\xAD**\xE0\xB9\x83" - "\xE0\xB8\xAB\xE0\xB9\x89**\xE0\xB8\x82\xE0\xB9\x89\xE0\xB8\xAD" - "\xE0\xB8\xA1\xE0\xB8\xB9\xE0\xB8\xA5\xE0\xB8\x94\xE0\xB8\xB1" - "\xE0\xB8\x87\xE0\xB8\x81\xE0\xB8\xA5\xE0\xB9\x88\xE0\xB8\xB2" - "\xE0\xB8\xA7\xE0\xB9\x82\xE0\xB8\x94\xE0\xB8\xA2\xE0\xB8\xAA" - "\xE0\xB8\xA1\xE0\xB8\xB1\xE0\xB8\x84\xE0\xB8\xA3\xE0\xB9\x83" - "\xE0\xB8\x88 \xE0\xB9\x80\xE0\xB8\xA3\xE0\xB8\xB2\xE0\xB8\xAD" - "\xE0\xB8\xB2\xE0\xB8\x88\xE0\xB8\xA3\xE0\xB8\xA7\xE0\xB8\xA1" - "\xE0\xB8\x82\xE0\xB9\x89\xE0\xB8\xAD\xE0\xB8\xA1\xE0\xB8\xB9" - "\xE0\xB8\xA5\xE0\xB8\xAA\xE0\xB9\x88\xE0\xB8\xA7\xE0\xB8\x99" - "\xE0\xB8\x9A\xE0\xB8\xB8\xE0\xB8\x84\xE0\xB8\x84\xE0\xB8\xA5" - "\xE0\xB8\x97\xE0\xB8\xB5\xE0\xB9\x88\xE0\xB9\x80\xE0\xB8\x81" - "\xE0\xB9\x87\xE0\xB8\x9A\xE0\xB8\xA3\xE0\xB8\xA7\xE0\xB8\x9A" - "\xE0\xB8\xA3\xE0\xB8\xA7\xE0\xB8\xA1 ... ... " + "\xE0\xB8\xA3\xE0\xB8\xA7\xE0\xB8\x9A\xE0\xB8\xA3\xE0\xB8\xA7" + "\xE0\xB8\xA1 \xE0\xB8\x82\xE0\xB9\x89\xE0\xB8\xAD\xE0\xB8" + "\xA1\xE0\xB8\xB9\xE0\xB8\xA5\xE0\xB8\xAA\xE0\xB9\x88\xE0\xB8" + "\xA7\xE0\xB8\x99\xE0\xB8\x9A\xE0\xB8\xB8\xE0\xB8\x84\xE0\xB8" + "\x84\xE0\xB8\xA5 \xE0\xB9\x80\xE0\xB8\xA1\xE0\xB8\xB7\xE0" + "\xB9\x88\xE0\xB8\xAD\xE0\xB8\x84\xE0\xB8\xB8\xE0\xB8\x93\xE0" + "\xB8\xA5\xE0\xB8\x87\xE0\xB8\x97\xE0\xB8\xB0\xE0\xB9\x80\xE0" + "\xB8\x9A\xE0\xB8\xB5\xE0\xB8\xA2\xE0\xB8\x99\xE0\xB9\x80\xE0" + "\xB8\x9E\xE0\xB8\xB7\xE0\xB9\x88\xE0\xB8\xAD\xE0\xB9\x83\xE0" + "\xB8\x8A\xE0\xB9\x89\xE0\xB8\x9A\xE0\xB8\xA3\xE0\xB8\xB4\xE0" + "\xB8\x81\xE0\xB8\xB2\xE0\xB8\xA3\xE0\xB8\x82\xE0\xB8\xAD\xE0" + "\xB8\x87 Google \xE0\xB8\xAB\xE0\xB8\xA3\xE0\xB8\xB7\xE0\xB8" + "\xAD**\xE0\xB9\x83\xE0\xB8\xAB\xE0\xB9\x89**\xE0\xB8\x82\xE0" + "\xB9\x89\xE0\xB8\xAD\xE0\xB8\xA1\xE0\xB8\xB9\xE0\xB8\xA5\xE0" + "\xB8\x94\xE0\xB8\xB1\xE0\xB8\x87\xE0\xB8\x81\xE0\xB8\xA5\xE0" + "\xB9\x88\xE0\xB8\xB2\xE0\xB8\xA7\xE0\xB9\x82\xE0\xB8\x94\xE0" + "\xB8\xA2\xE0\xB8\xAA\xE0\xB8\xA1\xE0\xB8\xB1\xE0\xB8\x84\xE0" + "\xB8\xA3\xE0\xB9\x83\xE0\xB8\x88 \xE0\xB9\x80\xE0\xB8\xA3" + "\xE0\xB8\xB2\xE0\xB8\xAD\xE0\xB8\xB2\xE0\xB8\x88\xE0\xB8\xA3" + "\xE0\xB8\xA7\xE0\xB8\xA1\xE0\xB8\x82\xE0\xB9\x89\xE0\xB8\xAD" + "\xE0\xB8\xA1\xE0\xB8\xB9\xE0\xB8\xA5\xE0\xB8\xAA\xE0\xB9\x88" + "\xE0\xB8\xA7\xE0\xB8\x99\xE0\xB8\x9A\xE0\xB8\xB8\xE0\xB8\x84" + "\xE0\xB8\x84\xE0\xB8\xA5\xE0\xB8\x97\xE0\xB8\xB5\xE0\xB9\x88" + "\xE0\xB9\x80\xE0\xB8\x81\xE0\xB9\x87\xE0\xB8\x9A\xE0\xB8\xA3" + "\xE0\xB8\xA7\xE0\xB8\x9A\xE0\xB8\xA3\xE0\xB8\xA7\xE0\xB8\xA1" "\xE0\xB8\x88\xE0\xB8\xB2\xE0\xB8\x81\xE0\xB8\x84\xE0\xB8\xB8" "\xE0\xB8\x93\xE0\xB9\x80\xE0\xB8\x82\xE0\xB9\x89\xE0\xB8\xB2" - "\xE0\xB8\x81\xE0\xB8\xB1\xE0\xB8\x9A\xE0\xB8\x82\xE0\xB9\x89" - "\xE0\xB8\xAD\xE0\xB8\xA1\xE0\xB8\xB9\xE0\xB8\xA5\xE0\xB8\x88" - "\xE0\xB8\xB2\xE0\xB8\x81\xE0\xB8\x9A\xE0\xB8\xA3\xE0\xB8\xB4" - "\xE0\xB8\x81\xE0\xB8\xB2\xE0\xB8\xA3\xE0\xB8\xAD\xE0\xB8\xB7" - "\xE0\xB9\x88\xE0\xB8\x99\xE0\xB8\x82\xE0\xB8\xAD\xE0\xB8\x87 " - "Google \xE0\xB8\xAB\xE0\xB8\xA3\xE0\xB8\xB7\xE0\xB8\xAD" - "\xE0\xB8\x9A\xE0\xB8\xB8\xE0\xB8\x84\xE0\xB8\x84\xE0\xB8\xA5" - "\xE0\xB8\x97\xE0\xB8\xB5\xE0\xB9\x88\xE0\xB8\xAA\xE0\xB8\xB2" - "\xE0\xB8\xA1 \xE0\xB9\x80\xE0\xB8\x9E\xE0\xB8\xB7\xE0\xB9\x88" - "\xE0\xB8\xAD**\xE0\xB9\x83\xE0\xB8\xAB\xE0\xB9\x89**\xE0\xB8\x9C" - "\xE0\xB8\xB9\xE0\xB9\x89\xE0\xB9\x83\xE0\xB8\x8A\xE0\xB9\x89" - "\xE0\xB9\x84\xE0\xB8\x94\xE0\xB9\x89\xE0\xB8\xA3\xE0\xB8\xB1" - "\xE0\xB8\x9A\xE0\xB8\x9B\xE0\xB8\xA3\xE0\xB8\xB0\xE0\xB8\xAA" - "\xE0\xB8\x9A\xE0\xB8\x81\xE0\xB8\xB2\xE0\xB8\xA3\xE0\xB8\x93" - "\xE0\xB9\x8C\xE0\xB8\x97\xE0\xB8\xB5\xE0\xB9\x88\xE0\xB8\x94" - "\xE0\xB8\xB5\xE0\xB8\x82\xE0\xB8\xB6\xE0\xB9\x89\xE0\xB8\x99 " - "\xE0\xB8\xA3\xE0\xB8\xA7\xE0\xB8\xA1\xE0\xB8\x97\xE0\xB8\xB1" - "\xE0\xB9\x89\xE0\xB8\x87\xE0\xB8\x9B\xE0\xB8\xA3\xE0\xB8\xB1" - "\xE0\xB8\x9A\xE0\xB9\x81\xE0\xB8\x95\xE0\xB9\x88\xE0\xB8\x87" - "\xE0\xB9\x80\xE0\xB8\x99\xE0\xB8\xB7\xE0\xB9\x89\xE0\xB8\xAD" - "\xE0\xB8\xAB\xE0\xB8\xB2", + "\xE0\xB8\x81\xE0\xB8\xB1\xE0\xB8\x9A ... ... \xE0\xB8\x82" + "\xE0\xB9\x89\xE0\xB8\xAD\xE0\xB8\xA1\xE0\xB8\xB9\xE0\xB8\xA5" + "\xE0\xB8\x88\xE0\xB8\xB2\xE0\xB8\x81\xE0\xB8\x9A\xE0\xB8\xA3" + "\xE0\xB8\xB4\xE0\xB8\x81\xE0\xB8\xB2\xE0\xB8\xA3\xE0\xB8\xAD" + "\xE0\xB8\xB7\xE0\xB9\x88\xE0\xB8\x99\xE0\xB8\x82\xE0\xB8\xAD" + "\xE0\xB8\x87 Google \xE0\xB8\xAB\xE0\xB8\xA3\xE0\xB8\xB7\xE0" + "\xB8\xAD\xE0\xB8\x9A\xE0\xB8\xB8\xE0\xB8\x84\xE0\xB8\x84\xE0" + "\xB8\xA5\xE0\xB8\x97\xE0\xB8\xB5\xE0\xB9\x88\xE0\xB8\xAA\xE0" + "\xB8\xB2\xE0\xB8\xA1 \xE0\xB9\x80\xE0\xB8\x9E\xE0\xB8\xB7" + "\xE0\xB9\x88\xE0\xB8\xAD**\xE0\xB9\x83\xE0\xB8\xAB\xE0\xB9" + "\x89**\xE0\xB8\x9C\xE0\xB8\xB9\xE0\xB9\x89\xE0\xB9\x83\xE0" + "\xB8\x8A\xE0\xB9\x89\xE0\xB9\x84\xE0\xB8\x94\xE0\xB9\x89\xE0" + "\xB8\xA3\xE0\xB8\xB1\xE0\xB8\x9A\xE0\xB8\x9B\xE0\xB8\xA3\xE0" + "\xB8\xB0\xE0\xB8\xAA\xE0\xB8\x9A\xE0\xB8\x81\xE0\xB8\xB2\xE0" + "\xB8\xA3\xE0\xB8\x93\xE0\xB9\x8C\xE0\xB8\x97\xE0\xB8\xB5\xE0" + "\xB9\x88\xE0\xB8\x94\xE0\xB8\xB5\xE0\xB8\x82\xE0\xB8\xB6\xE0" + "\xB9\x89\xE0\xB8\x99 \xE0\xB8\xA3\xE0\xB8\xA7\xE0\xB8\xA1" + "\xE0\xB8\x97\xE0\xB8\xB1\xE0\xB9\x89\xE0\xB8\x87\xE0\xB8\x9B" + "\xE0\xB8\xA3\xE0\xB8\xB1\xE0\xB8\x9A\xE0\xB9\x81\xE0\xB8\x95" + "\xE0\xB9\x88\xE0\xB8\x87\xE0\xB9\x80\xE0\xB8\x99\xE0\xB8\xB7" + "\xE0\xB9\x89\xE0\xB8\xAD\xE0\xB8\xAB\xE0\xB8\xB2", UTF16ToUTF8(BuildSnippet(kThaiSample, "\xE0\xB9\x83\xE0\xB8\xAB\xE0\xB9\x89"))); } diff --git a/chrome/browser/history/starred_url_database.cc b/chrome/browser/history/starred_url_database.cc index cf2a306..ba8b27d 100644 --- a/chrome/browser/history/starred_url_database.cc +++ b/chrome/browser/history/starred_url_database.cc @@ -4,7 +4,6 @@ #include "chrome/browser/history/starred_url_database.h" -#include "app/sql/connection.h" #include "app/sql/statement.h" #include "base/file_util.h" #include "base/logging.h" @@ -12,6 +11,7 @@ #include "base/scoped_vector.h" #include "base/stl_util-inl.h" #include "base/string_util.h" +#include "base/utf_string_conversions.h" #include "base/values.h" #include "chrome/browser/bookmarks/bookmark_codec.h" #include "chrome/browser/bookmarks/bookmark_model.h" diff --git a/chrome/browser/history/starred_url_database.h b/chrome/browser/history/starred_url_database.h index 8d327d8..aab2ae5 100644 --- a/chrome/browser/history/starred_url_database.h +++ b/chrome/browser/history/starred_url_database.h @@ -4,6 +4,7 @@ #ifndef CHROME_BROWSER_HISTORY_STARRED_URL_DATABASE_H_ #define CHROME_BROWSER_HISTORY_STARRED_URL_DATABASE_H_ +#pragma once #include <set> diff --git a/chrome/browser/history/starred_url_database_unittest.cc b/chrome/browser/history/starred_url_database_unittest.cc index f82e645..deb9750 100644 --- a/chrome/browser/history/starred_url_database_unittest.cc +++ b/chrome/browser/history/starred_url_database_unittest.cc @@ -7,6 +7,7 @@ #include "base/file_util.h" #include "base/path_service.h" #include "base/string_util.h" +#include "base/utf_string_conversions.h" #include "chrome/browser/history/history.h" #include "chrome/browser/history/starred_url_database.h" #include "chrome/common/chrome_paths.h" diff --git a/chrome/browser/history/text_database.cc b/chrome/browser/history/text_database.cc index 3327869..71c801f 100644 --- a/chrome/browser/history/text_database.cc +++ b/chrome/browser/history/text_database.cc @@ -8,12 +8,12 @@ #include "chrome/browser/history/text_database.h" -#include "app/sql/connection.h" #include "app/sql/statement.h" #include "app/sql/transaction.h" #include "base/file_util.h" #include "base/histogram.h" #include "base/logging.h" +#include "base/string_number_conversions.h" #include "base/string_util.h" #include "base/utf_string_conversions.h" #include "chrome/browser/diagnostics/sqlite_diagnostics.h" @@ -107,8 +107,9 @@ TextDatabase::DBIdent TextDatabase::FileNameToID(const FilePath& file_path) { return 0; } - int year = StringToInt(suffix.substr(0, 4)); - int month = StringToInt(suffix.substr(5, 2)); + int year, month; + base::StringToInt(suffix.substr(0, 4), &year); + base::StringToInt(suffix.substr(5, 2), &month); return year * 100 + month; } @@ -127,7 +128,7 @@ bool TextDatabase::Init() { // better performance (we're typically seek rather than bandwidth limited). // This only has an effect before any tables have been created, otherwise // this is a NOP. Must be a power of 2 and a max of 8192. - db_.set_page_size(2096); + db_.set_page_size(4096); // The default cache size is 2000 which give >8MB of data. Since we will often // have 2-3 of these objects, each with their own 8MB, this adds up very fast. diff --git a/chrome/browser/history/text_database.h b/chrome/browser/history/text_database.h index e34c071..e881888 100644 --- a/chrome/browser/history/text_database.h +++ b/chrome/browser/history/text_database.h @@ -4,6 +4,7 @@ #ifndef CHROME_BROWSER_HISTORY_TEXT_DATABASE_H_ #define CHROME_BROWSER_HISTORY_TEXT_DATABASE_H_ +#pragma once #include <set> #include <vector> diff --git a/chrome/browser/history/text_database_manager.h b/chrome/browser/history/text_database_manager.h index 7f25bf7..5f8d1a3 100644 --- a/chrome/browser/history/text_database_manager.h +++ b/chrome/browser/history/text_database_manager.h @@ -4,6 +4,7 @@ #ifndef CHROME_BROWSER_HISTORY_TEXT_DATABASE_MANAGER_H_ #define CHROME_BROWSER_HISTORY_TEXT_DATABASE_MANAGER_H_ +#pragma once #include <set> #include <vector> @@ -275,7 +276,7 @@ class TextDatabaseManager { typedef OwningMRUCache<TextDatabase::DBIdent, TextDatabase*> DBCache; DBCache db_cache_; - // Tells us about the existance of database files on disk. All existing + // Tells us about the existence of database files on disk. All existing // databases will be in here, and non-existant ones will not, so we don't // have to check the disk every time. // diff --git a/chrome/browser/history/text_database_manager_unittest.cc b/chrome/browser/history/text_database_manager_unittest.cc index 8e7f27e..1ec7744 100644 --- a/chrome/browser/history/text_database_manager_unittest.cc +++ b/chrome/browser/history/text_database_manager_unittest.cc @@ -79,7 +79,7 @@ void AddAllPages(TextDatabaseManager& manager, VisitDatabase* visit_db, visit_row.transition = 0; visit_row.segment_id = 0; visit_row.is_indexed = false; - VisitID visit_id = visit_db->AddVisit(&visit_row); + VisitID visit_id = visit_db->AddVisit(&visit_row, SOURCE_BROWSED); times->push_back(visit_row.visit_time); manager.AddPageData(GURL(kURL1), visit_row.url_id, visit_row.visit_id, @@ -89,7 +89,7 @@ void AddAllPages(TextDatabaseManager& manager, VisitDatabase* visit_db, exploded.day_of_month++; visit_row.url_id = 2; visit_row.visit_time = Time::FromUTCExploded(exploded); - visit_id = visit_db->AddVisit(&visit_row); + visit_id = visit_db->AddVisit(&visit_row, SOURCE_BROWSED); times->push_back(visit_row.visit_time); manager.AddPageData(GURL(kURL2), visit_row.url_id, visit_row.visit_id, visit_row.visit_time, UTF8ToUTF16(kTitle2), @@ -98,7 +98,7 @@ void AddAllPages(TextDatabaseManager& manager, VisitDatabase* visit_db, exploded.day_of_month++; visit_row.url_id = 2; visit_row.visit_time = Time::FromUTCExploded(exploded); - visit_id = visit_db->AddVisit(&visit_row); + visit_id = visit_db->AddVisit(&visit_row, SOURCE_BROWSED); times->push_back(visit_row.visit_time); manager.AddPageData(GURL(kURL3), visit_row.url_id, visit_row.visit_id, visit_row.visit_time, UTF8ToUTF16(kTitle3), @@ -108,7 +108,7 @@ void AddAllPages(TextDatabaseManager& manager, VisitDatabase* visit_db, exploded.month++; visit_row.url_id = 2; visit_row.visit_time = Time::FromUTCExploded(exploded); - visit_id = visit_db->AddVisit(&visit_row); + visit_id = visit_db->AddVisit(&visit_row, SOURCE_BROWSED); times->push_back(visit_row.visit_time); manager.AddPageData(GURL(kURL4), visit_row.url_id, visit_row.visit_id, visit_row.visit_time, UTF8ToUTF16(kTitle4), @@ -117,7 +117,7 @@ void AddAllPages(TextDatabaseManager& manager, VisitDatabase* visit_db, exploded.day_of_month++; visit_row.url_id = 2; visit_row.visit_time = Time::FromUTCExploded(exploded); - visit_id = visit_db->AddVisit(&visit_row); + visit_id = visit_db->AddVisit(&visit_row, SOURCE_BROWSED); times->push_back(visit_row.visit_time); manager.AddPageData(GURL(kURL5), visit_row.url_id, visit_row.visit_id, visit_row.visit_time, UTF8ToUTF16(kTitle5), @@ -127,7 +127,7 @@ void AddAllPages(TextDatabaseManager& manager, VisitDatabase* visit_db, exploded.day_of_month++; visit_row.url_id = 2; visit_row.visit_time = Time::FromUTCExploded(exploded); - visit_id = visit_db->AddVisit(&visit_row); + visit_id = visit_db->AddVisit(&visit_row, SOURCE_BROWSED); times->push_back(visit_row.visit_time); manager.AddPageData(GURL(kURL1), visit_row.url_id, visit_row.visit_id, visit_row.visit_time, UTF8ToUTF16(kTitle1), @@ -242,7 +242,7 @@ TEST_F(TextDatabaseManagerTest, InsertCompleteVisit) { visit.transition = PageTransition::LINK; visit.segment_id = 0; visit.is_indexed = false; - visit_db.AddVisit(&visit); + visit_db.AddVisit(&visit, SOURCE_BROWSED); // Add a full text indexed entry for that visit. const GURL url(kURL2); @@ -335,7 +335,7 @@ TEST_F(TextDatabaseManagerTest, PartialComplete) { VisitRow visit_row; visit_row.url_id = url_id; visit_row.visit_time = added_time; - visit_db.AddVisit(&visit_row); + visit_db.AddVisit(&visit_row, SOURCE_BROWSED); // Add a URL with no title or body, and say that it expired. manager.AddPageURL(url, 0, 0, added_time); diff --git a/chrome/browser/history/thumbnail_database.cc b/chrome/browser/history/thumbnail_database.cc index 8bf203d..36b8a20 100644 --- a/chrome/browser/history/thumbnail_database.cc +++ b/chrome/browser/history/thumbnail_database.cc @@ -8,21 +8,23 @@ #include "app/sql/transaction.h" #include "base/command_line.h" #include "base/file_util.h" -#if defined(OS_MACOSX) -#include "base/mac_util.h" -#endif #include "base/ref_counted_memory.h" #include "base/time.h" #include "base/string_util.h" #include "base/utf_string_conversions.h" #include "chrome/browser/diagnostics/sqlite_diagnostics.h" #include "chrome/browser/history/history_publisher.h" +#include "chrome/browser/history/top_sites.h" #include "chrome/browser/history/url_database.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/thumbnail_score.h" #include "gfx/codec/jpeg_codec.h" #include "third_party/skia/include/core/SkBitmap.h" +#if defined(OS_MACOSX) +#include "base/mac_util.h" +#endif + namespace history { // Version number of the database. @@ -130,7 +132,7 @@ sql::InitStatus ThumbnailDatabase::OpenDatabase(sql::Connection* db, bool ThumbnailDatabase::InitThumbnailTable() { if (!db_.DoesTableExist("thumbnails")) { - if (CommandLine::ForCurrentProcess()-> HasSwitch(switches::kTopSites)) { + if (history::TopSites::IsEnabled()) { use_top_sites_ = true; return true; } @@ -231,8 +233,10 @@ void ThumbnailDatabase::SetPageThumbnail( const SkBitmap& thumbnail, const ThumbnailScore& score, base::Time time) { - if (use_top_sites_) + if (use_top_sites_) { + LOG(WARNING) << "Use TopSites instead."; return; // Not possible after migration to TopSites. + } if (!thumbnail.isNull()) { bool add_thumbnail = true; @@ -286,8 +290,10 @@ void ThumbnailDatabase::SetPageThumbnail( bool ThumbnailDatabase::GetPageThumbnail(URLID id, std::vector<unsigned char>* data) { - if (use_top_sites_) + if (use_top_sites_) { + LOG(WARNING) << "Use TopSites instead."; return false; // Not possible after migration to TopSites. + } sql::Statement statement(db_.GetCachedStatement(SQL_FROM_HERE, "SELECT data FROM thumbnails WHERE url_id=?")); @@ -303,8 +309,10 @@ bool ThumbnailDatabase::GetPageThumbnail(URLID id, } bool ThumbnailDatabase::DeleteThumbnail(URLID id) { - if (use_top_sites_) + if (use_top_sites_) { + LOG(WARNING) << "Use TopSites instead."; return true; // Not possible after migration to TopSites. + } sql::Statement statement(db_.GetCachedStatement(SQL_FROM_HERE, "DELETE FROM thumbnails WHERE url_id = ?")); @@ -317,8 +325,10 @@ bool ThumbnailDatabase::DeleteThumbnail(URLID id) { bool ThumbnailDatabase::ThumbnailScoreForId(URLID id, ThumbnailScore* score) { - if (use_top_sites_) + if (use_top_sites_) { + LOG(WARNING) << "Use TopSites instead."; return false; // Not possible after migration to TopSites. + } // Fetch the current thumbnail's information to make sure we // aren't replacing a good thumbnail with one that's worse. @@ -491,7 +501,8 @@ bool ThumbnailDatabase::RenameAndDropThumbnails(const FilePath& old_db_file, favicons.Close(); // Can't attach within a transaction. - CommitTransaction(); + if (transaction_nesting()) + CommitTransaction(); // Attach new DB. { diff --git a/chrome/browser/history/thumbnail_database.h b/chrome/browser/history/thumbnail_database.h index 81d498c..94fb043 100644 --- a/chrome/browser/history/thumbnail_database.h +++ b/chrome/browser/history/thumbnail_database.h @@ -4,6 +4,7 @@ #ifndef CHROME_BROWSER_HISTORY_THUMBNAIL_DATABASE_H_ #define CHROME_BROWSER_HISTORY_THUMBNAIL_DATABASE_H_ +#pragma once #include <vector> diff --git a/chrome/browser/history/thumbnail_database_unittest.cc b/chrome/browser/history/thumbnail_database_unittest.cc index 4d2c2bf..23eccef 100644 --- a/chrome/browser/history/thumbnail_database_unittest.cc +++ b/chrome/browser/history/thumbnail_database_unittest.cc @@ -6,6 +6,7 @@ #include <vector> #include "base/basictypes.h" +#include "base/command_line.h" #include "base/file_path.h" #include "base/file_util.h" #include "base/path_service.h" @@ -13,6 +14,8 @@ #include "base/scoped_temp_dir.h" #include "chrome/browser/history/thumbnail_database.h" #include "chrome/common/chrome_paths.h" +#include "chrome/common/chrome_switches.h" +#include "chrome/browser/history/top_sites.h" #include "chrome/common/thumbnail_score.h" #include "chrome/tools/profiles/thumbnail-inl.h" #include "gfx/codec/jpeg_codec.h" @@ -70,6 +73,9 @@ class ThumbnailDatabaseTest : public testing::Test { }; TEST_F(ThumbnailDatabaseTest, AddDelete) { + if (history::TopSites::IsEnabled()) + return; // TopSitesTest replaces this. + ThumbnailDatabase db; ASSERT_EQ(sql::INIT_OK, db.Init(file_name_, NULL)); @@ -112,6 +118,9 @@ TEST_F(ThumbnailDatabaseTest, AddDelete) { } TEST_F(ThumbnailDatabaseTest, UseLessBoringThumbnails) { + if (history::TopSites::IsEnabled()) + return; // TopSitesTest replaces this. + ThumbnailDatabase db; Time now = Time::Now(); ASSERT_EQ(sql::INIT_OK, db.Init(file_name_, NULL)); @@ -146,6 +155,9 @@ TEST_F(ThumbnailDatabaseTest, UseLessBoringThumbnails) { } TEST_F(ThumbnailDatabaseTest, UseAtTopThumbnails) { + if (history::TopSites::IsEnabled()) + return; // TopSitesTest replaces this. + ThumbnailDatabase db; Time now = Time::Now(); ASSERT_EQ(sql::INIT_OK, db.Init(file_name_, NULL)); @@ -217,6 +229,9 @@ TEST_F(ThumbnailDatabaseTest, UseAtTopThumbnails) { } TEST_F(ThumbnailDatabaseTest, ThumbnailTimeDegradation) { + if (history::TopSites::IsEnabled()) + return; // TopSitesTest replaces this. + ThumbnailDatabase db; const Time kNow = Time::Now(); const Time kThreeHoursAgo = kNow - TimeDelta::FromHours(4); @@ -261,6 +276,9 @@ TEST_F(ThumbnailDatabaseTest, NeverAcceptTotallyBoringThumbnail) { // should replace a thumbnail with another because of reasons other // than straight up boringness score, still reject because the // thumbnail is totally boring. + if (history::TopSites::IsEnabled()) + return; // TopSitesTest replaces this. + ThumbnailDatabase db; Time now = Time::Now(); ASSERT_EQ(sql::INIT_OK, db.Init(file_name_, NULL)); @@ -332,6 +350,9 @@ TEST_F(ThumbnailDatabaseTest, NeverAcceptTotallyBoringThumbnail) { } TEST_F(ThumbnailDatabaseTest, NeedsMigrationToTopSites) { + if (history::TopSites::IsEnabled()) + return; // TopSitesTest replaces this. + ThumbnailDatabase db; ASSERT_EQ(sql::INIT_OK, db.Init(file_name_, NULL)); db.BeginTransaction(); diff --git a/chrome/browser/history/top_sites.cc b/chrome/browser/history/top_sites.cc index 1ef4dec..c9f4a94 100644 --- a/chrome/browser/history/top_sites.cc +++ b/chrome/browser/history/top_sites.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. @@ -6,22 +6,37 @@ #include <algorithm> +#include "app/l10n_util.h" +#include "base/command_line.h" #include "base/file_util.h" #include "base/logging.h" +#include "base/md5.h" +#include "base/string_util.h" +#include "base/utf_string_conversions.h" +#include "base/values.h" #include "chrome/browser/chrome_thread.h" -#include "chrome/browser/profile.h" -#include "chrome/browser/history/top_sites_database.h" +#include "chrome/browser/dom_ui/most_visited_handler.h" #include "chrome/browser/history/history_notifications.h" #include "chrome/browser/history/page_usage_data.h" +#include "chrome/browser/history/top_sites_database.h" +#include "chrome/browser/prefs/pref_service.h" +#include "chrome/browser/profile.h" #include "chrome/browser/tab_contents/navigation_controller.h" #include "chrome/browser/tab_contents/navigation_entry.h" +#include "chrome/common/chrome_switches.h" +#include "chrome/common/pref_names.h" +#include "chrome/common/thumbnail_score.h" #include "gfx/codec/jpeg_codec.h" +#include "grit/chromium_strings.h" +#include "grit/generated_resources.h" +#include "grit/locale_settings.h" #include "third_party/skia/include/core/SkBitmap.h" namespace history { // How many top sites to store in the cache. static const size_t kTopSitesNumber = 20; +static const size_t kTopSitesShown = 8; static const int kDaysOfHistory = 90; // Time from startup to first HistoryService query. static const int64 kUpdateIntervalSecs = 15; @@ -34,11 +49,28 @@ TopSites::TopSites(Profile* profile) : profile_(profile), mock_history_service_(NULL), last_num_urls_changed_(0), migration_in_progress_(false), - waiting_for_results_(true) { - registrar_.Add(this, NotificationType::HISTORY_URLS_DELETED, - Source<Profile>(profile_)); - registrar_.Add(this, NotificationType::NAV_ENTRY_COMMITTED, - NotificationService::AllSources()); + waiting_for_results_(true), + blacklist_(NULL), + pinned_urls_(NULL) { + if (!profile_) + return; + + if (NotificationService::current()) { + registrar_.Add(this, NotificationType::HISTORY_URLS_DELETED, + Source<Profile>(profile_)); + registrar_.Add(this, NotificationType::NAV_ENTRY_COMMITTED, + NotificationService::AllSources()); + } + + blacklist_ = profile_->GetPrefs()-> + GetMutableDictionary(prefs::kNTPMostVisitedURLsBlacklist); + pinned_urls_ = profile_->GetPrefs()-> + GetMutableDictionary(prefs::kNTPMostVisitedPinnedURLs); +} + +// static +bool TopSites::IsEnabled() { + return CommandLine::ForCurrentProcess()->HasSwitch(switches::kEnableTopSites); } TopSites::~TopSites() { @@ -66,19 +98,18 @@ void TopSites::ReadDatabase() { std::map<GURL, Images> thumbnails; DCHECK(db_.get()); - { - AutoLock lock(lock_); - MostVisitedURLList top_urls; - db_->GetPageThumbnails(&top_urls, &thumbnails); - StoreMostVisited(&top_urls); - } // Lock is released here. + MostVisitedURLList top_urls; + db_->GetPageThumbnails(&top_urls, &thumbnails); + MostVisitedURLList copy(top_urls); // StoreMostVisited destroys the list. + StoreMostVisited(&top_urls); + if (AddPrepopulatedPages(©)) + UpdateMostVisited(copy); + AutoLock lock(lock_); for (size_t i = 0; i < top_sites_.size(); i++) { GURL url = top_sites_[i].url; Images thumbnail = thumbnails[url]; - if (!thumbnail.thumbnail.get() || !thumbnail.thumbnail->size()) { - LOG(INFO) << "No thumbnail for " << url.spec(); - } else { + if (thumbnail.thumbnail.get() && thumbnail.thumbnail->size()) { SetPageThumbnailNoDB(url, thumbnail.thumbnail, thumbnail.thumbnail_score); } @@ -90,6 +121,7 @@ void TopSites::ReadDatabase() { bool TopSites::SetPageThumbnail(const GURL& url, const SkBitmap& thumbnail, const ThumbnailScore& score) { + AutoLock lock(lock_); bool add_temp_thumbnail = false; if (canonical_urls_.find(url) == canonical_urls_.end()) { if (top_sites_.size() < kTopSitesNumber) { @@ -118,12 +150,13 @@ bool TopSites::SetPageThumbnail(const GURL& url, return true; } - return SetPageThumbnail(url, thumbnail_data, score); + return SetPageThumbnailEncoded(url, thumbnail_data, score); } -bool TopSites::SetPageThumbnail(const GURL& url, - const RefCountedBytes* thumbnail, - const ThumbnailScore& score) { +bool TopSites::SetPageThumbnailEncoded(const GURL& url, + const RefCountedBytes* thumbnail, + const ThumbnailScore& score) { + lock_.AssertAcquired(); if (!SetPageThumbnailNoDB(url, thumbnail, score)) return false; @@ -144,7 +177,7 @@ bool TopSites::SetPageThumbnail(const GURL& url, void TopSites::WriteThumbnailToDB(const MostVisitedURL& url, int url_rank, - const TopSites::Images& thumbnail) { + const Images& thumbnail) { DCHECK(db_.get()); DCHECK(ChromeThread::CurrentlyOn(ChromeThread::DB)); db_->SetPageThumbnail(url, url_rank, thumbnail); @@ -154,7 +187,7 @@ void TopSites::WriteThumbnailToDB(const MostVisitedURL& url, bool TopSites::SetPageThumbnailNoDB(const GURL& url, const RefCountedBytes* thumbnail_data, const ThumbnailScore& score) { - AutoLock lock(lock_); + lock_.AssertAcquired(); std::map<GURL, size_t>::iterator found = canonical_urls_.find(url); if (found == canonical_urls_.end()) { @@ -195,45 +228,180 @@ bool TopSites::SetPageThumbnailNoDB(const GURL& url, void TopSites::GetMostVisitedURLs(CancelableRequestConsumer* consumer, GetTopSitesCallback* callback) { - scoped_refptr<CancelableRequest<GetTopSitesCallback> > request( new CancelableRequest<GetTopSitesCallback>(callback)); // This ensures cancelation of requests when either the consumer or the // provider is deleted. Deletion of requests is also guaranteed. AddRequest(request, consumer); - if (waiting_for_results_) { - // A request came in before we have any top sites. - // We have to keep track of the requests ourselves. - pending_callbacks_.insert(request); - return; + MostVisitedURLList filtered_urls; + { + AutoLock lock(lock_); + if (waiting_for_results_) { + // A request came in before we have any top sites. + // We have to keep track of the requests ourselves. + pending_callbacks_.insert(request); + return; + } + if (request->canceled()) + return; + + ApplyBlacklistAndPinnedURLs(top_sites_, &filtered_urls); } - if (request->canceled()) - return; - request->ForwardResult(GetTopSitesCallback::TupleType(top_sites_)); + request->ForwardResult(GetTopSitesCallback::TupleType(filtered_urls)); } bool TopSites::GetPageThumbnail(const GURL& url, RefCountedBytes** data) const { - std::map<GURL, Images>::const_iterator found = top_images_.find(url); - if (found == top_images_.end()) - return false; // No thumbnail for this URL. + AutoLock lock(lock_); + std::map<GURL, Images>::const_iterator found = + top_images_.find(GetCanonicalURL(url)); + if (found == top_images_.end()) { + found = temp_thumbnails_map_.find(url); + if (found == temp_thumbnails_map_.end()) + return false; // No thumbnail for this URL. + } Images image = found->second; *data = image.thumbnail.get(); return true; } +static int IndexOf(const MostVisitedURLList& urls, const GURL& url) { + for (size_t i = 0; i < urls.size(); i++) { + if (urls[i].url == url) + return i; + } + return -1; +} + +bool TopSites::AddPrepopulatedPages(MostVisitedURLList* urls) { + // TODO(arv): This needs to get the data from some configurable place. + // http://crbug.com/17630 + bool added = false; + GURL welcome_url(l10n_util::GetStringUTF8(IDS_CHROME_WELCOME_URL)); + if (urls->size() < kTopSitesNumber && IndexOf(*urls, welcome_url) == -1) { + MostVisitedURL url = { + welcome_url, + GURL("chrome://theme/IDR_NEWTAB_CHROME_WELCOME_PAGE_FAVICON"), + l10n_util::GetStringUTF16(IDS_NEW_TAB_CHROME_WELCOME_PAGE_TITLE) + }; + url.redirects.push_back(welcome_url); + urls->push_back(url); + added = true; + } + + GURL themes_url(l10n_util::GetStringUTF8(IDS_THEMES_GALLERY_URL)); + if (urls->size() < kTopSitesNumber && IndexOf(*urls, themes_url) == -1) { + MostVisitedURL url = { + themes_url, + GURL("chrome://theme/IDR_NEWTAB_THEMES_GALLERY_FAVICON"), + l10n_util::GetStringUTF16(IDS_NEW_TAB_THEMES_GALLERY_PAGE_TITLE) + }; + url.redirects.push_back(themes_url); + urls->push_back(url); + added = true; + } + + return added; +} + +void TopSites::MigratePinnedURLs() { + std::map<GURL, size_t> tmp_map; + for (DictionaryValue::key_iterator it = pinned_urls_->begin_keys(); + it != pinned_urls_->end_keys(); ++it) { + Value* value; + if (!pinned_urls_->GetWithoutPathExpansion(*it, &value)) + continue; + + if (value->IsType(DictionaryValue::TYPE_DICTIONARY)) { + DictionaryValue* dict = static_cast<DictionaryValue*>(value); + std::string url_string; + int index; + if (dict->GetString("url", &url_string) && + dict->GetInteger("index", &index)) + tmp_map[GURL(url_string)] = index; + } + } + pinned_urls_->Clear(); + for (std::map<GURL, size_t>::iterator it = tmp_map.begin(); + it != tmp_map.end(); ++it) + AddPinnedURL(it->first, it->second); +} + +void TopSites::ApplyBlacklistAndPinnedURLs(const MostVisitedURLList& urls, + MostVisitedURLList* out) { + lock_.AssertAcquired(); + MostVisitedURLList urls_copy; + for (size_t i = 0; i < urls.size(); i++) { + if (!IsBlacklisted(urls[i].url)) + urls_copy.push_back(urls[i]); + } + + for (size_t pinned_index = 0; pinned_index < kTopSitesShown; pinned_index++) { + GURL url; + bool found = GetPinnedURLAtIndex(pinned_index, &url); + if (!found) + continue; + + DCHECK(!url.is_empty()); + int cur_index = IndexOf(urls_copy, url); + MostVisitedURL tmp; + if (cur_index < 0) { + // Pinned URL not in urls. + tmp.url = url; + } else { + tmp = urls_copy[cur_index]; + urls_copy.erase(urls_copy.begin() + cur_index); + } + if (pinned_index > out->size()) + out->resize(pinned_index); // Add empty URLs as fillers. + out->insert(out->begin() + pinned_index, tmp); + } + + // Add non-pinned URLs in the empty spots. + size_t current_url = 0; // Index into the remaining URLs in urls_copy. + for (size_t i = 0; i < kTopSitesShown && current_url < urls_copy.size(); + i++) { + if (i == out->size()) { + out->push_back(urls_copy[current_url]); + current_url++; + } else if (i < out->size()) { + if ((*out)[i].url.is_empty()) { + // Replace the filler + (*out)[i] = urls_copy[current_url]; + current_url++; + } + } else { + NOTREACHED(); + } + } +} + +std::string TopSites::GetURLString(const GURL& url) { + lock_.AssertAcquired(); + return GetCanonicalURL(url).spec(); +} + +std::string TopSites::GetURLHash(const GURL& url) { + lock_.AssertAcquired(); + // We don't use canonical URLs here to be able to blacklist only one of + // the two 'duplicate' sites, e.g. 'gmail.com' and 'mail.google.com'. + return MD5String(url.spec()); +} + void TopSites::UpdateMostVisited(MostVisitedURLList most_visited) { DCHECK(ChromeThread::CurrentlyOn(ChromeThread::DB)); - // TODO(brettw) filter for blacklist! - if (!top_sites_.empty()) { - std::vector<size_t> added; // Indices into most_visited. - std::vector<size_t> deleted; // Indices into top_sites_. - std::vector<size_t> moved; // Indices into most_visited. - DiffMostVisited(top_sites_, most_visited, &added, &deleted, &moved); + std::vector<size_t> added; // Indices into most_visited. + std::vector<size_t> deleted; // Indices into top_sites_. + std::vector<size_t> moved; // Indices into most_visited. + + DiffMostVisited(top_sites_, most_visited, &added, &deleted, &moved); + + // #added == #deleted; #added + #moved = total. + last_num_urls_changed_ = added.size() + moved.size(); - // #added == #deleted; #added + #moved = total. - last_num_urls_changed_ = added.size() + moved.size(); + { + AutoLock lock(lock_); // Process the diff: delete from images and disk, add to disk. // Delete all the thumbnails associated with URLs that were deleted. @@ -243,22 +411,23 @@ void TopSites::UpdateMostVisited(MostVisitedURLList most_visited) { top_images_.find(deleted_url.url); if (found != top_images_.end()) top_images_.erase(found); + } + } - // Delete from disk. + // Write the updates to the DB. + if (db_.get()) { + for (size_t i = 0; i < deleted.size(); i++) { + const MostVisitedURL& deleted_url = top_sites_[deleted[i]]; if (db_.get()) db_->RemoveURL(deleted_url); } - - if (db_.get()) { - // Write both added and moved urls. - for (size_t i = 0; i < added.size(); i++) { - MostVisitedURL& added_url = most_visited[added[i]]; - db_->SetPageThumbnail(added_url, added[i], Images()); - } - for (size_t i = 0; i < moved.size(); i++) { - MostVisitedURL moved_url = most_visited[moved[i]]; - db_->UpdatePageRank(moved_url, moved[i]); - } + for (size_t i = 0; i < added.size(); i++) { + const MostVisitedURL& added_url = most_visited[added[i]]; + db_->SetPageThumbnail(added_url, added[i], Images()); + } + for (size_t i = 0; i < moved.size(); i++) { + const MostVisitedURL& moved_url = most_visited[moved[i]]; + db_->UpdatePageRank(moved_url, moved[i]); } } @@ -285,6 +454,9 @@ void TopSites::UpdateMostVisited(MostVisitedURLList most_visited) { void TopSites::OnMigrationDone() { migration_in_progress_ = false; + if (!profile_) + return; + HistoryService* hs = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS); // |hs| may be null during unit tests. if (!hs) @@ -302,6 +474,12 @@ void TopSites::AddTemporaryThumbnail(const GURL& url, void TopSites::StartQueryForThumbnail(size_t index) { DCHECK(migration_in_progress_); + if (top_sites_[index].url.spec() == + l10n_util::GetStringUTF8(IDS_CHROME_WELCOME_URL) || + top_sites_[index].url.spec() == + l10n_util::GetStringUTF8(IDS_THEMES_GALLERY_URL)) + return; // Don't need thumbnails for prepopulated URLs. + migration_pending_urls_.insert(top_sites_[index].url); if (mock_history_service_) { @@ -316,6 +494,9 @@ void TopSites::StartQueryForThumbnail(size_t index) { return; } + if (!profile_) + return; + HistoryService* hs = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS); // |hs| may be null during unit tests. if (!hs) @@ -327,54 +508,81 @@ void TopSites::StartQueryForThumbnail(size_t index) { cancelable_consumer_.SetClientData(hs, handle, index); } -void TopSites::StoreMostVisited(MostVisitedURLList* most_visited) { - DCHECK(ChromeThread::CurrentlyOn(ChromeThread::DB)); - // Take ownership of the most visited data. - top_sites_.clear(); - top_sites_.swap(*most_visited); - waiting_for_results_ = false; - - // Save the redirect information for quickly mapping to the canonical URLs. +void TopSites::GenerateCanonicalURLs() { + lock_.AssertAcquired(); canonical_urls_.clear(); for (size_t i = 0; i < top_sites_.size(); i++) { const MostVisitedURL& mv = top_sites_[i]; StoreRedirectChain(mv.redirects, i); + } +} + +void TopSites::StoreMostVisited(MostVisitedURLList* most_visited) { + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::DB)); + MostVisitedURLList filtered_urls; + PendingCallbackSet callbacks; + { + AutoLock lock(lock_); + top_sites_.clear(); + // Take ownership of the most visited data. + top_sites_.swap(*most_visited); + waiting_for_results_ = false; - std::map<GURL, Images>::iterator it = temp_thumbnails_map_.begin(); - GURL canonical_url = GetCanonicalURL(mv.url); - for (; it != temp_thumbnails_map_.end(); it++) { - // Must map all temp URLs to canonical ones. - // temp_thumbnails_map_ contains non-canonical URLs, because - // when we add a temp thumbnail, redirect chain is not known. - // This is slow, but temp_thumbnails_map_ should have very few URLs. - if (canonical_url == GetCanonicalURL(it->first)) { - SetPageThumbnail(mv.url, it->second.thumbnail, - it->second.thumbnail_score); - temp_thumbnails_map_.erase(it); - break; + // Save the redirect information for quickly mapping to the canonical URLs. + GenerateCanonicalURLs(); + + for (size_t i = 0; i < top_sites_.size(); i++) { + const MostVisitedURL& mv = top_sites_[i]; + std::map<GURL, Images>::iterator it = temp_thumbnails_map_.begin(); + GURL canonical_url = GetCanonicalURL(mv.url); + for (; it != temp_thumbnails_map_.end(); it++) { + // Must map all temp URLs to canonical ones. + // temp_thumbnails_map_ contains non-canonical URLs, because + // when we add a temp thumbnail, redirect chain is not known. + // This is slow, but temp_thumbnails_map_ should have very few URLs. + if (canonical_url == GetCanonicalURL(it->first)) { + SetPageThumbnailEncoded(mv.url, it->second.thumbnail, + it->second.thumbnail_score); + temp_thumbnails_map_.erase(it); + break; + } } } - } - if (top_sites_.size() >= kTopSitesNumber) - temp_thumbnails_map_.clear(); + if (top_sites_.size() >= kTopSitesNumber) + temp_thumbnails_map_.clear(); + + if (pending_callbacks_.empty()) + return; + + ApplyBlacklistAndPinnedURLs(top_sites_, &filtered_urls); + callbacks.swap(pending_callbacks_); + } // lock_ is released. + // Process callbacks outside the lock - ForwardResults may cause + // thread switches. + ProcessPendingCallbacks(callbacks, filtered_urls); } void TopSites::StoreRedirectChain(const RedirectList& redirects, size_t destination) { + lock_.AssertAcquired(); if (redirects.empty()) { NOTREACHED(); return; } // Map all the redirected URLs to the destination. - for (size_t i = 0; i < redirects.size(); i++) - canonical_urls_[redirects[i]] = destination; + for (size_t i = 0; i < redirects.size(); i++) { + // If this redirect is already known, don't replace it with a new one. + if (canonical_urls_.find(redirects[i]) == canonical_urls_.end()) + canonical_urls_[redirects[i]] = destination; + } } GURL TopSites::GetCanonicalURL(const GURL& url) const { + lock_.AssertAcquired(); std::map<GURL, size_t>::const_iterator found = canonical_urls_.find(url); if (found == canonical_urls_.end()) - return GURL(); // Don't know anything about this URL. + return url; // Unknown URL - return unchanged. return top_sites_[found->second].url; } @@ -435,16 +643,19 @@ void TopSites::StartQueryForMostVisited() { // Testing with a mockup. // QueryMostVisitedURLs is not virtual, so we have to duplicate the code. mock_history_service_->QueryMostVisitedURLs( - kTopSitesNumber, + kTopSitesNumber + blacklist_->size(), kDaysOfHistory, &cancelable_consumer_, NewCallback(this, &TopSites::OnTopSitesAvailable)); } else { + if (!profile_) + return; + HistoryService* hs = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS); // |hs| may be null during unit tests. if (hs) { hs->QueryMostVisitedURLs( - kTopSitesNumber, + kTopSitesNumber + blacklist_->size(), kDaysOfHistory, &cancelable_consumer_, NewCallback(this, &TopSites::OnTopSitesAvailable)); @@ -455,11 +666,106 @@ void TopSites::StartQueryForMostVisited() { } void TopSites::StartMigration() { + LOG(INFO) << "Starting migration to TopSites."; migration_in_progress_ = true; StartQueryForMostVisited(); + MigratePinnedURLs(); +} + +bool TopSites::HasBlacklistedItems() const { + AutoLock lock(lock_); + return !blacklist_->empty(); +} + +void TopSites::AddBlacklistedURL(const GURL& url) { + AutoLock lock(lock_); + RemovePinnedURLLocked(url); + Value* dummy = Value::CreateNullValue(); + blacklist_->SetWithoutPathExpansion(GetURLHash(url), dummy); +} + +bool TopSites::IsBlacklisted(const GURL& url) { + lock_.AssertAcquired(); + bool result = blacklist_->HasKey(GetURLHash(url)); + return result; +} + +void TopSites::RemoveBlacklistedURL(const GURL& url) { + AutoLock lock(lock_); + blacklist_->RemoveWithoutPathExpansion(GetURLHash(url), NULL); +} + +void TopSites::ClearBlacklistedURLs() { + blacklist_->Clear(); +} + +void TopSites::AddPinnedURL(const GURL& url, size_t pinned_index) { + GURL old; + if (GetPinnedURLAtIndex(pinned_index, &old)) { + RemovePinnedURL(old); + } + + if (IsURLPinned(url)) { + RemovePinnedURL(url); + } + + Value* index = Value::CreateIntegerValue(pinned_index); + AutoLock lock(lock_); + pinned_urls_->SetWithoutPathExpansion(GetURLString(url), index); +} + +void TopSites::RemovePinnedURL(const GURL& url) { + AutoLock lock(lock_); + RemovePinnedURLLocked(url); +} + +void TopSites::RemovePinnedURLLocked(const GURL& url) { + lock_.AssertAcquired(); + pinned_urls_->RemoveWithoutPathExpansion(GetURLString(url), NULL); +} + +bool TopSites::IsURLPinned(const GURL& url) { + AutoLock lock(lock_); + int tmp; + bool result = pinned_urls_->GetIntegerWithoutPathExpansion( + GetURLString(url), &tmp); + return result; +} + +bool TopSites::GetPinnedURLAtIndex(size_t index, GURL* url) { + for (DictionaryValue::key_iterator it = pinned_urls_->begin_keys(); + it != pinned_urls_->end_keys(); ++it) { + int current_index; + if (pinned_urls_->GetIntegerWithoutPathExpansion(*it, ¤t_index)) { + if (static_cast<size_t>(current_index) == index) { + *url = GURL(*it); + return true; + } + } + } + return false; +} + +// static +void TopSites::DeleteTopSites(scoped_refptr<TopSites>& ptr) { + if (!ptr.get() || !MessageLoop::current()) + return; + if (ChromeThread::IsWellKnownThread(ChromeThread::UI)) { + ptr = NULL; + } else { + // Need to roll our own UI thread. + ChromeThread ui_loop(ChromeThread::UI, MessageLoop::current()); + ptr = NULL; + MessageLoop::current()->RunAllPending(); + } +} + +void TopSites::ClearProfile() { + profile_ = NULL; } base::TimeDelta TopSites::GetUpdateDelay() { + AutoLock lock(lock_); if (top_sites_.size() == 0) return base::TimeDelta::FromSeconds(30); @@ -472,29 +778,36 @@ base::TimeDelta TopSites::GetUpdateDelay() { void TopSites::OnTopSitesAvailable( CancelableRequestProvider::Handle handle, MostVisitedURLList pages) { - if (!pending_callbacks_.empty()) { - PendingCallbackSet copy(pending_callbacks_); - PendingCallbackSet::iterator i; - for (i = pending_callbacks_.begin(); - i != pending_callbacks_.end(); ++i) { - scoped_refptr<CancelableRequest<GetTopSitesCallback> > request = *i; - if (!request->canceled()) - request->ForwardResult(GetTopSitesCallback::TupleType(pages)); - } - pending_callbacks_.clear(); - } - + AddPrepopulatedPages(&pages); ChromeThread::PostTask(ChromeThread::DB, FROM_HERE, NewRunnableMethod( this, &TopSites::UpdateMostVisited, pages)); } +// static +void TopSites::ProcessPendingCallbacks(PendingCallbackSet pending_callbacks, + const MostVisitedURLList& urls) { + PendingCallbackSet::iterator i; + for (i = pending_callbacks.begin(); + i != pending_callbacks.end(); ++i) { + scoped_refptr<CancelableRequest<GetTopSitesCallback> > request = *i; + if (!request->canceled()) + request->ForwardResult(GetTopSitesCallback::TupleType(urls)); + } + pending_callbacks.clear(); +} + void TopSites::OnThumbnailAvailable(CancelableRequestProvider::Handle handle, scoped_refptr<RefCountedBytes> thumbnail) { size_t index; if (mock_history_service_) { index = handle; } else { + if (!profile_) + return; + HistoryService* hs = profile_ ->GetHistoryService(Profile::EXPLICIT_ACCESS); + if (!hs) + return; index = cancelable_consumer_.GetClientData(hs, handle); } DCHECK(static_cast<size_t>(index) < top_sites_.size()); @@ -504,7 +817,8 @@ void TopSites::OnThumbnailAvailable(CancelableRequestProvider::Handle handle, if (thumbnail.get() && thumbnail->size()) { const MostVisitedURL& url = top_sites_[index]; - SetPageThumbnail(url.url, thumbnail, ThumbnailScore()); + AutoLock lock(lock_); + SetPageThumbnailEncoded(url.url, thumbnail, ThumbnailScore()); } if (migration_in_progress_ && migration_pending_urls_.empty() && @@ -519,6 +833,7 @@ void TopSites::SetMockHistoryService(MockHistoryService* mhs) { void TopSites::Observe(NotificationType type, const NotificationSource& source, const NotificationDetails& details) { + AutoLock lock(lock_); if (type == NotificationType::HISTORY_URLS_DELETED) { Details<history::URLsDeletedDetails> deleted_details(details); if (deleted_details->all_history) { @@ -526,25 +841,34 @@ void TopSites::Observe(NotificationType type, ChromeThread::PostTask(ChromeThread::DB, FROM_HERE, NewRunnableMethod(this, &TopSites::ResetDatabase)); } else { + std::set<size_t> indices_to_delete; // Indices into top_sites_. std::set<GURL>::iterator it; for (it = deleted_details->urls.begin(); it != deleted_details->urls.end(); ++it) { - for (size_t i = 0; i < top_sites_.size(); i++) { - if (top_sites_[i].url == *it) { - top_sites_.erase(top_sites_.begin() + i); - break; - } - } + std::map<GURL,size_t>::const_iterator found = canonical_urls_.find(*it); + if (found != canonical_urls_.end()) + indices_to_delete.insert(found->second); + } + + for (std::set<size_t>::reverse_iterator i = indices_to_delete.rbegin(); + i != indices_to_delete.rend(); i++) { + size_t index = *i; + RemovePinnedURLLocked(top_sites_[index].url); + top_sites_.erase(top_sites_.begin() + index); } } + // Canonical URLs are not valid any more. + GenerateCanonicalURLs(); StartQueryForMostVisited(); } else if (type == NotificationType::NAV_ENTRY_COMMITTED) { if (top_sites_.size() < kTopSitesNumber) { - const NavigationController::LoadCommittedDetails& load_details = - *Details<NavigationController::LoadCommittedDetails>(details).ptr(); - GURL url = load_details.entry->url(); + NavigationController::LoadCommittedDetails* load_details = + Details<NavigationController::LoadCommittedDetails>(details).ptr(); + if (!load_details) + return; + GURL url = load_details->entry->url(); if (canonical_urls_.find(url) == canonical_urls_.end() && - HistoryService::CanAddURL(url)) { + HistoryService::CanAddURL(url)) { // Add this page to the known pages in case the thumbnail comes // in before we get the results. MostVisitedURL mv; diff --git a/chrome/browser/history/top_sites.h b/chrome/browser/history/top_sites.h index 4cc6059..c491066 100644 --- a/chrome/browser/history/top_sites.h +++ b/chrome/browser/history/top_sites.h @@ -4,6 +4,7 @@ #ifndef CHROME_BROWSER_HISTORY_TOP_SITES_H_ #define CHROME_BROWSER_HISTORY_TOP_SITES_H_ +#pragma once #include <map> #include <set> @@ -17,6 +18,7 @@ #include "base/ref_counted.h" #include "base/ref_counted_memory.h" #include "chrome/browser/cancelable_request.h" +#include "chrome/browser/chrome_thread.h" #include "chrome/browser/history/history_types.h" #include "chrome/browser/history/history.h" #include "chrome/browser/history/page_usage_data.h" @@ -24,6 +26,7 @@ #include "chrome/common/thumbnail_score.h" #include "googleurl/src/gurl.h" +class DictionaryValue; class SkBitmap; class Profile; @@ -45,12 +48,17 @@ typedef std::vector<MostVisitedURL> MostVisitedURLList; // new tab page requests on the I/O thread without proxying to the UI thread is // a nontrivial performance win, especially when the browser is starting and // the UI thread is busy. -class TopSites : public NotificationObserver, - public base::RefCountedThreadSafe<TopSites>, - public CancelableRequestProvider { +class TopSites : + public base::RefCountedThreadSafe<TopSites, + ChromeThread::DeleteOnUIThread>, + public NotificationObserver, + public CancelableRequestProvider { public: explicit TopSites(Profile* profile); + // Returns whether top sites is enabled. + static bool IsEnabled(); + class MockHistoryService { // A mockup of a HistoryService used for testing TopSites. public: @@ -66,14 +74,6 @@ class TopSites : public NotificationObserver, size_t index) = 0; }; - struct Images { - scoped_refptr<RefCountedBytes> thumbnail; - ThumbnailScore thumbnail_score; - - // TODO(brettw): this will eventually store the favicon. - // scoped_refptr<RefCountedBytes> favicon; - }; - // Initializes TopSites. void Init(const FilePath& db_name); @@ -85,7 +85,9 @@ class TopSites : public NotificationObserver, const ThumbnailScore& score); // Callback for GetMostVisitedURLs. - typedef Callback1<const MostVisitedURLList&>::Type GetTopSitesCallback; + typedef Callback1<MostVisitedURLList>::Type GetTopSitesCallback; + typedef std::set<scoped_refptr<CancelableRequest<GetTopSitesCallback> > > + PendingCallbackSet; // Returns a list of most visited URLs via a callback. // NOTE: the callback may be called immediately if we have the data cached. @@ -101,18 +103,63 @@ class TopSites : public NotificationObserver, // Start reading thumbnails from the ThumbnailDatabase. void StartMigration(); + // Blacklisted URLs + + // Returns true if there is at least one item in the blacklist. + bool HasBlacklistedItems() const; + + // Add a URL to the blacklist. + void AddBlacklistedURL(const GURL& url); + + // Removes a URL from the blacklist. + void RemoveBlacklistedURL(const GURL& url); + + // Clear the blacklist. + void ClearBlacklistedURLs(); + + // Pinned URLs + + // Pin a URL at |index|. + void AddPinnedURL(const GURL& url, size_t index); + + // Returns true if a URL is pinned. + bool IsURLPinned(const GURL& url); + + // Unpin a URL. + void RemovePinnedURL(const GURL& url); + + // Return a URL pinned at |index| via |out|. Returns true if there + // is a URL pinned at |index|. + bool GetPinnedURLAtIndex(size_t index, GURL* out); + + // TopSites must be deleted on a UI thread. This happens + // automatically in a real browser, but in unit_tests we don't have + // a real UI thread. Use this function to delete a TopSites object. + static void DeleteTopSites(scoped_refptr<TopSites>& ptr); + + // Sets the profile pointer to NULL. This is for the case where + // TopSites outlives the profile, since TopSites is refcounted. + void ClearProfile(); + private: - friend class base::RefCountedThreadSafe<TopSites>; + friend struct ChromeThread::DeleteOnThread<ChromeThread::UI>; + friend class DeleteTask<TopSites>; friend class TopSitesTest; FRIEND_TEST_ALL_PREFIXES(TopSitesTest, GetMostVisited); FRIEND_TEST_ALL_PREFIXES(TopSitesTest, RealDatabase); FRIEND_TEST_ALL_PREFIXES(TopSitesTest, MockDatabase); FRIEND_TEST_ALL_PREFIXES(TopSitesTest, DeleteNotifications); + FRIEND_TEST_ALL_PREFIXES(TopSitesTest, PinnedURLsDeleted); FRIEND_TEST_ALL_PREFIXES(TopSitesTest, GetUpdateDelay); FRIEND_TEST_ALL_PREFIXES(TopSitesTest, Migration); FRIEND_TEST_ALL_PREFIXES(TopSitesTest, QueueingRequestsForTopSites); FRIEND_TEST_ALL_PREFIXES(TopSitesTest, CancelingRequestsForTopSites); FRIEND_TEST_ALL_PREFIXES(TopSitesTest, AddTemporaryThumbnail); + FRIEND_TEST_ALL_PREFIXES(TopSitesTest, Blacklisting); + FRIEND_TEST_ALL_PREFIXES(TopSitesTest, PinnedURLs); + FRIEND_TEST_ALL_PREFIXES(TopSitesTest, BlacklistingAndPinnedURLs); + FRIEND_TEST_ALL_PREFIXES(TopSitesTest, AddPrepopulatedPages); + FRIEND_TEST_ALL_PREFIXES(TopSitesTest, GetPageThumbnail); ~TopSites(); @@ -125,9 +172,9 @@ class TopSites : public NotificationObserver, // A version of SetPageThumbnail that takes RefCountedBytes as // returned by HistoryService. - bool SetPageThumbnail(const GURL& url, - const RefCountedBytes* thumbnail, - const ThumbnailScore& score); + bool SetPageThumbnailEncoded(const GURL& url, + const RefCountedBytes* thumbnail, + const ThumbnailScore& score); // Query history service for the list of available thumbnails. void StartQueryForMostVisited(); @@ -140,10 +187,17 @@ class TopSites : public NotificationObserver, void OnTopSitesAvailable(CancelableRequestProvider::Handle handle, MostVisitedURLList data); + // Returns a list of urls to each pending callback. + static void ProcessPendingCallbacks(PendingCallbackSet pending_callbacks, + const MostVisitedURLList& urls); + // Called when history service returns a thumbnail. void OnThumbnailAvailable(CancelableRequestProvider::Handle handle, scoped_refptr<RefCountedBytes> thumbnail); + // Sets canonical_urls_ from top_sites_. + void GenerateCanonicalURLs(); + // Saves the set of the top URLs visited by this user. The 0th item is the // most popular. // DANGER! This will clear all data from the input argument. @@ -186,6 +240,12 @@ class TopSites : public NotificationObserver, const NotificationSource& source, const NotificationDetails& details); + // Returns true if the URL is blacklisted. + bool IsBlacklisted(const GURL& url); + + // A variant of RemovePinnedURL that must be called within a lock. + void RemovePinnedURLLocked(const GURL& url); + // Returns the delay until the next update of history is needed. // Uses num_urls_changed base::TimeDelta GetUpdateDelay(); @@ -200,7 +260,7 @@ class TopSites : public NotificationObserver, // Write a thumbnail to database. void WriteThumbnailToDB(const MostVisitedURL& url, int url_rank, - const TopSites::Images& thumbnail); + const Images& thumbnail); // Updates the top sites list and writes the difference to disk. void UpdateMostVisited(MostVisitedURLList most_visited); @@ -216,6 +276,25 @@ class TopSites : public NotificationObserver, const RefCountedBytes* thumbnail, const ThumbnailScore& score); + // Add prepopulated pages: 'welcome to Chrome' and themes gallery. + // Returns true if any pages were added. + bool AddPrepopulatedPages(MostVisitedURLList* urls); + + // Convert pinned_urls_ dictionary to the new format. Use URLs as + // dictionary keys. + void MigratePinnedURLs(); + + // Takes |urls|, produces it's copy in |out| after removing + // blacklisted URLs and reordering pinned URLs. + void ApplyBlacklistAndPinnedURLs(const MostVisitedURLList& urls, + MostVisitedURLList* out); + + // Converts a url into a canonical string representation. + std::string GetURLString(const GURL& url); + + // Returns an MD5 hash of the URL. Hashing is required for blacklisted URLs. + std::string GetURLHash(const GURL& url); + Profile* profile_; // A mockup to use for testing. If NULL, use the real HistoryService // from the profile_. See SetMockHistoryService. @@ -257,8 +336,6 @@ class TopSites : public NotificationObserver, // The map of requests for the top sites list. Can only be // non-empty at startup. After we read the top sites from the DB, we'll // always have a cached list. - typedef std::set<scoped_refptr<CancelableRequest<GetTopSitesCallback> > > - PendingCallbackSet; PendingCallbackSet pending_callbacks_; // Are we waiting for the top sites from HistoryService? @@ -270,8 +347,20 @@ class TopSites : public NotificationObserver, // UpdateMostVisitedURLs call. std::map<GURL, Images> temp_thumbnails_map_; - // TODO(brettw): use the blacklist. - // std::set<GURL> blacklist_; + // Blacklisted and pinned URLs are stored in Preferences. + + // Blacklisted URLs. They are filtered out from the list of Top + // Sites when GetMostVisitedURLs is called. Note that we are still + // storing all URLs, but filtering on access. It is a dictionary, + // key is the URL, value is a dummy value. This is owned by the + // PrefService. + DictionaryValue* blacklist_; + + // This is a dictionary for the pinned URLs for the the most visited + // part of the new tab page. Key is the URL, value is + // index where it is pinned at (may be the same as key). This is + // owned by the PrefService. + DictionaryValue* pinned_urls_; DISALLOW_COPY_AND_ASSIGN(TopSites); }; diff --git a/chrome/browser/history/top_sites_database.cc b/chrome/browser/history/top_sites_database.cc index 99f0bb4..05a588a 100644 --- a/chrome/browser/history/top_sites_database.cc +++ b/chrome/browser/history/top_sites_database.cc @@ -5,6 +5,7 @@ #include "app/sql/transaction.h" #include "base/string_util.h" #include "chrome/browser/diagnostics/sqlite_diagnostics.h" +#include "chrome/browser/history/history_types.h" #include "chrome/browser/history/top_sites.h" #include "chrome/browser/history/top_sites_database.h" @@ -13,6 +14,9 @@ namespace history { TopSitesDatabaseImpl::TopSitesDatabaseImpl() { } +TopSitesDatabaseImpl::~TopSitesDatabaseImpl() { +} + bool TopSitesDatabaseImpl::Init(const FilePath& db_name) { // Settings copied from ThumbnailDatabase. db_.set_error_delegate(GetErrorHandlerForThumbnailDb()); @@ -48,7 +52,7 @@ bool TopSitesDatabaseImpl::InitThumbnailTable() { void TopSitesDatabaseImpl::GetPageThumbnails(MostVisitedURLList* urls, std::map<GURL, - TopSites::Images>* thumbnails) { + Images>* thumbnails) { sql::Statement statement(db_.GetCachedStatement( SQL_FROM_HERE, "SELECT url, url_rank, title, thumbnail, redirects, " @@ -75,7 +79,7 @@ void TopSitesDatabaseImpl::GetPageThumbnails(MostVisitedURLList* urls, std::vector<unsigned char> data; statement.ColumnBlobAsVector(3, &data); - TopSites::Images thumbnail; + Images thumbnail; thumbnail.thumbnail = RefCountedBytes::TakeVector(&data); thumbnail.thumbnail_score.boring_score = statement.ColumnDouble(5); thumbnail.thumbnail_score.good_clipping = statement.ColumnBool(6); @@ -106,7 +110,7 @@ void TopSitesDatabaseImpl::SetRedirects(const std::string& redirects, void TopSitesDatabaseImpl::SetPageThumbnail(const MostVisitedURL& url, int new_rank, - const TopSites::Images& thumbnail) { + const Images& thumbnail) { sql::Transaction transaction(&db_); transaction.Begin(); @@ -122,7 +126,7 @@ void TopSitesDatabaseImpl::SetPageThumbnail(const MostVisitedURL& url, } void TopSitesDatabaseImpl::UpdatePageThumbnail( - const MostVisitedURL& url, const TopSites::Images& thumbnail) { + const MostVisitedURL& url, const Images& thumbnail) { sql::Statement statement(db_.GetCachedStatement( SQL_FROM_HERE, "UPDATE thumbnails SET " @@ -150,7 +154,7 @@ void TopSitesDatabaseImpl::UpdatePageThumbnail( void TopSitesDatabaseImpl::AddPageThumbnail(const MostVisitedURL& url, int new_rank, - const TopSites::Images& thumbnail) { + const Images& thumbnail) { int count = GetRowCount(); sql::Statement statement(db_.GetCachedStatement( @@ -194,7 +198,7 @@ void TopSitesDatabaseImpl::UpdatePageRankNoTransaction( const MostVisitedURL& url, int new_rank) { int prev_rank = GetURLRank(url); if (prev_rank == -1) { - NOTREACHED() << "Updating rank of an unknown URL: " << url.url.spec(); + LOG(WARNING) << "Updating rank of an unknown URL: " << url.url.spec(); return; } @@ -236,7 +240,7 @@ void TopSitesDatabaseImpl::UpdatePageRankNoTransaction( } bool TopSitesDatabaseImpl::GetPageThumbnail(const GURL& url, - TopSites::Images* thumbnail) { + Images* thumbnail) { sql::Statement statement(db_.GetCachedStatement( SQL_FROM_HERE, "SELECT thumbnail, boring_score, good_clipping, at_top, last_updated " diff --git a/chrome/browser/history/top_sites_database.h b/chrome/browser/history/top_sites_database.h index cfb362c..edfb4e5 100644 --- a/chrome/browser/history/top_sites_database.h +++ b/chrome/browser/history/top_sites_database.h @@ -4,20 +4,19 @@ #ifndef CHROME_BROWSER_HISTORY_TOP_SITES_DATABASE_H_ #define CHROME_BROWSER_HISTORY_TOP_SITES_DATABASE_H_ +#pragma once #include <map> #include <string> -#include <vector> #include "app/sql/connection.h" #include "base/ref_counted.h" -#include "chrome/browser/history/history_types.h" #include "chrome/browser/history/url_database.h" // For DBCloseScoper. class FilePath; class RefCountedMemory; class SkBitmap; -class TopSites; +class Images; namespace base { class Time; @@ -37,7 +36,7 @@ class TopSitesDatabase { // Returns a list of all URLs currently in the table. virtual void GetPageThumbnails(MostVisitedURLList* urls, std::map<GURL, - TopSites::Images>* thumbnails) = 0; + Images>* thumbnails) = 0; // Set a thumbnail for a URL. |url_rank| is the position of the URL // in the list of TopURLs, zero-based. @@ -45,20 +44,20 @@ class TopSitesDatabase { // thumbnail. virtual void SetPageThumbnail(const MostVisitedURL& url, int url_rank, - const TopSites::Images& thumbnail) = 0; + const Images& thumbnail) = 0; // Update rank of a URL that's already in the database. virtual void UpdatePageRank(const MostVisitedURL& url, int new_rank) = 0; // Convenience wrapper. bool GetPageThumbnail(const MostVisitedURL& url, - TopSites::Images* thumbnail) { + Images* thumbnail) { return GetPageThumbnail(url.url, thumbnail); } // Get a thumbnail for a given page. Returns true iff we have the thumbnail. virtual bool GetPageThumbnail(const GURL& url, - TopSites::Images* thumbnail) = 0; + Images* thumbnail) = 0; // Remove the record for this URL. Returns true iff removed successfully. virtual bool RemoveURL(const MostVisitedURL& url) = 0; @@ -67,7 +66,7 @@ class TopSitesDatabase { class TopSitesDatabaseImpl : public TopSitesDatabase { public: TopSitesDatabaseImpl(); - ~TopSitesDatabaseImpl() {} + virtual ~TopSitesDatabaseImpl(); // Must be called after creation but before any other methods are called. // Returns true on success. If false, no other functions should be called. @@ -78,7 +77,7 @@ class TopSitesDatabaseImpl : public TopSitesDatabase { // Returns a list of all URLs currently in the table. // WARNING: clears both input arguments. virtual void GetPageThumbnails(MostVisitedURLList* urls, - std::map<GURL, TopSites::Images>* thumbnails); + std::map<GURL, Images>* thumbnails); // Set a thumbnail for a URL. |url_rank| is the position of the URL // in the list of TopURLs, zero-based. @@ -86,7 +85,7 @@ class TopSitesDatabaseImpl : public TopSitesDatabase { // thumbnail and rank. Shift the ranks of other URLs if necessary. virtual void SetPageThumbnail(const MostVisitedURL& url, int new_rank, - const TopSites::Images& thumbnail); + const Images& thumbnail); // Sets the rank for a given URL. The URL must be in the database. // Use SetPageThumbnail if it's not. @@ -94,7 +93,7 @@ class TopSitesDatabaseImpl : public TopSitesDatabase { // Get a thumbnail for a given page. Returns true iff we have the thumbnail. virtual bool GetPageThumbnail(const GURL& url, - TopSites::Images* thumbnail); + Images* thumbnail); // Remove the record for this URL. Returns true iff removed successfully. virtual bool RemoveURL(const MostVisitedURL& url); @@ -107,14 +106,14 @@ class TopSitesDatabaseImpl : public TopSitesDatabase { // Adds a new URL to the database. void AddPageThumbnail(const MostVisitedURL& url, int new_rank, - const TopSites::Images& thumbnail); + const Images& thumbnail); // Sets the page rank. Should be called within an open transaction. void UpdatePageRankNoTransaction(const MostVisitedURL& url, int new_rank); // Updates thumbnail of a URL that's already in the database. void UpdatePageThumbnail(const MostVisitedURL& url, - const TopSites::Images& thumbnail); + const Images& thumbnail); // Returns the URL's current rank or -1 if it is not present. int GetURLRank(const MostVisitedURL& url); diff --git a/chrome/browser/history/top_sites_unittest.cc b/chrome/browser/history/top_sites_unittest.cc index a6b7e7b..c327647 100644 --- a/chrome/browser/history/top_sites_unittest.cc +++ b/chrome/browser/history/top_sites_unittest.cc @@ -1,18 +1,27 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. +#include "app/l10n_util.h" +#include "base/file_util.h" #include "base/scoped_temp_dir.h" #include "base/string_util.h" +#include "base/utf_string_conversions.h" +#include "base/values.h" +#include "chrome/browser/chrome_thread.h" #include "chrome/browser/history/top_sites.h" -#include "chrome/common/chrome_paths.h" +#include "chrome/browser/dom_ui/most_visited_handler.h" #include "chrome/browser/history/history_marshaling.h" #include "chrome/browser/history/top_sites_database.h" #include "chrome/browser/history/history_notifications.h" +#include "chrome/common/chrome_paths.h" #include "chrome/test/testing_profile.h" #include "chrome/tools/profiles/thumbnail-inl.h" #include "gfx/codec/jpeg_codec.h" #include "googleurl/src/gurl.h" +#include "grit/chromium_strings.h" +#include "grit/generated_resources.h" +#include "grit/locale_settings.h" #include "testing/gtest/include/gtest/gtest.h" #include "third_party/skia/include/core/SkBitmap.h" @@ -38,6 +47,13 @@ class TopSitesTest : public testing::Test { RefCountedBytes* weewar_thumbnail() { return weewar_thumbnail_; } CancelableRequestConsumer* consumer() { return &consumer_; } size_t number_of_callbacks() {return number_of_callbacks_; } + // Prepopulated URLs - added at the back of TopSites. + GURL welcome_url() { + return GURL(l10n_util::GetStringUTF8(IDS_CHROME_WELCOME_URL)); + } + GURL themes_url() { + return GURL(l10n_util::GetStringUTF8(IDS_THEMES_GALLERY_URL)); + } virtual void SetUp() { profile_.reset(new TestingProfile); @@ -61,12 +77,12 @@ class TopSitesTest : public testing::Test { virtual void TearDown() { profile_.reset(); - top_sites_ = NULL; + TopSites::DeleteTopSites(top_sites_); EXPECT_TRUE(file_util::Delete(file_name_, false)); } // Callback for TopSites::GetMostVisitedURLs. - void OnTopSitesAvailable(const history::MostVisitedURLList& data) { + void OnTopSitesAvailable(history::MostVisitedURLList data) { urls_ = data; number_of_callbacks_++; } @@ -79,7 +95,6 @@ class TopSitesTest : public testing::Test { } void StoreMostVisited(std::vector<MostVisitedURL>* urls) { - AutoLock lock(top_sites_->lock_); // The function asserts it's in the lock. top_sites_->StoreMostVisited(urls); } @@ -92,6 +107,10 @@ class TopSitesTest : public testing::Test { added_urls, deleted_urls, moved_urls); } + Lock& lock() { + return top_sites_->lock_; + } + private: scoped_refptr<TopSites> top_sites_; MostVisitedURLList urls_; @@ -108,6 +127,7 @@ class TopSitesTest : public testing::Test { DISALLOW_COPY_AND_ASSIGN(TopSitesTest); }; + // A mockup of a HistoryService used for testing TopSites. class MockHistoryServiceImpl : public TopSites::MockHistoryService { public: @@ -151,7 +171,7 @@ class MockHistoryServiceImpl : public TopSites::MockHistoryService { MostVisitedURLList::iterator pos = std::find(most_visited_urls_.begin(), most_visited_urls_.end(), mvu); - EXPECT_TRUE(pos != most_visited_urls_.end()); + EXPECT_TRUE(pos != most_visited_urls_.end()) << url.spec(); scoped_refptr<RefCountedBytes> thumbnail; callback->Run(index, thumbnail); delete callback; @@ -175,14 +195,14 @@ class MockHistoryServiceImpl : public TopSites::MockHistoryService { class MockTopSitesDatabaseImpl : public TopSitesDatabase { public: virtual void GetPageThumbnails(MostVisitedURLList* urls, - std::map<GURL, TopSites::Images>* thumbnails) { + std::map<GURL, Images>* thumbnails) { // Return a copy of the vector. *urls = top_sites_list_; *thumbnails = thumbnails_map_; } virtual void SetPageThumbnail(const MostVisitedURL& url, int url_rank, - const TopSites::Images& thumbnail) { + const Images& thumbnail) { SetPageRank(url, url_rank); // Update thubmnail thumbnails_map_[url.url] = thumbnail; @@ -216,8 +236,8 @@ class MockTopSitesDatabaseImpl : public TopSitesDatabase { // Get a thumbnail for a given page. Returns true iff we have the thumbnail. virtual bool GetPageThumbnail(const GURL& url, - TopSites::Images* thumbnail) { - std::map<GURL, TopSites::Images>::const_iterator found = + Images* thumbnail) { + std::map<GURL, Images>::const_iterator found = thumbnails_map_.find(url); if (found == thumbnails_map_.end()) return false; // No thumbnail for this URL. @@ -242,7 +262,7 @@ class MockTopSitesDatabaseImpl : public TopSitesDatabase { private: MostVisitedURLList top_sites_list_; // Keeps the URLs sorted by score (rank). - std::map<GURL, TopSites::Images> thumbnails_map_; + std::map<GURL, Images> thumbnails_map_; }; @@ -294,9 +314,9 @@ TEST_F(TopSitesTest, GetCanonicalURL) { AppendMostVisitedURL(&most_visited, news); StoreMostVisited(&most_visited); - // Random URLs not in the database shouldn't be reported as being in there. + // Random URLs not in the database are returned unchanged. GURL result = GetCanonicalURL(GURL("http://fark.com/")); - EXPECT_TRUE(result.is_empty()); + EXPECT_EQ(GURL("http://fark.com/"), result); // Easy case, there are no redirects and the exact URL is stored. result = GetCanonicalURL(news); @@ -395,6 +415,50 @@ TEST_F(TopSitesTest, SetPageThumbnail) { EXPECT_FALSE(top_sites().SetPageThumbnail(url1a, thumbnail, medium_score)); } +TEST_F(TopSitesTest, GetPageThumbnail) { + ChromeThread db_loop(ChromeThread::DB, MessageLoop::current()); + MostVisitedURLList url_list; + MostVisitedURL url1 = {GURL("http://asdf.com")}; + url1.redirects.push_back(url1.url); + url_list.push_back(url1); + + MostVisitedURL url2 = {GURL("http://gmail.com")}; + url2.redirects.push_back(url2.url); + url2.redirects.push_back(GURL("http://mail.google.com")); + url_list.push_back(url2); + + top_sites().UpdateMostVisited(url_list); + MessageLoop::current()->RunAllPending(); + + // Create a dummy thumbnail. + SkBitmap thumbnail; + thumbnail.setConfig(SkBitmap::kARGB_8888_Config, 4, 4); + thumbnail.allocPixels(); + thumbnail.eraseRGB(0x00, 0x00, 0x00); + ThumbnailScore score(0.5, true, true, base::Time::Now()); + + RefCountedBytes* result = NULL; + EXPECT_TRUE(top_sites().SetPageThumbnail(url1.url, thumbnail, score)); + EXPECT_TRUE(top_sites().GetPageThumbnail(url1.url, &result)); + + EXPECT_TRUE(top_sites().SetPageThumbnail(GURL("http://gmail.com"), + thumbnail, score)); + EXPECT_TRUE(top_sites().GetPageThumbnail(GURL("http://gmail.com"), + &result)); + // Get a thumbnail via a redirect. + EXPECT_TRUE(top_sites().GetPageThumbnail(GURL("http://mail.google.com"), + &result)); + + EXPECT_TRUE(top_sites().SetPageThumbnail(GURL("http://mail.google.com"), + thumbnail, score)); + EXPECT_TRUE(top_sites().GetPageThumbnail(url2.url, &result)); + + scoped_ptr<SkBitmap> out_bitmap(gfx::JPEGCodec::Decode(result->front(), + result->size())); + EXPECT_EQ(0, memcmp(thumbnail.getPixels(), out_bitmap->getPixels(), + thumbnail.getSize())); +} + TEST_F(TopSitesTest, GetMostVisited) { ChromeThread db_loop(ChromeThread::DB, MessageLoop::current()); GURL news("http://news.google.com/"); @@ -411,9 +475,12 @@ TEST_F(TopSitesTest, GetMostVisited) { consumer(), NewCallback(static_cast<TopSitesTest*>(this), &TopSitesTest::OnTopSitesAvailable)); - ASSERT_EQ(2u, urls().size()); + // 2 extra prepopulated URLs. + ASSERT_EQ(4u, urls().size()); EXPECT_EQ(news, urls()[0].url); EXPECT_EQ(google, urls()[1].url); + EXPECT_EQ(welcome_url(), urls()[2].url); + EXPECT_EQ(themes_url(), urls()[3].url); } TEST_F(TopSitesTest, MockDatabase) { @@ -432,7 +499,7 @@ TEST_F(TopSitesTest, MockDatabase) { url.url = asdf_url; url.title = asdf_title; url.redirects.push_back(url.url); - TopSites::Images thumbnail; + Images thumbnail; db->SetPageThumbnail(url, 0, thumbnail); top_sites().ReadDatabase(); @@ -441,9 +508,11 @@ TEST_F(TopSitesTest, MockDatabase) { consumer(), NewCallback(static_cast<TopSitesTest*>(this), &TopSitesTest::OnTopSitesAvailable)); - ASSERT_EQ(1u, urls().size()); + ASSERT_EQ(3u, urls().size()); EXPECT_EQ(asdf_url, urls()[0].url); EXPECT_EQ(asdf_title, urls()[0].title); + EXPECT_EQ(welcome_url(), urls()[1].url); + EXPECT_EQ(themes_url(), urls()[2].url); MostVisitedURL url2; url2.url = google_url; @@ -459,11 +528,13 @@ TEST_F(TopSitesTest, MockDatabase) { consumer(), NewCallback(static_cast<TopSitesTest*>(this), &TopSitesTest::OnTopSitesAvailable)); - ASSERT_EQ(2u, urls().size()); + ASSERT_EQ(4u, urls().size()); EXPECT_EQ(google_url, urls()[0].url); EXPECT_EQ(google_title, urls()[0].title); EXPECT_EQ(asdf_url, urls()[1].url); EXPECT_EQ(asdf_title, urls()[1].title); + EXPECT_EQ(welcome_url(), urls()[2].url); + EXPECT_EQ(themes_url(), urls()[3].url); MockHistoryServiceImpl hs; // Add one old, one new URL to the history. @@ -475,10 +546,10 @@ TEST_F(TopSitesTest, MockDatabase) { top_sites().StartQueryForMostVisited(); MessageLoop::current()->RunAllPending(); - std::map<GURL, TopSites::Images> thumbnails; + std::map<GURL, Images> thumbnails; MostVisitedURLList result; db->GetPageThumbnails(&result, &thumbnails); - ASSERT_EQ(2u, result.size()); + ASSERT_EQ(4u, result.size()); EXPECT_EQ(google_title, result[0].title); EXPECT_EQ(news_title, result[1].title); } @@ -500,18 +571,18 @@ TEST_F(TopSitesTest, TopSitesDB) { url.url = asdf_url; url.title = asdf_title; url.redirects.push_back(url.url); - TopSites::Images thumbnail; + Images thumbnail; thumbnail.thumbnail = random_thumbnail(); // Add asdf at rank 0. db.SetPageThumbnail(url, 0, thumbnail); - TopSites::Images result; + Images result; EXPECT_TRUE(db.GetPageThumbnail(url.url, &result)); EXPECT_EQ(thumbnail.thumbnail->data.size(), result.thumbnail->data.size()); EXPECT_TRUE(ThumbnailsAreEqual(thumbnail.thumbnail, result.thumbnail)); MostVisitedURLList urls; - std::map<GURL, TopSites::Images> thumbnails; + std::map<GURL, Images> thumbnails; db.GetPageThumbnails(&urls, &thumbnails); ASSERT_EQ(1u, urls.size()); EXPECT_EQ(asdf_url, urls[0].url); @@ -587,7 +658,7 @@ TEST_F(TopSitesTest, RealDatabase) { url.url = asdf_url; url.title = asdf_title; url.redirects.push_back(url.url); - TopSites::Images thumbnail; + Images thumbnail; thumbnail.thumbnail = random_thumbnail(); db->SetPageThumbnail(url, 0, thumbnail); @@ -597,11 +668,13 @@ TEST_F(TopSitesTest, RealDatabase) { consumer(), NewCallback(static_cast<TopSitesTest*>(this), &TopSitesTest::OnTopSitesAvailable)); - ASSERT_EQ(1u, urls().size()); + ASSERT_EQ(3u, urls().size()); EXPECT_EQ(asdf_url, urls()[0].url); EXPECT_EQ(asdf_title, urls()[0].title); + EXPECT_EQ(welcome_url(), urls()[1].url); + EXPECT_EQ(themes_url(), urls()[2].url); - TopSites::Images img_result; + Images img_result; db->GetPageThumbnail(asdf_url, &img_result); EXPECT_TRUE(img_result.thumbnail != NULL); EXPECT_TRUE(ThumbnailsAreEqual(random_thumbnail(), img_result.thumbnail)); @@ -619,7 +692,7 @@ TEST_F(TopSitesTest, RealDatabase) { url2.redirects.push_back(google3_url); // Add new thumbnail at rank 0 and shift the other result to 1. - TopSites::Images g_thumbnail; + Images g_thumbnail; g_thumbnail.thumbnail = google_thumbnail(); db->SetPageThumbnail(url2, 0, g_thumbnail); @@ -629,7 +702,7 @@ TEST_F(TopSitesTest, RealDatabase) { consumer(), NewCallback(static_cast<TopSitesTest*>(this), &TopSitesTest::OnTopSitesAvailable)); - ASSERT_EQ(2u, urls().size()); + ASSERT_EQ(4u, urls().size()); EXPECT_EQ(google1_url, urls()[0].url); EXPECT_EQ(google_title, urls()[0].title); EXPECT_TRUE(top_sites().GetPageThumbnail(google1_url, &thumbnail_result)); @@ -641,6 +714,8 @@ TEST_F(TopSitesTest, RealDatabase) { EXPECT_EQ(asdf_url, urls()[1].url); EXPECT_EQ(asdf_title, urls()[1].title); + EXPECT_EQ(welcome_url(), urls()[2].url); + EXPECT_EQ(themes_url(), urls()[3].url); MockHistoryServiceImpl hs; // Add one old, one new URL to the history. @@ -652,10 +727,10 @@ TEST_F(TopSitesTest, RealDatabase) { top_sites().StartQueryForMostVisited(); MessageLoop::current()->RunAllPending(); - std::map<GURL, TopSites::Images> thumbnails; + std::map<GURL, Images> thumbnails; MostVisitedURLList results; db->GetPageThumbnails(&results, &thumbnails); - ASSERT_EQ(2u, results.size()); + ASSERT_EQ(4u, results.size()); EXPECT_EQ(google_title, results[0].title); EXPECT_EQ(news_title, results[1].title); @@ -673,7 +748,7 @@ TEST_F(TopSitesTest, RealDatabase) { *weewar_bitmap, medium_score)); RefCountedBytes* out_1; - TopSites::Images out_2; + Images out_2; EXPECT_TRUE(top_sites().GetPageThumbnail(google1_url, &out_1)); MessageLoop::current()->RunAllPending(); @@ -705,9 +780,7 @@ TEST_F(TopSitesTest, RealDatabase) { EXPECT_TRUE(high_score.Equals(out_2.thumbnail_score)); } -// This test has been crashing unit_tests on Mac 10.6. -// See http://crbug.com/49799 -TEST_F(TopSitesTest, DISABLED_DeleteNotifications) { +TEST_F(TopSitesTest, DeleteNotifications) { ChromeThread db_loop(ChromeThread::DB, MessageLoop::current()); GURL google1_url("http://google.com"); GURL google2_url("http://google.com/redirect"); @@ -731,35 +804,116 @@ TEST_F(TopSitesTest, DISABLED_DeleteNotifications) { consumer(), NewCallback(static_cast<TopSitesTest*>(this), &TopSitesTest::OnTopSitesAvailable)); - ASSERT_EQ(2u, urls().size()); + // 2 extra prepopulated URLs. + ASSERT_EQ(4u, urls().size()); hs.RemoveMostVisitedURL(); - history::URLsDeletedDetails details; - details.all_history = false; + history::URLsDeletedDetails history_details; + history_details.all_history = false; + Details<URLsDeletedDetails> details(&history_details); top_sites().Observe(NotificationType::HISTORY_URLS_DELETED, Source<Profile> (&profile()), - (const NotificationDetails&)details); + details); MessageLoop::current()->RunAllPending(); top_sites().GetMostVisitedURLs( consumer(), NewCallback(static_cast<TopSitesTest*>(this), &TopSitesTest::OnTopSitesAvailable)); - ASSERT_EQ(1u, urls().size()); + ASSERT_EQ(3u, urls().size()); EXPECT_EQ(google_title, urls()[0].title); + EXPECT_EQ(welcome_url(), urls()[1].url); + EXPECT_EQ(themes_url(), urls()[2].url); + + hs.RemoveMostVisitedURL(); + history_details.all_history = true; + details = Details<HistoryDetails>(&history_details); + top_sites().Observe(NotificationType::HISTORY_URLS_DELETED, + Source<Profile> (&profile()), + details); + MessageLoop::current()->RunAllPending(); + top_sites().GetMostVisitedURLs( + consumer(), + NewCallback(static_cast<TopSitesTest*>(this), + &TopSitesTest::OnTopSitesAvailable)); + ASSERT_EQ(2u, urls().size()); + EXPECT_EQ(welcome_url(), urls()[0].url); + EXPECT_EQ(themes_url(), urls()[1].url); +} + +TEST_F(TopSitesTest, PinnedURLsDeleted) { + ChromeThread db_loop(ChromeThread::DB, MessageLoop::current()); + GURL google1_url("http://google.com"); + GURL google2_url("http://google.com/redirect"); + GURL google3_url("http://www.google.com"); + string16 google_title(ASCIIToUTF16("Google")); + GURL news_url("http://news.google.com"); + string16 news_title(ASCIIToUTF16("Google News")); + + MockHistoryServiceImpl hs; + + top_sites().Init(file_name()); + + hs.AppendMockPage(google1_url, google_title); + hs.AppendMockPage(news_url, news_title); + top_sites().SetMockHistoryService(&hs); + + top_sites().StartQueryForMostVisited(); + MessageLoop::current()->RunAllPending(); + top_sites().GetMostVisitedURLs( + consumer(), + NewCallback(static_cast<TopSitesTest*>(this), + &TopSitesTest::OnTopSitesAvailable)); + MessageLoop::current()->RunAllPending(); + EXPECT_EQ(1u, number_of_callbacks()); + // 2 extra prepopulated URLs. + ASSERT_EQ(4u, urls().size()); + + top_sites().AddPinnedURL(news_url, 3); + EXPECT_TRUE(top_sites().IsURLPinned(news_url)); hs.RemoveMostVisitedURL(); - details.all_history = true; + history::URLsDeletedDetails history_details; + history_details.all_history = false; + history_details.urls.insert(news_url); + Details<URLsDeletedDetails> details(&history_details); top_sites().Observe(NotificationType::HISTORY_URLS_DELETED, Source<Profile> (&profile()), - (const NotificationDetails&)details); + details); MessageLoop::current()->RunAllPending(); top_sites().GetMostVisitedURLs( consumer(), NewCallback(static_cast<TopSitesTest*>(this), &TopSitesTest::OnTopSitesAvailable)); - ASSERT_EQ(0u, urls().size()); + MessageLoop::current()->RunAllPending(); + EXPECT_EQ(2u, number_of_callbacks()); + ASSERT_EQ(3u, urls().size()); + EXPECT_FALSE(top_sites().IsURLPinned(news_url)); + + hs.RemoveMostVisitedURL(); + history_details.all_history = true; + details = Details<HistoryDetails>(&history_details); + top_sites().Observe(NotificationType::HISTORY_URLS_DELETED, + Source<Profile> (&profile()), + details); + MessageLoop::current()->RunAllPending(); + top_sites().GetMostVisitedURLs( + consumer(), + NewCallback(static_cast<TopSitesTest*>(this), + &TopSitesTest::OnTopSitesAvailable)); + ASSERT_EQ(2u, urls().size()); + MessageLoop::current()->RunAllPending(); + + top_sites().StartQueryForMostVisited(); + MessageLoop::current()->RunAllPending(); + top_sites().GetMostVisitedURLs( + consumer(), + NewCallback(static_cast<TopSitesTest*>(this), + &TopSitesTest::OnTopSitesAvailable)); + ASSERT_EQ(2u, urls().size()); + EXPECT_EQ(welcome_url(), urls()[0].url); + EXPECT_EQ(themes_url(), urls()[1].url); } TEST_F(TopSitesTest, GetUpdateDelay) { @@ -780,8 +934,6 @@ TEST_F(TopSitesTest, GetUpdateDelay) { TEST_F(TopSitesTest, Migration) { ChromeThread db_loop(ChromeThread::DB, MessageLoop::current()); GURL google1_url("http://google.com"); - GURL google2_url("http://google.com/redirect"); - GURL google3_url("http://www.google.com"); string16 google_title(ASCIIToUTF16("Google")); GURL news_url("http://news.google.com"); string16 news_title(ASCIIToUTF16("Google News")); @@ -793,6 +945,7 @@ TEST_F(TopSitesTest, Migration) { hs.AppendMockPage(google1_url, google_title); hs.AppendMockPage(news_url, news_title); top_sites().SetMockHistoryService(&hs); + MessageLoop::current()->RunAllPending(); top_sites().StartMigration(); EXPECT_TRUE(top_sites().migration_in_progress_); @@ -837,9 +990,12 @@ TEST_F(TopSitesTest, QueueingRequestsForTopSites) { EXPECT_EQ(3u, number_of_callbacks()); - ASSERT_EQ(2u, urls().size()); + ASSERT_EQ(4u, urls().size()); EXPECT_EQ("http://1.com/", urls()[0].url.spec()); EXPECT_EQ("http://2.com/", urls()[1].url.spec()); + EXPECT_EQ(welcome_url(), urls()[2].url); + EXPECT_EQ(themes_url(), urls()[3].url); + url.url = GURL("http://3.com/"); url.redirects.push_back(url.url); @@ -854,13 +1010,17 @@ TEST_F(TopSitesTest, QueueingRequestsForTopSites) { EXPECT_EQ(4u, number_of_callbacks()); - ASSERT_EQ(3u, urls().size()); + ASSERT_EQ(5u, urls().size()); EXPECT_EQ("http://1.com/", urls()[0].url.spec()); EXPECT_EQ("http://2.com/", urls()[1].url.spec()); EXPECT_EQ("http://3.com/", urls()[2].url.spec()); + EXPECT_EQ(welcome_url(), urls()[3].url); + EXPECT_EQ(themes_url(), urls()[4].url); + } TEST_F(TopSitesTest, CancelingRequestsForTopSites) { + ChromeThread db_loop(ChromeThread::DB, MessageLoop::current()); CancelableRequestConsumer c1; CancelableRequestConsumer c2; top_sites().GetMostVisitedURLs( @@ -895,11 +1055,13 @@ TEST_F(TopSitesTest, CancelingRequestsForTopSites) { pages.push_back(url); top_sites().OnTopSitesAvailable(0, pages); + MessageLoop::current()->RunAllPending(); // 1 request was canceled. EXPECT_EQ(2u, number_of_callbacks()); - ASSERT_EQ(2u, urls().size()); + // 2 extra prepopulated URLs. + ASSERT_EQ(4u, urls().size()); EXPECT_EQ("http://1.com/", urls()[0].url.spec()); EXPECT_EQ("http://2.com/", urls()[1].url.spec()); } @@ -947,4 +1109,232 @@ TEST_F(TopSitesTest, AddTemporaryThumbnail) { thumbnail.getSize())); } +TEST_F(TopSitesTest, Blacklisting) { + ChromeThread db_loop(ChromeThread::DB, MessageLoop::current()); + MostVisitedURLList pages; + MostVisitedURL url, url1; + url.url = GURL("http://bbc.com/"); + url.redirects.push_back(url.url); + pages.push_back(url); + url1.url = GURL("http://google.com/"); + url1.redirects.push_back(url1.url); + pages.push_back(url1); + + CancelableRequestConsumer c; + top_sites().GetMostVisitedURLs( + &c, + NewCallback(static_cast<TopSitesTest*>(this), + &TopSitesTest::OnTopSitesAvailable)); + top_sites().OnTopSitesAvailable(0, pages); + MessageLoop::current()->RunAllPending(); + { + Lock& l = lock(); + AutoLock lock(l); // IsBlacklisted must be in a lock. + EXPECT_FALSE(top_sites().IsBlacklisted(GURL("http://bbc.com/"))); + } + + EXPECT_EQ(1u, number_of_callbacks()); + + ASSERT_EQ(4u, urls().size()); + EXPECT_EQ("http://bbc.com/", urls()[0].url.spec()); + EXPECT_EQ("http://google.com/", urls()[1].url.spec()); + EXPECT_EQ(welcome_url(), urls()[2].url); + EXPECT_EQ(themes_url(), urls()[3].url); + EXPECT_FALSE(top_sites().HasBlacklistedItems()); + + top_sites().AddBlacklistedURL(GURL("http://google.com/")); + EXPECT_TRUE(top_sites().HasBlacklistedItems()); + { + Lock& l = lock(); + AutoLock lock(l); // IsBlacklisted must be in a lock. + EXPECT_TRUE(top_sites().IsBlacklisted(GURL("http://google.com/"))); + EXPECT_FALSE(top_sites().IsBlacklisted(GURL("http://bbc.com/"))); + EXPECT_FALSE(top_sites().IsBlacklisted(welcome_url())); + } + + top_sites().GetMostVisitedURLs( + &c, + NewCallback(static_cast<TopSitesTest*>(this), + &TopSitesTest::OnTopSitesAvailable)); + MessageLoop::current()->RunAllPending(); + EXPECT_EQ(2u, number_of_callbacks()); + ASSERT_EQ(3u, urls().size()); + EXPECT_EQ("http://bbc.com/", urls()[0].url.spec()); + EXPECT_EQ(welcome_url(), urls()[1].url); + EXPECT_EQ(themes_url(), urls()[2].url); + + top_sites().AddBlacklistedURL(welcome_url()); + EXPECT_TRUE(top_sites().HasBlacklistedItems()); + top_sites().GetMostVisitedURLs( + &c, + NewCallback(static_cast<TopSitesTest*>(this), + &TopSitesTest::OnTopSitesAvailable)); + ASSERT_EQ(2u, urls().size()); + EXPECT_EQ("http://bbc.com/", urls()[0].url.spec()); + EXPECT_EQ(themes_url(), urls()[1].url); + + top_sites().RemoveBlacklistedURL(GURL("http://google.com/")); + EXPECT_TRUE(top_sites().HasBlacklistedItems()); + { + Lock& l = lock(); + AutoLock lock(l); // IsBlacklisted must be in a lock. + EXPECT_FALSE(top_sites().IsBlacklisted(GURL("http://google.com/"))); + } + + top_sites().GetMostVisitedURLs( + &c, + NewCallback(static_cast<TopSitesTest*>(this), + &TopSitesTest::OnTopSitesAvailable)); + ASSERT_EQ(3u, urls().size()); + EXPECT_EQ("http://bbc.com/", urls()[0].url.spec()); + EXPECT_EQ("http://google.com/", urls()[1].url.spec()); + EXPECT_EQ(themes_url(), urls()[2].url); + + top_sites().ClearBlacklistedURLs(); + EXPECT_FALSE(top_sites().HasBlacklistedItems()); + top_sites().GetMostVisitedURLs( + &c, + NewCallback(static_cast<TopSitesTest*>(this), + &TopSitesTest::OnTopSitesAvailable)); + ASSERT_EQ(4u, urls().size()); + EXPECT_EQ("http://bbc.com/", urls()[0].url.spec()); + EXPECT_EQ("http://google.com/", urls()[1].url.spec()); + EXPECT_EQ(welcome_url(), urls()[2].url); + EXPECT_EQ(themes_url(), urls()[3].url); +} + +TEST_F(TopSitesTest, PinnedURLs) { + ChromeThread db_loop(ChromeThread::DB, MessageLoop::current()); + MostVisitedURLList pages; + MostVisitedURL url, url1; + url.url = GURL("http://bbc.com/"); + url.redirects.push_back(url.url); + pages.push_back(url); + url1.url = GURL("http://google.com/"); + url1.redirects.push_back(url1.url); + pages.push_back(url1); + + CancelableRequestConsumer c; + top_sites().GetMostVisitedURLs( + &c, + NewCallback(static_cast<TopSitesTest*>(this), + &TopSitesTest::OnTopSitesAvailable)); + top_sites().OnTopSitesAvailable(0, pages); + MessageLoop::current()->RunAllPending(); + EXPECT_FALSE(top_sites().IsURLPinned(GURL("http://bbc.com/"))); + + ASSERT_EQ(4u, urls().size()); + EXPECT_EQ("http://bbc.com/", urls()[0].url.spec()); + EXPECT_EQ("http://google.com/", urls()[1].url.spec()); + EXPECT_EQ(welcome_url(), urls()[2].url); + EXPECT_EQ(themes_url(), urls()[3].url); + + top_sites().AddPinnedURL(GURL("http://google.com/"), 3); + EXPECT_FALSE(top_sites().IsURLPinned(GURL("http://bbc.com/"))); + EXPECT_FALSE(top_sites().IsURLPinned(welcome_url())); + + top_sites().GetMostVisitedURLs( + &c, + NewCallback(static_cast<TopSitesTest*>(this), + &TopSitesTest::OnTopSitesAvailable)); + EXPECT_EQ(2u, number_of_callbacks()); + ASSERT_EQ(4u, urls().size()); + EXPECT_EQ("http://bbc.com/", urls()[0].url.spec()); + EXPECT_EQ(welcome_url(), urls()[1].url); + EXPECT_EQ(themes_url(), urls()[2].url); + EXPECT_EQ("http://google.com/", urls()[3].url.spec()); + + top_sites().RemovePinnedURL(GURL("http://google.com/")); + EXPECT_FALSE(top_sites().IsURLPinned(GURL("http://google.com/"))); + top_sites().GetMostVisitedURLs( + &c, + NewCallback(static_cast<TopSitesTest*>(this), + &TopSitesTest::OnTopSitesAvailable)); + + ASSERT_EQ(4u, urls().size()); + EXPECT_EQ("http://bbc.com/", urls()[0].url.spec()); + EXPECT_EQ("http://google.com/", urls()[1].url.spec()); + EXPECT_EQ(welcome_url(), urls()[2].url); + EXPECT_EQ(themes_url(), urls()[3].url); + + top_sites().AddPinnedURL(GURL("http://bbc.com"), 1); + top_sites().AddPinnedURL(themes_url(), 0); + top_sites().GetMostVisitedURLs( + &c, + NewCallback(static_cast<TopSitesTest*>(this), + &TopSitesTest::OnTopSitesAvailable)); + + ASSERT_EQ(4u, urls().size()); + EXPECT_EQ(themes_url(), urls()[0].url); + EXPECT_EQ("http://bbc.com/", urls()[1].url.spec()); + EXPECT_EQ("http://google.com/", urls()[2].url.spec()); + EXPECT_EQ(welcome_url(), urls()[3].url); + + top_sites().RemovePinnedURL(GURL("http://bbc.com")); + top_sites().RemovePinnedURL(themes_url()); + + top_sites().AddPinnedURL(welcome_url(), 1); + top_sites().AddPinnedURL(GURL("http://bbc.com"), 3); + + top_sites().GetMostVisitedURLs( + &c, + NewCallback(static_cast<TopSitesTest*>(this), + &TopSitesTest::OnTopSitesAvailable)); + + ASSERT_EQ(4u, urls().size()); + EXPECT_EQ("http://google.com/", urls()[0].url.spec()); + EXPECT_EQ(welcome_url(), urls()[1].url); + EXPECT_EQ(themes_url(), urls()[2].url); + EXPECT_EQ("http://bbc.com/", urls()[3].url.spec()); +} + +TEST_F(TopSitesTest, BlacklistingAndPinnedURLs) { + ChromeThread db_loop(ChromeThread::DB, MessageLoop::current()); + MostVisitedURLList pages; + CancelableRequestConsumer c; + top_sites().GetMostVisitedURLs( + &c, + NewCallback(static_cast<TopSitesTest*>(this), + &TopSitesTest::OnTopSitesAvailable)); + top_sites().OnTopSitesAvailable(0, pages); + MessageLoop::current()->RunAllPending(); + + ASSERT_EQ(2u, urls().size()); + EXPECT_EQ(welcome_url(), urls()[0].url); + EXPECT_EQ(themes_url(), urls()[1].url); + + top_sites().AddPinnedURL(themes_url(), 1); + top_sites().AddBlacklistedURL(welcome_url()); + + top_sites().GetMostVisitedURLs( + &c, + NewCallback(static_cast<TopSitesTest*>(this), + &TopSitesTest::OnTopSitesAvailable)); + + ASSERT_EQ(2u, urls().size()); + EXPECT_EQ(GURL(), urls()[0].url); + EXPECT_EQ(themes_url(), urls()[1].url); + +} + +TEST_F(TopSitesTest, AddPrepopulatedPages) { + MostVisitedURLList pages; + top_sites().AddPrepopulatedPages(&pages); + ASSERT_EQ(2u, pages.size()); + EXPECT_EQ(welcome_url(), pages[0].url); + EXPECT_EQ(themes_url(), pages[1].url); + + pages.clear(); + + MostVisitedURL url = {themes_url()}; + pages.push_back(url); + + top_sites().AddPrepopulatedPages(&pages); + + // Themes URL is already in pages; should not be added twice. + ASSERT_EQ(2u, pages.size()); + EXPECT_EQ(themes_url(), pages[0].url); + EXPECT_EQ(welcome_url(), pages[1].url); +} + } // namespace history diff --git a/chrome/browser/history/url_database.cc b/chrome/browser/history/url_database.cc index 07f8881..b63d24a 100644 --- a/chrome/browser/history/url_database.cc +++ b/chrome/browser/history/url_database.cc @@ -10,7 +10,6 @@ #include <vector> #include "app/l10n_util.h" -#include "app/sql/connection.h" #include "app/sql/statement.h" #include "base/utf_string_conversions.h" #include "chrome/common/url_constants.h" @@ -346,7 +345,7 @@ bool URLDatabase::DropKeywordSearchTermsTable() { } bool URLDatabase::SetKeywordSearchTermsForURL(URLID url_id, - TemplateURL::IDType keyword_id, + TemplateURLID keyword_id, const string16& term) { DCHECK(url_id && keyword_id && !term.empty()); @@ -374,7 +373,7 @@ bool URLDatabase::SetKeywordSearchTermsForURL(URLID url_id, } void URLDatabase::DeleteAllSearchTermsForKeyword( - TemplateURL::IDType keyword_id) { + TemplateURLID keyword_id) { DCHECK(keyword_id); sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE, "DELETE FROM keyword_search_terms WHERE keyword_id=?")); @@ -386,7 +385,7 @@ void URLDatabase::DeleteAllSearchTermsForKeyword( } void URLDatabase::GetMostRecentKeywordSearchTerms( - TemplateURL::IDType keyword_id, + TemplateURLID keyword_id, const string16& prefix, int max_count, std::vector<KeywordSearchTermVisit>* matches) { diff --git a/chrome/browser/history/url_database.h b/chrome/browser/history/url_database.h index 84c8dde..36bfebb 100644 --- a/chrome/browser/history/url_database.h +++ b/chrome/browser/history/url_database.h @@ -4,11 +4,12 @@ #ifndef CHROME_BROWSER_HISTORY_URL_DATABASE_H_ #define CHROME_BROWSER_HISTORY_URL_DATABASE_H_ +#pragma once #include "app/sql/statement.h" #include "base/basictypes.h" #include "chrome/browser/history/history_types.h" -#include "chrome/browser/search_engines/template_url.h" +#include "chrome/browser/search_engines/template_url_id.h" class GURL; @@ -159,17 +160,17 @@ class URLDatabase { // Sets the search terms for the specified url/keyword pair. bool SetKeywordSearchTermsForURL(URLID url_id, - TemplateURL::IDType keyword_id, + TemplateURLID keyword_id, const string16& term); // Deletes all search terms for the specified keyword that have been added by // way of SetKeywordSearchTermsForURL. - void DeleteAllSearchTermsForKeyword(TemplateURL::IDType keyword_id); + void DeleteAllSearchTermsForKeyword(TemplateURLID keyword_id); // Returns up to max_count of the most recent search terms for the specified // keyword. void GetMostRecentKeywordSearchTerms( - TemplateURL::IDType keyword_id, + TemplateURLID keyword_id, const string16& prefix, int max_count, std::vector<KeywordSearchTermVisit>* matches); diff --git a/chrome/browser/history/url_database_unittest.cc b/chrome/browser/history/url_database_unittest.cc index 32ded0c..f567844 100644 --- a/chrome/browser/history/url_database_unittest.cc +++ b/chrome/browser/history/url_database_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. @@ -67,9 +67,9 @@ class URLDatabaseTest : public testing::Test, sql::Connection db_; }; -// Test add and query for the URL table in the HistoryDatabase +// Test add and query for the URL table in the HistoryDatabase. TEST_F(URLDatabaseTest, AddURL) { - // first, add two URLs + // First, add two URLs. const GURL url1("http://www.google.com/"); URLRow url_info1(url1); url_info1.set_title(UTF8ToUTF16("Google")); @@ -88,7 +88,7 @@ TEST_F(URLDatabaseTest, AddURL) { url_info2.set_hidden(true); EXPECT_TRUE(AddURL(url_info2)); - // query both of them + // Query both of them. URLRow info; EXPECT_TRUE(GetRowForURL(url1, &info)); EXPECT_TRUE(IsURLRowEqual(url_info1, info)); @@ -96,7 +96,7 @@ TEST_F(URLDatabaseTest, AddURL) { EXPECT_TRUE(id2); EXPECT_TRUE(IsURLRowEqual(url_info2, info)); - // update the second + // Update the second. url_info2.set_title(UTF8ToUTF16("Google Mail Too")); url_info2.set_visit_count(4); url_info2.set_typed_count(1); @@ -104,19 +104,19 @@ TEST_F(URLDatabaseTest, AddURL) { url_info2.set_hidden(false); EXPECT_TRUE(UpdateURLRow(id2, url_info2)); - // make sure it got updated + // Make sure it got updated. URLRow info2; EXPECT_TRUE(GetRowForURL(url2, &info2)); EXPECT_TRUE(IsURLRowEqual(url_info2, info2)); - // query a nonexistant URL + // Query a nonexistent URL. EXPECT_EQ(0, GetRowForURL(GURL("http://news.google.com/"), &info)); - // Delete all urls in the domain + // Delete all urls in the domain. // TODO(acw): test the new url based delete domain // EXPECT_TRUE(db.DeleteDomain(kDomainID)); - // Make sure the urls have been properly removed + // Make sure the urls have been properly removed. // TODO(acw): commented out because remove no longer works. // EXPECT_TRUE(db.GetURLInfo(url1, NULL) == NULL); // EXPECT_TRUE(db.GetURLInfo(url2, NULL) == NULL); diff --git a/chrome/browser/history/visit_database.cc b/chrome/browser/history/visit_database.cc index 80fa8c8..d0156c2 100644 --- a/chrome/browser/history/visit_database.cc +++ b/chrome/browser/history/visit_database.cc @@ -9,9 +9,9 @@ #include <map> #include <set> -#include "app/sql/connection.h" #include "app/sql/statement.h" #include "base/logging.h" +#include "base/string_number_conversions.h" #include "chrome/browser/history/url_database.h" #include "chrome/common/page_transition_types.h" #include "chrome/common/url_constants.h" @@ -51,6 +51,17 @@ bool VisitDatabase::InitVisitTable() { return false; } + // Visit source table contains the source information for all the visits. To + // save space, we do not record those user browsed visits which would be the + // majority in this table. Only other sources are recorded. + // Due to the tight relationship between visit_source and visits table, they + // should be created and dropped at the same time. + if (!GetDB().DoesTableExist("visit_source")) { + if (!GetDB().Execute("CREATE TABLE visit_source(" + "id INTEGER PRIMARY KEY,source INTEGER NOT NULL)")) + return false; + } + // Index over url so we can quickly find visits for a page. This will just // fail if it already exists and we'll ignore it. GetDB().Execute("CREATE INDEX visits_url_index ON visits (url)"); @@ -68,6 +79,7 @@ bool VisitDatabase::InitVisitTable() { } bool VisitDatabase::DropVisitTable() { + GetDB().Execute("DROP TABLE visit_source"); // This will also drop the indices over the table. return GetDB().Execute("DROP TABLE visits"); } @@ -94,7 +106,7 @@ void VisitDatabase::FillVisitVector(sql::Statement& statement, } } -VisitID VisitDatabase::AddVisit(VisitRow* visit) { +VisitID VisitDatabase::AddVisit(VisitRow* visit, VisitSource source) { sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE, "INSERT INTO visits " "(url, visit_time, from_visit, transition, segment_id, is_indexed) " @@ -112,6 +124,20 @@ VisitID VisitDatabase::AddVisit(VisitRow* visit) { return 0; visit->visit_id = GetDB().GetLastInsertRowId(); + + if (source != SOURCE_BROWSED) { + // Record the source of this visit when it is not browsed. + sql::Statement statement1(GetDB().GetCachedStatement(SQL_FROM_HERE, + "INSERT INTO visit_source (id, source) VALUES (?,?)")); + if (!statement1.is_valid()) + return 0; + + statement1.BindInt64(0, visit->visit_id); + statement1.BindInt64(1, source); + if (!statement1.Run()) + return 0; + } + return visit->visit_id; } @@ -133,6 +159,16 @@ void VisitDatabase::DeleteVisit(const VisitRow& visit) { return; del.BindInt64(0, visit.visit_id); del.Run(); + + // Try to delete the entry in visit_source table as well. + // If the visit was browsed, there is no corresponding entry in visit_source + // table, and nothing will be deleted. + del.Assign(GetDB().GetCachedStatement(SQL_FROM_HERE, + "DELETE FROM visit_source WHERE id=?")); + if (!del.is_valid()) + return; + del.BindInt64(0, visit.visit_id); + del.Run(); } bool VisitDatabase::GetRowForVisit(VisitID visit_id, VisitRow* out_visit) { @@ -437,4 +473,42 @@ bool VisitDatabase::GetStartDate(base::Time* first_visit) { return true; } +void VisitDatabase::GetVisitsSource(const VisitVector& visits, + VisitSourceMap* sources) { + DCHECK(sources); + sources->clear(); + + // We query the source in batch. Here defines the batch size. + const size_t batch_size = 500; + size_t visits_size = visits.size(); + + size_t start_index = 0, end_index = 0; + while (end_index < visits_size) { + start_index = end_index; + end_index = end_index + batch_size < visits_size ? end_index + batch_size + : visits_size; + + // Compose the sql statement with a list of ids. + std::string sql = "SELECT id,source FROM visit_source "; + sql.append("WHERE id IN ("); + // Append all the ids in the statement. + for (size_t j = start_index; j < end_index; j++) { + if (j != start_index) + sql.push_back(','); + sql.append(base::Int64ToString(visits[j].visit_id)); + } + sql.append(") ORDER BY id"); + sql::Statement statement(GetDB().GetUniqueStatement(sql.c_str())); + if (!statement) + return; + + // Get the source entries out of the query result. + while (statement.Step()) { + std::pair<VisitID, VisitSource> source_entry(statement.ColumnInt64(0), + static_cast<VisitSource>(statement.ColumnInt(1))); + sources->insert(source_entry); + } + } +} + } // namespace history diff --git a/chrome/browser/history/visit_database.h b/chrome/browser/history/visit_database.h index a6dbf3c..0e97cac 100644 --- a/chrome/browser/history/visit_database.h +++ b/chrome/browser/history/visit_database.h @@ -4,6 +4,7 @@ #ifndef CHROME_BROWSER_HISTORY_VISIT_DATABASE_H_ #define CHROME_BROWSER_HISTORY_VISIT_DATABASE_H_ +#pragma once #include "chrome/browser/history/history_types.h" @@ -33,8 +34,9 @@ class VisitDatabase { // Adds a line to the visit database with the given information, returning // the added row ID on success, 0 on failure. The given visit is updated with - // the new row ID on success. - VisitID AddVisit(VisitRow* visit); + // the new row ID on success. In addition, adds its source into visit_source + // table. + VisitID AddVisit(VisitRow* visit, VisitSource source); // Deletes the given visit from the database. If a visit with the given ID // doesn't exist, it will not do anything. @@ -143,6 +145,10 @@ class VisitDatabase { // Get the time of the first item in our database. bool GetStartDate(base::Time* first_visit); + // Get the source information about the given visits. + void GetVisitsSource(const VisitVector& visits, + VisitSourceMap* sources); + protected: // Returns the database for the functions in this interface. virtual sql::Connection& GetDB() = 0; diff --git a/chrome/browser/history/visit_database_unittest.cc b/chrome/browser/history/visit_database_unittest.cc index ebc2e1b..28a0e15 100644 --- a/chrome/browser/history/visit_database_unittest.cc +++ b/chrome/browser/history/visit_database_unittest.cc @@ -73,19 +73,19 @@ class VisitDatabaseTest : public PlatformTest, TEST_F(VisitDatabaseTest, Add) { // Add one visit. VisitRow visit_info1(1, Time::Now(), 0, PageTransition::LINK, 0); - EXPECT_TRUE(AddVisit(&visit_info1)); + EXPECT_TRUE(AddVisit(&visit_info1, SOURCE_BROWSED)); // Add second visit for the same page. VisitRow visit_info2(visit_info1.url_id, visit_info1.visit_time + TimeDelta::FromSeconds(1), 1, PageTransition::TYPED, 0); - EXPECT_TRUE(AddVisit(&visit_info2)); + EXPECT_TRUE(AddVisit(&visit_info2, SOURCE_BROWSED)); // Add third visit for a different page. VisitRow visit_info3(2, visit_info1.visit_time + TimeDelta::FromSeconds(2), 0, PageTransition::LINK, 0); - EXPECT_TRUE(AddVisit(&visit_info3)); + EXPECT_TRUE(AddVisit(&visit_info3, SOURCE_BROWSED)); // Query the first two. std::vector<VisitRow> matches; @@ -104,17 +104,17 @@ TEST_F(VisitDatabaseTest, Delete) { static const int kTime1 = 1000; VisitRow visit_info1(1, Time::FromInternalValue(kTime1), 0, PageTransition::LINK, 0); - EXPECT_TRUE(AddVisit(&visit_info1)); + EXPECT_TRUE(AddVisit(&visit_info1, SOURCE_BROWSED)); static const int kTime2 = kTime1 + 1; VisitRow visit_info2(1, Time::FromInternalValue(kTime2), visit_info1.visit_id, PageTransition::LINK, 0); - EXPECT_TRUE(AddVisit(&visit_info2)); + EXPECT_TRUE(AddVisit(&visit_info2, SOURCE_BROWSED)); static const int kTime3 = kTime2 + 1; VisitRow visit_info3(1, Time::FromInternalValue(kTime3), visit_info2.visit_id, PageTransition::LINK, 0); - EXPECT_TRUE(AddVisit(&visit_info3)); + EXPECT_TRUE(AddVisit(&visit_info3, SOURCE_BROWSED)); // First make sure all the visits are there. std::vector<VisitRow> matches; @@ -140,7 +140,7 @@ TEST_F(VisitDatabaseTest, Delete) { TEST_F(VisitDatabaseTest, Update) { // Make something in the database. VisitRow original(1, Time::Now(), 23, 22, 19); - AddVisit(&original); + AddVisit(&original, SOURCE_BROWSED); // Mutate that row. VisitRow modification(original); @@ -167,7 +167,7 @@ TEST_F(VisitDatabaseTest, GetVisibleVisitsInRange) { PageTransition::CHAIN_END), 0); visit_info1.visit_id = 1; - EXPECT_TRUE(AddVisit(&visit_info1)); + EXPECT_TRUE(AddVisit(&visit_info1, SOURCE_BROWSED)); // Add second visit for the same page. VisitRow visit_info2(visit_info1.url_id, @@ -177,7 +177,7 @@ TEST_F(VisitDatabaseTest, GetVisibleVisitsInRange) { PageTransition::CHAIN_END), 0); visit_info2.visit_id = 2; - EXPECT_TRUE(AddVisit(&visit_info2)); + EXPECT_TRUE(AddVisit(&visit_info2, SOURCE_BROWSED)); // Add third visit for a different page. VisitRow visit_info3(2, @@ -186,7 +186,7 @@ TEST_F(VisitDatabaseTest, GetVisibleVisitsInRange) { PageTransition::CHAIN_START), 0); visit_info3.visit_id = 3; - EXPECT_TRUE(AddVisit(&visit_info3)); + EXPECT_TRUE(AddVisit(&visit_info3, SOURCE_BROWSED)); // Add a redirect visit from the last page. VisitRow visit_info4(3, @@ -195,7 +195,7 @@ TEST_F(VisitDatabaseTest, GetVisibleVisitsInRange) { PageTransition::CHAIN_END), 0); visit_info4.visit_id = 4; - EXPECT_TRUE(AddVisit(&visit_info4)); + EXPECT_TRUE(AddVisit(&visit_info4, SOURCE_BROWSED)); // Add a subframe visit. VisitRow visit_info5(4, @@ -205,7 +205,7 @@ TEST_F(VisitDatabaseTest, GetVisibleVisitsInRange) { PageTransition::CHAIN_END), 0); visit_info5.visit_id = 5; - EXPECT_TRUE(AddVisit(&visit_info5)); + EXPECT_TRUE(AddVisit(&visit_info5, SOURCE_BROWSED)); // Query the visits for all time, we should not get the first (duplicate of // the second) or the redirect or subframe visits. @@ -227,4 +227,37 @@ TEST_F(VisitDatabaseTest, GetVisibleVisitsInRange) { ASSERT_EQ(static_cast<size_t>(1), results.size()); EXPECT_TRUE(IsVisitInfoEqual(results[0], visit_info4)); } + +TEST_F(VisitDatabaseTest, VisitSource) { + // Add visits. + VisitRow visit_info1(111, Time::Now(), 0, PageTransition::LINK, 0); + ASSERT_TRUE(AddVisit(&visit_info1, SOURCE_BROWSED)); + + VisitRow visit_info2(112, Time::Now(), 1, PageTransition::TYPED, 0); + ASSERT_TRUE(AddVisit(&visit_info2, SOURCE_SYNCED)); + + VisitRow visit_info3(113, Time::Now(), 0, PageTransition::TYPED, 0); + ASSERT_TRUE(AddVisit(&visit_info3, SOURCE_EXTENSION)); + + // Query each visit. + std::vector<VisitRow> matches; + ASSERT_TRUE(GetVisitsForURL(111, &matches)); + ASSERT_EQ(1U, matches.size()); + VisitSourceMap sources; + GetVisitsSource(matches, &sources); + EXPECT_EQ(0U, sources.size()); + + ASSERT_TRUE(GetVisitsForURL(112, &matches)); + ASSERT_EQ(1U, matches.size()); + GetVisitsSource(matches, &sources); + ASSERT_EQ(1U, sources.size()); + EXPECT_EQ(SOURCE_SYNCED, sources[matches[0].visit_id]); + + ASSERT_TRUE(GetVisitsForURL(113, &matches)); + ASSERT_EQ(1U, matches.size()); + GetVisitsSource(matches, &sources); + ASSERT_EQ(1U, sources.size()); + EXPECT_EQ(SOURCE_EXTENSION, sources[matches[0].visit_id]); +} + } // namespace history diff --git a/chrome/browser/history/visit_tracker.h b/chrome/browser/history/visit_tracker.h index 43de7a0..281c136 100644 --- a/chrome/browser/history/visit_tracker.h +++ b/chrome/browser/history/visit_tracker.h @@ -4,6 +4,7 @@ #ifndef CHROME_BROWSER_HISTORY_VISIT_TRACKER_H__ #define CHROME_BROWSER_HISTORY_VISIT_TRACKER_H__ +#pragma once #include <map> #include <vector> diff --git a/chrome/browser/history/visitsegment_database.cc b/chrome/browser/history/visitsegment_database.cc index f94d713..f4c4f9f 100644 --- a/chrome/browser/history/visitsegment_database.cc +++ b/chrome/browser/history/visitsegment_database.cc @@ -10,7 +10,6 @@ #include <string> #include <vector> -#include "app/sql/connection.h" #include "app/sql/statement.h" #include "base/logging.h" #include "base/stl_util-inl.h" diff --git a/chrome/browser/history/visitsegment_database.h b/chrome/browser/history/visitsegment_database.h index 16f0417..7c25ef7 100644 --- a/chrome/browser/history/visitsegment_database.h +++ b/chrome/browser/history/visitsegment_database.h @@ -4,6 +4,7 @@ #ifndef CHROME_BROWSER_HISTORY_VISITSEGMENT_DATABASE_H_ #define CHROME_BROWSER_HISTORY_VISITSEGMENT_DATABASE_H_ +#pragma once #include "base/basictypes.h" #include "chrome/browser/history/history_types.h" |