Title: [132401] trunk
Revision
132401
Author
jsb...@chromium.org
Date
2012-10-24 14:36:57 -0700 (Wed, 24 Oct 2012)

Log Message

IndexedDB: Cursor property value identities should be preserved
https://bugs.webkit.org/show_bug.cgi?id=100051

Reviewed by Tony Chang.

Source/WebCore:

Some W3C test submissions point out that subsequent accesses to cursor properties should
yield the same value until the cursor advances. We handled this with custom binding code for
IDBCursor.value but not IDBCursor.key or IDBCursor.primaryKey. The custom value code is
being removed in webkit.org/b/100034 in favor of returning ScriptValue and the same fix can
be applied to key/primaryKey.

Test: storage/indexeddb/cursor-properties.html

* Modules/indexeddb/IDBCursor.cpp: Compute/store/serve up ScriptValues instead of IDBKeys
(WebCore::IDBCursor::key):
(WebCore::IDBCursor::primaryKey):
(WebCore::IDBCursor::setValueReady):
* Modules/indexeddb/IDBCursor.h:
(IDBCursor):
(WebCore::IDBCursor::idbPrimaryKey): Expose this for callers that need access to the IDBKey
* Modules/indexeddb/IDBCursor.idl:
* Modules/indexeddb/IDBObjectStore.cpp: ... like this one.
(WebCore):
* Modules/indexeddb/IDBRequest.cpp:
(WebCore::IDBRequest::dispatchEvent): Pass along script context, for the conversion.
* bindings/v8/IDBBindingUtilities.cpp:
(WebCore::idbKeyToScriptValue): New method for explicit conversion.
(WebCore):
* bindings/v8/IDBBindingUtilities.h:
(WebCore):

LayoutTests:

Add tests to verify identity/read-only/mutability of cursor properties.

* storage/indexeddb/cursor-properties-expected.txt: Added.
* storage/indexeddb/cursor-properties.html: Added.
* storage/indexeddb/resources/cursor-properties.js: Added.
(test.request.onsuccess):
(test):
(onUpgradeNeeded):
(onOpenSuccess.request.onsuccess):
(onOpenSuccess):
(checkProperty):

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (132400 => 132401)


--- trunk/LayoutTests/ChangeLog	2012-10-24 21:34:16 UTC (rev 132400)
+++ trunk/LayoutTests/ChangeLog	2012-10-24 21:36:57 UTC (rev 132401)
@@ -1,3 +1,22 @@
+2012-10-24  Joshua Bell  <jsb...@chromium.org>
+
+        IndexedDB: Cursor property value identities should be preserved
+        https://bugs.webkit.org/show_bug.cgi?id=100051
+
+        Reviewed by Tony Chang.
+
+        Add tests to verify identity/read-only/mutability of cursor properties.
+
+        * storage/indexeddb/cursor-properties-expected.txt: Added.
+        * storage/indexeddb/cursor-properties.html: Added.
+        * storage/indexeddb/resources/cursor-properties.js: Added.
+        (test.request.onsuccess):
+        (test):
+        (onUpgradeNeeded):
+        (onOpenSuccess.request.onsuccess):
+        (onOpenSuccess):
+        (checkProperty):
+
 2012-10-24  Levi Weintraub  <le...@chromium.org>
 
         Unreviewed gardening. Updating slow expectations for two tests following a Chromium

Added: trunk/LayoutTests/storage/indexeddb/cursor-properties-expected.txt (0 => 132401)


