Title: [195765] trunk
Revision
195765
Author
beid...@apple.com
Date
2016-01-28 11:20:25 -0800 (Thu, 28 Jan 2016)

Log Message

Modern IDB: Index uniqueness broken in the SQLite backend.
https://bugs.webkit.org/show_bug.cgi?id=153596

Reviewed by Alex Christensen.

Source/WebCore:

No new tests (Many failing tests now pass, others improve).

* Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:
(WebCore::IDBServer::SQLiteIDBBackingStore::createIndex):
(WebCore::IDBServer::SQLiteIDBBackingStore::uncheckedHasIndexRecord):
(WebCore::IDBServer::SQLiteIDBBackingStore::uncheckedPutIndexKey):
(WebCore::IDBServer::SQLiteIDBBackingStore::updateOneIndexForAddRecord):
(WebCore::IDBServer::SQLiteIDBBackingStore::updateAllIndexesForAddRecord):
(WebCore::IDBServer::SQLiteIDBBackingStore::addRecord):
(WebCore::IDBServer::SQLiteIDBBackingStore::updateIndexesForAddRecord): Deleted.
* Modules/indexeddb/server/SQLiteIDBBackingStore.h:

LayoutTests:

* platform/mac-wk1/TestExpectations:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (195764 => 195765)


--- trunk/LayoutTests/ChangeLog	2016-01-28 19:17:17 UTC (rev 195764)
+++ trunk/LayoutTests/ChangeLog	2016-01-28 19:20:25 UTC (rev 195765)
@@ -1,3 +1,12 @@
+2016-01-28  Brady Eidson  <beid...@apple.com>
+
+        Modern IDB: Index uniqueness broken in the SQLite backend.
+        https://bugs.webkit.org/show_bug.cgi?id=153596
+
+        Reviewed by Alex Christensen.
+
+        * platform/mac-wk1/TestExpectations:
+
 2016-01-28  Zalan Bujtas  <za...@apple.com>
 
         [Win] gardening after r195740. (more to follow)

Modified: trunk/LayoutTests/platform/mac-wk1/TestExpectations (195764 => 195765)


--- trunk/LayoutTests/platform/mac-wk1/TestExpectations	2016-01-28 19:17:17 UTC (rev 195764)
+++ trunk/LayoutTests/platform/mac-wk1/TestExpectations	2016-01-28 19:20:25 UTC (rev 195765)
@@ -458,26 +458,14 @@
 storage/indexeddb/delete-range.html [ Failure ]
 storage/indexeddb/get-keyrange.html [ Failure ]
 storage/indexeddb/index-duplicate-keypaths.html [ Failure ]
-storage/indexeddb/index-multientry.html [ Failure ]
-storage/indexeddb/index-population.html [ Failure ]
-storage/indexeddb/index-unique.html [ Failure ]
 storage/indexeddb/key-generator.html [ Failure ]
-storage/indexeddb/lazy-index-population.html [ Failure ]
-storage/indexeddb/lazy-index-types.html [ Failure ]
 storage/indexeddb/modern/cursor-7.html [ Failure ]
-storage/indexeddb/modern/deleteindex-1.html [ Failure ]
-storage/indexeddb/modern/deleteindex-2.html [ Failure ]
 storage/indexeddb/modern/get-keyrange.html [ Failure ]
 storage/indexeddb/modern/idbobjectstore-delete-1.html [ Failure ]
 storage/indexeddb/modern/index-3.html [ Failure ]
-storage/indexeddb/modern/index-4.html [ Failure ]
 storage/indexeddb/mozilla/cursor-mutation.html [ Failure ]
 storage/indexeddb/mozilla/cursors.html [ Failure ]
-storage/indexeddb/mozilla/index-prev-no-duplicate.html [ Failure ]
-storage/indexeddb/mozilla/indexes.html [ Failure ]
 storage/indexeddb/objectstore-autoincrement.html [ Failure ]
-storage/indexeddb/objectstore-basics.html [ Failure ]
-storage/indexeddb/opencursor-key.html [ Failure ]
 
 # SQLite backend tests that timeout
 storage/indexeddb/modern/transaction-scheduler-1.html [ Skip ]

Modified: trunk/Source/WebCore/ChangeLog (195764 => 195765)


