[webkit-reviews] review denied: [Bug 45143] V8/JS bindings should not perform type checks if the parameter has Callback attribute : [Attachment 66450] Patch

[bugzilla-daemon at webkit.org]

Dumitru Daniliuc <dumi at chromium.org> has denied Kinuko Yasuda
<kinuko at chromium.org>'s request for review:
Bug 45143: V8/JS bindings should not perform type checks if the parameter has
Callback attribute
https://bugs.webkit.org/show_bug.cgi?id=45143

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

------- Additional Comments from Dumitru Daniliuc <dumi at chromium.org>
> WebCore/bindings/scripts/CodeGeneratorJS.pm:1137
> +	   if ($codeGenerator->IsStringType($type)) {
i think it might be cleaner to format this code as:

if (IsStringType($type)) {
  ...
} elsif ($parameter->extendedAttributes->{"Callback"}) {
  ...
} elsif (!IsNativeType($type)) {
  ...
}

same comment for CodeGeneratorV8.pm.


> WebCore/bindings/scripts/test/V8/V8TestObj.cpp:-981
> -    if ((args.Length() == 2 && (args[0]->IsNull() ||
V8TestObj::HasInstance(args[0])) && (args[1]->IsNull() ||
args[1]->IsUndefined() || args[1]->IsString() || args[1]->IsObject())))
i believe you need to change JS/JSTestObj.cpp too.