[webkit-reviews] review denied: [Bug 78210] Add support for unsigned long[] to IDL bindings to JSC. : [Attachment 127292] Patch.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Feb 15 19:16:57 PST 2012
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 127292: Patch.
https://bugs.webkit.org/attachment.cgi?id=127292&action=review
------- Additional Comments from Kentaro Hara <haraken at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=127292&action=review
> Source/WebCore/ChangeLog:7
> +
Please describe what this patch is doing.
> Source/WebCore/bindings/js/JSDOMBinding.h:360
> + result.append(static_cast<T>(indexedValue.toNumber(exec)));
I am afraid that this conversion rule is wrong with the Web IDL spec:
http://dev.w3.org/2006/webapi/WebIDL/#es-unsigned-long
It might be difficult to make such a template<class T> for all types due to
complicated conversion rules in the spec. For now, maybe we can write
jsUnsignedLongArrayToVector() for unsigned long[] only.
> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:281
> + } elsif ($type eq "unsigned long[]") {
We want to avoid such hard-coding, but hard-coding might be a modest solution
in this case. OK.
> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:2899
> + return "jsNumberArrayToVector<unsigned long>(exec, $value)";
I cannot find any "jsNumberArrayToVector()" in your run-bindings-tests
results...
> Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp:45
> +#include "JSunsigned long[].h"
This is wrong.
> Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp:55
> +#include "unsigned long[].h"
This is wrong.
>
Source/WebCore/bindings/scripts/test/JS/JSTestSerializedScriptValueInterface.cp
p:72
> + putDirect(exec->globalData(), exec->propertyNames().length, jsNumber(2),
ReadOnly | DontDelete | DontEnum);
This is not the change caused by your patch. Maybe you need to rebaseline
run-bindings-tests results before making your patch, if the results are not
correct on trunk. (Sometimes people forget to update run-bindings-tests
results.)
More information about the webkit-reviews
mailing list