The AutofillProfileSyncableService's lifetime should be managed by the WebDataService

BUG=96922,96363
TEST=sh tools/valgrind/chrome_tests.sh -t unit_tests --gtest_filter=ProfileSyncServiceAutofillTest.HasProfileEmptySync


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

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

5 files changed, 53 insertions, 15 deletions

diff --git a/chrome/browser/webdata/autofill_profile_syncable_service.cc b/chrome/browser/webdata/autofill_profile_syncable_service.cc
index 8dfe601..75db0fd 100644
--- a/chrome/browser/webdata/autofill_profile_syncable_service.cc
+++ b/chrome/browser/webdata/autofill_profile_syncable_service.cc
@@ -34,8 +34,6 @@ bool MergeField(FormGroup* form_group,
 
 }  // namespace
 
-namespace browser_sync {
-
 const char kAutofillProfileTag[] = "google_chrome_autofill_profiles";
 
 AutofillProfileSyncableService::AutofillProfileSyncableService(
@@ -56,6 +54,7 @@ AutofillProfileSyncableService::~AutofillProfileSyncableService() {
 AutofillProfileSyncableService::AutofillProfileSyncableService()
     : web_data_service_(NULL),
       sync_processor_(NULL) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
 }
 
 SyncError AutofillProfileSyncableService::MergeDataAndStartSyncing(
@@ -408,5 +407,3 @@ AutofillProfileSyncableService::DataBundle::DataBundle() {}
 
 AutofillProfileSyncableService::DataBundle::~DataBundle() {
 }
-
-}  // namespace browser_sync
diff --git a/chrome/browser/webdata/autofill_profile_syncable_service.h b/chrome/browser/webdata/autofill_profile_syncable_service.h
index 77ef314..362f03a 100644
--- a/chrome/browser/webdata/autofill_profile_syncable_service.h
+++ b/chrome/browser/webdata/autofill_profile_syncable_service.h
@@ -30,11 +30,11 @@ class ProfileSyncServiceAutofillTest;
 class WebDataService;
 
 namespace browser_sync {
+class UnrecoverableErrorHandler;
+}
 
 extern const char kAutofillProfileTag[];
 
-class UnrecoverableErrorHandler;
-
 // The sync implementation for AutofillProfiles.
 // MergeDataAndStartSyncing() called first, it does cloud->local and
 // local->cloud syncs. Then for each cloud change we receive
@@ -83,7 +83,7 @@ class AutofillProfileSyncableService
   virtual bool SaveChangesToWebData(const DataBundle& bundle);
 
  private:
-  friend class ::ProfileSyncServiceAutofillTest;
+  friend class ProfileSyncServiceAutofillTest;
   friend class MockAutofillProfileSyncableService;
   FRIEND_TEST_ALL_PREFIXES(AutofillProfileSyncableServiceTest,
                            MergeDataAndStartSyncing);
@@ -156,6 +156,4 @@ struct AutofillProfileSyncableService::DataBundle {
   std::vector<AutofillProfile*> profiles_to_add;
 };
 
-}  // namespace browser_sync
-
 #endif  // CHROME_BROWSER_WEBDATA_AUTOFILL_PROFILE_SYNCABLE_SERVICE_H_
diff --git a/chrome/browser/webdata/autofill_profile_syncable_service_unittest.cc b/chrome/browser/webdata/autofill_profile_syncable_service_unittest.cc
index 90cddb0..5639c66 100644
--- a/chrome/browser/webdata/autofill_profile_syncable_service_unittest.cc
+++ b/chrome/browser/webdata/autofill_profile_syncable_service_unittest.cc
@@ -22,10 +22,8 @@ using ::testing::Return;
 using ::testing::Property;
 class AutofillProfile;
 
-namespace browser_sync {
-
 class MockAutofillProfileSyncableService
-    : public browser_sync::AutofillProfileSyncableService {
+    : public AutofillProfileSyncableService {
  public:
   MockAutofillProfileSyncableService() {
   }
@@ -262,5 +260,3 @@ TEST_F(AutofillProfileSyncableServiceTest, ActOnChange) {
   autofill_syncable_service_.ActOnChange(change1);
   autofill_syncable_service_.ActOnChange(change2);
 }
-
-}  // namespace browser_sync
diff --git a/chrome/browser/webdata/web_data_service.cc b/chrome/browser/webdata/web_data_service.cc
index 80fae632..8f5bf93 100644
--- a/chrome/browser/webdata/web_data_service.cc
+++ b/chrome/browser/webdata/web_data_service.cc
@@ -15,6 +15,7 @@
 #include "chrome/browser/ui/profile_error_dialog.h"
 #include "chrome/browser/webdata/autofill_change.h"
 #include "chrome/browser/webdata/autofill_entry.h"
+#include "chrome/browser/webdata/autofill_profile_syncable_service.h"
 #include "chrome/browser/webdata/autofill_table.h"
 #include "chrome/browser/webdata/keyword_table.h"
 #include "chrome/browser/webdata/logins_table.h"
@@ -74,6 +75,7 @@ WDKeywordsResult::~WDKeywordsResult() {}
 WebDataService::WebDataService()
   : is_running_(false),
     db_(NULL),
+    autofill_profile_syncable_service_(NULL),
     failed_init_(false),
     should_commit_(false),
     next_request_handle_(1),
@@ -101,6 +103,7 @@ bool WebDataService::Init(const FilePath& profile_path) {
 }
 
 void WebDataService::Shutdown() {
+  ScheduleTask(Bind(&WebDataService::ShutdownSyncableServices, this));
   UnloadDatabase();
 }
 
@@ -538,6 +541,7 @@ bool WebDataService::InitWithPath(const FilePath& path) {
   path_ = path;
   is_running_ = true;
   ScheduleTask(Bind(&WebDataService::InitializeDatabaseIfNecessary, this));
+  ScheduleTask(Bind(&WebDataService::InitializeSyncableServices, this));
   return true;
 }
 
@@ -628,6 +632,13 @@ void WebDataService::InitializeDatabaseIfNecessary() {
   db_->BeginTransaction();
 }
 
+void WebDataService::InitializeSyncableServices() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
+  DCHECK(!autofill_profile_syncable_service_);
+
+  autofill_profile_syncable_service_ = new AutofillProfileSyncableService(this);
+}
+
 void WebDataService::NotifyDatabaseLoadedOnUIThread() {
   // Notify that the database has been initialized.
   NotificationService::current()->Notify(
@@ -645,6 +656,13 @@ void WebDataService::ShutdownDatabase() {
   }
 }
 
+void WebDataService::ShutdownSyncableServices() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
+
+  delete autofill_profile_syncable_service_;
+  autofill_profile_syncable_service_ = NULL;
+}
+
 void WebDataService::Commit() {
   if (should_commit_) {
     should_commit_ = false;
@@ -1360,6 +1378,14 @@ void WebDataService::RemoveAutofillProfilesAndCreditCardsModifiedBetweenImpl(
   request->RequestComplete();
 }
 
+AutofillProfileSyncableService*
+    WebDataService::GetAutofillProfileSyncableService() const {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
+  DCHECK(autofill_profile_syncable_service_);  // Make sure we're initialized.
+
+  return autofill_profile_syncable_service_;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 //
 // WebDataRequest implementation.
diff --git a/chrome/browser/webdata/web_data_service.h b/chrome/browser/webdata/web_data_service.h
index 98c5d0a..f40ad00 100644
--- a/chrome/browser/webdata/web_data_service.h
+++ b/chrome/browser/webdata/web_data_service.h
@@ -20,6 +20,7 @@
 
 class AutofillChange;
 class AutofillProfile;
+class AutofillProfileSyncableService;
 class CreditCard;
 class GURL;
 #if defined(OS_WIN)
@@ -512,6 +513,12 @@ class WebDataService
       const base::Time& delete_begin,
       const base::Time& delete_end);
 
+  // TODO(georgey): Add support for autocomplete as well: http://crbug.com/95759
+  // Returns the syncable service for Autofill addresses and credit cards stored
+  // in this table. The returned service is owned by |this| object.
+  virtual AutofillProfileSyncableService*
+      GetAutofillProfileSyncableService() const;
+
   // Testing
 #ifdef UNIT_TEST
   void set_failed_init(bool value) { failed_init_ = value; }
@@ -553,12 +560,18 @@ class WebDataService
   // Initialize the database, if it hasn't already been initialized.
   void InitializeDatabaseIfNecessary();
 
+  // Initialize any syncable services.
+  void InitializeSyncableServices();
+
   // The notification method.
   void NotifyDatabaseLoadedOnUIThread();
 
   // Commit any pending transaction and deletes the database.
   void ShutdownDatabase();
 
+  // Deletes the syncable services.
+  void ShutdownSyncableServices();
+
   // Commit the current transaction and creates a new one.
   void Commit();
 
@@ -664,9 +677,17 @@ class WebDataService
   // The path with which to initialize the database.
   FilePath path_;
 
-  // Our database.
+  // Our database.  We own the |db_|, but don't use a |scoped_ptr| because the
+  // |db_| lifetime must be managed on the database thread.
   WebDatabase* db_;
 
+  // Syncable services for the database data.  We own the services, but don't
+  // use |scoped_ptr|s because the lifetimes must be managed on the database
+  // thread.
+  // Currently only Autofill profiles (and credit cards) use the new Sync API,
+  // but all the database data should migrate to this API over time.
+  AutofillProfileSyncableService* autofill_profile_syncable_service_;
+
   // Whether the database failed to initialize.  We use this to avoid
   // continually trying to reinit.
   bool failed_init_;