IDB: storage/indexeddb/mozilla/cursors.html fails
authorbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 9 Feb 2014 00:00:30 +0000 (00:00 +0000)
committerbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 9 Feb 2014 00:00:30 +0000 (00:00 +0000)
<rdar://problem/16017998> and https://bugs.webkit.org/show_bug.cgi?id=128423

Reviewed by Dan Bernstein.

Source/WebCore:

Tested by storage/indexeddb/mozilla/cursors.html (And probably others)

* Modules/indexeddb/IDBTransactionBackendOperations.cpp:
(WebCore::OpenCursorOperation::perform): Distinguish between an error while opening the cursor
  and opening a cursor that points to no records.

Source/WebKit2:

* DatabaseProcess/IndexedDB/sqlite/SQLiteIDBCursor.cpp:
(WebKit::SQLiteIDBCursor::internalAdvanceOnce): Store the primary key for object store cursors.
(WebKit::SQLiteIDBCursor::iterate): We’re not supposed to check for equality to the target key.
  Depending on the direction of the cursor we should check for the next highest or next lowest key.

LayoutTests:

* platform/mac-wk2/TestExpectations: Unskip storage/indexeddb/mozilla/cursors.html.

Revert the change to storage/indexeddb/mozilla/clear.html as it is now clear how a
null cursor might be returned from openCursor:
* storage/indexeddb/mozilla/clear-expected.txt:
* storage/indexeddb/mozilla/resources/clear.js:

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

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/IDBTransactionBackendOperations.cpp
Source/WebKit2/ChangeLog
Source/WebKit2/DatabaseProcess/IndexedDB/sqlite/SQLiteIDBCursor.cpp

index 19f8e33..b24f8a1 100644 (file)
@@ -1,3 +1,17 @@
+2014-02-08  Brady Eidson  <beidson@apple.com>
+
+        IDB: storage/indexeddb/mozilla/cursors.html fails
+        <rdar://problem/16017998> and https://bugs.webkit.org/show_bug.cgi?id=128423
+
+        Reviewed by Dan Bernstein.
+
+        * platform/mac-wk2/TestExpectations: Unskip storage/indexeddb/mozilla/cursors.html.
+
+        Revert the change to storage/indexeddb/mozilla/clear.html as it is now clear how a 
+        null cursor might be returned from openCursor: 
+        * storage/indexeddb/mozilla/clear-expected.txt:
+        * storage/indexeddb/mozilla/resources/clear.js:
+
 2014-02-08  Chris J. Shull  <chrisjshull@gmail.com>
 
         Web Inspector: Find evaluates attributes in a case sensitive manner
index 467577b..d0b0aee 100644 (file)
@@ -477,7 +477,7 @@ storage/indexeddb/mozilla/clear.html [ Pass ]
 storage/indexeddb/mozilla/create-index-with-integer-keys.html [ Pass ]
 storage/indexeddb/mozilla/cursor-mutation-objectstore-only.html [ Pass ]
 storage/indexeddb/mozilla/cursor-mutation.html [ Pass ]
-
+storage/indexeddb/mozilla/cursors.html [ Pass ]
 
 ### END OF (5) Features that are not supported in WebKit1, so skipped in mac/TestExpectations then re-enabled here
 ########################################
index 4924620..505fe75 100644 (file)
@@ -18,8 +18,7 @@ db.transaction('foo', 'readwrite')
 transaction.objectStore('foo').clear();
 request = db.transaction('foo').objectStore('foo').openCursor();
 cursor = request.result;
-PASS cursor.key is undefined
-PASS cursor.value is undefined
+PASS cursor is null
 PASS successfullyParsed is true
 
 TEST COMPLETE
index 2f09c26..c5a0c62 100644 (file)
@@ -37,28 +37,9 @@ 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 336b70b..7424af1 100644 (file)
@@ -1,3 +1,16 @@
+2014-02-08  Brady Eidson  <beidson@apple.com>
+
+        IDB: storage/indexeddb/mozilla/cursors.html fails
+        <rdar://problem/16017998> and https://bugs.webkit.org/show_bug.cgi?id=128423
+
+        Reviewed by Dan Bernstein.
+
+        Tested by storage/indexeddb/mozilla/cursors.html (And probably others)
+
+        * Modules/indexeddb/IDBTransactionBackendOperations.cpp:
+        (WebCore::OpenCursorOperation::perform): Distinguish between an error while opening the cursor
+          and opening a cursor that points to no records.
+
 2014-02-08  Andreas Kling  <akling@apple.com>
 
         Remove unused ChromeClient::layoutUpdated().
