Title: [175378] trunk
Revision
175378
Author
beid...@apple.com
Date
2014-10-30 14:00:58 -0700 (Thu, 30 Oct 2014)

Log Message

IndexedDB is deleting data when a PK is shared amongst two objectStores
rdar://problem/18479306 and https://bugs.webkit.org/show_bug.cgi?id=137154

Reviewed by Jer Noble.

Source/WebKit2:

* DatabaseProcess/IndexedDB/sqlite/UniqueIDBDatabaseBackingStoreSQLite.cpp:
(WebKit::v1RecordsTableSchema): Store away the v1 schema for introspection into the database.
(WebKit::v2RecordsTableSchema): Add utility methods to get the v2 schema with different Table names.
(WebKit::createOrMigrateRecordsTableIfNecessary): Check to see if the Records table exists with
  the correct schema. If it is the v1 schema, then migrate the data to a new v2 table, drop the v1
  table, then slide the new table into place.
(WebKit::UniqueIDBDatabaseBackingStoreSQLite::ensureValidRecordsTable): Make sure the Records table
  exists and is v2, and then make sure the uniqueness index exists.
(WebKit::UniqueIDBDatabaseBackingStoreSQLite::createAndPopulateInitialMetadata): Don’t bother creating
  the Records table here as it will have already been established earlier.
(WebKit::UniqueIDBDatabaseBackingStoreSQLite::getOrEstablishMetadata):
* DatabaseProcess/IndexedDB/sqlite/UniqueIDBDatabaseBackingStoreSQLite.h:

LayoutTests:

* storage/indexeddb/primary-key-unique-to-objectstore-expected.txt: Added.
* storage/indexeddb/primary-key-unique-to-objectstore.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (175377 => 175378)


--- trunk/LayoutTests/ChangeLog	2014-10-30 20:57:57 UTC (rev 175377)
+++ trunk/LayoutTests/ChangeLog	2014-10-30 21:00:58 UTC (rev 175378)
@@ -1,3 +1,13 @@
+2014-10-30  Brady Eidson  <beid...@apple.com>
+
+        IndexedDB is deleting data when a PK is shared amongst two objectStores
+        rdar://problem/18479306 and https://bugs.webkit.org/show_bug.cgi?id=137154
+
+        Reviewed by Jer Noble.
+
+        * storage/indexeddb/primary-key-unique-to-objectstore-expected.txt: Added.
+        * storage/indexeddb/primary-key-unique-to-objectstore.html: Added.
+
 2014-10-30  Jer Noble  <jer.no...@apple.com>
 
         Unreviewed gardening; rebaseline two media/ tests.

Added: trunk/LayoutTests/storage/indexeddb/primary-key-unique-to-objectstore-expected.txt (0 => 175378)


--- trunk/LayoutTests/storage/indexeddb/primary-key-unique-to-objectstore-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/storage/indexeddb/primary-key-unique-to-objectstore-expected.txt	2014-10-30 21:00:58 UTC (rev 175378)
@@ -0,0 +1,10 @@
+About to add person and note
+Successfully added person
+Successfully added note
+Successfully got person
+name
+id
+Successfully got note
+note
+uid
+

Added: trunk/LayoutTests/storage/indexeddb/primary-key-unique-to-objectstore.html (0 => 175378)


