[webkit-reviews] review denied: [Bug 50745] Web Inspector: refactoring: make selected call frame a debugger model property : [Attachment 76034] Patch.

[bugzilla-daemon at webkit.org]

Yury Semikhatsky <yurys at chromium.org> has denied Pavel Podivilov
<podivilov at chromium.org>'s request for review:
Bug 50745: Web Inspector: refactoring: make selected call frame a debugger
model property
https://bugs.webkit.org/show_bug.cgi?id=50745

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

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

> WebCore/inspector/front-end/CallStackSidebarPane.js:31
> +   
WebInspector.debuggerModel.addEventListener("selected-call-frame-changed",
this._selectedCallFrameChanged, this);

Selected call frame is a property of CallStackSideBarPane there is no need in
moving it into the debuggerModel. If its owner(ScriptsPanel) needs that
information it can add "selected-call-frame-changed" listener to the sidebar.

> WebCore/inspector/front-end/CallStackSidebarPane.js:39
> +	   // FIXME: remove this when we'll have scripts in debugger model

Just get the map from the ScriptsPanel when you need it in the sidebar pane,
otherwise it's easy to break it by creating new sourceIDMap in the panel and
forget to update it here.

> WebCore/inspector/front-end/DebuggerModel.js:169
> +    selectedCallFrameIndex: function()

Consider changing these methods into getters for consistency with the rest of
the front-end code.