Make iterating over a corrupted IndexedDB fail.

content/browser/indexed_db/indexed_db_factory.cc

 // Copyright (c) 2013 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 "content/browser/indexed_db/indexed_db_factory.h"
 
 #include "base/logging.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "content/browser/indexed_db/indexed_db_backing_store.h"
@@ skipping 173 matching lines @@
                        &data_loss_message,
                        &disk_full);
   if (!backing_store) {
     callbacks->OnError(
         IndexedDBDatabaseError(blink::WebIDBDatabaseExceptionUnknownError,
                                "Internal error opening backing store for "
                                "indexedDB.webkitGetDatabaseNames."));
     return;
   }
 
-  callbacks->OnSuccess(backing_store->GetDatabaseNames());
+  leveldb::Status s;
+  std::vector<base::string16> names = backing_store->GetDatabaseNames(s);
+  if (s.ok()) {

[jsbell, 2014/04/14 20:44:20]

I'd reduce this to just an !s.ok() case for now, since OnSuccess is called
either way.

[cmumford, 2014/04/14 23:39:23]

Done.

+    callbacks->OnSuccess(names);
+  } else {
+    // TODO(cmumford): Handle this error
+    DLOG(ERROR) << "Internal error getting database names";
+    callbacks->OnSuccess(names);
+  }
   backing_store = NULL;
   ReleaseBackingStore(origin_url, false /* immediate */);
 }
 
 void IndexedDBFactory::DeleteDatabase(
     const base::string16& name,
     scoped_refptr<IndexedDBCallbacks> callbacks,
     const GURL& origin_url,
     const base::FilePath& data_directory) {
   IDB_TRACE("IndexedDBFactory::DeleteDatabase");
@@ skipping 18 matching lines @@
                        &disk_full);
   if (!backing_store) {
     callbacks->OnError(
         IndexedDBDatabaseError(blink::WebIDBDatabaseExceptionUnknownError,
                                ASCIIToUTF16(
                                    "Internal error opening backing store "
                                    "for indexedDB.deleteDatabase.")));
     return;
   }
 
-  scoped_refptr<IndexedDBDatabase> database =
-      IndexedDBDatabase::Create(name, backing_store, this, unique_identifier);
+  leveldb::Status s;
+  scoped_refptr<IndexedDBDatabase> database = IndexedDBDatabase::Create(
+      name, backing_store, this, unique_identifier, s);
   if (!database) {
-    callbacks->OnError(IndexedDBDatabaseError(
+    IndexedDBDatabaseError error(
         blink::WebIDBDatabaseExceptionUnknownError,
         ASCIIToUTF16(
             "Internal error creating database backend for "
-            "indexedDB.deleteDatabase.")));
+            "indexedDB.deleteDatabase."));
+    callbacks->OnError(error);
+    if (s.IsCorruption()) {
+      backing_store = NULL;  // Closes the LevelDB so that it can be deleted

[jsbell, 2014/04/14 20:44:20]

Does it actually close, or just unblock closing?

[cmumford, 2014/04/14 23:39:23]

If I'm reading that correctly that results in the leveldb::DBImpl instance being
deleted which is the equivalent of closing the LevelDB database.

That being said that NULL ptr assignment isn't strictly related to this bug so I
am going to take it out. I have three other changes related to specific API
failures and that change leaked into this CL from one of the others.

+      HandleBackingStoreCorruption(origin_url, error);
+    }
     return;
   }
 
   database_map_[unique_identifier] = database;
   origin_dbs_.insert(std::make_pair(origin_url, database));
   database->DeleteDatabase(callbacks);
   RemoveDatabaseFromMaps(unique_identifier);
   database = NULL;
   backing_store = NULL;
   ReleaseBackingStore(origin_url, false /* immediate */);
@@ skipping 21 matching lines @@
   // Make a copy of origin_url as this is likely a reference to a member of a
   // backing store which this function will be deleting.
   GURL saved_origin_url(origin_url);
   DCHECK(context_);
   base::FilePath path_base = context_->data_path();
   IndexedDBBackingStore::RecordCorruptionInfo(
       path_base, saved_origin_url, base::UTF16ToUTF8(error.message()));
   HandleBackingStoreFailure(saved_origin_url);
   // Note: DestroyBackingStore only deletes LevelDB files, leaving all others,
   //       so our corruption info file will remain.
-  if (!IndexedDBBackingStore::DestroyBackingStore(path_base, saved_origin_url)
-           .ok())
-    DLOG(ERROR) << "Unable to delete backing store";
+  leveldb::Status s =
+      IndexedDBBackingStore::DestroyBackingStore(path_base, saved_origin_url);
+  if (!s.ok())
+    DLOG(ERROR) << "Unable to delete backing store: " << s.ToString();
 }
 
 bool IndexedDBFactory::IsDatabaseOpen(const GURL& origin_url,
                                       const base::string16& name) const {
 
   return !!database_map_.count(IndexedDBDatabase::Identifier(origin_url, name));
 }
 
 bool IndexedDBFactory::IsBackingStoreOpen(const GURL& origin_url) const {
   return backing_store_map_.find(origin_url) != backing_store_map_.end();
@@ skipping 81 matching lines @@
                                        "backing store for indexedDB.open.")));
         return;
       }
       connection.callbacks->OnError(IndexedDBDatabaseError(
           blink::WebIDBDatabaseExceptionUnknownError,
           ASCIIToUTF16(
               "Internal error opening backing store for indexedDB.open.")));
       return;
     }
 
-    database =
-        IndexedDBDatabase::Create(name, backing_store, this, unique_identifier);
+    leveldb::Status s;
+    database = IndexedDBDatabase::Create(
+        name, backing_store, this, unique_identifier, s);
     if (!database) {
-      connection.callbacks->OnError(IndexedDBDatabaseError(
-          blink::WebIDBDatabaseExceptionUnknownError,
-          ASCIIToUTF16(
-              "Internal error creating database backend for indexedDB.open.")));
+      DLOG(ERROR) << "Unable to create the database";
+      IndexedDBDatabaseError error(blink::WebIDBDatabaseExceptionUnknownError,
+                                   ASCIIToUTF16(
+                                       "Internal error creating "
+                                       "database backend for "
+                                       "indexedDB.open."));
+      connection.callbacks->OnError(error);
+      if (s.IsCorruption()) {
+        backing_store = NULL;  // Closes the LevelDB so that it can be deleted
+        HandleBackingStoreCorruption(origin_url, error);
+      }
       return;
     }
   } else {
     database = it->second;
   }
 
   if (data_loss != blink::WebIDBDataLossNone)
     connection.callbacks->OnDataLoss(data_loss, data_loss_message);
 
   database->OpenConnection(connection);
@@ skipping 15 matching lines @@
 
   std::pair<OriginDBMapIterator, OriginDBMapIterator> range =
       GetOpenDatabasesForOrigin(origin_url);
   for (OriginDBMapIterator it = range.first; it != range.second; ++it)
     count += it->second->ConnectionCount();
 
   return count;
 }
 
 }  // namespace content