IndexedDB: pass along error of IDBBackingStore::renameIndex
https://bugs.webkit.org/show_bug.cgi?id=204900

Reviewed by Brady Eidson.

We ignored error of IDBBackingStore::renameIndex, so the operation may fail silently. This covered up two bugs
in our code as we were unaware of the failure.
One was in MemoryIDBBackingStore that we did not update objectStoreInfo properly when createIndex/deleteIndex;
another was in IDBObjectStoreInfo that we did not copy its members correctly.

Covered by existing test: storage/indexeddb//modern/index-rename-1-private.html

* Modules/indexeddb/server/MemoryIDBBackingStore.cpp:
(WebCore::IDBServer::MemoryIDBBackingStore::createIndex):
(WebCore::IDBServer::MemoryIDBBackingStore::deleteIndex):
* Modules/indexeddb/server/UniqueIDBDatabase.cpp:
(WebCore::IDBServer::UniqueIDBDatabase::performRenameIndex):
* Modules/indexeddb/shared/IDBObjectStoreInfo.cpp: If some index is deleted from IDBObjectStoreInfo, then
m_maxIndexID could be bigger than maximum index ID in m_indexMap, because we don't decrease m_maxIndexID for
deletion. Therefore, the assertion here is incorrect.
(WebCore::IDBObjectStoreInfo::isolatedCopy const):


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@253228 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 897d896..8345b5a 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,27 @@
+2019-12-06  Sihui Liu  <sihui_liu@apple.com>
+
+        IndexedDB: pass along error of IDBBackingStore::renameIndex
+        https://bugs.webkit.org/show_bug.cgi?id=204900
+
+        Reviewed by Brady Eidson.
+
+        We ignored error of IDBBackingStore::renameIndex, so the operation may fail silently. This covered up two bugs 
+        in our code as we were unaware of the failure.
+        One was in MemoryIDBBackingStore that we did not update objectStoreInfo properly when createIndex/deleteIndex; 
+        another was in IDBObjectStoreInfo that we did not copy its members correctly.
+
+        Covered by existing test: storage/indexeddb//modern/index-rename-1-private.html
+
+        * Modules/indexeddb/server/MemoryIDBBackingStore.cpp:
+        (WebCore::IDBServer::MemoryIDBBackingStore::createIndex):
+        (WebCore::IDBServer::MemoryIDBBackingStore::deleteIndex):
+        * Modules/indexeddb/server/UniqueIDBDatabase.cpp:
+        (WebCore::IDBServer::UniqueIDBDatabase::performRenameIndex):
+        * Modules/indexeddb/shared/IDBObjectStoreInfo.cpp: If some index is deleted from IDBObjectStoreInfo, then
+        m_maxIndexID could be bigger than maximum index ID in m_indexMap, because we don't decrease m_maxIndexID for
+        deletion. Therefore, the assertion here is incorrect.
+        (WebCore::IDBObjectStoreInfo::isolatedCopy const):
+
 2019-12-06  Don Olmstead  <don.olmstead@sony.com>
 
         [MathML] Should support conditional compilation
