Support 'restart call frame' debug command

Support 'restart call frame' debug command

Committed: https://code.google.com/p/v8/source/detail?r=11836

[Peter Rybin, 2012-06-14 00:54:29]

Hi Yang

This is a simple and long-awaited feature for debugger.
It is implemented as a LiveEdit's sidekick.

Unit test is my TODO.

Peter

[Yang, 2012-06-14 09:09:28]

Mostly style-related comments. Please run the presubmit test. It looks good so
far, but it would be great to include a test case.

http://codereview.chromium.org/10544151/diff/1/src/debug-debugger.js
File src/debug-debugger.js (right):

http://codereview.chromium.org/10544151/diff/1/src/debug-debugger.js#newcode2407
src/debug-debugger.js:2407: // No frames no evaluate in frame.
"to evaluate"?

http://codereview.chromium.org/10544151/diff/1/src/liveedit-debugger.js
File src/liveedit-debugger.js (right):

http://codereview.chromium.org/10544151/diff/1/src/liveedit-debugger.js#newco...
src/liveedit-debugger.js:50: const NEEDS_STEP_IN_PROPERTY_NAME =
"stack_update_needs_step_in";
Do not use const in native javascript.

http://codereview.chromium.org/10544151/diff/1/src/liveedit.cc
File src/liveedit.cc (right):

http://codereview.chromium.org/10544151/diff/1/src/liveedit.cc#newcode1603
src/liveedit.cc:1603: MultipleFunctionTarget(Handle<JSArray> shared_info_array,
Handle<JSArray> result)
80 char limit.

http://codereview.chromium.org/10544151/diff/1/src/liveedit.cc#newcode1607
src/liveedit.cc:1607: }
You can put the empty function body on the same line as the last item in the
initialization list.

