[webkit-reviews] review granted: [Bug 61482] [V8] Multiple NPObjects can be created from a single v8::Object : [Attachment 95036] patch #2

[bugzilla-daemon at webkit.org]

Darin Fisher (:fishd, Google) <fishd at chromium.org> has granted Nate Chapin
<japhet at chromium.org>'s request for review:
Bug 61482: [V8] Multiple NPObjects can be created from a single v8::Object
https://bugs.webkit.org/show_bug.cgi?id=61482

Attachment 95036: patch #2
https://bugs.webkit.org/attachment.cgi?id=95036&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=95036&action=review

R=me (deferring to Anton's LGTM)

>> Source/WebCore/bindings/v8/NPV8Object.cpp:151
>> +	    return reinterpret_cast<NPObject*>(v8npObject);
> 
> I am not sure it's safe from point of view of the Holy Standard.  And I would
definitely prefer &(v8npObject->object) which (IMHO) communicates intent ways
more clear.
> 
> Feel free to ignore though.

This is a good point.  Although it seems like the code is already using
reinterpret_cast like this in other places.  Maybe a cleanup CL?  It
would probably also be nice to create toNPObject / toV8NPObject helpers.