[webkit-reviews] review granted: [Bug 94882] [V8] V8DOMWindowShell should use ScopedPersistent : [Attachment 160323] Patch

[bugzilla-daemon at webkit.org]

Kentaro Hara <haraken at chromium.org> has granted Adam Barth
<abarth at webkit.org>'s request for review:
Bug 94882: [V8] V8DOMWindowShell should use ScopedPersistent
https://bugs.webkit.org/show_bug.cgi?id=94882

Attachment 160323: Patch
https://bugs.webkit.org/attachment.cgi?id=160323&action=review

------- Additional Comments from Kentaro Hara <haraken at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=160323&action=review


> Source/WebCore/bindings/v8/V8DOMWindowShell.cpp:200
> +    ASSERT(m_context.get().IsEmpty() || !m_global.get().IsEmpty());

Shall we implement ScopedContext::isEmpty() to avoid m_context.get().IsEmpty()
repeatedly?

> Source/WebCore/bindings/v8/V8DOMWindowShell.cpp:-201
> -#ifndef NDEBUG
> -	   V8GCController::unregisterGlobalHandle(this, m_global);
> -#endif

Are you intending to remove this?

> Source/WebCore/bindings/v8/V8DOMWindowShell.cpp:464
> +    m_context.get()->Global()->ForceSet(v8::String::New("document"),
documentWrapper, static_cast<v8::PropertyAttribute>(v8::ReadOnly |
v8::DontDelete));

Shall we implement ScopedPersistent::operator-> so that we can write this like
m_context->Global() ?