[webkit-dev] WebCore: Exposing exceptions through WebFrameImpl::executeScriptAndReturnValue and ScriptController::executeScript.

[Yury Semikhatsky]

On Wed, Dec 22, 2010 at 8:28 PM, Tom Rathbone <tom.rathbone at gmail.com>wrote:

> Sounds like this conversion might be being done in the wrong place, it
> doesn't feel like this should be the responsibility of this method
> since "undefined" is a valid return value from any JavaScript call.
>
> I agree, this looks like a bug and proper fix would be to remove this
conversion
from that place. We need to double check that this won't break Chromium's
code.




> On Wed, Dec 22, 2010 at 4:41 PM, Yury Semikhatsky <yurys at chromium.org>
> wrote:
> >
> >
> > On Wed, Dec 22, 2010 at 7:22 PM, Tom Rathbone <tom.rathbone at gmail.com>
> > wrote:
> >>
> >> From my experiments calling executeScriptAndReturnValue "function
> >> foo() { } foo();" will also return an empty handle meaning that this
> >> can't reliably be used to detect errors.  Is there another way to
> >> distinguish non values from errors?
> >
> > I see, the problem is that 'undefined' value is converted into an empty
> > ScriptValue in ScriptController::evaluate:
> >
> http://codesearch.google.com/codesearch/p?hl=en#OAMlx_jo-ck/src/third_party/WebKit/WebCore/bindings/v8/ScriptController.cpp&q=::executeScriptAndReturnValue%5C(&gs=cpp:WebCore::ScriptController::executeScript(const%2520WebCore::ScriptSourceCode%2520&amp
> ;,%2520enum%2520ShouldAllowXSS)@chrome/trunk/src/third_party/WebKit/WebCore/bindings/v8/ScriptController.h%257Cdecl&gsn=executeScript&d=5&l=260
> > Not sure what this conversion is used for.
> > Yury
> >
> >
> >>
> >> On Wed, Dec 22, 2010 at 2:35 PM, Yury Semikhatsky <yurys at chromium.org>
> >> wrote:
> >> > Hi Tom,
> >> > In case of uncaught JS exception returned v8::Handle will always be
> >> > empty
> >> > (v8::Handle<v8::Value>::IsEmpty() will return true) while in case of
> >> > successful
> >> > evaluation the result is always non-empty. Also uncaught JS exception
> >> > will
> >> > be
> >> > reported to the console object(see Console.cpp) which will propagate
> >> > them
> >> > to
> >> > InspectorController and ChromeClient.
> >> > Thanks,
> >> > Yury
> >> >
> >> > On Wed, Dec 22, 2010 at 12:58 PM, Tom Rathbone <
> tom.rathbone at gmail.com>
> >> > wrote:
> >> >>
> >> >> Hi Guys,
> >> >>
> >> >> I was wondering if it would be possible to expose v8 exceptions
> >> >> through WebFrameImpl::executeScriptAndReturnValue and
> >> >> ScriptController::executeScript, perhaps as an output reference
> >> >> argument.
> >> >>
> >> >> The effected methods would be:
> >> >>
> >> >> v8::Handle<v8::Value> WebFrameImpl::executeScriptAndReturnValue(const
> >> >> WebScriptSource& source)
> >> >> ScriptValue ScriptController::executeScript(const ScriptSourceCode&
> >> >> sourceCode, ShouldAllowXSS shouldAllowXSS)
> >> >> ScriptValue ScriptController::evaluate(const ScriptSourceCode&
> >> >> sourceCode, ShouldAllowXSS shouldAllowXSS)
> >> >> v8::Local<v8::Value> V8Proxy::evaluate(const ScriptSourceCode&
> source,
> >> >> Node* node)
> >> >>
> >> >> If I've read the source correctly V8 exceptions are caught in
> >> >> V8Proxy::evaluate and swallowed.  This means clients calling
> >> >> executeScriptAndReturnValue have no easy way of determining what went
> >> >> wrong or of responding to script errors.
> >> >>
> >> >> Maybe...
> >> >>
> >> >> v8::Handle<v8::Value> WebFrameImpl::executeScriptAndReturnValue(const
> >> >> WebScriptSource& source, bool& exceptionReturned)
> >> >>
> >> >> If others think these changes are worthwhile then I'll have a go at
> >> >> crafting a patch.
> >> >>
> >> >> Thanks,
> >> >>
> >> >> Tom.
> >> >> _______________________________________________
> >> >> webkit-dev mailing list
> >> >> webkit-dev at lists.webkit.org
> >> >> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
> >> >
> >> >
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20101222/199977ec/attachment.html>