diff --git a/Source/WebCore/Modules/indexeddb/server/MemoryIDBBackingStore.cpp b/Source/WebCore/Modules/indexeddb/server/MemoryIDBBackingStore.cpp
index 7cbfb77..59dd410 100644
--- a/Source/WebCore/Modules/indexeddb/server/MemoryIDBBackingStore.cpp
+++ b/Source/WebCore/Modules/indexeddb/server/MemoryIDBBackingStore.cpp
@@ -232,6 +232,11 @@
 {
     LOG(IndexedDB, "MemoryIDBBackingStore::createIndex");
 
+    ASSERT(m_databaseInfo);
+    auto* objectStoreInfo = m_databaseInfo->infoForExistingObjectStore(info.objectStoreIdentifier());
+    if (!objectStoreInfo)
+        return IDBError { ConstraintError };
+
     auto rawTransaction = m_transactions.get(transactionIdentifier);
     ASSERT(rawTransaction);
     ASSERT(rawTransaction->isVersionChange());
@@ -240,13 +245,26 @@
     if (!objectStore)
         return IDBError { ConstraintError };
 
-    return objectStore->createIndex(*rawTransaction, info);
+    auto error = objectStore->createIndex(*rawTransaction, info);
+    if (error.isNull())
+        objectStoreInfo->addExistingIndex(info);
+
+    return error;
 }
 
 IDBError MemoryIDBBackingStore::deleteIndex(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, uint64_t indexIdentifier, const LockHolder&)
 {
     LOG(IndexedDB, "MemoryIDBBackingStore::deleteIndex");
 
+    ASSERT(m_databaseInfo);
+    auto* objectStoreInfo = m_databaseInfo->infoForExistingObjectStore(objectStoreIdentifier);
+    if (!objectStoreInfo)
+        return IDBError { ConstraintError };
+
+    auto* indexInfo = objectStoreInfo->infoForExistingIndex(indexIdentifier);
+    if (!indexInfo)
+        return IDBError { ConstraintError };
+
     auto rawTransaction = m_transactions.get(transactionIdentifier);
     ASSERT(rawTransaction);
     ASSERT(rawTransaction->isVersionChange());
@@ -255,7 +273,11 @@
     if (!objectStore)
         return IDBError { ConstraintError };
 
-    return objectStore->deleteIndex(*rawTransaction, indexIdentifier);
+    auto error = objectStore->deleteIndex(*rawTransaction, indexIdentifier);
+    if (!error.isNull())
+        objectStoreInfo->deleteIndex(indexIdentifier);
+
+    return error;
 }
 
 IDBError MemoryIDBBackingStore::renameIndex(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, uint64_t indexIdentifier, const String& newName, const LockHolder&)
diff --git a/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp b/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp
index 039ba8c..a60006d 100644
--- a/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp
+++ b/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp
@@ -1112,6 +1112,11 @@
     ASSERT(!isMainThread());
     LOG(IndexedDB, "(db) UniqueIDBDatabase::performRenameIndex");
 
+    if (!error.isNull()) {
+        postDatabaseTaskReply(createCrossThreadTask(*this, &UniqueIDBDatabase::didPerformRenameIndex, callbackIdentifier, error, objectStoreIdentifier, indexIdentifier, newName));
+        return;
+    }
+
     // Quota check.
     auto taskSize = defaultWriteOperationCost + newName.sizeInBytes();
     if (m_server->requestSpace(m_origin, taskSize) == StorageQuotaManager::Decision::Deny) {
@@ -1120,12 +1125,13 @@
     }
 
     ASSERT(m_backingStore);
+    IDBError backingStoreError;
     {
         LockHolder locker(m_backingStoreLock);
-        m_backingStore->renameIndex(transactionIdentifier, objectStoreIdentifier, indexIdentifier, newName, locker);
+        backingStoreError = m_backingStore->renameIndex(transactionIdentifier, objectStoreIdentifier, indexIdentifier, newName, locker);
     }
 
-    postDatabaseTaskReply(createCrossThreadTask(*this, &UniqueIDBDatabase::didPerformRenameIndex, callbackIdentifier, error, objectStoreIdentifier, indexIdentifier, newName));
+    postDatabaseTaskReply(createCrossThreadTask(*this, &UniqueIDBDatabase::didPerformRenameIndex, callbackIdentifier, backingStoreError, objectStoreIdentifier, indexIdentifier, newName));
 }
 
 void UniqueIDBDatabase::didPerformRenameIndex(uint64_t callbackIdentifier, const IDBError& error, uint64_t objectStoreIdentifier, uint64_t indexIdentifier, const String& newName)
diff --git a/Source/WebCore/Modules/indexeddb/shared/IDBObjectStoreInfo.cpp b/Source/WebCore/Modules/indexeddb/shared/IDBObjectStoreInfo.cpp
index 2a051f3..5c7282a 100644
--- a/Source/WebCore/Modules/indexeddb/shared/IDBObjectStoreInfo.cpp
+++ b/Source/WebCore/Modules/indexeddb/shared/IDBObjectStoreInfo.cpp
@@ -98,13 +98,10 @@
 {
     IDBObjectStoreInfo result = { m_identifier, m_name.isolatedCopy(), WebCore::isolatedCopy(m_keyPath), m_autoIncrement };
 
-    for (auto& iterator : m_indexMap) {
+    for (auto& iterator : m_indexMap)
         result.m_indexMap.set(iterator.key, iterator.value.isolatedCopy());
-        if (iterator.key > result.m_maxIndexID)
-            result.m_maxIndexID = iterator.key;
-    }
 
-    ASSERT(result.m_maxIndexID == m_maxIndexID);
+    result.m_maxIndexID = m_maxIndexID;
 
     return result;
 }