[webkit-reviews] review canceled: [Bug 77409] Web Inspector: DOMDebugger.setEventListenerBreakpoint should accept regular DOM event names. : [Attachment 124678] Patch

[bugzilla-daemon at webkit.org]

Yury Semikhatsky <yurys at chromium.org> has canceled Pavel Feldman
<pfeldman at chromium.org>'s request for review:
Bug 77409: Web Inspector: DOMDebugger.setEventListenerBreakpoint should accept
regular DOM event names.
https://bugs.webkit.org/show_bug.cgi?id=77409

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

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


> Source/WebCore/inspector/Inspector.json:2283
> +		   "name": "setInstrumentationBreakpoint",

setInstrumentationBreakpoint is a bit confusing, probably setTimerBreakpoint as
we don't support other types of such breakpoints?

> Source/WebCore/inspector/InspectorDOMDebuggerAgent.cpp:153
> +    removeBreakpoint(error, String(listenerEventCategoryType) + eventName);

Why not aggregate breakpoints by categories in the storage?

> Source/WebCore/inspector/front-end/BreakpointsSidebarPane.js:569
> +	   if
(eventName.indexOf(WebInspector.EventListenerBreakpointsSidebarPane.categotyLis
tener) === 0)

You can pass category and event type as two separate parameters then you
wouldn't need to first combine and then split them.