[webkit-reviews] review granted: [Bug 110396] [v8] Fix an erroneous WrapperGrouper call in preparation for refactoring : [Attachment 189393] Patch

[bugzilla-daemon at webkit.org]

Kentaro Hara <haraken at chromium.org> has granted Adam Klein
<adamk at chromium.org>'s request for review:
Bug 110396: [v8] Fix an erroneous WrapperGrouper call in preparation for
refactoring
https://bugs.webkit.org/show_bug.cgi?id=110396

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

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


>>> Source/WebCore/bindings/v8/V8GCController.cpp:314
>>> +		    
m_grouper.addObjectToGroup(V8GCController::opaqueRootForGC(*it, m_isolate),
wrapper);
>> 
>> *it is a Node, and opaqueRootForGC() returns a Node, so I'm confused. Would
you elaborate on why you want to make this change?
> 
> Note that all the analogous calls to this one are from the CodeGenerator, in
GenerateOpaqueRootForGC, and the return value of that is always passed to
addObjectToGroup() (by V8GCController). Adam Barth may be able to shine more
light on this, but it appears to me that "addNodeToGroup" refers to the wrapper
being a Node wrapper, not the root being a Node (though that's definitely not
obvious from the type signatures of the functions, as you point out).

Thanks, understood. Makes sense to me.

I'll rename add{Node,Object}ToGroup to add{Node,Object}WrapperToGroup in a
follow-up patch.