Bug 106771 – [V8] Add m_isolate to ScriptController, WorkerScriptController and V8DOMWindowShell

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Kentaro Hara]

Add m_isolate to ScriptController, WorkerScriptController and V8DOMWindowShell.

[Kentaro Hara]

Created attachment 182533 [details]
Patch

[Adam Barth]

Comment on attachment 182533 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=182533&action=review

> Source/WebCore/bindings/v8/ScriptController.cpp:105
> +    , m_isolate(v8::Isolate::GetCurrent())

Will the isolate have been created by then?  I guess the tests will tell us!  :)

> Source/WebCore/bindings/v8/ScriptController.h:202
> +    v8::Isolate* m_isolate;

Can we hold onto the isolate forever?

[Kentaro Hara]

Comment on attachment 182533 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=182533&action=review

>> Source/WebCore/bindings/v8/ScriptController.cpp:105
>> +    , m_isolate(v8::Isolate::GetCurrent())
> 
> Will the isolate have been created by then?  I guess the tests will tell us!  :)

Tests are passing. As far as I see, 'Isolate::New()' is found only on WorkerScriptController's constructor. Maybe the Isolate for the main thread is already initialized before ScriptController's constructor (I don't yet understand exactly).

>> Source/WebCore/bindings/v8/ScriptController.h:202
>> +    v8::Isolate* m_isolate;
> 
> Can we hold onto the isolate forever?

As long as a ScriptController is alive, a corresponding Isolate should be alive. So this is safe, isn't it?

[WebKit Review Bot]

Comment on attachment 182533 [details]
Patch

Clearing flags on attachment: 182533

Committed r139610: <http://trac.webkit.org/changeset/139610>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.