Attachment 170021 Details for Bug 100051 – Patch

[patch] Patch

bug-100051-20121022164247.patch (text/plain), 14.97 KB, created by Joshua Bell on 2012-10-22 16:44:09 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Joshua Bell

Created: 2012-10-22 16:44:09 PDT

Size: 14.97 KB

patch

obsolete

>Subversion Revision: 132073
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 3438e8b9ddbe9e161f926108fc3141bf4256fd2c..12e3e8390930b158dbf18fe08dc52941f6f6dc23 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,36 @@
>+2012-10-22  Joshua Bell  <jsbell@chromium.org>
>+
>+        IndexedDB: Cursor property value identities should be preserved
>+        https://bugs.webkit.org/show_bug.cgi?id=100051
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        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-22  Erik Arvidsson  <arv@chromium.org>
> 
>         HTMLBaseElement href attribute binding returns wrong URL
>diff --git a/Source/WebCore/Modules/indexeddb/IDBCursor.cpp b/Source/WebCore/Modules/indexeddb/IDBCursor.cpp
>index 990e3a45515139095b55b5dc6ea907ddc228a419..2366ffca236e55381c22ad67537a5091bcf4c3f0 100644
>--- a/Source/WebCore/Modules/indexeddb/IDBCursor.cpp
>+++ b/Source/WebCore/Modules/indexeddb/IDBCursor.cpp
>@@ -101,16 +101,16 @@ const String& IDBCursor::direction() const
>     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;
> }
> 
> PassRefPtr<IDBAny> IDBCursor::value()
>@@ -259,10 +259,13 @@ void IDBCursor::close()
>     }
> }
> 
>-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();
>diff --git a/Source/WebCore/Modules/indexeddb/IDBCursor.h b/Source/WebCore/Modules/indexeddb/IDBCursor.h
>index 5589c222fc57a8b52f6e3f65ebb69c2e7f2225e0..c0e8117d9f171a3f468463b54130bd256b56c064 100644
>--- a/Source/WebCore/Modules/indexeddb/IDBCursor.h
>+++ b/Source/WebCore/Modules/indexeddb/IDBCursor.h
>@@ -70,8 +70,8 @@ public:
> 
>     // Implement the IDL
>     const String& direction() const;
>-    PassRefPtr<IDBKey> key() const;
>-    PassRefPtr<IDBKey> primaryKey() const;
>+    const ScriptValue& key() const;
>+    const ScriptValue& primaryKey() const;
>     PassRefPtr<IDBAny> value();
>     IDBAny* source() const;
> 
>@@ -82,7 +82,8 @@ public:
> 
>     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; }
> 
>     // The spec requires that the script object that wraps the value
>     // be unchanged until the value changes as a result of the cursor
>@@ -105,6 +106,8 @@ private:
>     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;
>     RefPtr<IDBAny> m_currentValue;
>diff --git a/Source/WebCore/Modules/indexeddb/IDBCursor.idl b/Source/WebCore/Modules/indexeddb/IDBCursor.idl
>index b514fe9517abcb1cb689728590ab4856b38e597d..4a8bd3284072fab81b49d93e0b9e1ead803861fe 100644
>--- a/Source/WebCore/Modules/indexeddb/IDBCursor.idl
>+++ b/Source/WebCore/Modules/indexeddb/IDBCursor.idl
>@@ -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)
>diff --git a/Source/WebCore/Modules/indexeddb/IDBObjectStore.cpp b/Source/WebCore/Modules/indexeddb/IDBObjectStore.cpp
>index e3ec15b29daf64417342aea20bf2145d2eb86476..8612244303d927f9dafd89b6bdb716bdb3e5537f 100644
>--- a/Source/WebCore/Modules/indexeddb/IDBObjectStore.cpp
>+++ b/Source/WebCore/Modules/indexeddb/IDBObjectStore.cpp
>@@ -325,7 +325,7 @@ private:
>             cursor->continueFunction(ec);
>             ASSERT(!ec);
> 
>-            RefPtr<IDBKey> primaryKey = cursor->primaryKey();
>+            RefPtr<IDBKey> primaryKey = cursor->idbPrimaryKey();
>             RefPtr<IDBAny> valueAny = cursor->value();
> 
>             ASSERT(valueAny->type() == IDBAny::ScriptValueType);
>diff --git a/Source/WebCore/Modules/indexeddb/IDBRequest.cpp b/Source/WebCore/Modules/indexeddb/IDBRequest.cpp
>index 0b502d742c4bac6cc1234036d3766ecece5a9522..1860bfc59e3ca3842f7565ade1064ce1bf83c3a8 100644
>--- a/Source/WebCore/Modules/indexeddb/IDBRequest.cpp
>+++ b/Source/WebCore/Modules/indexeddb/IDBRequest.cpp
>@@ -472,8 +472,10 @@ bool IDBRequest::dispatchEvent(PassRefPtr<Event> event)
>     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) {
>diff --git a/Source/WebCore/bindings/v8/IDBBindingUtilities.cpp b/Source/WebCore/bindings/v8/IDBBindingUtilities.cpp
>index 6a8311fb604ce5006d1dbe203e9ec5e00feda19c..5becb6a6681fdf32203c761a5b03af4d7ff2841e 100644
>--- a/Source/WebCore/bindings/v8/IDBBindingUtilities.cpp
>+++ b/Source/WebCore/bindings/v8/IDBBindingUtilities.cpp
>@@ -266,6 +266,15 @@ bool canInjectIDBKeyIntoScriptValue(const ScriptValue& scriptValue, const IDBKey
>     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
>diff --git a/Source/WebCore/bindings/v8/IDBBindingUtilities.h b/Source/WebCore/bindings/v8/IDBBindingUtilities.h
>index a482dc9741b29aec8a5e1113cf7fe5f37afaf4a1..f766056df66d2d708ce2bbc2c9fe6a6cf14e5bba 100644
>--- a/Source/WebCore/bindings/v8/IDBBindingUtilities.h
>+++ b/Source/WebCore/bindings/v8/IDBBindingUtilities.h
>@@ -44,6 +44,7 @@ bool injectIDBKeyIntoScriptValue(PassRefPtr<IDBKey>, ScriptValue&, const IDBKeyP
> PassRefPtr<IDBKey> createIDBKeyFromScriptValueAndKeyPath(const ScriptValue&, const IDBKeyPath&);
> bool canInjectIDBKeyIntoScriptValue(const ScriptValue&, const IDBKeyPath&);
> ScriptValue deserializeIDBValue(ScriptExecutionContext*, PassRefPtr<SerializedScriptValue>);
>+ScriptValue idbKeyToScriptValue(ScriptExecutionContext*, PassRefPtr<IDBKey>);
> 
> }
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 1b268c580cd64c5e6fa8348177aefba8533672a4..e40f6857845ed948f79062ca616560e8c6a6f368 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,22 @@
>+2012-10-22  Joshua Bell  <jsbell@chromium.org>
>+
>+        IndexedDB: Cursor property value identities should be preserved
>+        https://bugs.webkit.org/show_bug.cgi?id=100051
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        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-22  Erik Arvidsson  <arv@chromium.org>
> 
>         HTMLBaseElement href attribute binding returns wrong URL
>diff --git a/LayoutTests/storage/indexeddb/cursor-properties-expected.txt b/LayoutTests/storage/indexeddb/cursor-properties-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..736779844e0e74fbfbffdfe81db0a44f98c2227d
>--- /dev/null
>+++ b/LayoutTests/storage/indexeddb/cursor-properties-expected.txt
>@@ -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
>+
>diff --git a/LayoutTests/storage/indexeddb/cursor-properties.html b/LayoutTests/storage/indexeddb/cursor-properties.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..79e4a79e688027cfded16ba59d72c94950c2e720
>--- /dev/null
>+++ b/LayoutTests/storage/indexeddb/cursor-properties.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-properties.js"></script>
>+<script src="../../fast/js/resources/js-test-post.js"></script>
>+</body>
>+</html>
>diff --git a/LayoutTests/storage/indexeddb/resources/cursor-properties.js b/LayoutTests/storage/indexeddb/resources/cursor-properties.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..c9f7eb88e89ecf2b4f55ad919a03e1b1de126cb1
>--- /dev/null
>+++ b/LayoutTests/storage/indexeddb/resources/cursor-properties.js
>@@ -0,0 +1,78 @@
>+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.");
>+setTimeout(finishJSTest, 1000);
>+
>+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();

Actions: View | Formatted Diff | Diff

Attachments on bug 100051: 170021 | 170198