[webkit-reviews] review denied: [Bug 98385] Web Inspector: When uiSourceCode content has diverged from VM script, call frames should be shown in temporary script based uiSourceCodes. : [Attachment 167055] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Vsevolod Vlasov
<vsevik at chromium.org>'s request for review:
Bug 98385: Web Inspector: When uiSourceCode content has diverged from VM
script, call frames should be shown in temporary script based uiSourceCodes.
https://bugs.webkit.org/show_bug.cgi?id=98385

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

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=167055&action=review


> Source/WebCore/ChangeLog:8
> +	   Introduced uiSourceCode attribute concept and attributeChanged
event.

Please split this change, it has too many random components.

> Source/WebCore/inspector/front-end/DebuggerModel.js:779
> +	   return
this._script.rawLocationToUILocation(this.location.lineNumber,
this.location.columnNumber);

I think we should either expose live locations or location getters.

> Source/WebCore/inspector/front-end/JavaScriptSourceFrame.js:163
> +	   var breakpoints = this._getBreakpointDecorations();

This is quite expensive (you are doing it per key press, right?)
no _get prefixes in WebKit

> Source/WebCore/inspector/front-end/JavaScriptSourceFrame.js:172
> +	       this._setBreakpointDecorations(breakpoints);

You should instead get breakpoints from the vm version of UISourceCode.