Chromium developer reports an error in gecko's IDB implementation in the following corner case after the deletion of an index is aborted: https://codereview.chromium.org/2314933005/diff/20001/third_party/WebKit/LayoutTests/storage/indexeddb/abort-metadata-revert.html#newcode383 The problem is that after txn.abort(), the deleted index shall not be in NS_ERROR_DOM_INDEXEDDB_NOT_ALLOWED_ERR [1] but in NS_ERROR_DOM_INDEXEDDB_TRANSACTION_INACTIVE_ERR [2] in IDBIndex::GetInternal(). The attached test case shows the inconsistent results of the abortion of the deletion in IDBIndex and IDBObjectStore. [1] http://searchfox.org/mozilla-central/rev/591354c5e5521cf845bf6b6ef44e8b3b0aeda17d/dom/indexedDB/IDBIndex.cpp#323 [2] http://searchfox.org/mozilla-central/rev/591354c5e5521cf845bf6b6ef44e8b3b0aeda17d/dom/indexedDB/IDBIndex.cpp#329

Bug 755511 contains the fix for deleted objectstores. [1] We should do something similar for reverting deleted indexes. [1] http://searchfox.org/mozilla-central/rev/6b94deded39a868f248a492e74e05733d6c4ed48/dom/indexedDB/IDBTransaction.cpp#659-703

1. RefreshSpec for deleted IDBIndexes to reset the IDBIndex::mDeletedMetadata when version change transaction is aborted. (Same logic has already been provided in IDBTransaction::Refresh()). 2. Remove improper MOZ_ASSERT(transaction->IsOpen()) to prevent crash in debug build when IDBObjectStore::CreateIndex(), IDBObjectStore::DeleteIndex() and IDBObjectStore::SetName() are invoked right after IDBTransaction::Abort() is invoked(). Note: We don't do the same stuff of ensuring the correctness of mIndexes (which could be time-consuming after multiplied the iteration of mObjectStores) as we did in IDBTransaction::AbortInternal()[1] for mObjectStores/mDeletedObjectStores because the mIndexes won't be accessed anymore in IDBObjectStore::CreateIndex(), IDBObjectStore::DeleteIndex() and IDBObjectStore::Index() while all of them are returned earlier when !transaction->IsOpen(). That is, the correctness of IDBObjectStore::mIndexes is not important after its transaction is aborted. [1] http://searchfox.org/mozilla-central/rev/6b94deded39a868f248a492e74e05733d6c4ed48/dom/indexedDB/IDBTransaction.cpp#659-703

Comment on attachment 8791772 [details] [diff] [review] (v1) Patch: Refresh Spec For Deleted Indexes. Review of attachment 8791772 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/indexedDB/test/mochitest.ini @@ +37,5 @@ > unit/test_cursor_update_updates_indexes.js > unit/test_cursors.js > unit/test_database_onclose.js > + unit/test_abort_deleted_index.js > + unit/test_abort_deleted_objectStore.js Nit: alphabet order ? @@ +177,5 @@ > skip-if = (buildapp == 'b2g' && toolkit != 'gonk') # Bug 931116 > +[test_abort_deleted_index.html] > +skip-if = (buildapp == 'b2g' && toolkit != 'gonk') # Bug 931116 > +[test_abort_deleted_objectStore.html] > +skip-if = (buildapp == 'b2g' && toolkit != 'gonk') # Bug 931116 Nit: alphabet order ? ::: dom/indexedDB/test/unit/xpcshell-shared.ini @@ +24,5 @@ > [test_deleteDatabase_interactions.js] > [test_deleteDatabase_onblocked.js] > [test_deleteDatabase_onblocked_duringVersionChange.js] > +[test_abort_deleted_index.js] > +[test_abort_deleted_objectStore.js] Nit: alphabet order ?

(In reply to Bevis Tseng[:bevistseng][:btseng] from comment #3) > We don't do the same stuff of ensuring the correctness of mIndexes (which > could be time-consuming after multiplied the iteration of mObjectStores) as > we did in IDBTransaction::AbortInternal()[1] for > mObjectStores/mDeletedObjectStores because the mIndexes won't be accessed > anymore in IDBObjectStore::CreateIndex(), IDBObjectStore::DeleteIndex() and > IDBObjectStore::Index() while all of them are returned earlier when > !transaction->IsOpen(). > That is, the correctness of IDBObjectStore::mIndexes is not important after > its transaction is aborted. Can you add this comment to the code ?

(In reply to Jan Varga [:janv] from comment #5) > Can you add this comment to the code ? Sure, will do! :)

address nits in comment 4 and add more explanation in IDBTransaction::AbortInternal() about why a comprehensive reversion of cached IDBIndexes in IDBObjectstore is unnecessary.

treeherder results for reference: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f4e530d4f928

Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/73f6926645a9 Refresh Spec For Deleted Indexes. r=janv