Align IndexedDB metadata rollback on transaction abort to spec.

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

Index: third_party/WebKit/Source/modules/indexeddb/IDBDatabase.cpp
diff --git a/third_party/WebKit/Source/modules/indexeddb/IDBDatabase.cpp b/third_party/WebKit/Source/modules/indexeddb/IDBDatabase.cpp
index 27ac511383a68a6b98a538db508c9a15e929c52a..77b31053f265a1612efe320d3ec5dfb7cf9a2d8b 100644
--- a/third_party/WebKit/Source/modules/indexeddb/IDBDatabase.cpp
+++ b/third_party/WebKit/Source/modules/indexeddb/IDBDatabase.cpp
@@ -126,33 +126,15 @@ int64_t IDBDatabase::nextTransactionId() {
   return atomicIncrement(&currentTransactionId);
 }
 
-void IDBDatabase::indexCreated(int64_t objectStoreId,
-                               const IDBIndexMetadata& metadata) {
-  IDBDatabaseMetadata::ObjectStoreMap::iterator it =
-      m_metadata.objectStores.find(objectStoreId);
-  ASSERT_WITH_SECURITY_IMPLICATION(it != m_metadata.objectStores.end());
-  it->value.indexes.set(metadata.id, metadata);
+void IDBDatabase::setMetadata(const IDBDatabaseMetadata& metadata) {
+  m_metadata = metadata;
 }
 
-void IDBDatabase::indexDeleted(int64_t objectStoreId, int64_t indexId) {
-  IDBDatabaseMetadata::ObjectStoreMap::iterator it =
-      m_metadata.objectStores.find(objectStoreId);
-  ASSERT_WITH_SECURITY_IMPLICATION(it != m_metadata.objectStores.end());
-  it->value.indexes.remove(indexId);
-}
-
-void IDBDatabase::indexRenamed(int64_t objectStoreId,
-                               int64_t indexId,
-                               const String& newName) {
-  IDBDatabaseMetadata::ObjectStoreMap::iterator storeIterator =
-      m_metadata.objectStores.find(objectStoreId);
-  SECURITY_DCHECK(storeIterator != m_metadata.objectStores.end());
-
-  IDBObjectStoreMetadata& storeMetadata = storeIterator->value;
-  IDBObjectStoreMetadata::IndexMap::iterator indexIterator =
-      storeMetadata.indexes.find(indexId);
-  DCHECK_NE(indexIterator, storeMetadata.indexes.end());
-  indexIterator->value.name = newName;
+void IDBDatabase::setDatabaseMetadata(const IDBDatabaseMetadata& metadata) {

[cmumford, 2016/10/05 21:15:26]

I worry that the knowledge of which members to copy, and which to leave as-is
exists here in IDBDatabase. If, in the future, a new member is added to
IDBDatabaseMetadata, how will that author know to come here. Can this be moved
into a new method in IDBDatabaseMetadata?

[pwnall, 2016/10/05 23:15:45]

Done.
Thank you! I didn't like the assignments, but I couldn't come up with a fix
right away. This is a really good idea!

+  m_metadata.name = metadata.name;
+  m_metadata.id = metadata.id;
+  m_metadata.version = metadata.version;
+  m_metadata.maxObjectStoreId = metadata.maxObjectStoreId;
 }
 
 void IDBDatabase::transactionCreated(IDBTransaction* transaction) {
@@ -195,7 +177,7 @@ DOMStringList* IDBDatabase::objectStoreNames() const {
   DOMStringList* objectStoreNames =
       DOMStringList::create(DOMStringList::IndexedDB);
   for (const auto& it : m_metadata.objectStores)
-    objectStoreNames->append(it.value.name);
+    objectStoreNames->append(it.value->name);
   objectStoreNames->sort();
   return objectStoreNames;
 }
@@ -203,7 +185,7 @@ DOMStringList* IDBDatabase::objectStoreNames() const {
 const String& IDBDatabase::getObjectStoreName(int64_t objectStoreId) const {
   const auto& it = m_metadata.objectStores.find(objectStoreId);
   DCHECK(it != m_metadata.objectStores.end());
-  return it->value.name;
+  return it->value->name;
 }
 
 IDBObjectStore* IDBDatabase::createObjectStore(const String& name,
@@ -259,17 +241,19 @@ IDBObjectStore* IDBDatabase::createObjectStore(const String& name,
   }
 
   int64_t objectStoreId = m_metadata.maxObjectStoreId + 1;
+  DCHECK_NE(objectStoreId, IDBObjectStoreMetadata::InvalidId);
   m_backend->createObjectStore(m_versionChangeTransaction->id(), objectStoreId,
                                name, keyPath, autoIncrement);
 
-  IDBObjectStoreMetadata metadata(name, objectStoreId, keyPath, autoIncrement,
-                                  WebIDBDatabase::minimumIndexId);
+  RefPtr<IDBObjectStoreMetadata> storeMetadata = adoptRef(
+      new IDBObjectStoreMetadata(name, objectStoreId, keyPath, autoIncrement,
+                                 WebIDBDatabase::minimumIndexId));
   IDBObjectStore* objectStore =
-      IDBObjectStore::create(metadata, m_versionChangeTransaction.get());
-  m_metadata.objectStores.set(metadata.id, metadata);
+      IDBObjectStore::create(storeMetadata, m_versionChangeTransaction.get());
+  m_versionChangeTransaction->objectStoreCreated(name, objectStore);
+  m_metadata.objectStores.set(objectStoreId, std::move(storeMetadata));
   ++m_metadata.maxObjectStoreId;
 
-  m_versionChangeTransaction->objectStoreCreated(name, objectStore);
   return objectStore;
 }
 
@@ -309,7 +293,7 @@ void IDBDatabase::deleteObjectStore(const String& name,
   }
 
   m_backend->deleteObjectStore(m_versionChangeTransaction->id(), objectStoreId);
-  m_versionChangeTransaction->objectStoreDeleted(name);
+  m_versionChangeTransaction->objectStoreDeleted(objectStoreId, name);
   m_metadata.objectStores.remove(objectStoreId);
 }
 
@@ -478,7 +462,7 @@ DispatchEventResult IDBDatabase::dispatchEventInternal(Event* event) {
 
 int64_t IDBDatabase::findObjectStoreId(const String& name) const {
   for (const auto& it : m_metadata.objectStores) {
-    if (it.value.name == name) {
+    if (it.value->name == name) {
       DCHECK_NE(it.key, IDBObjectStoreMetadata::InvalidId);
       return it.key;
     }
@@ -486,17 +470,47 @@ int64_t IDBDatabase::findObjectStoreId(const String& name) const {
   return IDBObjectStoreMetadata::InvalidId;
 }
 
-void IDBDatabase::objectStoreRenamed(int64_t objectStoreId,
-                                     const String& newName) {
+void IDBDatabase::renameObjectStore(int64_t objectStoreId,
+                                    const String& newName) {
   DCHECK(m_versionChangeTransaction)
       << "Object store renamed on database without a versionchange transaction";
   DCHECK(m_versionChangeTransaction->isActive())
       << "Object store renamed when versionchange transaction is not active";
   DCHECK(m_backend) << "Object store renamed after database connection closed";
   DCHECK(m_metadata.objectStores.contains(objectStoreId));
+
+  m_backend->renameObjectStore(m_versionChangeTransaction->id(), objectStoreId,
+                               newName);
+
   IDBDatabaseMetadata::ObjectStoreMap::iterator it =
       m_metadata.objectStores.find(objectStoreId);
-  it->value.name = newName;
+  IDBObjectStoreMetadata* objectStoreMetadata = it->value.get();
+  m_versionChangeTransaction->objectStoreRenamed(objectStoreMetadata->name,
+                                                 newName);
+  objectStoreMetadata->name = newName;
+}
+
+void IDBDatabase::revertObjectStoreCreation(int64_t objectStoreId) {
+  DCHECK(m_versionChangeTransaction) << "Object store metadata reverted on "
+                                        "database without a versionchange "
+                                        "transaction";
+  DCHECK(!m_versionChangeTransaction->isActive())
+      << "Object store metadata reverted when versionchange transaction is "
+         "still active";
+  DCHECK(m_metadata.objectStores.contains(objectStoreId));
+  m_metadata.objectStores.remove(objectStoreId);
+}
+
+void IDBDatabase::revertObjectStoreMetadata(
+    RefPtr<IDBObjectStoreMetadata> oldMetadata) {
+  DCHECK(m_versionChangeTransaction) << "Object store metadata reverted on "
+                                        "database without a versionchange "
+                                        "transaction";
+  DCHECK(!m_versionChangeTransaction->isActive())
+      << "Object store metadata reverted when versionchange transaction is "
+         "still active";
+  DCHECK(oldMetadata.get());
+  m_metadata.objectStores.set(oldMetadata->id, std::move(oldMetadata));
 }
 
 bool IDBDatabase::hasPendingActivity() const {