Title: [103283] trunk
Revision
103283
Author
commit-qu...@webkit.org
Date
2011-12-19 16:57:07 -0800 (Mon, 19 Dec 2011)

Log Message

IndexedDB multiple calls to transaction.objectStore(name) should return the same instance
https://bugs.webkit.org/show_bug.cgi?id=60208

Patch by Joshua Bell <jsb...@chromium.org> on 2011-12-19
Reviewed by Tony Chang.

Source/WebCore:

Ditto for calls to IDBObjectStore.index(). Calling these methods after the
enclosing transaction has finished now consistently throws an error, which
allows us to break reference cycles.

Test: storage/indexeddb/mozilla/object-identity.html

* storage/IDBDatabase.cpp:
(WebCore::IDBDatabase::createObjectStore):
* storage/IDBObjectStore.cpp:
(WebCore::IDBObjectStore::createIndex):
(WebCore::IDBObjectStore::index):
(WebCore::IDBObjectStore::transactionFinished):
* storage/IDBObjectStore.h:
* storage/IDBTransaction.cpp:
(WebCore::IDBTransaction::objectStore):
(WebCore::IDBTransaction::objectStoreCreated):
(WebCore::IDBTransaction::dispatchEvent):
* storage/IDBTransaction.h:

LayoutTests:

* storage/indexeddb/mozilla/object-identity-expected.txt: Added.
* storage/indexeddb/mozilla/object-identity.html: Added.
* storage/indexeddb/transaction-and-objectstore-calls-expected.txt:
* storage/indexeddb/transaction-and-objectstore-calls.html:
* storage/indexeddb/tutorial.html:

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (103282 => 103283)


--- trunk/LayoutTests/ChangeLog	2011-12-20 00:52:59 UTC (rev 103282)
+++ trunk/LayoutTests/ChangeLog	2011-12-20 00:57:07 UTC (rev 103283)
@@ -1,3 +1,16 @@
+2011-12-19  Joshua Bell  <jsb...@chromium.org>
+
+        IndexedDB multiple calls to transaction.objectStore(name) should return the same instance
+        https://bugs.webkit.org/show_bug.cgi?id=60208
+
+        Reviewed by Tony Chang.
+
+        * storage/indexeddb/mozilla/object-identity-expected.txt: Added.
+        * storage/indexeddb/mozilla/object-identity.html: Added.
+        * storage/indexeddb/transaction-and-objectstore-calls-expected.txt:
+        * storage/indexeddb/transaction-and-objectstore-calls.html:
+        * storage/indexeddb/tutorial.html:
+
 2011-12-19  Tony Chang  <t...@chromium.org>
 
         [chromium] Unreviewed, adding a bug number for svg/carto.net/selectionlist.svg.

Added: trunk/LayoutTests/storage/indexeddb/mozilla/object-identity-expected.txt (0 => 103283)


--- trunk/LayoutTests/storage/indexeddb/mozilla/object-identity-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/storage/indexeddb/mozilla/object-identity-expected.txt	2011-12-20 00:57:07 UTC (rev 103283)
@@ -0,0 +1,34 @@
+Test IndexedDB: object identity
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+indexedDB = window.indexedDB || window.webkitIndexedDB || window.mozIndexedDB;
+PASS indexedDB == null is false
+indexedDB.open(name, description)
+db = event.target.result
+request = db.setVersion('1')
+transaction = event.target.transaction;
+Deleted all object stores.
+objectStore1 = db.createObjectStore('foo');
+objectStore2 = transaction.objectStore('foo');
+PASS objectStore1 === objectStore2 is true
+index1 = objectStore1.createIndex('bar', 'key');
+index2 = objectStore2.index('bar');
+PASS index1 === index2 is true
+transaction._oncomplete_ = testIdentitySomeMore;
+transaction = db.transaction('foo');
+objectStore3 = transaction.objectStore('foo');
+objectStore4 = transaction.objectStore('foo');
+PASS objectStore3 === objectStore4 is true
+PASS objectStore3 === objectStore1 is false
+PASS objectStore4 === objectStore2 is false
+index3 = objectStore3.index('bar');
+index4 = objectStore4.index('bar');
+PASS index3 === index4 is true
+PASS index3 === index1 is false
+PASS index4 === index2 is false
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/storage/indexeddb/mozilla/object-identity.html (0 => 103283)


