[webkit-reviews] review granted: [Bug 196089] JSC::createError should clear exception thrown by errorDescriptionForValue : [Attachment 365566] Patch

[bugzilla-daemon at webkit.org]

Mark Lam <mark.lam at apple.com> has granted Tadeu Zagallo <tzagallo at apple.com>'s
request for review:
Bug 196089: JSC::createError should clear exception thrown by
errorDescriptionForValue
https://bugs.webkit.org/show_bug.cgi?id=196089

Attachment 365566: Patch

https://bugs.webkit.org/attachment.cgi?id=365566&action=review




--- Comment #2 from Mark Lam <mark.lam at apple.com> ---
Comment on attachment 365566
  --> https://bugs.webkit.org/attachment.cgi?id=365566
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=365566&action=review

r=me with fix.

> Source/JavaScriptCore/runtime/ExceptionHelpers.cpp:279
>      String valueDescription = errorDescriptionForValue(exec, value);
> -    if (!valueDescription)
> +    if (!valueDescription) {
> +	   scope.clearException();

I think it's a hack that we return an OOME when we fail to create the requested
Error object.  I suspect that this is a semantic error because:

    var error = new RangeError();
    error.toString()l // expects "RangeError", but may get "Out of memory".

Anyway, that was a pre-existing thing and can be fixed later.  For this patch,
you also need to placate the exception validator before the call to
tryMakeString() below.	The way to do this is to add the following before the
"if (!valueDescription)" above:
    ASSERT(scope.exception() || valueDescription);

The assertion tickles scope.exception(), thereby telling the validator that we
did something with the exception.  This placates the validator.  Note: the
assertion assumes that we will never return a null string unless there's no
exception.  If that's not the case, just do a real exception check instead.