Improve accuracy of IndexedDB estimated write size computation
authornham@apple.com <nham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 May 2020 00:38:17 +0000 (00:38 +0000)
committernham@apple.com <nham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 May 2020 00:38:17 +0000 (00:38 +0000)
https://bugs.webkit.org/show_bug.cgi?id=211360

Reviewed by Brady Eidson.

Source/WebCore:

We currently estimate the size of a put in IndexedDB for quota check purposes with something
like:

  estimatedWriteSize = (1 + numIndices) * (keySize + valueSize)

However, this can lead to large overestimates of write sizes. This is because secondary
indices only store a mapping of secondary index key => primary key; they do not store the
entire value. In the example site attached to 202137 (another DB quota-related bug), the
the heuristic estimates that one of the put operations would use more than 800 MB when it
actually uses 220 MB. This inaccuracy leads to spurious disk quota permission modals being
presented in Safari.

This patch improves the write size computation by generating the secondary index keys before
estimating the write size. The performance should be about the same since we save the
generated index keys for later usage when we actually add the record to the DB.

* Headers.cmake:
* Modules/indexeddb/server/IDBBackingStore.h:
* Modules/indexeddb/server/MemoryIDBBackingStore.cpp:
(WebCore::IDBServer::MemoryIDBBackingStore::MemoryIDBBackingStore):
(WebCore::IDBServer::MemoryIDBBackingStore::addRecord):
(WebCore::IDBServer::MemoryIDBBackingStore::serializationContext):
* Modules/indexeddb/server/MemoryIDBBackingStore.h:
* Modules/indexeddb/server/MemoryObjectStore.cpp:
(WebCore::IDBServer::MemoryObjectStore::addRecord):
(WebCore::IDBServer::MemoryObjectStore::updateIndexesForPutRecord):
* Modules/indexeddb/server/MemoryObjectStore.h:
* Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:
(WebCore::IDBServer::SQLiteIDBBackingStore::updateAllIndexesForAddRecord):
(WebCore::IDBServer::SQLiteIDBBackingStore::addRecord):
(WebCore::IDBServer::SQLiteIDBBackingStore::serializationContext):
* Modules/indexeddb/server/SQLiteIDBBackingStore.h:
* Modules/indexeddb/server/UniqueIDBDatabase.cpp:
(WebCore::IDBServer::estimateSize):
(WebCore::IDBServer::UniqueIDBDatabase::putOrAdd):
* Modules/indexeddb/shared/IndexKey.h:
* WebCore.xcodeproj/project.pbxproj:
* bindings/js/IDBBindingUtilities.cpp:
(WebCore::generateIndexKeyMapForValue):
* bindings/js/IDBBindingUtilities.h:

LayoutTests:

Added a layout test to check that the size estimate associated with adding an object to a
store with many indices is reasonable.

* platform/mac-wk1/TestExpectations: Skip test on Mac WK1 since it doesn't implement quota checks.
* platform/win/TestExpectations: Skip test on Windows since it doesn't implement quota checks.
* storage/indexeddb/resources/storage-limit-with-indices.js: Added.
* storage/indexeddb/storage-limit-with-indices-expected.txt: Added.
* storage/indexeddb/storage-limit-with-indices.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@261533 268f45cc-cd09-0410-ab3c-d52691b4dbfc

20 files changed:
LayoutTests/ChangeLog
LayoutTests/platform/mac-wk1/TestExpectations
LayoutTests/platform/win/TestExpectations
LayoutTests/storage/indexeddb/resources/storage-limit-with-indices.js [new file with mode: 0644]
LayoutTests/storage/indexeddb/storage-limit-with-indices-expected.txt [new file with mode: 0644]
LayoutTests/storage/indexeddb/storage-limit-with-indices.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/Headers.cmake
Source/WebCore/Modules/indexeddb/server/IDBBackingStore.h
Source/WebCore/Modules/indexeddb/server/MemoryIDBBackingStore.cpp
Source/WebCore/Modules/indexeddb/server/MemoryIDBBackingStore.h
Source/WebCore/Modules/indexeddb/server/MemoryObjectStore.cpp
Source/WebCore/Modules/indexeddb/server/MemoryObjectStore.h
Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp
Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.h
Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp
Source/WebCore/Modules/indexeddb/shared/IndexKey.h
Source/WebCore/WebCore.xcodeproj/project.pbxproj
Source/WebCore/bindings/js/IDBBindingUtilities.cpp
Source/WebCore/bindings/js/IDBBindingUtilities.h

index c2b9fde..4f7645a 100644 (file)
@@ -1,3 +1,19 @@
+2020-05-11  Ben Nham  <nham@apple.com>
+
+        Improve accuracy of IndexedDB estimated write size computation
+        https://bugs.webkit.org/show_bug.cgi?id=211360
+
+        Reviewed by Brady Eidson.
+
+        Added a layout test to check that the size estimate associated with adding an object to a
+        store with many indices is reasonable.
+
+        * platform/mac-wk1/TestExpectations: Skip test on Mac WK1 since it doesn't implement quota checks.
+        * platform/win/TestExpectations: Skip test on Windows since it doesn't implement quota checks.
+        * storage/indexeddb/resources/storage-limit-with-indices.js: Added.
+        * storage/indexeddb/storage-limit-with-indices-expected.txt: Added.
+        * storage/indexeddb/storage-limit-with-indices.html: Added.
+
 2020-05-11  Kenneth Russell  <kbr@chromium.org>
 
         Enable conformance2/textures/canvas/ and image_data/ tests
