Follow up to fix v8::Exception::GetMessage() actually do what it was intended to.

The main thing for v8::Exception::GetMessage() is to extract message location from
error stack trace, even when stack trace capturing is off (when DevTools is closed).

BUG=chromium:427954
R=yangguo@chromium.org
LOG=N

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

Cr-Commit-Position: refs/heads/master@{#25101}
git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@25101 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/isolate.cc b/src/isolate.cc
index a49eee6..2595d2f 100644 (file)
--- a/src/isolate.cc
+++ b/src/isolate.cc
@@ -1047,15 +1047,15 @@ void Isolate::ComputeLocation(MessageLocation* target) {
 }
 
 
-void Isolate::ComputeLocationFromStackTrace(MessageLocation* target,
+bool Isolate::ComputeLocationFromStackTrace(MessageLocation* target,
                                             Handle<Object> exception) {
   *target = MessageLocation(Handle<Script>(heap_.empty_script()), -1, -1);
 
-  if (!exception->IsJSObject()) return;
+  if (!exception->IsJSObject()) return false;
   Handle<Name> key = factory()->stack_trace_symbol();
   Handle<Object> property =
       JSObject::GetDataProperty(Handle<JSObject>::cast(exception), key);
-  if (!property->IsJSArray()) return;
+  if (!property->IsJSArray()) return false;
   Handle<JSArray> simple_stack_trace = Handle<JSArray>::cast(property);
 
   Handle<FixedArray> elements(FixedArray::cast(simple_stack_trace->elements()));
@@ -1075,9 +1075,10 @@ void Isolate::ComputeLocationFromStackTrace(MessageLocation* target,
       int pos = code->SourcePosition(pc);
       Handle<Script> casted_script(Script::cast(script));
       *target = MessageLocation(casted_script, pos, pos + 1);
-      break;
+      return true;
     }
   }
+  return false;
 }
 
 
@@ -1149,10 +1150,6 @@ Handle<JSMessageObject> Isolate::CreateMessage(Handle<Object> exception,
       // at this throw site.
       stack_trace_object =
           GetDetailedStackTrace(Handle<JSObject>::cast(exception));
-      if (!location) {
-        ComputeLocationFromStackTrace(&potential_computed_location, exception);
-        location = &potential_computed_location;
-      }
     }
     if (stack_trace_object.is_null()) {
       // Not an error object, we capture stack and location at throw site.
@@ -1162,7 +1159,10 @@ Handle<JSMessageObject> Isolate::CreateMessage(Handle<Object> exception,
     }
   }
   if (!location) {
-    ComputeLocation(&potential_computed_location);
+    if (!ComputeLocationFromStackTrace(&potential_computed_location,
+                                       exception)) {
+      ComputeLocation(&potential_computed_location);
+    }
     location = &potential_computed_location;
   }
 

diff --git a/src/isolate.h b/src/isolate.h
index 7e50929..3551632 100644 (file)
--- a/src/isolate.h
+++ b/src/isolate.h
@@ -801,7 +801,7 @@ class Isolate {
   // Attempts to compute the current source location, storing the
   // result in the target out parameter.
   void ComputeLocation(MessageLocation* target);
-  void ComputeLocationFromStackTrace(MessageLocation* target,
+  bool ComputeLocationFromStackTrace(MessageLocation* target,
                                      Handle<Object> exception);
 
   Handle<JSMessageObject> CreateMessage(Handle<Object> exception,

diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index 0e6d128..068a07e 100644 (file)
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -8638,6 +8638,8 @@ static void ThrowV8Exception(const v8::FunctionCallbackInfo<v8::Value>& info) {
 THREADED_TEST(ExceptionGetMessage) {
   LocalContext context;
   v8::HandleScope scope(context->GetIsolate());
+  v8::Handle<String> foo_str = v8_str("foo");
+  v8::Handle<String> message_str = v8_str("message");
 
   v8::V8::SetCaptureStackTraceForUncaughtExceptions(true);
 
@@ -8655,8 +8657,6 @@ THREADED_TEST(ExceptionGetMessage) {
   CHECK(try_catch.HasCaught());
 
   v8::Handle<v8::Value> error = try_catch.Exception();
-  v8::Handle<String> foo_str = v8_str("foo");
-  v8::Handle<String> message_str = v8_str("message");
   CHECK(error->IsObject());
   CHECK(error.As<v8::Object>()->Get(message_str)->Equals(foo_str));
 
@@ -8670,6 +8670,30 @@ THREADED_TEST(ExceptionGetMessage) {
   CHECK_EQ(2, stackTrace->GetFrameCount());
 
   v8::V8::SetCaptureStackTraceForUncaughtExceptions(false);
+
+  // Now check message location when SetCaptureStackTraceForUncaughtExceptions
+  // is false.
+  try_catch.Reset();
+
+  CompileRun(
+      "function f2() {\n"
+      "  return throwV8Exception();\n"
+      "};\n"
+      "f2();");
+  CHECK(try_catch.HasCaught());
+
+  error = try_catch.Exception();
+  CHECK(error->IsObject());
+  CHECK(error.As<v8::Object>()->Get(message_str)->Equals(foo_str));
+
+  message = v8::Exception::GetMessage(error);
+  CHECK(!message.IsEmpty());
+  CHECK_EQ(2, message->GetLineNumber());
+  CHECK_EQ(9, message->GetStartColumn());
+
+  // Should be empty stack trace.
+  stackTrace = message->GetStackTrace();
+  CHECK(stackTrace.IsEmpty());
 }