[webkit-reviews] review denied: [Bug 24114] ESMP(ECMAScript Mobile Profile) not supported : [Attachment 41853] patch for ESMP exception.code support

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Robin Qiu
<robin.qiu at torchmobile.com.cn>'s request for review:
Bug 24114: ESMP(ECMAScript Mobile Profile) not supported
https://bugs.webkit.org/show_bug.cgi?id=24114

Attachment 41853: patch for ESMP exception.code support
https://bugs.webkit.org/attachment.cgi?id=41853&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
It's not right to use a single bug for all ESMP changes. We need to use
separate bug report for each feature.

> +#if ENABLE(ESMP)
> +    int code = 0;
> +    if (name == "RangeError")
> +	   code = 101;
> +    else if (name == "ReferenceError")
> +	   code = 102;
> +    else if (name == "SyntaxError")
> +	   code = 103;
> +    else if (name == "TypeError")
> +	   code = 104;
> +    else if (name == "URIError")
> +	   code = 105;
> +    else if (name == "EvalError")
> +	   code = 106;
> +    else if (name == "MemoryError")
> +	   code = 99;
> +    putDirect(Identifier(exec, "code"), jsNumber(exec, code), 0);
> +#endif

This is an inefficient way to do this operation. Doing 7 string comparisons to
accomplish this doesn't make sense.

I suggest putting this into Error.cpp's Error::create function instead of doing
what you're doing here.

It seems quite strange to make ESMP a compile-time switch. Is this really the
right way to do this? Who would use the ESMP version in what context? I'd like
to know more before taking more of these patches.