diff options
author | cmumford@chromium.org <cmumford@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-04-16 18:32:19 +0000 |
---|---|---|
committer | cmumford@chromium.org <cmumford@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-04-16 18:32:19 +0000 |
commit | b05831eeb04ff0c3b8d8c2174e91ef1c59a0b289 (patch) | |
tree | 87f326abafb9f47309eb51a3bf0c5a2eb0e13b73 /content | |
parent | 706a0f23f3c1a537569f395988689c3a78f4970c (diff) | |
download | chromium_src-b05831eeb04ff0c3b8d8c2174e91ef1c59a0b289.zip chromium_src-b05831eeb04ff0c3b8d8c2174e91ef1c59a0b289.tar.gz chromium_src-b05831eeb04ff0c3b8d8c2174e91ef1c59a0b289.tar.bz2 |
Make iterating over a corrupted IndexedDB fail.
If an open corrupted IndexedDB was iterated over it would silently fail and the
web page would never be informed. This change fixes that bug so that cursor
iteration fails just like a get operation did previously.
BUG=332524
Review URL: https://codereview.chromium.org/237143006
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@264256 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'content')
18 files changed, 546 insertions, 220 deletions
diff --git a/content/browser/indexed_db/indexed_db_backing_store.cc b/content/browser/indexed_db/indexed_db_backing_store.cc index 32c53bc..162d451 100644 --- a/content/browser/indexed_db/indexed_db_backing_store.cc +++ b/content/browser/indexed_db/indexed_db_backing_store.cc @@ -78,6 +78,8 @@ enum IndexedDBBackingStoreErrorSource { DELETE_DATABASE, TRANSACTION_COMMIT_METHOD, // TRANSACTION_COMMIT is a WinNT.h macro GET_DATABASE_NAMES, + DELETE_INDEX, + CLEAR_OBJECT_STORE, INTERNAL_ERROR_MAX, }; @@ -323,9 +325,9 @@ WARN_UNUSED_RESULT static bool SetUpMetadata( const std::string stop_key = DatabaseNameKey::EncodeStopKeyForOrigin(origin_identifier); scoped_ptr<LevelDBIterator> it = db->CreateIterator(); - for (it->Seek(start_key); - it->IsValid() && CompareKeys(it->Key(), stop_key) < 0; - it->Next()) { + for (s = it->Seek(start_key); + s.ok() && it->IsValid() && CompareKeys(it->Key(), stop_key) < 0; + s = it->Next()) { int64 database_id = 0; found = false; s = GetInt(transaction.get(), it->Key(), &database_id, &found); @@ -344,7 +346,7 @@ WARN_UNUSED_RESULT static bool SetUpMetadata( IndexedDBDatabaseMetadata::DEFAULT_INT_VERSION); } } - if (db_schema_version < 2) { + if (s.ok() && db_schema_version < 2) { db_schema_version = 2; PutInt(transaction.get(), schema_version_key, db_schema_version); db_data_version = blink::kSerializedScriptValueVersion; @@ -352,6 +354,11 @@ WARN_UNUSED_RESULT static bool SetUpMetadata( } } + if (!s.ok()) { + INTERNAL_READ_ERROR_UNTESTED(SET_UP_METADATA); + return false; + } + // All new values will be written using this serialization version. found = false; s = GetInt(transaction.get(), data_version_key, &db_data_version, &found); @@ -810,7 +817,9 @@ scoped_refptr<IndexedDBBackingStore> IndexedDBBackingStore::Create( return backing_store; } -std::vector<base::string16> IndexedDBBackingStore::GetDatabaseNames() { +std::vector<base::string16> IndexedDBBackingStore::GetDatabaseNames( + leveldb::Status* s) { + *s = leveldb::Status::OK(); std::vector<base::string16> found_names; const std::string start_key = DatabaseNameKey::EncodeMinKeyForOrigin(origin_identifier_); @@ -820,9 +829,9 @@ std::vector<base::string16> IndexedDBBackingStore::GetDatabaseNames() { DCHECK(found_names.empty()); scoped_ptr<LevelDBIterator> it = db_->CreateIterator(); - for (it->Seek(start_key); - it->IsValid() && CompareKeys(it->Key(), stop_key) < 0; - it->Next()) { + for (*s = it->Seek(start_key); + s->ok() && it->IsValid() && CompareKeys(it->Key(), stop_key) < 0; + *s = it->Next()) { StringPiece slice(it->Key()); DatabaseNameKey database_name_key; if (!DatabaseNameKey::Decode(&slice, &database_name_key)) { @@ -831,6 +840,10 @@ std::vector<base::string16> IndexedDBBackingStore::GetDatabaseNames() { } found_names.push_back(database_name_key.database_name()); } + + if (!s->ok()) + INTERNAL_READ_ERROR_UNTESTED(GET_DATABASE_NAMES); + return found_names; } @@ -843,7 +856,7 @@ leveldb::Status IndexedDBBackingStore::GetIDBDatabaseMetaData( leveldb::Status s = GetInt(db_.get(), key, &metadata->id, found); if (!s.ok()) { - INTERNAL_READ_ERROR_UNTESTED(GET_IDBDATABASE_METADATA); + INTERNAL_READ_ERROR(GET_IDBDATABASE_METADATA); return s; } if (!*found) @@ -959,13 +972,16 @@ bool IndexedDBBackingStore::UpdateIDBDatabaseIntVersion( return true; } -static void DeleteRange(LevelDBTransaction* transaction, - const std::string& begin, - const std::string& end) { +static leveldb::Status DeleteRange(LevelDBTransaction* transaction, + const std::string& begin, + const std::string& end) { scoped_ptr<LevelDBIterator> it = transaction->CreateIterator(); - for (it->Seek(begin); it->IsValid() && CompareKeys(it->Key(), end) < 0; - it->Next()) + leveldb::Status s; + for (s = it->Seek(begin); + s.ok() && it->IsValid() && CompareKeys(it->Key(), end) < 0; + s = it->Next()) transaction->Remove(it->Key()); + return s; } leveldb::Status IndexedDBBackingStore::DeleteDatabase( @@ -987,10 +1003,14 @@ leveldb::Status IndexedDBBackingStore::DeleteDatabase( const std::string stop_key = DatabaseMetaDataKey::Encode( metadata.id + 1, DatabaseMetaDataKey::ORIGIN_NAME); scoped_ptr<LevelDBIterator> it = db_->CreateIterator(); - for (it->Seek(start_key); - it->IsValid() && CompareKeys(it->Key(), stop_key) < 0; - it->Next()) + for (s = it->Seek(start_key); + s.ok() && it->IsValid() && CompareKeys(it->Key(), stop_key) < 0; + s = it->Next()) transaction->Remove(it->Key()); + if (!s.ok()) { + INTERNAL_WRITE_ERROR_UNTESTED(DELETE_DATABASE); + return s; + } const std::string key = DatabaseNameKey::Encode(origin_identifier_, name); transaction->Remove(key); @@ -1038,8 +1058,8 @@ leveldb::Status IndexedDBBackingStore::GetObjectStores( DCHECK(object_stores->empty()); scoped_ptr<LevelDBIterator> it = db_->CreateIterator(); - it->Seek(start_key); - while (it->IsValid() && CompareKeys(it->Key(), stop_key) < 0) { + leveldb::Status s = it->Seek(start_key); + while (s.ok() && it->IsValid() && CompareKeys(it->Key(), stop_key) < 0) { StringPiece slice(it->Key()); ObjectStoreMetaDataKey meta_data_key; bool ok = ObjectStoreMetaDataKey::Decode(&slice, &meta_data_key); @@ -1047,7 +1067,9 @@ leveldb::Status IndexedDBBackingStore::GetObjectStores( if (meta_data_key.MetaDataType() != ObjectStoreMetaDataKey::NAME) { INTERNAL_CONSISTENCY_ERROR_UNTESTED(GET_OBJECT_STORES); // Possible stale metadata, but don't fail the load. - it->Next(); + s = it->Next(); + if (!s.ok()) + break; continue; } @@ -1062,7 +1084,9 @@ leveldb::Status IndexedDBBackingStore::GetObjectStores( INTERNAL_CONSISTENCY_ERROR_UNTESTED(GET_OBJECT_STORES); } - it->Next(); + s = it->Next(); + if (!s.ok()) + break; if (!CheckObjectStoreAndMetaDataType(it.get(), stop_key, object_store_id, @@ -1077,7 +1101,9 @@ leveldb::Status IndexedDBBackingStore::GetObjectStores( INTERNAL_CONSISTENCY_ERROR_UNTESTED(GET_OBJECT_STORES); } - it->Next(); + s = it->Next(); + if (!s.ok()) + break; if (!CheckObjectStoreAndMetaDataType( it.get(), stop_key, @@ -1093,7 +1119,9 @@ leveldb::Status IndexedDBBackingStore::GetObjectStores( INTERNAL_CONSISTENCY_ERROR_UNTESTED(GET_OBJECT_STORES); } - it->Next(); // Is evicatble. + s = it->Next(); // Is evicatble. + if (!s.ok()) + break; if (!CheckObjectStoreAndMetaDataType(it.get(), stop_key, object_store_id, @@ -1102,7 +1130,9 @@ leveldb::Status IndexedDBBackingStore::GetObjectStores( break; } - it->Next(); // Last version. + s = it->Next(); // Last version. + if (!s.ok()) + break; if (!CheckObjectStoreAndMetaDataType( it.get(), stop_key, @@ -1112,7 +1142,9 @@ leveldb::Status IndexedDBBackingStore::GetObjectStores( break; } - it->Next(); // Maximum index id allocated. + s = it->Next(); // Maximum index id allocated. + if (!s.ok()) + break; if (!CheckObjectStoreAndMetaDataType( it.get(), stop_key, @@ -1128,7 +1160,9 @@ leveldb::Status IndexedDBBackingStore::GetObjectStores( INTERNAL_CONSISTENCY_ERROR_UNTESTED(GET_OBJECT_STORES); } - it->Next(); // [optional] has key path (is not null) + s = it->Next(); // [optional] has key path (is not null) + if (!s.ok()) + break; if (CheckObjectStoreAndMetaDataType(it.get(), stop_key, object_store_id, @@ -1151,7 +1185,9 @@ leveldb::Status IndexedDBBackingStore::GetObjectStores( } if (!has_key_path) key_path = IndexedDBKeyPath(); - it->Next(); + s = it->Next(); + if (!s.ok()) + break; } int64 key_generator_current_number = -1; @@ -1168,7 +1204,9 @@ leveldb::Status IndexedDBBackingStore::GetObjectStores( // object store, and write lazily to backing store. For now, // just assert that if it was written it was valid. DCHECK_GE(key_generator_current_number, kKeyGeneratorInitialNumber); - it->Next(); + s = it->Next(); + if (!s.ok()) + break; } IndexedDBObjectStoreMetadata metadata(object_store_name, @@ -1176,13 +1214,16 @@ leveldb::Status IndexedDBBackingStore::GetObjectStores( key_path, auto_increment, max_index_id); - leveldb::Status s = - GetIndexes(database_id, object_store_id, &metadata.indexes); + s = GetIndexes(database_id, object_store_id, &metadata.indexes); if (!s.ok()) - return s; + break; (*object_stores)[object_store_id] = metadata; } - return leveldb::Status::OK(); + + if (!s.ok()) + INTERNAL_READ_ERROR_UNTESTED(GET_OBJECT_STORES); + + return s; } WARN_UNUSED_RESULT static leveldb::Status SetMaxObjectStoreId( @@ -1286,20 +1327,32 @@ leveldb::Status IndexedDBBackingStore::DeleteObjectStore( return InternalInconsistencyStatus(); } - DeleteRange( + s = DeleteRange( leveldb_transaction, ObjectStoreMetaDataKey::Encode(database_id, object_store_id, 0), ObjectStoreMetaDataKey::EncodeMaxKey(database_id, object_store_id)); - leveldb_transaction->Remove( - ObjectStoreNamesKey::Encode(database_id, object_store_name)); + if (s.ok()) { + leveldb_transaction->Remove( + ObjectStoreNamesKey::Encode(database_id, object_store_name)); + + s = DeleteRange( + leveldb_transaction, + IndexFreeListKey::Encode(database_id, object_store_id, 0), + IndexFreeListKey::EncodeMaxKey(database_id, object_store_id)); + } + + if (s.ok()) { + s = DeleteRange( + leveldb_transaction, + IndexMetaDataKey::Encode(database_id, object_store_id, 0, 0), + IndexMetaDataKey::EncodeMaxKey(database_id, object_store_id)); + } - DeleteRange(leveldb_transaction, - IndexFreeListKey::Encode(database_id, object_store_id, 0), - IndexFreeListKey::EncodeMaxKey(database_id, object_store_id)); - DeleteRange(leveldb_transaction, - IndexMetaDataKey::Encode(database_id, object_store_id, 0, 0), - IndexMetaDataKey::EncodeMaxKey(database_id, object_store_id)); + if (!s.ok()) { + INTERNAL_WRITE_ERROR_UNTESTED(DELETE_OBJECT_STORE); + return s; + } return ClearObjectStore(transaction, database_id, object_store_id); } @@ -1429,8 +1482,11 @@ leveldb::Status IndexedDBBackingStore::ClearObjectStore( const std::string stop_key = KeyPrefix(database_id, object_store_id + 1).Encode(); - DeleteRange(transaction->transaction(), start_key, stop_key); - return leveldb::Status::OK(); + leveldb::Status s = + DeleteRange(transaction->transaction(), start_key, stop_key); + if (!s.ok()) + INTERNAL_WRITE_ERROR_UNTESTED(CLEAR_OBJECT_STORE); + return s; } leveldb::Status IndexedDBBackingStore::DeleteRecord( @@ -1500,9 +1556,9 @@ leveldb::Status IndexedDBBackingStore::GetKeyGeneratorCurrentNumber( scoped_ptr<LevelDBIterator> it = leveldb_transaction->CreateIterator(); int64 max_numeric_key = 0; - for (it->Seek(start_key); - it->IsValid() && CompareKeys(it->Key(), stop_key) < 0; - it->Next()) { + for (s = it->Seek(start_key); + s.ok() && it->IsValid() && CompareKeys(it->Key(), stop_key) < 0; + s = it->Next()) { StringPiece slice(it->Key()); ObjectStoreDataKey data_key; if (!ObjectStoreDataKey::Decode(&slice, &data_key)) { @@ -1517,7 +1573,11 @@ leveldb::Status IndexedDBBackingStore::GetKeyGeneratorCurrentNumber( } } - *key_generator_current_number = max_numeric_key + 1; + if (s.ok()) + *key_generator_current_number = max_numeric_key + 1; + else + INTERNAL_READ_ERROR_UNTESTED(GET_KEY_GENERATOR_CURRENT_NUMBER); + return s; } @@ -1624,8 +1684,8 @@ leveldb::Status IndexedDBBackingStore::GetIndexes( DCHECK(indexes->empty()); scoped_ptr<LevelDBIterator> it = db_->CreateIterator(); - it->Seek(start_key); - while (it->IsValid() && CompareKeys(it->Key(), stop_key) < 0) { + leveldb::Status s = it->Seek(start_key); + while (s.ok() && it->IsValid() && CompareKeys(it->Key(), stop_key) < 0) { StringPiece slice(it->Key()); IndexMetaDataKey meta_data_key; bool ok = IndexMetaDataKey::Decode(&slice, &meta_data_key); @@ -1634,7 +1694,9 @@ leveldb::Status IndexedDBBackingStore::GetIndexes( INTERNAL_CONSISTENCY_ERROR_UNTESTED(GET_INDEXES); // Possible stale metadata due to http://webkit.org/b/85557 but don't fail // the load. - it->Next(); + s = it->Next(); + if (!s.ok()) + break; continue; } @@ -1648,7 +1710,9 @@ leveldb::Status IndexedDBBackingStore::GetIndexes( INTERNAL_CONSISTENCY_ERROR_UNTESTED(GET_INDEXES); } - it->Next(); // unique flag + s = it->Next(); // unique flag + if (!s.ok()) + break; if (!CheckIndexAndMetaDataKey( it.get(), stop_key, index_id, IndexMetaDataKey::UNIQUE)) { INTERNAL_CONSISTENCY_ERROR_UNTESTED(GET_INDEXES); @@ -1661,7 +1725,9 @@ leveldb::Status IndexedDBBackingStore::GetIndexes( INTERNAL_CONSISTENCY_ERROR_UNTESTED(GET_INDEXES); } - it->Next(); // key_path + s = it->Next(); // key_path + if (!s.ok()) + break; if (!CheckIndexAndMetaDataKey( it.get(), stop_key, index_id, IndexMetaDataKey::KEY_PATH)) { INTERNAL_CONSISTENCY_ERROR_UNTESTED(GET_INDEXES); @@ -1674,7 +1740,9 @@ leveldb::Status IndexedDBBackingStore::GetIndexes( INTERNAL_CONSISTENCY_ERROR_UNTESTED(GET_INDEXES); } - it->Next(); // [optional] multi_entry flag + s = it->Next(); // [optional] multi_entry flag + if (!s.ok()) + break; bool index_multi_entry = false; if (CheckIndexAndMetaDataKey( it.get(), stop_key, index_id, IndexMetaDataKey::MULTI_ENTRY)) { @@ -1682,13 +1750,19 @@ leveldb::Status IndexedDBBackingStore::GetIndexes( if (!DecodeBool(&slice, &index_multi_entry) || !slice.empty()) INTERNAL_CONSISTENCY_ERROR_UNTESTED(GET_INDEXES); - it->Next(); + s = it->Next(); + if (!s.ok()) + break; } (*indexes)[index_id] = IndexedDBIndexMetadata( index_name, index_id, key_path, index_unique, index_multi_entry); } - return leveldb::Status::OK(); + + if (!s.ok()) + INTERNAL_READ_ERROR_UNTESTED(GET_INDEXES); + + return s; } WARN_UNUSED_RESULT static leveldb::Status SetMaxIndexId( @@ -1767,14 +1841,21 @@ leveldb::Status IndexedDBBackingStore::DeleteIndex( IndexMetaDataKey::Encode(database_id, object_store_id, index_id, 0); const std::string index_meta_data_end = IndexMetaDataKey::EncodeMaxKey(database_id, object_store_id, index_id); - DeleteRange(leveldb_transaction, index_meta_data_start, index_meta_data_end); + leveldb::Status s = DeleteRange( + leveldb_transaction, index_meta_data_start, index_meta_data_end); - const std::string index_data_start = - IndexDataKey::EncodeMinKey(database_id, object_store_id, index_id); - const std::string index_data_end = - IndexDataKey::EncodeMaxKey(database_id, object_store_id, index_id); - DeleteRange(leveldb_transaction, index_data_start, index_data_end); - return leveldb::Status::OK(); + if (s.ok()) { + const std::string index_data_start = + IndexDataKey::EncodeMinKey(database_id, object_store_id, index_id); + const std::string index_data_end = + IndexDataKey::EncodeMaxKey(database_id, object_store_id, index_id); + s = DeleteRange(leveldb_transaction, index_data_start, index_data_end); + } + + if (!s.ok()) + INTERNAL_WRITE_ERROR_UNTESTED(DELETE_INDEX); + + return s; } leveldb::Status IndexedDBBackingStore::PutIndexDataForRecord( @@ -1810,19 +1891,22 @@ leveldb::Status IndexedDBBackingStore::PutIndexDataForRecord( static bool FindGreatestKeyLessThanOrEqual(LevelDBTransaction* transaction, const std::string& target, - std::string* found_key) { + std::string* found_key, + leveldb::Status& s) { scoped_ptr<LevelDBIterator> it = transaction->CreateIterator(); - it->Seek(target); + s = it->Seek(target); + if (!s.ok()) + return false; if (!it->IsValid()) { - it->SeekToLast(); - if (!it->IsValid()) + s = it->SeekToLast(); + if (!s.ok() || !it->IsValid()) return false; } while (CompareIndexKeys(it->Key(), target) > 0) { - it->Prev(); - if (!it->IsValid()) + s = it->Prev(); + if (!s.ok() || !it->IsValid()) return false; } @@ -1830,8 +1914,8 @@ static bool FindGreatestKeyLessThanOrEqual(LevelDBTransaction* transaction, *found_key = it->Key().as_string(); // There can be several index keys that compare equal. We want the last one. - it->Next(); - } while (it->IsValid() && !CompareIndexKeys(it->Key(), target)); + s = it->Next(); + } while (s.ok() && it->IsValid() && !CompareIndexKeys(it->Key(), target)); return true; } @@ -1880,7 +1964,11 @@ leveldb::Status IndexedDBBackingStore::FindKeyInIndex( const std::string leveldb_key = IndexDataKey::Encode(database_id, object_store_id, index_id, key); scoped_ptr<LevelDBIterator> it = leveldb_transaction->CreateIterator(); - it->Seek(leveldb_key); + leveldb::Status s = it->Seek(leveldb_key); + if (!s.ok()) { + INTERNAL_READ_ERROR_UNTESTED(FIND_KEY_IN_INDEX); + return s; + } for (;;) { if (!it->IsValid()) @@ -1898,18 +1986,18 @@ leveldb::Status IndexedDBBackingStore::FindKeyInIndex( *found_encoded_primary_key = slice.as_string(); bool exists = false; - leveldb::Status s = VersionExists(leveldb_transaction, - database_id, - object_store_id, - version, - *found_encoded_primary_key, - &exists); + s = VersionExists(leveldb_transaction, + database_id, + object_store_id, + version, + *found_encoded_primary_key, + &exists); if (!s.ok()) return s; if (!exists) { // Delete stale index data entry and continue. leveldb_transaction->Remove(it->Key()); - it->Next(); + s = it->Next(); continue; } *found = true; @@ -2008,7 +2096,8 @@ IndexedDBBackingStore::Cursor::Cursor( iterator_ = transaction_->CreateIterator(); if (other->iterator_->IsValid()) { - iterator_->Seek(other->iterator_->Key()); + leveldb::Status s = iterator_->Seek(other->iterator_->Key()); + // TODO(cmumford): Handle this error (crbug.com/363397) DCHECK(iterator_->IsValid()); } } @@ -2019,19 +2108,22 @@ IndexedDBBackingStore::Cursor::Cursor(LevelDBTransaction* transaction, : transaction_(transaction), cursor_options_(cursor_options) {} IndexedDBBackingStore::Cursor::~Cursor() {} -bool IndexedDBBackingStore::Cursor::FirstSeek() { +bool IndexedDBBackingStore::Cursor::FirstSeek(leveldb::Status* s) { iterator_ = transaction_->CreateIterator(); if (cursor_options_.forward) - iterator_->Seek(cursor_options_.low_key); + *s = iterator_->Seek(cursor_options_.low_key); else - iterator_->Seek(cursor_options_.high_key); + *s = iterator_->Seek(cursor_options_.high_key); + if (!s->ok()) + return false; - return Continue(0, READY); + return Continue(0, READY, s); } -bool IndexedDBBackingStore::Cursor::Advance(uint32 count) { +bool IndexedDBBackingStore::Cursor::Advance(uint32 count, leveldb::Status* s) { + *s = leveldb::Status::OK(); while (count--) { - if (!Continue()) + if (!Continue(s)) return false; } return true; @@ -2039,9 +2131,11 @@ bool IndexedDBBackingStore::Cursor::Advance(uint32 count) { bool IndexedDBBackingStore::Cursor::Continue(const IndexedDBKey* key, const IndexedDBKey* primary_key, - IteratorState next_state) { + IteratorState next_state, + leveldb::Status* s) { DCHECK(!key || key->IsValid()); DCHECK(!primary_key || primary_key->IsValid()); + *s = leveldb::Status::OK(); // TODO(alecflett): avoid a copy here? IndexedDBKey previous_key = current_key_ ? *current_key_ : IndexedDBKey(); @@ -2065,13 +2159,15 @@ bool IndexedDBBackingStore::Cursor::Continue(const IndexedDBKey* key, } else { leveldb_key = EncodeKey(*key); } - iterator_->Seek(leveldb_key); + *s = iterator_->Seek(leveldb_key); first_iteration = false; } else if (forward) { - iterator_->Next(); + *s = iterator_->Next(); } else { - iterator_->Prev(); + *s = iterator_->Prev(); } + if (!s->ok()) + return false; } else { next_state = SEEK; // for subsequent iterations } @@ -2553,6 +2649,8 @@ bool ObjectStoreCursorOptions( cursor_options->low_open = range.lowerOpen(); } + leveldb::Status s; + if (!upper_bound) { cursor_options->high_key = ObjectStoreDataKey::Encode(database_id, object_store_id, MaxIDBKey()); @@ -2561,9 +2659,11 @@ bool ObjectStoreCursorOptions( cursor_options->high_open = true; // Not included. } else { // We need a key that exists. + // TODO(cmumford): Handle this error (crbug.com/363397) if (!FindGreatestKeyLessThanOrEqual(transaction, cursor_options->high_key, - &cursor_options->high_key)) + &cursor_options->high_key, + s)) return false; cursor_options->high_open = false; } @@ -2575,8 +2675,9 @@ bool ObjectStoreCursorOptions( if (!cursor_options->forward) { // For reverse cursors, we need a key that exists. std::string found_high_key; + // TODO(cmumford): Handle this error (crbug.com/363397) if (!FindGreatestKeyLessThanOrEqual( - transaction, cursor_options->high_key, &found_high_key)) + transaction, cursor_options->high_key, &found_high_key, s)) return false; // If the target key should not be included, but we end up with a smaller @@ -2626,6 +2727,8 @@ bool IndexCursorOptions( cursor_options->low_open = range.lowerOpen(); } + leveldb::Status s; + if (!upper_bound) { cursor_options->high_key = IndexDataKey::EncodeMaxKey(database_id, object_store_id, index_id); @@ -2634,7 +2737,8 @@ bool IndexCursorOptions( if (!cursor_options->forward) { // We need a key that exists. if (!FindGreatestKeyLessThanOrEqual(transaction, cursor_options->high_key, - &cursor_options->high_key)) + &cursor_options->high_key, + s)) return false; cursor_options->high_open = false; } @@ -2645,8 +2749,9 @@ bool IndexCursorOptions( std::string found_high_key; // Seek to the *last* key in the set of non-unique keys + // TODO(cmumford): Handle this error (crbug.com/363397) if (!FindGreatestKeyLessThanOrEqual( - transaction, cursor_options->high_key, &found_high_key)) + transaction, cursor_options->high_key, &found_high_key, s)) return false; // If the target key should not be included, but we end up with a smaller @@ -2667,8 +2772,10 @@ IndexedDBBackingStore::OpenObjectStoreCursor( int64 database_id, int64 object_store_id, const IndexedDBKeyRange& range, - indexed_db::CursorDirection direction) { + indexed_db::CursorDirection direction, + leveldb::Status* s) { IDB_TRACE("IndexedDBBackingStore::OpenObjectStoreCursor"); + *s = leveldb::Status::OK(); LevelDBTransaction* leveldb_transaction = transaction->transaction(); IndexedDBBackingStore::Cursor::CursorOptions cursor_options; if (!ObjectStoreCursorOptions(leveldb_transaction, @@ -2680,7 +2787,7 @@ IndexedDBBackingStore::OpenObjectStoreCursor( return scoped_ptr<IndexedDBBackingStore::Cursor>(); scoped_ptr<ObjectStoreCursorImpl> cursor( new ObjectStoreCursorImpl(leveldb_transaction, cursor_options)); - if (!cursor->FirstSeek()) + if (!cursor->FirstSeek(s)) return scoped_ptr<IndexedDBBackingStore::Cursor>(); return cursor.PassAs<IndexedDBBackingStore::Cursor>(); @@ -2692,8 +2799,10 @@ IndexedDBBackingStore::OpenObjectStoreKeyCursor( int64 database_id, int64 object_store_id, const IndexedDBKeyRange& range, - indexed_db::CursorDirection direction) { + indexed_db::CursorDirection direction, + leveldb::Status* s) { IDB_TRACE("IndexedDBBackingStore::OpenObjectStoreKeyCursor"); + *s = leveldb::Status::OK(); LevelDBTransaction* leveldb_transaction = transaction->transaction(); IndexedDBBackingStore::Cursor::CursorOptions cursor_options; if (!ObjectStoreCursorOptions(leveldb_transaction, @@ -2705,7 +2814,7 @@ IndexedDBBackingStore::OpenObjectStoreKeyCursor( return scoped_ptr<IndexedDBBackingStore::Cursor>(); scoped_ptr<ObjectStoreKeyCursorImpl> cursor( new ObjectStoreKeyCursorImpl(leveldb_transaction, cursor_options)); - if (!cursor->FirstSeek()) + if (!cursor->FirstSeek(s)) return scoped_ptr<IndexedDBBackingStore::Cursor>(); return cursor.PassAs<IndexedDBBackingStore::Cursor>(); @@ -2718,8 +2827,10 @@ IndexedDBBackingStore::OpenIndexKeyCursor( int64 object_store_id, int64 index_id, const IndexedDBKeyRange& range, - indexed_db::CursorDirection direction) { + indexed_db::CursorDirection direction, + leveldb::Status* s) { IDB_TRACE("IndexedDBBackingStore::OpenIndexKeyCursor"); + *s = leveldb::Status::OK(); LevelDBTransaction* leveldb_transaction = transaction->transaction(); IndexedDBBackingStore::Cursor::CursorOptions cursor_options; if (!IndexCursorOptions(leveldb_transaction, @@ -2732,7 +2843,7 @@ IndexedDBBackingStore::OpenIndexKeyCursor( return scoped_ptr<IndexedDBBackingStore::Cursor>(); scoped_ptr<IndexKeyCursorImpl> cursor( new IndexKeyCursorImpl(leveldb_transaction, cursor_options)); - if (!cursor->FirstSeek()) + if (!cursor->FirstSeek(s)) return scoped_ptr<IndexedDBBackingStore::Cursor>(); return cursor.PassAs<IndexedDBBackingStore::Cursor>(); @@ -2745,7 +2856,8 @@ IndexedDBBackingStore::OpenIndexCursor( int64 object_store_id, int64 index_id, const IndexedDBKeyRange& range, - indexed_db::CursorDirection direction) { + indexed_db::CursorDirection direction, + leveldb::Status* s) { IDB_TRACE("IndexedDBBackingStore::OpenIndexCursor"); LevelDBTransaction* leveldb_transaction = transaction->transaction(); IndexedDBBackingStore::Cursor::CursorOptions cursor_options; @@ -2759,7 +2871,7 @@ IndexedDBBackingStore::OpenIndexCursor( return scoped_ptr<IndexedDBBackingStore::Cursor>(); scoped_ptr<IndexCursorImpl> cursor( new IndexCursorImpl(leveldb_transaction, cursor_options)); - if (!cursor->FirstSeek()) + if (!cursor->FirstSeek(s)) return scoped_ptr<IndexedDBBackingStore::Cursor>(); return cursor.PassAs<IndexedDBBackingStore::Cursor>(); diff --git a/content/browser/indexed_db/indexed_db_backing_store.h b/content/browser/indexed_db/indexed_db_backing_store.h index ed2e395..09db8c2 100644 --- a/content/browser/indexed_db/indexed_db_backing_store.h +++ b/content/browser/indexed_db/indexed_db_backing_store.h @@ -97,7 +97,7 @@ class CONTENT_EXPORT IndexedDBBackingStore // Compact is public for testing. virtual void Compact(); - virtual std::vector<base::string16> GetDatabaseNames(); + virtual std::vector<base::string16> GetDatabaseNames(leveldb::Status*); virtual leveldb::Status GetIDBDatabaseMetaData( const base::string16& name, IndexedDBDatabaseMetadata* metadata, @@ -260,15 +260,18 @@ class CONTENT_EXPORT IndexedDBBackingStore }; const IndexedDBKey& key() const { return *current_key_; } - bool Continue() { return Continue(NULL, NULL, SEEK); } - bool Continue(const IndexedDBKey* key, IteratorState state) { - return Continue(key, NULL, state); + bool Continue(leveldb::Status* s) { return Continue(NULL, NULL, SEEK, s); } + bool Continue(const IndexedDBKey* key, + IteratorState state, + leveldb::Status* s) { + return Continue(key, NULL, state, s); } bool Continue(const IndexedDBKey* key, const IndexedDBKey* primary_key, - IteratorState state); - bool Advance(uint32 count); - bool FirstSeek(); + IteratorState state, + leveldb::Status*); + bool Advance(uint32 count, leveldb::Status*); + bool FirstSeek(leveldb::Status*); virtual Cursor* Clone() = 0; virtual const IndexedDBKey& primary_key() const; @@ -300,27 +303,31 @@ class CONTENT_EXPORT IndexedDBBackingStore int64 database_id, int64 object_store_id, const IndexedDBKeyRange& key_range, - indexed_db::CursorDirection); + indexed_db::CursorDirection, + leveldb::Status*); virtual scoped_ptr<Cursor> OpenObjectStoreCursor( IndexedDBBackingStore::Transaction* transaction, int64 database_id, int64 object_store_id, const IndexedDBKeyRange& key_range, - indexed_db::CursorDirection); + indexed_db::CursorDirection, + leveldb::Status*); virtual scoped_ptr<Cursor> OpenIndexKeyCursor( IndexedDBBackingStore::Transaction* transaction, int64 database_id, int64 object_store_id, int64 index_id, const IndexedDBKeyRange& key_range, - indexed_db::CursorDirection); + indexed_db::CursorDirection, + leveldb::Status*); virtual scoped_ptr<Cursor> OpenIndexCursor( IndexedDBBackingStore::Transaction* transaction, int64 database_id, int64 object_store_id, int64 index_id, const IndexedDBKeyRange& key_range, - indexed_db::CursorDirection); + indexed_db::CursorDirection, + leveldb::Status*); class Transaction { public: diff --git a/content/browser/indexed_db/indexed_db_browsertest.cc b/content/browser/indexed_db/indexed_db_browsertest.cc index 6e8b40a..206020f 100644 --- a/content/browser/indexed_db/indexed_db_browsertest.cc +++ b/content/browser/indexed_db/indexed_db_browsertest.cc @@ -473,7 +473,7 @@ static scoped_ptr<net::test_server::HttpResponse> CorruptDBRequestHandler( } // namespace -IN_PROC_BROWSER_TEST_F(IndexedDBBrowserTest, CorruptedOpenDatabase) { +IN_PROC_BROWSER_TEST_F(IndexedDBBrowserTest, CorruptedOpenDatabaseGet) { ASSERT_TRUE(embedded_test_server()->Started() || embedded_test_server()->InitializeAndWaitUntilReady()); const GURL& origin_url = embedded_test_server()->base_url(); @@ -484,7 +484,25 @@ IN_PROC_BROWSER_TEST_F(IndexedDBBrowserTest, CorruptedOpenDatabase) { s_corrupt_db_test_prefix)); std::string test_file = - s_corrupt_db_test_prefix + "corrupted_open_db_detection.html"; + s_corrupt_db_test_prefix + "corrupted_open_db_detection.html#get"; + SimpleTest(embedded_test_server()->GetURL(test_file)); + + test_file = s_corrupt_db_test_prefix + "corrupted_open_db_recovery.html"; + SimpleTest(embedded_test_server()->GetURL(test_file)); +} + +IN_PROC_BROWSER_TEST_F(IndexedDBBrowserTest, CorruptedOpenDatabaseIterate) { + ASSERT_TRUE(embedded_test_server()->Started() || + embedded_test_server()->InitializeAndWaitUntilReady()); + const GURL& origin_url = embedded_test_server()->base_url(); + embedded_test_server()->RegisterRequestHandler( + base::Bind(&CorruptDBRequestHandler, + base::ConstRef(GetContext()), + origin_url, + s_corrupt_db_test_prefix)); + + std::string test_file = + s_corrupt_db_test_prefix + "corrupted_open_db_detection.html#iterate"; SimpleTest(embedded_test_server()->GetURL(test_file)); test_file = s_corrupt_db_test_prefix + "corrupted_open_db_recovery.html"; diff --git a/content/browser/indexed_db/indexed_db_cursor.cc b/content/browser/indexed_db/indexed_db_cursor.cc index b32c7c2..9caf29d 100644 --- a/content/browser/indexed_db/indexed_db_cursor.cc +++ b/content/browser/indexed_db/indexed_db_cursor.cc @@ -60,7 +60,11 @@ void IndexedDBCursor::CursorAdvanceOperation( scoped_refptr<IndexedDBCallbacks> callbacks, IndexedDBTransaction* /*transaction*/) { IDB_TRACE("IndexedDBCursor::CursorAdvanceOperation"); - if (!cursor_ || !cursor_->Advance(count)) { + leveldb::Status s; + // TODO(cmumford): Handle this error (crbug.com/363397). Although this will + // properly fail, caller will not know why, and any corruption + // will be ignored. + if (!cursor_ || !cursor_->Advance(count, &s)) { cursor_.reset(); callbacks->OnSuccess(static_cast<IndexedDBValue*>(NULL)); return; @@ -75,9 +79,14 @@ void IndexedDBCursor::CursorIterationOperation( scoped_refptr<IndexedDBCallbacks> callbacks, IndexedDBTransaction* /*transaction*/) { IDB_TRACE("IndexedDBCursor::CursorIterationOperation"); - if (!cursor_ || - !cursor_->Continue( - key.get(), primary_key.get(), IndexedDBBackingStore::Cursor::SEEK)) { + leveldb::Status s; + // TODO(cmumford): Handle this error (crbug.com/363397). Although this will + // properly fail, caller will not know why, and any corruption + // will be ignored. + if (!cursor_ || !cursor_->Continue(key.get(), + primary_key.get(), + IndexedDBBackingStore::Cursor::SEEK, + &s) || !s.ok()) { cursor_.reset(); callbacks->OnSuccess(static_cast<IndexedDBValue*>(NULL)); return; @@ -112,9 +121,13 @@ void IndexedDBCursor::CursorPrefetchIterationOperation( saved_cursor_.reset(); const size_t max_size_estimate = 10 * 1024 * 1024; size_t size_estimate = 0; + leveldb::Status s; + // TODO(cmumford): Handle this error (crbug.com/363397). Although this will + // properly fail, caller will not know why, and any corruption + // will be ignored. for (int i = 0; i < number_to_fetch; ++i) { - if (!cursor_ || !cursor_->Continue()) { + if (!cursor_ || !cursor_->Continue(&s)) { cursor_.reset(); break; } @@ -158,22 +171,25 @@ void IndexedDBCursor::CursorPrefetchIterationOperation( found_keys, found_primary_keys, found_values); } -void IndexedDBCursor::PrefetchReset(int used_prefetches, - int /* unused_prefetches */) { +leveldb::Status IndexedDBCursor::PrefetchReset(int used_prefetches, + int /* unused_prefetches */) { IDB_TRACE("IndexedDBCursor::PrefetchReset"); cursor_.swap(saved_cursor_); saved_cursor_.reset(); + leveldb::Status s; if (closed_) - return; + return s; if (cursor_) { // First prefetched result is always used. DCHECK_GT(used_prefetches, 0); for (int i = 0; i < used_prefetches - 1; ++i) { - bool ok = cursor_->Continue(); + bool ok = cursor_->Continue(&s); DCHECK(ok); } } + + return s; } void IndexedDBCursor::Close() { diff --git a/content/browser/indexed_db/indexed_db_cursor.h b/content/browser/indexed_db/indexed_db_cursor.h index d6d1d79..4c1ec0c 100644 --- a/content/browser/indexed_db/indexed_db_cursor.h +++ b/content/browser/indexed_db/indexed_db_cursor.h @@ -32,7 +32,7 @@ class CONTENT_EXPORT IndexedDBCursor scoped_refptr<IndexedDBCallbacks> callbacks); void PrefetchContinue(int number_to_fetch, scoped_refptr<IndexedDBCallbacks> callbacks); - void PrefetchReset(int used_prefetches, int unused_prefetches); + leveldb::Status PrefetchReset(int used_prefetches, int unused_prefetches); const IndexedDBKey& key() const { return cursor_->key(); } const IndexedDBKey& primary_key() const { return cursor_->primary_key(); } diff --git a/content/browser/indexed_db/indexed_db_database.cc b/content/browser/indexed_db/indexed_db_database.cc index 6b090dd..5107fa8 100644 --- a/content/browser/indexed_db/indexed_db_database.cc +++ b/content/browser/indexed_db/indexed_db_database.cc @@ -92,12 +92,15 @@ 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(); + if (s->ok()) + return database; + else + return NULL; } namespace { @@ -527,6 +530,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 +543,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 +553,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 +563,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 +586,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 +1046,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 +1056,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 +1076,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 +1085,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? params->callbacks->OnSuccess(static_cast<IndexedDBValue*>(NULL)); return; } @@ -1114,13 +1146,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 +1162,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 +1181,9 @@ void IndexedDBDatabase::CountOperation( do { ++count; - } while (backing_store_cursor->Continue()); + } while (backing_store_cursor->Continue(&s)); + + // TODO(cmumford): Check for database corruption. callbacks->OnSuccess(count); } @@ -1169,14 +1215,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 +1237,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(); diff --git a/content/browser/indexed_db/indexed_db_database.h b/content/browser/indexed_db/indexed_db_database.h index 16fe79f..42362df 100644 --- a/content/browser/indexed_db/indexed_db_database.h +++ b/content/browser/indexed_db/indexed_db_database.h @@ -60,7 +60,8 @@ class CONTENT_EXPORT IndexedDBDatabase const base::string16& name, IndexedDBBackingStore* backing_store, IndexedDBFactory* factory, - const Identifier& unique_identifier); + const Identifier& unique_identifier, + leveldb::Status* s); const Identifier& identifier() const { return identifier_; } IndexedDBBackingStore* backing_store() { return backing_store_.get(); } diff --git a/content/browser/indexed_db/indexed_db_database_unittest.cc b/content/browser/indexed_db/indexed_db_database_unittest.cc index 5b1e73c..33ba330 100644 --- a/content/browser/indexed_db/indexed_db_database_unittest.cc +++ b/content/browser/indexed_db/indexed_db_database_unittest.cc @@ -35,11 +35,14 @@ TEST(IndexedDBDatabaseTest, BackingStoreRetention) { EXPECT_TRUE(backing_store->HasOneRef()); IndexedDBFactory* factory = 0; - scoped_refptr<IndexedDBDatabase> db = IndexedDBDatabase::Create( - ASCIIToUTF16("db"), - backing_store, - factory, - IndexedDBDatabase::Identifier()); + leveldb::Status s; + scoped_refptr<IndexedDBDatabase> db = + IndexedDBDatabase::Create(ASCIIToUTF16("db"), + backing_store, + factory, + IndexedDBDatabase::Identifier(), + &s); + ASSERT_TRUE(s.ok()); EXPECT_FALSE(backing_store->HasOneRef()); // local and db db = NULL; EXPECT_TRUE(backing_store->HasOneRef()); // local @@ -51,12 +54,14 @@ TEST(IndexedDBDatabaseTest, ConnectionLifecycle) { EXPECT_TRUE(backing_store->HasOneRef()); // local IndexedDBFactory* factory = 0; + leveldb::Status s; scoped_refptr<IndexedDBDatabase> db = IndexedDBDatabase::Create(ASCIIToUTF16("db"), backing_store, factory, - IndexedDBDatabase::Identifier()); - + IndexedDBDatabase::Identifier(), + &s); + ASSERT_TRUE(s.ok()); EXPECT_FALSE(backing_store->HasOneRef()); // local and db scoped_refptr<MockIndexedDBCallbacks> request1(new MockIndexedDBCallbacks()); @@ -107,12 +112,14 @@ TEST(IndexedDBDatabaseTest, ForcedClose) { EXPECT_TRUE(backing_store->HasOneRef()); IndexedDBFactory* factory = 0; + leveldb::Status s; scoped_refptr<IndexedDBDatabase> database = IndexedDBDatabase::Create(ASCIIToUTF16("db"), backing_store, factory, - IndexedDBDatabase::Identifier()); - + IndexedDBDatabase::Identifier(), + &s); + ASSERT_TRUE(s.ok()); EXPECT_FALSE(backing_store->HasOneRef()); // local and db scoped_refptr<MockIndexedDBDatabaseCallbacks> callbacks( @@ -168,12 +175,14 @@ TEST(IndexedDBDatabaseTest, PendingDelete) { EXPECT_TRUE(backing_store->HasOneRef()); // local IndexedDBFactory* factory = 0; + leveldb::Status s; scoped_refptr<IndexedDBDatabase> db = IndexedDBDatabase::Create(ASCIIToUTF16("db"), backing_store, factory, - IndexedDBDatabase::Identifier()); - + IndexedDBDatabase::Identifier(), + &s); + ASSERT_TRUE(s.ok()); EXPECT_FALSE(backing_store->HasOneRef()); // local and db scoped_refptr<MockIndexedDBCallbacks> request1(new MockIndexedDBCallbacks()); diff --git a/content/browser/indexed_db/indexed_db_dispatcher_host.cc b/content/browser/indexed_db/indexed_db_dispatcher_host.cc index 5366443..1f30b50 100644 --- a/content/browser/indexed_db/indexed_db_dispatcher_host.cc +++ b/content/browser/indexed_db/indexed_db_dispatcher_host.cc @@ -909,7 +909,11 @@ void IndexedDBDispatcherHost::CursorDispatcherHost::OnPrefetchReset( if (!idb_cursor) return; - idb_cursor->PrefetchReset(used_prefetches, unused_prefetches); + leveldb::Status s = + idb_cursor->PrefetchReset(used_prefetches, unused_prefetches); + // TODO(cmumford): Handle this error (crbug.com/363397) + if (!s.ok()) + DLOG(ERROR) << "Unable to reset prefetch"; } void IndexedDBDispatcherHost::CursorDispatcherHost::OnDestroyed( diff --git a/content/browser/indexed_db/indexed_db_factory.cc b/content/browser/indexed_db/indexed_db_factory.cc index e214785..663be9d 100644 --- a/content/browser/indexed_db/indexed_db_factory.cc +++ b/content/browser/indexed_db/indexed_db_factory.cc @@ -192,7 +192,13 @@ void IndexedDBFactory::GetDatabaseNames( return; } - callbacks->OnSuccess(backing_store->GetDatabaseNames()); + leveldb::Status s; + std::vector<base::string16> names = backing_store->GetDatabaseNames(&s); + if (!s.ok()) { + // TODO(cmumford): Handle this error + DLOG(ERROR) << "Internal error getting database names"; + } + callbacks->OnSuccess(names); backing_store = NULL; ReleaseBackingStore(origin_url, false /* immediate */); } @@ -233,14 +239,18 @@ void IndexedDBFactory::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()) + HandleBackingStoreCorruption(origin_url, error); return; } @@ -282,9 +292,10 @@ void IndexedDBFactory::HandleBackingStoreCorruption( 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, @@ -389,13 +400,21 @@ void IndexedDBFactory::Open(const base::string16& name, 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 { diff --git a/content/browser/indexed_db/indexed_db_fake_backing_store.cc b/content/browser/indexed_db/indexed_db_fake_backing_store.cc index a40e576..4bb5546 100644 --- a/content/browser/indexed_db/indexed_db_fake_backing_store.cc +++ b/content/browser/indexed_db/indexed_db_fake_backing_store.cc @@ -26,7 +26,9 @@ IndexedDBFakeBackingStore::IndexedDBFakeBackingStore( IndexedDBFakeBackingStore::~IndexedDBFakeBackingStore() {} -std::vector<base::string16> IndexedDBFakeBackingStore::GetDatabaseNames() { +std::vector<base::string16> IndexedDBFakeBackingStore::GetDatabaseNames( + leveldb::Status* s) { + *s = leveldb::Status::OK(); return std::vector<base::string16>(); } leveldb::Status IndexedDBFakeBackingStore::GetIDBDatabaseMetaData( @@ -138,7 +140,8 @@ IndexedDBFakeBackingStore::OpenObjectStoreKeyCursor( int64 database_id, int64 object_store_id, const IndexedDBKeyRange& key_range, - indexed_db::CursorDirection) { + indexed_db::CursorDirection, + leveldb::Status* s) { return scoped_ptr<IndexedDBBackingStore::Cursor>(); } scoped_ptr<IndexedDBBackingStore::Cursor> @@ -147,7 +150,8 @@ IndexedDBFakeBackingStore::OpenObjectStoreCursor( int64 database_id, int64 object_store_id, const IndexedDBKeyRange& key_range, - indexed_db::CursorDirection) { + indexed_db::CursorDirection, + leveldb::Status* s) { return scoped_ptr<IndexedDBBackingStore::Cursor>(); } scoped_ptr<IndexedDBBackingStore::Cursor> @@ -157,7 +161,8 @@ IndexedDBFakeBackingStore::OpenIndexKeyCursor( int64 object_store_id, int64 index_id, const IndexedDBKeyRange& key_range, - indexed_db::CursorDirection) { + indexed_db::CursorDirection, + leveldb::Status* s) { return scoped_ptr<IndexedDBBackingStore::Cursor>(); } scoped_ptr<IndexedDBBackingStore::Cursor> @@ -167,7 +172,8 @@ IndexedDBFakeBackingStore::OpenIndexCursor( int64 object_store_id, int64 index_id, const IndexedDBKeyRange& key_range, - indexed_db::CursorDirection) { + indexed_db::CursorDirection, + leveldb::Status* s) { return scoped_ptr<IndexedDBBackingStore::Cursor>(); } diff --git a/content/browser/indexed_db/indexed_db_fake_backing_store.h b/content/browser/indexed_db/indexed_db_fake_backing_store.h index f4f67d7..5c64370 100644 --- a/content/browser/indexed_db/indexed_db_fake_backing_store.h +++ b/content/browser/indexed_db/indexed_db_fake_backing_store.h @@ -22,7 +22,8 @@ class IndexedDBFakeBackingStore : public IndexedDBBackingStore { IndexedDBFakeBackingStore(); IndexedDBFakeBackingStore(IndexedDBFactory* factory, base::TaskRunner* task_runner); - virtual std::vector<base::string16> GetDatabaseNames() OVERRIDE; + virtual std::vector<base::string16> GetDatabaseNames(leveldb::Status* s) + OVERRIDE; virtual leveldb::Status GetIDBDatabaseMetaData(const base::string16& name, IndexedDBDatabaseMetadata*, bool* found) OVERRIDE; @@ -95,27 +96,30 @@ class IndexedDBFakeBackingStore : public IndexedDBBackingStore { int64 database_id, int64 object_store_id, const IndexedDBKeyRange& key_range, - indexed_db::CursorDirection) OVERRIDE; + indexed_db::CursorDirection, + leveldb::Status*) OVERRIDE; virtual scoped_ptr<Cursor> OpenObjectStoreCursor( Transaction* transaction, int64 database_id, int64 object_store_id, const IndexedDBKeyRange& key_range, - indexed_db::CursorDirection) OVERRIDE; + indexed_db::CursorDirection, + leveldb::Status*) OVERRIDE; virtual scoped_ptr<Cursor> OpenIndexKeyCursor( Transaction* transaction, int64 database_id, int64 object_store_id, int64 index_id, const IndexedDBKeyRange& key_range, - indexed_db::CursorDirection) OVERRIDE; + indexed_db::CursorDirection, + leveldb::Status*) OVERRIDE; virtual scoped_ptr<Cursor> OpenIndexCursor(Transaction* transaction, int64 database_id, int64 object_store_id, int64 index_id, const IndexedDBKeyRange& key_range, - indexed_db::CursorDirection) - OVERRIDE; + indexed_db::CursorDirection, + leveldb::Status*) OVERRIDE; class FakeTransaction : public IndexedDBBackingStore::Transaction { public: diff --git a/content/browser/indexed_db/indexed_db_transaction_unittest.cc b/content/browser/indexed_db/indexed_db_transaction_unittest.cc index 463ebdc..06de9ac 100644 --- a/content/browser/indexed_db/indexed_db_transaction_unittest.cc +++ b/content/browser/indexed_db/indexed_db_transaction_unittest.cc @@ -17,12 +17,22 @@ namespace content { class IndexedDBTransactionTest : public testing::Test { public: IndexedDBTransactionTest() { - IndexedDBFactory* factory = NULL; backing_store_ = new IndexedDBFakeBackingStore(); + CreateDB(); + } + + void CreateDB() { + // DB is created here instead of the constructor to workaround a + // "peculiarity of C++". More info at + // https://code.google.com/p/googletest/wiki/FAQ#My_compiler_complains_that_a_constructor_(or_destructor)_cannot + IndexedDBFactory* factory = NULL; + leveldb::Status s; db_ = IndexedDBDatabase::Create(base::ASCIIToUTF16("db"), backing_store_, factory, - IndexedDBDatabase::Identifier()); + IndexedDBDatabase::Identifier(), + &s); + ASSERT_TRUE(s.ok()); } void RunPostedTasks() { message_loop_.RunUntilIdle(); } diff --git a/content/browser/indexed_db/leveldb/leveldb_database.cc b/content/browser/indexed_db/leveldb/leveldb_database.cc index 96218b3..0efbcd1 100644 --- a/content/browser/indexed_db/leveldb/leveldb_database.cc +++ b/content/browser/indexed_db/leveldb/leveldb_database.cc @@ -378,10 +378,10 @@ class IteratorImpl : public LevelDBIterator { virtual ~IteratorImpl() {} virtual bool IsValid() const OVERRIDE; - virtual void SeekToLast() OVERRIDE; - virtual void Seek(const StringPiece& target) OVERRIDE; - virtual void Next() OVERRIDE; - virtual void Prev() OVERRIDE; + virtual leveldb::Status SeekToLast() OVERRIDE; + virtual leveldb::Status Seek(const StringPiece& target) OVERRIDE; + virtual leveldb::Status Next() OVERRIDE; + virtual leveldb::Status Prev() OVERRIDE; virtual StringPiece Key() const OVERRIDE; virtual StringPiece Value() const OVERRIDE; @@ -398,33 +398,37 @@ IteratorImpl::IteratorImpl(scoped_ptr<leveldb::Iterator> it) : iterator_(it.Pass()) {} void IteratorImpl::CheckStatus() { - const leveldb::Status s = iterator_->status(); + const leveldb::Status& s = iterator_->status(); if (!s.ok()) LOG(ERROR) << "LevelDB iterator error: " << s.ToString(); } bool IteratorImpl::IsValid() const { return iterator_->Valid(); } -void IteratorImpl::SeekToLast() { +leveldb::Status IteratorImpl::SeekToLast() { iterator_->SeekToLast(); CheckStatus(); + return iterator_->status(); } -void IteratorImpl::Seek(const StringPiece& target) { +leveldb::Status IteratorImpl::Seek(const StringPiece& target) { iterator_->Seek(MakeSlice(target)); CheckStatus(); + return iterator_->status(); } -void IteratorImpl::Next() { +leveldb::Status IteratorImpl::Next() { DCHECK(IsValid()); iterator_->Next(); CheckStatus(); + return iterator_->status(); } -void IteratorImpl::Prev() { +leveldb::Status IteratorImpl::Prev() { DCHECK(IsValid()); iterator_->Prev(); CheckStatus(); + return iterator_->status(); } StringPiece IteratorImpl::Key() const { diff --git a/content/browser/indexed_db/leveldb/leveldb_iterator.h b/content/browser/indexed_db/leveldb/leveldb_iterator.h index 96cd6b9..1069a8b 100644 --- a/content/browser/indexed_db/leveldb/leveldb_iterator.h +++ b/content/browser/indexed_db/leveldb/leveldb_iterator.h @@ -6,6 +6,7 @@ #define CONTENT_BROWSER_INDEXED_DB_LEVELDB_LEVELDB_ITERATOR_H_ #include "base/strings/string_piece.h" +#include "third_party/leveldatabase/src/include/leveldb/status.h" namespace content { @@ -13,10 +14,10 @@ class LevelDBIterator { public: virtual ~LevelDBIterator() {} virtual bool IsValid() const = 0; - virtual void SeekToLast() = 0; - virtual void Seek(const base::StringPiece& target) = 0; - virtual void Next() = 0; - virtual void Prev() = 0; + virtual leveldb::Status SeekToLast() = 0; + virtual leveldb::Status Seek(const base::StringPiece& target) = 0; + virtual leveldb::Status Next() = 0; + virtual leveldb::Status Prev() = 0; virtual base::StringPiece Key() const = 0; virtual base::StringPiece Value() const = 0; }; diff --git a/content/browser/indexed_db/leveldb/leveldb_transaction.cc b/content/browser/indexed_db/leveldb/leveldb_transaction.cc index 88839a5..e20d404 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,39 @@ 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(); + DCHECK(s.ok()); + 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); + DCHECK(s.ok()); + 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 +251,9 @@ 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(); + if (!s.ok()) + return s; } DCHECK(!non_current->IsValid() || comparator_->Compare(non_current->Key(), Key()) > 0); @@ -244,13 +261,17 @@ void LevelDBTransaction::TransactionIterator::Next() { direction_ = FORWARD; } - current_->Next(); + s = current_->Next(); + if (!s.ok()) + return s; HandleConflictsAndDeletes(); SetCurrentIteratorToSmallestKey(); + return leveldb::Status::OK(); } -void LevelDBTransaction::TransactionIterator::Prev() { +leveldb::Status LevelDBTransaction::TransactionIterator::Prev() { DCHECK(IsValid()); + leveldb::Status s; if (data_changed_) RefreshDataIterator(); @@ -261,7 +282,9 @@ void LevelDBTransaction::TransactionIterator::Prev() { ? data_iterator_.get() : db_iterator_.get(); - non_current->Seek(Key()); + s = non_current->Seek(Key()); + if (!s.ok()) + return s; if (non_current->IsValid()) { // Iterator is at first entry >= Key(). // Step back once to entry < key. @@ -278,9 +301,12 @@ void LevelDBTransaction::TransactionIterator::Prev() { direction_ = REVERSE; } - current_->Prev(); + s = current_->Prev(); + if (!s.ok()) + return s; HandleConflictsAndDeletes(); SetCurrentIteratorToLargestKey(); + return leveldb::Status::OK(); } StringPiece LevelDBTransaction::TransactionIterator::Key() const { diff --git a/content/browser/indexed_db/leveldb/leveldb_transaction.h b/content/browser/indexed_db/leveldb/leveldb_transaction.h index 8827db8..1102651 100644 --- a/content/browser/indexed_db/leveldb/leveldb_transaction.h +++ b/content/browser/indexed_db/leveldb/leveldb_transaction.h @@ -68,10 +68,10 @@ class CONTENT_EXPORT LevelDBTransaction virtual ~DataIterator(); virtual bool IsValid() const OVERRIDE; - virtual void SeekToLast() OVERRIDE; - virtual void Seek(const base::StringPiece& slice) OVERRIDE; - virtual void Next() OVERRIDE; - virtual void Prev() OVERRIDE; + virtual leveldb::Status SeekToLast() OVERRIDE; + virtual leveldb::Status Seek(const base::StringPiece& slice) OVERRIDE; + virtual leveldb::Status Next() OVERRIDE; + virtual leveldb::Status Prev() OVERRIDE; virtual base::StringPiece Key() const OVERRIDE; virtual base::StringPiece Value() const OVERRIDE; bool IsDeleted() const; @@ -89,10 +89,10 @@ class CONTENT_EXPORT LevelDBTransaction scoped_refptr<LevelDBTransaction> transaction); virtual bool IsValid() const OVERRIDE; - virtual void SeekToLast() OVERRIDE; - virtual void Seek(const base::StringPiece& target) OVERRIDE; - virtual void Next() OVERRIDE; - virtual void Prev() OVERRIDE; + virtual leveldb::Status SeekToLast() OVERRIDE; + virtual leveldb::Status Seek(const base::StringPiece& target) OVERRIDE; + virtual leveldb::Status Next() OVERRIDE; + virtual leveldb::Status Prev() OVERRIDE; virtual base::StringPiece Key() const OVERRIDE; virtual base::StringPiece Value() const OVERRIDE; void DataChanged(); diff --git a/content/test/data/indexeddb/corrupted_open_db_detection.html b/content/test/data/indexeddb/corrupted_open_db_detection.html index 198aed55..2778c02 100644 --- a/content/test/data/indexeddb/corrupted_open_db_detection.html +++ b/content/test/data/indexeddb/corrupted_open_db_detection.html @@ -10,7 +10,10 @@ <script type="text/javascript" src="common.js"></script> <script> +var testType = 'get'; + function test() { + testType = location.hash.substring(1); indexedDBTest(upgradeCallback, openCallback); } @@ -92,6 +95,27 @@ function getData() { request.onerror = requestError; } +function iterateData() { + transaction = db.transaction('storeName'); + db.onclose = databaseClosed; + transaction.onabort = transactionAbort; + transaction.onerror = transactionError; + var store = transaction.objectStore('storeName'); + var request = store.openCursor(); + request.oncomplete = unexpectedCompleteCallback; + request.onerror = requestError; + request.onsuccess = function (event){ + var cursor = request.result; + if (cursor) { + // Get an object. Probably shouldn't get this far, but won't call this an error. + cursor.continue(); + } else { + // Got the last object. We shouldn't get this far. + fail("Should *not* have been able to iterate over database."); + } + }; +} + function openCallback() { var xmlhttp = new window.XMLHttpRequest(); xmlhttp.open("GET", "/corrupt/test/corruptdb?storeName", false /*sync*/); @@ -99,7 +123,13 @@ function openCallback() { if (xmlhttp.readyState === 4) { if (xmlhttp.status === 200) { // The database is now corrupt. - getData(); + if (testType === 'get') { + getData(); + } else if (testType === 'iterate') { + iterateData(); + } else { + fail('Unknown test: "' + testType + '"'); + } } } }; |