[webkit-reviews] review denied: [Bug 78210] Add support for unsigned long[] to IDL bindings to JSC. : [Attachment 127325] Fixed patch.

[bugzilla-daemon at webkit.org]

Kentaro Hara <haraken at chromium.org> has denied Kihong Kwon
<kihong.kwon at samsung.com>'s request for review:
Bug 78210: Add support for unsigned long[] to IDL bindings to JSC.
https://bugs.webkit.org/show_bug.cgi?id=78210

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

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


> Source/WebCore/bindings/js/JSDOMBinding.h:350
> +	   if (!object)

"if (exec->hadException())" might be better. If "!object" is true, then
"exec->hadException()" is true.

> Source/WebCore/bindings/js/JSDOMBinding.h:354
> +	   JSC::JSValue indexedValue;

Nit: You can move this declaration into the loop.

> Source/WebCore/bindings/js/JSDOMBinding.h:357
> +	       if (indexedValue == JSC::JSValue(JSC::JSValue::JSUndefined) ||
!indexedValue.isNumber())

Maybe "if (exec->hadException() || indexedValue.isUndefinedOrNull() ||
!indexedValue.isUInt32())"?

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:2899
> +	   return "jsUnsignedLongArrayToVector<unsigned long>(exec, $value)";

This should be "jsUnsignedLongArrayToVector(exec, $value)", since the function
is not a template.

> Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp:2144
> +    Vector<unsigned long>
unsignedLongArray(jsUnsignedLongArrayToVector<unsigned long>(exec,
MAYBE_MISSING_PARAMETER(exec, 0, DefaultIsUndefined)));

Ditto.