Profile shouldn't own PersonalDataManager

chrome/browser/autofill/personal_data_manager.h

Index: chrome/browser/autofill/personal_data_manager.h
diff --git a/chrome/browser/autofill/personal_data_manager.h b/chrome/browser/autofill/personal_data_manager.h
index 46576c7bfd6f73719146fff7a1087df50b405aa6..9f7bb611c7e70f6e5c90235ce755d327ede7e9c2 100644
--- a/chrome/browser/autofill/personal_data_manager.h
+++ b/chrome/browser/autofill/personal_data_manager.h
@@ -10,7 +10,6 @@
 #include <vector>
 
 #include "base/basictypes.h"
-#include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
 #include "base/observer_list.h"
@@ -18,8 +17,11 @@
 #include "chrome/browser/autofill/autofill_profile.h"
 #include "chrome/browser/autofill/credit_card.h"
 #include "chrome/browser/autofill/field_types.h"
+#include "chrome/browser/profiles/profile_keyed_service.h"
 #include "chrome/browser/sync/profile_sync_service_observer.h"
 #include "chrome/browser/webdata/web_data_service.h"
+#include "content/common/notification_observer.h"
+#include "content/common/notification_registrar.h"
 
 class AutofillManager;
 class AutofillMetrics;
@@ -37,11 +39,13 @@ void SetProfiles(int, std::vector<AutofillProfile>*);
 class PersonalDataManager
     : public WebDataServiceConsumer,
       public ProfileSyncServiceObserver,
-      public base::RefCountedThreadSafe<PersonalDataManager> {
+      public ProfileKeyedService,
+      public NotificationObserver {
  public:
-  // WebDataServiceConsumer implementation:
-  virtual void OnWebDataServiceRequestDone(WebDataService::Handle h,
-                                           const WDTypedResult* result);
+  // WebDataServiceConsumer:
+  virtual void OnWebDataServiceRequestDone(
+      WebDataService::Handle h,
+      const WDTypedResult* result) OVERRIDE;
 
   // Sets the listener to be notified of PersonalDataManager events.
   virtual void SetObserver(PersonalDataManagerObserver* observer);
@@ -50,7 +54,19 @@ class PersonalDataManager
   virtual void RemoveObserver(PersonalDataManagerObserver* observer);
 
   // ProfileSyncServiceObserver:
-  virtual void OnStateChanged();
+  virtual void OnStateChanged() OVERRIDE;
+
+  // ProfileKeyedService:
+  // Cancels any pending requests to WebDataService and stops listening for Sync
+  // notifications.
+  virtual void Shutdown() OVERRIDE;
+
+  // NotificationObserver:
+  // Observes "batch" changes made by Sync and refreshes data from the
+  // WebDataService in response.
+  virtual void Observe(int type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details) OVERRIDE;
 
   // Scans the given |form| for importable Autofill data. If the form includes
   // sufficient address data, it is immediately imported. If the form includes
@@ -131,16 +147,16 @@ class PersonalDataManager
       std::vector<AutofillProfile>* merged_profiles);
 
  protected:
-  // Make sure that only Profile and certain tests can create an instance of
+  // Only PersonalDataManagerFactory and certain tests can create instances of
   // PersonalDataManager.
-  friend class base::RefCountedThreadSafe<PersonalDataManager>;
-  friend class AutofillMergeTest;
+  friend class PersonalDataManagerFactory;
   friend class PersonalDataManagerTest;
-  friend class ProfileImpl;
-  friend class ProfileSyncServiceAutofillTest;
+  friend class scoped_ptr<PersonalDataManager>;
   friend class TestingAutomationProvider;
   friend void autofill_helper::SetProfiles(int, std::vector<AutofillProfile>*);
 
+  // Creation and destruction is managed by PersonalDataManagerFactory only.
+  // Except for tests which subclass.

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

nit: I find this comment a bit confusing as written; and it seems redundant with
the comment immediately above.

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

Done.

   PersonalDataManager();
   virtual ~PersonalDataManager();
 
@@ -236,6 +252,9 @@ class PersonalDataManager
   // Whether we have already logged the number of profiles this session.
   mutable bool has_logged_profile_count_;
 
+  // Manages registration lifetime for NotificationObserver implementation.
+  NotificationRegistrar notification_registrar_;
+
   DISALLOW_COPY_AND_ASSIGN(PersonalDataManager);
 };