[webkit-reviews] review denied: [Bug 41461] Web Inspector: all breakpoints are activated on page reload : [Attachment 60511] Proposed 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 41461: Web Inspector: all breakpoints are activated on page reload
https://bugs.webkit.org/show_bug.cgi?id=41461

Attachment 60511: Proposed patch.
https://bugs.webkit.org/attachment.cgi?id=60511&action=review

------- Additional Comments from Yury Semikhatsky <yurys at chromium.org>
LayoutTests/inspector/debugger-breakpoints-not-activated-on-reload.html:75
 +   * Checks that all expected scripts are present in the scripts list
You don't need to port these comments.

WebCore/inspector/front-end/ScriptsPanel.js:614
 +	    if (!options)
I's rather change this to options = options || {};

LayoutTests/inspector/debugger-breakpoints-not-activated-on-reload.html:164
 +  Tests that breakpoints are not activated on page reload.
We usually put link to a bug in test descriptions.

LayoutTests/inspector/debugger-breakpoints-not-activated-on-reload.html:125
 +  function frontend_showMainPageScriptSource(scriptName, callback)
frontend_showScriptSource would better reflect what it does since it doesn't
use the fact that scriptName is the name of the main resource.

LayoutTests/inspector/debugger-breakpoints-not-activated-on-reload.html:145
 +	    WebInspector.currentPanel._showScriptOrResource(scriptResource);
Use scriptsPanel instead of WebInspector.currentPanel as you do below.


LayoutTests/inspector/debugger-breakpoints-not-activated-on-reload.html:27
 +	var testName = "debugger-breakpoints-not-activated-on-reload.html";
You can get the test name from document.location

LayoutTests/inspector/debugger-breakpoints-not-activated-on-reload.html:58
 +		testController.notifyDone();
You need to wait until all scripts are pushed to the front-end again, right?
r- for that.