[webkit-reviews] review granted: [Bug 97955] Fix compilation of V8DependentRetained and JSDependentRetained. : [Attachment 166337] Patch

[bugzilla-daemon at webkit.org]

Kentaro Hara <haraken at chromium.org> has granted Elliott Sprehn
<esprehn at chromium.org>'s request for review:
Bug 97955: Fix compilation of V8DependentRetained and JSDependentRetained.
https://bugs.webkit.org/show_bug.cgi?id=97955

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

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


>>> Source/WebCore/ChangeLog:9
>>> +	     incorrect assumptions about how weak handles work in V8. This is
refactored
>> 
>> The point of the V8 fix is:
>> 
>> - Use m_owner.set() instead of m_owern =.
>> - Call value->release() in the weak callback.
>> 
>> Am I understanding correctly?
> 
> - Yeah, need to use set() since ScopedPersistent doesn't have overloaded
operator=.
> - I originally had one weakCallback and abarth suggested using two callbacks,
one for the owner and one for the value and asserting about the order,	but
that was a wrong thing to do since the order of the callbacks doesn't seem to
work as expected when v8 destroys everything (whole page navigate). No code
uses this code yet, but if you did you'd get asserts.

Makes sense. Thanks for the clarification!