Remove extended attributes. The lame broken codepath in the DBS was causing

chrome/browser/sync/syncable/directory_backing_store.cc

Index: chrome/browser/sync/syncable/directory_backing_store.cc
diff --git a/chrome/browser/sync/syncable/directory_backing_store.cc b/chrome/browser/sync/syncable/directory_backing_store.cc
index 2a2978986804e2ddba0c06996c47f6889a850665..63df81d0a4e4a847a4f2535ce20dab3ad1395f81 100644
--- a/chrome/browser/sync/syncable/directory_backing_store.cc
+++ b/chrome/browser/sync/syncable/directory_backing_store.cc
@@ -40,7 +40,7 @@ static const string::size_type kUpdateStatementBufferSize = 2048;
 
 // Increment this version whenever updating DB tables.
 extern const int32 kCurrentDBVersion;  // Global visibility for our unittest.
-const int32 kCurrentDBVersion = 71;
+const int32 kCurrentDBVersion = 72;
 
 namespace {
 
@@ -215,7 +215,6 @@ bool DirectoryBackingStore::OpenAndConfigureHandleHelper(
 }
 
 DirOpenResult DirectoryBackingStore::Load(MetahandlesIndex* entry_bucket,
-    ExtendedAttributes* xattrs_bucket,
     Directory::KernelLoadInfo* kernel_load_info) {
   if (!BeginLoad())
     return FAILED_OPEN_DATABASE;
@@ -226,7 +225,6 @@ DirOpenResult DirectoryBackingStore::Load(MetahandlesIndex* entry_bucket,
 
   if (!DropDeletedEntries() ||
       !LoadEntries(entry_bucket) ||
-      !LoadExtendedAttributes(xattrs_bucket) ||
       !LoadInfo(kernel_load_info)) {
     return FAILED_DATABASE_CORRUPT;
   }
@@ -261,8 +259,7 @@ bool DirectoryBackingStore::SaveChanges(
   // just stop here if there's nothing to save.
   bool save_info =
     (Directory::KERNEL_SHARE_INFO_DIRTY == snapshot.kernel_info_status);
-  if (snapshot.dirty_metas.size() < 1 && snapshot.dirty_xattrs.size() < 1 &&
-      !save_info)
+  if (snapshot.dirty_metas.size() < 1 && !save_info)
     return true;
 
   SQLTransaction transaction(dbhandle);
@@ -276,18 +273,6 @@ bool DirectoryBackingStore::SaveChanges(
       return false;
   }
 
-  for (ExtendedAttributes::const_iterator i = snapshot.dirty_xattrs.begin();
-       i != snapshot.dirty_xattrs.end(); ++i) {
-    DCHECK(i->second.dirty);
-    if (i->second.is_deleted) {
-      if (!DeleteExtendedAttributeFromDB(i))
-        return false;
-    } else {
-      if (!SaveExtendedAttributeToDB(i))
-        return false;
-    }
-  }
-
   if (save_info) {
     const Directory::PersistedKernelInfo& info = snapshot.kernel_info;
     SQLStatement update;
@@ -355,6 +340,13 @@ DirOpenResult DirectoryBackingStore::InitializeTables() {
       version_on_disk = 71;
   }
 
+  // Version 72 removed extended attributes, a legacy way to do extensible
+  // key/value information, stored in their own table.
+  if (version_on_disk == 71) {
+    if (MigrateVersion71To72())
+      version_on_disk = 72;
+  }
+
   // If one of the migrations requested it, drop columns that aren't current.
   // It's only safe to do this after migrating all the way to the current
   // version.
@@ -450,31 +442,6 @@ bool DirectoryBackingStore::LoadEntries(MetahandlesIndex* entry_bucket) {
   return SQLITE_DONE == query_result;
 }
 
-bool DirectoryBackingStore::LoadExtendedAttributes(
-    ExtendedAttributes* xattrs_bucket) {
-  SQLStatement statement;
-  statement.prepare(
-      load_dbhandle_,
-      "SELECT metahandle, key, value FROM extended_attributes");
-  int step_result = statement.step();
-  while (SQLITE_ROW == step_result) {
-    int64 metahandle = statement.column_int64(0);
-
-    string path_string_key;
-    statement.column_string(1, &path_string_key);
-
-    ExtendedAttributeValue val;
-    statement.column_blob_as_vector(2, &(val.value));
-    val.is_deleted = false;
-
-    ExtendedAttributeKey key(metahandle, path_string_key);
-    xattrs_bucket->insert(std::make_pair(key, val));
-    step_result = statement.step();
-  }
-
-  return SQLITE_DONE == step_result;
-}
-
 bool DirectoryBackingStore::LoadInfo(Directory::KernelLoadInfo* info) {
   {
     SQLStatement query;
@@ -542,49 +509,7 @@ bool DirectoryBackingStore::SaveEntryToDB(const EntryKernel& entry) {
           1 == statement.changes());
 }
 
-bool DirectoryBackingStore::SaveExtendedAttributeToDB(
-    ExtendedAttributes::const_iterator i) {
-  DCHECK(save_dbhandle_);
-  SQLStatement insert;
-  insert.prepare(save_dbhandle_,
-                 "INSERT INTO extended_attributes "
-                 "(metahandle, key, value) "
-                 "values ( ?, ?, ? )");
-  insert.bind_int64(0, i->first.metahandle);
-  insert.bind_string(1, i->first.key);
-  insert.bind_blob(2, &i->second.value.at(0), i->second.value.size());
-  return (SQLITE_DONE == insert.step() &&
-          SQLITE_OK == insert.reset() &&
-          1 == insert.changes());
-}
-
-bool DirectoryBackingStore::DeleteExtendedAttributeFromDB(
-    ExtendedAttributes::const_iterator i) {
-  DCHECK(save_dbhandle_);
-  SQLStatement delete_attribute;
-  delete_attribute.prepare(save_dbhandle_,
-                           "DELETE FROM extended_attributes "
-                           "WHERE metahandle = ? AND key = ? ");
-  delete_attribute.bind_int64(0, i->first.metahandle);
-  delete_attribute.bind_string(1, i->first.key);
-  if (!(SQLITE_DONE == delete_attribute.step() &&
-        SQLITE_OK == delete_attribute.reset() &&
-        1 == delete_attribute.changes())) {
-    LOG(ERROR) << "DeleteExtendedAttributeFromDB(),StepDone() failed "
-        << "for metahandle: " << i->first.metahandle << " key: "
-        << i->first.key;
-    return false;
-  }
-  // The attribute may have never been saved to the database if it was
-  // created and then immediately deleted.  So don't check that we
-  // deleted exactly 1 row.
-  return true;
-}
-
 bool DirectoryBackingStore::DropDeletedEntries() {
-  static const char delete_extended_attributes[] =
-      "DELETE FROM extended_attributes WHERE metahandle IN "
-      "(SELECT metahandle from death_row)";
   static const char delete_metas[] = "DELETE FROM metas WHERE metahandle IN "
                                      "(SELECT metahandle from death_row)";
   // Put all statements into a transaction for better performance
@@ -602,9 +527,6 @@ bool DirectoryBackingStore::DropDeletedEntries() {
                                " AND is_unapplied_update < 1")) {
     return false;
   }
-  if (SQLITE_DONE != ExecQuery(load_dbhandle_, delete_extended_attributes)) {
-    return false;
-  }
   if (SQLITE_DONE != ExecQuery(load_dbhandle_, delete_metas)) {
     return false;
   }
@@ -630,19 +552,6 @@ int DirectoryBackingStore::SafeDropTable(const char* table_name) {
   return result;
 }
 
-int DirectoryBackingStore::CreateExtendedAttributeTable() {
-  int result = SafeDropTable("extended_attributes");
-  if (result != SQLITE_DONE)
-    return result;
-  LOG(INFO) << "CreateExtendedAttributeTable";
-  return ExecQuery(load_dbhandle_,
-                   "CREATE TABLE extended_attributes("
-                   "metahandle bigint, "
-                   "key varchar(127), "
-                   "value blob, "
-                   "PRIMARY KEY(metahandle, key) ON CONFLICT REPLACE)");
-}
-
 void DirectoryBackingStore::DropAllTables() {
   SafeDropTable("metas");
   SafeDropTable("temp_metas");
@@ -889,6 +798,12 @@ bool DirectoryBackingStore::MigrateVersion70To71() {
   return true;
 }
 
+bool DirectoryBackingStore::MigrateVersion71To72() {
+  SafeDropTable("extended_attributes");
+  SetVersion(72);
+  return true;
+}
+
 int DirectoryBackingStore::CreateTables() {
   LOG(INFO) << "First run, creating tables";
   // Create two little tables share_version and share_info
@@ -950,9 +865,6 @@ int DirectoryBackingStore::CreateTables() {
     statement.bind_int64(1, now);
     result = statement.step();
   }
-  if (result != SQLITE_DONE)
-    return result;
-  result = CreateExtendedAttributeTable();
   return result;
 }