[Webkit-unassigned] [Bug 64467] [V8] Avoid memory leaks with hidden references.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=64467


Adam Barth <abarth at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #100689|review?                     |review-
               Flag|                            |




--- Comment #3 from Adam Barth <abarth at webkit.org>  2011-07-13 11:23:34 PST ---
(From update of attachment 100689)
View in context: https://bugs.webkit.org/attachment.cgi?id=100689&action=review

Looks reasonable, but I'd like to see one more iteration with Vector and the namespace question questions addressed.

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:861
> -            push(@implContentDecls, "            V8DOMWrapper::setHiddenReference(info.Holder(), wrapper);\n");
> +            push(@implContentDecls, "            V8DOMWrapper::setNamedHiddenReference(info.Holder(), \"${attrName}\", wrapper);\n");

Do we want to put attribute names into their own namespace?

> Source/WebCore/bindings/v8/V8DOMWrapper.cpp:193
> +static const char hiddenPrefix[] = "WebCore::";

Don't we have this constant already in V8HiddenPropertyNames somewhere?

> Source/WebCore/bindings/v8/V8DOMWrapper.cpp:199
> +    char* prefixedName = new char[prefixLength + nameLength + 1];

Can we use Vector<char> instead?  All this bare-handed lifting looks wrong.  Vector<char> also can have an inline capacity, which means we can avoid the malloc in the common case of a short name.

> Source/WebCore/bindings/v8/custom/V8WebGLRenderingContextCustom.cpp:164
> +    const char* referenceName;

I'd initialize this to zero.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.