Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(111)

Issue 2799037: Provide actual breakpoints locations in response to setBreakpoint and listBreakpoints requests. (Closed)

Created:
10 years, 6 months ago by podivilov
Modified:
9 years, 4 months ago
CC:
v8-dev
Visibility:
Public.

Description

Provide actual breakpoints locations in response to setBreakpoint and listBreakpoints requests. Committed: http://code.google.com/p/v8/source/detail?r=4965

Patch Set 1 #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+55 lines, -21 lines) Patch
M src/debug-debugger.js View 9 chunks +46 lines, -14 lines 4 comments Download
M src/runtime.cc View 1 chunk +1 line, -1 line 0 comments Download
M test/mjsunit/debug-setbreakpoint.js View 4 chunks +8 lines, -6 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
podivilov
10 years, 6 months ago (2010-06-25 17:05:22 UTC) #1
Søren Thygesen Gjesse
LGTM http://codereview.chromium.org/2799037/diff/1/2 File src/debug-debugger.js (right): http://codereview.chromium.org/2799037/diff/1/2#newcode400 src/debug-debugger.js:400: if (IS_UNDEFINED(actual_position)) { Is this undefined check still ...
10 years, 6 months ago (2010-06-28 06:50:30 UTC) #2
podivilov
10 years, 5 months ago (2010-06-28 12:10:05 UTC) #3
http://codereview.chromium.org/2799037/diff/1/2
File src/debug-debugger.js (right):

http://codereview.chromium.org/2799037/diff/1/2#newcode400
src/debug-debugger.js:400: if (IS_UNDEFINED(actual_position)) {
On 2010/06/28 06:50:30, Søren Gjesse wrote:
> Is this undefined check still needed after the change to runtime.cc?
Yes, because Runtime_SetScriptBreakPoint returns undefined in case no
SharedFunctionInfo is available for the given position.

http://codereview.chromium.org/2799037/diff/1/2#newcode613
src/debug-debugger.js:613: var pos = %SetFunctionBreakPoint(func,
source_position, break_point);
On 2010/06/28 06:50:30, Søren Gjesse wrote:
> Maybe pos -> actual_position as above

Done.

Powered by Google App Engine
This is Rietveld 408576698