--- trunk/LayoutTests/storage/indexeddb/primary-key-unique-to-objectstore.html	                        (rev 0)
+++ trunk/LayoutTests/storage/indexeddb/primary-key-unique-to-objectstore.html	2014-10-30 21:00:58 UTC (rev 175378)
@@ -0,0 +1,147 @@
+<body>
+<div id="logger"></div>
+<script>
+if (testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+
+function log(msg) {
+    document.getElementById("logger").innerHTML += msg + "<br>";
+}
+
+var openRequest = indexedDB.open("pkutos", 1);
+
+openRequest._onupgradeneeded_ = function(e) {
+    var thisDB = e.target.result;
+
+    if(!thisDB.objectStoreNames.contains("people")) {
+        thisDB.createObjectStore("people", {keyPath:"id"});
+    }
+
+    if(!thisDB.objectStoreNames.contains("notes")) {
+        thisDB.createObjectStore("notes", {keyPath:"uid"});
+    }
+}
+
+openRequest._onsuccess_ = function(e) {
+    db = e.target.result;
+    addPerson();
+}    
+
+openRequest._onerror_ = function(e) {
+    log("openRequest error - " + e);
+}
+
+var sharedID = 0;
+
+function addPerson() {
+    log("About to add person and note");
+
+    //Get a transaction
+    //default for OS list is all, default for type is read
+    var transaction = db.transaction(["people"], "readwrite");
+
+    //Ask for the objectStore
+    var store = transaction.objectStore("people");
+
+    //Define a person
+    var person = {
+        name: "Person",
+        id: sharedID
+    }
+
+    //Perform the add
+    var request = store.add(person);
+
+    request._onerror_ = function(e) {
+        log("Error adding person - ", e);
+    }
+
+    request._onsuccess_ = function(e) {
+        log("Successfully added person");
+        setTimeout("addComplete();", 0);
+    }
+    
+    //Define a note
+    var note = {
+        note: "Note",
+        uid: sharedID
+    }
+
+    var transaction2 = db.transaction(["notes"], "readwrite");
+
+    //Ask for the objectStore
+    var store2 = transaction2.objectStore("notes");
+
+    //Perform the add
+    var request2 = store2.add(note);
+
+    request2._onerror_ = function(e) {
+        log("Error adding note - ", e);
+    }
+
+    request2._onsuccess_ = function(e) {
+        log("Successfully added note");
+        setTimeout("addComplete();", 0);
+    }
+    
+}
+
+var completedAdds = 0;
+function addComplete()
+{
+    if (++completedAdds < 2)
+        return;
+
+    //Get a transaction
+    var transaction = db.transaction(["people"], "readwrite");
+
+    //Ask for the objectStore
+    var store = transaction.objectStore("people");
+
+    //Perform the add
+    var request = store.get(sharedID);
+
+    request._onerror_ = function(e) {
+        log("Error getting person - ", e);
+    }
+
+    request._onsuccess_ = function(e) {
+        log("Successfully got person");
+        for (n in e.target.result) {
+            log(n);
+        }
+        getComplete();
+    }
+
+    var transaction2 = db.transaction(["notes"], "readwrite");
+
+    //Ask for the objectStore
+    var store2 = transaction2.objectStore("notes");
+
+    //Perform the add
+    var request2 = store2.get(sharedID);
+
+    request2._onerror_ = function(e) {
+        log("Error getting note - ", e);
+    }
+
+    request2._onsuccess_ = function(e) {
+        log("Successfully got note");
+        for (n in e.target.result) {
+            log(n);
+        }
+        getComplete();
+    }
+}
+
+var completedGets = 0;
+function getComplete()
+{
+    if (++completedGets == 2 && testRunner)
+        testRunner.notifyDone();
+}
+</script>
+</body>
+</html>
\ No newline at end of file

Modified: trunk/Source/WebKit2/ChangeLog (175377 => 175378)


--- trunk/Source/WebKit2/ChangeLog	2014-10-30 20:57:57 UTC (rev 175377)
+++ trunk/Source/WebKit2/ChangeLog	2014-10-30 21:00:58 UTC (rev 175378)
@@ -1,3 +1,23 @@
+2014-10-30  Brady Eidson  <beid...@apple.com>
+
+        IndexedDB is deleting data when a PK is shared amongst two objectStores
+        rdar://problem/18479306 and https://bugs.webkit.org/show_bug.cgi?id=137154
+
+        Reviewed by Jer Noble.
+
+        * DatabaseProcess/IndexedDB/sqlite/UniqueIDBDatabaseBackingStoreSQLite.cpp:
+        (WebKit::v1RecordsTableSchema): Store away the v1 schema for introspection into the database.
+        (WebKit::v2RecordsTableSchema): Add utility methods to get the v2 schema with different Table names.
+        (WebKit::createOrMigrateRecordsTableIfNecessary): Check to see if the Records table exists with
+          the correct schema. If it is the v1 schema, then migrate the data to a new v2 table, drop the v1
+          table, then slide the new table into place.
+        (WebKit::UniqueIDBDatabaseBackingStoreSQLite::ensureValidRecordsTable): Make sure the Records table
+          exists and is v2, and then make sure the uniqueness index exists.
+        (WebKit::UniqueIDBDatabaseBackingStoreSQLite::createAndPopulateInitialMetadata): Don’t bother creating
+          the Records table here as it will have already been established earlier.
+        (WebKit::UniqueIDBDatabaseBackingStoreSQLite::getOrEstablishMetadata):
+        * DatabaseProcess/IndexedDB/sqlite/UniqueIDBDatabaseBackingStoreSQLite.h:
+
 2014-10-30  Beth Dakin  <bda...@apple.com>
 
         Implement action menus for text

Modified: trunk/Source/WebKit2/DatabaseProcess/IndexedDB/sqlite/UniqueIDBDatabaseBackingStoreSQLite.cpp (175377 => 175378)


--- trunk/Source/WebKit2/DatabaseProcess/IndexedDB/sqlite/UniqueIDBDatabaseBackingStoreSQLite.cpp	2014-10-30 20:57:57 UTC (rev 175377)
+++ trunk/Source/WebKit2/DatabaseProcess/IndexedDB/sqlite/UniqueIDBDatabaseBackingStoreSQLite.cpp	2014-10-30 21:00:58 UTC (rev 175378)
@@ -41,7 +41,9 @@
 #include <WebCore/IDBKeyRange.h>
 #include <WebCore/SQLiteDatabase.h>
 #include <WebCore/SQLiteStatement.h>
+#include <WebCore/SQLiteTransaction.h>
 #include <WebCore/SharedBuffer.h>
+#include <wtf/NeverDestroyed.h>
 #include <wtf/RunLoop.h>
 
 using namespace JSC;
@@ -52,6 +54,29 @@
 // Current version of the metadata schema being used in the metadata database.
 static const int currentMetadataVersion = 1;
 
+static const String& v1RecordsTableSchema()
+{
+    static NeverDestroyed<WTF::String> v1RecordsTableSchemaString(ASCIILiteral(
+        "CREATE TABLE Records (objectStoreID INTEGER NOT NULL ON CONFLICT FAIL, key TEXT COLLATE IDBKEY NOT NULL ON CONFLICT FAIL UNIQUE ON CONFLICT REPLACE, value NOT NULL ON CONFLICT FAIL)"));
+    return v1RecordsTableSchemaString;
+}
+
+static const String v2RecordsTableSchema(const String& tableName)
+{
+    StringBuilder builder;
+    builder.append("CREATE TABLE ");
+    builder.append(tableName);
+    builder.append(" (objectStoreID INTEGER NOT NULL ON CONFLICT FAIL, key TEXT COLLATE IDBKEY NOT NULL ON CONFLICT FAIL, value NOT NULL ON CONFLICT FAIL)");
+
+    return builder.toString();
+}
+
+static const String& v2RecordsTableSchema()
+{
+    static NeverDestroyed<WTF::String> v2RecordsTableSchemaString(v2RecordsTableSchema("Records"));
+    return v2RecordsTableSchemaString;
+}
+
 static int64_t generateDatabaseId()
 {
     static int64_t databaseID = 0;
@@ -82,6 +107,97 @@
     }
 }
 
