Delete empty dom storage databases on destruction.

webkit/dom_storage/dom_storage_database.cc

Index: webkit/dom_storage/dom_storage_database.cc
diff --git a/webkit/dom_storage/dom_storage_database.cc b/webkit/dom_storage/dom_storage_database.cc
index 673ddde5380433e9b6d246b2799765cbef82a548..2c71bc7e454f2833234e3ba781f4e0477e60c4f7 100644
--- a/webkit/dom_storage/dom_storage_database.cc
+++ b/webkit/dom_storage/dom_storage_database.cc
@@ -30,7 +30,8 @@ DomStorageDatabase::DomStorageDatabase(const FilePath& file_path)
     : file_path_(file_path),
       db_(NULL),
       failed_to_open_(false),
-      tried_to_recreate_(false) {
+      tried_to_recreate_(false),
+      known_to_be_empty_(false) {
   // Note: in normal use we should never get an empty backing path here.
   // However, the unit test for this class defines another constructor
   // that will bypass this check to allow an empty path that signifies
@@ -40,6 +41,11 @@ DomStorageDatabase::DomStorageDatabase(const FilePath& file_path)
 }
 
 DomStorageDatabase::~DomStorageDatabase() {
+  if (known_to_be_empty_ && !file_path_.empty()) {
+    // Delete the db from disk, it's empty.
+    Close();
+    file_util::Delete(file_path_, false);
+  }
 }
 
 void DomStorageDatabase::ReadAllValues(ValuesMap* result) {
@@ -56,6 +62,7 @@ void DomStorageDatabase::ReadAllValues(ValuesMap* result) {
     statement.ColumnBlobAsString16(1, &value);
     (*result)[key] = NullableString16(value, false);
   }
+
 }
 
 bool DomStorageDatabase::CommitChanges(bool clear_all_first,
@@ -70,8 +77,11 @@ bool DomStorageDatabase::CommitChanges(bool clear_all_first,
   if (clear_all_first) {
     if (!db_->Execute("DELETE FROM ItemTable"))
       return false;
+    known_to_be_empty_ = true;
   }
 
+  bool did_delete = false;
+  bool did_insert = false;
   ValuesMap::const_iterator it = changes.begin();
   for(; it != changes.end(); ++it) {
     sql::Statement statement;
@@ -81,16 +91,28 @@ bool DomStorageDatabase::CommitChanges(bool clear_all_first,
       statement.Assign(db_->GetCachedStatement(SQL_FROM_HERE,
          "DELETE FROM ItemTable WHERE key=?"));
       statement.BindString16(0, key);
+      did_delete = true;
     } else {
       statement.Assign(db_->GetCachedStatement(SQL_FROM_HERE,
           "INSERT INTO ItemTable VALUES (?,?)"));
       statement.BindString16(0, key);
       statement.BindBlob(1, value.string().data(),
                          value.string().length() * sizeof(char16));
+      known_to_be_empty_ = false;
+      did_insert = true;
     }
     DCHECK(statement.is_valid());
     statement.Run();
   }
+
+  if (!known_to_be_empty_ && did_delete && !did_insert) {
+    // See if the database is empty.
+    sql::Statement statement(db_->GetCachedStatement(SQL_FROM_HERE,
+        "SELECT count(key) from ItemTable"));
+    if (statement.Step())
+      known_to_be_empty_ = statement.ColumnInt(0) == 0;
+  }
+
   return transaction.Commit();

[benm (inactive), 2012/02/27 12:19:24]

hm, if the transaction fails, then we need to switch back to the previous value
of known_to_be_empty_...

 }
 
@@ -149,18 +171,22 @@ bool DomStorageDatabase::LazyOpen(bool create_if_needed) {
     // and whether it's usable (i.e. not corrupted).
     SchemaVersion current_version = DetectSchemaVersion();
 
+    bool valid_database = false;
     if (current_version == V2) {
-      return true;
+      valid_database = true;
     } else if (current_version == V1) {
       if (UpgradeVersion1To2())
-        return true;
+        valid_database = true;
+    }
+
+    if (!valid_database) {
+      // Try and recover by attempting to delete the file and start again.
+      Close();
+      return DeleteFileAndRecreate();
     }
   }
 
-  // This is the exceptional case - to try and recover we'll attempt
-  // to delete the file and start again.
-  Close();
-  return DeleteFileAndRecreate();
+  return true;
 }
 
 DomStorageDatabase::SchemaVersion DomStorageDatabase::DetectSchemaVersion() {