index 989ae59..8d788eb 100644 (file)
@@ -332,6 +332,7 @@ http/tests/IndexedDB/storage-limit.https.html [ Skip ]
 http/tests/IndexedDB/storage-limit-1.https.html [ Skip ]
 http/tests/IndexedDB/storage-limit-2.https.html [ Skip ]
 storage/indexeddb/storage-limit.html [ Skip ]
+storage/indexeddb/storage-limit-with-indices.html [ Skip ]
 storage/indexeddb/request-size-estimate.html [ Skip ]
 
 # Skip WebRTC for now in WK1
index b1bfdaa..5a344b0 100644 (file)
@@ -2399,6 +2399,7 @@ http/tests/xmlhttprequest/web-apps/012.html
 http/tests/xmlhttprequest/web-apps/013.html
 
 storage/indexeddb/storage-limit.html [ Skip ]
+storage/indexeddb/storage-limit-with-indices.html [ Skip ]
 storage/indexeddb/request-size-estimate.html [ Skip ]
 http/tests/IndexedDB/storage-limit.https.html [ Skip ]
 http/tests/IndexedDB/storage-limit-1.https.html [ Skip ]
diff --git a/LayoutTests/storage/indexeddb/resources/storage-limit-with-indices.js b/LayoutTests/storage/indexeddb/resources/storage-limit-with-indices.js
new file mode 100644 (file)
index 0000000..4a45a62
--- /dev/null
@@ -0,0 +1,50 @@
+if (this.importScripts) {
+    importScripts('../../../resources/js-test.js');
+    importScripts('shared.js');
+}
+
+if (window.testRunner)
+    testRunner.setAllowStorageQuotaIncrease(false);
+
+var quota = 400 * 1024; // default quota for testing.
+var numIndices = 10;
+
+description("This test checks that the size estimate associated with adding an object to a store with many indices is reasonable.");
+
+indexedDBTest(prepareDatabase, onOpenSuccess);
+
+function prepareDatabase(event)
+{
+    preamble(event);
+
+    evalAndLog("db = event.target.result");
+    evalAndLog("store = db.createObjectStore('store')");
+
+    let db = event.target;
+    for (let i = 0; i < numIndices; i++) {
+        let indexName = 'a' + i;
+        store.createIndex(indexName, indexName);
+    }
+}
+
+function onOpenSuccess(event)
+{
+    preamble(event);
+    evalAndLog("db = event.target.result");
+    evalAndLog("store = db.transaction('store', 'readwrite').objectStore('store')");
+
+    // The size of the indexed attributes a0, a1, ... is small, so they shouldn't have a material
+    // effect on the estimated put size for quota purposes.
+    evalAndLog(`request = store.add({a0: 0, a1: 1, payload: new Uint8Array(${quota / numIndices})}, 42)`);
+
+    request.onsuccess = function(event) {
+        reqEvent = event;
+        shouldBe("reqEvent.target.result", "42");
+        finishJSTest();
+    }
+
+    request.onerror = function(event) {
+        testFailed("Add operation failed, but it should succeed because it uses much less space than the storage limit.");
+        finishJSTest();
+    }
+}
diff --git a/LayoutTests/storage/indexeddb/storage-limit-with-indices-expected.txt b/LayoutTests/storage/indexeddb/storage-limit-with-indices-expected.txt
new file mode 100644 (file)
index 0000000..a7f909b
--- /dev/null
@@ -0,0 +1,23 @@
+This test checks that the size estimate associated with adding an object to a store with many indices is reasonable.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+indexedDB = self.indexedDB || self.webkitIndexedDB || self.mozIndexedDB || self.msIndexedDB || self.OIndexedDB;
+
+indexedDB.deleteDatabase(dbname)
+indexedDB.open(dbname)
+
+prepareDatabase():
+db = event.target.result
+store = db.createObjectStore('store')
+
+onOpenSuccess():
+db = event.target.result
+store = db.transaction('store', 'readwrite').objectStore('store')
+request = store.add({a0: 0, a1: 1, payload: new Uint8Array(40960)}, 42)
+PASS reqEvent.target.result is 42
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/storage/indexeddb/storage-limit-with-indices.html b/LayoutTests/storage/indexeddb/storage-limit-with-indices.html
new file mode 100644 (file)
index 0000000..2d88ccc
--- /dev/null
@@ -0,0 +1,9 @@
+<html>
+<head>
+<script src="../../resources/js-test.js"></script>
+<script src="resources/shared.js"></script>
+</head>
+<body>
+<script src="resources/storage-limit-with-indices.js"></script>
+</body>
+</html>
\ No newline at end of file
index bd0e614..61718af 100644 (file)
@@ -1,3 +1,51 @@
+2020-05-11  Ben Nham  <nham@apple.com>
+
+        Improve accuracy of IndexedDB estimated write size computation
+        https://bugs.webkit.org/show_bug.cgi?id=211360
+
+        Reviewed by Brady Eidson.
+
+        We currently estimate the size of a put in IndexedDB for quota check purposes with something
+        like:
+
+          estimatedWriteSize = (1 + numIndices) * (keySize + valueSize)
+
+        However, this can lead to large overestimates of write sizes. This is because secondary
+        indices only store a mapping of secondary index key => primary key; they do not store the
+        entire value. In the example site attached to 202137 (another DB quota-related bug), the
+        the heuristic estimates that one of the put operations would use more than 800 MB when it
+        actually uses 220 MB. This inaccuracy leads to spurious disk quota permission modals being
+        presented in Safari.
+
+        This patch improves the write size computation by generating the secondary index keys before
+        estimating the write size. The performance should be about the same since we save the
+        generated index keys for later usage when we actually add the record to the DB.
+
+        * Headers.cmake:
+        * Modules/indexeddb/server/IDBBackingStore.h:
+        * Modules/indexeddb/server/MemoryIDBBackingStore.cpp:
+        (WebCore::IDBServer::MemoryIDBBackingStore::MemoryIDBBackingStore):
+        (WebCore::IDBServer::MemoryIDBBackingStore::addRecord):
+        (WebCore::IDBServer::MemoryIDBBackingStore::serializationContext):
+        * Modules/indexeddb/server/MemoryIDBBackingStore.h:
+        * Modules/indexeddb/server/MemoryObjectStore.cpp:
+        (WebCore::IDBServer::MemoryObjectStore::addRecord):
+        (WebCore::IDBServer::MemoryObjectStore::updateIndexesForPutRecord):
+        * Modules/indexeddb/server/MemoryObjectStore.h:
+        * Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:
+        (WebCore::IDBServer::SQLiteIDBBackingStore::updateAllIndexesForAddRecord):
+        (WebCore::IDBServer::SQLiteIDBBackingStore::addRecord):
+        (WebCore::IDBServer::SQLiteIDBBackingStore::serializationContext):
+        * Modules/indexeddb/server/SQLiteIDBBackingStore.h:
+        * Modules/indexeddb/server/UniqueIDBDatabase.cpp:
+        (WebCore::IDBServer::estimateSize):
+        (WebCore::IDBServer::UniqueIDBDatabase::putOrAdd):
+        * Modules/indexeddb/shared/IndexKey.h:
+        * WebCore.xcodeproj/project.pbxproj:
+        * bindings/js/IDBBindingUtilities.cpp:
+        (WebCore::generateIndexKeyMapForValue):
+        * bindings/js/IDBBindingUtilities.h:
+
 2020-05-11  Kate Cheney  <katherine_cheney@apple.com>
 
         Fail navigations to non app-bound domains after use of app-bound APIs
