[webkit-reviews] review denied: [Bug 54645] Web Inspector: move source mapping from SourceFrame to ScriptsPanel. : [Attachment 82953] Patch + test.

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Pavel Podivilov
<podivilov at chromium.org>'s request for review:
Bug 54645: Web Inspector: move source mapping from SourceFrame to ScriptsPanel.
https://bugs.webkit.org/show_bug.cgi?id=54645

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

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

> LayoutTests/http/tests/inspector/debugger-test.js:123
> +	   InspectorTest._addSniffer(WebInspector.panels.scripts,
"_addScriptToFilesMenu", InspectorTest.showScriptSource.bind(InspectorTest,
scriptName, callback));

Nit: It is not clear what part of the functionality you intend to test:
debugger model or scripts panel with this harness.

> LayoutTests/http/tests/inspector/inspector-test.js:138
> +InspectorTest.runTest = function(steps)

Please use runTestSuite or runDebuggerTestSuite.

> LayoutTests/http/tests/inspector/inspector-test.js:149
> +		   InspectorTest.completeTest();

What if that was a debugger test?

> Source/WebCore/inspector/front-end/ScriptsPanel.js:192
> +   
WebInspector.debuggerModel.addEventListener(WebInspector.DebuggerModel.Events.B
reakpointAdded, this._breakpointAdded, this);

It is strange that the client that adds breakpoints is listening to the
breakpoints addition. r- for that.