The AutofillProfileSyncableService's lifetime should be managed by the WebDataService

chrome/browser/webdata/autofill_table.cc

Index: chrome/browser/webdata/autofill_table.cc
diff --git a/chrome/browser/webdata/autofill_table.cc b/chrome/browser/webdata/autofill_table.cc
index fd152e1e7b0bf079c692ed4ff78bf80f226203a2..d288ef0382cc8c7bede04a20087c37025a84b8af 100644
--- a/chrome/browser/webdata/autofill_table.cc
+++ b/chrome/browser/webdata/autofill_table.cc
@@ -22,6 +22,7 @@
 #include "chrome/browser/autofill/credit_card.h"
 #include "chrome/browser/autofill/personal_data_manager.h"
 #include "chrome/browser/password_manager/encryptor.h"
+#include "chrome/browser/webdata/autofill_profile_syncable_service.h"
 #include "chrome/browser/webdata/autofill_change.h"
 #include "chrome/common/guid.h"
 #include "sql/statement.h"
@@ -346,6 +347,15 @@ bool RemoveAutofillProfilePieces(const std::string& guid, sql::Connection* db) {
 
 }  // namespace
 
+
+AutofillTable::AutofillTable(sql::Connection* db, sql::MetaTable* meta_table)
+    : WebDatabaseTable(db, meta_table),
+      autofill_profile_syncable_service_(NULL) {
+}
+
+AutofillTable::~AutofillTable() {
+}
+
 bool AutofillTable::Init() {
  return (InitMainTable() && InitCreditCardsTable() && InitDatesTable() &&
      InitProfilesTable() && InitProfileNamesTable() &&
@@ -357,6 +367,19 @@ bool AutofillTable::IsSyncable() {
   return true;
 }
 
+AutofillProfileSyncableService* AutofillTable::GetSyncableService(
+    WebDatabase* web_database,
+    Profile* profile) {
+  if (!autofill_profile_syncable_service_.get()) {
+    autofill_profile_syncable_service_.reset(
+        new AutofillProfileSyncableService(web_database, profile));
+  }
+
+  DCHECK_EQ(web_database, autofill_profile_syncable_service_->web_database());

[akalin, 2011/10/10 21:58:57]

this seems kinda gross.  It seems to me that WebDatabase or WebDataService would
be better owners of the syncable service, if this is needed.

[Ilya Sherman, 2011/10/10 22:33:13]

This would still be "needed" even if the WebDatabase were the owner -- it would
just become

DCHECK_EQ(this, autofill_profile_syncable_service_->web_database());

As far as I understand, GetSyncableService() should be called exactly once per
launch of a Profile.  That's essentially what these DCHECKs are enforcing,
though they allow a bit more flexibility.  If the additional flexibility is
undesirable, we could just

DCHECK(!autofill_profile_syncable_service_.get());

at the top of the function, and get rid of the if-stmt.

+  DCHECK_EQ(profile, autofill_profile_syncable_service_->profile());
+  return autofill_profile_syncable_service_.get();
+}
+
 bool AutofillTable::AddFormFieldValues(const std::vector<FormField>& elements,
                                        std::vector<AutofillChange>* changes) {
   return AddFormFieldValuesTime(elements, changes, Time::Now());