Bug 103404 – [V8] Replace toWebCoreString() in custom bindings with V8StringResource

• Status: RESOLVED
• Resolution: WONTFIX
• Component: WebCore JavaScript

[Kentaro Hara]

This is an incremental step for fixing bug 103331.

[Kentaro Hara]

Created attachment 176246 [details]
Patch

[Kentaro Hara]

If the patch looks OK, I'll replace the rest.

[Adam Barth]

Comment on attachment 176246 [details]
Patch

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

[Kentaro Hara]

Adding test cases makes sense. I'd like to do this work at some point, but this is not the highest priority task for now. It would require a bunch of efforts to replace 130~ places adding test cases and aligning the behavior with JSC in sync. (If someone is interested in this work, feel free to steal it:-)

[Anders Carlsson]

V8 is gone from WebKit.