Summary: | [V8] Move WorkerContextExecutionProxy::initializeIfNeeded() to WorkerScriptController | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kentaro Hara <haraken> | ||||
Component: | WebCore JavaScript | Assignee: | Kentaro Hara <haraken> | ||||
Status: | RESOLVED DUPLICATE | ||||||
Severity: | Normal | CC: | abarth, japhet, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 97057 | ||||||
Attachments: |
|
Description
Kentaro Hara
2012-09-18 22:42:24 PDT
Created attachment 164662 [details]
Patch
Comment on attachment 164662 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=164662&action=review > Source/WebCore/bindings/v8/WorkerScriptController.h:104 > + v8::Persistent<v8::Context> m_context; > + OwnPtr<V8PerContextData> m_perContextData; > + bool m_disableEvalPending; > + Vector<Event*> m_events; On the main thread, this sort of stuff is handled by the WindowShell. I worry slightly about WorkerScriptController becoming a giant object of doom. Maybe we need a V8WorkerContext object to handle setting up and tearing down the v8::Context for the worker? Comment on attachment 164662 [details]
Patch
Removing r? as I want to move initialization methods to V8Initializer.{h,cpp} first.
Comment on attachment 164662 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=164662&action=review > Source/WebCore/bindings/v8/WorkerContextExecutionProxy.cpp:-149 > - // Setup the security handlers and message listener. This only has > - // to be done once. > - static bool isV8Initialized = false; > - if (!isV8Initialized) > - v8::V8::AddMessageListener(&v8MessageHandler); BTW this code looks really strange: - Per the comment, 'isV8Initialized = true' should be added. - However, if I add 'isV8Initialized = true', fast/workers/worker-script-error.html and fast/workers/worker-close.html begin to fail. Will take a look in detail later. Comment on attachment 164662 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=164662&action=review >> Source/WebCore/bindings/v8/WorkerContextExecutionProxy.cpp:-149 >> - v8::V8::AddMessageListener(&v8MessageHandler); > > BTW this code looks really strange: > > - Per the comment, 'isV8Initialized = true' should be added. > - However, if I add 'isV8Initialized = true', fast/workers/worker-script-error.html and fast/workers/worker-close.html begin to fail. > > Will take a look in detail later. I would assume we'd need to do it once per isolate rather than once across the entire process. *** This bug has been marked as a duplicate of bug 103210 *** |