http://codereview.chromium.org/10544151/diff/1/src/liveedit.cc#newcode1608
src/liveedit.cc:1608: bool MatchActivation(StackFrame* frame,
LiveEdit::FunctionPatchabilityStatus status) {
80 char limit.

http://codereview.chromium.org/10544151/diff/1/src/liveedit.cc#newcode1638
src/liveedit.cc:1638: frame, LiveEdit::FUNCTION_BLOCKED_UNDER_NATIVE_CODE)) {
4 spaces more indent.

http://codereview.chromium.org/10544151/diff/1/src/liveedit.cc#newcode1661
src/liveedit.cc:1661: frame, LiveEdit::FUNCTION_BLOCKED_ON_ACTIVE_STACK)) {
ditto.

http://codereview.chromium.org/10544151/diff/1/src/liveedit.cc#newcode1674
src/liveedit.cc:1674: frame, LiveEdit::FUNCTION_BLOCKED_UNDER_NATIVE_CODE)) {
ditto.

http://codereview.chromium.org/10544151/diff/1/src/liveedit.cc#newcode1722
src/liveedit.cc:1722: const char* message =
DropActivationsInActiveThreadImpl(target, do_drop, zone);
80 char limit.

http://codereview.chromium.org/10544151/diff/1/src/liveedit.cc#newcode1810
src/liveedit.cc:1810: bool MatchActivation(StackFrame* frame,
LiveEdit::FunctionPatchabilityStatus status) {
80 char limit.

http://codereview.chromium.org/10544151/diff/1/src/mirror-debugger.js
File src/mirror-debugger.js (right):

http://codereview.chromium.org/10544151/diff/1/src/mirror-debugger.js#newcode...
src/mirror-debugger.js:1755: if (result === void 0) {
I think using the IS_UNDEFINED macro is nicer.

http://codereview.chromium.org/10544151/diff/1/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/10544151/diff/1/src/runtime.cc#newcode12796
src/runtime.cc:12796: // Returns 'true' if success or undefined or error message
elseway.
Ambiguous sentence. How about "Returns true if successful. Otherwise returns
undefined or an error message."

http://codereview.chromium.org/10544151/diff/1/src/runtime.cc#newcode12825
src/runtime.cc:12825: const char* error_message =
LiveEdit::RestartFrame(it.frame(), isolate->zone());
80 character limit.

[Peter Rybin, 2012-06-14 22:08:03]

Hi Yang

Thank you for review. I'm sorry for a half-done patch. Hope this one is better
plus it there is a test.

Peter

http://codereview.chromium.org/10544151/diff/1/src/debug-debugger.js
File src/debug-debugger.js (right):

http://codereview.chromium.org/10544151/diff/1/src/debug-debugger.js#newcode2407
src/debug-debugger.js:2407: // No frames no evaluate in frame.
On 2012/06/14 09:09:28, Yang wrote:
> "to evaluate"?

Done.

http://codereview.chromium.org/10544151/diff/1/src/liveedit-debugger.js
File src/liveedit-debugger.js (right):

http://codereview.chromium.org/10544151/diff/1/src/liveedit-debugger.js#newco...
src/liveedit-debugger.js:50: const NEEDS_STEP_IN_PROPERTY_NAME =
"stack_update_needs_step_in";
On 2012/06/14 09:09:28, Yang wrote:
> Do not use const in native javascript.

Done.

http://codereview.chromium.org/10544151/diff/1/src/liveedit.cc
File src/liveedit.cc (right):

http://codereview.chromium.org/10544151/diff/1/src/liveedit.cc#newcode1603
src/liveedit.cc:1603: MultipleFunctionTarget(Handle<JSArray> shared_info_array,
Handle<JSArray> result)
On 2012/06/14 09:09:28, Yang wrote:
> 80 char limit.

Done.

http://codereview.chromium.org/10544151/diff/1/src/liveedit.cc#newcode1607
src/liveedit.cc:1607: }
On 2012/06/14 09:09:28, Yang wrote:
> You can put the empty function body on the same line as the last item in the
> initialization list.

Done.

http://codereview.chromium.org/10544151/diff/1/src/liveedit.cc#newcode1608
src/liveedit.cc:1608: bool MatchActivation(StackFrame* frame,
LiveEdit::FunctionPatchabilityStatus status) {
On 2012/06/14 09:09:28, Yang wrote:
> 80 char limit.

Done.

http://codereview.chromium.org/10544151/diff/1/src/liveedit.cc#newcode1638
src/liveedit.cc:1638: frame, LiveEdit::FUNCTION_BLOCKED_UNDER_NATIVE_CODE)) {
On 2012/06/14 09:09:28, Yang wrote:
> 4 spaces more indent.

Done.

http://codereview.chromium.org/10544151/diff/1/src/liveedit.cc#newcode1661
src/liveedit.cc:1661: frame, LiveEdit::FUNCTION_BLOCKED_ON_ACTIVE_STACK)) {
On 2012/06/14 09:09:28, Yang wrote:
> ditto.

Done.

http://codereview.chromium.org/10544151/diff/1/src/liveedit.cc#newcode1674
src/liveedit.cc:1674: frame, LiveEdit::FUNCTION_BLOCKED_UNDER_NATIVE_CODE)) {
On 2012/06/14 09:09:28, Yang wrote:
> ditto.

Done.

http://codereview.chromium.org/10544151/diff/1/src/liveedit.cc#newcode1722
src/liveedit.cc:1722: const char* message =
DropActivationsInActiveThreadImpl(target, do_drop, zone);
On 2012/06/14 09:09:28, Yang wrote:
> 80 char limit.

Done.

http://codereview.chromium.org/10544151/diff/1/src/liveedit.cc#newcode1810
src/liveedit.cc:1810: bool MatchActivation(StackFrame* frame,
LiveEdit::FunctionPatchabilityStatus status) {
On 2012/06/14 09:09:28, Yang wrote:
> 80 char limit.

Done.

http://codereview.chromium.org/10544151/diff/1/src/mirror-debugger.js
File src/mirror-debugger.js (right):

http://codereview.chromium.org/10544151/diff/1/src/mirror-debugger.js#newcode...
src/mirror-debugger.js:1755: if (result === void 0) {
On 2012/06/14 09:09:28, Yang wrote:
> I think using the IS_UNDEFINED macro is nicer.

Done.

http://codereview.chromium.org/10544151/diff/1/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/10544151/diff/1/src/runtime.cc#newcode12796
src/runtime.cc:12796: // Returns 'true' if success or undefined or error message
elseway.
On 2012/06/14 09:09:28, Yang wrote:
> Ambiguous sentence. How about "Returns true if successful. Otherwise returns
> undefined or an error message."

Perfect fit for 80 columns!
Done

http://codereview.chromium.org/10544151/diff/1/src/runtime.cc#newcode12825
src/runtime.cc:12825: const char* error_message =
LiveEdit::RestartFrame(it.frame(), isolate->zone());
On 2012/06/14 09:09:28, Yang wrote:
> 80 character limit.

Done.

[Yang, 2012-06-15 07:18:15]

LGTM with nits.

http://codereview.chromium.org/10544151/diff/1008/src/liveedit-debugger.js
File src/liveedit-debugger.js (right):

http://codereview.chromium.org/10544151/diff/1008/src/liveedit-debugger.js#ne...
src/liveedit-debugger.js:1: // Copyright 2010 the V8 project authors. All rights
reserved.
2012

http://codereview.chromium.org/10544151/diff/1008/src/liveedit.h
File src/liveedit.h (right):

http://codereview.chromium.org/10544151/diff/1008/src/liveedit.h#newcode1
src/liveedit.h:1: // Copyright 2010 the V8 project authors. All rights reserved.
2012

http://codereview.chromium.org/10544151/diff/1008/src/mirror-debugger.js
File src/mirror-debugger.js (right):

http://codereview.chromium.org/10544151/diff/1008/src/mirror-debugger.js#newc...
src/mirror-debugger.js:1: // Copyright 2006-2008 the V8 project authors. All
rights reserved.
2012

http://codereview.chromium.org/10544151/diff/1008/test/mjsunit/debug-liveedit...
File test/mjsunit/debug-liveedit-restart-frame.js (right):

http://codereview.chromium.org/10544151/diff/1008/test/mjsunit/debug-liveedit...
test/mjsunit/debug-liveedit-restart-frame.js:1: // Copyright 2010 the V8 project
authors. All rights reserved.
2012

http://codereview.chromium.org/10544151/diff/1008/test/mjsunit/debug-liveedit...
test/mjsunit/debug-liveedit-restart-frame.js:56: // Restarter doesn't work on
slim function when stopped on 'debugger' statement.
80 char limit.

[Peter Rybin, 2012-06-15 16:51:45]

http://codereview.chromium.org/10544151/diff/1008/src/liveedit-debugger.js
File src/liveedit-debugger.js (right):

http://codereview.chromium.org/10544151/diff/1008/src/liveedit-debugger.js#ne...
src/liveedit-debugger.js:1: // Copyright 2010 the V8 project authors. All rights
reserved.
On 2012/06/15 07:18:15, Yang wrote:
> 2012

Done.

http://codereview.chromium.org/10544151/diff/1008/src/liveedit.h
File src/liveedit.h (right):

http://codereview.chromium.org/10544151/diff/1008/src/liveedit.h#newcode1
src/liveedit.h:1: // Copyright 2010 the V8 project authors. All rights reserved.
On 2012/06/15 07:18:15, Yang wrote:
> 2012

Done.

http://codereview.chromium.org/10544151/diff/1008/src/mirror-debugger.js
File src/mirror-debugger.js (right):

http://codereview.chromium.org/10544151/diff/1008/src/mirror-debugger.js#newc...
src/mirror-debugger.js:1: // Copyright 2006-2008 the V8 project authors. All
rights reserved.
On 2012/06/15 07:18:15, Yang wrote:
> 2012

Done.

http://codereview.chromium.org/10544151/diff/1008/test/mjsunit/debug-liveedit...
File test/mjsunit/debug-liveedit-restart-frame.js (right):

http://codereview.chromium.org/10544151/diff/1008/test/mjsunit/debug-liveedit...
test/mjsunit/debug-liveedit-restart-frame.js:1: // Copyright 2010 the V8 project
authors. All rights reserved.
On 2012/06/15 07:18:15, Yang wrote:
> 2012

Done.

http://codereview.chromium.org/10544151/diff/1008/test/mjsunit/debug-liveedit...
test/mjsunit/debug-liveedit-restart-frame.js:56: // Restarter doesn't work on
slim function when stopped on 'debugger' statement.
On 2012/06/15 07:18:15, Yang wrote:
> 80 char limit.

Done.