--- trunk/LayoutTests/storage/indexeddb/cursor-properties-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/storage/indexeddb/cursor-properties-expected.txt	2012-10-24 21:36:57 UTC (rev 132401)
@@ -0,0 +1,64 @@
+Test that IndexedDB's cursor key/primaryKey/value properties preserve mutations.
+
+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 = "cursor-properties.html"
+indexedDB.deleteDatabase(dbname)
+indexedDB.open(dbname, 1)
+
+onUpgradeNeeded():
+db = event.target.result
+store = db.createObjectStore('store')
+index = store.createIndex('index', 'id')
+store.put({id: ['indexKey']}, ['primaryKey'])
+
+onOpenSuccess():
+db = event.target.result
+trans = db.transaction('store')
+store = trans.objectStore('store')
+index = store.index('index')
+
+request = index.openCursor()
+
+onCursorSuccess():
+cursor = event.target.result
+PASS cursor is non-null.
+PASS areArraysEqual(cursor.key, ['indexKey']) is true
+PASS areArraysEqual(cursor.primaryKey, ['primaryKey']) is true
+
+Check identity:
+v = cursor.key
+PASS v === cursor.key is true
+Check read-only:
+cursor.key = null
+PASS v === cursor.key is true
+Check mutability:
+cursor.key.expando = 123
+PASS cursor.key.expando is 123
+
+Check identity:
+v = cursor.primaryKey
+PASS v === cursor.primaryKey is true
+Check read-only:
+cursor.primaryKey = null
+PASS v === cursor.primaryKey is true
+Check mutability:
+cursor.primaryKey.expando = 123
+PASS cursor.primaryKey.expando is 123
+
+Check identity:
+v = cursor.value
+PASS v === cursor.value is true
+Check read-only:
+cursor.value = null
+PASS v === cursor.value is true
+Check mutability:
+cursor.value.expando = 123
+PASS cursor.value.expando is 123
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/storage/indexeddb/cursor-properties.html (0 => 132401)


--- trunk/LayoutTests/storage/indexeddb/cursor-properties.html	                        (rev 0)
+++ trunk/LayoutTests/storage/indexeddb/cursor-properties.html	2012-10-24 21:36:57 UTC (rev 132401)
@@ -0,0 +1,10 @@
+<html>
+<head>
+<script src=""
+<script src=""
+</head>
+<body>
+<script src=""
+<script src=""
+</body>
+</html>

Added: trunk/LayoutTests/storage/indexeddb/resources/cursor-properties.js (0 => 132401)


--- trunk/LayoutTests/storage/indexeddb/resources/cursor-properties.js	                        (rev 0)
+++ trunk/LayoutTests/storage/indexeddb/resources/cursor-properties.js	2012-10-24 21:36:57 UTC (rev 132401)
@@ -0,0 +1,77 @@
+if (this.importScripts) {
+    importScripts('../../../fast/js/resources/js-test-pre.js');
+    importScripts('shared.js');
+}
+
+description("Test that IndexedDB's cursor key/primaryKey/value properties preserve mutations.");
+
+function test()
+{
+    removeVendorPrefixes();
+    setDBNameFromPath();
+
+    request = evalAndLog("indexedDB.deleteDatabase(dbname)");
+    request._onblocked_ = unexpectedBlockedCallback;
+    request._onerror_ = unexpectedErrorCallback;
+    request._onsuccess_ = function() {
+        request = evalAndLog("indexedDB.open(dbname, 1)");
+        request._onblocked_ = unexpectedBlockedCallback;
+        request._onerror_ = unexpectedErrorCallback;
+        request._onupgradeneeded_ = onUpgradeNeeded;
+        request._onsuccess_ = onOpenSuccess;
+    };
+}
+
+function onUpgradeNeeded(evt)
+{
+    preamble(evt);
+    evalAndLog("db = event.target.result");
+    evalAndLog("store = db.createObjectStore('store')");
+    evalAndLog("index = store.createIndex('index', 'id')");
+    evalAndLog("store.put({id: ['indexKey']}, ['primaryKey'])");
+}
+
+function onOpenSuccess(evt)
+{
+    preamble(evt);
+    evalAndLog("db = event.target.result");
+    evalAndLog("trans = db.transaction('store')");
+    trans._onabort_ = unexpectedAbortCallback;
+    evalAndLog("store = trans.objectStore('store')");
+    evalAndLog("index = store.index('index')");
+
+    debug("");
+    evalAndLog("request = index.openCursor()");
+    request._onerror_ = unexpectedErrorCallback;
+    request._onsuccess_ = function onCursorSuccess(evt) {
+        preamble(evt);
+        evalAndLog("cursor = event.target.result");
+        shouldBeNonNull("cursor");
+        shouldBeTrue("areArraysEqual(cursor.key, ['indexKey'])");
+        shouldBeTrue("areArraysEqual(cursor.primaryKey, ['primaryKey'])");
+        checkProperty("cursor.key");
+        checkProperty("cursor.primaryKey");
+        checkProperty("cursor.value");
+    };
+
+    trans._oncomplete_ = finishJSTest;
+}
+
+function checkProperty(property)
+{
+    debug("");
+
+    debug("Check identity:");
+    evalAndLog("v = " + property);
+    shouldBeTrue("v === " + property);
+
+    debug("Check read-only:");
+    evalAndLog(property + " = null");
+    shouldBeTrue("v === " + property);
+
+    debug("Check mutability:");
+    evalAndLog(property + ".expando = 123");
+    shouldBe(property + ".expando", "123");
+}
+
+test();

