Make iterating over a corrupted IndexedDB fail.

content/browser/indexed_db/leveldb/leveldb_transaction.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/leveldb/leveldb_transaction.h"
 
 #include "base/logging.h"
 #include "content/browser/indexed_db/leveldb/leveldb_database.h"
 #include "content/browser/indexed_db/leveldb/leveldb_write_batch.h"
 #include "third_party/leveldatabase/src/include/leveldb/db.h"
@@ skipping 112 matching lines @@
 
 scoped_ptr<LevelDBTransaction::DataIterator>
 LevelDBTransaction::DataIterator::Create(LevelDBTransaction* transaction) {
   return make_scoped_ptr(new DataIterator(transaction));
 }
 
 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 {
   DCHECK(IsValid());
   return iterator_->first;
 }
 
 StringPiece LevelDBTransaction::DataIterator::Value() const {
   DCHECK(IsValid());
   DCHECK(!IsDeleted());
@@ skipping 30 matching lines @@
 }
 
 LevelDBTransaction::TransactionIterator::~TransactionIterator() {
   transaction_->UnregisterIterator(this);
 }
 
 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().
 
     LevelDBIterator* non_current = (current_ == db_iterator_.get())
                                        ? data_iterator_.get()
                                        : db_iterator_.get();
 
     non_current->Seek(Key());
     if (non_current->IsValid() &&
         !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);
 
     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();
 
   if (direction_ != REVERSE) {
     // Ensure the non-current iterator is positioned before Key().
 
     LevelDBIterator* non_current = (current_ == db_iterator_.get())
                                        ? 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.
       // This is why we don't check for the keys being the same before
       // stepping, like we do in Next() above.
       non_current->Prev();
     } else {
       // Iterator has no entries >= Key(). Position at last entry.
       non_current->SeekToLast();
     }
     DCHECK(!non_current->IsValid() ||
            comparator_->Compare(non_current->Key(), Key()) < 0);
 
     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 {
   DCHECK(IsValid());
   if (data_changed_)
     RefreshDataIterator();
   return current_->Key();
 }
 
 StringPiece LevelDBTransaction::TransactionIterator::Value() const {
@@ skipping 166 matching lines @@
 
   leveldb::Status s = db_->Write(*write_batch_);
   if (s.ok()) {
     finished_ = true;
     write_batch_->Clear();
   }
   return s;
 }
 
 }  // namespace content