sync: Make ProfileSyncService a ProfileKeyedService.

chrome/browser/sync/profile_sync_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/sync/profile_sync_service_factory.h"
+
+#include "base/command_line.h"
+#include "base/memory/singleton.h"
+#include "chrome/browser/defaults.h"
+#include "chrome/browser/autofill/personal_data_manager_factory.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_dependency_manager.h"
+#include "chrome/browser/search_engines/template_url_service_factory.h"
+#include "chrome/browser/sessions/tab_restore_service_factory.h"
+#include "chrome/browser/signin/signin_manager.h"
+#include "chrome/browser/sync/profile_sync_components_factory_impl.h"
+#include "chrome/browser/sync/profile_sync_service.h"
+#include "chrome/browser/themes/theme_service_factory.h"
+#include "chrome/browser/ui/global_error_service_factory.h"
+
+// static
+ProfileSyncServiceFactory* ProfileSyncServiceFactory::GetInstance() {
+  return Singleton<ProfileSyncServiceFactory>::get();
+}
+
+// static
+ProfileSyncService* ProfileSyncServiceFactory::GetForProfile(
+    Profile* profile) {
+  if (!ProfileSyncService::IsSyncEnabled())
+    return NULL;
+
+  ProfileSyncService* service = static_cast<ProfileSyncService*>(
+      GetInstance()->GetServiceForProfile(profile, false));
+
+  // TODO(tim): This ugliness is necessary because part of ProfileSyncService
+  // initialization is registering enabled data types with the
+  // ProfileSyncComponentsFactory, which creates DataTypeControllers.
+  // The NewNonFrontendDataTypeController class doesn't have the sync service
+  // injected (oddly, since FrontendDTC does), and tries to get it from the
+  // profile / this factory.  This means we need to separate the act of creating
+  // the ProfileSyncService (and making GetForProfile return non-NULL) from
+  // initializing it, hence this code. We'll make the DTC take the service by
+  // injection to resolve this in a separate CL. (Or decide to move
+  // initialization elsewhere). Bug 93922.
+  if (!service) {
+    service = static_cast<ProfileSyncService*>(
+        GetInstance()->GetServiceForProfile(profile, true));
+    service->factory()->RegisterDataTypes(service);

[Andrew T Wilson (Slow), 2012/01/17 20:25:41]

Why does this live here and not down in BuildServiceInstanceFor()? I thought
that's where this type of initialization should live. Otherwise, people who try
to get instances of PSS without going through this static function will get an
improperly initialized object?

[Elliot Glaysher, 2012/01/17 20:56:07]

If I understand this correctly, Tim is using GetServiceForProfile(..., false) to
query if the service has been created and if it hasn't been, to create it and
then call some additional initialization stuff. I don't understand why this is
necessary. BuildServiceInstanceFor() is guarenteed to run once per |profile|.

[tim (not reviewing), 2012/01/17 21:17:26]

I tried to explain this in the TODO, but the reason is the
NonFrontendDataTypeController doesn't have the sync service injected, so it
calls GetForProfile.  If it does this as part of BuildServiceInstanceFor, you
wind up recursing. I want to fix this by just passing the service into the
controller, but that's a large mostly unrelated change I didn't feel was
necessary to couple here, so I decided to keep the mess localized to this one
spot and do it in two steps.

+    service->Initialize();
+  }
+
+  DCHECK(service);
+  return service;
+}
+
+ProfileSyncServiceFactory::ProfileSyncServiceFactory()
+    : ProfileKeyedServiceFactory(ProfileDependencyManager::GetInstance()) {
+
+  // The ProfileSyncService depends on various SyncableServices being around
+  // when it is shut down.  Specify those dependencies here to build the proper
+  // destruction order.
+  DependsOn(TemplateURLServiceFactory::GetInstance());
+  DependsOn(PersonalDataManagerFactory::GetInstance());
+  DependsOn(ThemeServiceFactory::GetInstance());
+  DependsOn(TabRestoreServiceFactory::GetInstance());
+  DependsOn(GlobalErrorServiceFactory::GetInstance());
+
+  // The following have not been converted to ProfileKeyedServices yet, and for
+  // now they are explicitly destroyed after the ProfileDependencyManager is
+  // told to DestroyProfileServices, so they will be around when the
+  // ProfileSyncService is destroyed.
+
+  // DependsOn(SigninManagerFactory::GetInstance());

[Andrew T Wilson (Slow), 2012/01/17 20:25:41]

Don't forget to uncomment this when my change re-lands (in CQ now).

+  // DependsOn(WebDataServiceFactory::GetInstance());
+  // DependsOn(HistoryServiceFactory::GetInstance());
+  // DependsOn(BookmarkBarModelFactory::GetInstance());
+  // DependsOn(FaviconServiceFactory::GetInstance());
+  // DependsOn(PasswordStoreService::GetInstance());
+  // DependsOn(ExtensionServiceFactory::GetInstance());
+}
+
+ProfileSyncServiceFactory::~ProfileSyncServiceFactory() {
+}
+
+ProfileKeyedService* ProfileSyncServiceFactory::BuildServiceInstanceFor(
+    Profile* profile) const {
+  ProfileSyncService::StartBehavior behavior =
+      browser_defaults::kSyncAutoStarts ? ProfileSyncService::AUTO_START
+                                        : ProfileSyncService::MANUAL_START;
+
+  SigninManager* signin = profile->GetSigninManager();
+
+  // TODO(tim): Currently, AUTO/MANUAL settings refer to the *first* time sync
+  // is set up and *not* a browser restart for a manual-start platform (where
+  // sync has already been set up, and should be able to start without user
+  // intervention). We can get rid of the browser_default eventually, but
+  // need to take care that ProfileSyncService doesn't get tripped up between
+  // those two cases. Bug 88109.
+  ProfileSyncService* pss = new ProfileSyncService(
+      new ProfileSyncComponentsFactoryImpl(profile,
+                                           CommandLine::ForCurrentProcess()),
+      profile,
+      signin,
+      behavior);
+  return pss;
+}
+
+// static
+bool ProfileSyncServiceFactory::HasProfileSyncService(Profile* profile) {
+  return GetInstance()->GetServiceForProfile(profile, false) != NULL;
+}