[webkit-reviews] review denied: [Bug 38132] Web Inspector: [Chromium] breakpoints are not preserved after reload : [Attachment 54321] Restore breakpoints associated with script's url once script is parsed

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Andrey Kosyakov
<caseq at chromium.org>'s request for review:
Bug 38132: Web Inspector: [Chromium] breakpoints are not preserved after reload
https://bugs.webkit.org/show_bug.cgi?id=38132

Attachment 54321: Restore breakpoints associated with script's url once script
is parsed
https://bugs.webkit.org/attachment.cgi?id=54321&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
WebCore/inspector/front-end/Breakpoint.js:74
 +	    return (this.url ? this.url : this.sourceID) + ":" + this.line;
It is not clear to me why this change is necessary. We'd like breakpoints to
point to exact scripts, while URL <-> sourceID mapping is external.


WebKit/chromium/src/js/DebuggerAgent.js:1100
 +	    if (line * 1 == line)
if (parseInt(line) === line)



WebKit/chromium/src/js/DebuggerAgent.js:1101
 +		WebInspector.restoredBreakpoint(sourceID, scriptUrl,
devtools.DebuggerAgent.v8ToWwebkitLineNumber_(line * 1), true,
breakpoints[line].condition());
line * 1 -> parseInt(line)


I think you should call devtools.DebuggerAgent.prototype.addBreakpoint for
disabled breakpoints as well and populate the maps with the appropriate
BreakpointInfo. You would need to cut the control flow further so that disabled
breakpoint does not get into the VM.