[webkit-reviews] review granted: [Bug 58047] Web Inspector: remove "enabled" from the setBreakpoint protocol. : [Attachment 88658] Patch

[bugzilla-daemon at webkit.org]

Yury Semikhatsky <yurys at chromium.org> has granted Pavel Feldman
<pfeldman at chromium.org>'s request for review:
Bug 58047: Web Inspector: remove "enabled" from the setBreakpoint protocol.
https://bugs.webkit.org/show_bug.cgi?id=58047

Attachment 88658: Patch
https://bugs.webkit.org/attachment.cgi?id=88658&action=review

------- Additional Comments from Yury Semikhatsky <yurys at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=88658&action=review

> Source/WebCore/inspector/front-end/DebuggerPresentationModel.js:123
> +		   breakpoint.debuggerId = breakpointData.debuggerId;

Extract method.

> Source/WebCore/inspector/front-end/DebuggerPresentationModel.js:168
> +	   if (!sourceFile)

Please remove this, it never happens.

> Source/WebCore/inspector/front-end/DebuggerPresentationModel.js:274
> +	       this._breakpointAdded(breakpoint);

New breakpoint set should be saved.

> Source/WebCore/inspector/front-end/DebuggerPresentationModel.js:339
> +	       this._removeBreakpointFromDebugger(breakpoint);

please pass afterUpdate as a callback to _removeBreakpointFromDebugger

> Source/WebCore/inspector/front-end/DebuggerPresentationModel.js:395
> +    _breakpointRemoved: function(breakpoint)

Please inline this method.