Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(585)

Unified Diff: src/isolate.cc

Issue 306463002: Don't clear exception pending message if we have externally TryCatch and finally on top of stack. (Closed) Base URL: git://github.com/v8/v8.git@master
Patch Set: Created 6 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« src/isolate.h ('K') | « src/isolate.h ('k') | test/cctest/test-api.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/isolate.cc
diff --git a/src/isolate.cc b/src/isolate.cc
index 3542b994ff3338ffd3860b763f49b4730dfe6d54..a5c859c6948c809208618fed9556ec1d0f15a48c 100644
--- a/src/isolate.cc
+++ b/src/isolate.cc
@@ -1144,7 +1144,7 @@ void Isolate::DoThrow(Object* exception, MessageLocation* location) {
}
-bool Isolate::IsExternallyCaught() {
+bool Isolate::HasExternalTryCatch() {
ASSERT(has_pending_exception());
if ((thread_local_top()->catcher_ == NULL) ||
@@ -1154,10 +1154,11 @@ bool Isolate::IsExternallyCaught() {
return false;
Yang 2014/06/02 15:21:16 Can we simply return an expression? return (threa
}
- if (!is_catchable_by_javascript(pending_exception())) {
vsevik 2014/06/02 15:31:28 Looking at this once again I think this check is s
- return true;
- }
+ 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 =
@@ -1177,18 +1178,18 @@ bool Isolate::IsExternallyCaught() {
StackHandler::FromAddress(Isolate::handler(thread_local_top()));
while (handler != NULL && handler->address() < external_handler_address) {
ASSERT(!handler->is_catch());
- if (handler->is_finally()) return false;
+ if (handler->is_finally()) return true;
handler = handler->next();
}
- return true;
+ return false;
}
void Isolate::ReportPendingMessages() {
ASSERT(has_pending_exception());
- PropagatePendingExceptionToExternalTryCatch();
+ bool canClearMessage = PropagatePendingExceptionToExternalTryCatch();
Yang 2014/06/02 15:21:16 please name this 'can_clear_message' instead.
HandleScope scope(this);
if (thread_local_top_.pending_exception_ ==
@@ -1215,7 +1216,8 @@ void Isolate::ReportPendingMessages() {
}
}
}
- clear_pending_message();
+ if (canClearMessage)
+ clear_pending_message();
Yang 2014/06/02 15:21:16 no line break here.
}
@@ -1722,13 +1724,16 @@ void Isolate::InitializeThreadLocal() {
}
-void Isolate::PropagatePendingExceptionToExternalTryCatch() {
+bool Isolate::PropagatePendingExceptionToExternalTryCatch() {
ASSERT(has_pending_exception());
- bool external_caught = IsExternallyCaught();
- thread_local_top_.external_caught_exception_ = external_caught;
+ bool has_external_try_catch = HasExternalTryCatch();
+ bool is_finally_on_top = IsFinallyOnTop();
- if (!external_caught) return;
+ bool should_propagate_now = has_external_try_catch && !is_finally_on_top;
+ thread_local_top_.external_caught_exception_ = should_propagate_now;
+
+ if (!should_propagate_now) return !has_external_try_catch;
if (thread_local_top_.pending_exception_ ==
heap()->termination_exception()) {
@@ -1745,13 +1750,14 @@ void 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;
+ if (thread_local_top_.pending_message_obj_->IsTheHole()) return true;
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;
}
« src/isolate.h ('K') | « src/isolate.h ('k') | test/cctest/test-api.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698