--- trunk/LayoutTests/storage/indexeddb/mozilla/object-identity.html	                        (rev 0)
+++ trunk/LayoutTests/storage/indexeddb/mozilla/object-identity.html	2011-12-20 00:57:07 UTC (rev 103283)
@@ -0,0 +1,80 @@
+<!DOCTYPE html>
+<!--
+  original test: http://mxr.mozilla.org/mozilla2.0/source/dom/indexedDB/test/test_object_identity.html?force=1
+  license of original test:
+    " Any copyright is dedicated to the Public Domain.
+      http://creativecommons.org/publicdomain/zero/1.0/ "
+-->
+<html>
+<head>
+<script src=""
+<script src=""
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script>
+
+description("Test IndexedDB: object identity");
+if (window.layoutTestController)
+    layoutTestController.waitUntilDone();
+
+function test()
+{
+    indexedDB = evalAndLog("indexedDB = window.indexedDB || window.webkitIndexedDB || window.mozIndexedDB;");
+    shouldBeFalse("indexedDB == null");
+
+    name = window.location.pathname;
+    description = "My Test Database";
+    request = evalAndLog("indexedDB.open(name, description)");
+    request._onsuccess_ = openSuccess;
+    request._onerror_ = unexpectedErrorCallback;
+}
+
+function openSuccess()
+{
+    db = evalAndLog("db = event.target.result");
+    request = evalAndLog("request = db.setVersion('1')");
+    request._onsuccess_ = createObjectStoreAndIndex;
+    request._onerror_ = unexpectedErrorCallback;
+}
+
+function createObjectStoreAndIndex()
+{
+    transaction = evalAndLog("transaction = event.target.transaction;");
+    deleteAllObjectStores(db);
+    objectStore1 = evalAndLog("objectStore1 = db.createObjectStore('foo');");
+    objectStore2 = evalAndLog("objectStore2 = transaction.objectStore('foo');");
+    shouldBeTrue("objectStore1 === objectStore2");
+    index1 = evalAndLog("index1 = objectStore1.createIndex('bar', 'key');");
+    index2 = evalAndLog("index2 = objectStore2.index('bar');");
+    shouldBeTrue("index1 === index2");
+    evalAndLog("transaction._oncomplete_ = testIdentitySomeMore;");
+}
+
+function testIdentitySomeMore()
+{
+    transaction = evalAndLog("transaction = db.transaction('foo');");
+    objectStore3 = evalAndLog("objectStore3 = transaction.objectStore('foo');");
+    objectStore4 = evalAndLog("objectStore4 = transaction.objectStore('foo');");
+    shouldBeTrue("objectStore3 === objectStore4");
+
+    shouldBeFalse("objectStore3 === objectStore1");
+    shouldBeFalse("objectStore4 === objectStore2");
+
+    index3 = evalAndLog("index3 = objectStore3.index('bar');");
+    index4 = evalAndLog("index4 = objectStore4.index('bar');");
+    shouldBeTrue("index3 === index4");
+
+    shouldBeFalse("index3 === index1");
+    shouldBeFalse("index4 === index2");
+
+    done();
+}
+
+test();
+
+</script>
+</body>
+</html>
+

Modified: trunk/LayoutTests/storage/indexeddb/transaction-and-objectstore-calls-expected.txt (103282 => 103283)


--- trunk/LayoutTests/storage/indexeddb/transaction-and-objectstore-calls-expected.txt	2011-12-20 00:52:59 UTC (rev 103282)
+++ trunk/LayoutTests/storage/indexeddb/transaction-and-objectstore-calls-expected.txt	2011-12-20 00:57:07 UTC (rev 103283)
@@ -12,6 +12,7 @@
 Deleted all object stores.
 db.createObjectStore('a')
 db.createObjectStore('b')
+db.createObjectStore('store').createIndex('index', 'some_path')
 trans.addEventListener('complete', created, true)
 
 trans = db.transaction(['a'])
@@ -107,6 +108,20 @@
 PASS Exception was thrown.
 PASS code is webkitIDBDatabaseException.NOT_FOUND_ERR
 
+trans = db.transaction(['store'])
+PASS trans != null is true
+store = trans.objectStore('store')
+PASS store != null is true
+store.get('some_key')
+transaction complete, ensuring methods fail
+PASS trans != null is true
+PASS store != null is true
+Expecting exception from trans.objectStore('store')
+PASS Exception was thrown.
+PASS code is webkitIDBDatabaseException.NOT_ALLOWED_ERR
+Expecting exception from store.index('index')
+PASS Exception was thrown.
+PASS code is webkitIDBDatabaseException.NOT_ALLOWED_ERR
 PASS successfullyParsed is true
 
 TEST COMPLETE

Modified: trunk/LayoutTests/storage/indexeddb/transaction-and-objectstore-calls.html (103282 => 103283)


