diff options
| author | rlp@chromium.org <rlp@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-06-08 21:54:21 +0000 |
|---|---|---|
| committer | rlp@chromium.org <rlp@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-06-08 21:54:21 +0000 |
| commit | f61f4781f80387b0fe2257c035cf95c61eca555a (patch) | |
| tree | dc8274275bc8603491db7df9f2e85dc211a5881f | |
| parent | 876b799d1a13019e2cef872a15b980c3b2e0964a (diff) | |
| download | chromium_src-f61f4781f80387b0fe2257c035cf95c61eca555a.zip chromium_src-f61f4781f80387b0fe2257c035cf95c61eca555a.tar.gz chromium_src-f61f4781f80387b0fe2257c035cf95c61eca555a.tar.bz2 | |
Converting BookmarkModel and HistoryService to ProfileKeyedServices. This just performs the initial conversion. Separate CLs to take care of the removal of profile_->Get<Serivce> will follow.
BUG=97804,112525
TEST=no new, passes existing unittests
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=138867
Review URL: https://chromiumcodereview.appspot.com/10399087
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@141294 0039d316-1c4b-4281-b951-d872f2087c98
| -rw-r--r-- | chrome/browser/bookmarks/bookmark_expanded_state_tracker.cc | 12 | ||||
| -rw-r--r-- | chrome/browser/bookmarks/bookmark_expanded_state_tracker.h | 6 | ||||
| -rw-r--r-- | chrome/browser/bookmarks/bookmark_model.cc | 17 | ||||
| -rw-r--r-- | chrome/browser/bookmarks/bookmark_model.h | 12 | ||||
| -rw-r--r-- | chrome/browser/bookmarks/bookmark_model_factory.cc | 58 | ||||
| -rw-r--r-- | chrome/browser/bookmarks/bookmark_model_factory.h | 39 | ||||
| -rw-r--r-- | chrome/browser/history/history.cc | 19 | ||||
| -rw-r--r-- | chrome/browser/history/history.h | 6 | ||||
| -rw-r--r-- | chrome/browser/history/history_service_factory.cc | 70 | ||||
| -rw-r--r-- | chrome/browser/history/history_service_factory.h | 47 | ||||
| -rw-r--r-- | chrome/browser/history/in_memory_url_index.cc | 7 | ||||
| -rw-r--r-- | chrome/browser/prefs/browser_prefs.cc | 2 | ||||
| -rw-r--r-- | chrome/browser/profiles/profile_dependency_manager.cc | 4 | ||||
| -rw-r--r-- | chrome/browser/profiles/profile_impl.cc | 49 | ||||
| -rw-r--r-- | chrome/browser/profiles/profile_impl.h | 3 | ||||
| -rw-r--r-- | chrome/chrome_browser.gypi | 4 | ||||
| -rw-r--r-- | chrome/test/base/testing_profile.cc | 84 | ||||
| -rw-r--r-- | chrome/test/base/testing_profile.h | 8 |
18 files changed, 334 insertions, 113 deletions
diff --git a/chrome/browser/bookmarks/bookmark_expanded_state_tracker.cc b/chrome/browser/bookmarks/bookmark_expanded_state_tracker.cc index 1b5d870..f23aca1 100644 --- a/chrome/browser/bookmarks/bookmark_expanded_state_tracker.cc +++ b/chrome/browser/bookmarks/bookmark_expanded_state_tracker.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// 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. @@ -10,15 +10,16 @@ #include "chrome/browser/prefs/pref_service.h" #include "chrome/browser/profiles/profile.h" -BookmarkExpandedStateTracker::BookmarkExpandedStateTracker(Profile* profile, - const char* path) +BookmarkExpandedStateTracker::BookmarkExpandedStateTracker( + Profile* profile, + const char* path, + BookmarkModel* bookmark_model) : profile_(profile), pref_path_(path) { - profile_->GetBookmarkModel()->AddObserver(this); + bookmark_model->AddObserver(this); } BookmarkExpandedStateTracker::~BookmarkExpandedStateTracker() { - profile_->GetBookmarkModel()->RemoveObserver(this); } void BookmarkExpandedStateTracker::SetExpandedNodes(const Nodes& nodes) { @@ -71,6 +72,7 @@ void BookmarkExpandedStateTracker::BookmarkModelChanged() { void BookmarkExpandedStateTracker::BookmarkModelBeingDeleted( BookmarkModel* model) { + model->RemoveObserver(this); } void BookmarkExpandedStateTracker::BookmarkNodeRemoved( diff --git a/chrome/browser/bookmarks/bookmark_expanded_state_tracker.h b/chrome/browser/bookmarks/bookmark_expanded_state_tracker.h index 0ed9bbb..2edf720 100644 --- a/chrome/browser/bookmarks/bookmark_expanded_state_tracker.h +++ b/chrome/browser/bookmarks/bookmark_expanded_state_tracker.h @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// 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. @@ -20,7 +20,9 @@ class BookmarkExpandedStateTracker : public BaseBookmarkModelObserver { public: typedef std::set<const BookmarkNode*> Nodes; - BookmarkExpandedStateTracker(Profile* profile, const char* path); + BookmarkExpandedStateTracker(Profile* profile, + const char* path, + BookmarkModel* bookmark_model); virtual ~BookmarkExpandedStateTracker(); // The set of expanded nodes. diff --git a/chrome/browser/bookmarks/bookmark_model.cc b/chrome/browser/bookmarks/bookmark_model.cc index fcb608c..ac38bd4 100644 --- a/chrome/browser/bookmarks/bookmark_model.cc +++ b/chrome/browser/bookmarks/bookmark_model.cc @@ -152,21 +152,12 @@ BookmarkModel::~BookmarkModel() { } } -// static -void BookmarkModel::RegisterUserPrefs(PrefService* prefs) { - // Don't sync this, as otherwise, due to a limitation in sync, it - // will cause a deadlock (see http://crbug.com/97955). If we truly - // want to sync the expanded state of folders, it should be part of - // bookmark sync itself (i.e., a property of the sync folder nodes). - prefs->RegisterListPref(prefs::kBookmarkEditorExpandedNodes, new ListValue, - PrefService::UNSYNCABLE_PREF); -} - -void BookmarkModel::Cleanup() { +void BookmarkModel::Shutdown() { if (loaded_) return; - // See comment in Profile shutdown code where this is invoked for details. + // See comment in HistoryService::ShutdownOnUIThread where this is invoked for + // details. It is also called when the BookmarkModel is deleted. loaded_signal_.Signal(); } @@ -179,7 +170,7 @@ void BookmarkModel::Load() { } expanded_state_tracker_.reset(new BookmarkExpandedStateTracker( - profile_, prefs::kBookmarkEditorExpandedNodes)); + profile_, prefs::kBookmarkEditorExpandedNodes, this)); // Listen for changes to favicons so that we can update the favicon of the // node appropriately. diff --git a/chrome/browser/bookmarks/bookmark_model.h b/chrome/browser/bookmarks/bookmark_model.h index 91cd1c8..7ec957c 100644 --- a/chrome/browser/bookmarks/bookmark_model.h +++ b/chrome/browser/bookmarks/bookmark_model.h @@ -21,6 +21,7 @@ #include "chrome/browser/cancelable_request.h" #include "chrome/browser/favicon/favicon_service.h" #include "chrome/browser/history/history.h" +#include "chrome/browser/profiles/profile_keyed_service.h" #include "content/public/browser/notification_registrar.h" #include "googleurl/src/gurl.h" #include "third_party/skia/include/core/SkBitmap.h" @@ -192,19 +193,18 @@ class BookmarkPermanentNode : public BookmarkNode { // An observer may be attached to observe relevant events. // // You should NOT directly create a BookmarkModel, instead go through the -// Profile. +// BookmarkModelFactory. class BookmarkModel : public content::NotificationObserver, - public BookmarkService { + public BookmarkService, + public ProfileKeyedService { public: explicit BookmarkModel(Profile* profile); virtual ~BookmarkModel(); - static void RegisterUserPrefs(PrefService* prefs); - // Invoked prior to destruction to release any necessary resources. - void Cleanup(); + virtual void Shutdown() OVERRIDE; - // Loads the bookmarks. This is called by Profile upon creation of the + // Loads the bookmarks. This is called upon creation of the // BookmarkModel. You need not invoke this directly. void Load(); diff --git a/chrome/browser/bookmarks/bookmark_model_factory.cc b/chrome/browser/bookmarks/bookmark_model_factory.cc new file mode 100644 index 0000000..536eb70 --- /dev/null +++ b/chrome/browser/bookmarks/bookmark_model_factory.cc @@ -0,0 +1,58 @@ +// 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/bookmarks/bookmark_model_factory.h" + +#include "chrome/browser/bookmarks/bookmark_model.h" +#include "chrome/browser/history/history_service_factory.h" +#include "chrome/browser/prefs/pref_service.h" +#include "chrome/browser/profiles/profile_dependency_manager.h" +#include "chrome/common/pref_names.h" + +// static +BookmarkModel* BookmarkModelFactory::GetForProfile(Profile* profile) { + return static_cast<BookmarkModel*>( + GetInstance()->GetServiceForProfile(profile, true)); +} + +BookmarkModel* BookmarkModelFactory::GetForProfileIfExists(Profile* profile) { + return static_cast<BookmarkModel*>( + GetInstance()->GetServiceForProfile(profile, false)); +} + +// static +BookmarkModelFactory* BookmarkModelFactory::GetInstance() { + return Singleton<BookmarkModelFactory>::get(); +} + +BookmarkModelFactory::BookmarkModelFactory() + : ProfileKeyedServiceFactory("BookmarkModelFactory", + ProfileDependencyManager::GetInstance()) { +} + +BookmarkModelFactory::~BookmarkModelFactory() {} + +ProfileKeyedService* BookmarkModelFactory::BuildServiceInstanceFor( + Profile* profile) const { + BookmarkModel* bookmark_model = new BookmarkModel(profile); + bookmark_model->Load(); + return bookmark_model; +} + +void BookmarkModelFactory::RegisterUserPrefs(PrefService* prefs) { + // Don't sync this, as otherwise, due to a limitation in sync, it + // will cause a deadlock (see http://crbug.com/97955). If we truly + // want to sync the expanded state of folders, it should be part of + // bookmark sync itself (i.e., a property of the sync folder nodes). + prefs->RegisterListPref(prefs::kBookmarkEditorExpandedNodes, new ListValue, + PrefService::UNSYNCABLE_PREF); +} + +bool BookmarkModelFactory::ServiceRedirectedInIncognito() { + return true; +} + +bool BookmarkModelFactory::ServiceIsNULLWhileTesting() { + return true; +} diff --git a/chrome/browser/bookmarks/bookmark_model_factory.h b/chrome/browser/bookmarks/bookmark_model_factory.h new file mode 100644 index 0000000..7312788 --- /dev/null +++ b/chrome/browser/bookmarks/bookmark_model_factory.h @@ -0,0 +1,39 @@ +// 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_BOOKMARKS_BOOKMARK_MODEL_FACTORY_H_ +#define CHROME_BROWSER_BOOKMARKS_BOOKMARK_MODEL_FACTORY_H_ +#pragma once + +#include "base/memory/singleton.h" +#include "chrome/browser/profiles/profile_keyed_service_factory.h" + +class Profile; +class BookmarkModel; + +// Singleton that owns all BookmarkModel and associates them with +// Profiles. +class BookmarkModelFactory : public ProfileKeyedServiceFactory { + public: + static BookmarkModel* GetForProfile(Profile* profile); + + static BookmarkModel* GetForProfileIfExists(Profile* profile); + + static BookmarkModelFactory* GetInstance(); + + private: + friend struct DefaultSingletonTraits<BookmarkModelFactory>; + + BookmarkModelFactory(); + virtual ~BookmarkModelFactory(); + + // ProfileKeyedServiceFactory: + virtual ProfileKeyedService* BuildServiceInstanceFor( + Profile* profile) const OVERRIDE; + virtual void RegisterUserPrefs(PrefService* user_prefs) OVERRIDE; + virtual bool ServiceRedirectedInIncognito() OVERRIDE; + virtual bool ServiceIsNULLWhileTesting() OVERRIDE; +}; + +#endif // CHROME_BROWSER_BOOKMARKS_BOOKMARK_MODEL_FACTORY_H_ diff --git a/chrome/browser/history/history.cc b/chrome/browser/history/history.cc index b765197..58b1d6a 100644 --- a/chrome/browser/history/history.cc +++ b/chrome/browser/history/history.cc @@ -32,6 +32,8 @@ #include "base/string_util.h" #include "base/threading/thread.h" #include "chrome/browser/autocomplete/history_url_provider.h" +#include "chrome/browser/bookmarks/bookmark_model.h" +#include "chrome/browser/bookmarks/bookmark_model_factory.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/history/history_backend.h" #include "chrome/browser/history/history_notifications.h" @@ -263,6 +265,23 @@ history::URLDatabase* HistoryService::InMemoryDatabase() { return NULL; } +void HistoryService::ShutdownOnUIThread() { + // It's possible that bookmarks haven't loaded and history is waiting for + // bookmarks to complete loading. In such a situation history can't shutdown + // (meaning if we invoked history_service_->Cleanup now, we would + // deadlock). To break the deadlock we tell BookmarkModel it's about to be + // deleted so that it can release the signal history is waiting on, allowing + // history to shutdown (history_service_->Cleanup to complete). In such a + // scenario history sees an incorrect view of bookmarks, but it's better + // than a deadlock. + BookmarkModel* bookmark_model = static_cast<BookmarkModel*>( + BookmarkModelFactory::GetForProfileIfExists(profile_)); + if (bookmark_model) + bookmark_model->Shutdown(); + + Cleanup(); +} + void HistoryService::SetSegmentPresentationIndex(int64 segment_id, int index) { ScheduleAndForget(PRIORITY_UI, &HistoryBackend::SetSegmentPresentationIndex, diff --git a/chrome/browser/history/history.h b/chrome/browser/history/history.h index dcdf1a5..adfa4be 100644 --- a/chrome/browser/history/history.h +++ b/chrome/browser/history/history.h @@ -19,6 +19,7 @@ #include "chrome/browser/cancelable_request.h" #include "chrome/browser/favicon/favicon_service.h" #include "chrome/browser/history/history_types.h" +#include "chrome/browser/profiles/refcounted_profile_keyed_service.h" #include "chrome/browser/search_engines/template_url_id.h" #include "chrome/common/ref_counted_util.h" #include "content/public/browser/notification_observer.h" @@ -96,7 +97,7 @@ class HistoryDBTask : public base::RefCountedThreadSafe<HistoryDBTask> { // thread that made the request. class HistoryService : public CancelableRequestProvider, public content::NotificationObserver, - public base::RefCountedThreadSafe<HistoryService> { + public RefcountedProfileKeyedService { public: // Miscellaneous commonly-used types. typedef std::vector<PageUsageData*> PageUsageDataList; @@ -159,6 +160,9 @@ class HistoryService : public CancelableRequestProvider, return in_memory_url_index_.get(); } + // RefcountedProfileKeyedService: + virtual void ShutdownOnUIThread() OVERRIDE; + // Navigation ---------------------------------------------------------------- // Adds the given canonical URL to history with the current time as the visit diff --git a/chrome/browser/history/history_service_factory.cc b/chrome/browser/history/history_service_factory.cc new file mode 100644 index 0000000..ca809b0 --- /dev/null +++ b/chrome/browser/history/history_service_factory.cc @@ -0,0 +1,70 @@ +// 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/history/history_service_factory.h" + +#include "chrome/browser/bookmarks/bookmark_model.h" +#include "chrome/browser/bookmarks/bookmark_model_factory.h" +#include "chrome/browser/history/history.h" +#include "chrome/browser/prefs/pref_service.h" +#include "chrome/browser/profiles/profile_dependency_manager.h" +#include "chrome/common/pref_names.h" + +// static +scoped_refptr<HistoryService> HistoryServiceFactory::GetForProfile( + Profile* profile, Profile::ServiceAccessType sat) { + // If saving history is disabled, only allow explicit access. + if (profile->GetPrefs()->GetBoolean(prefs::kSavingBrowserHistoryDisabled) && + sat != Profile::EXPLICIT_ACCESS) + return NULL; + + return static_cast<HistoryService*>( + GetInstance()->GetServiceForProfile(profile, true).get()); +} + +// static +scoped_refptr<HistoryService> +HistoryServiceFactory::GetForProfileIfExists(Profile* profile) { + return static_cast<HistoryService*>( + GetInstance()->GetServiceForProfile(profile, false).get()); +} + +// static +HistoryServiceFactory* HistoryServiceFactory::GetInstance() { + return Singleton<HistoryServiceFactory>::get(); +} + +// static +void HistoryServiceFactory::ShutdownForProfile(Profile* profile) { + HistoryServiceFactory* factory = GetInstance(); + factory->ProfileDestroyed(profile); +} + +HistoryServiceFactory::HistoryServiceFactory() + : RefcountedProfileKeyedServiceFactory( + "HistoryService", ProfileDependencyManager::GetInstance()) { + DependsOn(BookmarkModelFactory::GetInstance()); +} + +HistoryServiceFactory::~HistoryServiceFactory() { +} + +scoped_refptr<RefcountedProfileKeyedService> +HistoryServiceFactory::BuildServiceInstanceFor(Profile* profile) const { + scoped_refptr<HistoryService> history_service( + new HistoryService(profile)); + if (!history_service->Init(profile->GetPath(), + BookmarkModelFactory::GetForProfile(profile))) { + return NULL; + } + return history_service; +} + +bool HistoryServiceFactory::ServiceRedirectedInIncognito() { + return true; +} + +bool HistoryServiceFactory::ServiceIsNULLWhileTesting() { + return true; +} diff --git a/chrome/browser/history/history_service_factory.h b/chrome/browser/history/history_service_factory.h new file mode 100644 index 0000000..cc82de8 --- /dev/null +++ b/chrome/browser/history/history_service_factory.h @@ -0,0 +1,47 @@ +// 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_HISTORY_HISTORY_SERVICE_FACTORY_H_ +#define CHROME_BROWSER_HISTORY_HISTORY_SERVICE_FACTORY_H_ +#pragma once + +#include "base/memory/singleton.h" +#include "chrome/browser/profiles/profile.h" +#include "chrome/browser/profiles/refcounted_profile_keyed_service_factory.h" + +class HistoryService; + +// Singleton that owns all HistoryService and associates them with +// Profiles. +class HistoryServiceFactory + : public RefcountedProfileKeyedServiceFactory { + public: + static scoped_refptr<HistoryService> GetForProfile( + Profile* profile, Profile::ServiceAccessType sat); + + static scoped_refptr<HistoryService> GetForProfileIfExists( + Profile* profile); + + static HistoryServiceFactory* GetInstance(); + + // In the testing profile, we often clear the history before making a new + // one. This takes care of that work. It should only be used in tests. + // Note: This does not do any cleanup; it only destroys the service. The + // calling test is expected to do the cleanup before calling this function. + static void ShutdownForProfile(Profile* profile); + + private: + friend struct DefaultSingletonTraits<HistoryServiceFactory>; + + HistoryServiceFactory(); + virtual ~HistoryServiceFactory(); + + // ProfileKeyedServiceFactory: + virtual scoped_refptr<RefcountedProfileKeyedService> BuildServiceInstanceFor( + Profile* profile) const OVERRIDE; + virtual bool ServiceRedirectedInIncognito() OVERRIDE; + virtual bool ServiceIsNULLWhileTesting() OVERRIDE; +}; + +#endif // CHROME_BROWSER_HISTORY_HISTORY_SERVICE_FACTORY_H_ diff --git a/chrome/browser/history/in_memory_url_index.cc b/chrome/browser/history/in_memory_url_index.cc index cfd0463..b39c977 100644 --- a/chrome/browser/history/in_memory_url_index.cc +++ b/chrome/browser/history/in_memory_url_index.cc @@ -134,7 +134,12 @@ void InMemoryURLIndex::ShutDown() { registrar_.RemoveAll(); cache_reader_consumer_.CancelAllRequests(); shutdown_ = true; - PostSaveToCacheFileTask(); + FilePath path; + if (!GetCacheFilePath(&path)) + return; + scoped_refptr<RefCountedBool> succeeded(new RefCountedBool(false)); + URLIndexPrivateData::WritePrivateDataToCacheFileTask( + private_data_, path, succeeded); needs_to_be_cached_ = false; } diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc index 0e2cef5..d896951 100644 --- a/chrome/browser/prefs/browser_prefs.cc +++ b/chrome/browser/prefs/browser_prefs.cc @@ -8,7 +8,6 @@ #include "chrome/browser/accessibility/invert_bubble_prefs.h" #include "chrome/browser/autofill/autofill_manager.h" #include "chrome/browser/background/background_mode_manager.h" -#include "chrome/browser/bookmarks/bookmark_model.h" #include "chrome/browser/bookmarks/bookmark_utils.h" #include "chrome/browser/browser_shutdown.h" #include "chrome/browser/content_settings/host_content_settings_map.h" @@ -181,7 +180,6 @@ void RegisterUserPrefs(PrefService* user_prefs) { AppsPromo::RegisterUserPrefs(user_prefs); AutofillManager::RegisterUserPrefs(user_prefs); bookmark_utils::RegisterUserPrefs(user_prefs); - BookmarkModel::RegisterUserPrefs(user_prefs); ChromeVersionService::RegisterUserPrefs(user_prefs); chrome_browser_net::HttpServerPropertiesManager::RegisterPrefs(user_prefs); chrome_browser_net::Predictor::RegisterUserPrefs(user_prefs); diff --git a/chrome/browser/profiles/profile_dependency_manager.cc b/chrome/browser/profiles/profile_dependency_manager.cc index 25d5fc9..9230a22 100644 --- a/chrome/browser/profiles/profile_dependency_manager.cc +++ b/chrome/browser/profiles/profile_dependency_manager.cc @@ -10,6 +10,7 @@ #include "chrome/browser/autofill/personal_data_manager_factory.h" #include "chrome/browser/background/background_contents_service_factory.h" +#include "chrome/browser/bookmarks/bookmark_model_factory.h" #include "chrome/browser/captive_portal/captive_portal_service_factory.h" #include "chrome/browser/content_settings/cookie_settings.h" #include "chrome/browser/download/download_service_factory.h" @@ -17,6 +18,7 @@ #include "chrome/browser/extensions/api/discovery/suggested_links_registry_factory.h" #include "chrome/browser/extensions/extension_system_factory.h" #include "chrome/browser/google/google_url_tracker_factory.h" +#include "chrome/browser/history/history_service_factory.h" #include "chrome/browser/intents/web_intents_registry_factory.h" #include "chrome/browser/notifications/desktop_notification_service_factory.h" #include "chrome/browser/password_manager/password_store_factory.h" @@ -181,6 +183,7 @@ void ProfileDependencyManager::AssertFactoriesBuilt() { #if defined(ENABLE_BACKGROUND) BackgroundContentsServiceFactory::GetInstance(); #endif + BookmarkModelFactory::GetInstance(); #if !defined(OS_ANDROID) captive_portal::CaptivePortalServiceFactory::GetInstance(); #endif @@ -202,6 +205,7 @@ void ProfileDependencyManager::AssertFactoriesBuilt() { #endif GlobalErrorServiceFactory::GetInstance(); GoogleURLTrackerFactory::GetInstance(); + HistoryServiceFactory::GetInstance(); NTPResourceCacheFactory::GetInstance(); PasswordStoreFactory::GetInstance(); PersonalDataManagerFactory::GetInstance(); diff --git a/chrome/browser/profiles/profile_impl.cc b/chrome/browser/profiles/profile_impl.cc index 63992d3..cd8fe00 100644 --- a/chrome/browser/profiles/profile_impl.cc +++ b/chrome/browser/profiles/profile_impl.cc @@ -20,7 +20,7 @@ #include "chrome/browser/autocomplete/autocomplete_classifier.h" #include "chrome/browser/background/background_contents_service_factory.h" #include "chrome/browser/background/background_mode_manager.h" -#include "chrome/browser/bookmarks/bookmark_model.h" +#include "chrome/browser/bookmarks/bookmark_model_factory.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/chrome_plugin_service_filter.h" #include "chrome/browser/content_settings/cookie_settings.h" @@ -40,6 +40,7 @@ #include "chrome/browser/favicon/favicon_service.h" #include "chrome/browser/geolocation/chrome_geolocation_permission_context.h" #include "chrome/browser/history/history.h" +#include "chrome/browser/history/history_service_factory.h" #include "chrome/browser/history/shortcuts_backend.h" #include "chrome/browser/history/top_sites.h" #include "chrome/browser/instant/instant_controller.h" @@ -232,7 +233,6 @@ ProfileImpl::ProfileImpl(const FilePath& path, new VisitedLinkEventListener(this))), ALLOW_THIS_IN_INITIALIZER_LIST(io_data_(this)), host_content_settings_map_(NULL), - history_service_created_(false), favicon_service_created_(false), start_time_(Time::Now()), delegate_(delegate), @@ -515,29 +515,6 @@ ProfileImpl::~ProfileImpl() { if (top_sites_.get()) top_sites_->Shutdown(); - if (bookmark_bar_model_.get()) { - // It's possible that bookmarks haven't loaded and history is waiting for - // bookmarks to complete loading. In such a situation history can't shutdown - // (meaning if we invoked history_service_->Cleanup now, we would - // deadlock). To break the deadlock we tell BookmarkModel it's about to be - // deleted so that it can release the signal history is waiting on, allowing - // history to shutdown (history_service_->Cleanup to complete). In such a - // scenario history sees an incorrect view of bookmarks, but it's better - // than a deadlock. - bookmark_bar_model_->Cleanup(); - } - - if (history_service_.get()) - history_service_->Cleanup(); - - // HistoryService may call into the BookmarkModel, as such we need to - // delete HistoryService before the BookmarkModel. The destructor for - // HistoryService will join with HistoryService's backend thread so that - // by the time the destructor has finished we're sure it will no longer call - // into the BookmarkModel. - history_service_ = NULL; - bookmark_bar_model_.reset(); - // FaviconService depends on HistoryServce so make sure we delete // HistoryService first. favicon_service_.reset(); @@ -791,23 +768,11 @@ GAIAInfoUpdateService* ProfileImpl::GetGAIAInfoUpdateService() { } HistoryService* ProfileImpl::GetHistoryService(ServiceAccessType sat) { - // If saving history is disabled, only allow explicit access. - if (GetPrefs()->GetBoolean(prefs::kSavingBrowserHistoryDisabled) && - sat != EXPLICIT_ACCESS) - return NULL; - - if (!history_service_created_) { - history_service_created_ = true; - scoped_refptr<HistoryService> history(new HistoryService(this)); - if (!history->Init(GetPath(), GetBookmarkModel())) - return NULL; - history_service_.swap(history); - } - return history_service_.get(); + return HistoryServiceFactory::GetForProfile(this, sat).get(); } HistoryService* ProfileImpl::GetHistoryServiceWithoutCreating() { - return history_service_.get(); + return HistoryServiceFactory::GetForProfileIfExists(this).get(); } AutocompleteClassifier* ProfileImpl::GetAutocompleteClassifier() { @@ -843,11 +808,7 @@ quota::SpecialStoragePolicy* ProfileImpl::GetSpecialStoragePolicy() { } BookmarkModel* ProfileImpl::GetBookmarkModel() { - if (!bookmark_bar_model_.get()) { - bookmark_bar_model_.reset(new BookmarkModel(this)); - bookmark_bar_model_->Load(); - } - return bookmark_bar_model_.get(); + return BookmarkModelFactory::GetForProfile(this); } ProtocolHandlerRegistry* ProfileImpl::GetProtocolHandlerRegistry() { diff --git a/chrome/browser/profiles/profile_impl.h b/chrome/browser/profiles/profile_impl.h index b8ae51d..969cf47 100644 --- a/chrome/browser/profiles/profile_impl.h +++ b/chrome/browser/profiles/profile_impl.h @@ -209,7 +209,6 @@ class ProfileImpl : public Profile, scoped_refptr<ExtensionSpecialStoragePolicy> extension_special_storage_policy_; scoped_ptr<NetPrefObserver> net_pref_observer_; - scoped_ptr<BookmarkModel> bookmark_bar_model_; #if defined(ENABLE_PROMO_RESOURCE_SERVICE) scoped_refptr<PromoResourceService> promo_resource_service_; @@ -223,11 +222,9 @@ class ProfileImpl : public Profile, scoped_refptr<content::GeolocationPermissionContext> geolocation_permission_context_; scoped_ptr<GAIAInfoUpdateService> gaia_info_update_service_; - scoped_refptr<HistoryService> history_service_; scoped_ptr<FaviconService> favicon_service_; scoped_ptr<AutocompleteClassifier> autocomplete_classifier_; scoped_refptr<history::ShortcutsBackend> shortcuts_backend_; - bool history_service_created_; bool favicon_service_created_; // Whether or not the last session exited cleanly. This is set only once. diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index 7f8cc6a..d86e0cc 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -298,6 +298,8 @@ 'browser/bookmarks/bookmark_index.h', 'browser/bookmarks/bookmark_model.cc', 'browser/bookmarks/bookmark_model.h', + 'browser/bookmarks/bookmark_model_factory.cc', + 'browser/bookmarks/bookmark_model_factory.h', 'browser/bookmarks/bookmark_model_observer.h', 'browser/bookmarks/bookmark_node_data.cc', 'browser/bookmarks/bookmark_node_data.h', @@ -1120,6 +1122,8 @@ 'browser/history/history_publisher.h', 'browser/history/history_publisher_none.cc', 'browser/history/history_publisher_win.cc', + 'browser/history/history_service_factory.cc', + 'browser/history/history_service_factory.h', 'browser/history/history_tab_helper.cc', 'browser/history/history_tab_helper.h', 'browser/history/history_types.cc', diff --git a/chrome/test/base/testing_profile.cc b/chrome/test/base/testing_profile.cc index 90077d1..cf794f6 100644 --- a/chrome/test/base/testing_profile.cc +++ b/chrome/test/base/testing_profile.cc @@ -14,6 +14,7 @@ #include "base/string_number_conversions.h" #include "chrome/browser/autocomplete/autocomplete_classifier.h" #include "chrome/browser/bookmarks/bookmark_model.h" +#include "chrome/browser/bookmarks/bookmark_model_factory.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/content_settings/host_content_settings_map.h" #include "chrome/browser/custom_handlers/protocol_handler_registry.h" @@ -26,6 +27,7 @@ #include "chrome/browser/geolocation/chrome_geolocation_permission_context.h" #include "chrome/browser/history/history.h" #include "chrome/browser/history/history_backend.h" +#include "chrome/browser/history/history_service_factory.h" #include "chrome/browser/history/top_sites.h" #include "chrome/browser/net/proxy_service_factory.h" #include "chrome/browser/notifications/desktop_notification_service.h" @@ -245,8 +247,6 @@ TestingProfile::~TestingProfile() { host_content_settings_map_->ShutdownOnUIThread(); DestroyTopSites(); - DestroyHistoryService(); - // FaviconService depends on HistoryServce so destroying it later. DestroyFaviconService(); if (pref_proxy_config_tracker_.get()) @@ -257,6 +257,11 @@ void TestingProfile::CreateFaviconService() { favicon_service_.reset(new FaviconService(this)); } +static scoped_refptr<RefcountedProfileKeyedService> BuildHistoryService( + Profile* profile) { + return new HistoryService(profile); +} + void TestingProfile::CreateHistoryService(bool delete_file, bool no_db) { DestroyHistoryService(); if (delete_file) { @@ -264,18 +269,28 @@ void TestingProfile::CreateHistoryService(bool delete_file, bool no_db) { path = path.Append(chrome::kHistoryFilename); file_util::Delete(path, false); } - history_service_ = new HistoryService(this); - history_service_->Init(GetPath(), bookmark_bar_model_.get(), no_db); + // This will create and init the history service. + HistoryService* history_service = static_cast<HistoryService*>( + HistoryServiceFactory::GetInstance()->SetTestingFactoryAndUse( + this, BuildHistoryService).get()); + if (!history_service->Init(this->GetPath(), + reinterpret_cast<BookmarkService*>( + BookmarkModelFactory::GetForProfile(this)), + no_db)) { + HistoryServiceFactory::GetInstance()->SetTestingFactoryAndUse(this, NULL); + } } void TestingProfile::DestroyHistoryService() { - if (!history_service_.get()) + scoped_refptr<HistoryService> history_service = + HistoryServiceFactory::GetForProfileIfExists(this); + if (!history_service.get()) return; - history_service_->NotifyRenderProcessHostDestruction(0); - history_service_->SetOnBackendDestroyTask(MessageLoop::QuitClosure()); - history_service_->Cleanup(); - history_service_ = NULL; + history_service->NotifyRenderProcessHostDestruction(0); + history_service->SetOnBackendDestroyTask(MessageLoop::QuitClosure()); + history_service->Cleanup(); + HistoryServiceFactory::ShutdownForProfile(this); // Wait for the backend class to terminate before deleting the files and // moving to the next test. Note: if this never terminates, somebody is @@ -312,23 +327,34 @@ void TestingProfile::DestroyFaviconService() { favicon_service_.reset(); } +static ProfileKeyedService* BuildBookmarkModel(Profile* profile) { + BookmarkModel* bookmark_model = new BookmarkModel(profile); + bookmark_model->Load(); + return bookmark_model; +} + + void TestingProfile::CreateBookmarkModel(bool delete_file) { - // Nuke the model first, that way we're sure it's done writing to disk. - bookmark_bar_model_.reset(NULL); if (delete_file) { FilePath path = GetPath(); path = path.Append(chrome::kBookmarksFileName); file_util::Delete(path, false); } - bookmark_bar_model_.reset(new BookmarkModel(this)); - if (history_service_.get()) { - history_service_->history_backend_->bookmark_service_ = - bookmark_bar_model_.get(); - history_service_->history_backend_->expirer_.bookmark_service_ = - bookmark_bar_model_.get(); + // This will create a bookmark model. + BookmarkModel* bookmark_service = + static_cast<BookmarkModel*>( + BookmarkModelFactory::GetInstance()->SetTestingFactoryAndUse( + this, BuildBookmarkModel)); + + HistoryService* history_service = + HistoryServiceFactory::GetForProfileIfExists(this).get(); + if (history_service) { + history_service->history_backend_->bookmark_service_ = + bookmark_service; + history_service->history_backend_->expirer_.bookmark_service_ = + bookmark_service; } - bookmark_bar_model_->Load(); } void TestingProfile::CreateAutocompleteClassifier() { @@ -354,14 +380,14 @@ void TestingProfile::CreateWebDataService() { } void TestingProfile::BlockUntilBookmarkModelLoaded() { - DCHECK(bookmark_bar_model_.get()); - if (bookmark_bar_model_->IsLoaded()) + DCHECK(GetBookmarkModel()); + if (GetBookmarkModel()->IsLoaded()) return; BookmarkLoadObserver observer; - bookmark_bar_model_->AddObserver(&observer); + GetBookmarkModel()->AddObserver(&observer); MessageLoop::current()->Run(); - bookmark_bar_model_->RemoveObserver(&observer); - DCHECK(bookmark_bar_model_->IsLoaded()); + GetBookmarkModel()->RemoveObserver(&observer); + DCHECK(GetBookmarkModel()->IsLoaded()); } // TODO(phajdan.jr): Doesn't this hang if Top Sites are already loaded? @@ -476,11 +502,11 @@ FaviconService* TestingProfile::GetFaviconService(ServiceAccessType access) { } HistoryService* TestingProfile::GetHistoryService(ServiceAccessType access) { - return history_service_.get(); + return HistoryServiceFactory::GetForProfileIfExists(this); } HistoryService* TestingProfile::GetHistoryServiceWithoutCreating() { - return history_service_.get(); + return HistoryServiceFactory::GetForProfileIfExists(this); } net::CookieMonster* TestingProfile::GetCookieMonster() { @@ -652,7 +678,7 @@ bool TestingProfile::DidLastSessionExitCleanly() { } BookmarkModel* TestingProfile::GetBookmarkModel() { - return bookmark_bar_model_.get(); + return BookmarkModelFactory::GetForProfileIfExists(this); } bool TestingProfile::IsSameProfile(Profile *p) { @@ -684,11 +710,13 @@ PrefProxyConfigTracker* TestingProfile::GetProxyConfigTracker() { } void TestingProfile::BlockUntilHistoryProcessesPendingRequests() { - DCHECK(history_service_.get()); + scoped_refptr<HistoryService> history_service = + HistoryServiceFactory::GetForProfileIfExists(this); + DCHECK(history_service.get()); DCHECK(MessageLoop::current()); CancelableRequestConsumer consumer; - history_service_->ScheduleDBTask(new QuittingHistoryDBTask(), &consumer); + history_service->ScheduleDBTask(new QuittingHistoryDBTask(), &consumer); MessageLoop::current()->Run(); } diff --git a/chrome/test/base/testing_profile.h b/chrome/test/base/testing_profile.h index 9d734efd..ff9bf7b 100644 --- a/chrome/test/base/testing_profile.h +++ b/chrome/test/base/testing_profile.h @@ -32,12 +32,10 @@ class SpecialStoragePolicy; } class AutocompleteClassifier; -class BookmarkModel; class CommandLine; class ExtensionPrefs; class ExtensionSpecialStoragePolicy; class FaviconService; -class HistoryService; class HostContentSettingsMap; class PrefService; class ProfileDependencyManager; @@ -270,12 +268,6 @@ class TestingProfile : public Profile { // The favicon service. Only created if CreateFaviconService is invoked. scoped_ptr<FaviconService> favicon_service_; - // The history service. Only created if CreateHistoryService is invoked. - scoped_refptr<HistoryService> history_service_; - - // The BookmarkModel. Only created if CreateBookmarkModel is invoked. - scoped_ptr<BookmarkModel> bookmark_bar_model_; - // The ProtocolHandlerRegistry. Only created if CreateProtocolHandlerRegistry // is invoked. scoped_refptr<ProtocolHandlerRegistry> protocol_handler_registry_; |
