Bug 65004

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:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

61016

Attachments:

Description	Flags
Allocates v8::isolate per worker	levin: review+, levin: commit-queue-
CR feedback addressed	none

Dmitry Lomov

Reported 2011-07-21 18:26:21 PDT

Allocate a separate v8::Isolate workers.

Attachments
Allocates v8::isolate per worker (8.62 KB, patch) 2011-07-21 18:46 PDT, Dmitry Lomov	levin: review+ levin: commit-queue-	Details Formatted Diff Diff
CR feedback addressed (9.83 KB, patch) 2011-07-26 17:17 PDT, Dmitry Lomov	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Dmitry Lomov

Comment 1 2011-07-21 18:46:12 PDT

Created attachment 101685 [details] Allocates v8::isolate per worker

Dmitry Lomov

Comment 2 2011-07-21 18:46:39 PDT

Comment on attachment 101685 [details] Allocates v8::isolate per worker No cq yet - waiting on chromium trybots.

Dmitry Lomov

Comment 3 2011-07-21 18:47:18 PDT

Comment on attachment 101685 [details] Allocates v8::isolate per worker trybots happy.

David Levin

Comment 4 2011-07-26 13:22:24 PDT

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

David Levin

Comment 5 2011-07-26 13:23:42 PDT

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.

Dmitry Lomov

Comment 6 2011-07-26 17:17:45 PDT

Created attachment 102076 [details] CR feedback addressed

WebKit Review Bot

Comment 7 2011-07-26 19:41:28 PDT

Comment on attachment 102076 [details] CR feedback addressed Clearing flags on attachment: 102076 Committed r91814: <http://trac.webkit.org/changeset/91814>

WebKit Review Bot

Comment 8 2011-07-26 19:41:32 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.