diff options
| -rw-r--r-- | chrome/browser/history/DEPS | 2 | ||||
| -rw-r--r-- | chrome/browser/history/history.cc | 94 | ||||
| -rw-r--r-- | chrome/browser/history/history.h | 40 | ||||
| -rw-r--r-- | chrome/browser/history/history_backend.cc | 4 | ||||
| -rw-r--r-- | chrome/browser/history/history_backend.h | 2 | ||||
| -rw-r--r-- | chrome/browser/profiles/profile_dependency_manager.cc | 2 | ||||
| -rw-r--r-- | chrome/browser/visitedlink/visitedlink_delegate.h | 46 | ||||
| -rw-r--r-- | chrome/browser/visitedlink/visitedlink_event_listener.cc | 36 | ||||
| -rw-r--r-- | chrome/browser/visitedlink/visitedlink_event_listener.h | 14 | ||||
| -rw-r--r-- | chrome/browser/visitedlink/visitedlink_master.cc | 81 | ||||
| -rw-r--r-- | chrome/browser/visitedlink/visitedlink_master.h | 60 | ||||
| -rw-r--r-- | chrome/browser/visitedlink/visitedlink_master_factory.cc | 45 | ||||
| -rw-r--r-- | chrome/browser/visitedlink/visitedlink_master_factory.h | 38 | ||||
| -rw-r--r-- | chrome/browser/visitedlink/visitedlink_perftest.cc | 1 | ||||
| -rw-r--r-- | chrome/browser/visitedlink/visitedlink_unittest.cc | 157 | ||||
| -rw-r--r-- | chrome/chrome_browser.gypi | 3 |
16 files changed, 297 insertions, 328 deletions
diff --git a/chrome/browser/history/DEPS b/chrome/browser/history/DEPS index a798771..e8a94ab 100644 --- a/chrome/browser/history/DEPS +++ b/chrome/browser/history/DEPS @@ -51,8 +51,6 @@ include_rules = [ "!chrome/browser/ui/profile_error_dialog.h", "!chrome/browser/ui/webui/ntp/most_visited_handler.h", "!chrome/browser/ui/webui/ntp/new_tab_ui.h", - # TODO(boliu): Allow visitedlink after they are componentized. - "!chrome/browser/visitedlink/visitedlink_delegate.h", "!chrome/browser/visitedlink/visitedlink_master.h", ] diff --git a/chrome/browser/history/history.cc b/chrome/browser/history/history.cc index 50450bc..ae0f69a 100644 --- a/chrome/browser/history/history.cc +++ b/chrome/browser/history/history.cc @@ -111,29 +111,6 @@ void RunWithFaviconResults( callback.Run(results->bitmap_results, results->size_map); } -// Extract history::URLRows into GURLs for VisitedLinkMaster. -class URLIteratorFromURLRows : public VisitedLinkMaster::URLIterator { - public: - explicit URLIteratorFromURLRows(const history::URLRows& url_rows) - : itr_(url_rows.begin()), - end_(url_rows.end()) { - } - - virtual const GURL& NextURL() OVERRIDE { - return (itr_++)->url(); - } - - virtual bool HasNextURL() const OVERRIDE { - return itr_ != end_; - } - - private: - history::URLRows::const_iterator itr_; - history::URLRows::const_iterator end_; - - DISALLOW_COPY_AND_ASSIGN(URLIteratorFromURLRows); -}; - } // namespace // Sends messages from the backend to us on the main thread. This must be a @@ -231,8 +208,6 @@ HistoryService::HistoryService(Profile* profile) : weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)), thread_(new base::Thread(kHistoryThreadName)), profile_(profile), - visitedlink_master_(new VisitedLinkMaster( - profile, ALLOW_THIS_IN_INITIALIZER_LIST(this))), backend_loaded_(false), current_backend_id_(-1), bookmark_service_(NULL), @@ -497,8 +472,10 @@ void HistoryService::AddPage(const history::HistoryAddPageArgs& add_page_args) { return; // Add link & all redirects to visited link list. - if (visitedlink_master_) { - visitedlink_master_->AddURL(add_page_args.url); + VisitedLinkMaster* visited_links; + if (profile_ && + (visited_links = VisitedLinkMaster::FromProfile(profile_))) { + visited_links->AddURL(add_page_args.url); if (!add_page_args.redirects.empty()) { // We should not be asked to add a page in the middle of a redirect chain. @@ -508,7 +485,7 @@ void HistoryService::AddPage(const history::HistoryAddPageArgs& add_page_args) { // We need the !redirects.empty() condition above since size_t is unsigned // and will wrap around when we subtract one from a 0 size. for (size_t i = 0; i < add_page_args.redirects.size() - 1; i++) - visitedlink_master_->AddURL(add_page_args.redirects[i]); + visited_links->AddURL(add_page_args.redirects[i]); } } @@ -553,8 +530,11 @@ void HistoryService::AddPageWithDetails(const GURL& url, return; // Add to the visited links system. - if (visitedlink_master_) - visitedlink_master_->AddURL(url); + VisitedLinkMaster* visited_links; + if (profile_ && + (visited_links = VisitedLinkMaster::FromProfile(profile_))) { + visited_links->AddURL(url); + } history::URLRow row(url); row.set_title(title); @@ -574,14 +554,16 @@ void HistoryService::AddPagesWithDetails(const history::URLRows& info, history::VisitSource visit_source) { DCHECK(thread_checker_.CalledOnValidThread()); // Add to the visited links system. - if (visitedlink_master_) { + VisitedLinkMaster* visited_links; + if (profile_ && + (visited_links = VisitedLinkMaster::FromProfile(profile_))) { std::vector<GURL> urls; urls.reserve(info.size()); for (history::URLRows::const_iterator i = info.begin(); i != info.end(); ++i) urls.push_back(i->url()); - visitedlink_master_->AddURLs(urls); + visited_links->AddURLs(urls); } ScheduleAndForget(PRIORITY_NORMAL, @@ -738,6 +720,11 @@ void HistoryService::SetImportedFavicons( &HistoryBackend::SetImportedFavicons, favicon_usage); } +void HistoryService::IterateURLs(URLEnumerator* enumerator) { + DCHECK(thread_checker_.CalledOnValidThread()); + ScheduleAndForget(PRIORITY_NORMAL, &HistoryBackend::IterateURLs, enumerator); +} + HistoryService::Handle HistoryService::QueryURL( const GURL& url, bool want_visits, @@ -915,16 +902,17 @@ void HistoryService::Observe(int type, // delete notifications are by time. We would also like to be more // respectful of privacy and never tell the user something is gone when it // isn't. Therefore, we update the delete URLs after the fact. - if (visitedlink_master_) { - content::Details<history::URLsDeletedDetails> deleted_details(details); - - if (deleted_details->all_history) { - visitedlink_master_->DeleteAllURLs(); - } else { - URLIteratorFromURLRows iterator(deleted_details->rows); - visitedlink_master_->DeleteURLs(&iterator); - } - } + if (!profile_) + return; // No profile, probably unit testing. + content::Details<history::URLsDeletedDetails> deleted_details(details); + VisitedLinkMaster* visited_links = + VisitedLinkMaster::FromProfile(profile_); + if (!visited_links) + return; // Nobody to update. + if (deleted_details->all_history) + visited_links->DeleteAllURLs(); + else // Delete individual ones. + visited_links->DeleteURLs(deleted_details->rows); break; } @@ -938,22 +926,6 @@ void HistoryService::Observe(int type, } } -bool HistoryService::AreEquivalentContexts(content::BrowserContext* context1, - content::BrowserContext* context2) { - DCHECK(context1); - DCHECK(context2); - - Profile* profile1 = Profile::FromBrowserContext(context1); - Profile* profile2 = Profile::FromBrowserContext(context2); - - return profile1->IsSameProfile(profile2); -} - -void HistoryService::RebuildTable(URLEnumerator* enumerator) { - DCHECK(thread_checker_.CalledOnValidThread()); - ScheduleAndForget(PRIORITY_NORMAL, &HistoryBackend::IterateURLs, enumerator); -} - bool HistoryService::Init(const FilePath& history_dir, BookmarkService* bookmark_service, bool no_db) { @@ -977,12 +949,6 @@ bool HistoryService::Init(const FilePath& history_dir, // Create the history backend. LoadBackendIfNecessary(); - - if (visitedlink_master_) { - bool result = visitedlink_master_->Init(); - DCHECK(result); - } - return true; } diff --git a/chrome/browser/history/history.h b/chrome/browser/history/history.h index b8c48a2..011764f 100644 --- a/chrome/browser/history/history.h +++ b/chrome/browser/history/history.h @@ -25,7 +25,6 @@ #include "chrome/browser/history/history_types.h" #include "chrome/browser/profiles/profile_keyed_service.h" #include "chrome/browser/search_engines/template_url_id.h" -#include "chrome/browser/visitedlink/visitedlink_delegate.h" #include "chrome/common/cancelable_task_tracker.h" #include "chrome/common/ref_counted_util.h" #include "content/public/browser/notification_observer.h" @@ -46,7 +45,6 @@ class HistoryURLProvider; class PageUsageData; class PageUsageRequest; class Profile; -class VisitedLinkMaster; struct HistoryURLProviderParams; namespace base { @@ -109,8 +107,7 @@ class HistoryDBTask : public base::RefCountedThreadSafe<HistoryDBTask> { class HistoryService : public CancelableRequestProvider, public content::NotificationObserver, public syncer::SyncableService, - public ProfileKeyedService, - public VisitedLinkDelegate { + public ProfileKeyedService { public: // Miscellaneous commonly-used types. typedef std::vector<PageUsageData*> PageUsageDataList; @@ -264,6 +261,31 @@ class HistoryService : public CancelableRequestProvider, // Querying ------------------------------------------------------------------ + // Callback class that a client can implement to iterate over URLs. The + // callbacks WILL BE CALLED ON THE BACKGROUND THREAD! Your implementation + // should handle this appropriately. + class URLEnumerator { + public: + // Indicates that a URL is available. There will be exactly one call for + // every URL in history. + virtual void OnURL(const history::URLRow& url_row) = 0; + + // Indicates we are done iterating over URLs. Once called, there will be no + // more callbacks made. This call is guaranteed to occur, even if there are + // no URLs. If all URLs were iterated, success will be true. + virtual void OnComplete(bool success) = 0; + + protected: + virtual ~URLEnumerator() {} + }; + + // Enumerate all URLs in history. The given iterator will be owned by the + // caller, so the caller should ensure it exists until OnComplete is called. + // You should not generally use this since it will be slow to slurp all URLs + // in from the database. It is designed for rebuilding the visited link + // database from history. + void IterateURLs(URLEnumerator* iterator); + // Returns the information about the requested URL. If the URL is found, // success will be true and the information will be in the URLRow parameter. // On success, the visits, if requested, will be sorted by date. If they have @@ -657,12 +679,6 @@ class HistoryService : public CancelableRequestProvider, const content::NotificationSource& source, const content::NotificationDetails& details) OVERRIDE; - // Implementation of VisitedLinkDelegate. - virtual bool AreEquivalentContexts( - content::BrowserContext* context1, - content::BrowserContext* context2) OVERRIDE; - virtual void RebuildTable(URLEnumerator* enumerator) OVERRIDE; - // Low-level Init(). Same as the public version, but adds a |no_db| parameter // that is only set by unittests which causes the backend to not init its DB. bool Init(const FilePath& history_dir, @@ -1087,10 +1103,6 @@ class HistoryService : public CancelableRequestProvider, // The profile, may be null when testing. Profile* profile_; - // Used for propagating link highlighting data across renderers. May be null - // in tests. - scoped_ptr<VisitedLinkMaster> visitedlink_master_; - // Has the backend finished loading? The backend is loaded once Init has // completed. bool backend_loaded_; diff --git a/chrome/browser/history/history_backend.cc b/chrome/browser/history/history_backend.cc index 5246f88..d53f672 100644 --- a/chrome/browser/history/history_backend.cc +++ b/chrome/browser/history/history_backend.cc @@ -1058,13 +1058,13 @@ void HistoryBackend::AddPageNoVisitForBookmark(const GURL& url, db_->AddURL(url_info); } -void HistoryBackend::IterateURLs(VisitedLinkDelegate::URLEnumerator* iterator) { +void HistoryBackend::IterateURLs(HistoryService::URLEnumerator* iterator) { if (db_.get()) { HistoryDatabase::URLEnumerator e; if (db_->InitURLEnumeratorForEverything(&e)) { URLRow info; while (e.GetNextURL(&info)) { - iterator->OnURL(info.url()); + iterator->OnURL(info); } iterator->OnComplete(true); // Success. return; diff --git a/chrome/browser/history/history_backend.h b/chrome/browser/history/history_backend.h index 31698f1..ab0acb8 100644 --- a/chrome/browser/history/history_backend.h +++ b/chrome/browser/history/history_backend.h @@ -173,7 +173,7 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, void ScheduleAutocomplete(HistoryURLProvider* provider, HistoryURLProviderParams* params); - void IterateURLs(VisitedLinkDelegate::URLEnumerator* enumerator); + void IterateURLs(HistoryService::URLEnumerator* enumerator); void QueryURL(scoped_refptr<QueryURLRequest> request, const GURL& url, bool want_visits); diff --git a/chrome/browser/profiles/profile_dependency_manager.cc b/chrome/browser/profiles/profile_dependency_manager.cc index 6f35bc3..e1bb6c3 100644 --- a/chrome/browser/profiles/profile_dependency_manager.cc +++ b/chrome/browser/profiles/profile_dependency_manager.cc @@ -74,6 +74,7 @@ #include "chrome/browser/ui/webui/chrome_url_data_manager_factory.h" #include "chrome/browser/ui/webui/ntp/ntp_resource_cache_factory.h" #include "chrome/browser/user_style_sheet_watcher_factory.h" +#include "chrome/browser/visitedlink/visitedlink_master_factory.h" #include "chrome/browser/webdata/web_data_service_factory.h" #if defined(ENABLE_CAPTIVE_PORTAL_DETECTION) @@ -324,6 +325,7 @@ void ProfileDependencyManager::AssertFactoriesBuilt() { #endif TokenServiceFactory::GetInstance(); UserStyleSheetWatcherFactory::GetInstance(); + VisitedLinkMasterFactory::GetInstance(); WebDataServiceFactory::GetInstance(); #if defined(ENABLE_WEB_INTENTS) WebIntentsRegistryFactory::GetInstance(); diff --git a/chrome/browser/visitedlink/visitedlink_delegate.h b/chrome/browser/visitedlink/visitedlink_delegate.h deleted file mode 100644 index d58a227..0000000 --- a/chrome/browser/visitedlink/visitedlink_delegate.h +++ /dev/null @@ -1,46 +0,0 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef CHROME_BROWSER_VISITEDLINK_VISITEDLINK_DELEGATE_H_ -#define CHROME_BROWSER_VISITEDLINK_VISITEDLINK_DELEGATE_H_ - -class GURL; - -namespace content { -class BrowserContext; -} // namespace content - -// Delegate class that clients of VisitedLinkMaster must implement. -class VisitedLinkDelegate { - public: - // Returns true when the two BrowserContexts are equivalent. - virtual bool AreEquivalentContexts(content::BrowserContext* context1, - content::BrowserContext* context2) = 0; - - // See RebuildTable. - class URLEnumerator { - public: - // Call this with each URL to rebuild the table. - virtual void OnURL(const GURL& url) = 0; - - // This must be called by Delegate after RebuildTable is called. |success| - // indicates all URLs have been returned successfully. The URLEnumerator - // object cannot be used by the delegate after this call. - virtual void OnComplete(bool success) = 0; - - protected: - virtual ~URLEnumerator() {} - }; - - // Delegate class is responsible for persisting the list of visited URLs - // across browser runs. This is called by VisitedLinkMaster to repopulate - // its internal table. Note that methods on enumerator can be called on any - // thread but the delegate is responsible for synchronizating the calls. - virtual void RebuildTable(URLEnumerator* enumerator) = 0; - - protected: - virtual ~VisitedLinkDelegate() {} -}; - -#endif // CHROME_BROWSER_VISITEDLINK_VISITEDLINK_DELEGATE_H_ diff --git a/chrome/browser/visitedlink/visitedlink_event_listener.cc b/chrome/browser/visitedlink/visitedlink_event_listener.cc index 685c334..423ebc1 100644 --- a/chrome/browser/visitedlink/visitedlink_event_listener.cc +++ b/chrome/browser/visitedlink/visitedlink_event_listener.cc @@ -5,7 +5,8 @@ #include "chrome/browser/visitedlink/visitedlink_event_listener.h" #include "base/shared_memory.h" -#include "chrome/browser/visitedlink/visitedlink_delegate.h" +#include "chrome/browser/profiles/profile.h" +#include "chrome/browser/visitedlink/visitedlink_master_factory.h" #include "chrome/common/render_messages.h" #include "content/public/browser/notification_service.h" #include "content/public/browser/notification_types.h" @@ -107,11 +108,8 @@ class VisitedLinkUpdater { VisitedLinkCommon::Fingerprints pending_; }; -VisitedLinkEventListener::VisitedLinkEventListener( - VisitedLinkMaster* master, - content::BrowserContext* browser_context) - : master_(master), - browser_context_(browser_context) { +VisitedLinkEventListener::VisitedLinkEventListener(Profile* profile) + : profile_(profile) { registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_CREATED, content::NotificationService::AllBrowserContextsAndSources()); registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_TERMINATED, @@ -136,8 +134,12 @@ void VisitedLinkEventListener::NewTable(base::SharedMemory* table_memory) { content::RenderProcessHost::FromID(i->first); if (!process) continue; - - i->second->SendVisitedLinkTable(table_memory); + Profile* profile = Profile::FromBrowserContext( + process->GetBrowserContext()); + VisitedLinkMaster* master = + VisitedLinkMasterFactory::GetForProfile(profile); + if (master && master->shared_memory() == table_memory) + i->second->SendVisitedLinkTable(table_memory); } } @@ -179,18 +181,22 @@ void VisitedLinkEventListener::Observe( case content::NOTIFICATION_RENDERER_PROCESS_CREATED: { content::RenderProcessHost* process = content::Source<content::RenderProcessHost>(source).ptr(); - if (!master_->GetDelegate()->AreEquivalentContexts( - browser_context_, process->GetBrowserContext())) + Profile* profile = + Profile::FromBrowserContext(process->GetBrowserContext()); + if (!profile_->IsSameProfile(profile)) return; + updaters_[process->GetID()] = + make_linked_ptr(new VisitedLinkUpdater(process->GetID())); - // Happens on browser start up. - if (!master_->shared_memory()) + // Initialize support for visited links. Send the renderer process its + // initial set of visited links. + VisitedLinkMaster* master = + VisitedLinkMasterFactory::GetForProfile(profile); + if (!master) return; - updaters_[process->GetID()] = - make_linked_ptr(new VisitedLinkUpdater(process->GetID())); updaters_[process->GetID()]->SendVisitedLinkTable( - master_->shared_memory()); + master->shared_memory()); break; } case content::NOTIFICATION_RENDERER_PROCESS_TERMINATED: { diff --git a/chrome/browser/visitedlink/visitedlink_event_listener.h b/chrome/browser/visitedlink/visitedlink_event_listener.h index 2c3f3d4..543f46c 100644 --- a/chrome/browser/visitedlink/visitedlink_event_listener.h +++ b/chrome/browser/visitedlink/visitedlink_event_listener.h @@ -17,21 +17,17 @@ #include "content/public/browser/notification_observer.h" #include "content/public/browser/notification_registrar.h" +class Profile; class VisitedLinkUpdater; namespace base { class SharedMemory; } -namespace content { -class BrowserContext; -} // namespace content - class VisitedLinkEventListener : public VisitedLinkMaster::Listener, public content::NotificationObserver { public: - VisitedLinkEventListener(VisitedLinkMaster* master, - content::BrowserContext* browser_context); + explicit VisitedLinkEventListener(Profile* profile); virtual ~VisitedLinkEventListener(); virtual void NewTable(base::SharedMemory* table_memory) OVERRIDE; @@ -55,11 +51,7 @@ class VisitedLinkEventListener : public VisitedLinkMaster::Listener, typedef std::map<int, linked_ptr<VisitedLinkUpdater> > Updaters; Updaters updaters_; - VisitedLinkMaster* master_; - - // Used to filter RENDERER_PROCESS_CREATED notifications to renderers that - // belong to this BrowserContext. - content::BrowserContext* browser_context_; + Profile* profile_; DISALLOW_COPY_AND_ASSIGN(VisitedLinkEventListener); }; diff --git a/chrome/browser/visitedlink/visitedlink_master.cc b/chrome/browser/visitedlink/visitedlink_master.cc index 96e47fd..ec3c9e6 100644 --- a/chrome/browser/visitedlink/visitedlink_master.cc +++ b/chrome/browser/visitedlink/visitedlink_master.cc @@ -24,11 +24,11 @@ #include "base/rand_util.h" #include "base/string_util.h" #include "base/threading/thread_restrictions.h" -#include "chrome/browser/visitedlink/visitedlink_delegate.h" +#include "chrome/browser/history/history.h" +#include "chrome/browser/history/history_service_factory.h" +#include "chrome/browser/profiles/profile.h" #include "chrome/browser/visitedlink/visitedlink_event_listener.h" -#include "content/public/browser/browser_context.h" #include "content/public/browser/browser_thread.h" -#include "googleurl/src/gurl.h" using content::BrowserThread; using file_util::ScopedFILE; @@ -138,7 +138,7 @@ void AsyncClose(FILE** file) { // which case it notifies the builder via DisownMaster(). The builder will // delete itself once rebuilding is complete, and not execute any callback. class VisitedLinkMaster::TableBuilder - : public VisitedLinkDelegate::URLEnumerator, + : public HistoryService::URLEnumerator, public base::RefCountedThreadSafe<TableBuilder> { public: TableBuilder(VisitedLinkMaster* master, @@ -150,8 +150,8 @@ class VisitedLinkMaster::TableBuilder // table will be being rebuilt simultaneously on the other thread. void DisownMaster(); - // VisitedLinkDelegate::URLEnumerator - virtual void OnURL(const GURL& url); + // HistoryService::URLEnumerator + virtual void OnURL(const history::URLRow& url_row); virtual void OnComplete(bool succeed); private: @@ -174,34 +174,30 @@ class VisitedLinkMaster::TableBuilder // Stores the fingerprints we computed on the background thread. VisitedLinkCommon::Fingerprints fingerprints_; - - DISALLOW_COPY_AND_ASSIGN(TableBuilder); }; // VisitedLinkMaster ---------------------------------------------------------- -VisitedLinkMaster::VisitedLinkMaster(content::BrowserContext* browser_context, - VisitedLinkDelegate* delegate) - : browser_context_(browser_context), - delegate_(delegate), - listener_(new VisitedLinkEventListener( - ALLOW_THIS_IN_INITIALIZER_LIST(this), browser_context)) { +VisitedLinkMaster::VisitedLinkMaster(Profile* profile) + : profile_(profile) { + listener_.reset(new VisitedLinkEventListener(profile)); + DCHECK(listener_.get()); InitMembers(); } VisitedLinkMaster::VisitedLinkMaster(Listener* listener, - VisitedLinkDelegate* delegate, + HistoryService* history_service, bool suppress_rebuild, const FilePath& filename, int32 default_table_size) - : browser_context_(NULL), - delegate_(delegate) { + : profile_(NULL) { listener_.reset(listener); DCHECK(listener_.get()); InitMembers(); database_name_override_ = filename; table_size_override_ = default_table_size; + history_service_override_ = history_service; suppress_rebuild_ = suppress_rebuild; } @@ -224,6 +220,7 @@ void VisitedLinkMaster::InitMembers() { shared_memory_serial_ = 0; used_items_ = 0; table_size_override_ = 0; + history_service_override_ = NULL; suppress_rebuild_ = false; sequence_token_ = BrowserThread::GetBlockingPool()->GetSequenceToken(); @@ -244,9 +241,7 @@ bool VisitedLinkMaster::Init() { VisitedLinkMaster::Hash VisitedLinkMaster::TryToAddURL(const GURL& url) { // Extra check that we are not incognito. This should not happen. - // TODO(boliu): Move this check to HistoryService when IsOffTheRecord is - // removed from BrowserContext. - if (browser_context_ && browser_context_->IsOffTheRecord()) { + if (profile_ && profile_->IsOffTheRecord()) { NOTREACHED(); return null_hash_; } @@ -325,12 +320,10 @@ void VisitedLinkMaster::DeleteAllURLs() { listener_->Reset(); } -VisitedLinkDelegate* VisitedLinkMaster::GetDelegate() { - return delegate_; -} +void VisitedLinkMaster::DeleteURLs(const history::URLRows& rows) { + typedef std::set<GURL>::const_iterator SetIterator; -void VisitedLinkMaster::DeleteURLs(URLIterator* urls) { - if (!urls->HasNextURL()) + if (rows.empty()) return; listener_->Reset(); @@ -338,8 +331,9 @@ void VisitedLinkMaster::DeleteURLs(URLIterator* urls) { if (table_builder_) { // A rebuild is in progress, save this deletion in the temporary list so // it can be added once rebuild is complete. - while (urls->HasNextURL()) { - const GURL& url(urls->NextURL()); + for (history::URLRows::const_iterator i = rows.begin(); i != rows.end(); + ++i) { + const GURL& url(i->url()); if (!url.is_valid()) continue; @@ -363,8 +357,9 @@ void VisitedLinkMaster::DeleteURLs(URLIterator* urls) { // Compute the deleted URLs' fingerprints and delete them std::set<Fingerprint> deleted_fingerprints; - while (urls->HasNextURL()) { - const GURL& url(urls->NextURL()); + for (history::URLRows::const_iterator i = rows.begin(); i != rows.end(); + ++i) { + const GURL& url(i->url()); if (!url.is_valid()) continue; deleted_fingerprints.insert( @@ -586,7 +581,7 @@ bool VisitedLinkMaster::InitFromScratch(bool suppress_rebuild) { // to disk. We don't want to save explicitly here, since the rebuild may // not complete, leaving us with an empty but valid visited link database. // In the future, we won't know we need to try rebuilding again. - return RebuildTableFromDelegate(); + return RebuildTableFromHistory(); } bool VisitedLinkMaster::ReadFileHeader(FILE* file, @@ -646,10 +641,10 @@ bool VisitedLinkMaster::GetDatabaseFileName(FilePath* filename) { return true; } - if (!browser_context_ || browser_context_->GetPath().empty()) + if (!profile_ || profile_->GetPath().empty()) return false; - FilePath profile_dir = browser_context_->GetPath(); + FilePath profile_dir = profile_->GetPath(); *filename = profile_dir.Append(FILE_PATH_LITERAL("Visited Links")); return true; } @@ -815,8 +810,23 @@ uint32 VisitedLinkMaster::NewTableSizeForCount(int32 item_count) const { } // See the TableBuilder definition in the header file for how this works. -bool VisitedLinkMaster::RebuildTableFromDelegate() { +bool VisitedLinkMaster::RebuildTableFromHistory() { DCHECK(!table_builder_); + if (table_builder_) + return false; + + HistoryService* history_service = history_service_override_; + if (!history_service && profile_) { + history_service = + HistoryServiceFactory::GetForProfile(profile_, + Profile::EXPLICIT_ACCESS); + } + + if (!history_service) { + DLOG(WARNING) << "Attempted to rebuild visited link table, but couldn't " + "obtain a HistoryService."; + return false; + } // TODO(brettw) make sure we have reasonable salt! table_builder_ = new TableBuilder(this, salt_); @@ -824,7 +834,7 @@ bool VisitedLinkMaster::RebuildTableFromDelegate() { // Make sure the table builder stays live during the call, even if the // master is deleted. This is balanced in TableBuilder::OnCompleteMainThread. table_builder_->AddRef(); - delegate_->RebuildTable(table_builder_); + history_service->IterateURLs(table_builder_); return true; } @@ -948,7 +958,8 @@ void VisitedLinkMaster::TableBuilder::DisownMaster() { master_ = NULL; } -void VisitedLinkMaster::TableBuilder::OnURL(const GURL& url) { +void VisitedLinkMaster::TableBuilder::OnURL(const history::URLRow& url_row) { + const GURL& url(url_row.url()); if (!url.is_empty()) { fingerprints_.push_back(VisitedLinkMaster::ComputeURLFingerprint( url.spec().data(), url.spec().length(), salt_)); diff --git a/chrome/browser/visitedlink/visitedlink_master.h b/chrome/browser/visitedlink/visitedlink_master.h index ff07d5d..039ca31 100644 --- a/chrome/browser/visitedlink/visitedlink_master.h +++ b/chrome/browser/visitedlink/visitedlink_master.h @@ -11,26 +11,19 @@ #include <set> #include <vector> -#include "base/callback.h" #include "base/callback_forward.h" #include "base/file_path.h" #include "base/gtest_prod_util.h" #include "base/memory/ref_counted.h" #include "base/shared_memory.h" #include "base/threading/sequenced_worker_pool.h" +#include "chrome/browser/history/history.h" +#include "chrome/browser/history/history_types.h" +#include "chrome/browser/profiles/profile_keyed_service.h" #include "chrome/common/visitedlink_common.h" -#if defined(UNIT_TEST) || defined(PERF_TEST) || !defined(NDEBUG) -#include "base/logging.h" -#endif - class GURL; -class VisitedLinkDelegate; - -namespace content { -class BrowserContext; -} // namespace content - +class Profile; // Controls the link coloring database. The master controls all writing to the // database as well as disk I/O. There should be only one master. @@ -38,7 +31,8 @@ class BrowserContext; // This class will defer writing operations to the file thread. This means that // class destruction, the file may still be open since operations are pending on // another thread. -class VisitedLinkMaster : public VisitedLinkCommon { +class VisitedLinkMaster : public VisitedLinkCommon, + public ProfileKeyedService { public: // Listens to the link coloring database events. The master is given this // event as a constructor argument and dispatches events using it. @@ -59,8 +53,7 @@ class VisitedLinkMaster : public VisitedLinkCommon { virtual void Reset() = 0; }; - VisitedLinkMaster(content::BrowserContext* browser_context, - VisitedLinkDelegate* delegate); + explicit VisitedLinkMaster(Profile* profile); // In unit test mode, we allow the caller to optionally specify the database // filename so that it can be run from a unit test. The directory where this @@ -78,12 +71,15 @@ class VisitedLinkMaster : public VisitedLinkCommon { // history if the file can't be loaded. This should generally be set for // testing except when you want to test the rebuild process explicitly. VisitedLinkMaster(Listener* listener, - VisitedLinkDelegate* delegate, + HistoryService* history_service, bool suppress_rebuild, const FilePath& filename, int32 default_table_size); virtual ~VisitedLinkMaster(); + // Return the VisitedLinkMaster instance for a profile. + static VisitedLinkMaster* FromProfile(Profile* profile); + // Must be called immediately after object creation. Nothing else will work // until this is called. Returns true on success, false means that this // object won't work. @@ -97,31 +93,13 @@ class VisitedLinkMaster : public VisitedLinkCommon { // Adds a set of URLs to the table. void AddURLs(const std::vector<GURL>& url); - // See DeleteURLs. - class URLIterator { - public: - // HasNextURL must return true when this is called. Returns the next URL - // then advances the iterator. Note that the returned reference is only - // valid until the next call of NextURL. - virtual const GURL& NextURL() = 0; - - // Returns true if still has URLs to be iterated. - virtual bool HasNextURL() const = 0; - - protected: - virtual ~URLIterator() {} - }; - // Deletes the specified URLs from |rows| from the table. - void DeleteURLs(URLIterator* iterator); + void DeleteURLs(const history::URLRows& rows); // Clears the visited links table by deleting the file from disk. Used as // part of history clearing. void DeleteAllURLs(); - // Returns the Delegate of this Master. - VisitedLinkDelegate* GetDelegate(); - #if defined(UNIT_TEST) || !defined(NDEBUG) || defined(PERF_TEST) // This is a debugging function that can be called to double-check internal // data structures. It will assert if the check fails. @@ -311,7 +289,7 @@ class VisitedLinkMaster : public VisitedLinkCommon { // // Returns true on success. Failure means we're not attempting to rebuild // the database because something failed. - bool RebuildTableFromDelegate(); + bool RebuildTableFromHistory(); // Callback that the table rebuilder uses when the rebuild is complete. // |success| is true if the fingerprint generation succeeded, in which case @@ -342,13 +320,9 @@ class VisitedLinkMaster : public VisitedLinkCommon { bool posted_asynchronous_operation_; #endif - // Reference to the browser context that this object belongs to + // Reference to the user profile that this object belongs to // (it knows the path to where the data is stored) - content::BrowserContext* browser_context_; - - // Client owns the delegate and is responsible for it being valid through - // the life time this VisitedLinkMaster. - VisitedLinkDelegate* delegate_; + Profile* profile_; // VisitedLinkEventListener to handle incoming events. scoped_ptr<Listener> listener_; @@ -406,6 +380,10 @@ class VisitedLinkMaster : public VisitedLinkCommon { // When nonzero, overrides the table size for new databases for testing int32 table_size_override_; + // When non-NULL, overrides the history service to use rather than as the + // BrowserProcess. This is provided for unit tests. + HistoryService* history_service_override_; + // When set, indicates the task that should be run after the next rebuild from // history is complete. base::Closure rebuild_complete_task_; diff --git a/chrome/browser/visitedlink/visitedlink_master_factory.cc b/chrome/browser/visitedlink/visitedlink_master_factory.cc new file mode 100644 index 0000000..202fcd9 --- /dev/null +++ b/chrome/browser/visitedlink/visitedlink_master_factory.cc @@ -0,0 +1,45 @@ +// Copyright (c) 2012 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 "chrome/browser/visitedlink/visitedlink_master_factory.h" + +#include "base/memory/singleton.h" +#include "chrome/browser/profiles/profile_dependency_manager.h" +#include "chrome/browser/visitedlink/visitedlink_master.h" + +// static +VisitedLinkMaster* VisitedLinkMaster::FromProfile(Profile* profile) { + return VisitedLinkMasterFactory::GetForProfile(profile); +} + +// static +VisitedLinkMaster* VisitedLinkMasterFactory::GetForProfile(Profile* profile) { + return static_cast<VisitedLinkMaster*>( + GetInstance()->GetServiceForProfile(profile, true)); +} + +// static +VisitedLinkMasterFactory* VisitedLinkMasterFactory::GetInstance() { + return Singleton<VisitedLinkMasterFactory>::get(); +} + +VisitedLinkMasterFactory::VisitedLinkMasterFactory() + : ProfileKeyedServiceFactory( + "VisitedLinkMaster", ProfileDependencyManager::GetInstance()) { +} + +VisitedLinkMasterFactory::~VisitedLinkMasterFactory() { +} + +ProfileKeyedService* +VisitedLinkMasterFactory::BuildServiceInstanceFor(Profile* profile) const { + VisitedLinkMaster* visitedlink_master(new VisitedLinkMaster(profile)); + if (visitedlink_master->Init()) + return visitedlink_master; + return NULL; +} + +bool VisitedLinkMasterFactory::ServiceIsNULLWhileTesting() const { + return true; +} diff --git a/chrome/browser/visitedlink/visitedlink_master_factory.h b/chrome/browser/visitedlink/visitedlink_master_factory.h new file mode 100644 index 0000000..b1dea0b --- /dev/null +++ b/chrome/browser/visitedlink/visitedlink_master_factory.h @@ -0,0 +1,38 @@ +// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_BROWSER_VISITEDLINK_VISITEDLINK_MASTER_FACTORY_H_ +#define CHROME_BROWSER_VISITEDLINK_VISITEDLINK_MASTER_FACTORY_H_ + +#include "base/memory/singleton.h" +#include "chrome/browser/profiles/profile.h" +#include "chrome/browser/profiles/profile_keyed_service_factory.h" + +template <typename T> struct DefaultSingletonTraits; + +class VisitedLinkMaster; + +// Singleton that owns all VisitedLinkMaster and associates them with +// Profiles. +class VisitedLinkMasterFactory : public ProfileKeyedServiceFactory { + public: + static VisitedLinkMaster* GetForProfile(Profile* profile); + + static VisitedLinkMasterFactory* GetInstance(); + + private: + friend struct DefaultSingletonTraits<VisitedLinkMasterFactory>; + + VisitedLinkMasterFactory(); + virtual ~VisitedLinkMasterFactory(); + + // ProfileKeyedServiceFactory: + virtual ProfileKeyedService* BuildServiceInstanceFor( + Profile* profile) const OVERRIDE; + virtual bool ServiceIsNULLWhileTesting() const OVERRIDE; + + DISALLOW_COPY_AND_ASSIGN(VisitedLinkMasterFactory); +}; + +#endif // CHROME_BROWSER_VISITEDLINK_VISITEDLINK_MASTER_FACTORY_H_ diff --git a/chrome/browser/visitedlink/visitedlink_perftest.cc b/chrome/browser/visitedlink/visitedlink_perftest.cc index ccc414b..661a3fe 100644 --- a/chrome/browser/visitedlink/visitedlink_perftest.cc +++ b/chrome/browser/visitedlink/visitedlink_perftest.cc @@ -13,7 +13,6 @@ #include "base/stringprintf.h" #include "base/test/test_file_util.h" #include "chrome/browser/visitedlink/visitedlink_master.h" -#include "googleurl/src/gurl.h" #include "testing/gtest/include/gtest/gtest.h" using base::TimeDelta; diff --git a/chrome/browser/visitedlink/visitedlink_unittest.cc b/chrome/browser/visitedlink/visitedlink_unittest.cc index ef1762b..d13e988 100644 --- a/chrome/browser/visitedlink/visitedlink_unittest.cc +++ b/chrome/browser/visitedlink/visitedlink_unittest.cc @@ -13,9 +13,9 @@ #include "base/shared_memory.h" #include "base/string_util.h" #include "base/time.h" -#include "chrome/browser/visitedlink/visitedlink_delegate.h" #include "chrome/browser/visitedlink/visitedlink_event_listener.h" #include "chrome/browser/visitedlink/visitedlink_master.h" +#include "chrome/browser/visitedlink/visitedlink_master_factory.h" #include "chrome/common/render_messages.h" #include "chrome/renderer/visitedlink_slave.h" #include "chrome/test/base/chrome_render_view_host_test_harness.h" @@ -23,7 +23,6 @@ #include "content/public/browser/notification_service.h" #include "content/public/browser/notification_types.h" #include "content/public/test/mock_render_process_host.h" -#include "content/public/test/test_browser_context.h" #include "content/public/test/test_browser_thread.h" #include "content/public/test/test_renderer_host.h" #include "googleurl/src/gurl.h" @@ -35,8 +34,6 @@ using content::RenderViewHostTester; namespace { -typedef std::vector<GURL> URLs; - // a nice long URL that we can append numbers to to get new URLs const char g_test_prefix[] = "http://www.google.com/products/foo/index.html?id=45028640526508376&seq="; @@ -47,68 +44,13 @@ GURL TestURL(int i) { return GURL(StringPrintf("%s%d", g_test_prefix, i)); } -std::vector<VisitedLinkSlave*> g_slaves; - -// ========================== TestVisitedLinkDelegate ========================== -class TestVisitedLinkDelegate : public VisitedLinkDelegate { - public: - virtual bool AreEquivalentContexts( - content::BrowserContext* context1, - content::BrowserContext* context2) OVERRIDE; - virtual void RebuildTable(URLEnumerator* enumerator) OVERRIDE; - - void AddURLForRebuild(const GURL& url); - - private: - - URLs rebuild_urls_; -}; - -bool TestVisitedLinkDelegate::AreEquivalentContexts( - content::BrowserContext* context1, content::BrowserContext* context2) { - DCHECK_EQ(context1, context2); - return true; // Test only has one profile. -} - -void TestVisitedLinkDelegate::RebuildTable(URLEnumerator* enumerator) { - for (URLs::const_iterator itr = rebuild_urls_.begin(); - itr != rebuild_urls_.end(); - ++itr) - enumerator->OnURL(*itr); - enumerator->OnComplete(true); -} - -void TestVisitedLinkDelegate::AddURLForRebuild(const GURL& url) { - rebuild_urls_.push_back(url); -} -// ======================== TestVisitedLinkDelegate End ======================== - -// ============================== TestURLIterator ============================== -class TestURLIterator : public VisitedLinkMaster::URLIterator { - public: - explicit TestURLIterator(const URLs& urls); - - virtual const GURL& NextURL() OVERRIDE; - virtual bool HasNextURL() const OVERRIDE; - - private: - URLs::const_iterator iterator_; - URLs::const_iterator end_; -}; - -TestURLIterator::TestURLIterator(const URLs& urls) - : iterator_(urls.begin()), - end_(urls.end()) { -} - -const GURL& TestURLIterator::NextURL() { - return *(iterator_++); +ProfileKeyedService* BuildVisitedLinkMaster(Profile* profile) { + VisitedLinkMaster* master = new VisitedLinkMaster(profile); + master->Init(); + return master; } -bool TestURLIterator::HasNextURL() const { - return iterator_ != end_; -} -// ============================ TestURLIterator End ============================ +std::vector<VisitedLinkSlave*> g_slaves; } // namespace @@ -149,6 +91,12 @@ class VisitedLinkTest : public testing::Test { VisitedLinkTest() : ui_thread_(BrowserThread::UI, &message_loop_), file_thread_(BrowserThread::FILE, &message_loop_) {} + // Initialize the history system. This should be called before InitVisited(). + bool InitHistory() { + history_service_.reset(new HistoryService); + return history_service_->Init(history_dir_, NULL); + } + // Initializes the visited link objects. Pass in the size that you want a // freshly created table to be. 0 means use the default. // @@ -157,7 +105,7 @@ class VisitedLinkTest : public testing::Test { bool InitVisited(int initial_size, bool suppress_rebuild) { // Initialize the visited link system. master_.reset(new VisitedLinkMaster(new TrackingVisitedLinkEventListener(), - &delegate_, + history_service_.get(), suppress_rebuild, visited_file_, initial_size)); return master_->Init(); @@ -169,6 +117,18 @@ class VisitedLinkTest : public testing::Test { if (master_.get()) master_.reset(NULL); + if (history_service_.get()) { + history_service_->SetOnBackendDestroyTask(MessageLoop::QuitClosure()); + history_service_->Cleanup(); + history_service_.reset(); + + // Wait for the backend class to terminate before deleting the files and + // moving to the next test. Note: if this never terminates, somebody is + // probably leaking a reference to the history backend, so it never calls + // our destroy task. + MessageLoop::current()->Run(); + } + // Wait for all pending file I/O to be completed. BrowserThread::GetBlockingPool()->FlushForTesting(); } @@ -180,6 +140,7 @@ class VisitedLinkTest : public testing::Test { // Clean up after our caller, who may have left the database open. ClearDB(); + ASSERT_TRUE(InitHistory()); ASSERT_TRUE(InitVisited(0, true)); master_->DebugValidate(); @@ -241,12 +202,13 @@ class VisitedLinkTest : public testing::Test { FilePath visited_file_; scoped_ptr<VisitedLinkMaster> master_; - TestVisitedLinkDelegate delegate_; + scoped_ptr<HistoryService> history_service_; }; // This test creates and reads some databases to make sure the data is // preserved throughout those operations. TEST_F(VisitedLinkTest, DatabaseIO) { + ASSERT_TRUE(InitHistory()); ASSERT_TRUE(InitVisited(0, true)); for (int i = 0; i < g_test_count; i++) @@ -259,6 +221,7 @@ TEST_F(VisitedLinkTest, DatabaseIO) { // Checks that we can delete things properly when there are collisions. TEST_F(VisitedLinkTest, Delete) { static const int32 kInitialSize = 17; + ASSERT_TRUE(InitHistory()); ASSERT_TRUE(InitVisited(kInitialSize, true)); // Add a cluster from 14-17 wrapping around to 0. These will all hash to the @@ -297,6 +260,7 @@ TEST_F(VisitedLinkTest, Delete) { // When we delete more than kBigDeleteThreshold we trigger different behavior // where the entire file is rewritten. TEST_F(VisitedLinkTest, BigDelete) { + ASSERT_TRUE(InitHistory()); ASSERT_TRUE(InitVisited(16381, true)); // Add the base set of URLs that won't be deleted. @@ -306,21 +270,21 @@ TEST_F(VisitedLinkTest, BigDelete) { // Add more URLs than necessary to trigger this case. const int kTestDeleteCount = VisitedLinkMaster::kBigDeleteThreshold + 2; - URLs urls_to_delete; + history::URLRows urls_to_delete; for (int32 i = g_test_count; i < g_test_count + kTestDeleteCount; i++) { GURL url(TestURL(i)); master_->AddURL(url); - urls_to_delete.push_back(url); + urls_to_delete.push_back(history::URLRow(url)); } - TestURLIterator iterator(urls_to_delete); - master_->DeleteURLs(&iterator); + master_->DeleteURLs(urls_to_delete); master_->DebugValidate(); Reload(); } TEST_F(VisitedLinkTest, DeleteAll) { + ASSERT_TRUE(InitHistory()); ASSERT_TRUE(InitVisited(0, true)); { @@ -356,6 +320,7 @@ TEST_F(VisitedLinkTest, DeleteAll) { } // Reopen and validate. + ASSERT_TRUE(InitHistory()); ASSERT_TRUE(InitVisited(0, true)); master_->DebugValidate(); EXPECT_EQ(0, master_->GetUsedCount()); @@ -368,6 +333,7 @@ TEST_F(VisitedLinkTest, DeleteAll) { TEST_F(VisitedLinkTest, Resizing) { // Create a very small database. const int32 initial_size = 17; + ASSERT_TRUE(InitHistory()); ASSERT_TRUE(InitVisited(initial_size, true)); // ...and a slave @@ -416,11 +382,14 @@ TEST_F(VisitedLinkTest, Resizing) { // Tests that if the database doesn't exist, it will be rebuilt from history. TEST_F(VisitedLinkTest, Rebuild) { + ASSERT_TRUE(InitHistory()); + // Add half of our URLs to history. This needs to be done before we // initialize the visited link DB. int history_count = g_test_count / 2; for (int i = 0; i < history_count; i++) - delegate_.AddURLForRebuild(TestURL(i)); + history_service_->AddPage( + TestURL(i), base::Time::Now(), history::SOURCE_BROWSED); // Initialize the visited link DB. Since the visited links file doesn't exist // and we don't suppress history rebuilding, this will load from history. @@ -438,10 +407,9 @@ TEST_F(VisitedLinkTest, Rebuild) { // Add one more and then delete it. master_->AddURL(TestURL(g_test_count)); - URLs urls_to_delete; - urls_to_delete.push_back(TestURL(g_test_count)); - TestURLIterator iterator(urls_to_delete); - master_->DeleteURLs(&iterator); + history::URLRows deleted_urls; + deleted_urls.push_back(history::URLRow(TestURL(g_test_count))); + master_->DeleteURLs(deleted_urls); // Wait for the rebuild to complete. The task will terminate the message // loop when the rebuild is done. There's no chance that the rebuild will @@ -460,6 +428,7 @@ TEST_F(VisitedLinkTest, Rebuild) { // Test that importing a large number of URLs will work TEST_F(VisitedLinkTest, BigImport) { + ASSERT_TRUE(InitHistory()); ASSERT_TRUE(InitVisited(0, false)); // Before the table rebuilds, add a large number of URLs @@ -477,6 +446,7 @@ TEST_F(VisitedLinkTest, BigImport) { } TEST_F(VisitedLinkTest, Listener) { + ASSERT_TRUE(InitHistory()); ASSERT_TRUE(InitVisited(0, true)); // Add test URLs. @@ -485,12 +455,10 @@ TEST_F(VisitedLinkTest, Listener) { ASSERT_EQ(i + 1, master_->GetUsedCount()); } + history::URLRows deleted_urls; + deleted_urls.push_back(history::URLRow(TestURL(0))); // Delete an URL. - URLs urls_to_delete; - urls_to_delete.push_back(TestURL(0)); - TestURLIterator iterator(urls_to_delete); - master_->DeleteURLs(&iterator); - + master_->DeleteURLs(deleted_urls); // ... and all of the remaining ones. master_->DeleteAllURLs(); @@ -504,7 +472,6 @@ TEST_F(VisitedLinkTest, Listener) { EXPECT_EQ(2, listener->reset_count()); } -// TODO(boliu): Inherit content::TestBrowserContext when componentized. class VisitCountingProfile : public TestingProfile { public: VisitCountingProfile() @@ -556,7 +523,7 @@ class VisitRelayingRenderProcessHost : public MockRenderProcessHost { virtual bool Send(IPC::Message* msg) OVERRIDE { VisitCountingProfile* counting_profile = static_cast<VisitCountingProfile*>( - GetBrowserContext()); + Profile::FromBrowserContext(GetBrowserContext())); if (msg->type() == ChromeViewMsg_VisitedLink_Add::ID) { PickleIterator iter(*msg); @@ -592,7 +559,6 @@ class VisitedLinkRenderProcessHostFactory DISALLOW_COPY_AND_ASSIGN(VisitedLinkRenderProcessHostFactory); }; -// TODO(boliu): Inherit content::RenderViewHostTestHarness when componentized. class VisitedLinkEventsTest : public ChromeRenderViewHostTestHarness { public: VisitedLinkEventsTest() @@ -601,10 +567,12 @@ class VisitedLinkEventsTest : public ChromeRenderViewHostTestHarness { virtual ~VisitedLinkEventsTest() {} virtual void SetUp() { browser_context_.reset(new VisitCountingProfile()); - master_.reset(new VisitedLinkMaster(profile(), &delegate_)); - master_->Init(); + profile()->CreateHistoryService(true, false); + master_ = static_cast<VisitedLinkMaster*>( + VisitedLinkMasterFactory::GetInstance()-> + SetTestingFactoryAndUse(profile(), BuildVisitedLinkMaster)); SetRenderProcessHostFactory(&vc_rph_factory_); - content::RenderViewHostTestHarness::SetUp(); + ChromeRenderViewHostTestHarness::SetUp(); } VisitCountingProfile* profile() const { @@ -612,7 +580,7 @@ class VisitedLinkEventsTest : public ChromeRenderViewHostTestHarness { } VisitedLinkMaster* master() const { - return master_.get(); + return master_; } void WaitForCoalescense() { @@ -628,8 +596,7 @@ class VisitedLinkEventsTest : public ChromeRenderViewHostTestHarness { VisitedLinkRenderProcessHostFactory vc_rph_factory_; private: - TestVisitedLinkDelegate delegate_; - scoped_ptr<VisitedLinkMaster> master_; + VisitedLinkMaster* master_; content::TestBrowserThread ui_thread_; content::TestBrowserThread file_thread_; @@ -689,7 +656,7 @@ TEST_F(VisitedLinkEventsTest, Coalescense) { } TEST_F(VisitedLinkEventsTest, Basics) { - RenderViewHostTester::For(rvh())->CreateRenderView(string16(), + rvh_tester()->CreateRenderView(string16(), MSG_ROUTING_NONE, -1); @@ -714,12 +681,12 @@ TEST_F(VisitedLinkEventsTest, Basics) { } TEST_F(VisitedLinkEventsTest, TabVisibility) { - RenderViewHostTester::For(rvh())->CreateRenderView(string16(), + rvh_tester()->CreateRenderView(string16(), MSG_ROUTING_NONE, -1); // Simulate tab becoming inactive. - RenderViewHostTester::For(rvh())->SimulateWasHidden(); + rvh_tester()->SimulateWasHidden(); // Add a few URLs. master()->AddURL(GURL("http://acidtests.org/")); @@ -733,14 +700,14 @@ TEST_F(VisitedLinkEventsTest, TabVisibility) { EXPECT_EQ(0, profile()->reset_event_count()); // Simulate the tab becoming active. - RenderViewHostTester::For(rvh())->SimulateWasShown(); + rvh_tester()->SimulateWasShown(); // We should now have 3 add events, still no reset events. EXPECT_EQ(1, profile()->add_event_count()); EXPECT_EQ(0, profile()->reset_event_count()); // Deactivate the tab again. - RenderViewHostTester::For(rvh())->SimulateWasHidden(); + rvh_tester()->SimulateWasHidden(); // Add a bunch of URLs (over 50) to exhaust the link event buffer. for (int i = 0; i < 100; i++) @@ -753,7 +720,7 @@ TEST_F(VisitedLinkEventsTest, TabVisibility) { EXPECT_EQ(0, profile()->reset_event_count()); // Activate the tab. - RenderViewHostTester::For(rvh())->SimulateWasShown(); + rvh_tester()->SimulateWasShown(); // We should have only one more reset event. EXPECT_EQ(1, profile()->add_event_count()); diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index 77300cc..4d0e897 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -2166,11 +2166,12 @@ 'browser/value_store/value_store.h', 'browser/view_type_utils.cc', 'browser/view_type_utils.h', - 'browser/visitedlink/visitedlink_delegate.h', 'browser/visitedlink/visitedlink_event_listener.cc', 'browser/visitedlink/visitedlink_event_listener.h', 'browser/visitedlink/visitedlink_master.cc', 'browser/visitedlink/visitedlink_master.h', + 'browser/visitedlink/visitedlink_master_factory.cc', + 'browser/visitedlink/visitedlink_master_factory.h', 'browser/web_applications/web_app.cc', 'browser/web_applications/web_app.h', 'browser/web_applications/web_app_android.cc', |