From 4e9e1eedf1ba399d00b53adf6e5722d442ed9797 Mon Sep 17 00:00:00 2001
From: kozyatinskiy <kozyatinskiy@chromium.org>
Date: Tue, 3 Feb 2015 00:57:18 -0800
Subject: [PATCH] [V8] Added line, column and script symbols for SyntaxError

For exception in promise we generate v8::Message API object from exception object. And in cases of Syntax or Reference Error we don't have enough information in exception object - we can't restore Error location from top stack frame.
In this patch three aditional private fields introduced for exception object. In case of Syntax Error we store line, column and script on Exception object and receive this information when restoring message.

BUG=443140
LOG=Y
R=yurys@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#26393}
---
 src/heap/heap.h         |  5 ++++-
 src/isolate.cc          | 36 ++++++++++++++++++++++++++---
 src/isolate.h           |  2 ++
 src/parser.cc           | 21 ++++++++++++++++-
 test/cctest/test-api.cc | 60 ++++++++++++++++++++++++++++++++++++++++++-------
 5 files changed, 111 insertions(+), 13 deletions(-)

diff --git a/src/heap/heap.h b/src/heap/heap.h
index 9d7341a..2975d09 100644
--- a/src/heap/heap.h
+++ b/src/heap/heap.h
@@ -302,7 +302,10 @@ namespace internal {
   V(promise_has_handler_symbol)     \
   V(class_script_symbol)            \
   V(class_start_position_symbol)    \
-  V(class_end_position_symbol)
+  V(class_end_position_symbol)      \
+  V(error_start_pos_symbol)         \
+  V(error_end_pos_symbol)           \
+  V(error_script_symbol)
 
 #define PUBLIC_SYMBOL_LIST(V)                                    \
   V(has_instance_symbol, symbolHasInstance, Symbol.hasInstance)  \
diff --git a/src/isolate.cc b/src/isolate.cc
index 3479ae2..259c2a2 100644
--- a/src/isolate.cc
+++ b/src/isolate.cc
@@ -1069,6 +1069,33 @@ void Isolate::ComputeLocation(MessageLocation* target) {
 }
 
 
+bool Isolate::ComputeLocationFromException(MessageLocation* target,
+                                           Handle<Object> exception) {
+  if (!exception->IsJSObject()) return false;
+
+  Handle<Name> start_pos_symbol = factory()->error_start_pos_symbol();
+  Handle<Object> start_pos = JSObject::GetDataProperty(
+      Handle<JSObject>::cast(exception), start_pos_symbol);
+  if (!start_pos->IsSmi()) return false;
+  int start_pos_value = Handle<Smi>::cast(start_pos)->value();
+
+  Handle<Name> end_pos_symbol = factory()->error_end_pos_symbol();
+  Handle<Object> end_pos = JSObject::GetDataProperty(
+      Handle<JSObject>::cast(exception), end_pos_symbol);
+  if (!end_pos->IsSmi()) return false;
+  int end_pos_value = Handle<Smi>::cast(end_pos)->value();
+
+  Handle<Name> script_symbol = factory()->error_script_symbol();
+  Handle<Object> script = JSObject::GetDataProperty(
+      Handle<JSObject>::cast(exception), script_symbol);
+  if (!script->IsScript()) return false;
+
+  Handle<Script> cast_script(Script::cast(*script));
+  *target = MessageLocation(cast_script, start_pos_value, end_pos_value);
+  return true;
+}
+
+
 bool Isolate::ComputeLocationFromStackTrace(MessageLocation* target,
                                             Handle<Object> exception) {
   *target = MessageLocation(Handle<Script>(heap_.empty_script()), -1, -1);
@@ -1181,9 +1208,12 @@ Handle<JSMessageObject> Isolate::CreateMessage(Handle<Object> exception,
     }
   }
   if (!location) {
-    if (!ComputeLocationFromStackTrace(&potential_computed_location,
-                                       exception)) {
-      ComputeLocation(&potential_computed_location);
+    if (!ComputeLocationFromException(&potential_computed_location,
+                                      exception)) {
+      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 a737dae..21ac999 100644
--- a/src/isolate.h
+++ b/src/isolate.h
@@ -809,6 +809,8 @@ class Isolate {
   // Attempts to compute the current source location, storing the
   // result in the target out parameter.
   void ComputeLocation(MessageLocation* target);
+  bool ComputeLocationFromException(MessageLocation* target,
+                                    Handle<Object> exception);
   bool ComputeLocationFromStackTrace(MessageLocation* target,
                                      Handle<Object> exception);
 
diff --git a/src/parser.cc b/src/parser.cc
index a473552..f16473c 100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -4256,7 +4256,26 @@ void Parser::ThrowPendingError() {
         pending_error_is_reference_error_
             ? factory->NewReferenceError(pending_error_message_, array)
             : factory->NewSyntaxError(pending_error_message_, array);
-    if (maybe_error.ToHandle(&error)) isolate()->Throw(*error, &location);
+
+    if (maybe_error.ToHandle(&error)) {
+      Handle<JSObject> jserror = Handle<JSObject>::cast(error);
+
+      Handle<Name> key_start_pos = factory->error_start_pos_symbol();
+      JSObject::SetProperty(
+          jserror, key_start_pos,
+          handle(Smi::FromInt(location.start_pos()), isolate()),
+          SLOPPY).Check();
+
+      Handle<Name> key_end_pos = factory->error_end_pos_symbol();
+      JSObject::SetProperty(jserror, key_end_pos,
+                            handle(Smi::FromInt(location.end_pos()), isolate()),
+                            SLOPPY).Check();
+
+      Handle<Name> key_script = factory->error_script_symbol();
+      JSObject::SetProperty(jserror, key_script, script(), SLOPPY).Check();
+
+      isolate()->Throw(*error, &location);
+    }
   }
 }
 
diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index a043b36..3a9927c 100644
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -18142,29 +18142,39 @@ TEST(RethrowBogusErrorStackTrace) {
 v8::PromiseRejectEvent reject_event = v8::kPromiseRejectWithNoHandler;
 int promise_reject_counter = 0;
 int promise_revoke_counter = 0;
+int promise_reject_msg_line_number = -1;
+int promise_reject_msg_column_number = -1;
 int promise_reject_line_number = -1;
+int promise_reject_column_number = -1;
 int promise_reject_frame_count = -1;
 
-void PromiseRejectCallback(v8::PromiseRejectMessage message) {
-  if (message.GetEvent() == v8::kPromiseRejectWithNoHandler) {
+void PromiseRejectCallback(v8::PromiseRejectMessage reject_message) {
+  if (reject_message.GetEvent() == v8::kPromiseRejectWithNoHandler) {
     promise_reject_counter++;
-    CcTest::global()->Set(v8_str("rejected"), message.GetPromise());
-    CcTest::global()->Set(v8_str("value"), message.GetValue());
-    v8::Handle<v8::StackTrace> stack_trace =
-        v8::Exception::CreateMessage(message.GetValue())->GetStackTrace();
+    CcTest::global()->Set(v8_str("rejected"), reject_message.GetPromise());
+    CcTest::global()->Set(v8_str("value"), reject_message.GetValue());
+    v8::Handle<v8::Message> message =
+        v8::Exception::CreateMessage(reject_message.GetValue());
+    v8::Handle<v8::StackTrace> stack_trace = message->GetStackTrace();
+
+    promise_reject_msg_line_number = message->GetLineNumber();
+    promise_reject_msg_column_number = message->GetStartColumn() + 1;
+
     if (!stack_trace.IsEmpty()) {
       promise_reject_frame_count = stack_trace->GetFrameCount();
       if (promise_reject_frame_count > 0) {
         CHECK(stack_trace->GetFrame(0)->GetScriptName()->Equals(v8_str("pro")));
         promise_reject_line_number = stack_trace->GetFrame(0)->GetLineNumber();
+        promise_reject_column_number = stack_trace->GetFrame(0)->GetColumn();
       } else {
         promise_reject_line_number = -1;
+        promise_reject_column_number = -1;
       }
     }
   } else {
     promise_revoke_counter++;
-    CcTest::global()->Set(v8_str("revoked"), message.GetPromise());
-    CHECK(message.GetValue().IsEmpty());
+    CcTest::global()->Set(v8_str("revoked"), reject_message.GetPromise());
+    CHECK(reject_message.GetValue().IsEmpty());
   }
 }
 
@@ -18182,7 +18192,10 @@ v8::Handle<v8::Value> RejectValue() {
 void ResetPromiseStates() {
   promise_reject_counter = 0;
   promise_revoke_counter = 0;
+  promise_reject_msg_line_number = -1;
+  promise_reject_msg_column_number = -1;
   promise_reject_line_number = -1;
+  promise_reject_column_number = -1;
   promise_reject_frame_count = -1;
   CcTest::global()->Set(v8_str("rejected"), v8_str(""));
   CcTest::global()->Set(v8_str("value"), v8_str(""));
@@ -18411,6 +18424,9 @@ TEST(PromiseRejectCallback) {
   CHECK_EQ(0, promise_revoke_counter);
   CHECK_EQ(2, promise_reject_frame_count);
   CHECK_EQ(3, promise_reject_line_number);
+  CHECK_EQ(5, promise_reject_column_number);
+  CHECK_EQ(3, promise_reject_msg_line_number);
+  CHECK_EQ(5, promise_reject_msg_column_number);
 
   ResetPromiseStates();
 
@@ -18431,6 +18447,9 @@ TEST(PromiseRejectCallback) {
   CHECK_EQ(0, promise_revoke_counter);
   CHECK_EQ(2, promise_reject_frame_count);
   CHECK_EQ(5, promise_reject_line_number);
+  CHECK_EQ(23, promise_reject_column_number);
+  CHECK_EQ(5, promise_reject_msg_line_number);
+  CHECK_EQ(23, promise_reject_msg_column_number);
 
   // Throw in u3, which handles u1's rejection.
   CompileRunWithOrigin(
@@ -18454,6 +18473,9 @@ TEST(PromiseRejectCallback) {
   CHECK_EQ(2, promise_revoke_counter);
   CHECK_EQ(3, promise_reject_frame_count);
   CHECK_EQ(3, promise_reject_line_number);
+  CHECK_EQ(12, promise_reject_column_number);
+  CHECK_EQ(3, promise_reject_msg_line_number);
+  CHECK_EQ(12, promise_reject_msg_column_number);
 
   ResetPromiseStates();
 
@@ -18473,6 +18495,28 @@ TEST(PromiseRejectCallback) {
   CHECK_EQ(1, promise_revoke_counter);
   CHECK_EQ(0, promise_reject_frame_count);
   CHECK_EQ(-1, promise_reject_line_number);
+  CHECK_EQ(-1, promise_reject_column_number);
+
+  ResetPromiseStates();
+
+  // Create promise t1, which rejects by throwing syntax error from eval.
+  CompileRunWithOrigin(
+      "var t1 = new Promise(   \n"
+      "  function(res, rej) {  \n"
+      "    var content = '\\n\\\n"
+      "      }';               \n"
+      "    eval(content);      \n"
+      "  }                     \n"
+      ");                      \n",
+      "pro", 0, 0);
+  CHECK(!GetPromise("t1")->HasHandler());
+  CHECK_EQ(1, promise_reject_counter);
+  CHECK_EQ(0, promise_revoke_counter);
+  CHECK_EQ(2, promise_reject_frame_count);
+  CHECK_EQ(5, promise_reject_line_number);
+  CHECK_EQ(10, promise_reject_column_number);
+  CHECK_EQ(2, promise_reject_msg_line_number);
+  CHECK_EQ(7, promise_reject_msg_column_number);
 }
 
 
-- 
2.7.4