Bug 86576 – [V8][Refactoring] Support an optional 'message' argument for throwTypeError()

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

[Kentaro Hara]

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.

[Kentaro Hara]

Created attachment 142150 [details]
Patch

[Adam Barth]

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.

[Kentaro Hara]

(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".

[Adam Barth]

That's fine too.

[Kentaro Hara]

Created attachment 142464 [details]
Patch

[Adam Barth]

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.

[Kentaro Hara]

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.

[WebKit Review Bot]

Comment on attachment 142464 [details]
Patch

Clearing flags on attachment: 142464

Committed r117448: <http://trac.webkit.org/changeset/117448>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

Re-opened since this is blocked by 86752

[Raphael Kubo da Costa (:rakuco)]

Sorry for the noise, the rollout was for r117449.