Modern IDB: SQLite backend doesn't update index records as object records are added.
authorbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 27 Jan 2016 21:57:22 +0000 (21:57 +0000)
committerbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 27 Jan 2016 21:57:22 +0000 (21:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=153548

Reviewed by Alex Christensen.

Source/WebCore:

No new tests (4 more tests pass, others improve).

* Modules/indexeddb/server/IDBBackingStore.h:

* Modules/indexeddb/server/MemoryIDBBackingStore.cpp:
(WebCore::IDBServer::MemoryIDBBackingStore::addRecord):
* Modules/indexeddb/server/MemoryIDBBackingStore.h:

* Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:
(WebCore::IDBServer::SQLiteIDBBackingStore::initializeVM):
(WebCore::IDBServer::SQLiteIDBBackingStore::vm):
(WebCore::IDBServer::SQLiteIDBBackingStore::globalObject):
(WebCore::IDBServer::SQLiteIDBBackingStore::createIndex):
(WebCore::IDBServer::SQLiteIDBBackingStore::uncheckedPutIndexKey):
(WebCore::IDBServer::SQLiteIDBBackingStore::updateIndexesForAddRecord):
(WebCore::IDBServer::SQLiteIDBBackingStore::addRecord):
* Modules/indexeddb/server/SQLiteIDBBackingStore.h:

* Modules/indexeddb/server/UniqueIDBDatabase.cpp:
(WebCore::IDBServer::UniqueIDBDatabase::performPutOrAdd):

* Modules/indexeddb/shared/IDBObjectStoreInfo.h:

LayoutTests:

* platform/mac-wk1/TestExpectations:

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

LayoutTests/ChangeLog
LayoutTests/platform/mac-wk1/TestExpectations
Source/WebCore/ChangeLog
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/SQLiteIDBBackingStore.cpp
Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.h
Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp
Source/WebCore/Modules/indexeddb/shared/IDBObjectStoreInfo.h

index f56be03..998ab01 100644 (file)
@@ -1,5 +1,14 @@
 2016-01-27  Brady Eidson  <beidson@apple.com>
 
+        Modern IDB: SQLite backend doesn't update index records as object records are added.
+        https://bugs.webkit.org/show_bug.cgi?id=153548
+
+        Reviewed by Alex Christensen.
+
+        * platform/mac-wk1/TestExpectations:
+
+2016-01-27  Brady Eidson  <beidson@apple.com>
+
         LayoutTest fast/loader/stateobjects/replacestate-frequency-iframe.html is flaky on El Cap, always times out on Yosemite.
         https://bugs.webkit.org/show_bug.cgi?id=153551
         
index a6f135b..3291bb9 100644 (file)
@@ -464,11 +464,8 @@ storage/indexeddb/cursor-finished.html [ Failure ]
 storage/indexeddb/cursor-inconsistency.html [ Failure ]
 storage/indexeddb/cursor-index-delete.html [ Failure ]
 storage/indexeddb/cursor-key-order.html [ Failure ]
-storage/indexeddb/cursor-overloads.html [ Failure ]
 storage/indexeddb/cursor-prev-no-duplicate.html [ Failure ]
 storage/indexeddb/cursor-primary-key-order.html [ Failure ]
-storage/indexeddb/cursor-properties.html [ Failure ]
-storage/indexeddb/cursor-reverse-bug.html [ Failure ]
 storage/indexeddb/cursor-skip-deleted.html [ Failure ]
 storage/indexeddb/cursor-update.html [ Failure ]
 storage/indexeddb/cursor-value.html [ Failure ]
@@ -515,7 +512,6 @@ storage/indexeddb/modern/index-get-count-basic.html [ Failure ]
 storage/indexeddb/modern/objectstore-cursor-advance-failures.html [ Failure ]
 storage/indexeddb/modern/objectstore-cursor-continue-failures.html [ Failure ]
 storage/indexeddb/modern/request-readystate.html [ Failure ]
-storage/indexeddb/mozilla/autoincrement-indexes.html [ Failure ]
 storage/indexeddb/mozilla/clear.html [ Failure ]
 storage/indexeddb/mozilla/cursor-mutation-objectstore-only.html [ Failure ]
 storage/indexeddb/mozilla/cursor-mutation.html [ Failure ]
index cf5d736..c9d742e 100644 (file)
@@ -1,3 +1,33 @@
+2016-01-27  Brady Eidson  <beidson@apple.com>
+
+        Modern IDB: SQLite backend doesn't update index records as object records are added.
+        https://bugs.webkit.org/show_bug.cgi?id=153548
+
+        Reviewed by Alex Christensen.
+
+        No new tests (4 more tests pass, others improve).
+
+        * Modules/indexeddb/server/IDBBackingStore.h:
+        
+        * Modules/indexeddb/server/MemoryIDBBackingStore.cpp:
+        (WebCore::IDBServer::MemoryIDBBackingStore::addRecord):
+        * Modules/indexeddb/server/MemoryIDBBackingStore.h:
+        
+        * Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:
+        (WebCore::IDBServer::SQLiteIDBBackingStore::initializeVM):
+        (WebCore::IDBServer::SQLiteIDBBackingStore::vm):
+        (WebCore::IDBServer::SQLiteIDBBackingStore::globalObject):
+        (WebCore::IDBServer::SQLiteIDBBackingStore::createIndex):
+        (WebCore::IDBServer::SQLiteIDBBackingStore::uncheckedPutIndexKey):
+        (WebCore::IDBServer::SQLiteIDBBackingStore::updateIndexesForAddRecord):
+        (WebCore::IDBServer::SQLiteIDBBackingStore::addRecord):
+        * Modules/indexeddb/server/SQLiteIDBBackingStore.h:
+        
+        * Modules/indexeddb/server/UniqueIDBDatabase.cpp:
+        (WebCore::IDBServer::UniqueIDBDatabase::performPutOrAdd):
+        
+        * Modules/indexeddb/shared/IDBObjectStoreInfo.h:
+
 2016-01-27  Ryosuke Niwa  <rniwa@webkit.org>
 
         Add API to access closed shadowRoot in InjectedBundle
index f1e8838..b0f97d3 100644 (file)
@@ -67,7 +67,7 @@ public:
     virtual IDBError deleteIndex(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, uint64_t indexIdentifier) = 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, uint64_t objectStoreIdentifier, const IDBKeyData&, const ThreadSafeDataBuffer& value) = 0;
