Align IndexedDB metadata rollback on transaction abort to spec.

third_party/WebKit/Source/modules/indexeddb/IDBTransaction.cpp

Index: third_party/WebKit/Source/modules/indexeddb/IDBTransaction.cpp
diff --git a/third_party/WebKit/Source/modules/indexeddb/IDBTransaction.cpp b/third_party/WebKit/Source/modules/indexeddb/IDBTransaction.cpp
index 71ccf3d4f7fa6e7cf935eddf98135f78e28983c8..1d1ad267e0128ee4d62c98e0422057b4d893762b 100644
--- a/third_party/WebKit/Source/modules/indexeddb/IDBTransaction.cpp
+++ b/third_party/WebKit/Source/modules/indexeddb/IDBTransaction.cpp
@@ -46,17 +46,19 @@ using blink::WebIDBDatabase;
 
 namespace blink {
 
-IDBTransaction* IDBTransaction::create(ScriptState* scriptState, int64_t id, const HashSet<String>& objectStoreNames, WebIDBTransactionMode mode, IDBDatabase* db)
+IDBTransaction* IDBTransaction::createNonVersionChange(ScriptState* scriptState, int64_t id, const HashSet<String>& objectStoreNames, WebIDBTransactionMode mode, IDBDatabase* db)
 {
+    DCHECK_NE(mode,  WebIDBTransactionModeVersionChange);
+    DCHECK(!objectStoreNames.isEmpty()) << "Non-version transactions should operate on a well-defined set of stores";
     IDBOpenDBRequest* openDBRequest = nullptr;
-    IDBTransaction* transaction = new IDBTransaction(scriptState, id, objectStoreNames, mode, db, openDBRequest, IDBDatabaseMetadata());
+    IDBTransaction* transaction = new IDBTransaction(scriptState, id, objectStoreNames, mode, db, openDBRequest, IDBDatabaseOwnMetadata());
     transaction->suspendIfNeeded();
     return transaction;
 }
 
-IDBTransaction* IDBTransaction::create(ScriptState* scriptState, int64_t id, IDBDatabase* db, IDBOpenDBRequest* openDBRequest, const IDBDatabaseMetadata& previousMetadata)
+IDBTransaction* IDBTransaction::createVersionChange(ScriptState* scriptState, int64_t id, IDBDatabase* db, IDBOpenDBRequest* openDBRequest, const IDBDatabaseOwnMetadata& oldMetadata)
 {
-    IDBTransaction* transaction = new IDBTransaction(scriptState, id, HashSet<String>(), WebIDBTransactionModeVersionChange, db, openDBRequest, previousMetadata);
+    IDBTransaction* transaction = new IDBTransaction(scriptState, id, HashSet<String>(), WebIDBTransactionModeVersionChange, db, openDBRequest, oldMetadata);
     transaction->suspendIfNeeded();
     return transaction;
 }
@@ -85,19 +87,23 @@ private:
 
 } // namespace
 
-IDBTransaction::IDBTransaction(ScriptState* scriptState, int64_t id, const HashSet<String>& objectStoreNames, WebIDBTransactionMode mode, IDBDatabase* db, IDBOpenDBRequest* openDBRequest, const IDBDatabaseMetadata& previousMetadata)
+IDBTransaction::IDBTransaction(ScriptState* scriptState, int64_t id, const HashSet<String>& objectStoreNames, WebIDBTransactionMode mode, IDBDatabase* db, IDBOpenDBRequest* openDBRequest, const IDBDatabaseOwnMetadata& oldMetadata)
     : ActiveScriptWrappable(this)
     , ActiveDOMObject(scriptState->getExecutionContext())
     , m_id(id)
     , m_database(db)
-    , m_objectStoreNames(objectStoreNames)
+    , m_scope(objectStoreNames)
     , m_openDBRequest(openDBRequest)
     , m_mode(mode)
