Revert of Cleanup and unify Isolate::ReportPendingMessages. (patchset #4...

Revert of Cleanup and unify Isolate::ReportPendingMessages. (patchset #4 id:60001 of https://codereview.chromium.org/999923004/)

Reason for revert:
Failing Blink tests on Win and Mac:
- plugins/netscape-plugin-property-access-exception.html
- http/tests/plugins/cross-frame-object-access.html

Original issue's description:
> Cleanup and unify Isolate::ReportPendingMessages.
>
> Note that this is a pure cleanup CL and shouldn't have an observable
> impact on the functional behavior of message reporting.
>
> R=yangguo@chromium.org

TBR=yangguo@chromium.org,machenbach@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Review URL: https://codereview.chromium.org/1016323002

Cr-Commit-Position: refs/heads/master@{#27306}

src/isolate.cc

@@ -1143,7 +1143,7 @@ void Isolate::ScheduleThrow(Object* exception) {
   // When scheduling a throw we first throw the exception to get the
   // error reporting if it is uncaught before rescheduling it.
   Throw(exception);
-  ReportPendingMessages();
+  PropagatePendingExceptionToExternalTryCatch();
   if (has_pending_exception()) {
     thread_local_top()->scheduled_exception_ = pending_exception();
     thread_local_top()->external_caught_exception_ = false;
@@ -1410,50 +1410,32 @@ bool Isolate::IsExternalHandlerOnTop(Object* exception) {
 void Isolate::ReportPendingMessages() {
   Object* exception = pending_exception();
 
-  // Propagation is unsuccessful because there still is a JavaScript handler on
-  // top that might catch the exception and hence prevent message reporting.
-  if (IsJavaScriptHandlerOnTop(exception)) {
-    thread_local_top_.external_caught_exception_ = false;
-    return;
-  }
+  // Try to propagate the exception to an external v8::TryCatch handler. If
+  // propagation was unsuccessful, then we will get another chance at reporting
+  // the pending message if the exception is re-thrown.
+  bool has_been_propagated = PropagatePendingExceptionToExternalTryCatch();
+  if (!has_been_propagated) return;
+
+  // Clear the pending message object early to avoid endless recursion.
+  Object* message_obj = thread_local_top_.pending_message_obj_;
+  clear_pending_message();
+
+  // For uncatchable exceptions we do nothing. If needed, the exception and the
+  // message have already been propagated to v8::TryCatch.
+  if (!is_catchable_by_javascript(exception)) return;
 
   // Determine whether the message needs to be reported to all message handlers
-  // depending on whether an external v8::TryCatch handler is on top.
+  // depending on whether and external v8::TryCatch or an internal JavaScript
+  // handler is on top.
   bool should_report_exception;
-  Object* message_obj = thread_local_top_.pending_message_obj_;
-  DCHECK(message_obj->IsJSMessageObject() || message_obj->IsTheHole());
   if (IsExternalHandlerOnTop(exception)) {
-    // Propagate the exception to an external v8::TryCatch handler.
-    if (!is_catchable_by_javascript(exception)) {
-      try_catch_handler()->can_continue_ = false;
-      try_catch_handler()->has_terminated_ = true;
-      try_catch_handler()->exception_ = heap()->null_value();
-    } else {
-      try_catch_handler()->can_continue_ = true;
-      try_catch_handler()->has_terminated_ = false;
-      try_catch_handler()->exception_ = exception;
-      // Propagate to the external try-catch only if we got an actual message.
-      if (!message_obj->IsTheHole()) {
-        try_catch_handler()->message_obj_ = message_obj;
-      }
-    }
-
     // Only report the exception if the external handler is verbose.
-    thread_local_top_.external_caught_exception_ = true;
     should_report_exception = try_catch_handler()->is_verbose_;
   } else {
-    // Report the exception because it cannot be caught by JavaScript code.
-    thread_local_top_.external_caught_exception_ = false;
-    should_report_exception = true;
+    // Report the exception if it isn't caught by JavaScript code.
+    should_report_exception = !IsJavaScriptHandlerOnTop(exception);
   }
 
-  // Clear the pending message object early to avoid endless recursion.
-  clear_pending_message();
-
-  // For uncatchable exceptions we do nothing. If needed, the exception and the
-  // message have already been propagated to v8::TryCatch.
-  if (!is_catchable_by_javascript(exception)) return;
-
   // Actually report the pending message to all message handlers.
   if (!message_obj->IsTheHole() && should_report_exception) {
     HandleScope scope(this);
@@ -1488,7 +1470,7 @@ MessageLocation Isolate::GetMessageLocation() {
 
 bool Isolate::OptionalRescheduleException(bool is_bottom_call) {
   DCHECK(has_pending_exception());
-  ReportPendingMessages();
+  PropagatePendingExceptionToExternalTryCatch();
 
   bool is_termination_exception =
       pending_exception() == heap_.termination_exception();
@@ -1968,6 +1950,40 @@ void Isolate::InitializeThreadLocal() {
 }
 
 
+bool Isolate::PropagatePendingExceptionToExternalTryCatch() {
+  Object* exception = pending_exception();
+
+  if (IsJavaScriptHandlerOnTop(exception)) {
+    thread_local_top_.external_caught_exception_ = false;
+    return false;
+  }
+
+  if (!IsExternalHandlerOnTop(exception)) {
+    thread_local_top_.external_caught_exception_ = false;
+    return true;
+  }
+
+  thread_local_top_.external_caught_exception_ = true;
+  if (!is_catchable_by_javascript(exception)) {
+    try_catch_handler()->can_continue_ = false;
+    try_catch_handler()->has_terminated_ = true;
+    try_catch_handler()->exception_ = heap()->null_value();
+  } else {
+    v8::TryCatch* handler = try_catch_handler();
+    DCHECK(thread_local_top_.pending_message_obj_->IsJSMessageObject() ||
+           thread_local_top_.pending_message_obj_->IsTheHole());
+    handler->can_continue_ = true;
+    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;
+
+    handler->message_obj_ = thread_local_top_.pending_message_obj_;
+  }
+  return true;
+}
+
+
 void Isolate::InitializeLoggingAndCounters() {
   if (logger_ == NULL) {
     logger_ = new Logger(this);

src/isolate.h

@@ -767,16 +767,13 @@ class Isolate {
   // finally clause will behave as if the exception were consumed.
   bool PredictWhetherExceptionIsCaught(Object* exception);
 
-  // Propagate pending exception message to potential v8::TryCatch. Also call
-  // message handlers when the exception is guaranteed not to be caught.
-  void ReportPendingMessages();
-
   void ScheduleThrow(Object* exception);
   // 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();
 
@@ -1218,6 +1215,11 @@ 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();
+
   // Traverse prototype chain to find out whether the object is derived from
   // the Error object.
   bool IsErrorObject(Handle<Object> obj);