Allow recursive messages reporting as it is already used.

Allow recursive messages reporting as it is already used.

Instead discard unhandled exceptions thown while running
message listeners.

Committed: http://code.google.com/p/v8/source/detail?r=7574

[antonm, 2011-04-08 15:57:10]

Guys,

may you have a look?

Regression was detected by layout tests (but it wasn't for some reasonable
before :(

Many thanks to Yury for help!

[yurys, 2011-04-11 07:52:35]

http://codereview.chromium.org/6820003/diff/1/test/cctest/test-api.cc
File test/cctest/test-api.cc (left):

http://codereview.chromium.org/6820003/diff/1/test/cctest/test-api.cc#oldcode...
test/cctest/test-api.cc:8680: CompileRun("throw 'ThrowInJS';");
Can we rewrite this listener to make it reenterable? It would cover the scenario
when in WebKit window.onerror handler throws an exception.

[Mads Ager (chromium), 2011-04-11 11:39:29]

OK, LGTM

This takes it roughly back to what it was for this part, right?

[antonm, 2011-04-11 16:19:17]

thanks a lot for review, guys.  Landed.

http://codereview.chromium.org/6820003/diff/1/test/cctest/test-api.cc
File test/cctest/test-api.cc (left):

http://codereview.chromium.org/6820003/diff/1/test/cctest/test-api.cc#oldcode...
test/cctest/test-api.cc:8680: CompileRun("throw 'ThrowInJS';");
On 2011/04/11 07:52:36, Yury Semikhatsky wrote:
> Can we rewrite this listener to make it reenterable? It would cover the
scenario
> when in WebKit window.onerror handler throws an exception.

No doubt.

[Vitaly Repeshko, 2011-04-11 19:37:39]

LGTM

http://codereview.chromium.org/6820003/diff/6/src/messages.cc
File src/messages.cc (right):

http://codereview.chromium.org/6820003/diff/6/src/messages.cc#newcode134
src/messages.cc:134: v8::TryCatch tryCatch;
nit: noCamelCase

http://codereview.chromium.org/6820003/diff/6/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):

http://codereview.chromium.org/6820003/diff/6/test/cctest/test-api.cc#newcode...
test/cctest/test-api.cc:8683: if (--call_depth) CompileRun("throw
'ThrowInJS';");
In case we re-entered this message callback, should we assert that it got its
own exception?

http://codereview.chromium.org/6820003/diff/6/test/cctest/test-api.cc#newcode...
test/cctest/test-api.cc:8714: call_depth = 5;
5 is arbitrary here, right? In any case, it'd be nice to have a comment.

[antonm, 2011-04-15 12:15:34]

http://codereview.chromium.org/6875003 should address 2 of 3 your suggestions.

http://codereview.chromium.org/6820003/diff/6/src/messages.cc
File src/messages.cc (right):

http://codereview.chromium.org/6820003/diff/6/src/messages.cc#newcode134
src/messages.cc:134: v8::TryCatch tryCatch;
On 2011/04/11 19:37:39, Vitaly Repeshko wrote:
> nit: noCamelCase

Done.

http://codereview.chromium.org/6820003/diff/6/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):

http://codereview.chromium.org/6820003/diff/6/test/cctest/test-api.cc#newcode...
test/cctest/test-api.cc:8683: if (--call_depth) CompileRun("throw
'ThrowInJS';");
On 2011/04/11 19:37:39, Vitaly Repeshko wrote:
> In case we re-entered this message callback, should we assert that it got its
> own exception?

I don't think it's important for the given test.

http://codereview.chromium.org/6820003/diff/6/test/cctest/test-api.cc#newcode...
test/cctest/test-api.cc:8714: call_depth = 5;
On 2011/04/11 19:37:39, Vitaly Repeshko wrote:
> 5 is arbitrary here, right? In any case, it'd be nice to have a comment.

Done.