Bug 107020 – [V8] Make a creationContext parameter of toV8() mandatory

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

[Kentaro Hara]

Currently a creationContext parameter of toV8() is optional:

  Handle<Value> toV8(..., Handle<Object> creationContext = Handle<Object>(), ...) {
    ...;
  }

On the other hand, looking at the implementation of V8WrapperInstantiationScope:

    explicit V8WrapperInstantiationScope(v8::Handle<v8::Object> creationContext)
        : m_didEnterContext(false)
        , m_context(v8::Context::GetCurrent())
    {
        if (creationContext.IsEmpty()) // We do nothing for this case!
            return;
        v8::Handle<v8::Context> contextForWrapper = creationContext->CreationContext();
        if (contextForWrapper == m_context)
            return;
        m_context = v8::Local<v8::Context>::New(contextForWrapper);
        m_didEnterContext = true;
        m_context->Enter();
    }

we're assuming that a creationContext parameter is passed to toV8() when a creationContext exists. In other words, if we forget to pass a creationContext parameter, we will end up with creating an object on the current context, which is wrong. To avoid the mistake, we should make the creationContext parameter mandatory. When we really do not have a creationContext, we can pass Handle<Object>() explicitly.

[Kentaro Hara]

Created attachment 182993 [details]
Patch

[Adam Barth]

Comment on attachment 182993 [details]
Patch

Yeah, we should really pass non-empty handles in all these places.

[WebKit Review Bot]

Comment on attachment 182993 [details]
Patch

Clearing flags on attachment: 182993

Committed r139900: <http://trac.webkit.org/changeset/139900>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.