index f4c571d..5a1b45a 100644 (file)
@@ -68,6 +68,7 @@ set(WebCore_PRIVATE_FRAMEWORK_HEADERS
     Modules/indexeddb/server/UniqueIDBDatabaseConnection.h
     Modules/indexeddb/server/UniqueIDBDatabaseTransaction.h
 
+    Modules/indexeddb/shared/IndexKey.h
     Modules/indexeddb/shared/IDBCursorInfo.h
     Modules/indexeddb/shared/IDBCursorRecord.h
     Modules/indexeddb/shared/IDBDatabaseInfo.h
index a7df7a1..8889ab0 100644 (file)
@@ -29,6 +29,7 @@
 
 #include "IDBDatabaseInfo.h"
 #include "IDBError.h"
+#include "IndexKey.h"
 #include <wtf/MainThread.h>
 
 namespace WebCore {
@@ -56,6 +57,8 @@ enum class IndexRecordType;
 
 namespace IDBServer {
 
+class IDBSerializationContext;
+
 class IDBBackingStore {
 public:
     virtual ~IDBBackingStore() { RELEASE_ASSERT(!isMainThread()); }
@@ -75,7 +78,7 @@ public:
     virtual IDBError renameIndex(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, uint64_t indexIdentifier, const String& newName) = 0;
     virtual IDBError keyExistsInObjectStore(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, const IDBKeyData&, bool& keyExists) = 0;
     virtual IDBError deleteRange(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, const IDBKeyRangeData&) = 0;
-    virtual IDBError addRecord(const IDBResourceIdentifier& transactionIdentifier, const IDBObjectStoreInfo&, const IDBKeyData&, const IDBValue&) = 0;
+    virtual IDBError addRecord(const IDBResourceIdentifier& transactionIdentifier, const IDBObjectStoreInfo&, const IDBKeyData&, const IndexIDToIndexKeyMap&, const IDBValue&) = 0;
     virtual IDBError getRecord(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, const IDBKeyRangeData&, IDBGetRecordDataType, IDBGetResult& outValue) = 0;
     virtual IDBError getAllRecords(const IDBResourceIdentifier& transactionIdentifier, const IDBGetAllRecordsData&, IDBGetAllResult& outValue) = 0;
     virtual IDBError getIndexRecord(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, uint64_t indexIdentifier, IndexedDB::IndexRecordType, const IDBKeyRangeData&, IDBGetResult& outValue) = 0;
@@ -86,6 +89,8 @@ public:
     virtual IDBError openCursor(const IDBResourceIdentifier& transactionIdentifier, const IDBCursorInfo&, IDBGetResult& outResult) = 0;
     virtual IDBError iterateCursor(const IDBResourceIdentifier& transactionIdentifier, const IDBResourceIdentifier& cursorIdentifier, const IDBIterateCursorData&, IDBGetResult& outResult) = 0;
 
+    virtual IDBSerializationContext& serializationContext() = 0;
+
     virtual IDBObjectStoreInfo* infoForObjectStore(uint64_t objectStoreIdentifier) = 0;
     virtual void deleteBackingStore() = 0;
 
index 401ce3d..0f4b35c 100644 (file)
@@ -35,6 +35,7 @@
 #include "IDBIndexInfo.h"
 #include "IDBIterateCursorData.h"
 #include "IDBKeyRangeData.h"
+#include "IDBSerializationContext.h"
 #include "Logging.h"
 #include "MemoryIndexCursor.h"
 #include "MemoryObjectStore.h"
@@ -49,6 +50,7 @@ constexpr uint64_t maxGeneratedKeyValue = 0x20000000000000;
 MemoryIDBBackingStore::MemoryIDBBackingStore(PAL::SessionID sessionID, const IDBDatabaseIdentifier& identifier)
     : m_identifier(identifier)
     , m_sessionID(sessionID)
+    , m_serializationContext(IDBSerializationContext::getOrCreateIDBSerializationContext(sessionID))
 {
 }
 
@@ -355,7 +357,7 @@ IDBError MemoryIDBBackingStore::deleteRange(const IDBResourceIdentifier& transac
     return IDBError { };
 }
 
-IDBError MemoryIDBBackingStore::addRecord(const IDBResourceIdentifier& transactionIdentifier, const IDBObjectStoreInfo& objectStoreInfo, const IDBKeyData& keyData, const IDBValue& value)
+IDBError MemoryIDBBackingStore::addRecord(const IDBResourceIdentifier& transactionIdentifier, const IDBObjectStoreInfo& objectStoreInfo, const IDBKeyData& keyData, const IndexIDToIndexKeyMap& indexKeys, const IDBValue& value)
 {
     LOG(IndexedDB, "MemoryIDBBackingStore::addRecord");
 
@@ -369,7 +371,7 @@ IDBError MemoryIDBBackingStore::addRecord(const IDBResourceIdentifier& transacti
     if (!objectStore)
         return IDBError { UnknownError, "No backing store object store found to put record"_s };
 
-    return objectStore->addRecord(*transaction, keyData, value);
+    return objectStore->addRecord(*transaction, keyData, indexKeys, value);
 }
 
 IDBError MemoryIDBBackingStore::getRecord(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, const IDBKeyRangeData& range, IDBGetRecordDataType type, IDBGetResult& outValue)
@@ -611,6 +613,11 @@ RefPtr<MemoryObjectStore> MemoryIDBBackingStore::takeObjectStoreByIdentifier(uin
     return objectStoreByIdentifier;
 }
 
+IDBSerializationContext& MemoryIDBBackingStore::serializationContext()
+{
+    return m_serializationContext.get();
+}
+
 IDBObjectStoreInfo* MemoryIDBBackingStore::infoForObjectStore(uint64_t objectStoreIdentifier)
 {
     ASSERT(m_databaseInfo);
index 74dc950..b969226 100644 (file)
@@ -30,6 +30,7 @@
 #include "IDBBackingStore.h"
 #include "IDBDatabaseIdentifier.h"
 #include "IDBResourceIdentifier.h"
+#include "IndexKey.h"
 #include "MemoryBackingStoreTransaction.h"
 #include <pal/SessionID.h>
 #include <wtf/HashMap.h>
@@ -37,6 +38,7 @@
 namespace WebCore {
 namespace IDBServer {
 
+class IDBSerializationContext;
 class MemoryObjectStore;
 
 class MemoryIDBBackingStore final : public IDBBackingStore {
@@ -51,6 +53,8 @@ public:
     void removeObjectStoreForVersionChangeAbort(MemoryObjectStore&);
     void restoreObjectStoreForVersionChangeAbort(Ref<MemoryObjectStore>&&);
 
+    IDBSerializationContext& serializationContext() final;
+
 private:
     IDBError beginTransaction(const IDBTransactionInfo&) final;
     IDBError abortTransaction(const IDBResourceIdentifier& transactionIdentifier) final;
@@ -64,7 +68,7 @@ private:
     IDBError renameIndex(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, uint64_t indexIdentifier, const String& newName) final;
     IDBError keyExistsInObjectStore(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, const IDBKeyData&, bool& keyExists) final;
     IDBError deleteRange(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, const IDBKeyRangeData&) final;
-    IDBError addRecord(const IDBResourceIdentifier& transactionIdentifier, const IDBObjectStoreInfo&, const IDBKeyData&, const IDBValue&) final;
+    IDBError addRecord(const IDBResourceIdentifier& transactionIdentifier, const IDBObjectStoreInfo&, const IDBKeyData&, const IndexIDToIndexKeyMap&, const IDBValue&) final;
     IDBError getRecord(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, const IDBKeyRangeData&, IDBGetRecordDataType, IDBGetResult& outValue) final;
     IDBError getAllRecords(const IDBResourceIdentifier& transactionIdentifier, const IDBGetAllRecordsData&, IDBGetAllResult& outValue) final;
     IDBError getIndexRecord(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, uint64_t indexIdentifier, IndexedDB::IndexRecordType, const IDBKeyRangeData&, IDBGetResult& outValue) final;
@@ -92,6 +96,7 @@ private:
 
     IDBDatabaseIdentifier m_identifier;
     PAL::SessionID m_sessionID;
+    Ref<IDBSerializationContext> m_serializationContext;
     std::unique_ptr<IDBDatabaseInfo> m_databaseInfo;
 
     HashMap<IDBResourceIdentifier, std::unique_ptr<MemoryBackingStoreTransaction>> m_transactions;
index 7155b59..423f087 100644 (file)
@@ -252,6 +252,12 @@ void MemoryObjectStore::deleteRange(const IDBKeyRangeData& inputRange)
 
 IDBError MemoryObjectStore::addRecord(MemoryBackingStoreTransaction& transaction, const IDBKeyData& keyData, const IDBValue& value)
 {
+    auto indexKeys = generateIndexKeyMapForValue(m_serializationContext->execState(), m_info, keyData, value);
+    return addRecord(transaction, keyData, indexKeys, value);
+}
+
+IDBError MemoryObjectStore::addRecord(MemoryBackingStoreTransaction& transaction, const IDBKeyData& keyData, const IndexIDToIndexKeyMap& indexKeys, const IDBValue& value)
+{
     LOG(IndexedDB, "MemoryObjectStore::addRecord");
 
     ASSERT(m_writeTransaction);
@@ -271,7 +277,7 @@ IDBError MemoryObjectStore::addRecord(MemoryBackingStoreTransaction& transaction
     ASSERT(listResult.second);
 
     // If there was an error indexing this addition, then revert it.
-    auto error = updateIndexesForPutRecord(keyData, value.data());
+    auto error = updateIndexesForPutRecord(keyData, indexKeys);
     if (!error.isNull()) {
         m_keyValueStore->remove(mapResult.iterator);
         m_orderedKeys->erase(listResult.first);
@@ -299,29 +305,24 @@ void MemoryObjectStore::updateIndexesForDeleteRecord(const IDBKeyData& value)
         index->removeEntriesWithValueKey(value);
 }
 
-IDBError MemoryObjectStore::updateIndexesForPutRecord(const IDBKeyData& key, const ThreadSafeDataBuffer& value)
+IDBError MemoryObjectStore::updateIndexesForPutRecord(const IDBKeyData& key, const IndexIDToIndexKeyMap& indexKeys)
 {
-    JSLockHolder locker(m_serializationContext->vm());
-
-    auto jsValue = deserializeIDBValueToJSValue(m_serializationContext->execState(), value);
-    if (jsValue.isUndefinedOrNull())
-        return IDBError { };
-
     IDBError error;
     Vector<std::pair<MemoryIndex*, IndexKey>> changedIndexRecords;
 
-    for (auto& index : m_indexesByName.values()) {
-        IndexKey indexKey;
-        generateIndexKeyForValue(m_serializationContext->execState(), index->info(), jsValue, indexKey, m_info.keyPath(), key);
-
-        if (indexKey.isNull())
-            continue;
+    for (const auto& [indexID, indexKey] : indexKeys) {
+        auto* index = m_indexesByIdentifier.get(indexID);
+        ASSERT(index);
+        if (!index) {
+            error = IDBError { InvalidStateError, "Missing index metadata" };
+            break;
+        }
 
         error = index->putIndexKey(key, indexKey);
         if (!error.isNull())
             break;
 
-        changedIndexRecords.append(std::make_pair(index.get(), indexKey));
+        changedIndexRecords.append(std::make_pair(index, indexKey));
     }
 
     // If any of the index puts failed, revert all of the ones that went through.
index ac70108..b150299 100644 (file)
@@ -29,6 +29,7 @@
 
 #include "IDBKeyData.h"
 #include "IDBObjectStoreInfo.h"
+#include "IndexKey.h"
 #include "MemoryIndex.h"
 #include "MemoryObjectStoreCursor.h"
 #include "ThreadSafeDataBuffer.h"
@@ -80,6 +81,7 @@ public:
     void deleteRecord(const IDBKeyData&);
     void deleteRange(const IDBKeyRangeData&);
     IDBError addRecord(MemoryBackingStoreTransaction&, const IDBKeyData&, const IDBValue&);
+    IDBError addRecord(MemoryBackingStoreTransaction&, const IDBKeyData&, const IndexIDToIndexKeyMap&, const IDBValue&);
 
     uint64_t currentKeyGeneratorValue() const { return m_keyGeneratorValue; }
     void setKeyGeneratorValue(uint64_t value) { m_keyGeneratorValue = value; }
@@ -114,7 +116,7 @@ private:
     IDBKeyDataSet::iterator lowestIteratorInRange(const IDBKeyRangeData&, bool reverse) const;
 
     IDBError populateIndexWithExistingRecords(MemoryIndex&);
-    IDBError updateIndexesForPutRecord(const IDBKeyData&, const ThreadSafeDataBuffer& value);
+    IDBError updateIndexesForPutRecord(const IDBKeyData&, const IndexIDToIndexKeyMap&);
     void updateIndexesForDeleteRecord(const IDBKeyData& value);
     void updateCursorsForPutRecord(IDBKeyDataSet::iterator);
     void updateCursorsForDeleteRecord(const IDBKeyData&);
index ee3e7e8..875160a 100644 (file)
@@ -1852,24 +1852,22 @@ IDBError SQLiteIDBBackingStore::updateOneIndexForAddRecord(const IDBIndexInfo& i
     return uncheckedPutIndexKey(info, key, indexKey, recordID);
 }
 
-IDBError SQLiteIDBBackingStore::updateAllIndexesForAddRecord(const IDBObjectStoreInfo& info, const IDBKeyData& key, const ThreadSafeDataBuffer& value, int64_t recordID)
+IDBError SQLiteIDBBackingStore::updateAllIndexesForAddRecord(const IDBObjectStoreInfo& info, const IDBKeyData& key, const IndexIDToIndexKeyMap& indexKeys, int64_t recordID)
 {
-    JSLockHolder locker(m_serializationContext->vm());
-
-    auto jsValue = deserializeIDBValueToJSValue(m_serializationContext->execState(), value);
-    if (jsValue.isUndefinedOrNull())
-        return IDBError { };
-
     IDBError error;
+    const auto& indexMap = info.indexMap();
     bool anyRecordsSucceeded = false;
-    for (auto& index : info.indexMap().values()) {
-        IndexKey indexKey;
-        generateIndexKeyForValue(m_serializationContext->execState(), index, jsValue, indexKey, info.keyPath(), key);
 
-        if (indexKey.isNull())
-            continue;
+    for (const auto& [indexID, indexKey] : indexKeys) {
+        auto indexIterator = indexMap.find(indexID);
+        ASSERT(indexIterator != indexMap.end());
 
-        error = uncheckedPutIndexKey(index, key, indexKey, recordID);
+        if (indexIterator == indexMap.end()) {
+            error = IDBError { InvalidStateError, "Missing index metadata" };
+            break;
+        }
+
+        error = uncheckedPutIndexKey(indexIterator->value, key, indexKey, recordID);
         if (!error.isNull())
             break;
 
@@ -1891,7 +1889,7 @@ IDBError SQLiteIDBBackingStore::updateAllIndexesForAddRecord(const IDBObjectStor
     return error;
 }
 
-IDBError SQLiteIDBBackingStore::addRecord(const IDBResourceIdentifier& transactionIdentifier, const IDBObjectStoreInfo& objectStoreInfo, const IDBKeyData& keyData, const IDBValue& value)
+IDBError SQLiteIDBBackingStore::addRecord(const IDBResourceIdentifier& transactionIdentifier, const IDBObjectStoreInfo& objectStoreInfo, const IDBKeyData& keyData, const IndexIDToIndexKeyMap& indexKeys, const IDBValue& value)
 {
     LOG(IndexedDB, "SQLiteIDBBackingStore::addRecord - key %s, object store %" PRIu64, keyData.loggingString().utf8().data(), objectStoreInfo.identifier());
 
@@ -1930,7 +1928,7 @@ IDBError SQLiteIDBBackingStore::addRecord(const IDBResourceIdentifier& transacti
         recordID = m_sqliteDB->lastInsertRowID();
     }
 
-    auto error = updateAllIndexesForAddRecord(objectStoreInfo, keyData, value.data(), recordID);
+    auto error = updateAllIndexesForAddRecord(objectStoreInfo, keyData, indexKeys, recordID);
 
     if (!error.isNull()) {
         auto sql = cachedStatement(SQL::DeleteObjectStoreRecord, "DELETE FROM Records WHERE objectStoreID = ? AND key = CAST(? AS TEXT);"_s);
@@ -2737,6 +2735,11 @@ IDBError SQLiteIDBBackingStore::iterateCursor(const IDBResourceIdentifier& trans
     return IDBError { };
 }
 
+IDBSerializationContext& SQLiteIDBBackingStore::serializationContext()
+{
+    return m_serializationContext.get();
+}
+
 IDBObjectStoreInfo* SQLiteIDBBackingStore::infoForObjectStore(uint64_t objectStoreIdentifier)
 {
     ASSERT(m_databaseInfo);
index 336e293..bb5ebb5 100644 (file)
@@ -71,7 +71,7 @@ public:
     IDBError renameIndex(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, uint64_t indexIdentifier, const String& newName) final;
     IDBError keyExistsInObjectStore(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, const IDBKeyData&, bool& keyExists) final;
     IDBError deleteRange(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, const IDBKeyRangeData&) final;
-    IDBError addRecord(const IDBResourceIdentifier& transactionIdentifier, const IDBObjectStoreInfo&, const IDBKeyData&, const IDBValue&) final;
+    IDBError addRecord(const IDBResourceIdentifier& transactionIdentifier, const IDBObjectStoreInfo&, const IDBKeyData&, const IndexIDToIndexKeyMap&, const IDBValue&) final;
     IDBError getRecord(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, const IDBKeyRangeData&, IDBGetRecordDataType, IDBGetResult& outValue) final;
     IDBError getAllRecords(const IDBResourceIdentifier& transactionIdentifier, const IDBGetAllRecordsData&, IDBGetAllResult& outValue) final;
     IDBError getIndexRecord(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, uint64_t indexIdentifier, IndexedDB::IndexRecordType, const IDBKeyRangeData&, IDBGetResult& outValue) final;
@@ -82,6 +82,8 @@ public:
     IDBError openCursor(const IDBResourceIdentifier& transactionIdentifier, const IDBCursorInfo&, IDBGetResult& outResult) final;
     IDBError iterateCursor(const IDBResourceIdentifier& transactionIdentifier, const IDBResourceIdentifier& cursorIdentifier, const IDBIterateCursorData&, IDBGetResult& outResult) final;
 
+    IDBSerializationContext& serializationContext() final;
+
     IDBObjectStoreInfo* infoForObjectStore(uint64_t objectStoreIdentifier) final;
     void deleteBackingStore() final;
 
@@ -123,7 +125,7 @@ private:
     IDBError uncheckedGetKeyGeneratorValue(int64_t objectStoreID, uint64_t& outValue);
     IDBError uncheckedSetKeyGeneratorValue(int64_t objectStoreID, uint64_t value);
 
-    IDBError updateAllIndexesForAddRecord(const IDBObjectStoreInfo&, const IDBKeyData&, const ThreadSafeDataBuffer& value, int64_t recordID);
+    IDBError updateAllIndexesForAddRecord(const IDBObjectStoreInfo&, const IDBKeyData&, const IndexIDToIndexKeyMap&, int64_t recordID);
     IDBError updateOneIndexForAddRecord(const IDBIndexInfo&, const IDBKeyData&, const ThreadSafeDataBuffer& value, int64_t recordID);
     IDBError uncheckedPutIndexKey(const IDBIndexInfo&, const IDBKeyData& keyValue, const IndexKey&, int64_t recordID);
     IDBError uncheckedPutIndexRecord(int64_t objectStoreID, int64_t indexID, const IDBKeyData& keyValue, const IDBKeyData& indexKey, int64_t recordID);
index 429e442..3c8b7cc 100644 (file)
@@ -38,6 +38,7 @@
 #include "IDBServer.h"
 #include "IDBTransactionInfo.h"
 #include "IDBValue.h"
+#include "IndexKey.h"
 #include "Logging.h"
 #include "StorageQuotaManager.h"
 #include "UniqueIDBDatabaseConnection.h"
@@ -70,6 +71,29 @@ static inline uint64_t estimateSize(const IDBKeyData& keyData)
     return size;
 }
 
+static inline uint64_t estimateSize(const IDBObjectStoreInfo& info, const IndexIDToIndexKeyMap& indexKeys, uint64_t primaryKeySize)
+{
+    // Each IndexRecord has 5 columns:
+    //  - indexID, objectStoreID, recordID: these are varints, estimate 4 bytes each = 12 bytes
+    //  - primary key: use primaryKeySize
+    //  - secondary index key: use estimateSize(secondary key)
+    static constexpr uint64_t baseIndexRowSize = 12;
+    uint64_t size = 0;
+
+    for (const auto& [indexID, indexKey] : indexKeys) {
+        auto indexIterator = info.indexMap().find(indexID);
+        ASSERT(indexIterator != info.indexMap().end());
+
+        if (indexIterator != info.indexMap().end() && indexIterator->value.multiEntry()) {
+            for (const auto& secondaryKey : indexKey.multiEntry())
+                size += (baseIndexRowSize + primaryKeySize + estimateSize(secondaryKey));
+        } else
+            size += (baseIndexRowSize + primaryKeySize + estimateSize(indexKey.asOneKey()));
+    }
+
+    return size;
+}
+
 static inline uint64_t estimateSize(const IDBValue& value)
 {
     uint64_t size = 4;
@@ -712,16 +736,6 @@ void UniqueIDBDatabase::putOrAdd(const IDBRequestData& requestData, const IDBKey
         return;
     }
 
-    // Quota check.
-    auto taskSize = defaultWriteOperationCost + estimateSize(keyData) + estimateSize(value);
-    auto* objectStore = m_databaseInfo->infoForExistingObjectStore(objectStoreIdentifier);
-    if (objectStore)
-        taskSize += objectStore->indexNames().size() * taskSize;
-    if (m_server.requestSpace(m_identifier.origin(), taskSize) == StorageQuotaManager::Decision::Deny) {
-        callback(IDBError { QuotaExceededError, quotaErrorMessageName("PutOrAdd") }, usedKey);
-        return;
-    }
-
     bool usedKeyIsGenerated = false;
     uint64_t keyNumber;
     auto transactionIdentifier = requestData.transactionIdentifier();
@@ -741,6 +755,9 @@ void UniqueIDBDatabase::putOrAdd(const IDBRequestData& requestData, const IDBKey
     } else
         usedKey = keyData;
 
+    // Generate index keys up front for more accurate quota check.
+    auto indexKeys = generateIndexKeyMapForValue(m_backingStore->serializationContext().execState(), *objectStoreInfo, usedKey, value);
+
     if (overwriteMode == IndexedDB::ObjectStoreOverwriteMode::NoOverwrite) {
         bool keyExists;
         error = m_backingStore->keyExistsInObjectStore(transactionIdentifier, objectStoreIdentifier, usedKey, keyExists);
@@ -752,6 +769,20 @@ void UniqueIDBDatabase::putOrAdd(const IDBRequestData& requestData, const IDBKey
             return;
         }
     }
+
+    // Quota check.
+    auto keySize = estimateSize(usedKey);
+    auto valueSize = estimateSize(value);
+    auto indexSize = estimateSize(*objectStoreInfo, indexKeys, keySize);
+    auto taskSize = defaultWriteOperationCost + keySize + valueSize + indexSize;
+
+    LOG(IndexedDB, "UniqueIDBDatabase::putOrAdd quota check with task size: %llu key size: %llu value size: %llu index size: %llu", taskSize, keySize, valueSize, indexSize);
+
+    if (m_server.requestSpace(m_identifier.origin(), taskSize) == StorageQuotaManager::Decision::Deny) {
+        callback(IDBError { QuotaExceededError, quotaErrorMessageName("PutOrAdd") }, usedKey);
+        return;
+    }
+
     // If a record already exists in store, then remove the record from store using the steps for deleting records from an object store.
     // This is important because formally deleting it from from the object store also removes it from the appropriate indexes.
     error = m_backingStore->deleteRange(transactionIdentifier, objectStoreIdentifier, usedKey);
@@ -760,7 +791,7 @@ void UniqueIDBDatabase::putOrAdd(const IDBRequestData& requestData, const IDBKey
         return;
     }
 
-    error = m_backingStore->addRecord(transactionIdentifier, *objectStoreInfo, usedKey, value);
+    error = m_backingStore->addRecord(transactionIdentifier, *objectStoreInfo, usedKey, indexKeys, value);
     if (!error.isNull()) {
         callback(error, usedKey);
         return;
index c380c49..8751ca1 100644 (file)
@@ -28,6 +28,7 @@
 #if ENABLE(INDEXED_DATABASE)
 
 #include "IDBKeyData.h"
+#include <wtf/HashMap.h>
 #include <wtf/Vector.h>
 
 namespace WebCore {
@@ -48,6 +49,8 @@ private:
     Vector<IDBKeyData> m_keys;
 };
 
+typedef HashMap<uint64_t, IndexKey> IndexIDToIndexKeyMap;
+
 } // namespace WebCore
 
 #endif // ENABLE(INDEXED_DATABASE)
index 033aa59..3a8f8fa 100644 (file)
                516D7D701BB5F0BD00AF7C77 /* IDBConnectionToServerDelegate.h in Headers */ = {isa = PBXBuildFile; fileRef = 5185FCBD1BB5CB770012898F /* IDBConnectionToServerDelegate.h */; settings = {ATTRIBUTES = (Private, ); }; };
                516D7D721BB5F0BD00AF7C77 /* IDBConnectionToClientDelegate.h in Headers */ = {isa = PBXBuildFile; fileRef = 516D7D6E1BB5F06500AF7C77 /* IDBConnectionToClientDelegate.h */; settings = {ATTRIBUTES = (Private, ); }; };
                516F7F6D1C31E39A00F111DC /* ServerOpenDBRequest.h in Headers */ = {isa = PBXBuildFile; fileRef = 516F7F6C1C31C79D00F111DC /* ServerOpenDBRequest.h */; settings = {ATTRIBUTES = (Private, ); }; };
-               517138F01BED1D1A000D5F01 /* IndexKey.h in Headers */ = {isa = PBXBuildFile; fileRef = 517138EE1BED1D17000D5F01 /* IndexKey.h */; };
+               517138F01BED1D1A000D5F01 /* IndexKey.h in Headers */ = {isa = PBXBuildFile; fileRef = 517138EE1BED1D17000D5F01 /* IndexKey.h */; settings = {ATTRIBUTES = (Private, ); }; };
                517138F81BF128BB000D5F01 /* IndexValueStore.h in Headers */ = {isa = PBXBuildFile; fileRef = 517138F61BF12262000D5F01 /* IndexValueStore.h */; };
                517138FC1BF3ADF4000D5F01 /* IDBCursorInfo.h in Headers */ = {isa = PBXBuildFile; fileRef = 517138FA1BF3ADAC000D5F01 /* IDBCursorInfo.h */; settings = {ATTRIBUTES = (Private, ); }; };
                517139061BF64DEC000D5F01 /* MemoryObjectStoreCursor.h in Headers */ = {isa = PBXBuildFile; fileRef = 517139041BF64DE3000D5F01 /* MemoryObjectStoreCursor.h */; };
index 578c136..2f5af6c 100644 (file)
@@ -36,6 +36,7 @@
 #include "IDBKey.h"
 #include "IDBKeyData.h"
 #include "IDBKeyPath.h"
+#include "IDBObjectStoreInfo.h"
 #include "IDBValue.h"
 #include "IndexKey.h"
 #include "JSBlob.h"
@@ -463,6 +464,34 @@ void generateIndexKeyForValue(JSGlobalObject& lexicalGlobalObject, const IDBInde
     outKey = IndexKey(WTFMove(keyDatas));
 }
 
+IndexIDToIndexKeyMap generateIndexKeyMapForValue(JSC::JSGlobalObject& lexicalGlobalObject, const IDBObjectStoreInfo& storeInfo, const IDBKeyData& key, const IDBValue& value)
+{
+    auto& indexMap = storeInfo.indexMap();
+    auto indexCount = indexMap.size();
+    if (!indexCount)
+        return IndexIDToIndexKeyMap { };
+
+    JSLockHolder locker(lexicalGlobalObject.vm());
+    auto jsValue = deserializeIDBValueToJSValue(lexicalGlobalObject, value);
+    if (jsValue.isUndefinedOrNull())
+        return IndexIDToIndexKeyMap { };
+
+    IndexIDToIndexKeyMap indexKeys;
+    indexKeys.reserveInitialCapacity(indexCount);
+
+    for (const auto& [indexID, indexInfo] : indexMap) {
+        IndexKey indexKey;
+        generateIndexKeyForValue(lexicalGlobalObject, indexInfo, jsValue, indexKey, storeInfo.keyPath(), key);
+
+        if (indexKey.isNull())
+            continue;
+
+        indexKeys.add(indexID, WTFMove(indexKey));
+    }
+
+    return indexKeys;
+}
+
 Optional<JSC::JSValue> deserializeIDBValueWithKeyInjection(JSGlobalObject& lexicalGlobalObject, const IDBValue& value, const IDBKeyData& key, const Optional<IDBKeyPath>& keyPath)
 {
     auto jsValue = deserializeIDBValueToJSValue(lexicalGlobalObject, value);
index 46da5ae..8ab74f8 100644 (file)
@@ -29,6 +29,7 @@
 #if ENABLE(INDEXED_DATABASE)
 
 #include "IDBKeyPath.h"
+#include "IndexKey.h"
 #include <wtf/Forward.h>
 
 namespace JSC {
@@ -42,6 +43,7 @@ namespace WebCore {
 class IDBIndexInfo;
 class IDBKey;
 class IDBKeyData;
+class IDBObjectStoreInfo;
 class IDBValue;
 class IndexKey;
 class JSDOMGlobalObject;
@@ -52,6 +54,8 @@ bool injectIDBKeyIntoScriptValue(JSC::JSGlobalObject&, const IDBKeyData&, JSC::J
 
 void generateIndexKeyForValue(JSC::JSGlobalObject&, const IDBIndexInfo&, JSC::JSValue, IndexKey& outKey, const Optional<IDBKeyPath>&, const IDBKeyData&);
 
+IndexIDToIndexKeyMap generateIndexKeyMapForValue(JSC::JSGlobalObject&, const IDBObjectStoreInfo&, const IDBKeyData&, const IDBValue&);
+
 Ref<IDBKey> scriptValueToIDBKey(JSC::JSGlobalObject&, const JSC::JSValue&);
 
 JSC::JSValue deserializeIDBValueToJSValue(JSC::JSGlobalObject&, const IDBValue&, Vector<std::pair<String, String>>&);