Changeset 266138 in webkit

Timestamp:

Aug 25, 2020 11:49:57 AM (4 years ago)

Author:

Devin Rousso

Message:

Web Inspector: breakpoint condition should be evaluated before the ignore count
​https://bugs.webkit.org/show_bug.cgi?id=215364
<rdar://problem/67310703>

Reviewed by Joseph Pecoraro.

Source/JavaScriptCore:

Previously, when pausing, JSC::Breakpoint would check that it's ignoreCount before it
would even attempt to evaluate it's condition. This meant that a JSC::Breakpoint with
a condition of foo === 42 and an ignoreCount of 3 would ignore the first three
pauses and then only pause if foo === 42. This is likely contrary to the expectation of
most users (especially since the condition input is before the ignoreCount input in
the Web Inspector frontend UI) in that they would probably expect to ignore the first
three pauses if foo === 42.

• debugger/Breakpoint.cpp:

(JSC::Breakpoint::shouldPause):

LayoutTests:

• inspector/debugger/resources/condition-ignoreCount.js: Added.

(trigger): Added.

• inspector/debugger/setBreakpoint-condition-ignoreCount.html: Added.
• inspector/debugger/setBreakpoint-condition-ignoreCount-expected.txt: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/inspector/debugger/resources/condition-ignoreCount.js (added)
• LayoutTests/inspector/debugger/setBreakpoint-condition-ignoreCount-expected.txt (added)
• LayoutTests/inspector/debugger/setBreakpoint-condition-ignoreCount.html (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/debugger/Breakpoint.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2020-08-25  Devin Rousso  <drousso@apple.com>
+
+        Web Inspector: breakpoint condition should be evaluated before the ignore count
+        https://bugs.webkit.org/show_bug.cgi?id=215364
+        <rdar://problem/67310703>
+
+        Reviewed by Joseph Pecoraro.
+
+        * inspector/debugger/resources/condition-ignoreCount.js: Added.
+        (trigger): Added.
+        * inspector/debugger/setBreakpoint-condition-ignoreCount.html: Added.
+        * inspector/debugger/setBreakpoint-condition-ignoreCount-expected.txt: Added.
+
 2020-08-25  Hector Lopez  <hector_i_lopez@apple.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2020-08-25  Devin Rousso  <drousso@apple.com>
+
+        Web Inspector: breakpoint condition should be evaluated before the ignore count
+        https://bugs.webkit.org/show_bug.cgi?id=215364
+        <rdar://problem/67310703>
+
+        Reviewed by Joseph Pecoraro.
+
+        Previously, when pausing, `JSC::Breakpoint` would check that it's `ignoreCount` before it
+        would even attempt to evaluate it's `condition`. This meant that a `JSC::Breakpoint` with
+        a `condition` of `foo === 42` and an `ignoreCount` of `3` would ignore the first three
+        pauses and then only pause if `foo === 42`. This is likely contrary to the expectation of
+        most users (especially since the `condition` input is before the `ignoreCount` input in
+        the Web Inspector frontend UI) in that they would probably expect to ignore the first
+        three pauses if `foo === 42`.
+
+        * debugger/Breakpoint.cpp:
+        (JSC::Breakpoint::shouldPause):
+
 2020-08-25  Alexey Shvayka  <shvaikalesh@gmail.com>
 

trunk/Source/JavaScriptCore/debugger/Breakpoint.cpp

@@ -76 +76 @@
 bool Breakpoint::shouldPause(Debugger& debugger, JSGlobalObject* globalObject)
 {
-    if (++m_hitCount <= m_ignoreCount)
+    if (!debugger.evaluateBreakpointCondition(*this, globalObject))
         return false;
 
-    return debugger.evaluateBreakpointCondition(*this, globalObject);
+    return ++m_hitCount > m_ignoreCount;
 }