Reviewers: Yang,

Description:
Make v8::TryCatch able to consume natively thrown exceptions (again).

[email protected]
BUG=chromium:362388
TEST=cctest/test-api/TryCatchNative
LOG=N

Please review this at https://codereview.chromium.org/291393002/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files (+73, -11 lines):
  M src/api.cc
  M src/isolate.h
  M src/isolate.cc
  M test/cctest/test-api.cc


Index: src/api.cc
diff --git a/src/api.cc b/src/api.cc
index 304d84ffcf872d6163a19f8a38ce9e7f8df8eaf3..7a72bc2fb2c1e3413d9e9517af554ee0b64aab9e 100644
--- a/src/api.cc
+++ b/src/api.cc
@@ -1832,6 +1832,12 @@ v8::TryCatch::~TryCatch() {
     reinterpret_cast<Isolate*>(isolate_)->ThrowException(exc);
     ASSERT(!isolate_->thread_local_top()->rethrowing_message_);
   } else {
+    if (HasCaught() && isolate_->has_scheduled_exception()) {
+ // If an exception was caught but is still scheduled because no API call + // promoted it, then it is canceled to prevent it from being propagated.
+      // Note that this will not cancel termination exceptions.
+      isolate_->CancelScheduledExceptionFromTryCatch(this);
+    }
     isolate_->UnregisterTryCatchHandler(this);
     v8::internal::SimulatorStack::UnregisterCTryCatch();
   }
Index: src/isolate.cc
diff --git a/src/isolate.cc b/src/isolate.cc
index 7f7670ec2884d5c526e5b9b391dc49e49b88e7cb..865f5affa5a49143104d9c69fb5aa28085502dcf 100644
--- a/src/isolate.cc
+++ b/src/isolate.cc
@@ -908,6 +908,15 @@ void Isolate::RestorePendingMessageFromTryCatch(v8::TryCatch* handler) {
 }


+void Isolate::CancelScheduledExceptionFromTryCatch(v8::TryCatch* handler) {
+  ASSERT(has_scheduled_exception());
+  if (scheduled_exception() == handler->exception_) {
+    ASSERT(scheduled_exception() != heap()->termination_exception());
+    clear_scheduled_exception();
+  }
+}
+
+
 Object* Isolate::PromoteScheduledException() {
   Object* thrown = scheduled_exception();
   clear_scheduled_exception();
@@ -1195,8 +1204,7 @@ void Isolate::ReportPendingMessages() {
   PropagatePendingExceptionToExternalTryCatch();

   HandleScope scope(this);
-  if (thread_local_top_.pending_exception_ ==
-          heap()->termination_exception()) {
+ if (thread_local_top_.pending_exception_ == heap()->termination_exception()) {
     // Do nothing: if needed, the exception has been already propagated to
     // v8::TryCatch.
   } else {
@@ -1733,8 +1741,7 @@ void Isolate::PropagatePendingExceptionToExternalTryCatch() {

   if (!external_caught) return;

-  if (thread_local_top_.pending_exception_ ==
-             heap()->termination_exception()) {
+ if (thread_local_top_.pending_exception_ == heap()->termination_exception()) {
     try_catch_handler()->can_continue_ = false;
     try_catch_handler()->has_terminated_ = true;
     try_catch_handler()->exception_ = heap()->null_value();
Index: src/isolate.h
diff --git a/src/isolate.h b/src/isolate.h
index 382cb5dbb13156ab5d8b886bbd1293bd9652dd76..ca2c23ea74d70f4eb697286dcc77c09e65b9567b 100644
--- a/src/isolate.h
+++ b/src/isolate.h
@@ -735,6 +735,8 @@ class Isolate {
   // Re-set pending message, script and positions reported to the TryCatch
   // back to the TLS for re-use when rethrowing.
   void RestorePendingMessageFromTryCatch(v8::TryCatch* handler);
+  // Un-schedule an exception that was caught by a TryCatch handler.
+  void CancelScheduledExceptionFromTryCatch(v8::TryCatch* handler);
   void ReportPendingMessages();
   // Return pending location if any or unfilled structure.
   MessageLocation GetMessageLocation();
Index: test/cctest/test-api.cc
diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index 447437541ef935d8d0e58f2e7e60f7937a52f4f1..a66242b9b24703839adccd7533a96257f3be9e74 100644
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -5343,15 +5343,28 @@ THREADED_TEST(TryCatchAndFinally) {
 }


-static void TryCatchNestedHelper(int depth) {
+static void TryCatchNested1Helper(int depth) {
   if (depth > 0) {
     v8::TryCatch try_catch;
     try_catch.SetVerbose(true);
-    TryCatchNestedHelper(depth - 1);
+    TryCatchNested1Helper(depth - 1);
     CHECK(try_catch.HasCaught());
     try_catch.ReThrow();
   } else {
-    CcTest::isolate()->ThrowException(v8_str("back"));
+    CcTest::isolate()->ThrowException(v8_str("E1"));
+  }
+}
+
+
+static void TryCatchNested2Helper(int depth) {
+  if (depth > 0) {
+    v8::TryCatch try_catch;
+    try_catch.SetVerbose(true);
+    TryCatchNested2Helper(depth - 1);
+    CHECK(try_catch.HasCaught());
+    try_catch.ReThrow();
+  } else {
+    CompileRun("throw 'E2';");
   }
 }

@@ -5360,10 +5373,22 @@ TEST(TryCatchNested) {
   v8::V8::Initialize();
   LocalContext context;
   v8::HandleScope scope(context->GetIsolate());
-  v8::TryCatch try_catch;
-  TryCatchNestedHelper(5);
-  CHECK(try_catch.HasCaught());
- CHECK_EQ(0, strcmp(*v8::String::Utf8Value(try_catch.Exception()), "back"));
+
+  {
+    // Test nested try-catch with a native throw in the end.
+    v8::TryCatch try_catch;
+    TryCatchNested1Helper(5);
+    CHECK(try_catch.HasCaught());
+ CHECK_EQ(0, strcmp(*v8::String::Utf8Value(try_catch.Exception()), "E1"));
+  }
+
+  {
+    // Test nested try-catch with a JavaScript throw in the end.
+    v8::TryCatch try_catch;
+    TryCatchNested2Helper(5);
+    CHECK(try_catch.HasCaught());
+ CHECK_EQ(0, strcmp(*v8::String::Utf8Value(try_catch.Exception()), "E2"));
+  }
 }


@@ -5409,6 +5434,28 @@ TEST(TryCatchMixedNesting) {
 }


+void TryCatchNativeHelper(const v8::FunctionCallbackInfo<v8::Value>& args) {
+  ApiTestFuzzer::Fuzz();
+  v8::TryCatch try_catch;
+  args.GetIsolate()->ThrowException(v8_str("boom"));
+  CHECK(try_catch.HasCaught());
+}
+
+
+TEST(TryCatchNative) {
+  v8::Isolate* isolate = CcTest::isolate();
+  v8::HandleScope scope(isolate);
+  v8::V8::Initialize();
+  v8::TryCatch try_catch;
+  Local<ObjectTemplate> templ = ObjectTemplate::New(isolate);
+  templ->Set(v8_str("TryCatchNativeHelper"),
+             v8::FunctionTemplate::New(isolate, TryCatchNativeHelper));
+  LocalContext context(0, templ);
+  CompileRun("TryCatchNativeHelper();");
+  CHECK(!try_catch.HasCaught());
+}
+
+
 THREADED_TEST(Equality) {
   LocalContext context;
   v8::Isolate* isolate = context->GetIsolate();


--
--
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