Converting BookmarkModel and HistoryService to ProfileKeyedServices. This just...

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

chrome/browser/bookmarks/bookmark_expanded_state_tracker.cc

-// 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(

chrome/browser/bookmarks/bookmark_expanded_state_tracker.h

-// 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.

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.

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();
 

chrome/browser/bookmarks/bookmark_model_factory.cc

+// 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;
+}

chrome/browser/bookmarks/bookmark_model_factory.h

+// 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_

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,

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

chrome/browser/history/history_service_factory.cc

+// 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;
+}

chrome/browser/history/history_service_factory.h

+// 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_

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;
 }
 

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);

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();

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() {

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.

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',

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();
 }
 

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_;