Title: [99599] trunk
Revision
99599
Author
commit-qu...@webkit.org
Date
2011-11-08 11:41:41 -0800 (Tue, 08 Nov 2011)

Log Message

IndexedDB: reference cycle between IDBDatabase and IDBTransaction
https://bugs.webkit.org/show_bug.cgi?id=71749

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

Source/WebCore:

Break a cyclic reference leak following a setVersion call.

* storage/IDBDatabase.cpp:
(WebCore::IDBDatabase::setVersionChangeTransaction):
(WebCore::IDBDatabase::clearVersionChangeTransaction):
(WebCore::IDBDatabase::createObjectStore):
(WebCore::IDBDatabase::deleteObjectStore):
* storage/IDBDatabase.h:
* storage/IDBRequest.cpp:
(WebCore::IDBRequest::onSuccess):
* storage/IDBTransaction.cpp:
(WebCore::IDBTransaction::create):
(WebCore::IDBTransaction::onAbort):
(WebCore::IDBTransaction::onComplete):

LayoutTests:

Add test case to verify that creating/deleting a store outside
a transaction throws same error if store exists. Also fix
the test to actually try removing stores.

* storage/indexeddb/create-and-remove-object-store-expected.txt:
* storage/indexeddb/create-and-remove-object-store.html:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (99598 => 99599)


--- trunk/LayoutTests/ChangeLog	2011-11-08 19:29:58 UTC (rev 99598)
+++ trunk/LayoutTests/ChangeLog	2011-11-08 19:41:41 UTC (rev 99599)
@@ -1,3 +1,17 @@
+2011-11-08  Joshua Bell  <jsb...@chromium.org>
+
+        IndexedDB: reference cycle between IDBDatabase and IDBTransaction
+        https://bugs.webkit.org/show_bug.cgi?id=71749
+
+        Reviewed by Tony Chang.
+
+        Add test case to verify that creating/deleting a store outside
+        a transaction throws same error if store exists. Also fix
+        the test to actually try removing stores.
+
+        * storage/indexeddb/create-and-remove-object-store-expected.txt:
+        * storage/indexeddb/create-and-remove-object-store.html:
+
 2011-11-08  Ojan Vafai  <o...@chromium.org>
 
         Fix expectations mixup.

Modified: trunk/LayoutTests/storage/indexeddb/create-and-remove-object-store-expected.txt (99598 => 99599)


--- trunk/LayoutTests/storage/indexeddb/create-and-remove-object-store-expected.txt	2011-11-08 19:29:58 UTC (rev 99598)
+++ trunk/LayoutTests/storage/indexeddb/create-and-remove-object-store-expected.txt	2011-11-08 19:41:41 UTC (rev 99599)
@@ -11,7 +11,7 @@
 PASS Exception was thrown.
 PASS code is webkitIDBDatabaseException.NOT_ALLOWED_ERR
 Trying remove
-Expecting exception from db.createObjectStore("some os")
+Expecting exception from db.deleteObjectStore("some os")
 PASS Exception was thrown.
 PASS code is webkitIDBDatabaseException.NOT_ALLOWED_ERR
 result = db.setVersion('version 1')
@@ -20,7 +20,7 @@
 PASS Exception was thrown.
 PASS code is webkitIDBDatabaseException.NOT_ALLOWED_ERR
 Trying remove
-Expecting exception from db.createObjectStore("some os")
+Expecting exception from db.deleteObjectStore("some os")
 PASS Exception was thrown.
 PASS code is webkitIDBDatabaseException.NOT_ALLOWED_ERR
 Deleted all object stores.
@@ -36,9 +36,17 @@
 PASS Exception was thrown.
 PASS code is webkitIDBDatabaseException.NOT_ALLOWED_ERR
 Trying remove
-Expecting exception from db.createObjectStore("some os")
+Expecting exception from db.deleteObjectStore("some os")
 PASS Exception was thrown.
 PASS code is webkitIDBDatabaseException.NOT_ALLOWED_ERR
+Trying create with store that already exists
+Expecting exception from db.createObjectStore('tmp')
+PASS Exception was thrown.
+PASS code is webkitIDBDatabaseException.NOT_ALLOWED_ERR
+Trying remove with store that already exists
+Expecting exception from db.deleteObjectStore('tmp')
+PASS Exception was thrown.
+PASS code is webkitIDBDatabaseException.NOT_ALLOWED_ERR
 PASS successfullyParsed is true
 
 TEST COMPLETE

Modified: trunk/LayoutTests/storage/indexeddb/create-and-remove-object-store.html (99598 => 99599)


