diff options
author | sdefresne <sdefresne@chromium.org> | 2015-06-23 15:28:32 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-06-23 22:29:56 +0000 |
commit | 542b71e372426c2547be0a689c2d7053b1ffdeff (patch) | |
tree | 77a0c66631fd445da07d384082bf6e438bd7598c /components/history | |
parent | a98c2e971a8a9b6326bc7e92a9c10a4f0a76d15c (diff) | |
download | chromium_src-542b71e372426c2547be0a689c2d7053b1ffdeff.zip chromium_src-542b71e372426c2547be0a689c2d7053b1ffdeff.tar.gz chromium_src-542b71e372426c2547be0a689c2d7053b1ffdeff.tar.bz2 |
Split HistoryClient in two objects
HistoryClient exposed methods accessed by UI or History thread. Split
the object in two only exposing method accessed by a single thread.
The HistoryClient methods can only be invoked from UI thread while the
HistoryBackendClient methods can only be invoked from History thread.
This separation is required before the History code can be refactored
not to create it's own thread.
BUG=459548,477975
Review URL: https://codereview.chromium.org/1198373002
Cr-Commit-Position: refs/heads/master@{#335779}
Diffstat (limited to 'components/history')
18 files changed, 311 insertions, 210 deletions
diff --git a/components/history/core/browser/BUILD.gn b/components/history/core/browser/BUILD.gn index eaaf3f4..ea7fee6 100644 --- a/components/history/core/browser/BUILD.gn +++ b/components/history/core/browser/BUILD.gn @@ -17,9 +17,9 @@ static_library("browser") { "expire_history_backend.h", "history_backend.cc", "history_backend.h", + "history_backend_client.h", "history_backend_notifier.h", "history_backend_observer.h", - "history_client.cc", "history_client.h", "history_constants.cc", "history_constants.h", diff --git a/components/history/core/browser/expire_history_backend.cc b/components/history/core/browser/expire_history_backend.cc index 42a93c5..865299a 100644 --- a/components/history/core/browser/expire_history_backend.cc +++ b/components/history/core/browser/expire_history_backend.cc @@ -15,8 +15,8 @@ #include "base/location.h" #include "base/logging.h" #include "base/sequenced_task_runner.h" +#include "components/history/core/browser/history_backend_client.h" #include "components/history/core/browser/history_backend_notifier.h" -#include "components/history/core/browser/history_client.h" #include "components/history/core/browser/history_database.h" #include "components/history/core/browser/thumbnail_database.h" @@ -128,12 +128,12 @@ ExpireHistoryBackend::DeleteEffects::~DeleteEffects() { ExpireHistoryBackend::ExpireHistoryBackend( HistoryBackendNotifier* notifier, - HistoryClient* history_client, + HistoryBackendClient* backend_client, scoped_refptr<base::SequencedTaskRunner> task_runner) : notifier_(notifier), - main_db_(NULL), - thumb_db_(NULL), - history_client_(history_client), + main_db_(nullptr), + thumb_db_(nullptr), + backend_client_(backend_client), task_runner_(task_runner), weak_factory_(this) { DCHECK(notifier_); @@ -157,11 +157,10 @@ void ExpireHistoryBackend::DeleteURLs(const std::vector<GURL>& urls) { return; DeleteEffects effects; - HistoryClient* history_client = GetHistoryClient(); for (std::vector<GURL>::const_iterator url = urls.begin(); url != urls.end(); ++url) { const bool is_bookmarked = - history_client && history_client->IsBookmarked(*url); + backend_client_ && backend_client_->IsBookmarked(*url); URLRow url_row; if (!main_db_->GetRowForURL(*url, &url_row) && !is_bookmarked) { // If the URL isn't in the database and not bookmarked, we should still @@ -204,7 +203,7 @@ void ExpireHistoryBackend::ExpireHistoryBetween( std::set<URLID> url_ids; for (std::set<GURL>::const_iterator url = restrict_urls.begin(); url != restrict_urls.end(); ++url) - url_ids.insert(main_db_->GetRowForURL(*url, NULL)); + url_ids.insert(main_db_->GetRowForURL(*url, nullptr)); VisitVector all_visits; all_visits.swap(visits); for (VisitVector::iterator visit = all_visits.begin(); @@ -406,7 +405,6 @@ void ExpireHistoryBackend::ExpireURLsForVisits(const VisitVector& visits, } // Check each unique URL with deleted visits. - HistoryClient* history_client = GetHistoryClient(); for (std::map<URLID, ChangedURL>::const_iterator i = changed_urls.begin(); i != changed_urls.end(); ++i) { // The unique URL rows should already be filled in. @@ -425,7 +423,7 @@ void ExpireHistoryBackend::ExpireURLsForVisits(const VisitVector& visits, // Don't delete URLs with visits still in the DB, or bookmarked. bool is_bookmarked = - (history_client && history_client->IsBookmarked(url_row.url())); + (backend_client_ && backend_client_->IsBookmarked(url_row.url())); if (!is_bookmarked && url_row.last_visit().is_null()) { // Not bookmarked and no more visits. Nuke the url. DeleteOneURL(url_row, is_bookmarked, effects); @@ -509,13 +507,4 @@ void ExpireHistoryBackend::ParanoidExpireHistory() { // TODO(brettw): Bug 1067331: write this to clean up any errors. } -HistoryClient* ExpireHistoryBackend::GetHistoryClient() { - // We use the history client to determine if a URL is bookmarked. The data is - // loaded on a separate thread and may not be done by the time we get here. - // We therefore block until the bookmarks have finished loading. - if (history_client_) - history_client_->BlockUntilBookmarksLoaded(); - return history_client_; -} - } // namespace history diff --git a/components/history/core/browser/expire_history_backend.h b/components/history/core/browser/expire_history_backend.h index 8914687..70a20f8 100644 --- a/components/history/core/browser/expire_history_backend.h +++ b/components/history/core/browser/expire_history_backend.h @@ -21,8 +21,8 @@ class TestingProfile; namespace history { +class HistoryBackendClient; class HistoryBackendNotifier; -class HistoryClient; class HistoryDatabase; class ThumbnailDatabase; @@ -45,12 +45,12 @@ typedef std::vector<const ExpiringVisitsReader*> ExpiringVisitsReaders; // StartExpiringOldStuff(). class ExpireHistoryBackend { public: - // The delegate pointer must be non-NULL. We will NOT take ownership of it. - // HistoryClient may be NULL. The HistoryClient is used when expiring URLS so - // that we don't remove any URLs or favicons that are bookmarked (visits are - // removed though). + // The delegate pointer must be non-null. We will NOT take ownership of it. + // HistoryBackendClient may be null. The HistoryBackendClient is used when + // expiring URLS so that we don't remove any URLs or favicons that are + // bookmarked (visits are removed though). ExpireHistoryBackend(HistoryBackendNotifier* notifier, - HistoryClient* history_client, + HistoryBackendClient* backend_client, scoped_refptr<base::SequencedTaskRunner> task_runner); ~ExpireHistoryBackend(); @@ -214,10 +214,6 @@ class ExpireHistoryBackend { // and deletes items. For example, URLs with no visits. void ParanoidExpireHistory(); - // Returns the HistoryClient, blocking until the bookmarks are loaded. This - // may return NULL during testing. - HistoryClient* GetHistoryClient(); - // Initializes periodic expiration work queue by populating it with with tasks // for all known readers. void InitWorkQueue(); @@ -255,11 +251,8 @@ class ExpireHistoryBackend { scoped_ptr<ExpiringVisitsReader> all_visits_reader_; scoped_ptr<ExpiringVisitsReader> auto_subframe_visits_reader_; - // The HistoryClient; may be NULL. - // - // Use GetHistoryClient to access this, which makes sure the bookmarks are - // loaded before returning. - HistoryClient* history_client_; + // The HistoryBackendClient; may be null. + HistoryBackendClient* backend_client_; scoped_refptr<base::SequencedTaskRunner> task_runner_; diff --git a/components/history/core/browser/expire_history_backend_unittest.cc b/components/history/core/browser/expire_history_backend_unittest.cc index c378320..f1219a4 100644 --- a/components/history/core/browser/expire_history_backend_unittest.cc +++ b/components/history/core/browser/expire_history_backend_unittest.cc @@ -21,6 +21,7 @@ #include "base/scoped_observer.h" #include "base/strings/string16.h" #include "base/strings/utf_string_conversions.h" +#include "components/history/core/browser/history_backend_client.h" #include "components/history/core/browser/history_backend_notifier.h" #include "components/history/core/browser/history_constants.h" #include "components/history/core/browser/history_database.h" @@ -51,7 +52,8 @@ bool MockCanAddURLToHistory(const GURL& url) { class ExpireHistoryTest : public testing::Test, public HistoryBackendNotifier { public: ExpireHistoryTest() - : expirer_(this, &history_client_, message_loop_.task_runner()), + : backend_client_(history_client_.CreateBackendClient()), + expirer_(this, backend_client_.get(), message_loop_.task_runner()), now_(base::Time::Now()) {} protected: @@ -93,6 +95,7 @@ class ExpireHistoryTest : public testing::Test, public HistoryBackendNotifier { base::ScopedTempDir tmp_dir_; HistoryClientFakeBookmarks history_client_; + scoped_ptr<HistoryBackendClient> backend_client_; base::MessageLoopForUI message_loop_; diff --git a/components/history/core/browser/history_backend.cc b/components/history/core/browser/history_backend.cc index b75252b..fc347ba 100644 --- a/components/history/core/browser/history_backend.cc +++ b/components/history/core/browser/history_backend.cc @@ -27,8 +27,8 @@ #include "components/favicon_base/select_favicon_frames.h" #include "components/history/core/browser/download_constants.h" #include "components/history/core/browser/download_row.h" +#include "components/history/core/browser/history_backend_client.h" #include "components/history/core/browser/history_backend_observer.h" -#include "components/history/core/browser/history_client.h" #include "components/history/core/browser/history_constants.h" #include "components/history/core/browser/history_database.h" #include "components/history/core/browser/history_database_params.h" @@ -199,15 +199,15 @@ HistoryBackendHelper::~HistoryBackendHelper() { HistoryBackend::HistoryBackend( Delegate* delegate, - HistoryClient* history_client, + scoped_ptr<HistoryBackendClient> backend_client, scoped_refptr<base::SequencedTaskRunner> task_runner) : delegate_(delegate), scheduled_kill_db_(false), - expirer_(this, history_client, task_runner), + expirer_(this, backend_client.get(), task_runner), recent_redirects_(kMaxRedirectCount), backend_destroy_message_loop_(nullptr), segment_queried_(false), - history_client_(history_client), + backend_client_(backend_client.Pass()), task_runner_(task_runner) { } @@ -229,8 +229,10 @@ HistoryBackend::~HistoryBackend() { backend_destroy_message_loop_->PostTask(FROM_HERE, backend_destroy_task_); } - if (history_client_ && !history_dir_.empty()) - history_client_->OnHistoryBackendDestroyed(this, history_dir_); +#if defined(OS_ANDROID) + if (backend_client_ && !history_dir_.empty()) + backend_client_->OnHistoryBackendDestroyed(this, history_dir_); +#endif } void HistoryBackend::Init( @@ -657,7 +659,7 @@ void HistoryBackend::InitImpl( // favicons. Thumbnails are stored in "top sites". Consider // renaming "thumbnail" references to "favicons" or something of the // sort. - thumbnail_db_.reset(new ThumbnailDatabase(history_client_)); + thumbnail_db_.reset(new ThumbnailDatabase(backend_client_.get())); if (thumbnail_db_->Init(thumbnail_name) != sql::INIT_OK) { // Unlike the main database, we don't error out when the database is too // new because this error is much less severe. Generally, this shouldn't @@ -696,10 +698,12 @@ void HistoryBackend::InitImpl( // Start expiring old stuff. expirer_.StartExpiringOldStuff(TimeDelta::FromDays(kExpireDaysThreshold)); - if (history_client_) { - history_client_->OnHistoryBackendInitialized( +#if defined(OS_ANDROID) + if (backend_client_) { + backend_client_->OnHistoryBackendInitialized( this, db_.get(), thumbnail_db_.get(), history_dir_); } +#endif LOCAL_HISTOGRAM_TIMES("History.InitTime", TimeTicks::Now() - beginning_time); } @@ -1814,7 +1818,6 @@ void HistoryBackend::SetImportedFavicons( } // Save the mapping from all the URLs to the favicon. - HistoryClient* history_client = GetHistoryClient(); for (std::set<GURL>::const_iterator url = favicon_usage[i].urls.begin(); url != favicon_usage[i].urls.end(); ++url) { URLRow url_row; @@ -1824,7 +1827,7 @@ void HistoryBackend::SetImportedFavicons( // for regular bookmarked URLs with favicons - when history db is // cleaned, we keep an entry in the db with 0 visits as long as that // url is bookmarked. - if (history_client && history_client->IsBookmarked(*url)) { + if (backend_client_ && backend_client_->IsBookmarked(*url)) { URLRow url_info(*url); url_info.set_visit_count(0); url_info.set_typed_count(0); @@ -2558,9 +2561,8 @@ void HistoryBackend::DeleteAllHistory() { // Get the bookmarked URLs. std::vector<URLAndTitle> starred_url_and_titles; - HistoryClient* history_client = GetHistoryClient(); - if (history_client) - history_client->GetBookmarks(&starred_url_and_titles); + if (backend_client_) + backend_client_->GetBookmarks(&starred_url_and_titles); URLRows kept_url_rows; std::vector<GURL> starred_urls; @@ -2670,10 +2672,4 @@ bool HistoryBackend::ClearAllMainHistory(const URLRows& kept_urls) { return true; } -HistoryClient* HistoryBackend::GetHistoryClient() { - if (history_client_) - history_client_->BlockUntilBookmarksLoaded(); - return history_client_; -} - } // namespace history diff --git a/components/history/core/browser/history_backend.h b/components/history/core/browser/history_backend.h index aa2ce2f..3038475 100644 --- a/components/history/core/browser/history_backend.h +++ b/components/history/core/browser/history_backend.h @@ -42,10 +42,10 @@ class SingleThreadTaskRunner; namespace history { class CommitLaterTask; struct DownloadRow; +class HistoryBackendClient; class HistoryBackendDBBaseTest; class HistoryBackendObserver; class HistoryBackendTest; -class HistoryClient; class HistoryDatabase; struct HistoryDatabaseParams; struct HistoryDetails; @@ -171,7 +171,8 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, // may be null. // // This constructor is fast and does no I/O, so can be called at any time. - HistoryBackend(Delegate* delegate, HistoryClient* history_client, + HistoryBackend(Delegate* delegate, + scoped_ptr<HistoryBackendClient> backend_client, scoped_refptr<base::SequencedTaskRunner> task_runner); // Must be called after creation but before any objects are created. If this @@ -762,10 +763,6 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, // Deletes the FTS index database files, which are no longer used. void DeleteFTSIndexDatabases(); - // Returns the HistoryClient, blocking until the bookmarks are loaded. This - // may return null during testing. - HistoryClient* GetHistoryClient(); - // Data ---------------------------------------------------------------------- // Delegate. See the class definition above for more information. This will @@ -823,10 +820,7 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, std::list<QueuedHistoryDBTask*> queued_history_db_tasks_; // Used to determine if a URL is bookmarked; may be null. - // - // Use GetHistoryClient to access this, which makes sure the bookmarks are - // loaded before returning. - HistoryClient* history_client_; + scoped_ptr<HistoryBackendClient> backend_client_; scoped_refptr<base::SequencedTaskRunner> task_runner_; diff --git a/components/history/core/browser/history_backend_client.cc b/components/history/core/browser/history_backend_client.cc new file mode 100644 index 0000000..082a07d --- /dev/null +++ b/components/history/core/browser/history_backend_client.cc @@ -0,0 +1,15 @@ +// Copyright 2015 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 "components/history/core/browser/history_backend_client.h" + +namespace history { + +HistoryBackendClient::HistoryBackendClient() { +} + +HistoryBackendClient::~HistoryBackendClient() { +} + +} // namespace history diff --git a/components/history/core/browser/history_backend_client.h b/components/history/core/browser/history_backend_client.h new file mode 100644 index 0000000..4ebc496 --- /dev/null +++ b/components/history/core/browser/history_backend_client.h @@ -0,0 +1,65 @@ +// Copyright 2015 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 COMPONENTS_HISTORY_CORE_BROWSER_HISTORY_BACKEND_CLIENT_H_ +#define COMPONENTS_HISTORY_CORE_BROWSER_HISTORY_BACKEND_CLIENT_H_ + +#include <vector> + +#include "base/macros.h" +#include "base/strings/string16.h" +#include "url/gurl.h" + +namespace base { +class FilePath; +} + +namespace history { + +class HistoryBackend; +class HistoryDatabase; +class ThumbnailDatabase; + +struct URLAndTitle { + GURL url; + base::string16 title; +}; + +class HistoryBackendClient { + public: + HistoryBackendClient() {} + virtual ~HistoryBackendClient() {} + + // Returns true if the specified URL is bookmarked. + virtual bool IsBookmarked(const GURL& url) = 0; + + // Returns, by reference in |bookmarks|, the set of bookmarked URLs and their + // title. This returns the unique set of URLs. For example, if two bookmarks + // reference the same URL only one entry is added even if the title are not + // the same. + virtual void GetBookmarks(std::vector<URLAndTitle>* bookmarks) = 0; + + // Returns whether database errors should be reported to the crash server. + virtual bool ShouldReportDatabaseError() = 0; + +#if defined(OS_ANDROID) + // Called upon initialization of the HistoryBackend. + virtual void OnHistoryBackendInitialized( + HistoryBackend* history_backend, + HistoryDatabase* history_database, + ThumbnailDatabase* thumbnail_database, + const base::FilePath& history_dir) = 0; + + // Called upon destruction of the HistoryBackend. + virtual void OnHistoryBackendDestroyed(HistoryBackend* history_backend, + const base::FilePath& history_dir) = 0; +#endif // defined(OS_ANDROID) + + private: + DISALLOW_COPY_AND_ASSIGN(HistoryBackendClient); +}; + +} // namespace history + +#endif // COMPONENTS_HISTORY_CORE_BROWSER_HISTORY_BACKEND_CLIENT_H_ diff --git a/components/history/core/browser/history_backend_unittest.cc b/components/history/core/browser/history_backend_unittest.cc index b1c0b07..c9aad33 100644 --- a/components/history/core/browser/history_backend_unittest.cc +++ b/components/history/core/browser/history_backend_unittest.cc @@ -26,6 +26,7 @@ #include "base/strings/utf_string_conversions.h" #include "base/thread_task_runner_handle.h" #include "components/favicon_base/favicon_usage_data.h" +#include "components/history/core/browser/history_backend_client.h" #include "components/history/core/browser/history_constants.h" #include "components/history/core/browser/history_database_params.h" #include "components/history/core/browser/history_service.h" @@ -68,15 +69,6 @@ typedef base::Callback<void(const history::URLRow*, const history::URLRow*)> SimulateNotificationCallback; -class HistoryClientMock : public history::HistoryClientFakeBookmarks { - public: - explicit HistoryClientMock(const GURL& url_to_bookmark) { - AddBookmark(url_to_bookmark); - } - - MOCK_METHOD0(BlockUntilBookmarksLoaded, void()); -}; - void SimulateNotificationURLVisited(history::HistoryServiceObserver* observer, const history::URLRow* row1, const history::URLRow* row2, @@ -249,7 +241,7 @@ class HistoryBackendTestBase : public testing::Test { &test_dir_)) return; backend_ = new HistoryBackend(new HistoryBackendTestDelegate(this), - &history_client_, + history_client_.CreateBackendClient(), base::ThreadTaskRunnerHandle::Get()); backend_->Init(std::string(), false, TestHistoryDatabaseParamsForPath(test_dir_)); @@ -1697,7 +1689,7 @@ TEST_F(HistoryBackendTest, MigrationVisitSource) { ASSERT_TRUE(base::CopyFile(old_history_path, new_history_file)); backend_ = new HistoryBackend(new HistoryBackendTestDelegate(this), - &history_client_, + history_client_.CreateBackendClient(), base::ThreadTaskRunnerHandle::Get()); backend_->Init(std::string(), false, TestHistoryDatabaseParamsForPath(new_history_path)); @@ -3116,7 +3108,7 @@ TEST_F(HistoryBackendTest, MigrationVisitDuration) { ASSERT_TRUE(base::CopyFile(old_history, new_history_file)); backend_ = new HistoryBackend(new HistoryBackendTestDelegate(this), - &history_client_, + history_client_.CreateBackendClient(), base::ThreadTaskRunnerHandle::Get()); backend_->Init(std::string(), false, TestHistoryDatabaseParamsForPath(new_history_path)); @@ -3340,10 +3332,9 @@ TEST_F(HistoryBackendTest, RemoveNotification) { // Add a URL. GURL url("http://www.google.com"); - scoped_ptr<HistoryClientMock> history_client(new HistoryClientMock(url)); - HistoryClientMock* history_client_mock = history_client.get(); - scoped_ptr<HistoryService> service(new HistoryService( - history_client.Pass(), scoped_ptr<history::VisitDelegate>())); + scoped_ptr<HistoryService> service( + new HistoryService(make_scoped_ptr(new HistoryClientFakeBookmarks), + scoped_ptr<history::VisitDelegate>())); EXPECT_TRUE( service->Init(kAcceptLanguagesForTest, TestHistoryDatabaseParamsForPath(scoped_temp_dir.path()))); @@ -3354,7 +3345,6 @@ TEST_F(HistoryBackendTest, RemoveNotification) { // This won't actually delete the URL, rather it'll empty out the visits. // This triggers blocking on the BookmarkModel. - EXPECT_CALL(*history_client_mock, BlockUntilBookmarksLoaded()); service->DeleteURL(url); } diff --git a/components/history/core/browser/history_client.cc b/components/history/core/browser/history_client.cc deleted file mode 100644 index 8cfa60f..0000000 --- a/components/history/core/browser/history_client.cc +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright 2014 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 "components/history/core/browser/history_client.h" - -namespace history { - -HistoryClient::HistoryClient() { -} - -HistoryClient::~HistoryClient() { -} - -void HistoryClient::Shutdown() { -} - -void HistoryClient::BlockUntilBookmarksLoaded() { -} - -bool HistoryClient::IsBookmarked(const GURL& url) { - return false; -} - -void HistoryClient::GetBookmarks(std::vector<URLAndTitle>* bookmarks) { -} - -bool HistoryClient::CanAddURL(const GURL& url) { - return true; -} - -void HistoryClient::NotifyProfileError(sql::InitStatus init_status) { -} - -bool HistoryClient::ShouldReportDatabaseError() { - return false; -} - -void HistoryClient::OnHistoryBackendInitialized( - HistoryBackend* history_backend, - HistoryDatabase* history_database, - ThumbnailDatabase* thumbnail_database, - const base::FilePath& history_dir) { -} - -void HistoryClient::OnHistoryBackendDestroyed( - HistoryBackend* history_backend, - const base::FilePath& history_dir) { -} - -} // namespace history diff --git a/components/history/core/browser/history_client.h b/components/history/core/browser/history_client.h index ff6af5f..ae9c195 100644 --- a/components/history/core/browser/history_client.h +++ b/components/history/core/browser/history_client.h @@ -5,12 +5,11 @@ #ifndef COMPONENTS_HISTORY_CORE_BROWSER_HISTORY_CLIENT_H_ #define COMPONENTS_HISTORY_CORE_BROWSER_HISTORY_CLIENT_H_ -#include <vector> - #include "base/macros.h" -#include "base/strings/string16.h" +#include "base/memory/scoped_ptr.h" #include "sql/init_status.h" -#include "url/gurl.h" + +class GURL; namespace base { class FilePath; @@ -19,64 +18,29 @@ class FilePath; namespace history { class HistoryBackend; +class HistoryBackendClient; class HistoryDatabase; class ThumbnailDatabase; -struct URLAndTitle { - GURL url; - base::string16 title; -}; - // This class abstracts operations that depend on the embedder's environment, // e.g. Chrome. class HistoryClient { public: - HistoryClient(); - virtual ~HistoryClient(); + HistoryClient() {} + virtual ~HistoryClient() {} // Called before HistoryService is shutdown. - virtual void Shutdown(); - - // Waits until the bookmarks have been loaded. - // - // Must not be called from the main thread. - virtual void BlockUntilBookmarksLoaded(); - - // Returns true if the specified URL is bookmarked. - // - // If not on the main thread, then BlockUntilBookmarksLoaded must be called. - virtual bool IsBookmarked(const GURL& url); - - // Returns, by reference in |bookmarks|, the set of bookmarked urls and their - // titles. This returns the unique set of URLs. For example, if two bookmarks - // reference the same URL only one entry is added even if the title are not - // the same. - // - // If not on the main thread, then BlockUntilBookmarksLoaded must be called. - virtual void GetBookmarks(std::vector<URLAndTitle>* bookmarks); + virtual void Shutdown() = 0; // Returns true if this look like the type of URL that should be added to the // history. - virtual bool CanAddURL(const GURL& url); + virtual bool CanAddURL(const GURL& url) = 0; // Notifies the embedder that there was a problem reading the database. - // - // Must be called from the main thread. - virtual void NotifyProfileError(sql::InitStatus init_status); - - // Returns whether database errors should be reported to the crash server. - virtual bool ShouldReportDatabaseError(); - - // Called upon initialization of the HistoryBackend. - virtual void OnHistoryBackendInitialized( - HistoryBackend* history_backend, - HistoryDatabase* history_database, - ThumbnailDatabase* thumbnail_database, - const base::FilePath& history_dir); + virtual void NotifyProfileError(sql::InitStatus init_status) = 0; - // Called upon destruction of the HistoryBackend. - virtual void OnHistoryBackendDestroyed(HistoryBackend* history_backend, - const base::FilePath& history_dir); + // Returns a new HistoryBackendClient instance. + virtual scoped_ptr<HistoryBackendClient> CreateBackendClient() = 0; private: DISALLOW_COPY_AND_ASSIGN(HistoryClient); diff --git a/components/history/core/browser/history_service.cc b/components/history/core/browser/history_service.cc index 0f13670..da0ed64 100644 --- a/components/history/core/browser/history_service.cc +++ b/components/history/core/browser/history_service.cc @@ -32,6 +32,7 @@ #include "base/trace_event/trace_event.h" #include "components/history/core/browser/download_row.h" #include "components/history/core/browser/history_backend.h" +#include "components/history/core/browser/history_backend_client.h" #include "components/history/core/browser/history_client.h" #include "components/history/core/browser/history_database_params.h" #include "components/history/core/browser/history_db_task.h" @@ -877,7 +878,8 @@ bool HistoryService::Init( scoped_refptr<HistoryBackend> backend(new HistoryBackend( new BackendDelegate(weak_ptr_factory_.GetWeakPtr(), base::ThreadTaskRunnerHandle::Get()), - history_client_.get(), thread_->task_runner())); + history_client_ ? history_client_->CreateBackendClient() : nullptr, + thread_->task_runner())); history_backend_.swap(backend); ScheduleTask(PRIORITY_UI, diff --git a/components/history/core/browser/thumbnail_database.cc b/components/history/core/browser/thumbnail_database.cc index 2e699ce..35afa12 100644 --- a/components/history/core/browser/thumbnail_database.cc +++ b/components/history/core/browser/thumbnail_database.cc @@ -18,7 +18,7 @@ #include "base/strings/string_util.h" #include "base/strings/stringprintf.h" #include "base/time/time.h" -#include "components/history/core/browser/history_client.h" +#include "components/history/core/browser/history_backend_client.h" #include "components/history/core/browser/url_database.h" #include "sql/recovery.h" #include "sql/statement.h" @@ -550,14 +550,14 @@ void RecoverDatabaseOrRaze(sql::Connection* db, const base::FilePath& db_path) { void DatabaseErrorCallback(sql::Connection* db, const base::FilePath& db_path, size_t startup_kb, - HistoryClient* history_client, + HistoryBackendClient* backend_client, int extended_error, sql::Statement* stmt) { // TODO(shess): Assert that this is running on a safe thread. // AFAICT, should be the history thread, but at this level I can't // see how to reach that. - if (history_client && history_client->ShouldReportDatabaseError()) { + if (backend_client && backend_client->ShouldReportDatabaseError()) { GenerateDiagnostics(db, startup_kb, extended_error); } @@ -590,8 +590,8 @@ bool ThumbnailDatabase::IconMappingEnumerator::GetNextIconMapping( return true; } -ThumbnailDatabase::ThumbnailDatabase(HistoryClient* history_client) - : history_client_(history_client) { +ThumbnailDatabase::ThumbnailDatabase(HistoryBackendClient* backend_client) + : backend_client_(backend_client) { } ThumbnailDatabase::~ThumbnailDatabase() { @@ -1212,7 +1212,7 @@ sql::InitStatus ThumbnailDatabase::OpenDatabase(sql::Connection* db, db->set_histogram_tag("Thumbnail"); db->set_error_callback(base::Bind(&DatabaseErrorCallback, - db, db_name, startup_kb, history_client_)); + db, db_name, startup_kb, backend_client_)); // Thumbnails db now only stores favicons, so we don't need that big a page // size or cache. diff --git a/components/history/core/browser/thumbnail_database.h b/components/history/core/browser/thumbnail_database.h index 4253cb2..7e7eef1 100644 --- a/components/history/core/browser/thumbnail_database.h +++ b/components/history/core/browser/thumbnail_database.h @@ -23,7 +23,7 @@ class Time; namespace history { -class HistoryClient; +class HistoryBackendClient; // This database interface is owned by the history backend and runs on the // history thread. It is a totally separate component from history partially @@ -34,7 +34,7 @@ class HistoryClient; // higher priority history operations. class ThumbnailDatabase { public: - explicit ThumbnailDatabase(HistoryClient* history_client); + explicit ThumbnailDatabase(HistoryBackendClient* backend_client); ~ThumbnailDatabase(); // Must be called after creation but before any other methods are called. @@ -272,7 +272,7 @@ class ThumbnailDatabase { sql::Connection db_; sql::MetaTable meta_table_; - HistoryClient* history_client_; + HistoryBackendClient* backend_client_; }; } // namespace history diff --git a/components/history/core/browser/typed_url_syncable_service_unittest.cc b/components/history/core/browser/typed_url_syncable_service_unittest.cc index a0c6ea2..bbdceba8c 100644 --- a/components/history/core/browser/typed_url_syncable_service_unittest.cc +++ b/components/history/core/browser/typed_url_syncable_service_unittest.cc @@ -11,6 +11,7 @@ #include "base/strings/utf_string_conversions.h" #include "base/thread_task_runner_handle.h" #include "components/history/core/browser/history_backend.h" +#include "components/history/core/browser/history_backend_client.h" #include "components/history/core/browser/history_types.h" #include "sync/api/fake_sync_change_processor.h" #include "sync/api/sync_change_processor_wrapper_for_test.h" diff --git a/components/history/core/test/history_backend_db_base_test.cc b/components/history/core/test/history_backend_db_base_test.cc index cd6ca28..36bad37 100644 --- a/components/history/core/test/history_backend_db_base_test.cc +++ b/components/history/core/test/history_backend_db_base_test.cc @@ -12,6 +12,7 @@ #include "components/history/core/browser/download_constants.h" #include "components/history/core/browser/download_row.h" #include "components/history/core/browser/history_backend.h" +#include "components/history/core/browser/history_backend_client.h" #include "components/history/core/browser/history_constants.h" #include "components/history/core/browser/history_database_params.h" #include "components/history/core/browser/in_memory_history_backend.h" diff --git a/components/history/core/test/history_client_fake_bookmarks.cc b/components/history/core/test/history_client_fake_bookmarks.cc index 8f2e084..6874442 100644 --- a/components/history/core/test/history_client_fake_bookmarks.cc +++ b/components/history/core/test/history_client_fake_bookmarks.cc @@ -4,44 +4,175 @@ #include "components/history/core/test/history_client_fake_bookmarks.h" +#include <map> + +#include "base/synchronization/lock.h" +#include "components/history/core/browser/history_backend_client.h" +#include "url/gurl.h" + namespace history { +class FakeBookmarkDatabase : public base::RefCounted<FakeBookmarkDatabase> { + public: + FakeBookmarkDatabase() {} + + void ClearAllBookmarks(); + void AddBookmarkWithTitle(const GURL& url, const base::string16& title); + void DelBookmark(const GURL& url); + + bool IsBookmarked(const GURL& url); + void GetBookmarks(std::vector<URLAndTitle>* bookmarks); + + private: + friend class base::RefCounted<FakeBookmarkDatabase>; + + ~FakeBookmarkDatabase() {} + + base::Lock lock_; + std::map<GURL, base::string16> bookmarks_; + + DISALLOW_COPY_AND_ASSIGN(FakeBookmarkDatabase); +}; + +void FakeBookmarkDatabase::ClearAllBookmarks() { + base::AutoLock with_lock(lock_); + bookmarks_.clear(); +} + +void FakeBookmarkDatabase::AddBookmarkWithTitle(const GURL& url, + const base::string16& title) { + base::AutoLock with_lock(lock_); + bookmarks_.insert(std::make_pair(url, title)); +} + +void FakeBookmarkDatabase::DelBookmark(const GURL& url) { + base::AutoLock with_lock(lock_); + auto iter = bookmarks_.find(url); + if (iter != bookmarks_.end()) + bookmarks_.erase(iter); +} + +bool FakeBookmarkDatabase::IsBookmarked(const GURL& url) { + base::AutoLock with_lock(lock_); + return bookmarks_.find(url) != bookmarks_.end(); +} + +void FakeBookmarkDatabase::GetBookmarks(std::vector<URLAndTitle>* bookmarks) { + base::AutoLock with_lock(lock_); + bookmarks->reserve(bookmarks->size() + bookmarks_.size()); + for (const auto& pair : bookmarks_) { + URLAndTitle url_and_title = { pair.first, pair.second }; + bookmarks->push_back(url_and_title); + } +} + +namespace { + +class HistoryBackendClientFakeBookmarks : public HistoryBackendClient { + public: + explicit HistoryBackendClientFakeBookmarks( + const scoped_refptr<FakeBookmarkDatabase>& bookmarks); + ~HistoryBackendClientFakeBookmarks() override; + + // HistoryBackendClient implementation. + bool IsBookmarked(const GURL& url) override; + void GetBookmarks(std::vector<URLAndTitle>* bookmarks) override; + bool ShouldReportDatabaseError() override; +#if defined(OS_ANDROID) + void OnHistoryBackendInitialized(HistoryBackend* history_backend, + HistoryDatabase* history_database, + ThumbnailDatabase* thumbnail_database, + const base::FilePath& history_dir) override; + void OnHistoryBackendDestroyed(HistoryBackend* history_backend, + const base::FilePath& history_dir) override; +#endif // defined(OS_ANDROID) + + private: + scoped_refptr<FakeBookmarkDatabase> bookmarks_; + + DISALLOW_COPY_AND_ASSIGN(HistoryBackendClientFakeBookmarks); +}; + +HistoryBackendClientFakeBookmarks::HistoryBackendClientFakeBookmarks( + const scoped_refptr<FakeBookmarkDatabase>& bookmarks) + : bookmarks_(bookmarks) { +} + +HistoryBackendClientFakeBookmarks::~HistoryBackendClientFakeBookmarks() { +} + +bool HistoryBackendClientFakeBookmarks::IsBookmarked(const GURL& url) { + return bookmarks_->IsBookmarked(url); +} + +void HistoryBackendClientFakeBookmarks::GetBookmarks( + std::vector<URLAndTitle>* bookmarks) { + bookmarks_->GetBookmarks(bookmarks); +} + +bool HistoryBackendClientFakeBookmarks::ShouldReportDatabaseError() { + return false; +} + +#if defined(OS_ANDROID) +void HistoryBackendClientFakeBookmarks::OnHistoryBackendInitialized( + HistoryBackend* history_backend, + HistoryDatabase* history_database, + ThumbnailDatabase* thumbnail_database, + const base::FilePath& history_dir) { +} + +void HistoryBackendClientFakeBookmarks::OnHistoryBackendDestroyed( + HistoryBackend* history_backend, + const base::FilePath& history_dir) { +} +#endif // defined(OS_ANDROID) + +} // namespace + HistoryClientFakeBookmarks::HistoryClientFakeBookmarks() { + bookmarks_ = new FakeBookmarkDatabase; } HistoryClientFakeBookmarks::~HistoryClientFakeBookmarks() { } void HistoryClientFakeBookmarks::ClearAllBookmarks() { - bookmarks_.clear(); + bookmarks_->ClearAllBookmarks(); } void HistoryClientFakeBookmarks::AddBookmark(const GURL& url) { - AddBookmarkWithTitle(url, base::string16()); + bookmarks_->AddBookmarkWithTitle(url, base::string16()); } void HistoryClientFakeBookmarks::AddBookmarkWithTitle( const GURL& url, const base::string16& title) { - bookmarks_.insert(std::make_pair(url, title)); + bookmarks_->AddBookmarkWithTitle(url, title); } void HistoryClientFakeBookmarks::DelBookmark(const GURL& url) { - bookmarks_.erase(url); + bookmarks_->DelBookmark(url); } bool HistoryClientFakeBookmarks::IsBookmarked(const GURL& url) { - return bookmarks_.find(url) != bookmarks_.end(); + return bookmarks_->IsBookmarked(url); } -void HistoryClientFakeBookmarks::GetBookmarks( - std::vector<URLAndTitle>* bookmarks) { - bookmarks->reserve(bookmarks->size() + bookmarks_.size()); - typedef std::map<GURL, base::string16>::const_iterator iterator; - for (iterator i = bookmarks_.begin(); i != bookmarks_.end(); ++i) { - URLAndTitle urlAndTitle = {i->first, i->second}; - bookmarks->push_back(urlAndTitle); - } +void HistoryClientFakeBookmarks::Shutdown() { +} + +bool HistoryClientFakeBookmarks::CanAddURL(const GURL& url) { + return url.is_valid(); +} + +void HistoryClientFakeBookmarks::NotifyProfileError( + sql::InitStatus init_status) { +} + +scoped_ptr<HistoryBackendClient> +HistoryClientFakeBookmarks::CreateBackendClient() { + return make_scoped_ptr(new HistoryBackendClientFakeBookmarks(bookmarks_)); } } // namespace history diff --git a/components/history/core/test/history_client_fake_bookmarks.h b/components/history/core/test/history_client_fake_bookmarks.h index ee5c540..65041ba 100644 --- a/components/history/core/test/history_client_fake_bookmarks.h +++ b/components/history/core/test/history_client_fake_bookmarks.h @@ -5,12 +5,17 @@ #ifndef COMPONENTS_HISTORY_CORE_TEST_HISTORY_CLIENT_FAKE_BOOKMARKS_H_ #define COMPONENTS_HISTORY_CORE_TEST_HISTORY_CLIENT_FAKE_BOOKMARKS_H_ -#include <map> - +#include "base/memory/ref_counted.h" +#include "base/strings/string16.h" #include "components/history/core/browser/history_client.h" +class GURL; + namespace history { +class FakeBookmarkDatabase; +class HistoryBackendClient; + // The class HistoryClientFakeBookmarks implements HistoryClient faking the // methods relating to bookmarks for unit testing. class HistoryClientFakeBookmarks : public HistoryClient { @@ -22,13 +27,16 @@ class HistoryClientFakeBookmarks : public HistoryClient { void AddBookmark(const GURL& url); void AddBookmarkWithTitle(const GURL& url, const base::string16& title); void DelBookmark(const GURL& url); + bool IsBookmarked(const GURL& url); - // HistoryClient: - bool IsBookmarked(const GURL& url) override; - void GetBookmarks(std::vector<URLAndTitle>* bookmarks) override; + // HistoryClient implementation. + void Shutdown() override; + bool CanAddURL(const GURL& url) override; + void NotifyProfileError(sql::InitStatus init_status) override; + scoped_ptr<HistoryBackendClient> CreateBackendClient() override; private: - std::map<GURL, base::string16> bookmarks_; + scoped_refptr<FakeBookmarkDatabase> bookmarks_; DISALLOW_COPY_AND_ASSIGN(HistoryClientFakeBookmarks); }; |