- Revision
- 142483
- Author
- jsb...@chromium.org
- Date
- 2013-02-11 09:28:22 -0800 (Mon, 11 Feb 2013)
Log Message
[V8] IndexedDB: Minor GC can collect IDBDatabase wrapper with versionchange handler
https://bugs.webkit.org/show_bug.cgi?id=108670
Reviewed by Kentaro Hara.
Source/WebCore:
Prevent IDBDatabase's wrapper from being GC'd while the database is open if it has
listeners, as those listeners may close the database in response to events.
Also, removed extraneous super-calls from hasPendingActivity() overrides.
Test: storage/indexeddb/database-wrapper.html
* Modules/indexeddb/IDBDatabase.cpp:
(WebCore::IDBDatabase::hasPendingActivity): Implemented.
* Modules/indexeddb/IDBDatabase.h: Declared.
* Modules/indexeddb/IDBRequest.cpp:
(WebCore::IDBRequest::hasPendingActivity): Simplified.
* Modules/indexeddb/IDBTransaction.cpp:
(WebCore::IDBTransaction::hasPendingActivity): Simplified.
LayoutTests:
* storage/indexeddb/database-wrapper-expected.txt: Added.
* storage/indexeddb/database-wrapper.html: Added.
* storage/indexeddb/resources/database-wrapper.js: Added.
(test):
(openDB):
(onUpgradeNeeded):
(openSuccess.get request.onsuccess):
(onVersionChange):
(collectGarbage):
(openAgain):
(onBlocked):
(openAgainSuccess):
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (142482 => 142483)
--- trunk/LayoutTests/ChangeLog 2013-02-11 17:17:45 UTC (rev 142482)
+++ trunk/LayoutTests/ChangeLog 2013-02-11 17:28:22 UTC (rev 142483)
@@ -1,3 +1,23 @@
+2013-02-11 Joshua Bell <jsb...@chromium.org>
+
+ [V8] IndexedDB: Minor GC can collect IDBDatabase wrapper with versionchange handler
+ https://bugs.webkit.org/show_bug.cgi?id=108670
+
+ Reviewed by Kentaro Hara.
+
+ * storage/indexeddb/database-wrapper-expected.txt: Added.
+ * storage/indexeddb/database-wrapper.html: Added.
+ * storage/indexeddb/resources/database-wrapper.js: Added.
+ (test):
+ (openDB):
+ (onUpgradeNeeded):
+ (openSuccess.get request.onsuccess):
+ (onVersionChange):
+ (collectGarbage):
+ (openAgain):
+ (onBlocked):
+ (openAgainSuccess):
+
2013-02-11 Christophe Dumez <ch.du...@sisa.samsung.com>
Unreviewed EFL gardening.
Added: trunk/LayoutTests/storage/indexeddb/database-wrapper-expected.txt (0 => 142483)
--- trunk/LayoutTests/storage/indexeddb/database-wrapper-expected.txt (rev 0)
+++ trunk/LayoutTests/storage/indexeddb/database-wrapper-expected.txt 2013-02-11 17:28:22 UTC (rev 142483)
@@ -0,0 +1,38 @@
+Ensure IDBDatabase wrapper isn't prematurely collected.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+dbname = "database-wrapper.html"
+indexedDB.deleteDatabase(dbname)
+
+openDB():
+indexedDB.open(dbname, 1)
+
+onUpgradeNeeded():
+db = event.target.result
+db.createObjectStore('store').createIndex('index', 'keyPath')
+db = null
+
+openSuccess():
+sawVersionChangeEvent = false
+
+collectGarbage():
+self.gc()
+
+openAgain():
+indexedDB.open(dbname, 2)
+
+onVersionChange():
+event.target.close()
+sawVersionChangeEvent = true
+
+onBlocked():
+FIXME: Blocked event shouldn't fire. http://wkbug.com/71130
+
+openAgainSuccess():
+PASS sawVersionChangeEvent is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/storage/indexeddb/database-wrapper.html (0 => 142483)
--- trunk/LayoutTests/storage/indexeddb/database-wrapper.html (rev 0)
+++ trunk/LayoutTests/storage/indexeddb/database-wrapper.html 2013-02-11 17:28:22 UTC (rev 142483)
@@ -0,0 +1,10 @@
+<html>
+<head>
+<script src=""
+<script src=""
+</head>
+<body>
+<script src=""
+<script src=""
+</body>
+</html>
Added: trunk/LayoutTests/storage/indexeddb/resources/database-wrapper.js (0 => 142483)
--- trunk/LayoutTests/storage/indexeddb/resources/database-wrapper.js (rev 0)
+++ trunk/LayoutTests/storage/indexeddb/resources/database-wrapper.js 2013-02-11 17:28:22 UTC (rev 142483)
@@ -0,0 +1,90 @@
+if (this.importScripts) {
+ importScripts('../../../fast/js/resources/js-test-pre.js');
+ importScripts('shared.js');
+}
+
+description("Ensure IDBDatabase wrapper isn't prematurely collected.");
+
+test();
+
+function test()
+{
+ setDBNameFromPath();
+ var deleteRequest = evalAndLog("indexedDB.deleteDatabase(dbname)");
+ deleteRequest._onblocked_ = unexpectedBlockedCallback;
+ deleteRequest._onerror_ = unexpectedErrorCallback;
+ deleteRequest._onsuccess_ = openDB;
+}
+
+function openDB()
+{
+ preamble();
+
+ var openRequest = evalAndLog("indexedDB.open(dbname, 1)");
+ openRequest._onblocked_ = unexpectedBlockedCallback;
+ openRequest._onerror_ = unexpectedErrorCallback;
+ openRequest._onupgradeneeded_ = onUpgradeNeeded;
+ openRequest._onsuccess_ = openSuccess;
+}
+
+function onUpgradeNeeded(evt) {
+ preamble(evt);
+ evalAndLog("db = event.target.result");
+ evalAndLog("db.createObjectStore('store').createIndex('index', 'keyPath')");
+ evalAndLog("db = null");
+}
+
+function openSuccess(evt)
+{
+ preamble(evt);
+ var db = event.target.result;
+ db._onversionchange_ = onVersionChange;
+ evalAndLog("sawVersionChangeEvent = false");
+
+ // All these local references should get collected, but the database's
+ // wrapper shouldn't get collected before the database itself.
+ var transaction = db.transaction('store', 'readonly');
+ var objectStore = transaction.objectStore('store');
+ var request = objectStore.get(0);
+ request._onsuccess_ = function() {
+ setTimeout(collectGarbage, 0);
+ };
+}
+
+function onVersionChange(evt)
+{
+ preamble(evt);
+ evalAndLog("event.target.close()");
+ evalAndLog("sawVersionChangeEvent = true");
+}
+
+function collectGarbage()
+{
+ preamble();
+ evalAndLog("self.gc()");
+
+ setTimeout(openAgain, 0);
+}
+
+function openAgain()
+{
+ preamble();
+
+ var openRequest = evalAndLog("indexedDB.open(dbname, 2)");
+ openRequest._onblocked_ = onBlocked;
+ openRequest._onerror_ = unexpectedErrorCallback;
+ openRequest._onsuccess_ = openAgainSuccess;
+}
+
+function onBlocked()
+{
+ preamble();
+ debug("FIXME: Blocked event shouldn't fire. http://wkbug.com/71130");
+}
+
+function openAgainSuccess(evt)
+{
+ preamble(evt);
+ shouldBeTrue("sawVersionChangeEvent");
+ finishJSTest();
+}
Modified: trunk/Source/WebCore/ChangeLog (142482 => 142483)
--- trunk/Source/WebCore/ChangeLog 2013-02-11 17:17:45 UTC (rev 142482)
+++ trunk/Source/WebCore/ChangeLog 2013-02-11 17:28:22 UTC (rev 142483)
@@ -1,3 +1,25 @@
+2013-02-11 Joshua Bell <jsb...@chromium.org>
+
+ [V8] IndexedDB: Minor GC can collect IDBDatabase wrapper with versionchange handler
+ https://bugs.webkit.org/show_bug.cgi?id=108670
+
+ Reviewed by Kentaro Hara.
+
+ Prevent IDBDatabase's wrapper from being GC'd while the database is open if it has
+ listeners, as those listeners may close the database in response to events.
+
+ Also, removed extraneous super-calls from hasPendingActivity() overrides.
+
+ Test: storage/indexeddb/database-wrapper.html
+
+ * Modules/indexeddb/IDBDatabase.cpp:
+ (WebCore::IDBDatabase::hasPendingActivity): Implemented.
+ * Modules/indexeddb/IDBDatabase.h: Declared.
+ * Modules/indexeddb/IDBRequest.cpp:
+ (WebCore::IDBRequest::hasPendingActivity): Simplified.
+ * Modules/indexeddb/IDBTransaction.cpp:
+ (WebCore::IDBTransaction::hasPendingActivity): Simplified.
+
2013-02-11 Eric Seidel <e...@webkit.org>
Remove AttributeBase now that NEW_XML is gone
Modified: trunk/Source/WebCore/Modules/indexeddb/IDBDatabase.cpp (142482 => 142483)
--- trunk/Source/WebCore/Modules/indexeddb/IDBDatabase.cpp 2013-02-11 17:17:45 UTC (rev 142482)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBDatabase.cpp 2013-02-11 17:28:22 UTC (rev 142483)
@@ -354,6 +354,13 @@
return IDBObjectStoreMetadata::InvalidId;
}
+bool IDBDatabase::hasPendingActivity() const
+{
+ // The script wrapper must not be collected before the object is closed or
+ // we can't fire a "versionchange" event to let script manually close the connection.
+ return !m_closePending && !m_eventTargetData.eventListenerMap.isEmpty();
+}
+
void IDBDatabase::stop()
{
ActiveDOMObject::stop();
Modified: trunk/Source/WebCore/Modules/indexeddb/IDBDatabase.h (142482 => 142483)
--- trunk/Source/WebCore/Modules/indexeddb/IDBDatabase.h 2013-02-11 17:17:45 UTC (rev 142482)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBDatabase.h 2013-02-11 17:28:22 UTC (rev 142483)
@@ -82,6 +82,7 @@
virtual void onComplete(int64_t);
// ActiveDOMObject
+ virtual bool hasPendingActivity() const OVERRIDE;
virtual void stop() OVERRIDE;
// EventTarget
Modified: trunk/Source/WebCore/Modules/indexeddb/IDBRequest.cpp (142482 => 142483)
--- trunk/Source/WebCore/Modules/indexeddb/IDBRequest.cpp 2013-02-11 17:17:45 UTC (rev 142482)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBRequest.cpp 2013-02-11 17:28:22 UTC (rev 142483)
@@ -406,7 +406,7 @@
// FIXME: In an ideal world, we should return true as long as anyone has a or can
// get a handle to us and we have event listeners. This is order to handle
// user generated events properly.
- return m_hasPendingActivity || ActiveDOMObject::hasPendingActivity();
+ return m_hasPendingActivity;
}
void IDBRequest::stop()
Modified: trunk/Source/WebCore/Modules/indexeddb/IDBTransaction.cpp (142482 => 142483)
--- trunk/Source/WebCore/Modules/indexeddb/IDBTransaction.cpp 2013-02-11 17:17:45 UTC (rev 142482)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBTransaction.cpp 2013-02-11 17:28:22 UTC (rev 142483)
@@ -332,7 +332,7 @@
// FIXME: In an ideal world, we should return true as long as anyone has a or can
// get a handle to us or any child request object and any of those have
// event listeners. This is in order to handle user generated events properly.
- return m_hasPendingActivity || ActiveDOMObject::hasPendingActivity();
+ return m_hasPendingActivity;
}
IDBTransaction::Mode IDBTransaction::stringToMode(const String& modeString, ScriptExecutionContext* context, ExceptionCode& ec)
