Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(110)

Issue 10232019: Merge 113818 - Notify observers of WorkerRunLoop stopping before the V8 isolate dies. (Closed)

Created:
8 years, 8 months ago by dgrogan
Modified:
8 years, 8 months ago
Reviewers:
dgrogan
CC:
chromium-reviews
Base URL:
http://svn.webkit.org/repository/webkit/branches/chromium/1084/
Visibility:
Public.

Description

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): TBR=dgrogan@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=115370

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -1 line) Patch
A + LayoutTests/storage/indexeddb/pending-version-change-on-exit.html View 0 chunks +-1 lines, --1 lines 0 comments Download
A + LayoutTests/storage/indexeddb/pending-version-change-on-exit-expected.txt View 0 chunks +-1 lines, --1 lines 0 comments Download
A + LayoutTests/storage/indexeddb/resources/pending-version-change-on-exit.js View 0 chunks +-1 lines, --1 lines 0 comments Download
M Source/WebCore/bindings/v8/WorkerScriptController.cpp View 1 chunk +6 lines, -0 lines 0 comments Download
M Source/WebCore/workers/WorkerThread.cpp View 2 chunks +2 lines, -4 lines 0 comments Download

Messages

Total messages: 1 (0 generated)
dgrogan
8 years, 8 months ago (2012-04-26 21:35:36 UTC) #1

          

Powered by Google App Engine
This is Rietveld 408576698