Modified: trunk/Source/WebCore/ChangeLog (132400 => 132401)


--- trunk/Source/WebCore/ChangeLog	2012-10-24 21:34:16 UTC (rev 132400)
+++ trunk/Source/WebCore/ChangeLog	2012-10-24 21:36:57 UTC (rev 132401)
@@ -1,3 +1,36 @@
+2012-10-24  Joshua Bell  <jsb...@chromium.org>
+
+        IndexedDB: Cursor property value identities should be preserved
+        https://bugs.webkit.org/show_bug.cgi?id=100051
+
+        Reviewed by Tony Chang.
+
+        Some W3C test submissions point out that subsequent accesses to cursor properties should
+        yield the same value until the cursor advances. We handled this with custom binding code for
+        IDBCursor.value but not IDBCursor.key or IDBCursor.primaryKey. The custom value code is
+        being removed in webkit.org/b/100034 in favor of returning ScriptValue and the same fix can
+        be applied to key/primaryKey.
+
+        Test: storage/indexeddb/cursor-properties.html
+
+        * Modules/indexeddb/IDBCursor.cpp: Compute/store/serve up ScriptValues instead of IDBKeys
+        (WebCore::IDBCursor::key):
+        (WebCore::IDBCursor::primaryKey):
+        (WebCore::IDBCursor::setValueReady):
+        * Modules/indexeddb/IDBCursor.h:
+        (IDBCursor):
+        (WebCore::IDBCursor::idbPrimaryKey): Expose this for callers that need access to the IDBKey
+        * Modules/indexeddb/IDBCursor.idl:
+        * Modules/indexeddb/IDBObjectStore.cpp: ... like this one.
+        (WebCore):
+        * Modules/indexeddb/IDBRequest.cpp:
+        (WebCore::IDBRequest::dispatchEvent): Pass along script context, for the conversion.
+        * bindings/v8/IDBBindingUtilities.cpp:
+        (WebCore::idbKeyToScriptValue): New method for explicit conversion.
+        (WebCore):
+        * bindings/v8/IDBBindingUtilities.h:
+        (WebCore):
+
 2012-10-24  Ami Fischman  <fisch...@chromium.org>
 
         call to setNeedsLayout during RenderVideo::paintReplaced

Modified: trunk/Source/WebCore/Modules/indexeddb/IDBCursor.cpp (132400 => 132401)


--- trunk/Source/WebCore/Modules/indexeddb/IDBCursor.cpp	2012-10-24 21:34:16 UTC (rev 132400)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBCursor.cpp	2012-10-24 21:36:57 UTC (rev 132401)
@@ -101,16 +101,16 @@
     return direction;
 }
 
