Bug 100853 – [V8] Cleanup V8DOMWrapper::getCachedWrapper

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Dan Carney]

[V8] Cleanup V8DOMWrapper::getCachedWrapper

[Dan Carney]

Created attachment 171643 [details]
Patch

[Kentaro Hara]

Comment on attachment 171643 [details]
Patch

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

> Source/WebCore/bindings/v8/V8DOMWrapper.h:-115
> -                v8::Persistent<v8::Object> wrapper = node->wrapper();
> -                if (LIKELY(!wrapper.IsEmpty()))

Why is this branch redundant? This branch will fail and has to fall back to the following slow path in a case where we are in the main world and a wrapper has not yet created for a given node, won't it?

[Adam Barth]

Comment on attachment 171643 [details]
Patch

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

>> Source/WebCore/bindings/v8/V8DOMWrapper.h:-115
>> -                if (LIKELY(!wrapper.IsEmpty()))
> 
> Why is this branch redundant? This branch will fail and has to fall back to the following slow path in a case where we are in the main world and a wrapper has not yet created for a given node, won't it?

You're right Kentaro, but this function is only responsible for getting the cached wrapper.  The caller of this function will create the wrapper if this function returns an empty handle.

[Kentaro Hara]

(In reply to comment #3)
> > Why is this branch redundant? This branch will fail and has to fall back to the following slow path in a case where we are in the main world and a wrapper has not yet created for a given node, won't it?
> 
> You're right Kentaro, but this function is only responsible for getting the cached wrapper.  The caller of this function will create the wrapper if this function returns an empty handle.

Makes sense. Thanks.

[WebKit Review Bot]

Comment on attachment 171643 [details]
Patch

Clearing flags on attachment: 171643

Committed r133734: <http://trac.webkit.org/changeset/133734>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.