Make exception thrown via v8 public API propagate to v8::TryCatch as JS thrown exceptions do.

Make exception thrown via v8 public API propagate to v8::TryCatch as JS thrown exceptions do.

Correctly process failures which can be returned by Object::GetProperty
when performing GetRealNamedProperty* queries.

Callback properties can produce exceptions so we need to wrap access to them
into exception checks.  However, despite of many other methods with exception
checks, property access doesn't mandatroy go via JavaScript and hence we
need to inject code to propagate exception to public API TryCatch handlers.

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

[antonm, 2011-03-18 16:05:59]

Guys,

may you have a look?

It is rebaselined version of http://code.google.com/p/v8/source/detail?r=7258
modulo two changes:

1) do not recurse in MessageHandler::ReportMessage (implemented with static
in_progress guard): that solves the principal reason for rollback---we exhausted
stack on many platforms (alas, ia32 Linux was almost fine---we hit
stack-overflow check which allowed to properly unroll ReportMessage chain;

2) external_caught_exception_ is removed from ExceptionScope---we actually
always recalculate it before usage AFAIK;  I'll clean it up when this CL stays
for some time in Chromium.

[Mads Ager (chromium), 2011-03-21 10:33:21]

http://codereview.chromium.org/6685087/diff/1/src/api.cc
File src/api.cc (right):

http://codereview.chromium.org/6685087/diff/1/src/api.cc#newcode2395
src/api.cc:2395: // propagate outside.
propagate -> to propagate?

http://codereview.chromium.org/6685087/diff/1/src/messages.cc
File src/messages.cc (right):

http://codereview.chromium.org/6685087/diff/1/src/messages.cc#newcode115
src/messages.cc:115: static int in_process = false;
This will not work with isolates. You need this to be store in the isolate
somewhere. Also, it will probably not work with Lockers either. There might be a
thread switch while reporting messages with on thread. 

I think this should be added to the archived thread state that is part of the
isolate (make it part of Top like the rest of the exception state).

[antonm, 2011-03-21 18:15:48]

http://codereview.chromium.org/6685087/diff/1/src/api.cc
File src/api.cc (right):

http://codereview.chromium.org/6685087/diff/1/src/api.cc#newcode2395
src/api.cc:2395: // propagate outside.
On 2011/03/21 10:33:22, Mads Ager wrote:
> propagate -> to propagate?

Done.

http://codereview.chromium.org/6685087/diff/1/src/messages.cc
File src/messages.cc (right):

http://codereview.chromium.org/6685087/diff/1/src/messages.cc#newcode115
src/messages.cc:115: static int in_process = false;
On 2011/03/21 10:33:22, Mads Ager wrote:
> This will not work with isolates. You need this to be store in the isolate
> somewhere. Also, it will probably not work with Lockers either. There might be
a
> thread switch while reporting messages with on thread. 
> 
> I think this should be added to the archived thread state that is part of the
> isolate (make it part of Top like the rest of the exception state).

Done.

[Mads Ager (chromium), 2011-03-22 07:32:14]

LGTM

http://codereview.chromium.org/6685087/diff/7001/src/api.cc
File src/api.cc (right):

http://codereview.chromium.org/6685087/diff/7001/src/api.cc#newcode2784
src/api.cc:2784: static Local<Value> GetPropertyByLookup(i::Isolate* isolate,
The isolate parameter is not used. Remove?

[Vitaly Repeshko, 2011-03-22 08:53:30]

LGTM

[Vitaly Repeshko, 2011-03-22 08:53:30]

LGTM

[antonm, 2011-03-22 10:57:06]

Thanks a lot for review, Mads and Vitaly.

http://codereview.chromium.org/6685087/diff/7001/src/api.cc
File src/api.cc (right):

http://codereview.chromium.org/6685087/diff/7001/src/api.cc#newcode2784
src/api.cc:2784: static Local<Value> GetPropertyByLookup(i::Isolate* isolate,
On 2011/03/22 07:32:14, Mads Ager wrote:
> The isolate parameter is not used. Remove?

It's implicitly used in EXCEPTION_BAILOUT_CHECK.