[webkit-reviews] review granted: [Bug 108579] [V8] Remove V8GCController::m_edenNodes and make minor DOM GC more precise : [Attachment 186506] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Kentaro Hara
<haraken at chromium.org>'s request for review:
Bug 108579: [V8] Remove V8GCController::m_edenNodes and make minor DOM GC more
precise
https://bugs.webkit.org/show_bug.cgi?id=108579

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

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=186506&action=review


> Source/WebCore/bindings/v8/DOMWrapperWorld.h:74
> +    static DOMWrapperWorld*
getWorldWithoutContextCheck(v8::Handle<v8::Context> context)

Can you add a comment about when you'd wan to call getWorldWithoutContextCheck
instead of getWorld()/isolated() ?

> Source/WebCore/bindings/v8/V8Binding.h:455
> +	   v8::Handle<v8::Context> context = v8::Context::GetEntered();

Is this safe to call if !InContext()?  I guess so if we pass all the tests.

>> Source/WebCore/bindings/v8/V8Binding.h:467
>>	}
> 
> abarth: I just changed these lines, which I think fixed the test failures.
Given that we are using an entered context, we have to do an empty check for
the entered context instead of InContext().

In what situations does InContext return true but GetEntered returns an empty
context?  That seems very odd.