+static bool createOrMigrateRecordsTableIfNecessary(SQLiteDatabase& database)
+{
+    String currentSchema;
+    {
+        // Fetch the schema for an existing records table.
+        SQLiteStatement statement(database, "SELECT type, sql FROM sqlite_master WHERE tbl_name='Records'");
+        if (statement.prepare() != SQLResultOk) {
+            LOG_ERROR("Unable to prepare statement to fetch schema for the Records table.");
+            return false;
+        }
+
+        int sqliteResult = statement.step();
+
+        // If there is no Records table at all, create it and then bail.
+        if (sqliteResult == SQLResultDone) {
+            if (!database.executeCommand(v2RecordsTableSchema())) {
+                LOG_ERROR("Could not create Records table in database (%i) - %s", database.lastError(), database.lastErrorMsg());
+                return false;
+            }
+
+            return true;
+        }
+
+        if (sqliteResult != SQLResultRow) {
+            LOG_ERROR("Error executing statement to fetch schema for the Records table.");
+            return false;
+        }
+
+        currentSchema = statement.getColumnText(1);
+    }
+
+    ASSERT(!currentSchema.isEmpty());
+
+    // If the schema in the backing store is the current schema, we're done.
+    if (currentSchema == v2RecordsTableSchema())
+        return true;
+
+    // Currently the Records table should only be one of either the v1 or v2 schemas.
+    if (currentSchema != v1RecordsTableSchema()) {
+        ASSERT_NOT_REACHED();
+        return false;
+    }
+
+    SQLiteTransaction transaction(database);
+    transaction.begin();
+
+    // Create a temporary table with the correct schema and migrate all existing content over.
+    if (!database.executeCommand(v2RecordsTableSchema("_Temp_Records"))) {
+        LOG_ERROR("Could not create temporary records table in database (%i) - %s", database.lastError(), database.lastErrorMsg());
+        return false;
+    }
+
+    if (!database.executeCommand("INSERT INTO _Temp_Records SELECT * FROM Records")) {
+        LOG_ERROR("Could not migrate existing Records content (%i) - %s", database.lastError(), database.lastErrorMsg());
+        return false;
+    }
+
+    if (!database.executeCommand("DROP TABLE Records")) {
+        LOG_ERROR("Could not drop existing Records table (%i) - %s", database.lastError(), database.lastErrorMsg());
+        return false;
+    }
+
+    if (!database.executeCommand("ALTER TABLE _Temp_Records RENAME TO Records")) {
+        LOG_ERROR("Could not rename temporary Records table (%i) - %s", database.lastError(), database.lastErrorMsg());
+        return false;
+    }
+
+    transaction.commit();
+
+    return true;
+}
+
+bool UniqueIDBDatabaseBackingStoreSQLite::ensureValidRecordsTable()
+{
+    ASSERT(!RunLoop::isMain());
+    ASSERT(m_sqliteDB);
+    ASSERT(m_sqliteDB->isOpen());
+
+    if (!createOrMigrateRecordsTableIfNecessary(*m_sqliteDB))
+        return false;
+
+    // Whether the updated records table already existed or if it was just created and the data migrated over,
+    // make sure the uniqueness index exists.
+    if (!m_sqliteDB->executeCommand("CREATE UNIQUE INDEX IF NOT EXISTS RecordsIndex ON Records (objectStoreID, key);")) {
+        LOG_ERROR("Could not create RecordsIndex on Records table in database (%i) - %s", m_sqliteDB->lastError(), m_sqliteDB->lastErrorMsg());
+        return false;
+    }
+
+    return true;
+}
+
 std::unique_ptr<IDBDatabaseMetadata> UniqueIDBDatabaseBackingStoreSQLite::createAndPopulateInitialMetadata()
 {
     ASSERT(!RunLoop::isMain());
@@ -106,12 +222,6 @@
         return nullptr;
     }
 
