Summary: | [V8] Replace toWebCoreString() in custom bindings with V8StringResource | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kentaro Hara <haraken> | ||||
Component: | WebCore JavaScript | Assignee: | Kentaro Hara <haraken> | ||||
Status: | RESOLVED WONTFIX | ||||||
Severity: | Normal | CC: | abarth, andersca, japhet, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 103331 | ||||||
Attachments: |
|
Description
Kentaro Hara
2012-11-27 05:40:43 PST
Created attachment 176246 [details]
Patch
If the patch looks OK, I'll replace the rest. 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 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:-) V8 is gone from WebKit. |