--- trunk/LayoutTests/storage/indexeddb/create-and-remove-object-store.html	2011-11-08 19:29:58 UTC (rev 99598)
+++ trunk/LayoutTests/storage/indexeddb/create-and-remove-object-store.html	2011-11-08 19:41:41 UTC (rev 99599)
@@ -35,7 +35,7 @@
     debug("Trying create");
     evalAndExpectException('db.createObjectStore("some os")', "webkitIDBDatabaseException.NOT_ALLOWED_ERR");
     debug("Trying remove");
-    evalAndExpectException('db.createObjectStore("some os")', "webkitIDBDatabaseException.NOT_ALLOWED_ERR");
+    evalAndExpectException('db.deleteObjectStore("some os")', "webkitIDBDatabaseException.NOT_ALLOWED_ERR");
 }
 
 function cleanDatabase()
@@ -57,6 +57,11 @@
 
     testCreateAndRemove();
 
+    debug("Trying create with store that already exists");
+    evalAndExpectException("db.createObjectStore('tmp')", "webkitIDBDatabaseException.NOT_ALLOWED_ERR");
+    debug("Trying remove with store that already exists");
+    evalAndExpectException("db.deleteObjectStore('tmp')", "webkitIDBDatabaseException.NOT_ALLOWED_ERR");
+
     done();
 }
 

Modified: trunk/Source/WebCore/ChangeLog (99598 => 99599)


--- trunk/Source/WebCore/ChangeLog	2011-11-08 19:29:58 UTC (rev 99598)
+++ trunk/Source/WebCore/ChangeLog	2011-11-08 19:41:41 UTC (rev 99599)
@@ -1,3 +1,25 @@
+2011-11-08  Joshua Bell  <jsb...@chromium.org>
+
+        IndexedDB: reference cycle between IDBDatabase and IDBTransaction
+        https://bugs.webkit.org/show_bug.cgi?id=71749
+
+        Reviewed by Tony Chang.
+
+        Break a cyclic reference leak following a setVersion call.
+
+        * storage/IDBDatabase.cpp:
+        (WebCore::IDBDatabase::setVersionChangeTransaction):
+        (WebCore::IDBDatabase::clearVersionChangeTransaction):
+        (WebCore::IDBDatabase::createObjectStore):
+        (WebCore::IDBDatabase::deleteObjectStore):
+        * storage/IDBDatabase.h:
+        * storage/IDBRequest.cpp:
+        (WebCore::IDBRequest::onSuccess):
+        * storage/IDBTransaction.cpp:
+        (WebCore::IDBTransaction::create):
+        (WebCore::IDBTransaction::onAbort):
+        (WebCore::IDBTransaction::onComplete):
+
 2011-11-08  Daniel Bates  <dba...@webkit.org>
 
         Remove get() defined in CredentialStorageSoup.cpp

Modified: trunk/Source/WebCore/storage/IDBDatabase.cpp (99598 => 99599)


--- trunk/Source/WebCore/storage/IDBDatabase.cpp	2011-11-08 19:29:58 UTC (rev 99598)
+++ trunk/Source/WebCore/storage/IDBDatabase.cpp	2011-11-08 19:41:41 UTC (rev 99599)
@@ -69,14 +69,21 @@
     m_databaseCallbacks->unregisterDatabase(this);
 }
 
-void IDBDatabase::setSetVersionTransaction(IDBTransaction* transaction)
+void IDBDatabase::setVersionChangeTransaction(IDBTransaction* transaction)
 {
-    m_setVersionTransaction = transaction;
+    ASSERT(!m_versionChangeTransaction);
+    m_versionChangeTransaction = transaction;
 }
 
