[webkit-reviews] review denied: [Bug 24812] Add custom V8 bindings for HTMLDocument : [Attachment 28943] patch

[bugzilla-daemon at webkit.org]

Dimitri Glazkov (Google) <dglazkov at chromium.org> has denied Mike Belshe
<mike at belshe.com>'s request for review:
Bug 24812: Add custom V8 bindings for HTMLDocument
https://bugs.webkit.org/show_bug.cgi?id=24812

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

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
Looks good, except for:

Add bug URL in ChangeLog. I usually put it two line-breaks after Reviewed by
...

> +CALLBACK_FUNC_DECL(HTMLDocumentOpen) {

Brace on new line.

> +	       if (context.IsEmpty()) return v8::Undefined();

return on new line.

> +	       if (!function->IsFunction()) {
> +		   V8Proxy::ThrowError(V8Proxy::TYPE_ERROR, "open is not a
function");
> +		   return v8::Undefined();
> +	       }

use throwError helper from V8Proxy.h

> +    RefPtr<HTMLCollection> v = WTF::getPtr(imp->all());
> +    return V8Proxy::ToV8Object(V8ClassIndex::HTMLCOLLECTION,
WTF::getPtr(v));

v -> collection?