--- trunk/LayoutTests/storage/indexeddb/transaction-and-objectstore-calls.html	2011-12-20 00:52:59 UTC (rev 103282)
+++ trunk/LayoutTests/storage/indexeddb/transaction-and-objectstore-calls.html	2011-12-20 00:57:07 UTC (rev 103283)
@@ -37,6 +37,7 @@
 
     evalAndLog("db.createObjectStore('a')");
     evalAndLog("db.createObjectStore('b')");
+    evalAndLog("db.createObjectStore('store').createIndex('index', 'some_path')");
     evalAndLog("trans.addEventListener('complete', created, true)");
     debug("");
 }
@@ -105,12 +106,29 @@
     debug("... but you still need to specify a real store:");
     evalAndExpectException("db.transaction([{toString:function(){return 'x';}}])", "webkitIDBDatabaseException.NOT_FOUND_ERR");
     evalAndExpectException("db.transaction([{toString:function(){return 'x';}}])", "webkitIDBDatabaseException.NOT_FOUND_ERR");
+    debug("");
 
-    debug("");
+    trans = evalAndLog("trans = db.transaction(['store'])");
+    shouldBeTrue("trans != null");
+    trans._onabort_ = unexpectedAbortCallback;
+    trans._onerror_ = unexpectedErrorCallback;
+    trans._oncomplete_ = afterComplete;
+    evalAndLog("store = trans.objectStore('store')");
+    shouldBeTrue("store != null");
+    evalAndLog("store.get('some_key')");
+}
+
+function afterComplete()
+{
+    debug("transaction complete, ensuring methods fail");
+    shouldBeTrue("trans != null");
+    shouldBeTrue("store != null");
+    evalAndExpectException("trans.objectStore('store')", "webkitIDBDatabaseException.NOT_ALLOWED_ERR");
+    evalAndExpectException("store.index('index')", "webkitIDBDatabaseException.NOT_ALLOWED_ERR");
+
     done();
 }
 
-
 test();
 
 </script>

Modified: trunk/LayoutTests/storage/indexeddb/tutorial.html (103282 => 103283)


--- trunk/LayoutTests/storage/indexeddb/tutorial.html	2011-12-20 00:52:59 UTC (rev 103282)
+++ trunk/LayoutTests/storage/indexeddb/tutorial.html	2011-12-20 00:57:07 UTC (rev 103283)
@@ -312,10 +312,10 @@
     request._onsuccess_ = onGetSuccess;
     request._onerror_ = unexpectedError;
 
-    // Note that multiple objectStore (or index) method calls will return different objects (that still
-    // refer to the same objectStore/index on disk).
-    people.someProperty = true;
-    if (currentTransaction.objectStore("people").someProperty)
+    // Note that multiple objectStore (or index) method calls will return the same object, within
+    // a transaction:
+    people.someProperty = "xyz";
+    if (currentTransaction.objectStore("people").someProperty != "xyz")
         logError("Something went wrong.");
 }
 

Modified: trunk/Source/WebCore/ChangeLog (103282 => 103283)


--- trunk/Source/WebCore/ChangeLog	2011-12-20 00:52:59 UTC (rev 103282)
+++ trunk/Source/WebCore/ChangeLog	2011-12-20 00:57:07 UTC (rev 103283)
@@ -1,3 +1,29 @@
+2011-12-19  Joshua Bell  <jsb...@chromium.org>
+
+        IndexedDB multiple calls to transaction.objectStore(name) should return the same instance
+        https://bugs.webkit.org/show_bug.cgi?id=60208
+
+        Reviewed by Tony Chang.
+
+        Ditto for calls to IDBObjectStore.index(). Calling these methods after the
+        enclosing transaction has finished now consistently throws an error, which
+        allows us to break reference cycles.
+
+        Test: storage/indexeddb/mozilla/object-identity.html
+
+        * storage/IDBDatabase.cpp:
+        (WebCore::IDBDatabase::createObjectStore):
+        * storage/IDBObjectStore.cpp:
+        (WebCore::IDBObjectStore::createIndex):
+        (WebCore::IDBObjectStore::index):
+        (WebCore::IDBObjectStore::transactionFinished):
+        * storage/IDBObjectStore.h:
+        * storage/IDBTransaction.cpp:
+        (WebCore::IDBTransaction::objectStore):
+        (WebCore::IDBTransaction::objectStoreCreated):
+        (WebCore::IDBTransaction::dispatchEvent):
+        * storage/IDBTransaction.h:
+
 2011-12-19  Anders Carlsson  <ander...@apple.com>
 
         Send gesture events through the event dispatcher and scrolling coordinator