index f58f6ab..81f33c1 100644 (file)
@@ -151,16 +151,15 @@ void OpenCursorOperation::perform(std::function<void()> completionCallback)
     LOG(StorageAPI, "OpenCursorOperation");
 
     RefPtr<OpenCursorOperation> operation(this);
-    auto callback = [this, operation, completionCallback](int64_t cursorID, PassRefPtr<IDBKey> key, PassRefPtr<IDBKey> primaryKey, PassRefPtr<SharedBuffer> valueBuffer, PassRefPtr<IDBDatabaseError>) {
-        // FIXME: When the LevelDB port fails to open a backing store cursor it calls onSuccess(nullptr);
-        // This seems nonsensical and might have to change soon, breaking them.
-        if (!cursorID)
+    auto callback = [this, operation, completionCallback](int64_t cursorID, PassRefPtr<IDBKey> key, PassRefPtr<IDBKey> primaryKey, PassRefPtr<SharedBuffer> valueBuffer, PassRefPtr<IDBDatabaseError> error) {
+        if (error) {
+            m_callbacks->onError(error);
+        } else if (!key) {
+            // If there's no error but also no key, then the cursor had no records.
             m_callbacks->onSuccess(static_cast<SharedBuffer*>(0));
-        else {
+        else {
             RefPtr<IDBCursorBackend> cursor = IDBCursorBackend::create(cursorID, m_cursorType, m_taskType, *m_transaction, m_objectStoreID);
-            if (key || primaryKey || valueBuffer)
-                cursor->updateCursorData(key.get(), primaryKey.get(), valueBuffer.get());
-
+            cursor->updateCursorData(key.get(), primaryKey.get(), valueBuffer.get());
             m_callbacks->onSuccess(cursor.release());
         }
 
index 421b954..1905e72 100644 (file)
@@ -1,3 +1,15 @@
+2014-02-08  Brady Eidson  <beidson@apple.com>
+
+        IDB: storage/indexeddb/mozilla/cursors.html fails
+        <rdar://problem/16017998> and https://bugs.webkit.org/show_bug.cgi?id=128423
+
+        Reviewed by Dan Bernstein.
+
+        * DatabaseProcess/IndexedDB/sqlite/SQLiteIDBCursor.cpp:
+        (WebKit::SQLiteIDBCursor::internalAdvanceOnce): Store the primary key for object store cursors.
+        (WebKit::SQLiteIDBCursor::iterate): We’re not supposed to check for equality to the target key.
+          Depending on the direction of the cursor we should check for the next highest or next lowest key.
+
 2014-02-08  Dan Bernstein  <mitz@apple.com>
 
         Remove client-drawn highlights (-webkit-highlight, WebHTMLHighlighter)
index c7a8a2e..6329181 100644 (file)
@@ -337,7 +337,10 @@ SQLiteIDBCursor::AdvanceResult SQLiteIDBCursor::internalAdvanceOnce()
     m_statement->getColumnBlobAsVector(2, keyData);
     m_currentValueBuffer = keyData;
 
-    if (m_indexID != IDBIndexMetadata::InvalidId) {
+    // The primaryKey of an ObjectStore cursor is the same as its key.
+    if (m_indexID == IDBIndexMetadata::InvalidId)
+        m_currentPrimaryKey = m_currentKey;
+    else {
         if (!deserializeIDBKeyData(keyData.data(), keyData.size(), m_currentPrimaryKey)) {
             LOG_ERROR("Unable to deserialize value data from database while advancing index cursor");
             m_completed = true;
@@ -387,10 +390,18 @@ bool SQLiteIDBCursor::iterate(const WebCore::IDBKeyData& targetKey)
     if (targetKey.isNull || !result)
         return result;
 
-    while (!m_completed && m_currentKey.compare(targetKey)) {
-        result = advance(1);
+    while (!m_completed) {
         if (!result)
             return false;
+
+        // Search for the next key >= the target if the cursor is a Next cursor, or the next key <= if the cursor is a Previous cursor.
+        if (m_cursorDirection == IndexedDB::CursorDirection::Next || m_cursorDirection == IndexedDB::CursorDirection::NextNoDuplicate) {
+            if (m_currentKey.compare(targetKey) >= 0)
+                break;
+        } else if (m_currentKey.compare(targetKey) <= 0)
+            break;
+
+        result = advance(1);
     }
 
     return result;