IDB: storage/indexeddb/mozilla/clear.html fails
authorbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 6 Feb 2014 17:42:12 +0000 (17:42 +0000)
committerbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 6 Feb 2014 17:42:12 +0000 (17:42 +0000)
<rdar://problem/15997155> and https://bugs.webkit.org/show_bug.cgi?id=128282

Reviewed by David Kilzer.

Source/WebCore:

Covered by storage/indexeddb/mozilla/clear.html (and probably others)

Update the value deserializer to take into account whether or not there was an IDBKey:
* bindings/js/IDBBindingUtilities.cpp:
(WebCore::deserializeIDBValueBuffer):
* bindings/js/IDBBindingUtilities.h:

* Modules/indexeddb/IDBRequest.cpp:
(WebCore::IDBRequest::onSuccess): Call the new form of deserializeIDBValueBuffer.

* Modules/indexeddb/IDBDatabaseBackend.cpp:
(WebCore::IDBDatabaseBackend::clearObjectStore): Update logging.

* Modules/indexeddb/IDBTransaction.cpp:
(WebCore::IDBTransaction::setActive): Update logging.

* Modules/indexeddb/IDBTransactionBackend.cpp:
(WebCore::IDBTransactionBackend::commit): Fix ASSERTs to reflect multi-process worlds.

Source/WebKit2:

* DatabaseProcess/IndexedDB/DatabaseProcessIDBConnection.cpp:
(WebKit::DatabaseProcessIDBConnection::openTransaction): Update logging.

* DatabaseProcess/IndexedDB/sqlite/SQLiteIDBTransaction.cpp:
(WebKit::SQLiteIDBTransaction::commit): Update ASSERT.

LayoutTests:

* platform/mac-wk2/TestExpectations: Enable this test.

Update the test for (what I can only assume are) changes in the spec:
* storage/indexeddb/mozilla/clear-expected.txt:
* storage/indexeddb/mozilla/resources/clear.js:
(areWeClearYet):

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

14 files changed:
LayoutTests/ChangeLog
LayoutTests/platform/mac-wk2/TestExpectations
LayoutTests/storage/indexeddb/mozilla/clear-expected.txt
LayoutTests/storage/indexeddb/mozilla/resources/clear.js
Source/WebCore/ChangeLog
Source/WebCore/Modules/indexeddb/IDBDatabaseBackend.cpp
Source/WebCore/Modules/indexeddb/IDBRequest.cpp
Source/WebCore/Modules/indexeddb/IDBTransaction.cpp
Source/WebCore/Modules/indexeddb/IDBTransactionBackend.cpp
Source/WebCore/bindings/js/IDBBindingUtilities.cpp
Source/WebCore/bindings/js/IDBBindingUtilities.h
Source/WebKit2/ChangeLog
Source/WebKit2/DatabaseProcess/IndexedDB/DatabaseProcessIDBConnection.cpp
Source/WebKit2/DatabaseProcess/IndexedDB/sqlite/SQLiteIDBTransaction.cpp

index 131f2e0..afde978 100644 (file)
@@ -1,3 +1,17 @@
+2014-02-06  Brady Eidson  <beidson@apple.com>
+
+        IDB: storage/indexeddb/mozilla/clear.html fails
+        <rdar://problem/15997155> and https://bugs.webkit.org/show_bug.cgi?id=128282
+
+        Reviewed by David Kilzer.
+
+        * platform/mac-wk2/TestExpectations: Enable this test.
+
+        Update the test for (what I can only assume are) changes in the spec:
+        * storage/indexeddb/mozilla/clear-expected.txt:
+        * storage/indexeddb/mozilla/resources/clear.js:
+        (areWeClearYet):
+
 2014-02-06  Michał Pakuła vel Rutka  <m.pakula@samsung.com>
 
         Unreviewed EFL gardening
index 926f635..6b70318 100644 (file)
@@ -471,6 +471,7 @@ fullscreen/anonymous-block-merge-crash.html [ Pass ]
 # Reenable individual tests here that are known to pass, with the eventual goal of re-enabling the entire directory.
 storage/indexeddb/mozilla/add-twice-failure.html [ Pass ]
 storage/indexeddb/mozilla/autoincrement-indexes.html [ Pass ]
+storage/indexeddb/mozilla/clear.html [ Pass ]
 
 
 ### END OF (5) Features that are not supported in WebKit1, so skipped in mac/TestExpectations then re-enabled here
index 505fe75..4924620 100644 (file)
@@ -18,7 +18,8 @@ db.transaction('foo', 'readwrite')
 transaction.objectStore('foo').clear();
 request = db.transaction('foo').objectStore('foo').openCursor();
 cursor = request.result;
