[webkit-reviews] review granted: [Bug 95519] Add new V8DependentRetained that allows keeping a v8::Object alive as long as another v8::Object is alive : [Attachment 161804] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Sep 1 01:07:29 PDT 2012


Adam Barth <abarth at webkit.org> has granted Elliott Sprehn
<esprehn at chromium.org>'s request for review:
Bug 95519: Add new V8DependentRetained that allows keeping a v8::Object alive
as long as another v8::Object is alive
https://bugs.webkit.org/show_bug.cgi?id=95519

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

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


This looks great.  One nit about using the preprocessor.

> Source/WebCore/bindings/v8/V8DependentRetained.h:43
> +#define V8_DEPENDENT_RETAINED "V8DependentRetained"

Generally we prefer to avoid using the preprocessor for constants.  Perhaps
this can be a local constant in createPropertyName?

> Source/WebCore/bindings/v8/V8DependentRetained.h:56
> +	   m_owner.get().MakeWeak(this, &V8DependentRetained::weakCallback);
> +	   m_value.get().MakeWeak(this, &V8DependentRetained::weakCallback);

It seems like we'll always get the weak callback from m_owner first because no
one else is going to drop the reference from m_owner to m_value.


More information about the webkit-reviews mailing list