-    if (!m_sqliteDB->executeCommand("CREATE TABLE Records (objectStoreID INTEGER NOT NULL ON CONFLICT FAIL, key TEXT COLLATE IDBKEY NOT NULL ON CONFLICT FAIL UNIQUE ON CONFLICT REPLACE, value NOT NULL ON CONFLICT FAIL);")) {
-        LOG_ERROR("Could not create Records table in database (%i) - %s", m_sqliteDB->lastError(), m_sqliteDB->lastErrorMsg());
-        m_sqliteDB = nullptr;
-        return nullptr;
-    }
-
     if (!m_sqliteDB->executeCommand("CREATE TABLE IndexRecords (indexID INTEGER NOT NULL ON CONFLICT FAIL, objectStoreID INTEGER NOT NULL ON CONFLICT FAIL, key TEXT COLLATE IDBKEY NOT NULL ON CONFLICT FAIL, value NOT NULL ON CONFLICT FAIL);")) {
         LOG_ERROR("Could not create IndexRecords table in database (%i) - %s", m_sqliteDB->lastError(), m_sqliteDB->lastErrorMsg());
         m_sqliteDB = nullptr;
@@ -317,6 +427,12 @@
         return idbKeyCollate(aLength, a, bLength, b);
     });
 
+    if (!ensureValidRecordsTable()) {
+        LOG_ERROR("Error creating or migrating Records table in database");
+        m_sqliteDB = nullptr;
+        return nullptr;
+    }
+
     std::unique_ptr<IDBDatabaseMetadata> metadata = extractExistingMetadata();
     if (!metadata)
         metadata = createAndPopulateInitialMetadata();

Modified: trunk/Source/WebKit2/DatabaseProcess/IndexedDB/sqlite/UniqueIDBDatabaseBackingStoreSQLite.h (175377 => 175378)


--- trunk/Source/WebKit2/DatabaseProcess/IndexedDB/sqlite/UniqueIDBDatabaseBackingStoreSQLite.h	2014-10-30 20:57:57 UTC (rev 175377)
+++ trunk/Source/WebKit2/DatabaseProcess/IndexedDB/sqlite/UniqueIDBDatabaseBackingStoreSQLite.h	2014-10-30 21:00:58 UTC (rev 175378)
@@ -101,6 +101,8 @@
     std::unique_ptr<WebCore::IDBDatabaseMetadata> extractExistingMetadata();
     std::unique_ptr<WebCore::IDBDatabaseMetadata> createAndPopulateInitialMetadata();
 
+    bool ensureValidRecordsTable();
+
     bool deleteRecord(SQLiteIDBTransaction&, int64_t objectStoreID, const WebCore::IDBKeyData&);
     bool uncheckedPutIndexRecord(int64_t objectStoreID, int64_t indexID, const WebCore::IDBKeyData& keyValue, const WebCore::IDBKeyData& indexKey);
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to