Make iterating over a corrupted IndexedDB fail.

content/browser/indexed_db/leveldb/leveldb_transaction.cc

Index: content/browser/indexed_db/leveldb/leveldb_transaction.cc
diff --git a/content/browser/indexed_db/leveldb/leveldb_transaction.cc b/content/browser/indexed_db/leveldb/leveldb_transaction.cc
index 88839a57d5b9d5da3e852280598ff5b7892b7b6a..0e0295ea28fdb2940978f30fdd8a596ebf23e725 100644
--- a/content/browser/indexed_db/leveldb/leveldb_transaction.cc
+++ b/content/browser/indexed_db/leveldb/leveldb_transaction.cc
@@ -130,27 +130,32 @@ bool LevelDBTransaction::DataIterator::IsValid() const {
   return iterator_ != data_->end();
 }
 
-void LevelDBTransaction::DataIterator::SeekToLast() {
+leveldb::Status LevelDBTransaction::DataIterator::SeekToLast() {
   iterator_ = data_->end();
   if (iterator_ != data_->begin())
     --iterator_;
+  return leveldb::Status::OK();
 }
 
-void LevelDBTransaction::DataIterator::Seek(const StringPiece& target) {
+leveldb::Status LevelDBTransaction::DataIterator::Seek(
+    const StringPiece& target) {
   iterator_ = data_->lower_bound(target);
+  return leveldb::Status::OK();
 }
 
-void LevelDBTransaction::DataIterator::Next() {
+leveldb::Status LevelDBTransaction::DataIterator::Next() {
   DCHECK(IsValid());
   ++iterator_;
+  return leveldb::Status::OK();
 }
 
-void LevelDBTransaction::DataIterator::Prev() {
+leveldb::Status LevelDBTransaction::DataIterator::Prev() {
   DCHECK(IsValid());
   if (iterator_ != data_->begin())
     --iterator_;
   else
     iterator_ = data_->end();
+  return leveldb::Status::OK();
 }
 
 StringPiece LevelDBTransaction::DataIterator::Key() const {
@@ -201,29 +206,41 @@ bool LevelDBTransaction::TransactionIterator::IsValid() const {
   return !!current_;
 }
 
-void LevelDBTransaction::TransactionIterator::SeekToLast() {
-  data_iterator_->SeekToLast();
-  db_iterator_->SeekToLast();
+leveldb::Status LevelDBTransaction::TransactionIterator::SeekToLast() {
+  leveldb::Status s = data_iterator_->SeekToLast();

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

DCHECK(s.ok()) since data_iterator_ should never fail?

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

Yes, DataIterator::SeekToLast() always returns OK(). Fixed.

+  if (!s.ok())
+    return s;
+  s = db_iterator_->SeekToLast();
+  if (!s.ok())
+    return s;
   direction_ = REVERSE;
 
   HandleConflictsAndDeletes();
   SetCurrentIteratorToLargestKey();
+  return s;
 }
 
-void LevelDBTransaction::TransactionIterator::Seek(const StringPiece& target) {
-  data_iterator_->Seek(target);
-  db_iterator_->Seek(target);
+leveldb::Status LevelDBTransaction::TransactionIterator::Seek(
+    const StringPiece& target) {
+  leveldb::Status s = data_iterator_->Seek(target);
+  if (!s.ok())
+    return s;
+  s = db_iterator_->Seek(target);
+  if (!s.ok())
+    return s;
   direction_ = FORWARD;
 
   HandleConflictsAndDeletes();
   SetCurrentIteratorToSmallestKey();
+  return s;
 }
 
-void LevelDBTransaction::TransactionIterator::Next() {
+leveldb::Status LevelDBTransaction::TransactionIterator::Next() {
   DCHECK(IsValid());
   if (data_changed_)
     RefreshDataIterator();
 
+  leveldb::Status s;
   if (direction_ != FORWARD) {
     // Ensure the non-current iterator is positioned after Key().
 
@@ -236,7 +253,7 @@ void LevelDBTransaction::TransactionIterator::Next() {
         !comparator_->Compare(non_current->Key(), Key())) {
       // Take an extra step so the non-current key is
       // strictly greater than Key().
-      non_current->Next();
+      s = non_current->Next();
     }
     DCHECK(!non_current->IsValid() ||
            comparator_->Compare(non_current->Key(), Key()) > 0);
@@ -244,13 +261,18 @@ void LevelDBTransaction::TransactionIterator::Next() {
     direction_ = FORWARD;
   }
 
-  current_->Next();
-  HandleConflictsAndDeletes();
-  SetCurrentIteratorToSmallestKey();
+  if (s.ok())
+    s = current_->Next();
+  if (s.ok()) {
+    HandleConflictsAndDeletes();
+    SetCurrentIteratorToSmallestKey();
+  }
+  return s;
 }
 
-void LevelDBTransaction::TransactionIterator::Prev() {
+leveldb::Status LevelDBTransaction::TransactionIterator::Prev() {
   DCHECK(IsValid());
+  leveldb::Status s;
   if (data_changed_)
     RefreshDataIterator();
 
@@ -261,7 +283,7 @@ void LevelDBTransaction::TransactionIterator::Prev() {
                                        ? data_iterator_.get()
                                        : db_iterator_.get();
 
-    non_current->Seek(Key());
+    s = non_current->Seek(Key());

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

Does s need to be tested here?

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

Done.

     if (non_current->IsValid()) {
       // Iterator is at first entry >= Key().
       // Step back once to entry < key.
@@ -278,9 +300,13 @@ void LevelDBTransaction::TransactionIterator::Prev() {
     direction_ = REVERSE;
   }
 
-  current_->Prev();
-  HandleConflictsAndDeletes();
-  SetCurrentIteratorToLargestKey();
+  if (s.ok())

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

This repeated testing of s.ok() is weird. Can we just early exit?

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

Done.

+    s = current_->Prev();
+  if (s.ok()) {
+    HandleConflictsAndDeletes();
+    SetCurrentIteratorToLargestKey();
+  }
+  return s;
 }
 
 StringPiece LevelDBTransaction::TransactionIterator::Key() const {