Summary: | [V8][Chromium] Run workers in a separate v8::Isolate | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dmitry Lomov <dslomov> | ||||||
Component: | WebCore Misc. | Assignee: | Dmitry Lomov <dslomov> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, antonm, dimich, dslomov, levin, vitalyr, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 61016 | ||||||||
Attachments: |
|
Description
Dmitry Lomov
2011-07-21 18:26:21 PDT
Created attachment 101685 [details]
Allocates v8::isolate per worker
Comment on attachment 101685 [details]
Allocates v8::isolate per worker
No cq yet - waiting on chromium trybots.
Comment on attachment 101685 [details]
Allocates v8::isolate per worker
trybots happy.
Comment on attachment 101685 [details] Allocates v8::isolate per worker View in context: https://bugs.webkit.org/attachment.cgi?id=101685&action=review > Source/WebCore/ChangeLog:9 > + No new tests. (OOPS!) This should either say what test were added or why test can't be written or what test cover it or that no new functionality is exposed so no new tests are needed. > Source/WebCore/bindings/v8/V8Binding.h:112 > DOMDataList& allStores() { return m_DOMDataList; } Add blank line. > Source/WebCore/bindings/v8/V8Binding.h:120 > + m_DOMDataList.remove(m_DOMDataList.find(domDataStore)); What happens if domDataStore isn't in there? > Source/WebCore/bindings/v8/WorkerScriptController.cpp:59 > + data->allStores().append(&m_DOMDataStore); consider swapping these two lines. (It seems like setDOMDataStore should assert that what it is given is in the allStores.) > Source/WebCore/bindings/v8/WorkerScriptController.h:82 > + ScopedDOMDataStore m_DOMDataStore; m_domDataStore Comment on attachment 101685 [details] Allocates v8::isolate per worker View in context: https://bugs.webkit.org/attachment.cgi?id=101685&action=review >> Source/WebCore/bindings/v8/V8Binding.h:120 >> + m_DOMDataList.remove(m_DOMDataList.find(domDataStore)); > > What happens if domDataStore isn't in there? Please add an assert here. Created attachment 102076 [details]
CR feedback addressed
Comment on attachment 102076 [details] CR feedback addressed Clearing flags on attachment: 102076 Committed r91814: <http://trac.webkit.org/changeset/91814> All reviewed patches have been landed. Closing bug. |