Issue 2429, core implementation and the protocol change

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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@13123 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/debug-debugger.js

@@ -1306,9 +1306,12 @@ ProtocolMessage.prototype.setOption = function(name, value) {
 };
 
 
-ProtocolMessage.prototype.failed = function(message) {
+ProtocolMessage.prototype.failed = function(message, opt_details) {
   this.success = false;
   this.message = message;
+  if (IS_OBJECT(opt_details)) {
+    this.error_details = opt_details;
+  }
 };
 
 
@@ -1355,6 +1358,9 @@ ProtocolMessage.prototype.toJSONProtocol = function() {
   if (this.message) {
     json.message = this.message;
   }
+  if (this.error_details) {
+    json.error_details = this.error_details;
+  }
   json.running = this.running;
   return JSON.stringify(json);
 };
@@ -2461,8 +2467,17 @@ DebugCommandProcessor.prototype.changeLiveRequest_ = function(
 
   var new_source = request.arguments.new_source;
 
-  var result_description = Debug.LiveEdit.SetScriptSource(the_script,
-      new_source, preview_only, change_log);
+  var result_description;
+  try {
+    result_description = Debug.LiveEdit.SetScriptSource(the_script,
+        new_source, preview_only, change_log);
+  } catch (e) {
+    if (e instanceof Debug.LiveEdit.Failure && "details" in e) {
+      response.failed(e.message, e.details);
+      return;
+    }
+    throw e;
+  }
   response.body = {change_log: change_log, result: result_description};
 
   if (!preview_only && !this.running_ && result_description.stack_modified) {

src/isolate.cc

@@ -1366,6 +1366,24 @@ void Isolate::ReportPendingMessages() {
 }
 
 
+MessageLocation Isolate::GetMessageLocation() {
+  ASSERT(has_pending_exception());
+
+  if (thread_local_top_.pending_exception_ != Failure::OutOfMemoryException() &&
+      thread_local_top_.pending_exception_ != heap()->termination_exception() &&
+      thread_local_top_.has_pending_message_ &&
+      !thread_local_top_.pending_message_obj_->IsTheHole() &&
+      thread_local_top_.pending_message_script_ != NULL) {
+    Handle<Script> script(thread_local_top_.pending_message_script_);
+    int start_pos = thread_local_top_.pending_message_start_pos_;
+    int end_pos = thread_local_top_.pending_message_end_pos_;
+    return MessageLocation(script, start_pos, end_pos);
+  }
+
+  return MessageLocation();
+}
+
+
 void Isolate::TraceException(bool flag) {
   FLAG_trace_exception = flag;  // TODO(isolates): This is an unfortunate use.
 }

src/isolate.h

@@ -743,6 +743,8 @@ class Isolate {
   Failure* ReThrow(MaybeObject* exception);
   void ScheduleThrow(Object* exception);
   void ReportPendingMessages();
+  // Return pending location if any or unfilled structure.
+  MessageLocation GetMessageLocation();
   Failure* ThrowIllegalOperation();
 
   // Promote a scheduled exception to pending. Asserts has_scheduled_exception.

src/liveedit-debugger.js

@@ -76,7 +76,17 @@ Debug.LiveEdit = new function() {
     try {
       new_compile_info = GatherCompileInfo(new_source, script);
     } catch (e) {
-      throw new Failure("Failed to compile new version of script: " + e);
+      var failure =
+          new Failure("Failed to compile new version of script: " + e);
+      if (e instanceof SyntaxError) {
+        var details = {
+          type: "liveedit_compile_error",
+          syntaxErrorMessage: e.message
+        };
+        CopyErrorPositionToDetails(e, details);
+        failure.details = details;
+      }
+      throw failure;
     }
     var root_new_node = BuildCodeInfoTree(new_compile_info);
 
@@ -978,6 +988,31 @@ Debug.LiveEdit = new function() {
     return "LiveEdit Failure: " + this.message;
   };
 
+  function CopyErrorPositionToDetails(e, details) {
+    function createPositionStruct(script, position) {
+      if (position == -1) return;
+      var location = script.locationFromPosition(position, true);
+      if (location == null) return;
+      return {
+        line: location.line + 1,
+        column: location.column + 1,
+        position: position
+      };
+    }
+
+    if (!("scriptObject" in e) || !("startPosition" in e)) {
+      return;
+    }
+
+    var script = e.scriptObject;
+
+    var position_struct = {
+      start: createPositionStruct(script, e.startPosition),
+      end: createPositionStruct(script, e.endPosition)
+    };
+    details.position = position_struct;
+  }
+
   // A testing entry.
   function GetPcFromSourcePos(func, source_pos) {
     return %GetFunctionCodePositionFromSource(func, source_pos);

src/liveedit.cc

@@ -36,6 +36,7 @@
 #include "debug.h"
 #include "deoptimizer.h"
 #include "global-handles.h"
+#include "messages.h"
 #include "parser.h"
 #include "scopeinfo.h"
 #include "scopes.h"
@@ -925,11 +926,58 @@ JSArray* LiveEdit::GatherCompileInfo(Handle<Script> script,
   Handle<Object> original_source = Handle<Object>(script->source());
   script->set_source(*source);
   isolate->set_active_function_info_listener(&listener);
-  CompileScriptForTracker(isolate, script);
+
+  {
+    // Creating verbose TryCatch from public API is currently the only way to
+    // force code save location. We do not use this the object directly.
+    v8::TryCatch try_catch;
+    try_catch.SetVerbose(true);
+
+    // A logical 'try' section.
+    CompileScriptForTracker(isolate, script);
+  }
+
+  // A logical 'catch' section.
+  Handle<Object> rethrow_exception;
+  if (isolate->has_pending_exception()) {
+    Handle<Object> exception(isolate->pending_exception()->ToObjectChecked());
+    MessageLocation message_location = isolate->GetMessageLocation();
+
+    isolate->clear_pending_message();
+    isolate->clear_pending_exception();
+
+    // If possible, copy positions from message object to exception object.
+    if (exception->IsJSObject() && !message_location.script().is_null()) {
+      Handle<JSObject> exception_struct = Handle<JSObject>::cast(exception);
+
+      Factory* factory = isolate->factory();
+      JSReceiver::SetProperty(exception_struct,
+          factory->LookupAsciiSymbol("startPosition"),
+          Handle<Smi>(Smi::FromInt(message_location.start_pos())),
+          NONE, kNonStrictMode);
+      JSReceiver::SetProperty(exception_struct,
+          factory->LookupAsciiSymbol("endPosition"),
+          Handle<Smi>(Smi::FromInt(message_location.end_pos())),
+          NONE, kNonStrictMode);
+      JSReceiver::SetProperty(exception_struct,
+          factory->LookupAsciiSymbol("scriptObject"),
+          GetScriptWrapper(message_location.script()),
+          NONE, kNonStrictMode);
+    }
+
+    rethrow_exception = exception;
+  }
+
+  // A logical 'finally' section.
   isolate->set_active_function_info_listener(NULL);
   script->set_source(*original_source);
 
-  return *(listener.GetResult());
+  if (rethrow_exception.is_null()) {
+    return *(listener.GetResult());
+  } else {
+    isolate->Throw(*rethrow_exception);
+    return 0;
+  }
 }
 
 

test/mjsunit/debug-liveedit-compile-error.js

@@ -0,0 +1,60 @@
+// Copyright 2012 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Flags: --expose-debug-as debug
+// Get the Debug object exposed from the debug context global object.
+
+Debug = debug.Debug
+
+eval("var something1 = 25; \n"
+     + " function ChooseAnimal() { return          'Cat';          } \n"
+     + " ChooseAnimal.Helper = function() { return 'Help!'; }\n");
+
+assertEquals("Cat", ChooseAnimal());
+
+var script = Debug.findScript(ChooseAnimal);
+
+var orig_animal = "Cat";
+var patch_pos = script.source.indexOf(orig_animal);
+var new_animal_patch = "Cap' + ) + 'bara";
+
+var change_log = new Array();
+var caught_exception = null;
+try {
+  Debug.LiveEdit.TestApi.ApplySingleChunkPatch(script, patch_pos,
+      orig_animal.length, new_animal_patch, change_log);
+} catch (e) {
+  caught_exception = e;
+}
+
+assertNotNull(caught_exception);
+assertEquals("Unexpected token )",
+    caught_exception.details.syntaxErrorMessage);
+
+assertEquals(2, caught_exception.details.position.start.line);
+
+