[webkit-reviews] review granted: [Bug 56931] Web Inspector: fix breakpoints positions in formatted scripts. : [Attachment 86629] Patch.

[bugzilla-daemon at webkit.org]

Yury Semikhatsky <yurys at chromium.org> has granted Pavel Podivilov
<podivilov at chromium.org>'s request for review:
Bug 56931: Web Inspector: fix breakpoints positions in formatted scripts.
https://bugs.webkit.org/show_bug.cgi?id=56931

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

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

> Source/WebCore/inspector/front-end/DebuggerPresentationModel.js:276
> +	   function loadSnippet(callback)

Can you turn it into a method on the PresentationBreakpoint? We don't usually
set closures as object methods.

> Source/WebCore/inspector/front-end/SourceFile.js:65
> +    requestMapping: function(callback)

It's not clear from the method name what kind of mapping is requested, consider
renaming it to requestPosistionMapping or requestSourceMapping.

Also why is this a request and not a synchronous getter?

> Source/WebCore/inspector/front-end/SourceFile.js:190
> +    this._scripts = scripts;

You lean on the fact that the scripts are sorted, can you use a name reflecting
this?