From 9ba20809ed26f2dc47444c44e1f34b810b2760fc Mon Sep 17 00:00:00 2001
From: "nshkrob@chromium.org"
<nshkrob@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Tue, 10 Aug 2010 20:02:08 +0000
Subject: Replace --top-sites flag with --no-top-sites flag. TopSites becomes
the default.
BUG=none
TEST=manual
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=55244
Review URL: http://codereview.chromium.org/3054028
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@55610 0039d316-1c4b-4281-b951-d872f2087c98
---
chrome/browser/chrome_thread.h | 2 +-
chrome/browser/dom_ui/dom_ui_thumbnail_source.cc | 4 +-
chrome/browser/dom_ui/most_visited_handler.cc | 14 +-
.../history/expire_history_backend_unittest.cc | 20 ++-
chrome/browser/history/history_backend.cc | 4 +-
chrome/browser/history/history_backend_unittest.cc | 8 +
chrome/browser/history/history_database.cc | 13 +-
chrome/browser/history/history_types.h | 11 ++
chrome/browser/history/history_unittest.cc | 5 +
chrome/browser/history/thumbnail_database.cc | 18 +-
.../browser/history/thumbnail_database_unittest.cc | 20 +++
chrome/browser/history/top_sites.cc | 183 ++++++++++++++-------
chrome/browser/history/top_sites.h | 49 +++---
chrome/browser/history/top_sites_database.cc | 15 +-
chrome/browser/history/top_sites_database.h | 21 ++-
chrome/browser/history/top_sites_unittest.cc | 64 ++++---
chrome/browser/profile_impl.cc | 3 +
chrome/browser/tab_contents/tab_contents.cc | 2 +-
chrome/common/chrome_switches.cc | 7 +-
chrome/common/chrome_switches.h | 2 +-
.../data/profiles/typical_history/Default/Favicons | Bin 0 -> 20480 bytes
.../data/profiles/typical_history/Default/History | Bin 13770752 -> 13770752 bytes
.../profiles/typical_history/Default/Thumbnails | Bin 32768 -> 0 bytes
.../profiles/typical_history/Default/Top Sites | Bin 0 -> 12288 bytes
chrome/test/testing_profile.cc | 4 +
chrome/test/testing_profile.h | 15 +-
26 files changed, 326 insertions(+), 158 deletions(-)
create mode 100644 chrome/test/data/profiles/typical_history/Default/Favicons
delete mode 100644 chrome/test/data/profiles/typical_history/Default/Thumbnails
create mode 100644 chrome/test/data/profiles/typical_history/Default/Top Sites
(limited to 'chrome')
diff --git a/chrome/browser/chrome_thread.h b/chrome/browser/chrome_thread.h
index c3d6f97..3831b7c 100644
--- a/chrome/browser/chrome_thread.h
+++ b/chrome/browser/chrome_thread.h
@@ -165,7 +165,7 @@ class ChromeThread : public base::Thread {
//
// ...
// private:
- // friend class ChromeThread;
+ // friend struct ChromeThread::DeleteOnThread<ChromeThread::IO>;
// friend class DeleteTask<Foo>;
//
// ~Foo();
diff --git a/chrome/browser/dom_ui/dom_ui_thumbnail_source.cc b/chrome/browser/dom_ui/dom_ui_thumbnail_source.cc
index fd6eae7..ebff335 100644
--- a/chrome/browser/dom_ui/dom_ui_thumbnail_source.cc
+++ b/chrome/browser/dom_ui/dom_ui_thumbnail_source.cc
@@ -28,8 +28,8 @@ DOMUIThumbnailSource::~DOMUIThumbnailSource() {
void DOMUIThumbnailSource::StartDataRequest(const std::string& path,
bool is_off_the_record,
int request_id) {
- if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kTopSites)) {
- scoped_refptr<history::TopSites> top_sites = profile_->GetTopSites();
+ if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoTopSites)) {
+ history::TopSites* top_sites = profile_->GetTopSites();
RefCountedBytes* data = NULL;
if (top_sites->GetPageThumbnail(GURL(path), &data)) {
// We have the thumbnail.
diff --git a/chrome/browser/dom_ui/most_visited_handler.cc b/chrome/browser/dom_ui/most_visited_handler.cc
index 1670ac3..be9f9c5 100644
--- a/chrome/browser/dom_ui/most_visited_handler.cc
+++ b/chrome/browser/dom_ui/most_visited_handler.cc
@@ -151,7 +151,7 @@ void MostVisitedHandler::SendPagesValue() {
}
void MostVisitedHandler::StartQueryForMostVisited() {
- if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kTopSites)) {
+ if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoTopSites)) {
// Use TopSites.
history::TopSites* ts = dom_ui_->GetProfile()->GetTopSites();
ts->GetMostVisitedURLs(
@@ -213,7 +213,7 @@ void MostVisitedHandler::HandleRemoveURLsFromBlacklist(const Value* urls) {
}
UserMetrics::RecordAction(UserMetricsAction("MostVisited_UrlRemoved"),
dom_ui_->GetProfile());
- if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kTopSites)) {
+ if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoTopSites)) {
history::TopSites* ts = dom_ui_->GetProfile()->GetTopSites();
ts->RemoveBlacklistedURL(GURL(url));
return;
@@ -228,7 +228,7 @@ void MostVisitedHandler::HandleClearBlacklist(const Value* value) {
UserMetrics::RecordAction(UserMetricsAction("MostVisited_BlacklistCleared"),
dom_ui_->GetProfile());
- if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kTopSites)) {
+ if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoTopSites)) {
history::TopSites* ts = dom_ui_->GetProfile()->GetTopSites();
ts->ClearBlacklistedURLs();
return;
@@ -278,7 +278,7 @@ void MostVisitedHandler::HandleAddPinnedURL(const Value* value) {
}
void MostVisitedHandler::AddPinnedURL(const MostVisitedPage& page, int index) {
- if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kTopSites)) {
+ if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoTopSites)) {
history::TopSites* ts = dom_ui_->GetProfile()->GetTopSites();
ts->AddPinnedURL(page.url, index);
return;
@@ -317,7 +317,7 @@ void MostVisitedHandler::HandleRemovePinnedURL(const Value* value) {
}
void MostVisitedHandler::RemovePinnedURL(const GURL& url) {
- if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kTopSites)) {
+ if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoTopSites)) {
history::TopSites* ts = dom_ui_->GetProfile()->GetTopSites();
ts->RemovePinnedURL(url);
return;
@@ -487,7 +487,7 @@ void MostVisitedHandler::SetPagesValue(std::vector<PageUsageData*>* data) {
void MostVisitedHandler::SetPagesValueFromTopSites(
const history::MostVisitedURLList& data) {
- DCHECK(CommandLine::ForCurrentProcess()->HasSwitch(switches::kTopSites));
+ DCHECK(!CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoTopSites));
pages_value_.reset(new ListValue);
for (size_t i = 0; i < data.size(); i++) {
const history::MostVisitedURL& url = data[i];
@@ -608,7 +608,7 @@ void MostVisitedHandler::Observe(NotificationType type,
}
void MostVisitedHandler::BlacklistURL(const GURL& url) {
- if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kTopSites)) {
+ if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoTopSites)) {
history::TopSites* ts = dom_ui_->GetProfile()->GetTopSites();
ts->AddBlacklistedURL(url);
return;
diff --git a/chrome/browser/history/expire_history_backend_unittest.cc b/chrome/browser/history/expire_history_backend_unittest.cc
index ca822bc..95b391f 100644
--- a/chrome/browser/history/expire_history_backend_unittest.cc
+++ b/chrome/browser/history/expire_history_backend_unittest.cc
@@ -16,8 +16,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"
@@ -90,6 +92,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 +138,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 +150,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,9 +217,9 @@ 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;
@@ -271,8 +277,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) {
diff --git a/chrome/browser/history/history_backend.cc b/chrome/browser/history/history_backend.cc
index 8e1c5fb..1c58f74 100644
--- a/chrome/browser/history/history_backend.cc
+++ b/chrome/browser/history/history_backend.cc
@@ -581,7 +581,7 @@ void HistoryBackend::InitImpl() {
// Thumbnail database.
thumbnail_db_.reset(new ThumbnailDatabase());
- if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kTopSites)) {
+ if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoTopSites)) {
if (!db_->needs_version_18_migration()) {
// No convertion needed - use new filename right away.
thumbnail_name = GetFaviconsFileName();
@@ -598,7 +598,7 @@ void HistoryBackend::InitImpl() {
thumbnail_db_.reset();
}
- if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kTopSites)) {
+ if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoTopSites)) {
if (db_->needs_version_18_migration()) {
LOG(INFO) << "Starting TopSites migration";
delegate_->StartTopSitesMigration();
diff --git a/chrome/browser/history/history_backend_unittest.cc b/chrome/browser/history/history_backend_unittest.cc
index 24cc1cd..780038d 100644
--- a/chrome/browser/history/history_backend_unittest.cc
+++ b/chrome/browser/history/history_backend_unittest.cc
@@ -4,6 +4,7 @@
#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"
@@ -13,6 +14,7 @@
#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/common/chrome_switches.h"
#include "chrome/common/notification_service.h"
#include "chrome/common/thumbnail_score.h"
#include "chrome/tools/profiles/thumbnail-inl.h"
@@ -402,6 +404,9 @@ TEST_F(HistoryBackendTest, URLsNoLongerBookmarked) {
TEST_F(HistoryBackendTest, GetPageThumbnailAfterRedirects) {
ASSERT_TRUE(backend_.get());
+ if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoTopSites))
+ return;
+
const char* base_url = "http://mail";
const char* thumbnail_url = "http://mail.google.com";
@@ -598,6 +603,9 @@ TEST_F(HistoryBackendTest, StripUsernamePasswordTest) {
}
TEST_F(HistoryBackendTest, DeleteThumbnailsDatabaseTest) {
+ if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoTopSites))
+ return;
+
EXPECT_TRUE(backend_->thumbnail_db_->NeedsMigrationToTopSites());
backend_->delegate_->StartTopSitesMigration();
EXPECT_FALSE(backend_->thumbnail_db_->NeedsMigrationToTopSites());
diff --git a/chrome/browser/history/history_database.cc b/chrome/browser/history/history_database.cc
index 26bb81a..7869fcc 100644
--- a/chrome/browser/history/history_database.cc
+++ b/chrome/browser/history/history_database.cc
@@ -134,14 +134,11 @@ 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)) {
+ // If we don't use TopSites, we are one version number behind.
+ if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoTopSites))
+ return 17; // Last pre-TopSites version.
+ else
return kCurrentVersionNumber;
- } else {
- return kCurrentVersionNumber - 1;
- }
}
void HistoryDatabase::BeginTransaction() {
@@ -286,7 +283,7 @@ sql::InitStatus HistoryDatabase::EnsureCurrentVersion(
if (cur_version == 17)
needs_version_18_migration_ = true;
- if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kTopSites) &&
+ if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoTopSites) &&
cur_version == 18) {
// Set DB version back to pre-top sites.
cur_version = 17;
diff --git a/chrome/browser/history/history_types.h b/chrome/browser/history/history_types.h
index 0c283b1..21e8067 100644
--- a/chrome/browser/history/history_types.h
+++ b/chrome/browser/history/history_types.h
@@ -13,12 +13,14 @@
#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 {
@@ -527,6 +529,15 @@ 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;
// Used for intermediate history result operations.
diff --git a/chrome/browser/history/history_unittest.cc b/chrome/browser/history/history_unittest.cc
index c8db05a..063414f 100644
--- a/chrome/browser/history/history_unittest.cc
+++ b/chrome/browser/history/history_unittest.cc
@@ -24,6 +24,7 @@
#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"
@@ -41,6 +42,7 @@
#include "chrome/browser/history/in_memory_history_backend.h"
#include "chrome/browser/history/page_usage_data.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"
@@ -685,6 +687,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 (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoTopSites))
+ return; // TopSitesTest replaces this.
+
scoped_refptr<HistoryService> history(new HistoryService);
history_service_ = history;
ASSERT_TRUE(history->Init(history_dir_, NULL));
diff --git a/chrome/browser/history/thumbnail_database.cc b/chrome/browser/history/thumbnail_database.cc
index 8bf203d..979accf 100644
--- a/chrome/browser/history/thumbnail_database.cc
+++ b/chrome/browser/history/thumbnail_database.cc
@@ -130,7 +130,7 @@ sql::InitStatus ThumbnailDatabase::OpenDatabase(sql::Connection* db,
bool ThumbnailDatabase::InitThumbnailTable() {
if (!db_.DoesTableExist("thumbnails")) {
- if (CommandLine::ForCurrentProcess()-> HasSwitch(switches::kTopSites)) {
+ if (!CommandLine::ForCurrentProcess()-> HasSwitch(switches::kNoTopSites)) {
use_top_sites_ = true;
return true;
}
@@ -231,8 +231,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 +288,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 +307,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 +323,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.
diff --git a/chrome/browser/history/thumbnail_database_unittest.cc b/chrome/browser/history/thumbnail_database_unittest.cc
index 4d2c2bf..24c990b 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,7 @@
#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/common/thumbnail_score.h"
#include "chrome/tools/profiles/thumbnail-inl.h"
#include "gfx/codec/jpeg_codec.h"
@@ -70,6 +72,9 @@ class ThumbnailDatabaseTest : public testing::Test {
};
TEST_F(ThumbnailDatabaseTest, AddDelete) {
+ if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoTopSites))
+ return; // TopSitesTest replaces this.
+
ThumbnailDatabase db;
ASSERT_EQ(sql::INIT_OK, db.Init(file_name_, NULL));
@@ -112,6 +117,9 @@ TEST_F(ThumbnailDatabaseTest, AddDelete) {
}
TEST_F(ThumbnailDatabaseTest, UseLessBoringThumbnails) {
+ if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoTopSites))
+ return; // TopSitesTest replaces this.
+
ThumbnailDatabase db;
Time now = Time::Now();
ASSERT_EQ(sql::INIT_OK, db.Init(file_name_, NULL));
@@ -146,6 +154,9 @@ TEST_F(ThumbnailDatabaseTest, UseLessBoringThumbnails) {
}
TEST_F(ThumbnailDatabaseTest, UseAtTopThumbnails) {
+ if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoTopSites))
+ return; // TopSitesTest replaces this.
+
ThumbnailDatabase db;
Time now = Time::Now();
ASSERT_EQ(sql::INIT_OK, db.Init(file_name_, NULL));
@@ -217,6 +228,9 @@ TEST_F(ThumbnailDatabaseTest, UseAtTopThumbnails) {
}
TEST_F(ThumbnailDatabaseTest, ThumbnailTimeDegradation) {
+ if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoTopSites))
+ return; // TopSitesTest replaces this.
+
ThumbnailDatabase db;
const Time kNow = Time::Now();
const Time kThreeHoursAgo = kNow - TimeDelta::FromHours(4);
@@ -261,6 +275,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 (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoTopSites))
+ return; // TopSitesTest replaces this.
+
ThumbnailDatabase db;
Time now = Time::Now();
ASSERT_EQ(sql::INIT_OK, db.Init(file_name_, NULL));
@@ -332,6 +349,9 @@ TEST_F(ThumbnailDatabaseTest, NeverAcceptTotallyBoringThumbnail) {
}
TEST_F(ThumbnailDatabaseTest, NeedsMigrationToTopSites) {
+ if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoTopSites))
+ 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 a7e5deb..d1b349c 100644
--- a/chrome/browser/history/top_sites.cc
+++ b/chrome/browser/history/top_sites.cc
@@ -54,10 +54,15 @@ TopSites::TopSites(Profile* profile) : profile_(profile),
waiting_for_results_(true),
blacklist_(NULL),
pinned_urls_(NULL) {
- registrar_.Add(this, NotificationType::HISTORY_URLS_DELETED,
- Source<Profile>(profile_));
- registrar_.Add(this, NotificationType::NAV_ENTRY_COMMITTED,
- NotificationService::AllSources());
+ 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);
@@ -90,16 +95,14 @@ void TopSites::ReadDatabase() {
std::map<GURL, Images> thumbnails;
DCHECK(db_.get());
- {
- AutoLock lock(lock_);
- MostVisitedURLList top_urls;
- db_->GetPageThumbnails(&top_urls, &thumbnails);
- MostVisitedURLList copy(top_urls); // StoreMostVisited destroys the list.
- StoreMostVisited(&top_urls);
- if (AddPrepopulatedPages(©))
- UpdateMostVisited(copy);
- } // 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];
@@ -145,12 +148,14 @@ bool TopSites::SetPageThumbnail(const GURL& url,
return true;
}
- return SetPageThumbnail(url, thumbnail_data, score);
+ AutoLock lock(lock_);
+ 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;
@@ -171,7 +176,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);
@@ -181,7 +186,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()) {
@@ -237,15 +242,21 @@ void TopSites::GetMostVisitedURLs(CancelableRequestConsumer* consumer,
return;
MostVisitedURLList filtered_urls;
- ApplyBlacklistAndPinnedURLs(top_sites_, &filtered_urls);
-
+ {
+ AutoLock lock(lock_);
+ ApplyBlacklistAndPinnedURLs(top_sites_, &filtered_urls);
+ }
request->ForwardResult(GetTopSitesCallback::TupleType(filtered_urls));
}
bool TopSites::GetPageThumbnail(const GURL& url, RefCountedBytes** data) const {
+ AutoLock lock(lock_);
std::map<GURL, Images>::const_iterator found = top_images_.find(url);
- if (found == top_images_.end())
- return false; // No thumbnail for this 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();
@@ -262,8 +273,11 @@ static int IndexOf(const MostVisitedURLList& urls, const GURL& url) {
int TopSites::GetIndexForChromeStore(const MostVisitedURLList& urls) {
GURL store_url = MostVisitedHandler::GetChromeStoreURLWithLocale();
- if (IsBlacklisted(store_url))
- return -1;
+ {
+ AutoLock lock(lock_);
+ if (IsBlacklisted(store_url))
+ return -1;
+ }
if (IndexOf(urls, store_url) != -1)
return -1; // It's already there, no need to add.
@@ -290,6 +304,9 @@ bool TopSites::AddChromeStore(MostVisitedURLList* urls) {
if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kDisableApps))
return false;
+ if (!profile_)
+ return false;
+
ExtensionsService* service = profile_->GetExtensionsService();
if (!service || service->HasApps())
return false;
@@ -371,6 +388,7 @@ void TopSites::MigratePinnedURLs() {
void TopSites::ApplyBlacklistAndPinnedURLs(const MostVisitedURLList& urls,
MostVisitedURLList* out) {
MostVisitedURLList urls_copy;
+ lock_.AssertAcquired();
for (size_t i = 0; i < urls.size(); i++) {
if (!IsBlacklisted(urls[i].url))
urls_copy.push_back(urls[i]);
@@ -417,10 +435,12 @@ void TopSites::ApplyBlacklistAndPinnedURLs(const MostVisitedURLList& urls,
}
std::wstring TopSites::GetURLString(const GURL& url) {
+ lock_.AssertAcquired();
return ASCIIToWide(GetCanonicalURL(url).spec());
}
std::wstring TopSites::GetURLHash(const GURL& url) {
+ lock_.AssertAcquired();
return ASCIIToWide(MD5String(GetCanonicalURL(url).spec()));
}
@@ -430,27 +450,33 @@ void TopSites::UpdateMostVisited(MostVisitedURLList most_visited) {
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();
- // Process the diff: delete from images and disk, add to disk.
- // Delete all the thumbnails associated with URLs that were deleted.
- for (size_t i = 0; i < deleted.size(); i++) {
- const MostVisitedURL& deleted_url = top_sites_[deleted[i]];
- std::map<GURL, Images>::iterator found =
- top_images_.find(deleted_url.url);
- if (found != top_images_.end())
- top_images_.erase(found);
+ {
+ AutoLock lock(lock_);
- // Delete from disk.
- if (db_.get())
- db_->RemoveURL(deleted_url);
+ // Process the diff: delete from images and disk, add to disk.
+ // Delete all the thumbnails associated with URLs that were deleted.
+ for (size_t i = 0; i < deleted.size(); i++) {
+ const MostVisitedURL& deleted_url = top_sites_[deleted[i]];
+ std::map<GURL, Images>::iterator found =
+ top_images_.find(deleted_url.url);
+ if (found != top_images_.end())
+ top_images_.erase(found);
+ }
}
+ // Write the updates to the DB.
if (db_.get()) {
- // Write both added and moved urls.
+ for (size_t i = 0; i < deleted.size(); i++) {
+ const MostVisitedURL& deleted_url = top_sites_[deleted[i]];
+ if (db_.get())
+ db_->RemoveURL(deleted_url);
+ }
for (size_t i = 0; i < added.size(); i++) {
const MostVisitedURL& added_url = most_visited[added[i]];
db_->SetPageThumbnail(added_url, added[i], Images());
@@ -484,6 +510,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)
@@ -521,6 +550,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)
@@ -533,6 +565,7 @@ void TopSites::StartQueryForThumbnail(size_t index) {
}
void TopSites::GenerateCanonicalURLs() {
+ lock_.AssertAcquired();
canonical_urls_.clear();
for (size_t i = 0; i < top_sites_.size(); i++) {
const MostVisitedURL& mv = top_sites_[i];
@@ -542,8 +575,9 @@ void TopSites::GenerateCanonicalURLs() {
void TopSites::StoreMostVisited(MostVisitedURLList* most_visited) {
DCHECK(ChromeThread::CurrentlyOn(ChromeThread::DB));
- // Take ownership of the most visited data.
+ AutoLock lock(lock_);
top_sites_.clear();
+ // Take ownership of the most visited data.
top_sites_.swap(*most_visited);
waiting_for_results_ = false;
@@ -560,8 +594,8 @@ void TopSites::StoreMostVisited(MostVisitedURLList* most_visited) {
// 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);
+ SetPageThumbnailEncoded(mv.url, it->second.thumbnail,
+ it->second.thumbnail_score);
temp_thumbnails_map_.erase(it);
break;
}
@@ -584,6 +618,7 @@ void TopSites::StoreRedirectChain(const RedirectList& redirects,
}
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 url; // Unknown URL - return unchanged.
@@ -652,6 +687,9 @@ void TopSites::StartQueryForMostVisited() {
&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) {
@@ -667,23 +705,27 @@ void TopSites::StartQueryForMostVisited() {
}
void TopSites::StartMigration() {
+ LOG(INFO) << "Starting migration to TopSites.";
migration_in_progress_ = true;
StartQueryForMostVisited();
MigratePinnedURLs();
}
void TopSites::AddBlacklistedURL(const GURL& url) {
- RemovePinnedURL(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);
}
@@ -702,22 +744,22 @@ void TopSites::AddPinnedURL(const GURL& url, size_t pinned_index) {
}
Value* index = Value::CreateIntegerValue(pinned_index);
+ AutoLock lock(lock_);
pinned_urls_->SetWithoutPathExpansion(GetURLString(url), index);
}
void TopSites::RemovePinnedURL(const GURL& url) {
- pinned_urls_->RemoveWithoutPathExpansion(GetURLString(url), NULL);
+ AutoLock lock(lock_);
+ RemovePinnedURLLocked(url);
}
-bool TopSites::GetIndexOfPinnedURL(const GURL& url, size_t* index) {
- int tmp;
- bool result = pinned_urls_->GetIntegerWithoutPathExpansion(
- GetURLString(url), &tmp);
- *index = static_cast<size_t>(tmp);
- return result;
+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);
@@ -738,6 +780,24 @@ bool TopSites::GetPinnedURLAtIndex(size_t index, GURL* url) {
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() {
if (top_sites_.size() == 0)
return base::TimeDelta::FromSeconds(30);
@@ -758,7 +818,10 @@ void TopSites::OnTopSitesAvailable(
if (!pending_callbacks_.empty()) {
MostVisitedURLList filtered_urls;
- ApplyBlacklistAndPinnedURLs(pages, &filtered_urls);
+ {
+ AutoLock lock(lock_);
+ ApplyBlacklistAndPinnedURLs(pages, &filtered_urls);
+ }
PendingCallbackSet::iterator i;
for (i = pending_callbacks_.begin();
@@ -777,7 +840,12 @@ void TopSites::OnThumbnailAvailable(CancelableRequestProvider::Handle handle,
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());
@@ -787,7 +855,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() &&
@@ -803,6 +872,7 @@ void TopSites::Observe(NotificationType type,
const NotificationSource& source,
const NotificationDetails& details) {
if (type == NotificationType::HISTORY_URLS_DELETED) {
+ AutoLock lock(lock_);
Details<history::URLsDeletedDetails> deleted_details(details);
if (deleted_details->all_history) {
top_sites_.clear();
@@ -821,7 +891,7 @@ void TopSites::Observe(NotificationType type,
for (std::set<size_t>::reverse_iterator i = indices_to_delete.rbegin();
i != indices_to_delete.rend(); i++) {
size_t index = *i;
- RemovePinnedURL(top_sites_[index].url);
+ RemovePinnedURLLocked(top_sites_[index].url);
top_sites_.erase(top_sites_.begin() + index);
}
}
@@ -830,11 +900,14 @@ void TopSites::Observe(NotificationType type,
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)) {
+ AutoLock lock(lock_);
// 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 09d0af0..b460afb 100644
--- a/chrome/browser/history/top_sites.h
+++ b/chrome/browser/history/top_sites.h
@@ -18,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"
@@ -47,9 +48,11 @@ 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);
@@ -68,14 +71,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);
@@ -108,9 +103,6 @@ class TopSites : public NotificationObserver,
// Add a URL to the blacklist.
void AddBlacklistedURL(const GURL& url);
- // Returns true if the URL is blacklisted.
- bool IsBlacklisted(const GURL& url);
-
// Removes a URL from the blacklist.
void RemoveBlacklistedURL(const GURL& url);
@@ -122,9 +114,6 @@ class TopSites : public NotificationObserver,
// Pin a URL at |index|.
void AddPinnedURL(const GURL& url, size_t index);
- // Get the index of a URL. Returns true if |url| is pinned.
- bool GetIndexOfPinnedURL(const GURL& url, size_t* index);
-
// Returns true if a URL is pinned.
bool IsURLPinned(const GURL& url);
@@ -135,8 +124,18 @@ class TopSites : public NotificationObserver,
// 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);
@@ -165,9 +164,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();
@@ -229,6 +228,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();
@@ -243,7 +248,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);
diff --git a/chrome/browser/history/top_sites_database.cc b/chrome/browser/history/top_sites_database.cc
index 99f0bb4..96da3f9 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"
@@ -48,7 +49,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 +76,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 +107,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 +123,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 +151,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 +195,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 +237,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 18ec201..11188a7 100644
--- a/chrome/browser/history/top_sites_database.h
+++ b/chrome/browser/history/top_sites_database.h
@@ -12,13 +12,12 @@
#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;
@@ -38,7 +37,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.
@@ -46,20 +45,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;
@@ -79,7 +78,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.
@@ -87,7 +86,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.
@@ -95,7 +94,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);
@@ -108,14 +107,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 b685319..d06f671 100644
--- a/chrome/browser/history/top_sites_unittest.cc
+++ b/chrome/browser/history/top_sites_unittest.cc
@@ -74,7 +74,7 @@ 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));
}
@@ -92,7 +92,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);
}
@@ -105,6 +104,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_;
@@ -121,6 +124,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:
@@ -188,14 +192,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;
@@ -229,8 +233,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.
@@ -255,7 +259,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_;
};
@@ -448,7 +452,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();
@@ -495,7 +499,7 @@ 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(4u, result.size());
@@ -520,18 +524,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);
@@ -607,7 +611,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);
@@ -623,7 +627,7 @@ TEST_F(TopSitesTest, RealDatabase) {
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));
@@ -641,7 +645,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);
@@ -676,7 +680,7 @@ 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(4u, results.size());
@@ -697,7 +701,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();
@@ -1074,7 +1078,11 @@ TEST_F(TopSitesTest, Blacklisting) {
&TopSitesTest::OnTopSitesAvailable));
top_sites().OnTopSitesAvailable(0, pages);
MessageLoop::current()->RunAllPending();
- EXPECT_FALSE(top_sites().IsBlacklisted(GURL("http://bbc.com/")));
+ {
+ 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());
@@ -1085,9 +1093,13 @@ TEST_F(TopSitesTest, Blacklisting) {
EXPECT_EQ(themes_url(), urls()[3].url);
top_sites().AddBlacklistedURL(GURL("http://google.com/"));
- 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()));
+ {
+ 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,
@@ -1110,7 +1122,11 @@ TEST_F(TopSitesTest, Blacklisting) {
EXPECT_EQ(themes_url(), urls()[1].url);
top_sites().RemoveBlacklistedURL(GURL("http://google.com/"));
- EXPECT_FALSE(top_sites().IsBlacklisted(GURL("http://google.com/")));
+ {
+ 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,
@@ -1151,7 +1167,6 @@ TEST_F(TopSitesTest, PinnedURLs) {
&TopSitesTest::OnTopSitesAvailable));
top_sites().OnTopSitesAvailable(0, pages);
MessageLoop::current()->RunAllPending();
- size_t index = 0;
EXPECT_FALSE(top_sites().IsURLPinned(GURL("http://bbc.com/")));
ASSERT_EQ(4u, urls().size());
@@ -1161,9 +1176,6 @@ TEST_F(TopSitesTest, PinnedURLs) {
EXPECT_EQ(themes_url(), urls()[3].url);
top_sites().AddPinnedURL(GURL("http://google.com/"), 3);
- EXPECT_TRUE(top_sites().GetIndexOfPinnedURL(GURL("http://google.com/"),
- &index));
- EXPECT_EQ(3u, index);
EXPECT_FALSE(top_sites().IsURLPinned(GURL("http://bbc.com/")));
EXPECT_FALSE(top_sites().IsURLPinned(welcome_url()));
diff --git a/chrome/browser/profile_impl.cc b/chrome/browser/profile_impl.cc
index bb335bf..63269b2 100644
--- a/chrome/browser/profile_impl.cc
+++ b/chrome/browser/profile_impl.cc
@@ -465,6 +465,9 @@ ProfileImpl::~ProfileImpl() {
if (web_data_service_.get())
web_data_service_->Shutdown();
+ if (top_sites_.get())
+ top_sites_->ClearProfile();
+
if (history_service_.get())
history_service_->Cleanup();
diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc
index 96c2411..3e43c8f 100644
--- a/chrome/browser/tab_contents/tab_contents.cc
+++ b/chrome/browser/tab_contents/tab_contents.cc
@@ -2485,7 +2485,7 @@ void TabContents::UpdateThumbnail(const GURL& url,
const SkBitmap& bitmap,
const ThumbnailScore& score) {
// Tell History about this thumbnail
- if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kTopSites)) {
+ if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoTopSites)) {
if (!profile()->IsOffTheRecord())
profile()->GetTopSites()->SetPageThumbnail(url, bitmap, score);
} else {
diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc
index 4df9c3c..27e5cf4 100644
--- a/chrome/common/chrome_switches.cc
+++ b/chrome/common/chrome_switches.cc
@@ -688,6 +688,9 @@ const char kNoProxyServer[] = "no-proxy-server";
// Runs the renderer outside the sandbox.
const char kNoSandbox[] = "no-sandbox";
+// Don't use TopSites; use old ThumbnailDatabase code instead.
+const char kNoTopSites[] = "no-top-sites";
+
// Specifies the maximum number of threads to use for running the Proxy
// Autoconfig (PAC) script.
const char kNumPacThreads[] = "num-pac-threads";
@@ -948,10 +951,6 @@ const char kTestType[] = "test-type";
// testing-related messages on IPC channel with the given ID.
const char kTestingChannelID[] = "testing-channel";
-// Enables using TopSites instead of ThumbnailDatabase (and
-// ThumbnailStore) for getting thumbnails for the new tab page.
-const char kTopSites[] = "top-sites";
-
// Excludes these plugins from the plugin sandbox.
// This is a comma-separated list of plugin library names.
const char kTrustedPlugins[] = "trusted-plugins";
diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h
index 00385c3..362a3a6 100644
--- a/chrome/common/chrome_switches.h
+++ b/chrome/common/chrome_switches.h
@@ -204,6 +204,7 @@ extern const char kNoJsRandomness[];
extern const char kNoProxyServer[];
extern const char kNoReferrers[];
extern const char kNoSandbox[];
+extern const char kNoTopSites[];
extern const char kNumPacThreads[];
extern const char kOpenInNewWindow[];
extern const char kOrganicInstall[];
@@ -272,7 +273,6 @@ extern const char kTestName[];
extern const char kTestSandbox[];
extern const char kTestType[];
extern const char kTestingChannelID[];
-extern const char kTopSites[];
extern const char kTrustedPlugins[];
extern const char kTryChromeAgain[];
extern const char kUninstall[];
diff --git a/chrome/test/data/profiles/typical_history/Default/Favicons b/chrome/test/data/profiles/typical_history/Default/Favicons
new file mode 100644
index 0000000..b42db08
Binary files /dev/null and b/chrome/test/data/profiles/typical_history/Default/Favicons differ
diff --git a/chrome/test/data/profiles/typical_history/Default/History b/chrome/test/data/profiles/typical_history/Default/History
index 98481ce..cf186c9 100644
Binary files a/chrome/test/data/profiles/typical_history/Default/History and b/chrome/test/data/profiles/typical_history/Default/History differ
diff --git a/chrome/test/data/profiles/typical_history/Default/Thumbnails b/chrome/test/data/profiles/typical_history/Default/Thumbnails
deleted file mode 100644
index fc26118..0000000
Binary files a/chrome/test/data/profiles/typical_history/Default/Thumbnails and /dev/null differ
diff --git a/chrome/test/data/profiles/typical_history/Default/Top Sites b/chrome/test/data/profiles/typical_history/Default/Top Sites
new file mode 100644
index 0000000..2661211
Binary files /dev/null and b/chrome/test/data/profiles/typical_history/Default/Top Sites differ
diff --git a/chrome/test/testing_profile.cc b/chrome/test/testing_profile.cc
index 547aff0..e0a3968 100644
--- a/chrome/test/testing_profile.cc
+++ b/chrome/test/testing_profile.cc
@@ -178,6 +178,10 @@ TestingProfile::~TestingProfile() {
// FaviconService depends on HistoryServce so destroying it later.
DestroyFaviconService();
DestroyWebDataService();
+ if (top_sites_.get())
+ top_sites_->ClearProfile();
+ history::TopSites::DeleteTopSites(top_sites_);
+
file_util::Delete(path_, true);
}
diff --git a/chrome/test/testing_profile.h b/chrome/test/testing_profile.h
index 36f638f..dd8976e 100644
--- a/chrome/test/testing_profile.h
+++ b/chrome/test/testing_profile.h
@@ -9,6 +9,7 @@
#include "base/base_paths.h"
#include "base/file_util.h"
#include "base/path_service.h"
+#include "base/scoped_temp_dir.h"
#include "chrome/browser/autocomplete/autocomplete_classifier.h"
#include "chrome/browser/bookmarks/bookmark_model.h"
#include "chrome/browser/browser_prefs.h"
@@ -20,6 +21,7 @@
#include "chrome/browser/geolocation/geolocation_permission_context.h"
#include "chrome/browser/host_content_settings_map.h"
#include "chrome/browser/history/history.h"
+#include "chrome/browser/history/top_sites.h"
#include "chrome/browser/in_process_webkit/webkit_context.h"
#include "chrome/browser/notifications/desktop_notification_service.h"
#include "chrome/browser/pref_service.h"
@@ -163,7 +165,16 @@ class TestingProfile : public Profile {
return template_url_model_.get();
}
virtual TemplateURLFetcher* GetTemplateURLFetcher() { return NULL; }
- virtual history::TopSites* GetTopSites() { return NULL; }
+ virtual history::TopSites* GetTopSites() {
+ if (!top_sites_.get()) {
+ top_sites_ = new history::TopSites(this);
+ if (!temp_dir_.CreateUniqueTempDir())
+ return NULL;
+ FilePath file_name = temp_dir_.path().AppendASCII("TopSites.db");
+ top_sites_->Init(file_name);
+ }
+ return top_sites_;
+ }
virtual DownloadManager* GetDownloadManager() { return NULL; }
virtual PersonalDataManager* GetPersonalDataManager() { return NULL; }
virtual bool HasCreatedDownloadManager() const { return false; }
@@ -372,6 +383,8 @@ class TestingProfile : public Profile {
scoped_ptr<FindBarState> find_bar_state_;
FilePath last_selected_directory_;
+ scoped_refptr<history::TopSites> top_sites_; // For history and thumbnails.
+ ScopedTempDir temp_dir_;
};
// A profile that derives from another profile. This does not actually
--
cgit v1.1