Align IndexedDB metadata rollback on transaction abort to spec.

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

 /*
  * Copyright (C) 2010 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
@@ skipping 44 matching lines @@
 using blink::WebIDBDatabase;
 using blink::WebVector;
 
 namespace blink {
 
 namespace {
 
 using IndexKeys = HeapVector<Member<IDBKey>>;
 }
 
-IDBObjectStore::IDBObjectStore(const IDBObjectStoreMetadata& metadata,
+IDBObjectStore::IDBObjectStore(RefPtr<IDBObjectStoreMetadata> metadata,
                                IDBTransaction* transaction)
-    : m_metadata(metadata), m_transaction(transaction) {
+    : m_metadata(std::move(metadata)), m_transaction(transaction) {
   DCHECK(m_transaction);
+  DCHECK(m_metadata.get());
 }
 
 DEFINE_TRACE(IDBObjectStore) {
   visitor->trace(m_transaction);
   visitor->trace(m_indexMap);
-  visitor->trace(m_createdIndexes);
 }
 
 void IDBObjectStore::setName(const String& name,
                              ExceptionState& exceptionState) {
   if (!RuntimeEnabledFeatures::indexedDBExperimentalEnabled())
     return;
 
   IDB_TRACE("IDBObjectStore::setName");
   if (!m_transaction->isVersionChange()) {
     exceptionState.throwDOMException(
@@ skipping 23 matching lines @@
     exceptionState.throwDOMException(
         ConstraintError, IDBDatabase::objectStoreNameTakenErrorMessage);
     return;
   }
   if (!backendDB()) {
     exceptionState.throwDOMException(InvalidStateError,
                                      IDBDatabase::databaseClosedErrorMessage);
     return;
   }
 
-  backendDB()->renameObjectStore(m_transaction->id(), id(), name);

[jsbell, 2016/10/06 20:01:31]

IDBIndex::setName calls WebIDBDatabase::renameIndex directly, but
IDBObjectStore::setName relies on IDBDatabase to call
WebIDBDatabase::renameObjectStore - can we make these consistent?

[pwnall, 2016/10/06 22:15:22]

Done.
This is a great suggestion! Thank you!

-  m_transaction->objectStoreRenamed(m_metadata.name, name);
-  m_metadata.name = name;
-
-  // The name inside the database's version of the object store metadata is used
-  // by IDBDatabase.objectStoreNames().  If the transaction is aborted, this
-  // name will be reverted when the metadata is overwritten with the
-  // previousMetadata in IDBTransaction.
-  m_transaction->db()->objectStoreRenamed(id(), name);
+  m_transaction->db()->renameObjectStore(id(), name);
 }
 
 ScriptValue IDBObjectStore::keyPath(ScriptState* scriptState) const {
   return ScriptValue::from(scriptState, metadata().keyPath);
 }
 
 DOMStringList* IDBObjectStore::indexNames() const {
   IDB_TRACE("IDBObjectStore::indexNames");
   DOMStringList* indexNames = DOMStringList::create(DOMStringList::IndexedDB);
   for (const auto& it : metadata().indexes)
-    indexNames->append(it.value.name);
+    indexNames->append(it.value->name);
   indexNames->sort();
   return indexNames;
 }
 
 IDBRequest* IDBObjectStore::get(ScriptState* scriptState,
                                 const ScriptValue& key,
                                 ExceptionState& exceptionState) {
   IDB_TRACE("IDBObjectStore::get");
   if (isDeleted()) {
     exceptionState.throwDOMException(
@@ skipping 355 matching lines @@
     return nullptr;
   }
 
   Vector<int64_t> indexIds;
   HeapVector<IndexKeys> indexKeys;
   for (const auto& it : metadata().indexes) {
     if (clone.isEmpty())
       clone =
           deserializeScriptValue(scriptState, serializedValue.get(), &blobInfo);
     IndexKeys keys;
-    generateIndexKeysForValue(scriptState->isolate(), it.value, clone, &keys);
+    generateIndexKeysForValue(scriptState->isolate(), *it.value, clone, &keys);
     indexIds.append(it.key);
     indexKeys.append(keys);
   }
 
   IDBRequest* request =
       IDBRequest::create(scriptState, source, m_transaction.get());
   Vector<char> wireBytes;
   serializedValue->toWireBytes(wireBytes);
   RefPtr<SharedBuffer> valueBuffer = SharedBuffer::adoptVector(wireBytes);
 
@@ skipping 92 matching lines @@
 // them from the SerializedScriptValue, for all the existing values in
 // the objectStore. It only needs to be kept alive by virtue of being
 // a listener on an IDBRequest object, in the same way that JavaScript
 // cursor success handlers are kept alive.
 class IndexPopulator final : public EventListener {
  public:
   static IndexPopulator* create(ScriptState* scriptState,
                                 IDBDatabase* database,
                                 int64_t transactionId,
                                 int64_t objectStoreId,
-                                const IDBIndexMetadata& indexMetadata) {
+                                RefPtr<const IDBIndexMetadata> indexMetadata) {
     return new IndexPopulator(scriptState, database, transactionId,
-                              objectStoreId, indexMetadata);
+                              objectStoreId, std::move(indexMetadata));
   }
 
   bool operator==(const EventListener& other) const override {
     return this == &other;
   }
 
   DEFINE_INLINE_VIRTUAL_TRACE() {
     visitor->trace(m_database);
     EventListener::trace(visitor);
   }
 
  private:
   IndexPopulator(ScriptState* scriptState,
                  IDBDatabase* database,
                  int64_t transactionId,
                  int64_t objectStoreId,
-                 const IDBIndexMetadata& indexMetadata)
+                 RefPtr<const IDBIndexMetadata> indexMetadata)
       : EventListener(CPPEventListenerType),
         m_scriptState(scriptState),
         m_database(database),
         m_transactionId(transactionId),
         m_objectStoreId(objectStoreId),
-        m_indexMetadata(indexMetadata) {}
+        m_indexMetadata(std::move(indexMetadata)) {
+    DCHECK(m_indexMetadata.get());
+  }
 
-  const IDBIndexMetadata& indexMetadata() const { return m_indexMetadata; }
+  const IDBIndexMetadata& indexMetadata() const { return *m_indexMetadata; }
 
   void handleEvent(ExecutionContext* executionContext, Event* event) override {
     DCHECK_EQ(m_scriptState->getExecutionContext(), executionContext);
     DCHECK_EQ(event->type(), EventTypeNames::success);
     EventTarget* target = event->target();
     IDBRequest* request = static_cast<IDBRequest*>(target);
 
     if (!m_database->backend())  // If database is stopped?
       return;
 
@@ skipping 25 matching lines @@
       m_database->backend()->setIndexesReady(m_transactionId, m_objectStoreId,
                                              indexIds);
       m_database.clear();
     }
   }
 
   RefPtr<ScriptState> m_scriptState;
   Member<IDBDatabase> m_database;
   const int64_t m_transactionId;
   const int64_t m_objectStoreId;
-  const IDBIndexMetadata m_indexMetadata;
+  RefPtr<const IDBIndexMetadata> m_indexMetadata;
 };
 }  // namespace
 
 IDBIndex* IDBObjectStore::createIndex(ScriptState* scriptState,
                                       const String& name,
                                       const IDBKeyPath& keyPath,
                                       const IDBIndexParameters& options,
                                       ExceptionState& exceptionState) {
   IDB_TRACE("IDBObjectStore::createIndex");
   if (!m_transaction->isVersionChange()) {
@@ skipping 32 matching lines @@
         InvalidAccessError,
         "The keyPath argument was an array and the multiEntry option is true.");
     return nullptr;
   }
   if (!backendDB()) {
     exceptionState.throwDOMException(InvalidStateError,
                                      IDBDatabase::databaseClosedErrorMessage);
     return nullptr;
   }
 
-  int64_t indexId = m_metadata.maxIndexId + 1;
+  int64_t indexId = m_metadata->maxIndexId + 1;
   DCHECK_NE(indexId, IDBIndexMetadata::InvalidId);
   backendDB()->createIndex(m_transaction->id(), id(), indexId, name, keyPath,
                            options.unique(), options.multiEntry());
 
-  ++m_metadata.maxIndexId;
+  ++m_metadata->maxIndexId;
 
-  IDBIndexMetadata indexMetadata(name, indexId, keyPath, options.unique(),
-                                 options.multiEntry());
+  RefPtr<IDBIndexMetadata> indexMetadata = adoptRef(new IDBIndexMetadata(
+      name, indexId, keyPath, options.unique(), options.multiEntry()));
   IDBIndex* index = IDBIndex::create(indexMetadata, this, m_transaction.get());
   m_indexMap.set(name, index);
-  m_createdIndexes.add(index);
-  m_metadata.indexes.set(indexId, indexMetadata);
-  m_transaction->db()->indexCreated(id(), indexMetadata);
+  m_metadata->indexes.set(indexId, indexMetadata);
 
   DCHECK(!exceptionState.hadException());
   if (exceptionState.hadException())
     return nullptr;
 
   IDBRequest* indexRequest =
       openCursor(scriptState, nullptr, WebIDBCursorDirectionNext,
                  WebIDBTaskTypePreemptive);
   indexRequest->preventPropagation();
 
   // This is kept alive by being the success handler of the request, which is in
   // turn kept alive by the owning transaction.
-  IndexPopulator* indexPopulator =
-      IndexPopulator::create(scriptState, transaction()->db(),
-                             m_transaction->id(), id(), indexMetadata);
+  IndexPopulator* indexPopulator = IndexPopulator::create(
+      scriptState, transaction()->db(), m_transaction->id(), id(),
+      std::move(indexMetadata));
   indexRequest->setOnsuccess(indexPopulator);
   return index;
 }
 
 IDBIndex* IDBObjectStore::index(const String& name,
                                 ExceptionState& exceptionState) {
   IDB_TRACE("IDBObjectStore::index");
   if (isDeleted()) {
     exceptionState.throwDOMException(
         InvalidStateError, IDBDatabase::objectStoreDeletedErrorMessage);
@@ skipping 10 matching lines @@
     return it->value;
 
   int64_t indexId = findIndexId(name);
   if (indexId == IDBIndexMetadata::InvalidId) {
     exceptionState.throwDOMException(NotFoundError,
                                      IDBDatabase::noSuchIndexErrorMessage);
     return nullptr;
   }
 
   DCHECK(metadata().indexes.contains(indexId));
-  const IDBIndexMetadata& indexMetadata = metadata().indexes.get(indexId);
-  IDBIndex* index = IDBIndex::create(indexMetadata, this, m_transaction.get());
+  RefPtr<IDBIndexMetadata> indexMetadata = metadata().indexes.get(indexId);
+  DCHECK(indexMetadata.get());
+  IDBIndex* index =
+      IDBIndex::create(std::move(indexMetadata), this, m_transaction.get());
   m_indexMap.set(name, index);
   return index;
 }
 
 void IDBObjectStore::deleteIndex(const String& name,
                                  ExceptionState& exceptionState) {
   IDB_TRACE("IDBObjectStore::deleteIndex");
   if (!m_transaction->isVersionChange()) {
     exceptionState.throwDOMException(
         InvalidStateError,
@@ skipping 22 matching lines @@
     return;
   }
   if (!backendDB()) {
     exceptionState.throwDOMException(InvalidStateError,
                                      IDBDatabase::databaseClosedErrorMessage);
     return;
   }
 
   backendDB()->deleteIndex(m_transaction->id(), id(), indexId);
 
-  m_metadata.indexes.remove(indexId);
-  m_transaction->db()->indexDeleted(id(), indexId);
+  m_metadata->indexes.remove(indexId);
   IDBIndexMap::iterator it = m_indexMap.find(name);
   if (it != m_indexMap.end()) {
+    m_transaction->indexDeleted(it->value);
     it->value->markDeleted();
     m_indexMap.remove(name);
   }
 }
 
 IDBRequest* IDBObjectStore::openCursor(ScriptState* scriptState,
                                        const ScriptValue& range,
                                        const String& directionString,
                                        ExceptionState& exceptionState) {
   IDB_TRACE("IDBObjectStore::openCursor");
@@ skipping 122 matching lines @@
   IDBRequest* request = IDBRequest::create(scriptState, IDBAny::create(this),
                                            m_transaction.get());
   backendDB()->count(m_transaction->id(), id(), IDBIndexMetadata::InvalidId,
                      keyRange, WebIDBCallbacksImpl::create(request).release());
   return request;
 }
 
 void IDBObjectStore::markDeleted() {
   DCHECK(m_transaction->isVersionChange())
       << "An object store got deleted outside a versionchange transaction.";
+
   m_deleted = true;
+  m_metadata->indexes.clear();
+
+  for (auto& it : m_indexMap) {
+    IDBIndex* index = it.value;
+    index->markDeleted();
+  }
 }
 
-void IDBObjectStore::abort() {
-  for (auto& index : m_createdIndexes)
-    index->markDeleted();
+void IDBObjectStore::clearIndexCache() {
+  DCHECK(!m_transaction->isActive() ||
+         (isDeleted() && id() > m_transaction->oldMaxObjectStoreId()));
+
+// There is no harm in having clearIndexCache() happen multiple times for
+// the same object. We assert that it is called once to uncover potential
+// object store accounting bugs.
+#if DCHECK_IS_ON()
+  DCHECK(!m_clearIndexCacheCalled);
+  m_clearIndexCacheCalled = true;
+#endif  // DCHECK_IS_ON()
+
+  m_indexMap.clear();
 }
 
-void IDBObjectStore::transactionFinished() {
+void IDBObjectStore::revertMetadata(
+    RefPtr<IDBObjectStoreMetadata> oldMetadata) {
+  DCHECK(m_transaction->isVersionChange());
   DCHECK(!m_transaction->isActive());
+  DCHECK(oldMetadata.get());
+  DCHECK(id() == oldMetadata->id);
 
-  // Break reference cycles.
-  // TODO(jsbell): This can be removed c/o Oilpan.
-  m_indexMap.clear();
-  m_createdIndexes.clear();
+  // Index IDs are allocated sequentially, so we can tell if an index was
+  // created in this transaction by comparing its ID with the object store's
+  // maximum index ID at the time when the transaction was started.
+  const int64_t oldMaxIndexId = oldMetadata->maxIndexId;
+  for (auto& it : m_indexMap) {
+    IDBIndex* index = it.value;
+    const int64_t indexId = index->id();
+
+    if (indexId > oldMaxIndexId) {
+      // The index was created by this transaction. According to the spec,
+      // its metadata will remain as-is.
+      DCHECK(!oldMetadata->indexes.contains(indexId));
+      index->markDeleted();
+      continue;
+    }
+
+    // The index was created in a previous transaction. We need to revert
+    // its metadata. The index might have been deleted, so we
+    // unconditionally reset the deletion marker.
+    DCHECK(oldMetadata->indexes.contains(indexId));
+    RefPtr<IDBIndexMetadata> oldIndexMetadata =
+        oldMetadata->indexes.get(indexId);
+    index->revertMetadata(std::move(oldIndexMetadata));
+  }
+  m_metadata = std::move(oldMetadata);
+
+  // An object store's metadata will only get reverted if the index was in the
+  // database when the versionchange transaction started.
+  m_deleted = false;
 }
 
-void IDBObjectStore::indexRenamed(int64_t indexId, const String& newName) {
+void IDBObjectStore::revertDeletedIndexMetadata(IDBIndex& deletedIndex) {
+  DCHECK(m_transaction->isVersionChange());
+  DCHECK(!m_transaction->isActive());
+  DCHECK(deletedIndex.objectStore() == this);
+  DCHECK(deletedIndex.isDeleted());
+
+  const int64_t indexId = deletedIndex.id();
+  DCHECK(m_metadata->indexes.contains(indexId))
+      << "The object store's metadata was not correctly reverted";
+  RefPtr<IDBIndexMetadata> oldIndexMetadata = m_metadata->indexes.get(indexId);
+  deletedIndex.revertMetadata(std::move(oldIndexMetadata));
+}
+
+void IDBObjectStore::renameIndex(int64_t indexId, const String& newName) {
   DCHECK(m_transaction->isVersionChange());
   DCHECK(m_transaction->isActive());
 
-  auto metadataIterator = m_metadata.indexes.find(indexId);
-  DCHECK_NE(metadataIterator, m_metadata.indexes.end()) << "Invalid indexId";
-  const String& oldName = metadataIterator->value.name;
+  auto metadataIterator = m_metadata->indexes.find(indexId);
+  DCHECK_NE(metadataIterator, m_metadata->indexes.end()) << "Invalid indexId";
+  const String& oldName = metadataIterator->value->name;
 
   DCHECK(m_indexMap.contains(oldName))
       << "The index had to be accessed in order to be renamed.";
   DCHECK(!m_indexMap.contains(newName));
   m_indexMap.set(newName, m_indexMap.take(oldName));
 
-  metadataIterator->value.name = newName;
+  metadataIterator->value->name = newName;
 }
 
 int64_t IDBObjectStore::findIndexId(const String& name) const {
   for (const auto& it : metadata().indexes) {
-    if (it.value.name == name) {
+    if (it.value->name == name) {
       DCHECK_NE(it.key, IDBIndexMetadata::InvalidId);
       return it.key;
     }
   }
   return IDBIndexMetadata::InvalidId;
 }
 
 WebIDBDatabase* IDBObjectStore::backendDB() const {
   return m_transaction->backendDB();
 }
 
 }  // namespace blink