Make iterating over a corrupted IndexedDB fail.

content/browser/indexed_db/indexed_db_database.cc

Index: content/browser/indexed_db/indexed_db_database.cc
diff --git a/content/browser/indexed_db/indexed_db_database.cc b/content/browser/indexed_db/indexed_db_database.cc
index c97048b5612094eb54796602bc067543d9ac3515..50090c64d99e3c98aeb7c48cf9bf82ffb471e784 100644
--- a/content/browser/indexed_db/indexed_db_database.cc
+++ b/content/browser/indexed_db/indexed_db_database.cc
@@ -92,12 +92,12 @@ scoped_refptr<IndexedDBDatabase> IndexedDBDatabase::Create(
     const base::string16& name,
     IndexedDBBackingStore* backing_store,
     IndexedDBFactory* factory,
-    const Identifier& unique_identifier) {
+    const Identifier& unique_identifier,
+    leveldb::Status& s) {
   scoped_refptr<IndexedDBDatabase> database =
       new IndexedDBDatabase(name, backing_store, factory, unique_identifier);
-  if (!database->OpenInternal().ok())
-    return 0;
-  return database;
+  s = database->OpenInternal();
+  return s.ok() ? database : 0;

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

Does this compile everywhere? The trinary operator is a bit picky if the
arguments aren't obviously the same type.

Also, can you make it NULL? (0 is webkit-ism, probably a result of copy/pasting
this code)

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

I didn't compile it everywhere. I'll be safe and go back to the prior form, but
with NULL.

 }
 
 namespace {
@@ -527,6 +527,7 @@ void IndexedDBDatabase::GetOperation(
 
   const IndexedDBKey* key;
 
+  leveldb::Status s;
   scoped_ptr<IndexedDBBackingStore::Cursor> backing_store_cursor;
   if (key_range->IsOnlyKey()) {
     key = &key_range->lower();
@@ -539,7 +540,8 @@ void IndexedDBDatabase::GetOperation(
           id(),
           object_store_id,
           *key_range,
-          indexed_db::CURSOR_NEXT);
+          indexed_db::CURSOR_NEXT,
+          s);
     } else if (cursor_type == indexed_db::CURSOR_KEY_ONLY) {
       // Index Value Retrieval Operation
       backing_store_cursor = backing_store_->OpenIndexKeyCursor(
@@ -548,7 +550,8 @@ void IndexedDBDatabase::GetOperation(
           object_store_id,
           index_id,
           *key_range,
-          indexed_db::CURSOR_NEXT);
+          indexed_db::CURSOR_NEXT,
+          s);
     } else {
       // Index Referenced Value Retrieval Operation
       backing_store_cursor = backing_store_->OpenIndexCursor(
@@ -557,7 +560,18 @@ void IndexedDBDatabase::GetOperation(
           object_store_id,
           index_id,
           *key_range,
-          indexed_db::CURSOR_NEXT);
+          indexed_db::CURSOR_NEXT,
+          s);
+    }
+
+    if (!s.ok()) {
+      DLOG(ERROR) << "Unable to open cursor operation: " << s.ToString();
+      IndexedDBDatabaseError error(blink::WebIDBDatabaseExceptionUnknownError,
+                                   "Internal error deleting data in range");
+      if (s.IsCorruption()) {
+        factory_->HandleBackingStoreCorruption(backing_store_->origin_url(),
+                                               error);
+      }
     }
 
     if (!backing_store_cursor) {
@@ -569,7 +583,6 @@ void IndexedDBDatabase::GetOperation(
   }
 
   scoped_ptr<IndexedDBKey> primary_key;
-  leveldb::Status s;
   if (index_id == IndexedDBIndexMetadata::kInvalidId) {
     // Object Store Retrieval Operation
     IndexedDBValue value;
@@ -1030,6 +1043,7 @@ void IndexedDBDatabase::OpenCursorOperation(
   if (params->task_type == IndexedDBDatabase::PREEMPTIVE_TASK)
     transaction->AddPreemptiveEvent();
 
+  leveldb::Status s;
   scoped_ptr<IndexedDBBackingStore::Cursor> backing_store_cursor;
   if (params->index_id == IndexedDBIndexMetadata::kInvalidId) {
     if (params->cursor_type == indexed_db::CURSOR_KEY_ONLY) {
@@ -1039,14 +1053,16 @@ void IndexedDBDatabase::OpenCursorOperation(
           id(),
           params->object_store_id,
           *params->key_range,
-          params->direction);
+          params->direction,
+          s);
     } else {
       backing_store_cursor = backing_store_->OpenObjectStoreCursor(
           transaction->BackingStoreTransaction(),
           id(),
           params->object_store_id,
           *params->key_range,
-        params->direction);
+          params->direction,
+          s);
     }
   } else {
     DCHECK_EQ(params->task_type, IndexedDBDatabase::NORMAL_TASK);
@@ -1057,7 +1073,8 @@ void IndexedDBDatabase::OpenCursorOperation(
           params->object_store_id,
           params->index_id,
           *params->key_range,
-          params->direction);
+          params->direction,
+          s);
     } else {
       backing_store_cursor = backing_store_->OpenIndexCursor(
           transaction->BackingStoreTransaction(),
@@ -1065,11 +1082,23 @@ void IndexedDBDatabase::OpenCursorOperation(
           params->object_store_id,
           params->index_id,
           *params->key_range,
-          params->direction);
+          params->direction,
+          s);
+    }
+  }
+
+  if (!s.ok()) {
+    DLOG(ERROR) << "Unable to open cursor operation: " << s.ToString();
+    IndexedDBDatabaseError error(blink::WebIDBDatabaseExceptionUnknownError,
+                                 "Internal error opening cursor operation");
+    if (s.IsCorruption()) {
+      factory_->HandleBackingStoreCorruption(backing_store_->origin_url(),
+                                             error);
     }
   }
 
   if (!backing_store_cursor) {
+    // Why is Success being called?

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

In script, if a cursor is opened and there's nothing in range, it's like it hit
the end of its range - i.e. success w/ NULL.

(I'm assuming that logic is applying here as well.)

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

Are you saying that if the db is corrupt you want Success(NULL) to be called?
Like above at line 1095?

[jsbell, 2014/04/15 16:39:57]

I'm just answering the question "Why is Success being called?": if you have keys
in the range 0...10 and open a cursor on range (20,30) then script sees
onsuccess w/ result == null; IndexedDBBackingStore::OpenXXXCursor has the same
semantics, that backing_store_cursor will be NULL in such a case.

     params->callbacks->OnSuccess(static_cast<IndexedDBValue*>(NULL));
     return;
   }
@@ -1114,13 +1143,15 @@ void IndexedDBDatabase::CountOperation(
   uint32 count = 0;
   scoped_ptr<IndexedDBBackingStore::Cursor> backing_store_cursor;
 
+  leveldb::Status s;
   if (index_id == IndexedDBIndexMetadata::kInvalidId) {
     backing_store_cursor = backing_store_->OpenObjectStoreKeyCursor(
         transaction->BackingStoreTransaction(),
         id(),
         object_store_id,
         *key_range,
-        indexed_db::CURSOR_NEXT);
+        indexed_db::CURSOR_NEXT,
+        s);
   } else {
     backing_store_cursor = backing_store_->OpenIndexKeyCursor(
         transaction->BackingStoreTransaction(),
@@ -1128,7 +1159,17 @@ void IndexedDBDatabase::CountOperation(
         object_store_id,
         index_id,
         *key_range,
-        indexed_db::CURSOR_NEXT);
+        indexed_db::CURSOR_NEXT,
+        s);
+  }
+  if (!s.ok()) {
+    DLOG(ERROR) << "Unable perform count operation: " << s.ToString();
+    IndexedDBDatabaseError error(blink::WebIDBDatabaseExceptionUnknownError,
+                                 "Internal error performing count operation");
+    if (s.IsCorruption()) {
+      factory_->HandleBackingStoreCorruption(backing_store_->origin_url(),
+                                             error);
+    }
   }
   if (!backing_store_cursor) {
     callbacks->OnSuccess(count);
@@ -1137,7 +1178,9 @@ void IndexedDBDatabase::CountOperation(
 
   do {
     ++count;
-  } while (backing_store_cursor->Continue());
+  } while (backing_store_cursor->Continue(s));
+
+  // Should probably check for corruption here too?

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

Make this a TODO

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

Done.

 
   callbacks->OnSuccess(count);
 }
@@ -1169,14 +1212,16 @@ void IndexedDBDatabase::DeleteRangeOperation(
     scoped_refptr<IndexedDBCallbacks> callbacks,
     IndexedDBTransaction* transaction) {
   IDB_TRACE("IndexedDBDatabase::DeleteRangeOperation");
+  leveldb::Status s;
   scoped_ptr<IndexedDBBackingStore::Cursor> backing_store_cursor =
       backing_store_->OpenObjectStoreCursor(
           transaction->BackingStoreTransaction(),
           id(),
           object_store_id,
           *key_range,
-          indexed_db::CURSOR_NEXT);
-  if (backing_store_cursor) {
+          indexed_db::CURSOR_NEXT,
+          s);
+  if (backing_store_cursor && s.ok()) {
     do {
       if (!backing_store_->DeleteRecord(
                                transaction->BackingStoreTransaction(),
@@ -1189,7 +1234,18 @@ void IndexedDBDatabase::DeleteRangeOperation(
                                    "Internal error deleting data in range"));
         return;
       }
-    } while (backing_store_cursor->Continue());
+    } while (backing_store_cursor->Continue(s));
+  }
+
+  if (!s.ok()) {
+    IndexedDBDatabaseError error(blink::WebIDBDatabaseExceptionUnknownError,
+                                 ASCIIToUTF16("Internal error deleting range"));
+    transaction->Abort(error);
+    if (s.IsCorruption()) {
+      factory_->HandleBackingStoreCorruption(backing_store_->origin_url(),
+                                             error);
+    }
+    return;
   }
 
   callbacks->OnSuccess();