It seems very odd to me that the hit count is incremented without checking whether or not the condition matches. As an example, <https://webkit.org/b/215362> adds the ability for event breakpoints to be configured, meaning that the All Events breakpoint could be configured with a `window.event.type.includes("key")` condition and an ignore count of 2. There are two ways this can be interpreted a) ignore the first two events (whatever they may be) and then break on every event that includes "key" b) pause on all but the first two "key" events I think that (b) is much preferred and more intuitive/useful. additionally, in the frontend UI, the condition input is above the ignore count input
<rdar://problem/67310703>
Created attachment 407039 [details] Patch
Comment on attachment 407039 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=407039&action=review r=me > Source/JavaScriptCore/debugger/Breakpoint.cpp:78 > + return debugger.evaluateBreakpointCondition(*this, globalObject) && ++m_hitCount > m_ignoreCount; I find this one line unnecessarily complicated when all combined on one line. I'd break it up kinda like it was before
Created attachment 407209 [details] Patch
Committed r266138: <https://trac.webkit.org/changeset/266138> All reviewed patches have been landed. Closing bug and clearing flags on attachment 407209 [details].