--- trunk/Source/WebCore/ChangeLog	2016-01-28 19:17:17 UTC (rev 195764)
+++ trunk/Source/WebCore/ChangeLog	2016-01-28 19:20:25 UTC (rev 195765)
@@ -1,3 +1,22 @@
+2016-01-28  Brady Eidson  <beid...@apple.com>
+
+        Modern IDB: Index uniqueness broken in the SQLite backend.
+        https://bugs.webkit.org/show_bug.cgi?id=153596
+
+        Reviewed by Alex Christensen.
+
+        No new tests (Many failing tests now pass, others improve).
+
+        * Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:
+        (WebCore::IDBServer::SQLiteIDBBackingStore::createIndex):
+        (WebCore::IDBServer::SQLiteIDBBackingStore::uncheckedHasIndexRecord):
+        (WebCore::IDBServer::SQLiteIDBBackingStore::uncheckedPutIndexKey):
+        (WebCore::IDBServer::SQLiteIDBBackingStore::updateOneIndexForAddRecord):
+        (WebCore::IDBServer::SQLiteIDBBackingStore::updateAllIndexesForAddRecord):
+        (WebCore::IDBServer::SQLiteIDBBackingStore::addRecord):
+        (WebCore::IDBServer::SQLiteIDBBackingStore::updateIndexesForAddRecord): Deleted.
+        * Modules/indexeddb/server/SQLiteIDBBackingStore.h:
+
 2016-01-08  Jer Noble  <jer.no...@apple.com>
 
         Custom protocol loading through AVFoundation does not support byte-range requests.

Modified: trunk/Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp (195764 => 195765)


--- trunk/Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp	2016-01-28 19:17:17 UTC (rev 195764)
+++ trunk/Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp	2016-01-28 19:20:25 UTC (rev 195765)
@@ -711,6 +711,7 @@
 
 IDBError SQLiteIDBBackingStore::createIndex(const IDBResourceIdentifier& transactionIdentifier, const IDBIndexInfo& info)
 {
+    LOG(IndexedDB, "SQLiteIDBBackingStore::createIndex - ObjectStore %" PRIu64 ", Index %" PRIu64, info.objectStoreIdentifier(), info.identifier());
     ASSERT(m_sqliteDB);
     ASSERT(m_sqliteDB->isOpen());
 
@@ -752,47 +753,16 @@
         return { IDBDatabaseException::UnknownError, ASCIILiteral("Unable to populate indexes in database") };
     }
 
-    JSLockHolder locker(vm());
-
     while (!cursor->currentKey().isNull()) {
         auto& key = cursor->currentKey();
-        auto& valueBuffer = cursor->currentValueBuffer();
+        auto valueBuffer = ThreadSafeDataBuffer::copyVector(cursor->currentValueBuffer());
 
-        auto value = deserializeIDBValueBuffer(m_globalObject->globalExec(), Vector<uint8_t>(valueBuffer), true);
-
-        IndexKey indexKey;
-        generateIndexKeyForValue(*m_globalObject->globalExec(), info, value.jsValue(), indexKey);
-
-        if (!info.multiEntry()) {
-            IDBError 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;
-            }
+        IDBError error = updateOneIndexForAddRecord(info, key, valueBuffer);
+        if (!error.isNull()) {
+            // FIXME: Remove this newly added 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 (hasRecord)
-                    return IDBError(IDBDatabaseException::ConstraintError);
-                if (!error.isNull())
-                    return error;
-            }
-        }
-
-        for (auto& indexKey : indexKeys) {
-            IDBError error = uncheckedPutIndexRecord(info.objectStoreIdentifier(), info.identifier(), key, indexKey);
-            if (!error.isNull()) {
-                LOG_ERROR("Unable to put index record for newly created index");
-                return error;
-            }
-        }
-
         if (!cursor->advance(1)) {
             LOG_ERROR("Error advancing cursor while indexing existing records for new index.");
             return { IDBDatabaseException::UnknownError, ASCIILiteral("Error advancing cursor while indexing existing records for new index") };
@@ -802,7 +772,7 @@
     return { };
 }
 
-IDBError SQLiteIDBBackingStore::uncheckedHasIndexRecord(int64_t indexID, const IDBKeyData& indexKey, bool& hasRecord)
+IDBError SQLiteIDBBackingStore::uncheckedHasIndexRecord(const IDBIndexInfo& info, const IDBKeyData& indexKey, bool& hasRecord)
 {
     hasRecord = false;
 
@@ -812,10 +782,11 @@
         return { IDBDatabaseException::UnknownError, ASCIILiteral("Unable to serialize IDBKey to check for index record in database") };
     }
 