+    virtual IDBError addRecord(const IDBResourceIdentifier& transactionIdentifier, const IDBObjectStoreInfo&, const IDBKeyData&, const ThreadSafeDataBuffer& value) = 0;
     virtual IDBError getRecord(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, const IDBKeyRangeData&, ThreadSafeDataBuffer& outValue) = 0;
     virtual IDBError getIndexRecord(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, uint64_t indexIdentifier, IndexedDB::IndexRecordType, const IDBKeyRangeData&, IDBGetResult& outValue) = 0;
     virtual IDBError getCount(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, uint64_t indexIdentifier, const IDBKeyRangeData&, uint64_t& outCount) = 0;
index e992ce0..74dd6f4 100644 (file)
@@ -266,17 +266,17 @@ IDBError MemoryIDBBackingStore::deleteRange(const IDBResourceIdentifier& transac
     return IDBError();
 }
 
-IDBError MemoryIDBBackingStore::addRecord(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, const IDBKeyData& keyData, const ThreadSafeDataBuffer& value)
+IDBError MemoryIDBBackingStore::addRecord(const IDBResourceIdentifier& transactionIdentifier, const IDBObjectStoreInfo& objectStoreInfo, const IDBKeyData& keyData, const ThreadSafeDataBuffer& value)
 {
     LOG(IndexedDB, "MemoryIDBBackingStore::addRecord");
 
-    ASSERT(objectStoreIdentifier);
+    ASSERT(objectStoreInfo.identifier());
 
     auto transaction = m_transactions.get(transactionIdentifier);
     if (!transaction)
         return IDBError(IDBDatabaseException::UnknownError, ASCIILiteral("No backing store transaction found to put record"));
 
-    MemoryObjectStore* objectStore = m_objectStoresByIdentifier.get(objectStoreIdentifier);
+    MemoryObjectStore* objectStore = m_objectStoresByIdentifier.get(objectStoreInfo.identifier());
     if (!objectStore)
         return IDBError(IDBDatabaseException::UnknownError, ASCIILiteral("No backing store object store found to put record"));
 
index 4b13af6..63a9ea5 100644 (file)
@@ -59,7 +59,7 @@ public:
     virtual IDBError deleteIndex(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, uint64_t indexIdentifier) override final;
     virtual IDBError keyExistsInObjectStore(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, const IDBKeyData&, bool& keyExists) override final;
     virtual IDBError deleteRange(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, const IDBKeyRangeData&) override final;
-    virtual IDBError addRecord(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, const IDBKeyData&, const ThreadSafeDataBuffer& value) override final;
+    virtual IDBError addRecord(const IDBResourceIdentifier& transactionIdentifier, const IDBObjectStoreInfo&, const IDBKeyData&, const ThreadSafeDataBuffer& value) override final;
     virtual IDBError getRecord(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, const IDBKeyRangeData&, ThreadSafeDataBuffer& outValue) override final;
     virtual IDBError getIndexRecord(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, uint64_t indexIdentifier, IndexedDB::IndexRecordType, const IDBKeyRangeData&, IDBGetResult& outValue) override final;
     virtual IDBError getCount(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, uint64_t indexIdentifier, const IDBKeyRangeData&, uint64_t& outCount) override final;
index 93660ce..ed82d90 100644 (file)
@@ -33,6 +33,7 @@
 #include "IDBDatabaseException.h"
 #include "IDBGetResult.h"
 #include "IDBKeyData.h"
+#include "IDBObjectStoreInfo.h"
 #include "IDBSerialization.h"
 #include "IDBTransactionInfo.h"
 #include "IndexKey.h"
@@ -126,6 +127,30 @@ SQLiteIDBBackingStore::~SQLiteIDBBackingStore()
     }
 }
 
