Profile shouldn't own PersonalDataManager

chrome/browser/autofill/personal_data_manager_factory.cc

Index: chrome/browser/autofill/personal_data_manager_factory.cc
diff --git a/chrome/browser/autofill/personal_data_manager_factory.cc b/chrome/browser/autofill/personal_data_manager_factory.cc
new file mode 100644
index 0000000000000000000000000000000000000000..1d09fac332511284575a605d4358751b8ec66d49
--- /dev/null
+++ b/chrome/browser/autofill/personal_data_manager_factory.cc
@@ -0,0 +1,37 @@
+// Copyright (c) 2011 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/autofill/personal_data_manager_factory.h"
+
+#include "chrome/browser/autofill/personal_data_manager.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_dependency_manager.h"
+
+// static
+PersonalDataManager* PersonalDataManagerFactory::GetForProfile(
+    Profile* profile) {
+  return static_cast<PersonalDataManager*>(
+      GetInstance()->GetServiceForProfile(profile, true));
+}
+
+// static
+PersonalDataManagerFactory* PersonalDataManagerFactory::GetInstance() {
+  return Singleton<PersonalDataManagerFactory>::get();
+}
+
+PersonalDataManagerFactory::PersonalDataManagerFactory()
+    : ProfileKeyedServiceFactory(ProfileDependencyManager::GetInstance()) {
+  // TODO(erg): For Shutdown() order, we need to:

[Ilya Sherman, 2011/09/23 03:56:34]

Is this TODO intentionally unimplemented for now?

[dhollowa, 2011/09/23 16:30:03]

Yes, this is a note to erg@ to set up dependencies explicitly once the other
"services" have moved to the new API.  There are similar TODOs in other factory
constructors.

+  //     DependsOn(WebDataServiceFactory::GetInstance());
+}
+
+PersonalDataManagerFactory::~PersonalDataManagerFactory() {
+}
+
+ProfileKeyedService* PersonalDataManagerFactory::BuildServiceInstanceFor(
+    Profile* profile) const {
+  PersonalDataManager* personal_data_manager = new PersonalDataManager();
+  personal_data_manager->Init(profile);

[Ilya Sherman, 2011/09/23 03:56:34]

Do we still need to provide a constructor and an Init method separately?  If so,
is that purely for testing?  Should the method be private, i.e. only accessible
to friends?

[dhollowa, 2011/09/23 16:30:03]

Yes, the two-phase init is required for testing.  The common pattern is to
subclass the PDM and then mock specific calls to sever ties to the WDS backend. 
I agree about making it private.  I've done that.

+  return personal_data_manager;
+}