+void IDBDatabase::clearVersionChangeTransaction(IDBTransaction* transaction)
+{
+    ASSERT_UNUSED(transaction, m_versionChangeTransaction == transaction);
+    m_versionChangeTransaction = 0;
+}
+
 PassRefPtr<IDBObjectStore> IDBDatabase::createObjectStore(const String& name, const OptionsObject& options, ExceptionCode& ec)
 {
-    if (!m_setVersionTransaction) {
+    if (!m_versionChangeTransaction) {
         ec = IDBDatabaseException::NOT_ALLOWED_ERR;
         return 0;
     }
@@ -92,22 +99,22 @@
     options.get("autoIncrement", autoIncrement);
     // FIXME: Look up evictable and pass that on as well.
 
-    RefPtr<IDBObjectStoreBackendInterface> objectStore = m_backend->createObjectStore(name, keyPath, autoIncrement, m_setVersionTransaction->backend(), ec);
+    RefPtr<IDBObjectStoreBackendInterface> objectStore = m_backend->createObjectStore(name, keyPath, autoIncrement, m_versionChangeTransaction->backend(), ec);
     if (!objectStore) {
         ASSERT(ec);
         return 0;
     }
-    return IDBObjectStore::create(objectStore.release(), m_setVersionTransaction.get());
+    return IDBObjectStore::create(objectStore.release(), m_versionChangeTransaction.get());
 }
 
 void IDBDatabase::deleteObjectStore(const String& name, ExceptionCode& ec)
 {
-    if (!m_setVersionTransaction) {
+    if (!m_versionChangeTransaction) {
         ec = IDBDatabaseException::NOT_ALLOWED_ERR;
         return;
     }
 
-    m_backend->deleteObjectStore(name, m_setVersionTransaction->backend(), ec);
+    m_backend->deleteObjectStore(name, m_versionChangeTransaction->backend(), ec);
 }
 
 PassRefPtr<IDBVersionChangeRequest> IDBDatabase::setVersion(ScriptExecutionContext* context, const String& version, ExceptionCode& ec)

Modified: trunk/Source/WebCore/storage/IDBDatabase.h (99598 => 99599)


--- trunk/Source/WebCore/storage/IDBDatabase.h	2011-11-08 19:29:58 UTC (rev 99598)
+++ trunk/Source/WebCore/storage/IDBDatabase.h	2011-11-08 19:41:41 UTC (rev 99599)
@@ -52,7 +52,8 @@
     static PassRefPtr<IDBDatabase> create(ScriptExecutionContext*, PassRefPtr<IDBDatabaseBackendInterface>);
     ~IDBDatabase();
 
-    void setSetVersionTransaction(IDBTransaction*);
+    void setVersionChangeTransaction(IDBTransaction*);
+    void clearVersionChangeTransaction(IDBTransaction*);
 
     // Implement the IDL
     String name() const { return m_backend->name(); }
@@ -101,7 +102,7 @@
     virtual EventTargetData* ensureEventTargetData();
 
     RefPtr<IDBDatabaseBackendInterface> m_backend;
-    RefPtr<IDBTransaction> m_setVersionTransaction;
+    RefPtr<IDBTransaction> m_versionChangeTransaction;
 
     bool m_noNewTransactions;
     bool m_stopped;

Modified: trunk/Source/WebCore/storage/IDBRequest.cpp (99598 => 99599)


--- trunk/Source/WebCore/storage/IDBRequest.cpp	2011-11-08 19:29:58 UTC (rev 99598)
+++ trunk/Source/WebCore/storage/IDBRequest.cpp	2011-11-08 19:41:41 UTC (rev 99599)
@@ -243,7 +243,8 @@
     m_transaction = frontend;
 
     ASSERT(m_source->type() == IDBAny::IDBDatabaseType);
-    m_source->idbDatabase()->setSetVersionTransaction(frontend.get());
+    ASSERT(m_transaction->mode() == IDBTransaction::VERSION_CHANGE);
+    m_source->idbDatabase()->setVersionChangeTransaction(frontend.get());
 
     IDBPendingTransactionMonitor::removePendingTransaction(m_transaction->backend());
 

Modified: trunk/Source/WebCore/storage/IDBTransaction.cpp (99598 => 99599)


--- trunk/Source/WebCore/storage/IDBTransaction.cpp	2011-11-08 19:29:58 UTC (rev 99598)
+++ trunk/Source/WebCore/storage/IDBTransaction.cpp	2011-11-08 19:41:41 UTC (rev 99599)
@@ -41,7 +41,7 @@
 namespace WebCore {
 
 PassRefPtr<IDBTransaction> IDBTransaction::create(ScriptExecutionContext* context, PassRefPtr<IDBTransactionBackendInterface> backend, IDBDatabase* db)
-{ 
+{
     return adoptRef(new IDBTransaction(context, backend, db));
 }
 
@@ -121,11 +121,17 @@
         request->abort();
     }
 
+    if (m_mode == IDBTransaction::VERSION_CHANGE)
+        m_database->clearVersionChangeTransaction(this);
+
     enqueueEvent(Event::create(eventNames().abortEvent, true, false));
 }
 
 void IDBTransaction::onComplete()
 {
+    if (m_mode == IDBTransaction::VERSION_CHANGE)
+        m_database->clearVersionChangeTransaction(this);
+
     enqueueEvent(Event::create(eventNames().completeEvent, false, false));
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to