[webkit-reviews] review denied: [Bug 103404] [V8] Replace toWebCoreString() in custom bindings with V8StringResource : [Attachment 176246] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has denied Kentaro Hara <haraken at chromium.org>'s
request for review:
Bug 103404: [V8] Replace toWebCoreString() in custom bindings with
V8StringResource
https://bugs.webkit.org/show_bug.cgi?id=103404

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

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=176246&action=review


> Source/WebCore/bindings/v8/custom/V8BlobCustom.cpp:115
> -	       V8TRYCATCH(String, stringValue, toWebCoreString(item));
> +	       V8TRYCATCH_FOR_V8STRINGRESOURCE(V8StringResource<>, stringValue,
item);

We should be able to write a test for this.  We just need to pass an argument
with a toString function that throws an exception.

> Source/WebCore/bindings/v8/custom/V8CanvasRenderingContext2DCustom.cpp:83
> +	   V8TRYCATCH_FOR_V8STRINGRESOURCE_VOID(V8StringResource<>,
propertyValue, value);

This is again the case where IsString() is true.  I think we want an optimized
path that doesn't generate code for a TryCatch that will never catch anything.

> Source/WebCore/bindings/v8/custom/V8ClipboardCustom.cpp:77
> -    String type = toWebCoreString(args[0]);
> +    V8TRYCATCH_FOR_V8STRINGRESOURCE(V8StringResource<>, type, args[0]);

This change ought to be testable.

> Source/WebCore/bindings/v8/custom/V8DOMFormDataCustom.cpp:63
> -    String name = toWebCoreStringWithNullCheck(args[0]);
> +    V8TRYCATCH_FOR_V8STRINGRESOURCE(V8StringResource<WithNullCheck>, name,
args[0]);

This ought to be testable.

> Source/WebCore/bindings/v8/custom/V8DOMFormDataCustom.cpp:73
> +	       V8TRYCATCH_FOR_V8STRINGRESOURCE(V8StringResource<WithNullCheck>,
stringResource, args[2]);

ditto

> Source/WebCore/bindings/v8/custom/V8DOMFormDataCustom.cpp:79
> +	   V8TRYCATCH_FOR_V8STRINGRESOURCE(V8StringResource<WithNullCheck>,
stringResource, arg);

ditto