Title: [115370] branches/chromium/1084
- Revision
- 115370
- Author
- [email protected]
- Date
- 2012-04-26 14:36:45 -0700 (Thu, 26 Apr 2012)
Log Message
Merge 113818 - Notify observers of WorkerRunLoop stopping before the V8 isolate dies.
https://bugs.webkit.org/show_bug.cgi?id=83104
Source/WebCore:
PlatformSupport::didStopWorkerRunLoop ultimately causes
~V8AbstractEventListener to call
v8::Local<v8::Object>::New(m_listener) after the V8 isolate has been
disposed, which manifests as a crash in V8.
The current code in trunk runs this at shutdown:
1) removeAllDOMObjects()
2) dispose of V8
3) didStopWorkerRunLoop() <-- problem
This patch changes the order to be:
1) removeAllDOMObjects()
2) didStopWorkerRunLoop()
3) dispose of V8
We put didStopWorkerRunLoop after removeAllDOMObjects because we don't
want chromium code that runs on a webcore worker to run after it
receives the didStopWorkerRunLoop signal. The destructors of some IDB
objects are run by removeAllDOMObjects, so putting
didStopWorkerRunLoop before removeAllDOMObjects would violate that
constraint.
It's possible that there's a lower layer fix available in V8 or the
bindings.
Reviewed by David Levin.
Test: storage/indexeddb/pending-version-change-on-exit.html
* bindings/v8/WorkerScriptController.cpp:
(WebCore::WorkerScriptController::~WorkerScriptController):
New location of didStopWorkerRunLoop. removeAllDOMObjects and V8
disposal are called here, to run something between them it also has
to go here.
* workers/WorkerThread.cpp:
(WebCore::WorkerThread::workerThread): Old location of
didStopWorkerRunLoop.
LayoutTests:
Reviewed by David Levin.
* storage/indexeddb/pending-version-change-on-exit-expected.txt: Added.
* storage/indexeddb/pending-version-change-on-exit.html: Added.
* storage/indexeddb/resources/pending-version-change-on-exit.js: Added.
(test.request.onsuccess.request.onblocked):
(test.request.onsuccess):
(test):
[email protected]
Review URL: https://chromiumcodereview.appspot.com/10232019
Modified Paths
Added Paths
Diff
Copied: branches/chromium/1084/LayoutTests/storage/indexeddb/pending-version-change-on-exit-expected.txt (from rev 113818, trunk/LayoutTests/storage/indexeddb/pending-version-change-on-exit-expected.txt) (0 => 115370)
--- branches/chromium/1084/LayoutTests/storage/indexeddb/pending-version-change-on-exit-expected.txt (rev 0)
+++ branches/chromium/1084/LayoutTests/storage/indexeddb/pending-version-change-on-exit-expected.txt 2012-04-26 21:36:45 UTC (rev 115370)
@@ -0,0 +1,12 @@
+No crashes when there are event listeners in a worker on shutdown
+
+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;
+
+PASS Didn't crash!
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Copied: branches/chromium/1084/LayoutTests/storage/indexeddb/pending-version-change-on-exit.html (from rev 113818, trunk/LayoutTests/storage/indexeddb/pending-version-change-on-exit.html) (0 => 115370)
--- branches/chromium/1084/LayoutTests/storage/indexeddb/pending-version-change-on-exit.html (rev 0)
+++ branches/chromium/1084/LayoutTests/storage/indexeddb/pending-version-change-on-exit.html 2012-04-26 21:36:45 UTC (rev 115370)
@@ -0,0 +1,42 @@
+<html>
+<head>
+<script src=""
+<script src=""
+</head>
+<body>
+<script>
+
+description("No crashes when there are event listeners in a worker on shutdown");
+
+function test() {
+ removeVendorPrefixes();
+ if (self.location.search == "?second") {
+ testPassed("Didn't crash!");
+ finishJSTest();
+ return;
+ }
+ dbname = "pending-version-change-on-exit";
+ evalAndLog("request = indexedDB.open(\"" + dbname + "\")");
+ request._onsuccess_ = startTheWorker;
+ request._onblocked_ = unexpectedBlockedCallback;
+ request._onerror_ = unexpectedErrorCallback;
+}
+
+function startTheWorker() {
+ var worker = startWorker("resources/pending-version-change-on-exit.js");
+ realFinishJSTest = finishJSTest;
+ worker._onerror_ = function(e) {
+ testFailed(e.message);
+ realFinishJSTest();
+ };
+ finishJSTest = function() {
+ self.location = self.location + "?second";
+ }
+}
+
+test();
+
+</script>
+<script src=""
+</body>
+</html>
Copied: branches/chromium/1084/LayoutTests/storage/indexeddb/resources/pending-version-change-on-exit.js (from rev 113818, trunk/LayoutTests/storage/indexeddb/resources/pending-version-change-on-exit.js) (0 => 115370)
--- branches/chromium/1084/LayoutTests/storage/indexeddb/resources/pending-version-change-on-exit.js (rev 0)
+++ branches/chromium/1084/LayoutTests/storage/indexeddb/resources/pending-version-change-on-exit.js 2012-04-26 21:36:45 UTC (rev 115370)
@@ -0,0 +1,21 @@
+if (this.importScripts) {
+ importScripts('../../../fast/js/resources/js-test-pre.js');
+ importScripts('shared.js');
+}
+
+function test() {
+ removeVendorPrefixes();
+ dbname = "pending-version-change-on-exit";
+ evalAndLog("request = indexedDB.open(\"" + dbname + "\")");
+ request._onsuccess_ = function(e) {
+ db = request.result;
+ evalAndLog("request = db.setVersion(1)");
+ request._onsuccess_ = unexpectedSuccessCallback;
+ request._onblocked_ = function() {
+ testPassed("worker received blocked event.");
+ finishJSTest();
+ };
+ };
+}
+
+test();
Modified: branches/chromium/1084/Source/WebCore/bindings/v8/WorkerScriptController.cpp (115369 => 115370)
--- branches/chromium/1084/Source/WebCore/bindings/v8/WorkerScriptController.cpp 2012-04-26 21:35:34 UTC (rev 115369)
+++ branches/chromium/1084/Source/WebCore/bindings/v8/WorkerScriptController.cpp 2012-04-26 21:36:45 UTC (rev 115370)
@@ -65,6 +65,12 @@
WorkerScriptController::~WorkerScriptController()
{
removeAllDOMObjects();
+#if PLATFORM(CHROMIUM)
+ // The corresponding call to didStartWorkerRunLoop is in
+ // WorkerThread::workerThread().
+ // See http://webkit.org/b/83104#c14 for why this is here.
+ PlatformSupport::didStopWorkerRunLoop(&m_workerContext->thread()->runLoop());
+#endif
m_proxy.clear();
m_isolate->Exit();
V8BindingPerIsolateData::dispose(m_isolate);
Modified: branches/chromium/1084/Source/WebCore/workers/WorkerThread.cpp (115369 => 115370)
--- branches/chromium/1084/Source/WebCore/workers/WorkerThread.cpp 2012-04-26 21:35:34 UTC (rev 115369)
+++ branches/chromium/1084/Source/WebCore/workers/WorkerThread.cpp 2012-04-26 21:36:45 UTC (rev 115370)
@@ -142,6 +142,8 @@
}
}
#if PLATFORM(CHROMIUM)
+ // The corresponding call to didStopWorkerRunLoop is in
+ // ~WorkerScriptController.
PlatformSupport::didStartWorkerRunLoop(&m_runLoop);
#endif
@@ -157,10 +159,6 @@
runEventLoop();
-#if PLATFORM(CHROMIUM)
- PlatformSupport::didStopWorkerRunLoop(&m_runLoop);
-#endif
-
ThreadIdentifier threadID = m_threadID;
ASSERT(m_workerContext->hasOneRef());
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
