[Webkit-unassigned] [Bug 72484] [v8] Exception thrown in npObjectInvokeImpl may overwrite the exception message thrown by NPN_SetException

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=72484


Nate Chapin <japhet at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #116897|review?                     |review-
               Flag|                            |




--- Comment #10 from Nate Chapin <japhet at chromium.org>  2011-11-29 09:01:55 PST ---
(From update of attachment 116897)
View in context: https://bugs.webkit.org/attachment.cgi?id=116897&action=review

> Source/WebCore/ChangeLog:21
> +        No new tests because it depends on NPN_SetException implementation and
> +        JS engine used in browser. For v8 engine, it is covered by the test in
> +        http://codereview.chromium.org/8576001/ 

This should be testable using TestNetscapePlugin (See http://trac.webkit.org/browser/trunk/Tools/DumpRenderTree/TestNetscapePlugIn/PluginObject.cpp#L1008).

> Source/WebCore/bindings/v8/V8NPObject.cpp:144
> +    if (!retval) {
> +        // If an exception is already thrown by invoke/invokeDefault/construct,
> +        // native method writter is allowed to use the result as an indicator
> +        // to tell v8 engine that there is already a pending exception by 
> +        // setting it as boolean type with true value. Under this case, the
> +        // general exception will not be thrown again.
> +        if (NPVARIANT_IS_BOOLEAN(result) && NPVARIANT_TO_BOOLEAN(result))
> +          VOID_TO_NPVARIANT(result); // Restore back to VOID type
> +        else
> +          throwError("Error calling method on NPObject.", V8Proxy::GeneralError);
> +    }

This seems dangerous, it's assigning a deep meaning to a boolean result of true, and I'm betting it will cause regressions. There has got to be a better way catch this case (possibly using ExceptionCatcher? I don't know much about it, but it looks like it might be relevant)

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.