-    SQLiteStatement sql(*m_sqliteDB, ASCIILiteral("SELECT rowid FROM IndexRecords WHERE indexID = ? AND key = CAST(? AS TEXT);"));
+    SQLiteStatement sql(*m_sqliteDB, ASCIILiteral("SELECT rowid FROM IndexRecords WHERE indexID = ? AND objectStoreID = ? AND key = CAST(? AS TEXT);"));
     if (sql.prepare() != SQLITE_OK
-        || sql.bindInt64(1, indexID) != SQLITE_OK
-        || sql.bindBlob(2, indexKeyBuffer->data(), indexKeyBuffer->size()) != SQLITE_OK) {
+        || sql.bindInt64(1, info.identifier()) != SQLITE_OK
+        || sql.bindInt64(2, info.objectStoreIdentifier()) != SQLITE_OK
+        || sql.bindBlob(3, indexKeyBuffer->data(), indexKeyBuffer->size()) != SQLITE_OK) {
         LOG_ERROR("Error checking for index record in database");
         return { IDBDatabaseException::UnknownError, ASCIILiteral("Error checking for index record in database") };
     }
@@ -836,21 +807,19 @@
 
 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");
+    LOG(IndexedDB, "SQLiteIDBBackingStore::uncheckedPutIndexKey - (%" PRIu64 ") %s, %s", info.identifier(), key.loggingString().utf8().data(), indexKey.asOneKey().loggingString().utf8().data());
 
-        return error;
-    }
+    Vector<IDBKeyData> indexKeys;
+    if (info.multiEntry())
+        indexKeys = indexKey.multiEntry();
+    else
+        indexKeys.append(indexKey.asOneKey());
 
-    Vector<IDBKeyData> indexKeys = indexKey.multiEntry();
-
     if (info.unique()) {
         bool hasRecord;
         IDBError error;
         for (auto& indexKey : indexKeys) {
-            error = uncheckedHasIndexRecord(info.identifier(), indexKey, hasRecord);
+            error = uncheckedHasIndexRecord(info, indexKey, hasRecord);
             if (!error.isNull())
                 return error;
             if (hasRecord)
@@ -1065,7 +1034,7 @@
     return { IDBDatabaseException::UnknownError, ASCIILiteral("Currently unable to delete all records in a multi-key range") };
 }
 
-IDBError SQLiteIDBBackingStore::updateIndexesForAddRecord(const IDBObjectStoreInfo& info, const IDBKeyData& key, const ThreadSafeDataBuffer& value)
+IDBError SQLiteIDBBackingStore::updateOneIndexForAddRecord(const IDBIndexInfo& info, const IDBKeyData& key, const ThreadSafeDataBuffer& value)
 {
     JSLockHolder locker(vm());
 
@@ -1073,6 +1042,23 @@
     if (jsValue.isUndefinedOrNull())
         return { };
 
+    IndexKey indexKey;
+    generateIndexKeyForValue(*m_globalObject->globalExec(), info, jsValue, indexKey);
+
+    if (indexKey.isNull())
+        return { };
+
+    return uncheckedPutIndexKey(info, key, indexKey);
+}
+
+IDBError SQLiteIDBBackingStore::updateAllIndexesForAddRecord(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;
 
@@ -1130,7 +1116,7 @@
         }
     }
 
-    auto error = updateIndexesForAddRecord(objectStoreInfo, keyData, value);
+    auto error = updateAllIndexesForAddRecord(objectStoreInfo, keyData, value);
 
     // FIXME: If there was an error indexing this record, remove it.
 

Modified: trunk/Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.h (195764 => 195765)


--- trunk/Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.h	2016-01-28 19:17:17 UTC (rev 195764)
+++ trunk/Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.h	2016-01-28 19:20:25 UTC (rev 195765)
@@ -88,13 +88,15 @@
     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);
 
+    IDBError updateAllIndexesForAddRecord(const IDBObjectStoreInfo&, const IDBKeyData&, const ThreadSafeDataBuffer& value);
+    IDBError updateOneIndexForAddRecord(const IDBIndexInfo&, const IDBKeyData&, const ThreadSafeDataBuffer& value);
+    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(const IDBIndexInfo&, const IDBKeyData&, bool& hasRecord);
+
     JSC::VM& vm();
     JSC::JSGlobalObject& globalObject();
     void initializeVM();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to