AutoFill: Make PersonalDataManager RefCountedThreadSafe.

BUG=40617
TEST=none

Review URL: http://codereview.chromium.org/2521001

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@48858 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 58 insertions, 40 deletions

diff --git a/chrome/browser/autofill/autofill_dialog_controller_mac_unittest.mm b/chrome/browser/autofill/autofill_dialog_controller_mac_unittest.mm
index fb1f38e..b82c377 100644
--- a/chrome/browser/autofill/autofill_dialog_controller_mac_unittest.mm
+++ b/chrome/browser/autofill/autofill_dialog_controller_mac_unittest.mm
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/ref_counted.h"
 #import "chrome/browser/autofill/autofill_address_model_mac.h"
 #import "chrome/browser/autofill/autofill_address_view_controller_mac.h"
 #import "chrome/browser/autofill/autofill_credit_card_model_mac.h"
@@ -70,7 +71,7 @@ class PersonalDataManagerMock : public PersonalDataManager {
 class ProfileMock : public TestingProfile {
  public:
   ProfileMock() {
-    test_manager_.reset(new PersonalDataManagerMock);
+    test_manager_ =new PersonalDataManagerMock;
   }
   virtual ~ProfileMock() {}
 
@@ -78,7 +79,7 @@ class ProfileMock : public TestingProfile {
     return test_manager_.get();
   }
 
-  scoped_ptr<PersonalDataManagerMock> test_manager_;
+  scoped_refptr<PersonalDataManagerMock> test_manager_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(ProfileMock);
@@ -245,16 +246,18 @@ TEST_F(AutoFillDialogControllerTest, AutoFillDataMutation) {
   profile.SetInfo(AutoFillType(EMAIL_ADDRESS),
       ASCIIToUTF16("dhollowa@chromium.org"));
   profile.SetInfo(AutoFillType(COMPANY_NAME), ASCIIToUTF16("Google Inc."));
-  profile.SetInfo(
-      AutoFillType(ADDRESS_HOME_LINE1), ASCIIToUTF16("1122 Mountain View Road"));
+  profile.SetInfo(AutoFillType(ADDRESS_HOME_LINE1),
+                  ASCIIToUTF16("1122 Mountain View Road"));
   profile.SetInfo(AutoFillType(ADDRESS_HOME_LINE2), ASCIIToUTF16("Suite #1"));
   profile.SetInfo(AutoFillType(ADDRESS_HOME_CITY),
       ASCIIToUTF16("Mountain View"));
   profile.SetInfo(AutoFillType(ADDRESS_HOME_STATE), ASCIIToUTF16("CA"));
   profile.SetInfo(AutoFillType(ADDRESS_HOME_ZIP), ASCIIToUTF16("94111"));
   profile.SetInfo(AutoFillType(ADDRESS_HOME_COUNTRY), ASCIIToUTF16("USA"));
-  profile.SetInfo(AutoFillType(PHONE_HOME_WHOLE_NUMBER), ASCIIToUTF16("014155552258"));
-  profile.SetInfo(AutoFillType(PHONE_FAX_WHOLE_NUMBER), ASCIIToUTF16("024087172258"));
+  profile.SetInfo(
+      AutoFillType(PHONE_HOME_WHOLE_NUMBER), ASCIIToUTF16("014155552258"));
+  profile.SetInfo(
+      AutoFillType(PHONE_FAX_WHOLE_NUMBER), ASCIIToUTF16("024087172258"));
   profiles().push_back(&profile);
 
   LoadDialog();
diff --git a/chrome/browser/autofill/autofill_manager.h b/chrome/browser/autofill/autofill_manager.h
index 262a8a0..546f02c 100644
--- a/chrome/browser/autofill/autofill_manager.h
+++ b/chrome/browser/autofill/autofill_manager.h
@@ -99,6 +99,9 @@ class AutoFillManager : public RenderViewHostDelegate::AutoFill,
   AutoFillManager();
   AutoFillManager(TabContents* tab_contents,
                   PersonalDataManager* personal_data);
+  void set_personal_data_manager(PersonalDataManager* personal_data) {
+    personal_data_ = personal_data;
+  }
 
  private:
   // Returns a list of values from the stored profiles that match |type| and the
diff --git a/chrome/browser/autofill/autofill_manager_unittest.cc b/chrome/browser/autofill/autofill_manager_unittest.cc
index a54b04d..e8ecac3 100644
--- a/chrome/browser/autofill/autofill_manager_unittest.cc
+++ b/chrome/browser/autofill/autofill_manager_unittest.cc
@@ -4,6 +4,7 @@
 
 #include <vector>
 
+#include "base/ref_counted.h"
 #include "base/scoped_ptr.h"
 #include "base/scoped_vector.h"
 #include "base/string16.h"
@@ -87,17 +88,19 @@ class TestPersonalDataManager : public PersonalDataManager {
 class TestAutoFillManager : public AutoFillManager {
  public:
   explicit TestAutoFillManager(TabContents* tab_contents)
-      : AutoFillManager(tab_contents, &test_personal_data_) {
+      : AutoFillManager(tab_contents, NULL) {
+    test_personal_data_ = new TestPersonalDataManager();
+    set_personal_data_manager(test_personal_data_.get());
   }
 
   virtual bool IsAutoFillEnabled() const { return true; }
 
   AutoFillProfile* GetLabeledProfile(const char* label) {
-    return test_personal_data_.GetLabeledProfile(label);
+    return test_personal_data_->GetLabeledProfile(label);
   }
 
  private:
-  TestPersonalDataManager test_personal_data_;
+  scoped_refptr<TestPersonalDataManager> test_personal_data_;
 
   DISALLOW_COPY_AND_ASSIGN(TestAutoFillManager);
 };
diff --git a/chrome/browser/autofill/personal_data_manager.h b/chrome/browser/autofill/personal_data_manager.h
index 9b5ac12..678d001 100644
--- a/chrome/browser/autofill/personal_data_manager.h
+++ b/chrome/browser/autofill/personal_data_manager.h
@@ -8,6 +8,7 @@
 #include <set>
 #include <vector>
 
+#include "base/ref_counted.h"
 #include "base/scoped_ptr.h"
 #include "base/scoped_vector.h"
 #include "base/string16.h"
@@ -24,8 +25,10 @@ class Profile;
 // 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 AutoFillDialogObserver {
+class PersonalDataManager
+    : public WebDataServiceConsumer,
+      public AutoFillDialogObserver,
+      public base::RefCountedThreadSafe<PersonalDataManager> {
  public:
   // An interface the PersonalDataManager uses to notify its clients (observers)
   // when it has finished loading personal data from the web database.  Register
@@ -39,8 +42,6 @@ class PersonalDataManager : public WebDataServiceConsumer,
     virtual ~Observer() {}
   };
 
-  virtual ~PersonalDataManager();
-
   // WebDataServiceConsumer implementation:
   virtual void OnWebDataServiceRequestDone(WebDataService::Handle h,
                                            const WDTypedResult* result);
@@ -153,12 +154,15 @@ class PersonalDataManager : public WebDataServiceConsumer,
   void Init(Profile* profile);
 
  protected:
-  // Make sure that only Profile and the PersonalDataManager tests can create an
-  // instance of PersonalDataManager.
-  friend class ProfileImpl;
+  // Make sure that only Profile and certain tests can create an instance of
+  // PersonalDataManager.
+  friend class base::RefCountedThreadSafe<PersonalDataManager>;
   friend class PersonalDataManagerTest;
+  friend class ProfileImpl;
+  friend class ProfileSyncServiceAutofillTest;
 
   PersonalDataManager();
+  ~PersonalDataManager();
 
   // Returns the profile of the tab contents.
   Profile* profile();
diff --git a/chrome/browser/autofill/personal_data_manager_unittest.cc b/chrome/browser/autofill/personal_data_manager_unittest.cc
index 8656e25..a1ea3d0 100644
--- a/chrome/browser/autofill/personal_data_manager_unittest.cc
+++ b/chrome/browser/autofill/personal_data_manager_unittest.cc
@@ -4,6 +4,7 @@
 
 #include "base/basictypes.h"
 #include "base/message_loop.h"
+#include "base/ref_counted.h"
 #include "base/scoped_ptr.h"
 #include "chrome/browser/autofill/autofill_common_unittest.h"
 #include "chrome/browser/autofill/autofill_profile.h"
@@ -61,7 +62,7 @@ class PersonalDataManagerTest : public testing::Test {
   }
 
   void ResetPersonalDataManager() {
-    personal_data_.reset(new PersonalDataManager());
+    personal_data_ = new PersonalDataManager();
     personal_data_->Init(profile_.get());
     personal_data_->SetObserver(&personal_data_observer_);
 
@@ -82,7 +83,7 @@ class PersonalDataManagerTest : public testing::Test {
   ChromeThread ui_thread_;
   ChromeThread db_thread_;
   scoped_ptr<TestingProfile> profile_;
-  scoped_ptr<PersonalDataManager> personal_data_;
+  scoped_refptr<PersonalDataManager> personal_data_;
   NotificationRegistrar registrar_;
   NotificationObserverMock observer_;
   PersonalDataLoadedObserverMock personal_data_observer_;
diff --git a/chrome/browser/profile.cc b/chrome/browser/profile.cc
index 2f46701..89e5545 100644
--- a/chrome/browser/profile.cc
+++ b/chrome/browser/profile.cc
@@ -1287,7 +1287,7 @@ bool ProfileImpl::HasCreatedDownloadManager() const {
 
 PersonalDataManager* ProfileImpl::GetPersonalDataManager() {
   if (!personal_data_manager_.get()) {
-    personal_data_manager_.reset(new PersonalDataManager());
+    personal_data_manager_ = new PersonalDataManager();
     personal_data_manager_->Init(this);
   }
   return personal_data_manager_.get();
diff --git a/chrome/browser/profile.h b/chrome/browser/profile.h
index 118608c..7f8a6a1 100644
--- a/chrome/browser/profile.h
+++ b/chrome/browser/profile.h
@@ -12,6 +12,7 @@
 
 #include "base/basictypes.h"
 #include "base/file_path.h"
+#include "base/ref_counted.h"
 #include "base/scoped_ptr.h"
 #include "base/timer.h"
 #include "chrome/browser/spellcheck_host_observer.h"
@@ -627,7 +628,7 @@ class ProfileImpl : public Profile,
   scoped_ptr<BrowserThemeProvider> theme_provider_;
   scoped_refptr<WebKitContext> webkit_context_;
   scoped_ptr<DesktopNotificationService> desktop_notification_service_;
-  scoped_ptr<PersonalDataManager> personal_data_manager_;
+  scoped_refptr<PersonalDataManager> personal_data_manager_;
   scoped_ptr<PinnedTabService> pinned_tab_service_;
   bool history_service_created_;
   bool favicon_service_created_;
diff --git a/chrome/browser/sync/glue/autofill_data_type_controller_unittest.cc b/chrome/browser/sync/glue/autofill_data_type_controller_unittest.cc
index 854ea80..c2a4dbf 100644
--- a/chrome/browser/sync/glue/autofill_data_type_controller_unittest.cc
+++ b/chrome/browser/sync/glue/autofill_data_type_controller_unittest.cc
@@ -90,6 +90,7 @@ class AutofillDataTypeControllerTest : public testing::Test {
   virtual void SetUp() {
     db_thread_.Start();
     web_data_service_ = new WebDataServiceFake(true);
+    personal_data_manager_ = new PersonalDataManagerMock();
     autofill_dtc_ =
         new AutofillDataTypeController(&profile_sync_factory_,
                                        &profile_,
@@ -104,8 +105,8 @@ class AutofillDataTypeControllerTest : public testing::Test {
  protected:
   void SetStartExpectations() {
     EXPECT_CALL(profile_, GetPersonalDataManager()).
-        WillRepeatedly(Return(&personal_data_manager_));
-    EXPECT_CALL(personal_data_manager_, IsDataLoaded()).
+        WillRepeatedly(Return(personal_data_manager_.get()));
+    EXPECT_CALL(*(personal_data_manager_.get()), IsDataLoaded()).
         WillRepeatedly(Return(true));
     EXPECT_CALL(profile_, GetWebDataService(_)).
         WillOnce(Return(web_data_service_.get()));
@@ -149,7 +150,7 @@ class AutofillDataTypeControllerTest : public testing::Test {
   scoped_refptr<AutofillDataTypeController> autofill_dtc_;
   ProfileSyncFactoryMock profile_sync_factory_;
   ProfileMock profile_;
-  PersonalDataManagerMock personal_data_manager_;
+  scoped_refptr<PersonalDataManagerMock> personal_data_manager_;
   scoped_refptr<WebDataService> web_data_service_;
   ProfileSyncServiceMock service_;
   ModelAssociatorMock* model_associator_;
@@ -173,8 +174,8 @@ TEST_F(AutofillDataTypeControllerTest, StartPDMAndWDSReady) {
 
 TEST_F(AutofillDataTypeControllerTest, AbortWhilePDMStarting) {
   EXPECT_CALL(profile_, GetPersonalDataManager()).
-      WillRepeatedly(Return(&personal_data_manager_));
-  EXPECT_CALL(personal_data_manager_, IsDataLoaded()).
+      WillRepeatedly(Return(personal_data_manager_.get()));
+  EXPECT_CALL(*(personal_data_manager_.get()), IsDataLoaded()).
       WillRepeatedly(Return(false));
   autofill_dtc_->Start(NewCallback(&start_callback_, &StartCallback::Run));
   EXPECT_EQ(DataTypeController::MODEL_STARTING, autofill_dtc_->state());
@@ -187,8 +188,8 @@ TEST_F(AutofillDataTypeControllerTest, AbortWhilePDMStarting) {
 
 TEST_F(AutofillDataTypeControllerTest, AbortWhileWDSStarting) {
   EXPECT_CALL(profile_, GetPersonalDataManager()).
-      WillRepeatedly(Return(&personal_data_manager_));
-  EXPECT_CALL(personal_data_manager_, IsDataLoaded()).
+      WillRepeatedly(Return(personal_data_manager_.get()));
+  EXPECT_CALL(*(personal_data_manager_.get()), IsDataLoaded()).
       WillRepeatedly(Return(true));
   scoped_refptr<WebDataServiceFake> web_data_service_not_loaded =
       new WebDataServiceFake(false);
diff --git a/chrome/browser/sync/glue/autofill_model_associator.h b/chrome/browser/sync/glue/autofill_model_associator.h
index 4de5894..aa89d61 100644
--- a/chrome/browser/sync/glue/autofill_model_associator.h
+++ b/chrome/browser/sync/glue/autofill_model_associator.h
@@ -12,6 +12,7 @@
 
 #include "base/basictypes.h"
 #include "base/lock.h"
+#include "base/ref_counted.h"
 #include "base/scoped_ptr.h"
 #include "chrome/browser/autofill/personal_data_manager.h"
 #include "chrome/browser/chrome_thread.h"
@@ -62,7 +63,7 @@ class AutofillModelAssociator
       pdm_->Refresh();
     }
    private:
-    PersonalDataManager* pdm_;
+    scoped_refptr<PersonalDataManager> pdm_;
   };
 
   // PerDataTypeAssociatorInterface implementation.
diff --git a/chrome/browser/sync/profile_sync_service_autofill_unittest.cc b/chrome/browser/sync/profile_sync_service_autofill_unittest.cc
index 25225fa..3183d31 100644
--- a/chrome/browser/sync/profile_sync_service_autofill_unittest.cc
+++ b/chrome/browser/sync/profile_sync_service_autofill_unittest.cc
@@ -138,7 +138,8 @@ class ProfileSyncServiceAutofillTest : public testing::Test {
 
   virtual void SetUp() {
     web_data_service_ = new WebDataServiceFake(&web_database_);
-    personal_data_manager_.Init(&profile_);
+    personal_data_manager_ = new PersonalDataManagerMock();
+    personal_data_manager_->Init(&profile_);
     db_thread_.Start();
 
     notification_service_ = new ThreadNotificationService(&db_thread_);
@@ -165,7 +166,7 @@ class ProfileSyncServiceAutofillTest : public testing::Test {
       EXPECT_CALL(factory_, CreateAutofillSyncComponents(_, _, _, _)).
           WillOnce(MakeAutofillSyncComponents(service_.get(),
                                               &web_database_,
-                                              &personal_data_manager_,
+                                              personal_data_manager_.get(),
                                               data_type_controller));
       EXPECT_CALL(factory_, CreateDataTypeManager(_, _)).
           WillOnce(MakeDataTypeManager(&backend_));
@@ -174,9 +175,9 @@ class ProfileSyncServiceAutofillTest : public testing::Test {
           WillOnce(Return(web_data_service_.get()));
 
       EXPECT_CALL(profile_, GetPersonalDataManager()).
-          WillRepeatedly(Return(&personal_data_manager_));
+          WillRepeatedly(Return(personal_data_manager_.get()));
 
-      EXPECT_CALL(personal_data_manager_, IsDataLoaded()).
+      EXPECT_CALL(*(personal_data_manager_.get()), IsDataLoaded()).
         WillRepeatedly(Return(true));
 
       // State changes once for the backend init and once for startup done.
@@ -321,7 +322,7 @@ class ProfileSyncServiceAutofillTest : public testing::Test {
   SyncBackendHostMock backend_;
   WebDatabaseMock web_database_;
   scoped_refptr<WebDataService> web_data_service_;
-  PersonalDataManagerMock personal_data_manager_;
+  scoped_refptr<PersonalDataManagerMock> personal_data_manager_;
 
   TestIdFactory ids_;
 };
@@ -427,7 +428,7 @@ TEST_F(ProfileSyncServiceAutofillTest, HasMixedNativeEmptySync) {
   expected_profiles.push_back(*profile0);
   EXPECT_CALL(web_database_, GetAutoFillProfiles(_)).
       WillOnce(DoAll(SetArgumentPointee<0>(profiles), Return(true)));
-  EXPECT_CALL(personal_data_manager_, Refresh());
+  EXPECT_CALL(*(personal_data_manager_.get()), Refresh());
   SetIdleChangeProcessorExpectations();
   CreateAutofillRootTask task(this);
   StartSyncService(&task);
@@ -488,7 +489,7 @@ TEST_F(ProfileSyncServiceAutofillTest, HasDuplicateProfileLabelsEmptySync) {
   expected_profiles.push_back(*profile1);
   AutoFillProfile relabelled_profile;
   EXPECT_CALL(web_database_, GetAllAutofillEntries(_)).WillOnce(Return(true));
-  EXPECT_CALL(personal_data_manager_, Refresh());
+  EXPECT_CALL(*(personal_data_manager_.get()), Refresh());
   EXPECT_CALL(web_database_, GetAutoFillProfiles(_)).
       WillOnce(DoAll(SetArgumentPointee<0>(profiles), Return(true)));
   EXPECT_CALL(web_database_, UpdateAutoFillProfile(
@@ -568,7 +569,7 @@ TEST_F(ProfileSyncServiceAutofillTest, HasNativeHasSyncNoMerge) {
       WillOnce(Return(true));
   EXPECT_CALL(web_database_, AddAutoFillProfile(Eq(to_be_added))).
       WillOnce(Return(true));
-  EXPECT_CALL(personal_data_manager_, Refresh());
+  EXPECT_CALL(*(personal_data_manager_.get()), Refresh());
   StartSyncService(&task);
 
   std::set<AutofillEntry> expected_entries;
@@ -640,7 +641,7 @@ TEST_F(ProfileSyncServiceAutofillTest, HasNativeHasSyncMergeProfile) {
 
   EXPECT_CALL(web_database_, UpdateAutoFillProfile(Eq(sync_profile))).
       WillOnce(Return(true));
-  EXPECT_CALL(personal_data_manager_, Refresh());
+  EXPECT_CALL(*(personal_data_manager_.get()), Refresh());
   StartSyncService(&task);
 
   std::vector<AutofillEntry> new_sync_entries;
@@ -737,7 +738,7 @@ TEST_F(ProfileSyncServiceAutofillTest, ProcessUserChangeAddProfileConflict) {
   EXPECT_CALL(web_database_, UpdateAutoFillProfile(
       ProfileMatchesExceptLabel(added_profile))).
       WillOnce(DoAll(SaveArg<0>(&relabelled_profile), Return(true)));
-  EXPECT_CALL(personal_data_manager_, Refresh());
+  EXPECT_CALL(*(personal_data_manager_.get()), Refresh());
 
   scoped_refptr<ThreadNotifier> notifier = new ThreadNotifier(&db_thread_);
   notifier->Notify(NotificationType::AUTOFILL_PROFILE_CHANGED,
@@ -887,7 +888,7 @@ TEST_F(ProfileSyncServiceAutofillTest,
   EXPECT_CALL(web_database_, UpdateAutoFillProfile(
       ProfileMatchesExceptLabel(josephine_update))).
       WillOnce(DoAll(SaveArg<0>(&relabelled_profile), Return(true)));
-  EXPECT_CALL(personal_data_manager_, Refresh());
+  EXPECT_CALL(*(personal_data_manager_.get()), Refresh());
 
   AutofillProfileChange change(AutofillProfileChange::UPDATE,
                                josephine_update.Label(), &josephine_update,
@@ -956,7 +957,7 @@ TEST_F(ProfileSyncServiceAutofillTest, ProcessUserChangeRemoveProfile) {
   sync_profiles.push_back(sync_profile);
   AddAutofillEntriesTask task(this, sync_entries, sync_profiles);
 
-  EXPECT_CALL(personal_data_manager_, Refresh());
+  EXPECT_CALL(*(personal_data_manager_.get()), Refresh());
   StartSyncService(&task);
 
   AutofillProfileChange change(AutofillProfileChange::REMOVE,