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#newcode50
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#newcode1755
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.
http://codereview.chromium.org/10544151/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
