Bug 215364 – Web Inspector: breakpoint condition should be evaluated before the ignore count

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Devin Rousso]

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

[Radar WebKit Bug Importer]

<rdar://problem/67310703>

[Devin Rousso]

Created attachment 407039 [details]
Patch

[Joseph Pecoraro]

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

[Devin Rousso]

Created attachment 407209 [details]
Patch

[EWS]

Committed r266138: <https://trac.webkit.org/changeset/266138>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 407209 [details].