Topsites become keyedService based.

chrome/test/base/testing_profile.cc

Index: chrome/test/base/testing_profile.cc
diff --git a/chrome/test/base/testing_profile.cc b/chrome/test/base/testing_profile.cc
index 0d7d1442be8593268994b60de6071ef7ce6e5bec..09e40948971ad64a73f5828685749e8642c6dfc7 100644
--- a/chrome/test/base/testing_profile.cc
+++ b/chrome/test/base/testing_profile.cc
@@ -27,6 +27,8 @@
 #include "chrome/browser/history/history_service.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/history/top_sites.h"
+#include "chrome/browser/history/top_sites_factory.h"
+#include "chrome/browser/history/top_sites_impl.h"
 #include "chrome/browser/history/web_history_service_factory.h"
 #include "chrome/browser/net/pref_proxy_config_tracker.h"
 #include "chrome/browser/net/proxy_service_factory.h"
@@ -112,6 +114,21 @@ using testing::Return;
 
 namespace {
 
+// TopSitesImpl::Shutdown schedules some tasks (from TopSitesBackend) that
+// need to be run to properly shutdown. Run all pending tasks now. This is
+// normally handled by browser_process shutdown.
+
+void CleanupAfterTopSitesDestroyed() {
+  if (base::MessageLoop::current())
+    base::MessageLoop::current()->RunUntilIdle();
+}
+
+// Returns true if a TopSites service has been registered for |profile|.
+bool HasTopSites(Profile* profile) {
+  TopSitesFactory* top_sites_factory = TopSitesFactory::GetInstance();

[sdefresne, 2015/01/22 19:05:05]

This does not compile on Windows, and I don't really understand why. You can fix
the compilation failure by rewriting the code as:

return !!TopSitesFactory::GetInstance()->GetForProfileIfExists(profile);

[Bernhard Bauer, 2015/01/22 21:23:27]

In any case, the .get() and the comparison to nullptr isn't necessary.

+  return top_sites_factory->GetForProfileIfExists(profile).get() != nullptr;
+}
+
 // Used to make sure TopSites has finished loading
 class WaitTopSitesLoadedObserver : public history::TopSitesObserver {
  public:
@@ -242,6 +259,14 @@ KeyedService* BuildWebDataService(content::BrowserContext* context) {
       &TestProfileErrorCallback);
 }
 
+scoped_refptr<RefcountedKeyedService> BuildTopSites(
+    content::BrowserContext* profile) {
+  history::TopSitesImpl* top_sites =
+      new history::TopSitesImpl(static_cast<Profile*>(profile));
+  top_sites->Init(profile->GetPath().Append(chrome::kTopSitesFilename));
+  return make_scoped_refptr(top_sites);
+}
+
 }  // namespace
 
 // static
@@ -492,12 +517,19 @@ TestingProfile::~TestingProfile() {
 
   MaybeSendDestroyedNotification();
 
+  // Remember whether a TopSites has beed created for the current profile,

[Bernhard Bauer, 2015/01/22 21:23:27]

Nit: "has been", or "had been" (below you use the latter, so it should be
consistent).

[Jitu( very slow this week), 2015/01/23 11:36:53]

Done.

+  // so that we can run cleanup after destroying all services.
+  bool had_top_sites = HasTopSites(this);
+
   browser_context_dependency_manager_->DestroyBrowserContextServices(this);
 
   if (host_content_settings_map_.get())
     host_content_settings_map_->ShutdownOnUIThread();
 
-  DestroyTopSites();
+  // Wait until TopSites shutdown tasks have completed if a TopSites had
+  // been created for the current profile.
+  if (had_top_sites)
+    CleanupAfterTopSitesDestroyed();
 
   if (pref_proxy_config_tracker_.get())
     pref_proxy_config_tracker_->DetachFromPrefService();
@@ -570,24 +602,17 @@ void TestingProfile::DestroyHistoryService() {
 
 void TestingProfile::CreateTopSites() {
   DestroyTopSites();
-  top_sites_ = history::TopSites::Create(
-      this, GetPath().Append(chrome::kTopSitesFilename));
-}
-
-void TestingProfile::SetTopSites(history::TopSites* top_sites) {
-  DestroyTopSites();
-  top_sites_ = top_sites;
+  TopSitesFactory::GetInstance()->SetTestingFactoryAndUse(this, BuildTopSites);
 }
 
 void TestingProfile::DestroyTopSites() {
-  if (top_sites_.get()) {
-    top_sites_->Shutdown();
-    top_sites_ = NULL;
-    // TopSitesImpl::Shutdown schedules some tasks (from TopSitesBackend) that
-    // need to be run to properly shutdown. Run all pending tasks now. This is
-    // normally handled by browser_process shutdown.
-    if (base::MessageLoop::current())
-      base::MessageLoop::current()->RunUntilIdle();
+  TopSitesFactory* top_sites_factory = TopSitesFactory::GetInstance();
+  if (top_sites_factory->GetForProfileIfExists(this)) {
+    // BrowserContextKeyedServiceFactory will destroy the previous service when
+    // registering a new testing factory so use this to ensure that destroy the
+    // old service.
+    top_sites_factory->SetTestingFactory(this, nullptr);
+    CleanupAfterTopSitesDestroyed();
   }
 }
 
@@ -633,9 +658,11 @@ void TestingProfile::BlockUntilTopSitesLoaded() {
   scoped_refptr<content::MessageLoopRunner> runner =
       new content::MessageLoopRunner;
   WaitTopSitesLoadedObserver observer(runner.get());
-  top_sites_->AddObserver(&observer);
+  scoped_refptr<history::TopSites> top_sites =
+      TopSitesFactory::GetForProfile(this);
+  top_sites->AddObserver(&observer);
   runner->Run();
-  top_sites_->RemoveObserver(&observer);
+  top_sites->RemoveObserver(&observer);
 }
 
 void TestingProfile::SetGuestSession(bool guest) {
@@ -793,14 +820,6 @@ PrefService* TestingProfile::GetPrefs() {
   return prefs_.get();
 }
 
-history::TopSites* TestingProfile::GetTopSites() {
-  return top_sites_.get();
-}
-
-history::TopSites* TestingProfile::GetTopSitesWithoutCreating() {
-  return top_sites_.get();
-}
-
 DownloadManagerDelegate* TestingProfile::GetDownloadManagerDelegate() {
   return NULL;
 }