-PASS cursor is null
+PASS cursor.key is undefined
+PASS cursor.value is undefined
 PASS successfullyParsed is true
 
 TEST COMPLETE
index c5a0c62..2f09c26 100644 (file)
@@ -37,9 +37,28 @@ function cleared()
     request.onerror = unexpectedErrorCallback;
 }
 
+// The version of this test that existed up until revision ~163500 had the following areWeClearYet handler.
+// The intention was apparently to verify that calling openCursor() on an empty object store returned a null cursor.
+/*
 function areWeClearYet()
 {
     cursor = evalAndLog("cursor = request.result;");
     shouldBe("cursor", "null");
     finishJSTest();
 }
+*/
+
+// The spec's current description of IDBObjectStore.openCursor(), as found here:
+// http://www.w3.org/TR/IndexedDB/#widl-IDBObjectStore-openCursor-IDBRequest-any-range-IDBCursorDirection-direction
+// does not allow for a null cursor to be returned. It states:
+// "...this method creates a cursor. The cursor must implement the IDBCursorWithValue interface."
+// and then gives no allowance for not returning that created cursor.
+// ---
+// So our current reading of the spec is that a cursor must be returned, but it must be pointing to an undefined key/value record.
+function areWeClearYet()
+{
+    cursor = evalAndLog("cursor = request.result;");
+    shouldBe("cursor.key", "undefined");
+    shouldBe("cursor.value", "undefined");
+    finishJSTest();
+}
index 2c4cfb4..a08ac68 100644 (file)
@@ -1,3 +1,29 @@
+2014-02-06  Brady Eidson  <beidson@apple.com>
+
+        IDB: storage/indexeddb/mozilla/clear.html fails
+        <rdar://problem/15997155> and https://bugs.webkit.org/show_bug.cgi?id=128282
+
+        Reviewed by David Kilzer.
+
+        Covered by storage/indexeddb/mozilla/clear.html (and probably others)
+
+        Update the value deserializer to take into account whether or not there was an IDBKey:
+        * bindings/js/IDBBindingUtilities.cpp:
+        (WebCore::deserializeIDBValueBuffer):
+        * bindings/js/IDBBindingUtilities.h:
+
+        * Modules/indexeddb/IDBRequest.cpp:
+        (WebCore::IDBRequest::onSuccess): Call the new form of deserializeIDBValueBuffer.
+
+        * Modules/indexeddb/IDBDatabaseBackend.cpp:
+        (WebCore::IDBDatabaseBackend::clearObjectStore): Update logging.
+
+        * Modules/indexeddb/IDBTransaction.cpp:
+        (WebCore::IDBTransaction::setActive): Update logging.
+
+        * Modules/indexeddb/IDBTransactionBackend.cpp:
+        (WebCore::IDBTransactionBackend::commit): Fix ASSERTs to reflect multi-process worlds.
+
 2014-02-06  Csaba Osztrogonác  <ossy@webkit.org>
 
         Re-enable simple line layout on non-Mac platforms
