From 5586f1f3099f035ad4e89af830f6c0bd896a8b5d Mon Sep 17 00:00:00 2001 From: "yurys@chromium.org" Date: Thu, 5 Jun 2014 13:58:57 +0000 Subject: [PATCH] Revert "V8 can clear exception pending message, when should not do this." This reverts commit 2c6665a7a21bd38f3dea28eb9b303f913c69be8d. Broke too many tests. TBR=yangguo@chromium.org BUG=None LOG=N Review URL: https://codereview.chromium.org/318773006 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@21698 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/isolate.cc | 39 +++++++++++++++++++-------------------- src/isolate.h | 8 ++------ test/cctest/test-api.cc | 35 ----------------------------------- 3 files changed, 21 insertions(+), 61 deletions(-) diff --git a/src/isolate.cc b/src/isolate.cc index eb13949..1e0b094 100644 --- a/src/isolate.cc +++ b/src/isolate.cc @@ -1166,15 +1166,20 @@ void Isolate::DoThrow(Object* exception, MessageLocation* location) { } -bool Isolate::HasExternalTryCatch() { +bool Isolate::IsExternallyCaught() { ASSERT(has_pending_exception()); - return (thread_local_top()->catcher_ != NULL) && - (try_catch_handler() == thread_local_top()->catcher_); -} + if ((thread_local_top()->catcher_ == NULL) || + (try_catch_handler() != thread_local_top()->catcher_)) { + // When throwing the exception, we found no v8::TryCatch + // which should care about this exception. + return false; + } + if (!is_catchable_by_javascript(pending_exception())) { + return true; + } -bool Isolate::IsFinallyOnTop() { // Get the address of the external handler so we can compare the address to // determine which one is closer to the top of the stack. Address external_handler_address = @@ -1194,18 +1199,18 @@ bool Isolate::IsFinallyOnTop() { StackHandler::FromAddress(Isolate::handler(thread_local_top())); while (handler != NULL && handler->address() < external_handler_address) { ASSERT(!handler->is_catch()); - if (handler->is_finally()) return true; + if (handler->is_finally()) return false; handler = handler->next(); } - return false; + return true; } void Isolate::ReportPendingMessages() { ASSERT(has_pending_exception()); - bool can_clear_message = PropagatePendingExceptionToExternalTryCatch(); + PropagatePendingExceptionToExternalTryCatch(); HandleScope scope(this); if (thread_local_top_.pending_exception_ == @@ -1232,7 +1237,7 @@ void Isolate::ReportPendingMessages() { } } } - if (can_clear_message) clear_pending_message(); + clear_pending_message(); } @@ -1737,18 +1742,13 @@ void Isolate::InitializeThreadLocal() { } -bool Isolate::PropagatePendingExceptionToExternalTryCatch() { +void Isolate::PropagatePendingExceptionToExternalTryCatch() { ASSERT(has_pending_exception()); - bool has_external_try_catch = HasExternalTryCatch(); - bool is_catchable_by_js = is_catchable_by_javascript(pending_exception()); - bool is_finally_on_top = IsFinallyOnTop(); + bool external_caught = IsExternallyCaught(); + thread_local_top_.external_caught_exception_ = external_caught; - bool should_propagate_now = has_external_try_catch && - (!is_catchable_by_js || !is_finally_on_top); - thread_local_top_.external_caught_exception_ = should_propagate_now; - - if (!should_propagate_now) return !has_external_try_catch; + if (!external_caught) return; if (thread_local_top_.pending_exception_ == heap()->termination_exception()) { @@ -1765,14 +1765,13 @@ bool Isolate::PropagatePendingExceptionToExternalTryCatch() { handler->has_terminated_ = false; handler->exception_ = pending_exception(); // Propagate to the external try-catch only if we got an actual message. - if (thread_local_top_.pending_message_obj_->IsTheHole()) return true; + if (thread_local_top_.pending_message_obj_->IsTheHole()) return; handler->message_obj_ = thread_local_top_.pending_message_obj_; handler->message_script_ = thread_local_top_.pending_message_script_; handler->message_start_pos_ = thread_local_top_.pending_message_start_pos_; handler->message_end_pos_ = thread_local_top_.pending_message_end_pos_; } - return true; } diff --git a/src/isolate.h b/src/isolate.h index 426a738..a52cfb5 100644 --- a/src/isolate.h +++ b/src/isolate.h @@ -604,8 +604,7 @@ class Isolate { thread_local_top_.scheduled_exception_ = heap_.the_hole_value(); } - bool HasExternalTryCatch(); - bool IsFinallyOnTop(); + bool IsExternallyCaught(); bool is_catchable_by_javascript(Object* exception) { return exception != heap()->termination_exception(); @@ -1180,10 +1179,7 @@ class Isolate { void FillCache(); - // Propagate pending exception message to the v8::TryCatch. - // If there is no external try-catch or message was successfully propagated, - // then return true. - bool PropagatePendingExceptionToExternalTryCatch(); + void PropagatePendingExceptionToExternalTryCatch(); // Traverse prototype chain to find out whether the object is derived from // the Error object. diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 89255f2..f4c618e 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -8362,41 +8362,6 @@ TEST(TryCatchFinallyUsingTryCatchHandler) { } -void CEvaluate(const v8::FunctionCallbackInfo& args) { - v8::HandleScope scope(args.GetIsolate()); - CompileRun(args[0]->ToString()); -} - - -TEST(TryCatchFinallyStoresMessageUsingTryCatchHandler) { - v8::Isolate* isolate = CcTest::isolate(); - v8::HandleScope scope(isolate); - Local templ = ObjectTemplate::New(isolate); - templ->Set(v8_str("CEvaluate"), - v8::FunctionTemplate::New(isolate, CEvaluate)); - LocalContext context(0, templ); - v8::TryCatch try_catch; - CompileRun("try {" - " CEvaluate('throw 1;');" - "} finally {" - "}"); - CHECK(try_catch.HasCaught()); - CHECK(!try_catch.Message().IsEmpty()); - String::Utf8Value exception_value(try_catch.Exception()); - CHECK_EQ(*exception_value, "1"); - try_catch.Reset(); - CompileRun("try {" - " CEvaluate('throw 1;');" - "} finally {" - " throw 2;" - "}"); - CHECK(try_catch.HasCaught()); - CHECK(!try_catch.Message().IsEmpty()); - String::Utf8Value finally_exception_value(try_catch.Exception()); - CHECK_EQ(*finally_exception_value, "2"); -} - - // For use within the TestSecurityHandler() test. static bool g_security_callback_result = false; static bool NamedSecurityTestCallback(Local global, -- 2.7.4