[webkit-reviews] review denied: [Bug 48711] Web Inspector: event listener breakpoints are not preserved upon navigation / refresh. : [Attachment 72513] Patch.

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Pavel Podivilov
<podivilov at chromium.org>'s request for review:
Bug 48711: Web Inspector: event listener breakpoints are not preserved upon
navigation / refresh.
https://bugs.webkit.org/show_bug.cgi?id=48711

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

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

> WebCore/inspector/front-end/BreakpointManager.js:259
> +	   for (var i = 0; i < breakpoints.length; ++i) {

breakpoints can be non-array / empty.

> WebCore/inspector/front-end/BreakpointManager.js:261
> +		  
this.createEventListenerBreakpoint(breakpoints[i].condition.eventName);

Breakpoint can be of wrong structure here as well.

> WebCore/inspector/front-end/BreakpointManager.js:272
> +	   for (var i = 0; i < breakpoints.length; ++i) {

ditto

> WebCore/inspector/front-end/BreakpointManager.js:283
> +	   function didPushNodeByPathToFrontend(path, nodeId)

We usually put local functions above the usage.

> WebCore/inspector/front-end/Settings.js:68
> +    this.installProjectSetting("nativeBreakpoints", {});

{} -> []

> WebCore/inspector/front-end/inspector.js:1570
> +    this.applicationSettings.inspectedURLChanged(url);

It is strange that project-related settings that are affected by the URL change
are accessed via applicationSettings object.