index 12873ba..b774eb3 100644 (file)
@@ -308,7 +308,7 @@ void IDBDatabaseBackend::deleteRange(int64_t transactionId, int64_t objectStoreI
 
 void IDBDatabaseBackend::clearObjectStore(int64_t transactionId, int64_t objectStoreId, PassRefPtr<IDBCallbacks> callbacks)
 {
-    LOG(StorageAPI, "IDBDatabaseBackend::clear");
+    LOG(StorageAPI, "IDBDatabaseBackend::clearObjectStore %lli in transaction %lli", objectStoreId, transactionId);
     IDBTransactionBackend* transaction = m_transactions.get(transactionId);
     if (!transaction)
         return;
index 5b2ead5..b421125 100644 (file)
@@ -287,7 +287,7 @@ void IDBRequest::onSuccess(PassRefPtr<IDBCursorBackend> prpBackend)
     RefPtr<IDBKey> key = backend->key();
     RefPtr<IDBKey> primaryKey = backend->primaryKey();
 
-    Deprecated::ScriptValue value = deserializeIDBValueBuffer(requestState(), backend->valueBuffer());
+    Deprecated::ScriptValue value = deserializeIDBValueBuffer(requestState(), backend->valueBuffer(), !!key);
 
     ASSERT(!m_pendingCursor);
     RefPtr<IDBCursor> cursor;
@@ -327,7 +327,10 @@ void IDBRequest::onSuccess(PassRefPtr<SharedBuffer> valueBuffer)
         return;
 
     DOMRequestState::Scope scope(m_requestState);
-    Deprecated::ScriptValue value = deserializeIDBValueBuffer(requestState(), valueBuffer);
+
+    // FIXME: By not knowing whether or not the key is defined here, we don't know
+    // if a null valueBuffer means the value is null or the value is undefined.
+    Deprecated::ScriptValue value = deserializeIDBValueBuffer(requestState(), valueBuffer, true);
     onSuccessInternal(value);
 }
 
@@ -354,7 +357,10 @@ void IDBRequest::onSuccess(PassRefPtr<SharedBuffer> valueBuffer, PassRefPtr<IDBK
     ASSERT(keyPath == effectiveObjectStore(m_source)->keyPath());
 #endif
     DOMRequestState::Scope scope(m_requestState);
-    Deprecated::ScriptValue value = deserializeIDBValueBuffer(requestState(), valueBuffer);
+
+    // FIXME: By not knowing whether or not the key is defined here, we don't know
+    // if a null valueBuffer means the value is null or the value is undefined.
+    Deprecated::ScriptValue value = deserializeIDBValueBuffer(requestState(), valueBuffer, true);
 
     RefPtr<IDBKey> primaryKey = prpPrimaryKey;
 #ifndef NDEBUG
@@ -412,7 +418,7 @@ void IDBRequest::onSuccess(PassRefPtr<IDBKey> key, PassRefPtr<IDBKey> primaryKey
 
     DOMRequestState::Scope scope(m_requestState);
 
-    Deprecated::ScriptValue value = deserializeIDBValueBuffer(requestState(), buffer);
+    Deprecated::ScriptValue value = deserializeIDBValueBuffer(requestState(), buffer, !!key);
 
     ASSERT(m_pendingCursor);
     setResultCursor(m_pendingCursor.release(), key, primaryKey, value);
index 5d2f3f8..8d6d53a 100644 (file)
@@ -193,6 +193,7 @@ void IDBTransaction::objectStoreDeleted(const String& name)
 
 void IDBTransaction::setActive(bool active)
 {
+    LOG(StorageAPI, "IDBTransaction::setActive(%s) for transaction id %lli", active ? "true" : "false", m_id);
     ASSERT_WITH_MESSAGE(m_state != Finished, "A finished transaction tried to setActive(%s)", active ? "true" : "false");
     if (m_state == Finishing)
         return;
index 0fd8d73..14201b7 100644 (file)
@@ -207,14 +207,14 @@ void IDBTransactionBackend::start()
 
 void IDBTransactionBackend::commit()
 {
-    LOG(StorageAPI, "IDBTransactionBackend::commit (Transaction %lli)", static_cast<long long>(m_id));
+    LOG(StorageAPI, "IDBTransactionBackend::commit transaction %lli in state %u", static_cast<long long>(m_id), m_state);
 
     // In multiprocess ports, front-end may have requested a commit but an abort has already
     // been initiated asynchronously by the back-end.
     if (m_state == Finished)
         return;
 
-    ASSERT(m_state == Unused || m_state == Running);
+    ASSERT(m_state == Unopened || m_state == Unused || m_state == Running);
     m_commitPending = true;
 
     // Front-end has requested a commit, but there may be tasks like createIndex which
@@ -229,7 +229,7 @@ void IDBTransactionBackend::commit()
     // alive while executing this method.
     RefPtr<IDBTransactionBackend> backend(this);
 
-    bool unused = m_state == Unused;
+    bool unused = m_state == Unused || m_state == Unopened;
     m_state = Finished;
 
     bool committed = unused;
index 7a77db4..c29d368 100644 (file)
@@ -303,10 +303,18 @@ Deprecated::ScriptValue deserializeIDBValue(DOMRequestState* requestState, PassR
     return Deprecated::ScriptValue(exec->vm(), jsNull());
 }
 
-Deprecated::ScriptValue deserializeIDBValueBuffer(DOMRequestState* requestState, PassRefPtr<SharedBuffer> prpBuffer)
+Deprecated::ScriptValue deserializeIDBValueBuffer(DOMRequestState* requestState, PassRefPtr<SharedBuffer> prpBuffer, bool keyIsDefined)
 {
     ExecState* exec = requestState->exec();
     RefPtr<SharedBuffer> buffer = prpBuffer;
+
+    // If the key doesn't exist, then the value must be undefined (as opposed to null).
+    if (!keyIsDefined) {
+        // We either shouldn't have a buffer or it should be of size 0.
+        ASSERT(!buffer || !buffer->size());
+        return Deprecated::ScriptValue(exec->vm(), jsUndefined());
+    }
+
     if (buffer) {
         // FIXME: The extra copy here can be eliminated by allowing SerializedScriptValue to take a raw const char* or const uint8_t*.
         Vector<uint8_t> value;
@@ -314,6 +322,7 @@ Deprecated::ScriptValue deserializeIDBValueBuffer(DOMRequestState* requestState,
         RefPtr<SerializedScriptValue> serializedValue = SerializedScriptValue::createFromWireBytes(value);
         return SerializedScriptValue::deserialize(exec, serializedValue.get(), NonThrowing);
     }
+
     return Deprecated::ScriptValue(exec->vm(), jsNull());
 }
 
index f012aef..8047f87 100644 (file)
@@ -45,7 +45,7 @@ bool injectIDBKeyIntoScriptValue(DOMRequestState*, PassRefPtr<IDBKey>, Deprecate
 PassRefPtr<IDBKey> createIDBKeyFromScriptValueAndKeyPath(DOMRequestState*, const Deprecated::ScriptValue&, const IDBKeyPath&);
 bool canInjectIDBKeyIntoScriptValue(DOMRequestState*, const Deprecated::ScriptValue&, const IDBKeyPath&);
 Deprecated::ScriptValue deserializeIDBValue(DOMRequestState*, PassRefPtr<SerializedScriptValue>);
-Deprecated::ScriptValue deserializeIDBValueBuffer(DOMRequestState*, PassRefPtr<SharedBuffer>);
+Deprecated::ScriptValue deserializeIDBValueBuffer(DOMRequestState*, PassRefPtr<SharedBuffer>, bool keyIsDefined);
 Deprecated::ScriptValue idbKeyToScriptValue(DOMRequestState*, PassRefPtr<IDBKey>);
 PassRefPtr<IDBKey> scriptValueToIDBKey(DOMRequestState*, const Deprecated::ScriptValue&);
 
index 26f3c6f..d7ead49 100644 (file)
@@ -1,3 +1,16 @@
+2014-02-06  Brady Eidson  <beidson@apple.com>
+
+        IDB: storage/indexeddb/mozilla/clear.html fails
+        <rdar://problem/15997155> and https://bugs.webkit.org/show_bug.cgi?id=128282
+
+        Reviewed by David Kilzer.
+
+        * DatabaseProcess/IndexedDB/DatabaseProcessIDBConnection.cpp:
+        (WebKit::DatabaseProcessIDBConnection::openTransaction): Update logging.
+
+        * DatabaseProcess/IndexedDB/sqlite/SQLiteIDBTransaction.cpp:
+        (WebKit::SQLiteIDBTransaction::commit): Update ASSERT.
+
 2014-02-06  Csaba Osztrogonác  <ossy@webkit.org>
 
         Fix the build after r163516 for !ENABLE(ASYNC_SCROLLING) platforms
index 7d84074..9ff3d75 100644 (file)
@@ -100,7 +100,22 @@ void DatabaseProcessIDBConnection::openTransaction(uint64_t requestID, int64_t t
 {
     ASSERT(m_uniqueIDBDatabase);
 
-    LOG(IDB, "DatabaseProcess openTransaction request ID %llu", requestID);
+#ifndef NDEBUG
+    const char* modeString = nullptr;
+    switch (static_cast<IndexedDB::TransactionMode>(intMode)) {
+    case IndexedDB::TransactionMode::ReadOnly:
+        modeString = "readonly";
+        break;
+    case IndexedDB::TransactionMode::ReadWrite:
+        modeString = "readwrite";
+        break;
+    case IndexedDB::TransactionMode::VersionChange:
+        modeString = "versionchange";
+        break;
+    }
+
+    LOG(IDB, "DatabaseProcess openTransaction id %llu in mode '%s', request ID %llu", transactionID, modeString, requestID);
+#endif
 
     if (intMode > IndexedDB::TransactionModeMaximum) {
         send(Messages::WebIDBServerConnection::DidOpenTransaction(requestID, false));
index 22bcccc..786dcd6 100644 (file)
@@ -67,8 +67,9 @@ bool SQLiteIDBTransaction::begin(SQLiteDatabase& database)
 
 bool SQLiteIDBTransaction::commit()
 {
-    ASSERT(m_sqliteTransaction);
-    if (!m_sqliteTransaction->inProgress())
+    // It's okay to not have a SQLite transaction or not have started it yet because it's okay for a WebProcess
+    // to request the commit of a transaction immediately after creating it before it has even been used.
+    if (!m_sqliteTransaction || !m_sqliteTransaction->inProgress())
         return false;
 
     m_sqliteTransaction->commit();