Profile shouldn't own PersonalDataManager

chrome/browser/autofill/personal_data_manager.h

 // 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.
 
 #ifndef CHROME_BROWSER_AUTOFILL_PERSONAL_DATA_MANAGER_H_
 #define CHROME_BROWSER_AUTOFILL_PERSONAL_DATA_MANAGER_H_
 #pragma once
 
 #include <set>
 #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"
 #include "base/string16.h"
 #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;
 class FormStructure;
 class PersonalDataManagerObserver;
 class Profile;
 
 namespace autofill_helper {
 void SetProfiles(int, std::vector<AutofillProfile>*);
 }
 
 // Handles loading and saving Autofill profile information to the web database.
 // This class also stores the profiles loaded from the database for use during
 // Autofill.
 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);
 
   // Removes |observer| as the observer of this 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
   // sufficient credit card data, it is stored into |credit_card|, so that we
   // can prompt the user whether to save this data.
   // Returns |true| if sufficient address or credit card data was found.
   bool ImportFormData(const FormStructure& form,
                       const CreditCard** credit_card);
 
   // Saves a credit card value detected in |ImportedFormData|.
@@ skipping 60 matching lines @@
 
   // Merges |profile| into one of the |existing_profiles| if possible; otherwise
   // appends |profile| to the end of that list. Fills |merged_profiles| with the
   // result.
   static bool MergeProfile(
       const AutofillProfile& profile,
       const std::vector<AutofillProfile*>& existing_profiles,
       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();
 
   // Sets |web_profiles_| to the contents of |profiles| and updates the web
   // database by adding, updating and removing profiles.
   // The relationship between this and Refresh is subtle.
   // A call to |SetProfiles| could include out-of-date data that may conflict
   // if we didn't refresh-to-latest before an Autofill window was opened for
   // editing. |SetProfiles| is implemented to make a "best effort" to apply the
   // changes, but in extremely rare edge cases it is possible not all of the
@@ skipping 75 matching lines @@
   // The observers.
   ObserverList<PersonalDataManagerObserver> observers_;
 
  private:
   // For logging UMA metrics. Overridden by metrics tests.
   scoped_ptr<const AutofillMetrics> metric_logger_;
 
   // 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);
 };
 
 #endif  // CHROME_BROWSER_AUTOFILL_PERSONAL_DATA_MANAGER_H_