Attachment 155832 Details for Bug 92562 – Patch

[patch] Patch

bug-92562-20120801112550.patch (text/plain), 15.91 KB, created by Joshua Bell on 2012-08-01 10:26:08 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Joshua Bell

Created: 2012-08-01 10:26:08 PDT

Size: 15.91 KB

patch

obsolete

>Subversion Revision: 124329
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index e90d3b1c2613210d3d3db54348836c673ab44e90..e637a5655696aee858e96c6171285f060bfde2c2 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,31 @@
>+2012-08-01  Joshua Bell  <jsbell@chromium.org>
>+
>+        IndexedDB: IDBCursor.continue(key) does not throw for key "behind" cursor
>+        https://bugs.webkit.org/show_bug.cgi?id=92562
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Per spec, IDBCursor.continue(key) should throw for forward cursors
>+        if key <= current key, and for reverse cursors if key >= current key.
>+        Code was present on the back end but the error code was ignored, possibly
>+        broken in a recent refactor, and there were no tests for this case.
>+
>+        Test: storage/indexeddb/cursor-continue-dir.html
>+
>+        * Modules/indexeddb/IDBCursor.cpp:
>+        (WebCore::IDBCursor::advance): Handle async transaction termination.
>+        (WebCore::IDBCursor::continueFunction): Move checks here from backend.
>+        * Modules/indexeddb/IDBCursorBackendImpl.cpp:
>+        (WebCore::IDBCursorBackendImpl::IDBCursorBackendImpl): Don't need direction param.
>+        (WebCore::IDBCursorBackendImpl::continueFunction): Remove checks.
>+        * Modules/indexeddb/IDBCursorBackendImpl.h: Only IDBBackingStore::cursor needs direction now.
>+        (WebCore::IDBCursorBackendImpl::create):
>+        (IDBCursorBackendImpl):
>+        * Modules/indexeddb/IDBIndexBackendImpl.cpp:
>+        (WebCore::IDBIndexBackendImpl::openCursorInternal): Drop unused param.
>+        * Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:
>+        (WebCore::IDBObjectStoreBackendImpl::openCursorInternal): Ditto.
>+
> 2012-08-01  Keishi Hattori  <keishi@webkit.org>
> 
>         Attempt to fix flakiness of color-suggestion-picker-appearance.html
>diff --git a/Source/WebCore/Modules/indexeddb/IDBCursor.cpp b/Source/WebCore/Modules/indexeddb/IDBCursor.cpp
>index 05ddb251e4fa2c1796eb5e23a887df28e7a47da6..59a59ad317f7cbc6ee4edec6c15c9dea6d59a351 100644
>--- a/Source/WebCore/Modules/indexeddb/IDBCursor.cpp
>+++ b/Source/WebCore/Modules/indexeddb/IDBCursor.cpp
>@@ -183,6 +183,8 @@ void IDBCursor::advance(unsigned long count, ExceptionCode& ec)
>     m_request->setPendingCursor(this);
>     m_gotValue = false;
>     m_backend->advance(count, m_request, ec);
>+    if (ec)
>+        m_request->markEarlyDeath();
> }
> 
> void IDBCursor::continueFunction(PassRefPtr<IDBKey> key, ExceptionCode& ec)
>@@ -203,11 +205,28 @@ void IDBCursor::continueFunction(PassRefPtr<IDBKey> key, ExceptionCode& ec)
>         return;
>     }
> 
>+    if (key) {
>+        ASSERT(m_currentKey);
>+        if (m_direction == IDBCursor::NEXT || m_direction == IDBCursor::NEXT_NO_DUPLICATE) {
>+            if (!m_currentKey->isLessThan(key.get())) {
>+                ec = IDBDatabaseException::DATA_ERR;
>+                return;
>+            }
>+        } else {
>+            if (!key->isLessThan(m_currentKey.get())) {
>+                ec = IDBDatabaseException::DATA_ERR;
>+                return;
>+            }
>+        }
>+    }
>+
>     // FIXME: We're not using the context from when continue was called, which means the callback
>     //        will be on the original context openCursor was called on. Is this right?
>     m_request->setPendingCursor(this);
>     m_gotValue = false;
>     m_backend->continueFunction(key, m_request, ec);
>+    if (ec)
>+        m_request->markEarlyDeath();
> }
> 
> PassRefPtr<IDBRequest> IDBCursor::deleteFunction(ScriptExecutionContext* context, ExceptionCode& ec)
>diff --git a/Source/WebCore/Modules/indexeddb/IDBCursorBackendImpl.cpp b/Source/WebCore/Modules/indexeddb/IDBCursorBackendImpl.cpp
>index 0a39a8fbc69e0673f73727612b91ecb51ad9a097..9569990cd3d695c59378e3cbbd0b019bea55c7fc 100644
>--- a/Source/WebCore/Modules/indexeddb/IDBCursorBackendImpl.cpp
>+++ b/Source/WebCore/Modules/indexeddb/IDBCursorBackendImpl.cpp
>@@ -42,9 +42,8 @@
> 
> namespace WebCore {
> 
>-IDBCursorBackendImpl::IDBCursorBackendImpl(PassRefPtr<IDBBackingStore::Cursor> cursor, IDBCursor::Direction direction, CursorType cursorType, IDBTransactionBackendImpl* transaction, IDBObjectStoreBackendImpl* objectStore)
>+IDBCursorBackendImpl::IDBCursorBackendImpl(PassRefPtr<IDBBackingStore::Cursor> cursor, CursorType cursorType, IDBTransactionBackendImpl* transaction, IDBObjectStoreBackendImpl* objectStore)
>     : m_cursor(cursor)
>-    , m_direction(direction)
>     , m_cursorType(cursorType)
>     , m_transaction(transaction)
>     , m_objectStore(objectStore)
>@@ -88,21 +87,6 @@ void IDBCursorBackendImpl::continueFunction(PassRefPtr<IDBKey> prpKey, PassRefPt
>     IDB_TRACE("IDBCursorBackendImpl::continue");
>     RefPtr<IDBKey> key = prpKey;
> 
>-    if (m_cursor && key) {
>-        ASSERT(m_cursor->key());
>-        if (m_direction == IDBCursor::NEXT || m_direction == IDBCursor::NEXT_NO_DUPLICATE) {
>-            if (!m_cursor->key()->isLessThan(key.get())) {
>-                ec = IDBDatabaseException::DATA_ERR;
>-                return;
>-            }
>-        } else {
>-            if (!key->isLessThan(m_cursor->key().get())) {
>-                ec = IDBDatabaseException::DATA_ERR;
>-                return;
>-            }
>-        }
>-    }
>-
>     if (!m_transaction->scheduleTask(createCallbackTask(&IDBCursorBackendImpl::continueFunctionInternal, this, key, prpCallbacks)))
>         ec = IDBDatabaseException::TRANSACTION_INACTIVE_ERR;
> }
>diff --git a/Source/WebCore/Modules/indexeddb/IDBCursorBackendImpl.h b/Source/WebCore/Modules/indexeddb/IDBCursorBackendImpl.h
>index 5097c538768f29f09194b29490b1d412de4e1f3b..8c9b2e6eddaff35a9d371b143947126b8a60e2a3 100644
>--- a/Source/WebCore/Modules/indexeddb/IDBCursorBackendImpl.h
>+++ b/Source/WebCore/Modules/indexeddb/IDBCursorBackendImpl.h
>@@ -47,9 +47,9 @@ class SerializedScriptValue;
> 
> class IDBCursorBackendImpl : public IDBCursorBackendInterface {
> public:
>-    static PassRefPtr<IDBCursorBackendImpl> create(PassRefPtr<IDBBackingStore::Cursor> cursor, IDBCursor::Direction direction, CursorType cursorType, IDBTransactionBackendImpl* transaction, IDBObjectStoreBackendImpl* objectStore)
>+    static PassRefPtr<IDBCursorBackendImpl> create(PassRefPtr<IDBBackingStore::Cursor> cursor, CursorType cursorType, IDBTransactionBackendImpl* transaction, IDBObjectStoreBackendImpl* objectStore)
>     {
>-        return adoptRef(new IDBCursorBackendImpl(cursor, direction, cursorType, transaction, objectStore));
>+        return adoptRef(new IDBCursorBackendImpl(cursor, cursorType, transaction, objectStore));
>     }
>     virtual ~IDBCursorBackendImpl();
> 
>@@ -67,7 +67,7 @@ public:
>     void close();
> 
> private:
>-    IDBCursorBackendImpl(PassRefPtr<IDBBackingStore::Cursor>, IDBCursor::Direction, CursorType, IDBTransactionBackendImpl*, IDBObjectStoreBackendImpl*);
>+    IDBCursorBackendImpl(PassRefPtr<IDBBackingStore::Cursor>, CursorType, IDBTransactionBackendImpl*, IDBObjectStoreBackendImpl*);
> 
>     static void advanceInternal(ScriptExecutionContext*, PassRefPtr<IDBCursorBackendImpl>, unsigned long, PassRefPtr<IDBCallbacks>);
>     static void continueFunctionInternal(ScriptExecutionContext*, PassRefPtr<IDBCursorBackendImpl>, PassRefPtr<IDBKey>, PassRefPtr<IDBCallbacks>);
>@@ -75,7 +75,6 @@ private:
> 
>     RefPtr<IDBBackingStore::Cursor> m_cursor;
>     RefPtr<IDBBackingStore::Cursor> m_savedCursor;
>-    IDBCursor::Direction m_direction;
>     CursorType m_cursorType;
>     RefPtr<IDBTransactionBackendImpl> m_transaction;
>     RefPtr<IDBObjectStoreBackendImpl> m_objectStore;
>diff --git a/Source/WebCore/Modules/indexeddb/IDBIndexBackendImpl.cpp b/Source/WebCore/Modules/indexeddb/IDBIndexBackendImpl.cpp
>index 134008491f5a743196e2fc9ab882daee6cb39596..61289a5b8437a15bac1e159c21cb808f509d7a7c 100644
>--- a/Source/WebCore/Modules/indexeddb/IDBIndexBackendImpl.cpp
>+++ b/Source/WebCore/Modules/indexeddb/IDBIndexBackendImpl.cpp
>@@ -99,7 +99,7 @@ void IDBIndexBackendImpl::openCursorInternal(ScriptExecutionContext*, PassRefPtr
>         return;
>     }
> 
>-    RefPtr<IDBCursorBackendInterface> cursor = IDBCursorBackendImpl::create(backingStoreCursor.get(), direction, cursorType, transaction.get(), index->m_objectStoreBackend);
>+    RefPtr<IDBCursorBackendInterface> cursor = IDBCursorBackendImpl::create(backingStoreCursor.get(), cursorType, transaction.get(), index->m_objectStoreBackend);
>     callbacks->onSuccess(cursor.release());
> }
> 
>diff --git a/Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp b/Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp
>index 746933ef04de66400423baec98cd8556cdfba880..ce0915a306f1a73f4bc9f5e29cd70ca6f3a9f275 100644
>--- a/Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp
>+++ b/Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp
>@@ -539,7 +539,7 @@ void IDBObjectStoreBackendImpl::openCursorInternal(ScriptExecutionContext*, Pass
>         return;
>     }
> 
>-    RefPtr<IDBCursorBackendInterface> cursor = IDBCursorBackendImpl::create(backingStoreCursor.release(), direction, IDBCursorBackendInterface::ObjectStoreCursor, transaction.get(), objectStore.get());
>+    RefPtr<IDBCursorBackendInterface> cursor = IDBCursorBackendImpl::create(backingStoreCursor.release(), IDBCursorBackendInterface::ObjectStoreCursor, transaction.get(), objectStore.get());
>     callbacks->onSuccess(cursor.release());
> }
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 65310f51348efbba303a7487178e20233cf94d75..c6366f778eee1237b481e35d788d8c0ced5a6f29 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,25 @@
>+2012-08-01  Joshua Bell  <jsbell@chromium.org>
>+
>+        IndexedDB: IDBCursor.continue(key) does not throw for key "behind" cursor
>+        https://bugs.webkit.org/show_bug.cgi?id=92562
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Ensure cursor.continue(key) throws if key is "behind" the cursor.
>+
>+        * storage/indexeddb/cursor-continue-dir-expected.txt: Added.
>+        * storage/indexeddb/cursor-continue-dir.html: Added.
>+        * storage/indexeddb/resources/cursor-continue-dir.js: Added.
>+        (test.request.onsuccess.request.onsuccess.request.onsuccess):
>+        (test.request.onsuccess.request.onsuccess):
>+        (test.request.onsuccess):
>+        (test):
>+        (testCursors):
>+        (testForwardCursor.request.onsuccess):
>+        (testForwardCursor):
>+        (testReverseCursor.request.onsuccess):
>+        (testReverseCursor):
>+
> 2012-08-01  Keishi Hattori  <keishi@webkit.org>
> 
>         Attempt to fix flakiness of color-suggestion-picker-appearance.html
>diff --git a/LayoutTests/storage/indexeddb/cursor-continue-dir-expected.txt b/LayoutTests/storage/indexeddb/cursor-continue-dir-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..3d909232ebdd5c2e6f1aa0e2fb21309d7430c3e3
>--- /dev/null
>+++ b/LayoutTests/storage/indexeddb/cursor-continue-dir-expected.txt
>@@ -0,0 +1,47 @@
>+Test that continue() calls against cursors are validated by direction.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+indexedDB = self.indexedDB || self.webkitIndexedDB || self.mozIndexedDB || self.msIndexedDB || self.OIndexedDB;
>+
>+dbname = self.location.pathname
>+indexedDB.deleteDatabase(dbname)
>+request = indexedDB.open(dbname)
>+db = request.result
>+db.setVersion('1')
>+store = db.createObjectStore('store')
>+store.put(1,1)
>+store.put(2,2)
>+store.put(3,3)
>+store.put(4,4)
>+store.put(5,5)
>+store.put(6,6)
>+store.put(7,7)
>+store.put(8,8)
>+store.put(9,9)
>+store.put(10,10)
>+trans = db.transaction('store')
>+store = trans.objectStore('store')
>+request = store.openCursor(IDBKeyRange.bound(-Infinity, Infinity), 'next')
>+cursor = request.result
>+PASS cursor is non-null.
>+Expect DataError if: The parameter is less than or equal to this cursor's position and this cursor's direction is "next" or "nextunique".
>+PASS cursor.key is 1
>+Expecting exception from cursor.continue(-1)
>+PASS Exception was thrown.
>+PASS code is IDBDatabaseException.DATA_ERR
>+PASS ename is 'DataError'
>+request = store.openCursor(IDBKeyRange.bound(-Infinity, Infinity), 'prev')
>+cursor = request.result
>+PASS cursor is non-null.
>+Expect DataError if: The parameter is greater than or equal to this cursor's position and this cursor's direction is "prev" or "prevunique".
>+PASS cursor.key is 10
>+Expecting exception from cursor.continue(11)
>+PASS Exception was thrown.
>+PASS code is IDBDatabaseException.DATA_ERR
>+PASS ename is 'DataError'
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/storage/indexeddb/cursor-continue-dir.html b/LayoutTests/storage/indexeddb/cursor-continue-dir.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..af185d37433aa1d76d9434951c5be369cb14af4c
>--- /dev/null
>+++ b/LayoutTests/storage/indexeddb/cursor-continue-dir.html
>@@ -0,0 +1,10 @@
>+<html>
>+<head>
>+<script src="../../fast/js/resources/js-test-pre.js"></script>
>+<script src="resources/shared.js"></script>
>+</head>
>+<body>
>+<script src="resources/cursor-continue-dir.js"></script>
>+<script src="../../fast/js/resources/js-test-post.js"></script>
>+</body>
>+</html>
>diff --git a/LayoutTests/storage/indexeddb/resources/cursor-continue-dir.js b/LayoutTests/storage/indexeddb/resources/cursor-continue-dir.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..bfec0ebc7af74c97b12faa08ba5a2d2a7745b4db
>--- /dev/null
>+++ b/LayoutTests/storage/indexeddb/resources/cursor-continue-dir.js
>@@ -0,0 +1,75 @@
>+if (this.importScripts) {
>+    importScripts('../../../fast/js/resources/js-test-pre.js');
>+    importScripts('shared.js');
>+}
>+
>+description("Test that continue() calls against cursors are validated by direction.");
>+
>+function test()
>+{
>+    removeVendorPrefixes();
>+    evalAndLog("dbname = self.location.pathname");
>+    request = evalAndLog("indexedDB.deleteDatabase(dbname)");
>+    request.onerror = unexpectedErrorCallback;
>+    request.onblocked = unexpectedBlockedCallback;
>+    request.onsuccess = function() {
>+        evalAndLog("request = indexedDB.open(dbname)");
>+        request.onerror = unexpectedErrorCallback;
>+        request.onsuccess = function() {
>+            evalAndLog("db = request.result");
>+            request = evalAndLog("db.setVersion('1')");
>+            request.onerror = unexpectedErrorCallback;
>+            request.onblocked = unexpectedBlockedCallback;
>+            request.onsuccess = function() {
>+                trans = request.result;
>+                trans.onabort = unexpectedAbortCallback;
>+
>+                evalAndLog("store = db.createObjectStore('store')");
>+                for (i = 1; i <= 10; ++i) {
>+                    evalAndLog("store.put(" + i + "," + i + ")");
>+                }
>+
>+                trans.oncomplete = testCursors;
>+            };
>+        };
>+    };
>+}
>+
>+function testCursors()
>+{
>+    evalAndLog("trans = db.transaction('store')");
>+    evalAndLog("store = trans.objectStore('store')");
>+    testForwardCursor();
>+}
>+
>+function testForwardCursor()
>+{
>+    evalAndLog("request = store.openCursor(IDBKeyRange.bound(-Infinity, Infinity), 'next')");
>+    request.onerror = unexpectedErrorCallback;
>+    request.onsuccess = function() {
>+        evalAndLog("cursor = request.result");
>+        shouldBeNonNull("cursor");
>+        debug("Expect DataError if: The parameter is less than or equal to this cursor's position and this cursor's direction is \"next\" or \"nextunique\".");
>+        shouldBe("cursor.key", "1");
>+        evalAndExpectException("cursor.continue(-1)", "IDBDatabaseException.DATA_ERR", "'DataError'");
>+
>+        testReverseCursor();
>+    };
>+}
>+
>+function testReverseCursor()
>+{
>+    evalAndLog("request = store.openCursor(IDBKeyRange.bound(-Infinity, Infinity), 'prev')");
>+    request.onerror = unexpectedErrorCallback;
>+    request.onsuccess = function() {
>+        evalAndLog("cursor = request.result");
>+        shouldBeNonNull("cursor");
>+        debug("Expect DataError if: The parameter is greater than or equal to this cursor's position and this cursor's direction is \"prev\" or \"prevunique\".");
>+        shouldBe("cursor.key", "10");
>+        evalAndExpectException("cursor.continue(11)", "IDBDatabaseException.DATA_ERR", "'DataError'");
>+
>+        finishJSTest();
>+    };
>+}
>+
>+test();

Actions: View | Formatted Diff | Diff

Attachments on bug 92562: 155090 | 155092 | 155832