As commented here (https://bugs.webkit.org/show_bug.cgi?id=84074#c5), I am planning to refactor a series of confusing throwError()s. As a first step, I replace throwTypeError() with throwTypeError("Type error"). Actually I think we should use a more descriptive error message than "Type error", but to keep the behavior of JSC and V8 consistent, this patch just uses "Type error" for now.
Created attachment 142150 [details] Patch
Can we avoid repeating this string everywhere? Perhaps we should have a header that contains constants for these strings so we don't typo them somewhere.
(In reply to comment #2) > Can we avoid repeating this string everywhere? Perhaps we should have a header that contains constants for these strings so we don't typo them somewhere. Or we might want to make the 'message' argument optional, like throwTypeError(const char* message = 0). If omitted, it uses "Type error".
That's fine too.
Created attachment 142464 [details] Patch
Comment on attachment 142464 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=142464&action=review > Source/WebCore/bindings/v8/V8Proxy.cpp:614 > - return throwError(TypeError, "Type error"); > + return throwError(TypeError, (message ? message : "Type error")); You don't need the outer ( ) around the ? : operator.
Comment on attachment 142464 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=142464&action=review >> Source/WebCore/bindings/v8/V8Proxy.cpp:614 >> + return throwError(TypeError, (message ? message : "Type error")); > > You don't need the outer ( ) around the ? : operator. Yeah, but previously some apple guy told me to put () around ?: for clarification.
Comment on attachment 142464 [details] Patch Clearing flags on attachment: 142464 Committed r117448: <http://trac.webkit.org/changeset/117448>
All reviewed patches have been landed. Closing bug.
Re-opened since this is blocked by 86752
Sorry for the noise, the rollout was for r117449.