On 2014/09/23 13:32:06, Yang wrote:
On 2014/09/23 12:13:17, Jens Widell wrote:
> This CL works around the test failure:
> https://codereview.chromium.org/592293003/
>
> Note though that the way it does that is by completely eliminating the
> v8::TryCatch::ReThrow() usage that the test is supposed to be testing in the
> first place, so we should probably not be happy with just that.
>
> Note also that the CL is not just a workaround; it's the right thing to do
and
> would have been done eventually either way.


Not sure I'm following. If I understand correctly, calling Debug::OnThrow
somehow clears the pending message, and with this CL, we end up with no
message
at the end, hence test failure.

Yes, that's correct. (Or so I think, at least.)


How does https://codereview.chromium.org/592293003/ fix this issue?

It removes a v8::TryCatch in the generated bindings code. With this TryCatch, we
get two calls to Isolate::DoThrow():

1) Initial: has no message to begin with, calls Debug::OnThrow(), then generates
the message and stores in v8::TryCatch.

2) via ReThrow(): has message already, calls Debug::OnThrow(), then avoids
generating another message since it's supposed to be rethrowing with the message
from 1.

Without that TryCatch, we get only the one call to Isolate::DoThrow():

1) Initial: has no message, calls Debug::OnThrow(), then generates the message.


In any case, I guess it would be fine to land this once the work around lands?

Well... it won't break layout tests, and we will have exchanged one buggy
behavior with another, and it's a bit unclear which is worse.

The old buggy behavior was that exception messages were sometimes (far from
always, I think) regenerated so that the exception would appear to have been
thrown at the call to one native/blink callback on the call stack when in fact
it had been thrown somewhere else.

The new buggy behavior is that exceptions are sometimes not reported to the
console at all, but only with a debugger attached, and only (I think) when the debugger is configured to intercept the exception in question. IOW, either the debugger or the console gets to see the message, but not both. (And still only
if there is an "extra" v8::TryCatch somewhere in the bindings layer.)

https://codereview.chromium.org/587703002/

--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to