-    , m_previousMetadata(previousMetadata)
+    , m_oldDatabaseMetadata(oldMetadata)
 {
-    if (mode == WebIDBTransactionModeVersionChange) {
+    if (isVersionChange()) {
+        DCHECK(objectStoreNames.isEmpty());
+
         // Not active until the callback.
         m_state = Inactive;
+    } else {
+        DCHECK(!objectStoreNames.isEmpty()) << "Non-versionchange transactions must operate on a well-defined set of stores";
     }
 
     if (m_state == Active)
@@ -118,28 +124,26 @@ DEFINE_TRACE(IDBTransaction)
     visitor->trace(m_error);
     visitor->trace(m_requestList);
     visitor->trace(m_objectStoreMap);
-    visitor->trace(m_createdObjectStores);
-    visitor->trace(m_deletedObjectStores);
-    visitor->trace(m_objectStoreCleanupMap);
+    visitor->trace(m_oldStoreMetadata);
+    visitor->trace(m_deletedIndexes);
     EventTargetWithInlineData::trace(visitor);
     ActiveDOMObject::trace(visitor);
 }
 
 void IDBTransaction::setError(DOMException* error)
 {
-    ASSERT(m_state != Finished);
-    ASSERT(error);
+    DCHECK_NE(m_state, Finished);
+    DCHECK(error);
 
     // The first error to be set is the true cause of the
     // transaction abort.
-    if (!m_error) {
+    if (!m_error)
         m_error = error;
-    }
 }
 
 IDBObjectStore* IDBTransaction::objectStore(const String& name, ExceptionState& exceptionState)
 {
-    if (m_state == Finished) {
+    if (isFinished()) {
         exceptionState.throwDOMException(InvalidStateError, IDBDatabase::transactionFinishedErrorMessage);
         return nullptr;
     }
@@ -148,7 +152,7 @@ IDBObjectStore* IDBTransaction::objectStore(const String& name, ExceptionState&
     if (it != m_objectStoreMap.end())
         return it->value;
 
-    if (!isVersionChange() && !m_objectStoreNames.contains(name)) {
+    if (!isVersionChange() && !m_scope.contains(name)) {
         exceptionState.throwDOMException(NotFoundError, IDBDatabase::noSuchObjectStoreErrorMessage);
         return nullptr;
     }
@@ -162,46 +166,75 @@ IDBObjectStore* IDBTransaction::objectStore(const String& name, ExceptionState&
 
     const IDBDatabaseMetadata& metadata = m_database->metadata();
 
-    IDBObjectStore* objectStore = IDBObjectStore::create(metadata.objectStores.get(objectStoreId), this);
+    RefPtr<IDBObjectStoreMetadata> objectStoreMetadata = metadata.objectStores.get(objectStoreId);
+    DCHECK(objectStoreMetadata.get());
+    IDBObjectStore* objectStore = IDBObjectStore::create(std::move(objectStoreMetadata), this);
+    DCHECK(!m_objectStoreMap.contains(name));
     m_objectStoreMap.set(name, objectStore);
-    m_objectStoreCleanupMap.set(objectStore, objectStore->metadata());
+
+    if (isVersionChange()) {
+        RefPtr<IDBObjectStoreMetadata> backupMetadata = objectStore->metadata().createCopy();
+        m_oldStoreMetadata.set(objectStore, std::move(backupMetadata));
+    }
     return objectStore;
 }
 
 void IDBTransaction::objectStoreCreated(const String& name, IDBObjectStore* objectStore)
 {
-    ASSERT(m_state != Finished);
-    ASSERT(isVersionChange());
+    DCHECK_NE(m_state, Finished) << "A finished transaction created an object store";
+    DCHECK_EQ(m_mode, WebIDBTransactionModeVersionChange) << "A non-versionchange transaction created an object store";
+    DCHECK(!m_objectStoreMap.contains(name)) << "An object store was created with the name of an existing store";
     m_objectStoreMap.set(name, objectStore);
-    m_objectStoreCleanupMap.set(objectStore, objectStore->metadata());
-    m_createdObjectStores.add(objectStore);
 }
 
-void IDBTransaction::objectStoreDeleted(const String& name)
+void IDBTransaction::objectStoreDeleted(const int64_t objectStoreId, const String& name)
 {
-    ASSERT(m_state != Finished);
-    ASSERT(isVersionChange());
+    DCHECK_NE(m_state, Finished) << "A finished transaction deleted an object store";
+    DCHECK_EQ(m_mode, WebIDBTransactionModeVersionChange) << "A non-versionchange transaction deleted an object store";
     IDBObjectStoreMap::iterator it = m_objectStoreMap.find(name);
-    if (it != m_objectStoreMap.end()) {
+    if (it == m_objectStoreMap.end()) {
+        DCHECK(m_database->metadata().objectStores.contains(objectStoreId));
+        RefPtr<IDBObjectStoreMetadata> metadata = m_database->metadata().objectStores.get(objectStoreId);
+        DCHECK(metadata.get());
+        DCHECK(metadata->own.name == name);
+        m_deletedObjectStores.append(std::move(metadata));
+    } else {
         IDBObjectStore* objectStore = it->value;
         m_objectStoreMap.remove(name);
         objectStore->markDeleted();
-        m_objectStoreCleanupMap.set(objectStore, objectStore->metadata());
-        m_deletedObjectStores.add(objectStore);
     }
 }
 
 void IDBTransaction::objectStoreRenamed(const String& oldName, const String& newName)
 {
-    DCHECK(m_state != Finished);
-    DCHECK(isVersionChange());
+    DCHECK_NE(m_state, Finished) << "A finished transaction renamed an object store";
+    DCHECK_EQ(m_mode, WebIDBTransactionModeVersionChange) << "A non-versionchange transaction renamed an object store";
 
     DCHECK(!m_objectStoreMap.contains(newName));
     DCHECK(m_objectStoreMap.contains(oldName)) << "The object store had to be accessed in order to be renamed.";
+    IDBObjectStore* objectStore = m_objectStoreMap.take(oldName);
+    m_objectStoreMap.set(newName, objectStore);
+}
+
+void IDBTransaction::indexDeleted(IDBIndex* index)
+{
+    DCHECK(index);
+    DCHECK(!index->isDeleted()) << "indexDeleted called twice for the same index";
 
-    IDBObjectStoreMap::iterator it = m_objectStoreMap.find(oldName);
-    m_objectStoreMap.set(newName, it->value);
-    m_objectStoreMap.remove(oldName);
+    IDBObjectStore* objectStore = index->objectStore();
+    DCHECK(objectStore->transaction() == this);
+    DCHECK(m_objectStoreMap.contains(objectStore->name())) << "An index was deleted without accessing its object store";
+
+    const auto& objectStoreIterator = m_oldStoreMetadata.find(objectStore);
+    if (objectStoreIterator == m_oldStoreMetadata.end())
+        return;
+
+    const IDBObjectStoreMetadata* oldStoreMetadata = objectStoreIterator->value.get();
+    DCHECK(oldStoreMetadata);
+    if (!oldStoreMetadata->indexes.contains(index->id()))
+        return;
+
+    m_deletedIndexes.append(index);
 }
 
 void IDBTransaction::setActive(bool active)
@@ -228,14 +261,8 @@ void IDBTransaction::abort(ExceptionState& exceptionState)
     if (m_contextStopped)
         return;
 
-    for (IDBRequest* request : m_requestList)
-        request->abort();
-    m_requestList.clear();
-
-    for (IDBObjectStore* store : m_createdObjectStores) {
-        store->abort();
-        store->markDeleted();
-    }
+    abortOutstandingRequests();
+    revertDatabaseMetadata();
 
     if (backendDB())
         backendDB()->abort(m_id);
@@ -259,7 +286,7 @@ void IDBTransaction::onAbort(DOMException* error)
 {
     IDB_TRACE("IDBTransaction::onAbort");
     if (m_contextStopped) {
-        m_database->transactionFinished(this);
+        finish();
         return;
     }
 
@@ -269,52 +296,34 @@ void IDBTransaction::onAbort(DOMException* error)
         ASSERT(error);
         setError(error);
 
-        // Outstanding requests must be aborted.
-        for (IDBRequest* request : m_requestList)
-            request->abort();
-        m_requestList.clear();
-
-        // Newly created stores must be marked as deleted.
-        for (IDBObjectStore* store : m_createdObjectStores)
-            store->markDeleted();
-
-        // Used stores may need to mark indexes as deleted.
-        for (auto& it : m_objectStoreCleanupMap)
-            it.key->abort();
+        abortOutstandingRequests();
+        revertDatabaseMetadata();
 
         m_state = Finishing;
     }
 
-    if (isVersionChange()) {
-        for (auto& it : m_objectStoreCleanupMap)
-            it.key->setMetadata(it.value);
-        m_database->setMetadata(m_previousMetadata);
+    if (isVersionChange())
         m_database->close();
-    }
-    m_objectStoreCleanupMap.clear();
 
     // Enqueue events before notifying database, as database may close which enqueues more events and order matters.
     enqueueEvent(Event::createBubble(EventTypeNames::abort));
-
-    m_database->transactionFinished(this);
+    finish();
 }
 
 void IDBTransaction::onComplete()
 {
     IDB_TRACE("IDBTransaction::onComplete");
     if (m_contextStopped) {
-        m_database->transactionFinished(this);
+        finish();
         return;
     }
 
     ASSERT(m_state != Finished);
     m_state = Finishing;
-    m_objectStoreCleanupMap.clear();
 
     // Enqueue events before notifying database, as database may close which enqueues more events and order matters.
     enqueueEvent(Event::create(EventTypeNames::complete));
-
-    m_database->transactionFinished(this);
+    finish();
 }
 
 bool IDBTransaction::hasPendingActivity() const
@@ -337,6 +346,11 @@ WebIDBTransactionMode IDBTransaction::stringToMode(const String& modeString)
     return WebIDBTransactionModeReadOnly;
 }
 
+WebIDBDatabase* IDBTransaction::backendDB() const
+{
+    return m_database->backend();
+}
+
 const String& IDBTransaction::mode() const
 {
     switch (m_mode) {
@@ -356,12 +370,12 @@ const String& IDBTransaction::mode() const
 
 DOMStringList* IDBTransaction::objectStoreNames() const
 {
-    if (m_mode == WebIDBTransactionModeVersionChange)
+    if (isVersionChange())
         return m_database->objectStoreNames();
 
     DOMStringList* objectStoreNames = DOMStringList::create(DOMStringList::IndexedDB);
-    for (const String& name : m_objectStoreNames)
-        objectStoreNames->append(name);
+    for (const String& objectStoreName : m_scope)
+        objectStoreNames->append(objectStoreName);
     objectStoreNames->sort();
     return objectStoreNames;
 }
@@ -389,16 +403,6 @@ DispatchEventResult IDBTransaction::dispatchEventInternal(Event* event)
     ASSERT(event->target() == this);
     m_state = Finished;
 
-    // Break reference cycles.
-    // TODO(jsbell): This can be removed c/o Oilpan.
-    for (auto& it : m_objectStoreMap)
-        it.value->transactionFinished();
-    m_objectStoreMap.clear();
-    for (auto& it : m_deletedObjectStores)
-        it->transactionFinished();
-    m_createdObjectStores.clear();
-    m_deletedObjectStores.clear();
-
     HeapVector<Member<EventTarget>> targets;
     targets.append(this);
     targets.append(db());
@@ -437,9 +441,64 @@ void IDBTransaction::enqueueEvent(Event* event)
     eventQueue->enqueueEvent(event);
 }
 
-WebIDBDatabase* IDBTransaction::backendDB() const
+void IDBTransaction::abortOutstandingRequests()
 {
-    return m_database->backend();
+    for (IDBRequest* request : m_requestList)
+        request->abort();
+    m_requestList.clear();
+}
+
+void IDBTransaction::revertDatabaseMetadata()
+{
+    DCHECK_NE(m_state, Active);
+    if (!isVersionChange())
+        return;
+
+    // Mark stores created by this transaction as deleted.
+    // Object store IDs are allocated sequentially, so we can tell if an object
+    // store was created in this transaction by comparing its ID against the
+    // database's maximum object store ID at the time when the transaction was
+    // started.
+    const int64_t oldMaxObjectStoreId = m_oldDatabaseMetadata.maxObjectStoreId;
+    for (auto& it : m_objectStoreMap) {
+        IDBObjectStore* objectStore = it.value;
+        const int64_t objectStoreId = objectStore->id();
+        if (objectStoreId < oldMaxObjectStoreId)
+            continue;
+
+        m_database->revertObjectStoreCreation(objectStoreId);
+        objectStore->markDeleted();
+    }
+
+    for (auto& it : m_oldStoreMetadata) {
+        IDBObjectStore* objectStore = it.key;
+        RefPtr<IDBObjectStoreMetadata> oldMetadata = it.value;
+
+        m_database->revertObjectStoreMetadata(oldMetadata);
+        objectStore->revertMetadata(oldMetadata);
+    }
+    for (auto& it : m_deletedIndexes) {
+        IDBIndex* index = static_cast<IDBIndex*>(it);
+        index->objectStore()->revertDeletedIndexMetadata(*index);
+    }
+    for (auto& it: m_deletedObjectStores) {
+        RefPtr<IDBObjectStoreMetadata> oldMedata = static_cast<RefPtr<IDBObjectStoreMetadata>>(it);
+        m_database->revertObjectStoreMetadata(std::move(it));
+    }
+    m_database->setOwnMetadata(m_oldDatabaseMetadata);
+}
+
+void IDBTransaction::finish()
+{
+    m_database->transactionFinished(this);
+
+    // Remove references to the IDBObjectStore and IDBIndex instances held by
+    // this transaction, so OilPan can garbage-collect the instances that aren't
+    // used by JavaScript.
+    m_objectStoreMap.clear();
+    m_oldStoreMetadata.clear();
+    m_deletedIndexes.clear();
+    m_deletedObjectStores.clear();
 }
 
 } // namespace blink