+
+void SQLiteIDBBackingStore::initializeVM()
+{
+    if (!m_vm) {
+        ASSERT(!m_globalObject);
+        m_vm = VM::create();
+
+        JSLockHolder locker(m_vm.get());
+        m_globalObject.set(*m_vm, JSGlobalObject::create(*m_vm, JSGlobalObject::createStructure(*m_vm, jsNull())));
+    }
+}
+
+VM& SQLiteIDBBackingStore::vm()
+{
+    initializeVM();
+    return *m_vm;
+}
+
+JSGlobalObject& SQLiteIDBBackingStore::globalObject()
+{
+    initializeVM();
+    return **m_globalObject;
+}
+
 static bool createOrMigrateRecordsTableIfNecessary(SQLiteDatabase& database)
 {
     String currentSchema;
@@ -727,15 +752,7 @@ IDBError SQLiteIDBBackingStore::createIndex(const IDBResourceIdentifier& transac
         return { IDBDatabaseException::UnknownError, ASCIILiteral("Unable to populate indexes in database") };
     }
 
-    if (!m_vm) {
-        ASSERT(!m_globalObject);
-        m_vm = VM::create();
-    }
-
-    JSLockHolder locker(m_vm.get());
-
-    if (!m_globalObject)
-        m_globalObject.set(*m_vm, JSGlobalObject::create(*m_vm, JSGlobalObject::createStructure(*m_vm, jsNull())));
+    JSLockHolder locker(vm());
 
     while (!cursor->currentKey().isNull()) {
         auto& key = cursor->currentKey();
@@ -817,6 +834,40 @@ IDBError SQLiteIDBBackingStore::uncheckedHasIndexRecord(int64_t indexID, const I
     return { };
 }
 
+IDBError SQLiteIDBBackingStore::uncheckedPutIndexKey(const IDBIndexInfo& info, const IDBKeyData& key, const IndexKey& indexKey)
+{
+    if (!info.multiEntry()) {
+        auto error = uncheckedPutIndexRecord(info.objectStoreIdentifier(), info.identifier(), key, indexKey.asOneKey());
+        if (!error.isNull()) {
+            LOG_ERROR("Unable to put index record for newly created index");
+            return error;
+        }
+    }
+
+    Vector<IDBKeyData> indexKeys = indexKey.multiEntry();
+
+    if (info.unique()) {
+        bool hasRecord;
+        IDBError error;
+        for (auto& indexKey : indexKeys) {
+            error = uncheckedHasIndexRecord(info.identifier(), indexKey, hasRecord);
+            if (!error.isNull())
+                return error;
+            if (hasRecord)
+                return IDBError(IDBDatabaseException::ConstraintError);
+        }
+    }
+
+    for (auto& indexKey : indexKeys) {
+        auto error = uncheckedPutIndexRecord(info.objectStoreIdentifier(), info.identifier(), key, indexKey);
+        if (!error.isNull()) {
+            LOG_ERROR("Unable to put index record for newly created index");
+            return error;
+        }
+    }
+
+    return { };
+}
 
 IDBError SQLiteIDBBackingStore::uncheckedPutIndexRecord(int64_t objectStoreID, int64_t indexID, const WebCore::IDBKeyData& keyValue, const WebCore::IDBKeyData& indexKey)
 {
@@ -1012,9 +1063,39 @@ IDBError SQLiteIDBBackingStore::deleteRange(const IDBResourceIdentifier& transac
     return { IDBDatabaseException::UnknownError, ASCIILiteral("Currently unable to delete all records in a multi-key range") };
 }
 
-IDBError SQLiteIDBBackingStore::addRecord(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreID, const IDBKeyData& keyData, const ThreadSafeDataBuffer& value)
+IDBError SQLiteIDBBackingStore::updateIndexesForAddRecord(const IDBObjectStoreInfo& info, const IDBKeyData& key, const ThreadSafeDataBuffer& value)
+{
+    JSLockHolder locker(vm());
+
+    auto jsValue = deserializeIDBValueDataToJSValue(*globalObject().globalExec(), value);
+    if (jsValue.isUndefinedOrNull())
+        return { };
+
+    IDBError error;
+    Vector<std::pair<uint64_t, IndexKey>> changedIndexRecords;
+
+    for (auto& index : info.indexMap().values()) {
+        IndexKey indexKey;
+        generateIndexKeyForValue(*m_globalObject->globalExec(), index, jsValue, indexKey);
+
+        if (indexKey.isNull())
+            continue;
+
+        error = uncheckedPutIndexKey(index, key, indexKey);
+        if (!error.isNull())
+            break;
+
+        changedIndexRecords.append(std::make_pair(index.identifier(), indexKey));
+    }
+
+    // FIXME: If any of the index puts failed, revert the ones that went through (changedIndexRecords).
+
+    return error;
+}
+
+IDBError SQLiteIDBBackingStore::addRecord(const IDBResourceIdentifier& transactionIdentifier, const IDBObjectStoreInfo& objectStoreInfo, const IDBKeyData& keyData, const ThreadSafeDataBuffer& value)
 {
-    LOG(IndexedDB, "SQLiteIDBBackingStore::addRecord - key %s, object store %" PRIu64, keyData.loggingString().utf8().data(), objectStoreID);
+    LOG(IndexedDB, "SQLiteIDBBackingStore::addRecord - key %s, object store %" PRIu64, keyData.loggingString().utf8().data(), objectStoreInfo.identifier());
 
     ASSERT(m_sqliteDB);
     ASSERT(m_sqliteDB->isOpen());
@@ -1038,16 +1119,20 @@ IDBError SQLiteIDBBackingStore::addRecord(const IDBResourceIdentifier& transacti
     {
         SQLiteStatement sql(*m_sqliteDB, ASCIILiteral("INSERT INTO Records VALUES (?, CAST(? AS TEXT), ?);"));
         if (sql.prepare() != SQLITE_OK
-            || sql.bindInt64(1, objectStoreID) != SQLITE_OK
+            || sql.bindInt64(1, objectStoreInfo.identifier()) != SQLITE_OK
             || sql.bindBlob(2, keyBuffer->data(), keyBuffer->size()) != SQLITE_OK
             || sql.bindBlob(3, value.data()->data(), value.data()->size()) != SQLITE_OK
             || sql.step() != SQLITE_DONE) {
-            LOG_ERROR("Could not put record for object store %" PRIi64 " in Records table (%i) - %s", objectStoreID, m_sqliteDB->lastError(), m_sqliteDB->lastErrorMsg());
+            LOG_ERROR("Could not put record for object store %" PRIi64 " in Records table (%i) - %s", objectStoreInfo.identifier(), m_sqliteDB->lastError(), m_sqliteDB->lastErrorMsg());
             return { IDBDatabaseException::UnknownError, ASCIILiteral("Unable to store record in object store") };
         }
     }
 
-    return { };
+    auto error = updateIndexesForAddRecord(objectStoreInfo, keyData, value);
+
+    // FIXME: If there was an error indexing this record, remove it.
+
+    return error;
 }
 
 IDBError SQLiteIDBBackingStore::getRecord(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreID, const IDBKeyRangeData& keyRange, ThreadSafeDataBuffer& resultValue)
index 3d213f0..d195adc 100644 (file)
@@ -38,6 +38,7 @@
 
 namespace WebCore {
 
+class IndexKey;
 class SQLiteDatabase;
 
 namespace IDBServer {
@@ -62,7 +63,7 @@ public:
     virtual IDBError deleteIndex(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, uint64_t indexIdentifier) override final;
     virtual IDBError keyExistsInObjectStore(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, const IDBKeyData&, bool& keyExists) override final;
     virtual IDBError deleteRange(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, const IDBKeyRangeData&) override final;
-    virtual IDBError addRecord(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, const IDBKeyData&, const ThreadSafeDataBuffer& value) override final;
+    virtual IDBError addRecord(const IDBResourceIdentifier& transactionIdentifier, const IDBObjectStoreInfo&, const IDBKeyData&, const ThreadSafeDataBuffer& value) override final;
     virtual IDBError getRecord(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, const IDBKeyRangeData&, ThreadSafeDataBuffer& outValue) override final;
     virtual IDBError getIndexRecord(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, uint64_t indexIdentifier, IndexedDB::IndexRecordType, const IDBKeyRangeData&, IDBGetResult& outValue) override final;
     virtual IDBError getCount(const IDBResourceIdentifier& transactionIdentifier, uint64_t objectStoreIdentifier, uint64_t indexIdentifier, const IDBKeyRangeData&, uint64_t& outCount) override final;
@@ -87,10 +88,16 @@ private:
     std::unique_ptr<IDBDatabaseInfo> extractExistingDatabaseInfo();
 
     IDBError deleteRecord(SQLiteIDBTransaction&, int64_t objectStoreID, const IDBKeyData&);
+    IDBError uncheckedPutIndexKey(const IDBIndexInfo&, const IDBKeyData& keyValue, const IndexKey&);
     IDBError uncheckedPutIndexRecord(int64_t objectStoreID, int64_t indexID, const IDBKeyData& keyValue, const IDBKeyData& indexKey);
     IDBError uncheckedHasIndexRecord(int64_t indexID, const IDBKeyData&, bool& hasRecord);
     IDBError uncheckedGetKeyGeneratorValue(int64_t objectStoreID, uint64_t& outValue);
     IDBError uncheckedSetKeyGeneratorValue(int64_t objectStoreID, uint64_t value);
+    IDBError updateIndexesForAddRecord(const IDBObjectStoreInfo&, const IDBKeyData&, const ThreadSafeDataBuffer& value);
+
+    JSC::VM& vm();
+    JSC::JSGlobalObject& globalObject();
+    void initializeVM();
 
     IDBDatabaseIdentifier m_identifier;
     std::unique_ptr<IDBDatabaseInfo> m_databaseInfo;
index 80c21a0..8058de4 100644 (file)
@@ -791,7 +791,7 @@ void UniqueIDBDatabase::performPutOrAdd(uint64_t callbackIdentifier, const IDBRe
         return;
     }
 
-    error = m_backingStore->addRecord(transactionIdentifier, objectStoreIdentifier, usedKey, injectedRecordValue.data() ? injectedRecordValue : originalRecordValue);
+    error = m_backingStore->addRecord(transactionIdentifier, *objectStoreInfo, usedKey, injectedRecordValue.data() ? injectedRecordValue : originalRecordValue);
     if (!error.isNull()) {
         m_server.postDatabaseTaskReply(createCrossThreadTask(*this, &UniqueIDBDatabase::didPerformPutOrAdd, callbackIdentifier, error, usedKey));
         return;
index d67b2cc..2498d85 100644 (file)
@@ -57,6 +57,7 @@ public:
     IDBIndexInfo* infoForExistingIndex(const String& name);
 
     Vector<String> indexNames() const;
+    const HashMap<uint64_t, IDBIndexInfo>& indexMap() const { return m_indexMap; }
 
     void deleteIndex(const String& indexName);
     void deleteIndex(uint64_t indexIdentifier);