[V8] IndexedDB: Cursor value modifications should be preserved until cursor iterates
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Jun 2012 20:29:50 +0000 (20:29 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Jun 2012 20:29:50 +0000 (20:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=83526

Patch by Alec Flett <alecflett@chromium.org> on 2012-06-13
Reviewed by Kentaro Hara.

Source/WebCore:

Cache the 'value' attribute of IDBCursorWithValue with policy
determined by IDBCursor.cpp, to follow spec behavior of keeping a
consistent script object until the cursor advances. See
http://www.w3.org/TR/IndexedDB/#widl-IDBCursorWithValueSync-value
for details.

Test: storage/indexeddb/cursor-value.html

* Modules/indexeddb/IDBCursor.cpp:
(WebCore::IDBCursor::IDBCursor):
(WebCore::IDBCursor::value):
(WebCore::IDBCursor::setValueReady):
* Modules/indexeddb/IDBCursor.h:
(IDBCursor):
(WebCore::IDBCursor::valueIsDirty):
* Modules/indexeddb/IDBCursorWithValue.idl:
* WebCore.gypi:
* bindings/v8/IDBCustomBindings.cpp: Added.
(WebCore):
(WebCore::V8IDBCursorWithValue::valueAccessorGetter):

LayoutTests:

* storage/indexeddb/cursor-value-expected.txt: Added.
* storage/indexeddb/cursor-value.html: Added.
* storage/indexeddb/resources/cursor-value.js: Added.
(openDatabase.request.onsuccess.request.onsuccess):
(openDatabase.request.onsuccess):
(openDatabase):
(testCursor.request.onsuccess):
(testCursor):
(ensureModificationsNotPersisted.request.onsuccess):
(ensureModificationsNotPersisted):

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

12 files changed:
LayoutTests/ChangeLog
LayoutTests/storage/indexeddb/cursor-value-expected.txt [new file with mode: 0644]
LayoutTests/storage/indexeddb/cursor-value.html [new file with mode: 0644]
LayoutTests/storage/indexeddb/resources/cursor-value.js [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/Modules/indexeddb/IDBCursor.cpp
Source/WebCore/Modules/indexeddb/IDBCursor.h
Source/WebCore/Modules/indexeddb/IDBCursorWithValue.idl
Source/WebCore/UseV8.cmake
Source/WebCore/WebCore.gypi
Source/WebCore/WebCore.vcproj/WebCore.vcproj
Source/WebCore/bindings/v8/IDBCustomBindings.cpp [new file with mode: 0644]

index 1c2c079..0b08d18 100644 (file)
@@ -1,3 +1,21 @@
+2012-06-13  Alec Flett  <alecflett@chromium.org>
+
+        [V8] IndexedDB: Cursor value modifications should be preserved until cursor iterates
+        https://bugs.webkit.org/show_bug.cgi?id=83526
+
+        Reviewed by Kentaro Hara.
+
+        * storage/indexeddb/cursor-value-expected.txt: Added.
+        * storage/indexeddb/cursor-value.html: Added.
+        * storage/indexeddb/resources/cursor-value.js: Added.
+        (openDatabase.request.onsuccess.request.onsuccess):
+        (openDatabase.request.onsuccess):
+        (openDatabase):
+        (testCursor.request.onsuccess):
+        (testCursor):
+        (ensureModificationsNotPersisted.request.onsuccess):
+        (ensureModificationsNotPersisted):
+
 2012-06-13  Shrey Banga  <banga@chromium.org>
 
         REGRESSION(r120108): It made http/tests/loading/gmail-assert-on-load.html fail
diff --git a/LayoutTests/storage/indexeddb/cursor-value-expected.txt b/LayoutTests/storage/indexeddb/cursor-value-expected.txt
new file mode 100644 (file)
index 0000000..303a421
--- /dev/null
@@ -0,0 +1,129 @@
+Test IndexedDB's cursor value property.
+
+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;
+
+request = indexedDB.open('cursor-value')
+db = request.result
+request = db.setVersion('new version')
+transaction = request.result
+Deleted all object stores.
+db.createObjectStore('store')
+
+testCursor():
+transaction = db.transaction('store', 'readwrite')
+store = transaction.objectStore('store')
+store.put({a: 1, b: 10}, 'key1')
+store.put({a: 2, b: 20}, 'key2')
+store.put({a: 3, b: 30}, 'key3')
+store.put({a: 4, b: 40}, 'key4')
+store.put({a: 5, b: 50}, 'key5')
+request = store.openCursor()
+
+----------
+Value at index: 0
+cursor = request.result
+PASS cursor.key is expectedKey
+
+Check expected values:
+PASS cursor.value.a is expectedA
+PASS cursor.value.b is expectedB
+PASS cursor.value.foo is undefined
+
+Modify values:
+cursor.value.a = 3
+delete cursor.value.b
+cursor.value.foo = 'bar'
+
+Ensure modifications are retained:
+PASS cursor.value.a is 3
+PASS cursor.value.b is undefined
+PASS cursor.value.foo is 'bar'
+
+Check object value survives gc
+gc()
+PASS cursor.value.a is 3
+PASS cursor.value.b is undefined
+PASS cursor.value.foo is 'bar'
+
+Check object identity
+localValueRef = cursor.value
+PASS localValueRef is cursor.value
+
+----------
+Value at index: 1
+cursor = request.result
+PASS cursor.key is expectedKey
+
+Check expected values:
+PASS cursor.value.a is expectedA
+PASS cursor.value.b is expectedB
+PASS cursor.value.foo is undefined
+
+Modify values:
+cursor.value.a = 3
+delete cursor.value.b
+cursor.value.foo = 'bar'
+
+Ensure modifications are retained:
+PASS cursor.value.a is 3
+PASS cursor.value.b is undefined
+PASS cursor.value.foo is 'bar'
+
+Check object value survives gc
+gc()
+PASS cursor.value.a is 3
+PASS cursor.value.b is undefined
+PASS cursor.value.foo is 'bar'
+
+Check object identity
+localValueRef = cursor.value
+PASS localValueRef is cursor.value
+
+----------
+Value at index: 3
+cursor = request.result
+PASS cursor.key is expectedKey
+
+Check expected values:
+PASS cursor.value.a is expectedA
+PASS cursor.value.b is expectedB
+PASS cursor.value.foo is undefined
+
+Modify values:
+cursor.value.a = 3
+delete cursor.value.b
+cursor.value.foo = 'bar'
+
+Ensure modifications are retained:
+PASS cursor.value.a is 3
+PASS cursor.value.b is undefined
+PASS cursor.value.foo is 'bar'
+
+Check object value survives gc
+gc()
+PASS cursor.value.a is 3
+PASS cursor.value.b is undefined
+PASS cursor.value.foo is 'bar'
+
+Check object identity
+localValueRef = cursor.value
+PASS localValueRef is cursor.value
+
+ensureModificationsNotPersisted():
+transaction = db.transaction('store', 'readonly')
+store = transaction.objectStore('store')
+request = store.openCursor()
+cursor = request.result
+PASS cursor.key is 'key1'
+
+Check expected values:
+PASS cursor.value.a is 1
+PASS cursor.value.b is 10
+PASS cursor.value.foo is undefined
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/storage/indexeddb/cursor-value.html b/LayoutTests/storage/indexeddb/cursor-value.html
new file mode 100644 (file)
index 0000000..6f1bc14
--- /dev/null
@@ -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-value.js"></script>
+<script src="../../fast/js/resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/storage/indexeddb/resources/cursor-value.js b/LayoutTests/storage/indexeddb/resources/cursor-value.js
new file mode 100644 (file)
index 0000000..91efe87
--- /dev/null
@@ -0,0 +1,124 @@
+if (this.importScripts) {
+    importScripts('../../../fast/js/resources/js-test-pre.js');
+    importScripts('shared.js');
+}
+
+description("Test IndexedDB's cursor value property.");
+
+removeVendorPrefixes();
+openDatabase();
+
+function openDatabase()
+{
+    evalAndLog("request = indexedDB.open('cursor-value')");
+    request.onerror = unexpectedErrorCallback;
+    request.onsuccess = function() {
+        evalAndLog("db = request.result");
+        evalAndLog("request = db.setVersion('new version')");
+        request.onerror = unexpectedErrorCallback;
+        request.onsuccess = function() {
+            evalAndLog("transaction = request.result");
+            transaction.onabort = unexpectedAbortCallback;
+            deleteAllObjectStores(db);
+            evalAndLog("db.createObjectStore('store')");
+            transaction.oncomplete = testCursor;
+        };
+    };
+}
+
+function testCursor()
+{
+    debug("");
+    debug("testCursor():");
+    evalAndLog("transaction = db.transaction('store', 'readwrite')");
+    evalAndLog("store = transaction.objectStore('store')");
+    evalAndLog("store.put({a: 1, b: 10}, 'key1')");
+    evalAndLog("store.put({a: 2, b: 20}, 'key2')");
+    evalAndLog("store.put({a: 3, b: 30}, 'key3')");
+    evalAndLog("store.put({a: 4, b: 40}, 'key4')");
+    evalAndLog("store.put({a: 5, b: 50}, 'key5')");
+    evalAndLog("request = store.openCursor()");
+    request.onerror = unexpectedErrorCallback;
+    var index = 0;
+    request.onsuccess = function() {
+        debug("");
+        debug("----------");
+        debug("Value at index: " + index);
+        evalAndLog("cursor = request.result");
+        if (index == 0) {
+            ensureObjectData(1, 10, 'key1');
+            cursor.continue();
+            index++;
+        } else if (index == 1) {
+            ensureObjectData(2, 20, 'key2');
+            cursor.advance(2);
+            index += 2;
+        } else if (index == 3) {
+            ensureObjectData(4, 40, 'key4');
+        } else {
+            testFailed("Bad index: " + index);
+        }
+    };
+
+    transaction.oncomplete = ensureModificationsNotPersisted;
+}
+
+function ensureObjectData(a, b, key)
+{
+    expectedA = a;
+    expectedB = b;
+    expectedKey = key;
+    shouldBe("cursor.key", "expectedKey");
+
+    debug("");
+    debug("Check expected values:");
+    shouldBe("cursor.value.a", "expectedA");
+    shouldBe("cursor.value.b", "expectedB");
+    shouldBe("cursor.value.foo", "undefined");
+
+    debug("");
+    debug("Modify values:");
+    evalAndLog("cursor.value.a = 3");
+    evalAndLog("delete cursor.value.b");
+    evalAndLog("cursor.value.foo = 'bar'");
+
+    debug("");
+    debug("Ensure modifications are retained:");
+    shouldBe("cursor.value.a", "3");
+    shouldBe("cursor.value.b", "undefined");
+    shouldBe("cursor.value.foo", "'bar'");
+
+    // make sure to test GC before holding a specific ref to the value
+    debug("");
+    debug("Check object value survives gc");
+    evalAndLog("gc()");
+    shouldBe("cursor.value.a", "3");
+    shouldBe("cursor.value.b", "undefined");
+    shouldBe("cursor.value.foo", "'bar'");
+
+    debug("");
+    debug("Check object identity");
+    evalAndLog("localValueRef = cursor.value");
+    shouldBe("localValueRef", "cursor.value");
+}
+
+function ensureModificationsNotPersisted()
+{
+    debug("");
+    debug("ensureModificationsNotPersisted():");
+    evalAndLog("transaction = db.transaction('store', 'readonly')");
+    evalAndLog("store = transaction.objectStore('store')");
+    evalAndLog("request = store.openCursor()");
+    request.onerror = unexpectedErrorCallback;
+    request.onsuccess = function() {
+        evalAndLog("cursor = request.result");
+        shouldBe("cursor.key", "'key1'");
+
+        debug("");
+        debug("Check expected values:");
+        shouldBe("cursor.value.a", "1");
+        shouldBe("cursor.value.b", "10");
+        shouldBe("cursor.value.foo", "undefined");
+    };
+    transaction.oncomplete = finishJSTest;
+ }
\ No newline at end of file
index 2bbe78b..d0e50f9 100644 (file)
@@ -1,3 +1,31 @@
+2012-06-13  Alec Flett  <alecflett@chromium.org>
+
+        [V8] IndexedDB: Cursor value modifications should be preserved until cursor iterates
+        https://bugs.webkit.org/show_bug.cgi?id=83526
+
+        Reviewed by Kentaro Hara.
+
+        Cache the 'value' attribute of IDBCursorWithValue with policy
+        determined by IDBCursor.cpp, to follow spec behavior of keeping a
+        consistent script object until the cursor advances. See
+        http://www.w3.org/TR/IndexedDB/#widl-IDBCursorWithValueSync-value
+        for details.
+
+        Test: storage/indexeddb/cursor-value.html
+
+        * Modules/indexeddb/IDBCursor.cpp:
+        (WebCore::IDBCursor::IDBCursor):
+        (WebCore::IDBCursor::value):
+        (WebCore::IDBCursor::setValueReady):
+        * Modules/indexeddb/IDBCursor.h:
+        (IDBCursor):
+        (WebCore::IDBCursor::valueIsDirty):
+        * Modules/indexeddb/IDBCursorWithValue.idl:
+        * WebCore.gypi:
+        * bindings/v8/IDBCustomBindings.cpp: Added.
+        (WebCore):
+        (WebCore::V8IDBCursorWithValue::valueAccessorGetter):
+
 2012-06-13  Silvia Pfeiffer  <silviapf@chromium.org>
 
         Code cleanup from bug 88881 to share the SliderVerticalPart code.
index fc79375..a8c4330 100644 (file)
@@ -77,6 +77,7 @@ IDBCursor::IDBCursor(PassRefPtr<IDBCursorBackendInterface> backend, IDBRequest*
     , m_transaction(transaction)
     , m_transactionNotifier(transaction, this)
     , m_gotValue(false)
+    , m_valueIsDirty(true)
 {
     ASSERT(m_backend);
     ASSERT(m_request);
@@ -109,9 +110,10 @@ PassRefPtr<IDBKey> IDBCursor::primaryKey() const
     return m_currentPrimaryKey;
 }
 
-PassRefPtr<IDBAny> IDBCursor::value() const
+PassRefPtr<IDBAny> IDBCursor::value()
 {
     IDB_TRACE("IDBCursor::value");
+    m_valueIsDirty = false;
     return m_currentValue;
 }
 
@@ -241,6 +243,7 @@ void IDBCursor::setValueReady()
     m_currentPrimaryKey = m_backend->primaryKey();
     m_currentValue = IDBAny::create(m_backend->value());
     m_gotValue = true;
+    m_valueIsDirty = true;
 }
 
 unsigned short IDBCursor::stringToDirection(const String& directionString, ExceptionCode& ec)
index cece00a..3f9d84f 100644 (file)
@@ -72,7 +72,7 @@ public:
     const String& direction() const;
     PassRefPtr<IDBKey> key() const;
     PassRefPtr<IDBKey> primaryKey() const;
-    PassRefPtr<IDBAny> value() const;
+    PassRefPtr<IDBAny> value();
     IDBAny* source() const;
 
     PassRefPtr<IDBRequest> update(ScriptExecutionContext*, PassRefPtr<SerializedScriptValue>, ExceptionCode&);
@@ -84,6 +84,11 @@ public:
     void close();
     void setValueReady();
 
+    // The spec requires that the script object that wraps the value
+    // be unchanged until the value changes as a result of the cursor
+    // advancing.
+    bool valueIsDirty() { return m_valueIsDirty; }
+
 protected:
     IDBCursor(PassRefPtr<IDBCursorBackendInterface>, IDBRequest*, IDBAny* source, IDBTransaction*);
 
@@ -99,6 +104,7 @@ private:
     RefPtr<IDBKey> m_currentKey;
     RefPtr<IDBKey> m_currentPrimaryKey;
     RefPtr<IDBAny> m_currentValue;
+    bool m_valueIsDirty;
 };
 
 } // namespace WebCore
index 6ffc5b5..512ea40 100644 (file)
@@ -28,6 +28,6 @@ module storage {
     interface [
         Conditional=INDEXED_DATABASE
     ] IDBCursorWithValue : IDBCursor {
-        readonly attribute IDBAny value;
+        readonly attribute [V8CustomGetter] IDBAny value;
     };
 }
index 448e266..fffc26b 100755 (executable)
@@ -22,6 +22,7 @@ LIST(APPEND WebCore_SOURCES
     bindings/v8/DOMWrapperWorld.cpp
     bindings/v8/DateExtension.cpp
     bindings/v8/IDBBindingUtilities.cpp
+    bindings/v8/IDBCustomBindings.cpp
     bindings/v8/IsolatedWorld.cpp
     bindings/v8/Dictionary.cpp
     bindings/v8/PageScriptDebugServer.cpp
index b75b1b2..7632a3f 100644 (file)
             'bindings/v8/DateExtension.h',
             'bindings/v8/IDBBindingUtilities.cpp',
             'bindings/v8/IDBBindingUtilities.h',
+            'bindings/v8/IDBCustomBindings.cpp',
             'bindings/v8/IntrusiveDOMWrapperMap.h',
             'bindings/v8/IsolatedWorld.cpp',
             'bindings/v8/IsolatedWorld.h',
index bf53fdd..62b6400 100755 (executable)
                                        >
                                </File>
                                <File
+                                       RelativePath="..\bindings\js\IDBCustomBindings.cpp"
+                                       >
+                                       <FileConfiguration
+                                               Name="Debug|Win32"
+                                               ExcludedFromBuild="true"
+                                               >
+                                               <Tool
+                                                       Name="VCCLCompilerTool"
+                                               />
+                                       </FileConfiguration>
+                                       <FileConfiguration
+                                               Name="Release|Win32"
+                                               ExcludedFromBuild="true"
+                                               >
+                                               <Tool
+                                                       Name="VCCLCompilerTool"
+                                               />
+                                       </FileConfiguration>
+                                       <FileConfiguration
+                                               Name="Debug_Cairo_CFLite|Win32"
+                                               ExcludedFromBuild="true"
+                                               >
+                                               <Tool
+                                                       Name="VCCLCompilerTool"
+                                               />
+                                       </FileConfiguration>
+                                       <FileConfiguration
+                                               Name="Release_Cairo_CFLite|Win32"
+                                               ExcludedFromBuild="true"
+                                               >
+                                               <Tool
+                                                       Name="VCCLCompilerTool"
+                                               />
+                                       </FileConfiguration>
+                                       <FileConfiguration
+                                               Name="Debug_All|Win32"
+                                               ExcludedFromBuild="true"
+                                               >
+                                               <Tool
+                                                       Name="VCCLCompilerTool"
+                                               />
+                                       </FileConfiguration>
+                                       <FileConfiguration
+                                               Name="Production|Win32"
+                                               ExcludedFromBuild="true"
+                                               >
+                                               <Tool
+                                                       Name="VCCLCompilerTool"
+                                               />
+                                       </FileConfiguration>
+                               </File>
+                               <File
                                        RelativePath="..\bindings\js\JavaScriptCallFrame.cpp"
                                        >
                                        <FileConfiguration
diff --git a/Source/WebCore/bindings/v8/IDBCustomBindings.cpp b/Source/WebCore/bindings/v8/IDBCustomBindings.cpp
new file mode 100644 (file)
index 0000000..e656976
--- /dev/null
@@ -0,0 +1,60 @@
+/*
+ * Copyright (C) 2012 Google Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ *     * Neither the name of Google Inc. nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+#include "V8IDBCursorWithValue.h"
+
+#if ENABLE(INDEXED_DATABASE)
+
+#include "V8IDBAny.h"
+
+namespace WebCore {
+
+v8::Handle<v8::Value> V8IDBCursorWithValue::valueAccessorGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
+{
+    INC_STATS("DOM.IDBCursorWithValue.value._get");
+
+    IDBCursorWithValue* imp = V8IDBCursorWithValue::toNative(info.Holder());
+    v8::Handle<v8::String> propertyName = v8::String::NewSymbol("value");
+    if (!imp->valueIsDirty()) {
+        v8::Handle<v8::Value> value = info.Holder()->GetHiddenValue(propertyName);
+        if (!value.IsEmpty())
+            return value;
+    }
+
+    RefPtr<IDBAny> result = imp->value();
+    v8::Handle<v8::Value> wrapper = toV8(result.get(), info.GetIsolate());
+    info.Holder()->SetHiddenValue(propertyName, wrapper);
+    return wrapper;
+}
+
+}
+
+#endif