Bug 65004 – [V8][Chromium] Run workers in a separate v8::Isolate

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Dmitry Lomov]

Allocate a separate v8::Isolate workers.

[Dmitry Lomov]

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

[Dmitry Lomov]

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

No cq yet - waiting on chromium trybots.

[Dmitry Lomov]

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

trybots happy.

[David Levin]

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 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]

Created attachment 102076 [details]
CR feedback addressed

[WebKit Review Bot]

Comment on attachment 102076 [details]
CR feedback addressed

Clearing flags on attachment: 102076

Committed r91814: <http://trac.webkit.org/changeset/91814>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.