Use TRYCATCH_FOR_V8STRINGRESOURCE in AbstractEventListener::invokeEventHandler()

Use TRYCATCH_FOR_V8STRINGRESOURCE in AbstractEventListener::invokeEventHandler()

Use TRYCATCH_FOR_V8STRINGRESOURCE in AbstractEventListener::invokeEventHandler() instead
of toWebCoreString() so that exceptions raised by ToString() are no longer ignored.
This affects the string returned by the onbeforeunload callback.

R=haraken
BUG=

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=157673

[do-not-use, 2013-09-11 09:00:42]

https://codereview.chromium.org/23600034/diff/1/LayoutTests/fast/events/befor...
File LayoutTests/fast/events/before-unload-return-bad-value.html (right):

https://codereview.chromium.org/23600034/diff/1/LayoutTests/fast/events/befor...
LayoutTests/fast/events/before-unload-return-bad-value.html:20:
setTimeout(finishJSTest, 0);
I am not quite sure why this setTimeout() is needed but the test times out
otherwise.

[haraken, 2013-09-11 09:08:43]

LGTM

https://codereview.chromium.org/23600034/diff/1/LayoutTests/fast/events/befor...
File LayoutTests/fast/events/before-unload-return-bad-value.html (right):

https://codereview.chromium.org/23600034/diff/1/LayoutTests/fast/events/befor...
LayoutTests/fast/events/before-unload-return-bad-value.html:20:
setTimeout(finishJSTest, 0);
On 2013/09/11 09:00:43, Christophe Dumez wrote:
> I am not quite sure why this setTimeout() is needed but the test times out
> otherwise.

Looks strange. arv, do you have any idea?

[arv (Not doing code reviews), 2013-09-11 13:42:18]

LGTM

https://codereview.chromium.org/23600034/diff/1/LayoutTests/fast/events/befor...
File LayoutTests/fast/events/before-unload-return-bad-value.html (right):

https://codereview.chromium.org/23600034/diff/1/LayoutTests/fast/events/befor...
LayoutTests/fast/events/before-unload-return-bad-value.html:11:
setTimeout(finishJSTest, 0);
Why is this needed? window.onerror calls finishJSTest already.

https://codereview.chromium.org/23600034/diff/1/LayoutTests/fast/events/befor...
LayoutTests/fast/events/before-unload-return-bad-value.html:12: }
semicolons after assignment.

https://codereview.chromium.org/23600034/diff/1/LayoutTests/fast/events/befor...
LayoutTests/fast/events/before-unload-return-bad-value.html:20:
setTimeout(finishJSTest, 0);
On 2013/09/11 09:08:44, haraken wrote:
> On 2013/09/11 09:00:43, Christophe Dumez wrote:
> > I am not quite sure why this setTimeout() is needed but the test times out
> > otherwise.
> 
> Looks strange. arv, do you have any idea?

I'm not sure. Maybe it is related to
setShouldStayOnPageAfterHandlingBeforeUnload somehow?

https://codereview.chromium.org/23600034/diff/1/LayoutTests/fast/events/befor...
LayoutTests/fast/events/before-unload-return-bad-value.html:32:
window.jsTestIsAsync = true;
move this to other script block, before if window.testRunner.

[do-not-use, 2013-09-11 14:12:28]

https://codereview.chromium.org/23600034/diff/1/LayoutTests/fast/events/befor...
File LayoutTests/fast/events/before-unload-return-bad-value.html (right):

https://codereview.chromium.org/23600034/diff/1/LayoutTests/fast/events/befor...
LayoutTests/fast/events/before-unload-return-bad-value.html:11:
setTimeout(finishJSTest, 0);
On 2013/09/11 13:42:19, arv wrote:
> Why is this needed? window.onerror calls finishJSTest already.

I had it in case it did not throw, to avoid a timeout and fail quickly. However,
now I noticed that if I remove this setTimeout() and keep only the second one,
the test times out...

For some reason, the test does not time out only if I keep BOTH setTimeout()
calls.

[arv (Not doing code reviews), 2013-09-11 14:16:53]

On 2013/09/11 14:12:28, Christophe Dumez wrote:
>
https://codereview.chromium.org/23600034/diff/1/LayoutTests/fast/events/befor...
> File LayoutTests/fast/events/before-unload-return-bad-value.html (right):
> 
>
https://codereview.chromium.org/23600034/diff/1/LayoutTests/fast/events/befor...
> LayoutTests/fast/events/before-unload-return-bad-value.html:11:
> setTimeout(finishJSTest, 0);
> On 2013/09/11 13:42:19, arv wrote:
> > Why is this needed? window.onerror calls finishJSTest already.
> 
> I had it in case it did not throw, to avoid a timeout and fail quickly.
However,
> now I noticed that if I remove this setTimeout() and keep only the second one,
> the test times out...
> 
> For some reason, the test does not time out only if I keep BOTH setTimeout()
> calls.

I would try it without js-test-pre/post and see if things work more reliable.

[do-not-use, 2013-09-11 14:39:07]

On 2013/09/11 14:16:53, arv wrote:
> On 2013/09/11 14:12:28, Christophe Dumez wrote:
> >
>
https://codereview.chromium.org/23600034/diff/1/LayoutTests/fast/events/befor...
> > File LayoutTests/fast/events/before-unload-return-bad-value.html (right):
> > 
> >
>
https://codereview.chromium.org/23600034/diff/1/LayoutTests/fast/events/befor...
> > LayoutTests/fast/events/before-unload-return-bad-value.html:11:
> > setTimeout(finishJSTest, 0);
> > On 2013/09/11 13:42:19, arv wrote:
> > > Why is this needed? window.onerror calls finishJSTest already.
> > 
> > I had it in case it did not throw, to avoid a timeout and fail quickly.
> However,
> > now I noticed that if I remove this setTimeout() and keep only the second
one,
> > the test times out...
> > 
> > For some reason, the test does not time out only if I keep BOTH setTimeout()
> > calls.
> 
> I would try it without js-test-pre/post and see if things work more reliable.

No, the behavior is unfortunately the same without js-test-pre/post...

[do-not-use, 2013-09-11 15:13:33]

https://codereview.chromium.org/23600034/diff/16001/LayoutTests/fast/events/b...
File LayoutTests/fast/events/before-unload-return-bad-value.html (right):

https://codereview.chromium.org/23600034/diff/16001/LayoutTests/fast/events/b...
LayoutTests/fast/events/before-unload-return-bad-value.html:15:
setTimeout(finishJSTest, 0);
I updated the test to use an iframe. This way, we no longer need to use
testRunner.setShouldStayOnPageAfterHandlingBeforeUnload().

Note however that the test still works only if I keep both setTimeout() calls. I
really don't get it.

[commit-bot: I haz the power, 2013-09-12 11:06:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@sisa.samsung.com/23600034/16001

[commit-bot: I haz the power, 2013-09-12 11:42:58]

Change committed as 157673