Modified: trunk/Source/WebCore/storage/IDBDatabase.cpp (103282 => 103283)


--- trunk/Source/WebCore/storage/IDBDatabase.cpp	2011-12-20 00:52:59 UTC (rev 103282)
+++ trunk/Source/WebCore/storage/IDBDatabase.cpp	2011-12-20 00:57:07 UTC (rev 103283)
@@ -100,12 +100,15 @@
     options.get("autoIncrement", autoIncrement);
     // FIXME: Look up evictable and pass that on as well.
 
-    RefPtr<IDBObjectStoreBackendInterface> objectStore = m_backend->createObjectStore(name, keyPath, autoIncrement, m_versionChangeTransaction->backend(), ec);
-    if (!objectStore) {
+    RefPtr<IDBObjectStoreBackendInterface> objectStoreBackend = m_backend->createObjectStore(name, keyPath, autoIncrement, m_versionChangeTransaction->backend(), ec);
+    if (!objectStoreBackend) {
         ASSERT(ec);
         return 0;
     }
-    return IDBObjectStore::create(objectStore.release(), m_versionChangeTransaction.get());
+
+    RefPtr<IDBObjectStore> objectStore = IDBObjectStore::create(objectStoreBackend.release(), m_versionChangeTransaction.get());
+    m_versionChangeTransaction->objectStoreCreated(name, objectStore);
+    return objectStore.release();
 }
 
 void IDBDatabase::deleteObjectStore(const String& name, ExceptionCode& ec)
@@ -116,6 +119,7 @@
     }
 
     m_backend->deleteObjectStore(name, m_versionChangeTransaction->backend(), ec);
+    m_versionChangeTransaction->objectStoreDeleted(name);
 }
 
 PassRefPtr<IDBVersionChangeRequest> IDBDatabase::setVersion(ScriptExecutionContext* context, const String& version, ExceptionCode& ec)

Modified: trunk/Source/WebCore/storage/IDBObjectStore.cpp (103282 => 103283)


--- trunk/Source/WebCore/storage/IDBObjectStore.cpp	2011-12-20 00:52:59 UTC (rev 103282)
+++ trunk/Source/WebCore/storage/IDBObjectStore.cpp	2011-12-20 00:57:07 UTC (rev 103283)
@@ -163,20 +163,34 @@
 
     // FIXME: When Array-type keyPaths are supported, throw exception if keyPath is Array and multiEntry is true.
 
-    RefPtr<IDBIndexBackendInterface> index = m_backend->createIndex(name, keyPath, unique, multiEntry, m_transaction->backend(), ec);
-    ASSERT(!index != !ec); // If we didn't get an index, we should have gotten an exception code. And vice versa.
-    if (!index)
+    RefPtr<IDBIndexBackendInterface> indexBackend = m_backend->createIndex(name, keyPath, unique, multiEntry, m_transaction->backend(), ec);
+    ASSERT(!indexBackend != !ec); // If we didn't get an index, we should have gotten an exception code. And vice versa.
+    if (!indexBackend)
         return 0;
-    return IDBIndex::create(index.release(), this, m_transaction.get());
+    RefPtr<IDBIndex> index = IDBIndex::create(indexBackend.release(), this, m_transaction.get());
+    m_indexMap.set(name, index);
+    return index.release();
 }
 
 PassRefPtr<IDBIndex> IDBObjectStore::index(const String& name, ExceptionCode& ec)
 {
-    RefPtr<IDBIndexBackendInterface> index = m_backend->index(name, ec);
-    ASSERT(!index != !ec); // If we didn't get an index, we should have gotten an exception code. And vice versa.
-    if (!index)
+    if (m_transaction->finished()) {
+        ec = IDBDatabaseException::NOT_ALLOWED_ERR;
         return 0;
-    return IDBIndex::create(index.release(), this, m_transaction.get());
+    }
+
+    IDBIndexMap::iterator it = m_indexMap.find(name);
+    if (it != m_indexMap.end())
+        return it->second;
+
+    RefPtr<IDBIndexBackendInterface> indexBackend = m_backend->index(name, ec);
+    ASSERT(!indexBackend != !ec); // If we didn't get an index, we should have gotten an exception code. And vice versa.
+    if (!indexBackend)
+        return 0;
+
+    RefPtr<IDBIndex> index = IDBIndex::create(indexBackend.release(), this, m_transaction.get());
+    m_indexMap.set(name, index);
+    return index.release();
 }
 
 void IDBObjectStore::deleteIndex(const String& name, ExceptionCode& ec)
@@ -213,6 +227,15 @@
     return request.release();
 }
 