-PassRefPtr<IDBKey> IDBCursor::key() const
+const ScriptValue& IDBCursor::key() const
 {
     IDB_TRACE("IDBCursor::key");
-    return m_currentKey;
+    return m_currentKeyValue;
 }
 
-PassRefPtr<IDBKey> IDBCursor::primaryKey() const
+const ScriptValue& IDBCursor::primaryKey() const
 {
     IDB_TRACE("IDBCursor::primaryKey");
-    return m_currentPrimaryKey;
+    return m_currentPrimaryKeyValue;
 }
 
 const ScriptValue& IDBCursor::value() const
@@ -257,10 +257,13 @@
     }
 }
 
-void IDBCursor::setValueReady(PassRefPtr<IDBKey> key, PassRefPtr<IDBKey> primaryKey, ScriptValue& value)
+void IDBCursor::setValueReady(ScriptExecutionContext* context, PassRefPtr<IDBKey> key, PassRefPtr<IDBKey> primaryKey, ScriptValue& value)
 {
     m_currentKey = key;
+    m_currentKeyValue = idbKeyToScriptValue(context, m_currentKey);
+
     m_currentPrimaryKey = primaryKey;
+    m_currentPrimaryKeyValue = idbKeyToScriptValue(context, m_currentPrimaryKey);
 
     if (!isKeyCursor()) {
         RefPtr<IDBObjectStore> objectStore = effectiveObjectStore();

Modified: trunk/Source/WebCore/Modules/indexeddb/IDBCursor.h (132400 => 132401)


--- trunk/Source/WebCore/Modules/indexeddb/IDBCursor.h	2012-10-24 21:34:16 UTC (rev 132400)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBCursor.h	2012-10-24 21:36:57 UTC (rev 132401)
@@ -70,8 +70,8 @@
 
     // Implement the IDL
     const String& direction() const;
-    PassRefPtr<IDBKey> key() const;
-    PassRefPtr<IDBKey> primaryKey() const;
+    const ScriptValue& key() const;
+    const ScriptValue& primaryKey() const;
     const ScriptValue& value() const;
     IDBAny* source() const;
 
@@ -83,7 +83,8 @@
 
     void postSuccessHandlerCallback();
     void close();
-    void setValueReady(PassRefPtr<IDBKey>, PassRefPtr<IDBKey> primaryKey, ScriptValue&);
+    void setValueReady(ScriptExecutionContext*, PassRefPtr<IDBKey>, PassRefPtr<IDBKey> primaryKey, ScriptValue&);
+    PassRefPtr<IDBKey> idbPrimaryKey() { return m_currentPrimaryKey; }
 
 protected:
     IDBCursor(PassRefPtr<IDBCursorBackendInterface>, Direction, IDBRequest*, IDBAny* source, IDBTransaction*);
@@ -101,6 +102,8 @@
     bool m_gotValue;
     // These values are held because m_backend may advance while they
     // are still valid for the current success handlers.
+    ScriptValue m_currentKeyValue;
+    ScriptValue m_currentPrimaryKeyValue;
     RefPtr<IDBKey> m_currentKey;
     RefPtr<IDBKey> m_currentPrimaryKey;
     ScriptValue m_currentValue;

Modified: trunk/Source/WebCore/Modules/indexeddb/IDBCursor.idl (132400 => 132401)


--- trunk/Source/WebCore/Modules/indexeddb/IDBCursor.idl	2012-10-24 21:34:16 UTC (rev 132400)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBCursor.idl	2012-10-24 21:36:57 UTC (rev 132401)
@@ -33,8 +33,8 @@
     const unsigned short PREV_NO_DUPLICATE = 3;
 
     readonly attribute DOMString direction;
-    readonly attribute IDBKey key;
-    readonly attribute IDBKey primaryKey;
+    readonly attribute any key;
+    readonly attribute any primaryKey;
     readonly attribute IDBAny source;
 
     [CallWith=ScriptExecutionContext] IDBRequest update(in any value)

Modified: trunk/Source/WebCore/Modules/indexeddb/IDBObjectStore.cpp (132400 => 132401)


--- trunk/Source/WebCore/Modules/indexeddb/IDBObjectStore.cpp	2012-10-24 21:34:16 UTC (rev 132400)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBObjectStore.cpp	2012-10-24 21:36:57 UTC (rev 132401)
@@ -325,7 +325,7 @@
             cursor->continueFunction(ec);
             ASSERT(!ec);
 
-            RefPtr<IDBKey> primaryKey = cursor->primaryKey();
+            RefPtr<IDBKey> primaryKey = cursor->idbPrimaryKey();
             ScriptValue value = cursor->value();
 
             IDBObjectStore::IndexKeys indexKeys;

Modified: trunk/Source/WebCore/Modules/indexeddb/IDBRequest.cpp (132400 => 132401)


--- trunk/Source/WebCore/Modules/indexeddb/IDBRequest.cpp	2012-10-24 21:34:16 UTC (rev 132400)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBRequest.cpp	2012-10-24 21:36:57 UTC (rev 132401)
@@ -472,8 +472,10 @@
     RefPtr<IDBCursor> cursorToNotify;
     if (event->type() == eventNames().successEvent) {
         cursorToNotify = getResultCursor();
-        if (cursorToNotify)
-            cursorToNotify->setValueReady(m_cursorKey.release(), m_cursorPrimaryKey.release(), m_cursorValue);
+        if (cursorToNotify) {
+            cursorToNotify->setValueReady(scriptExecutionContext(), m_cursorKey.release(), m_cursorPrimaryKey.release(), m_cursorValue);
+            m_cursorValue.clear();
+        }
     }
 
     if (event->type() == eventNames().upgradeneededEvent) {

Modified: trunk/Source/WebCore/bindings/v8/IDBBindingUtilities.cpp (132400 => 132401)


--- trunk/Source/WebCore/bindings/v8/IDBBindingUtilities.cpp	2012-10-24 21:34:16 UTC (rev 132400)
+++ trunk/Source/WebCore/bindings/v8/IDBBindingUtilities.cpp	2012-10-24 21:36:57 UTC (rev 132401)
@@ -266,6 +266,15 @@
     return canInjectNthValueOnKeyPath(v8Value, keyPathElements, keyPathElements.size() - 1);
 }
 
+ScriptValue idbKeyToScriptValue(ScriptExecutionContext* scriptContext, PassRefPtr<IDBKey> key)
+{
+    v8::HandleScope handleScope;
+    v8::Context::Scope contextScope(toV8Context(scriptContext, UseCurrentWorld));
+
+    v8::Handle<v8::Value> v8Value(toV8(key.get()));
+    return ScriptValue(v8Value);
+}
+
 } // namespace WebCore
 
 #endif

Modified: trunk/Source/WebCore/bindings/v8/IDBBindingUtilities.h (132400 => 132401)


--- trunk/Source/WebCore/bindings/v8/IDBBindingUtilities.h	2012-10-24 21:34:16 UTC (rev 132400)
+++ trunk/Source/WebCore/bindings/v8/IDBBindingUtilities.h	2012-10-24 21:36:57 UTC (rev 132401)
@@ -44,6 +44,7 @@
 PassRefPtr<IDBKey> createIDBKeyFromScriptValueAndKeyPath(const ScriptValue&, const IDBKeyPath&);
 bool canInjectIDBKeyIntoScriptValue(const ScriptValue&, const IDBKeyPath&);
 ScriptValue deserializeIDBValue(ScriptExecutionContext*, PassRefPtr<SerializedScriptValue>);
+ScriptValue idbKeyToScriptValue(ScriptExecutionContext*, PassRefPtr<IDBKey>);
 
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to