The AutofillProfileSyncableService's lifetime should be managed by the WebDataService

chrome/browser/webdata/autofill_table.cc

 // 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.
 
 #include "chrome/browser/webdata/autofill_table.h"
 
 #include <algorithm>
 #include <limits>
 #include <map>
 #include <set>
 #include <string>
 #include <vector>
 
 #include "base/i18n/case_conversion.h"
 #include "base/logging.h"
 #include "base/string_number_conversions.h"
 #include "base/time.h"
 #include "base/tuple.h"
 #include "chrome/browser/autofill/autofill_country.h"
 #include "chrome/browser/autofill/autofill_profile.h"
 #include "chrome/browser/autofill/autofill_type.h"
 #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"
 #include "ui/base/l10n/l10n_util.h"
 #include "webkit/glue/form_field.h"
 
 using base::Time;
 using webkit_glue::FormField;
 
 namespace {
@@ skipping 304 matching lines @@
     NOTREACHED() << "Statement prepare failed";
     return false;
   }
 
   s3.BindString(0, guid);
   return s3.Run();
 }
 
 }  // 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() &&
      InitProfileEmailsTable() && InitProfilePhonesTable() &&
      InitProfileTrashTable());
 }
 
 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());
 }
 
 bool AutofillTable::AddFormFieldValue(const FormField& element,
                                       std::vector<AutofillChange>* changes) {
   return AddFormFieldValueTime(element, changes, Time::Now());
 }
 
@@ skipping 1731 matching lines @@
         "UPDATE autofill_profiles SET date_modified=? "
         "WHERE guid=?"));
     s_date.BindInt64(0, date_item->second);
     s_date.BindString(1, iter->guid());
     if (!s_date.Run())
       return false;
   }
 
   return true;
 }