+void IDBObjectStore::transactionFinished()
+{
+    ASSERT(m_transaction->finished());
+
+    // Break reference cycles.
+    m_indexMap.clear();
+}
+
+
 } // namespace WebCore
 
 #endif // ENABLE(INDEXED_DATABASE)

Modified: trunk/Source/WebCore/storage/IDBObjectStore.h (103282 => 103283)


--- trunk/Source/WebCore/storage/IDBObjectStore.h	2011-12-20 00:52:59 UTC (rev 103282)
+++ trunk/Source/WebCore/storage/IDBObjectStore.h	2011-12-20 00:57:07 UTC (rev 103283)
@@ -81,12 +81,17 @@
     PassRefPtr<IDBRequest> count(ScriptExecutionContext* context, ExceptionCode& ec) { return count(context, 0, ec); }
     PassRefPtr<IDBRequest> count(ScriptExecutionContext*, PassRefPtr<IDBKeyRange>, ExceptionCode&);
 
+    void transactionFinished();
+
 private:
     IDBObjectStore(PassRefPtr<IDBObjectStoreBackendInterface>, IDBTransaction*);
     void removeTransactionFromPendingList();
 
     RefPtr<IDBObjectStoreBackendInterface> m_backend;
     RefPtr<IDBTransaction> m_transaction;
+
+    typedef HashMap<String, RefPtr<IDBIndex> > IDBIndexMap;
+    IDBIndexMap m_indexMap;
 };
 
 } // namespace WebCore

Modified: trunk/Source/WebCore/storage/IDBTransaction.cpp (103282 => 103283)


--- trunk/Source/WebCore/storage/IDBTransaction.cpp	2011-12-20 00:52:59 UTC (rev 103282)
+++ trunk/Source/WebCore/storage/IDBTransaction.cpp	2011-12-20 00:57:07 UTC (rev 103283)
@@ -87,15 +87,33 @@
         ec = IDBDatabaseException::NOT_ALLOWED_ERR;
         return 0;
     }
+
+    IDBObjectStoreMap::iterator it = m_objectStoreMap.find(name);
+    if (it != m_objectStoreMap.end())
+        return it->second;
+
     RefPtr<IDBObjectStoreBackendInterface> objectStoreBackend = m_backend->objectStore(name, ec);
     if (!objectStoreBackend) {
         ASSERT(ec);
         return 0;
     }
     RefPtr<IDBObjectStore> objectStore = IDBObjectStore::create(objectStoreBackend, this);
+    objectStoreCreated(name, objectStore);
     return objectStore.release();
 }
 
+void IDBTransaction::objectStoreCreated(const String& name, PassRefPtr<IDBObjectStore> objectStore)
+{
+    ASSERT(!m_transactionFinished);
+    m_objectStoreMap.set(name, objectStore);
+}
+
+void IDBTransaction::objectStoreDeleted(const String& name)
+{
+    ASSERT(!m_transactionFinished);
+    m_objectStoreMap.remove(name);
+}
+
 void IDBTransaction::abort()
 {
     if (m_transactionFinished)
@@ -171,6 +189,11 @@
     ASSERT(event->target() == this);
     m_transactionFinished = true;
 
+    // Break reference cycles.
+    for (IDBObjectStoreMap::iterator it = m_objectStoreMap.begin(); it != m_objectStoreMap.end(); ++it)
+        it->second->transactionFinished();
+    m_objectStoreMap.clear();
+
     Vector<RefPtr<EventTarget> > targets;
     targets.append(this);
     targets.append(db());

Modified: trunk/Source/WebCore/storage/IDBTransaction.h (103282 => 103283)


--- trunk/Source/WebCore/storage/IDBTransaction.h	2011-12-20 00:52:59 UTC (rev 103282)
+++ trunk/Source/WebCore/storage/IDBTransaction.h	2011-12-20 00:57:07 UTC (rev 103283)
@@ -64,6 +64,8 @@
 
     void registerRequest(IDBRequest*);
     void unregisterRequest(IDBRequest*);
+    void objectStoreCreated(const String&, PassRefPtr<IDBObjectStore>);
+    void objectStoreDeleted(const String&);
 
     DEFINE_ATTRIBUTE_EVENT_LISTENER(abort);
     DEFINE_ATTRIBUTE_EVENT_LISTENER(complete);
@@ -106,6 +108,9 @@
 
     ListHashSet<IDBRequest*> m_childRequests;
 
+    typedef HashMap<String, RefPtr<IDBObjectStore> > IDBObjectStoreMap;
+    IDBObjectStoreMap m_objectStoreMap;
+
     EventTargetData m_eventTargetData;
 };
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to