Issue 2399 part 1: In debugger allow modifying local variable values

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

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

src/debug-debugger.js

@@ -1427,6 +1427,8 @@ DebugCommandProcessor.prototype.processDebugJSONRequest = function(
         this.scopesRequest_(request, response);
       } else if (request.command == 'scope') {
         this.scopeRequest_(request, response);
+      } else if (request.command == 'setVariableValue') {
+        this.setVariableValueRequest_(request, response);
       } else if (request.command == 'evaluate') {
         this.evaluateRequest_(request, response);
       } else if (lol_is_enabled && request.command == 'getobj') {
@@ -1953,11 +1955,12 @@ DebugCommandProcessor.prototype.frameRequest_ = function(request, response) {
 };
 
 
-DebugCommandProcessor.prototype.frameForScopeRequest_ = function(request) {
+DebugCommandProcessor.prototype.resolveFrameFromScopeDescription_ =
+    function(scope_description) {
   // Get the frame for which the scope or scopes are requested.
   // With no frameNumber argument use the currently selected frame.
-  if (request.arguments && !IS_UNDEFINED(request.arguments.frameNumber)) {
-    frame_index = request.arguments.frameNumber;
+  if (scope_description && !IS_UNDEFINED(scope_description.frameNumber)) {
+    frame_index = scope_description.frameNumber;
     if (frame_index < 0 || this.exec_state_.frameCount() <= frame_index) {
       throw new Error('Invalid frame number');
     }
@@ -1971,13 +1974,13 @@ DebugCommandProcessor.prototype.frameForScopeRequest_ = function(request) {
 // Gets scope host object from request. It is either a function
 // ('functionHandle' argument must be specified) or a stack frame
 // ('frameNumber' may be specified and the current frame is taken by default).
-DebugCommandProcessor.prototype.scopeHolderForScopeRequest_ =
-    function(request) {
-  if (request.arguments && "functionHandle" in request.arguments) {
-    if (!IS_NUMBER(request.arguments.functionHandle)) {
+DebugCommandProcessor.prototype.resolveScopeHolder_ =
+    function(scope_description) {
+  if (scope_description && "functionHandle" in scope_description) {
+    if (!IS_NUMBER(scope_description.functionHandle)) {
       throw new Error('Function handle must be a number');
     }
-    var function_mirror = LookupMirror(request.arguments.functionHandle);
+    var function_mirror = LookupMirror(scope_description.functionHandle);
     if (!function_mirror) {
       throw new Error('Failed to find function object by handle');
     }
@@ -1992,14 +1995,14 @@ DebugCommandProcessor.prototype.scopeHolderForScopeRequest_ =
     }
 
     // Get the frame for which the scopes are requested.
-    var frame = this.frameForScopeRequest_(request);
+    var frame = this.resolveFrameFromScopeDescription_(scope_description);
     return frame;
   }
 }
 
 
 DebugCommandProcessor.prototype.scopesRequest_ = function(request, response) {
-  var scope_holder = this.scopeHolderForScopeRequest_(request);
+  var scope_holder = this.resolveScopeHolder_(request.arguments);
 
   // Fill all scopes for this frame or function.
   var total_scopes = scope_holder.scopeCount();
@@ -2018,7 +2021,7 @@ DebugCommandProcessor.prototype.scopesRequest_ = function(request, response) {
 
 DebugCommandProcessor.prototype.scopeRequest_ = function(request, response) {
   // Get the frame or function for which the scope is requested.
-  var scope_holder = this.scopeHolderForScopeRequest_(request);
+  var scope_holder = this.resolveScopeHolder_(request.arguments);
 
   // With no scope argument just return top scope.
   var scope_index = 0;
@@ -2033,6 +2036,77 @@ DebugCommandProcessor.prototype.scopeRequest_ = function(request, response) {
 };
 
 
+// Reads value from protocol description. Description may be in form of type
+// (for singletons), raw value (primitive types supported in JSON),
+// string value description plus type (for primitive values) or handle id.
+// Returns raw value or throws exception.
+DebugCommandProcessor.resolveValue_ = function(value_description) {
+  if ("handle" in value_description) {
+    var value_mirror = LookupMirror(value_description.handle);
+    if (!value_mirror) {
+      throw new Error("Failed to resolve value by handle, ' #" +
+          mapping.handle + "# not found");
+    }
+    return value_mirror.value();
+  } else if ("stringDescription" in value_description) {
+    if (value_description.type == BOOLEAN_TYPE) {
+      return Boolean(value_description.stringDescription);
+    } else if (value_description.type == NUMBER_TYPE) {
+      return Number(value_description.stringDescription);
+    } if (value_description.type == STRING_TYPE) {
+      return String(value_description.stringDescription);
+    } else {
+      throw new Error("Unknown type");
+    }
+  } else if ("value" in value_description) {
+    return value_description.value;
+  } else if (value_description.type == UNDEFINED_TYPE) {
+    return void 0;
+  } else if (value_description.type == NULL_TYPE) {
+    return null;
+  } else {
+    throw new Error("Failed to parse value description");
+  }
+};
+
+
+DebugCommandProcessor.prototype.setVariableValueRequest_ =
+    function(request, response) {
+  if (!request.arguments) {
+    response.failed('Missing arguments');
+    return;
+  }
+
+  if (IS_UNDEFINED(request.arguments.name)) {
+    response.failed('Missing variable name');
+  }
+  var variable_name = request.arguments.name;
+
+  var scope_description = request.arguments.scope;
+
+  // Get the frame or function for which the scope is requested.
+  var scope_holder = this.resolveScopeHolder_(scope_description);
+
+  if (IS_UNDEFINED(scope_description.number)) {
+    response.failed('Missing scope number');
+  }
+  var scope_index = %ToNumber(scope_description.number);
+
+  var scope = scope_holder.scope(scope_index);
+
+  var new_value =
+      DebugCommandProcessor.resolveValue_(request.arguments.newValue);
+
+  scope.setVariableValue(variable_name, new_value);
+
+  var new_value_mirror = MakeMirror(new_value);
+
+  response.body = {
+    newValue: new_value_mirror
+  };
+};
+
+
 DebugCommandProcessor.prototype.evaluateRequest_ = function(request, response) {
   if (!request.arguments) {
     return response.failed('Missing arguments');
@@ -2663,3 +2737,7 @@ function ValueToProtocolValue_(value, mirror_serializer) {
   }
   return json;
 }
+
+Debug.TestApi = {
+  CommandProcessorResolveValue: DebugCommandProcessor.resolveValue_
+};

src/mirror-debugger.js

@@ -1844,10 +1844,14 @@ function ScopeDetails(frame, fun, index) {
                                      frame.details_.frameId(),
                                      frame.details_.inlinedFrameIndex(),
                                      index);
+    this.frame_id_ = frame.details_.frameId();
+    this.inlined_frame_id_ = frame.details_.inlinedFrameIndex();
   } else {
     this.details_ = %GetFunctionScopeDetails(fun.value(), index);
+    this.fun_value_ = fun.value();
     this.break_id_ = undefined;
   }
+  this.index_ = index;
 }
 
 
@@ -1867,6 +1871,22 @@ ScopeDetails.prototype.object = function() {
 };
 
 
+ScopeDetails.prototype.setVariableValueImpl = function(name, new_value) {
+  var raw_res;
+  if (!IS_UNDEFINED(this.break_id_)) {
+    %CheckExecutionState(this.break_id_);
+    raw_res = %SetScopeVariableValue(this.break_id_, this.frame_id_,
+        this.inlined_frame_id_, this.index_, name, new_value);
+  } else {
+    raw_res = %SetScopeVariableValue(this.fun_value_, null, null, this.index_,
+        name, new_value);
+  }
+  if (!raw_res) {
+    throw new Error("Failed to set variable value");
+  }
+};
+
+
 /**
  * Mirror object for scope of frame or function. Either frame or function must
  * be specified.
@@ -1914,6 +1934,11 @@ ScopeMirror.prototype.scopeObject = function() {
 };
 
 
+ScopeMirror.prototype.setVariableValue = function(name, new_value) {
+  this.details_.setVariableValueImpl(name, new_value);
+};
+
+
 /**
  * Mirror object for script source.
  * @param {Script} script The script object

src/runtime.cc

@@ -10902,6 +10902,52 @@ static Handle<JSObject> MaterializeClosure(Isolate* isolate,
 }
 
 
+// This method copies structure of MaterializeClosure method above.
+static bool SetClosureVariableValue(Isolate* isolate,
+                                    Handle<Context> context,
+                                    Handle<String> variable_name,
+                                    Handle<Object> new_value) {
+  ASSERT(context->IsFunctionContext());
+
+  Handle<SharedFunctionInfo> shared(context->closure()->shared());
+  Handle<ScopeInfo> scope_info(shared->scope_info());
+
+  // Context locals to the context extension.
+  for (int i = 0; i < scope_info->ContextLocalCount(); i++) {
+    Handle<String> next_name(scope_info->ContextLocalName(i));
+    if (variable_name->Equals(*next_name)) {
+      VariableMode mode;
+      InitializationFlag init_flag;
+      int context_index =
+          scope_info->ContextSlotIndex(*next_name, &mode, &init_flag);
+      if (context_index < 0) {
+        return false;
+      }
+      context->set(context_index, *new_value);
+      return true;
+    }
+  }
+
+  // Properties from the function context extension. This will
+  // be variables introduced by eval.
+  if (context->has_extension()) {
+    Handle<JSObject> ext(JSObject::cast(context->extension()));
+    if (ext->HasProperty(*variable_name)) {
+      // We don't expect this to do anything except replacing property value.
+      SetProperty(isolate,
+                  ext,
+                  variable_name,
+                  new_value,
+                  NONE,
+                  kNonStrictMode);
+      return true;
+    }
+  }
+
+  return false;
+}
+
+
 // Create a plain JSObject which materializes the scope for the specified
 // catch context.
 static Handle<JSObject> MaterializeCatchScope(Isolate* isolate,
@@ -11182,6 +11228,33 @@ class ScopeIterator {
     return Handle<JSObject>();
   }
 
+  bool SetVariableValue(Handle<String> variable_name,
+                        Handle<Object> new_value) {
+    ASSERT(!failed_);
+    switch (Type()) {
+      case ScopeIterator::ScopeTypeGlobal:
+        break;
+      case ScopeIterator::ScopeTypeLocal:
+        // TODO(2399): implement.
+        break;
+      case ScopeIterator::ScopeTypeWith:
+        break;
+      case ScopeIterator::ScopeTypeCatch:
+        // TODO(2399): implement.
+        break;
+      case ScopeIterator::ScopeTypeClosure:
+        return SetClosureVariableValue(isolate_, CurrentContext(),
+            variable_name, new_value);
+      case ScopeIterator::ScopeTypeBlock:
+        // TODO(2399): should we implement it?
+        break;
+      case ScopeIterator::ScopeTypeModule:
+        // TODO(2399): should we implement it?
+        break;
+    }
+    return false;
+  }
+
   Handle<ScopeInfo> CurrentScopeInfo() {
     ASSERT(!failed_);
     if (!nested_scope_chain_.is_empty()) {
@@ -11421,6 +11494,64 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetFunctionScopeDetails) {
 }
 
 
+static bool SetScopeVariableValue(ScopeIterator* it, int index,
+                                  Handle<String> variable_name,
+                                  Handle<Object> new_value) {
+  for (int n = 0; !it->Done() && n < index; it->Next()) {
+    n++;
+  }
+  if (it->Done()) {
+    return false;
+  }
+  return it->SetVariableValue(variable_name, new_value);
+}
+
+
+// Change variable value in closure or local scope
+// args[0]: number or JsFunction: break id or function
+// args[1]: number: frame index (when arg[0] is break id)
+// args[2]: number: inlined frame index (when arg[0] is break id)
+// args[3]: number: scope index
+// args[4]: string: variable name
+// args[5]: object: new value
+//
+// Return true if success and false otherwise
+RUNTIME_FUNCTION(MaybeObject*, Runtime_SetScopeVariableValue) {
+  HandleScope scope(isolate);
+  ASSERT(args.length() == 6);
+
+  // Check arguments.
+  CONVERT_NUMBER_CHECKED(int, index, Int32, args[3]);
+  CONVERT_ARG_HANDLE_CHECKED(String, variable_name, 4);
+  Handle<Object> new_value = args.at<Object>(5);
+
+  bool res;
+  if (args[0]->IsNumber()) {
+    Object* check;
+    { MaybeObject* maybe_check = Runtime_CheckExecutionState(
+        RUNTIME_ARGUMENTS(isolate, args));
+      if (!maybe_check->ToObject(&check)) return maybe_check;
+    }
+    CONVERT_SMI_ARG_CHECKED(wrapped_id, 1);
+    CONVERT_NUMBER_CHECKED(int, inlined_jsframe_index, Int32, args[2]);
+
+    // Get the frame where the debugging is performed.
+    StackFrame::Id id = UnwrapFrameId(wrapped_id);
+    JavaScriptFrameIterator frame_it(isolate, id);
+    JavaScriptFrame* frame = frame_it.frame();
+
+    ScopeIterator it(isolate, frame, inlined_jsframe_index);
+    res = SetScopeVariableValue(&it, index, variable_name, new_value);
+  } else {
+    CONVERT_ARG_HANDLE_CHECKED(JSFunction, fun, 0);
+    ScopeIterator it(isolate, fun);
+    res = SetScopeVariableValue(&it, index, variable_name, new_value);
+  }
+
+  return isolate->heap()->ToBoolean(res);
+}
+
+
 RUNTIME_FUNCTION(MaybeObject*, Runtime_DebugPrintScopes) {
   HandleScope scope(isolate);
   ASSERT(args.length() == 0);

src/runtime.h

@@ -433,6 +433,7 @@ namespace internal {
   F(GetScopeDetails, 4, 1) \
   F(GetFunctionScopeCount, 1, 1) \
   F(GetFunctionScopeDetails, 2, 1) \
+  F(SetScopeVariableValue, 6, 1) \
   F(DebugPrintScopes, 0, 1) \
   F(GetThreadCount, 1, 1) \
   F(GetThreadDetails, 2, 1) \

test/mjsunit/debug-set-variable-value.js

+// 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.
+var Debug = debug.Debug;
+
+// Accepts a function/closure 'fun' that must have a debugger statement inside.
+// A variable 'variable_name' must be initialized before debugger statement
+// and returned after the statement. The test will alter variable value when
+// on debugger statement and check that returned value reflects the change.
+function RunPauseTest(scope_number, variable_name, expected_new_value, fun) {
+  var old_value = fun();
+
+  var listener_delegate;
+  var listener_called = false;
+  var exception = null;
+
+  function listener_delegate(exec_state) {
+    var scope = exec_state.frame(0).scope(scope_number);
+    scope.setVariableValue(variable_name, expected_new_value);
+  }
+
+  function listener(event, exec_state, event_data, data) {
+    try {
+      if (event == Debug.DebugEvent.Break) {
+        listener_called = true;
+        listener_delegate(exec_state);
+      }
+    } catch (e) {
+      exception = e;
+    }
+  }
+
+  // Add the debug event listener.
+  Debug.setListener(listener);
+
+  var actual_new_value;
+  try {
+    actual_new_value = fun();
+  } finally {
+    Debug.setListener(null);
+  }
+
+  if (exception != null) {
+   assertUnreachable("Exception: " + exception);
+  }
+  assertTrue(listener_called);
+
+  assertTrue(old_value != actual_new_value);
+  assertTrue(expected_new_value == actual_new_value);
+}
+
+// Accepts a closure 'fun' that returns a variable from it's outer scope.
+// The test changes the value of variable via the handle to function and checks
+// that the return value changed accordingly.
+function RunClosureTest(scope_number, variable_name, expected_new_value, fun) {
+  var old_value = fun();
+
+  var fun_mirror = Debug.MakeMirror(fun);
+
+  var scope = fun_mirror.scope(scope_number);
+  scope.setVariableValue(variable_name, expected_new_value);
+
+  var actual_new_value = fun();
+
+  assertTrue(old_value != actual_new_value);
+  assertTrue(expected_new_value == actual_new_value);
+}
+
+// Test changing variable value when in pause
+RunPauseTest(1, 'v1', 5, (function Factory() {
+  var v1 = 'cat';
+  return function() {
+    debugger;
+    return v1;
+  }
+})());
+
+RunPauseTest(1, 'v2', 11, (function Factory(v2) {
+  return function() {
+    debugger;
+    return v2;
+  }
+})('dog'));
+
+RunPauseTest(3, 'foo', 77, (function Factory() {
+  var foo = "capybara";
+  return (function() {
+    var bar = "fish";
+    try {
+      throw {name: "test exception"};
+    } catch (e) {
+      return function() {
+        debugger;
+        bar = "beast";
+        return foo;
+      }
+    }
+  })();
+})());
+
+
+
+// Test changing variable value in closure by handle
+RunClosureTest(0, 'v1', 5, (function Factory() {
+  var v1 = 'cat';
+  return function() {
+    return v1;
+  }
+})());
+
+RunClosureTest(0, 'v2', 11, (function Factory(v2) {
+  return function() {
+    return v2;
+  }
+})('dog'));
+
+RunClosureTest(2, 'foo', 77, (function Factory() {
+  var foo = "capybara";
+  return (function() {
+    var bar = "fish";
+    try {
+      throw {name: "test exception"};
+    } catch (e) {
+      return function() {
+        bar = "beast";
+        return foo;
+      }
+    }
+  })();
+})());
+
+
+// Test value description protocol JSON
+assertEquals(true, Debug.TestApi.CommandProcessorResolveValue({value: true}));
+
+assertSame(null, Debug.TestApi.CommandProcessorResolveValue({type: "null"}));
+assertSame(undefined,
+    Debug.TestApi.CommandProcessorResolveValue({type: "undefined"}));
+
+assertSame("123", Debug.TestApi.CommandProcessorResolveValue(
+    {type: "string", stringDescription: "123"}));
+assertSame(123, Debug.TestApi.CommandProcessorResolveValue(
+    {type: "number", stringDescription: "123"}));
+
+assertSame(Number, Debug.TestApi.CommandProcessorResolveValue(
+    {handle: Debug.MakeMirror(Number).handle()}));
+assertSame(RunClosureTest, Debug.TestApi.CommandProcessorResolveValue(
+    {handle: Debug.MakeMirror(RunClosureTest).handle()}));
+