DevTools: Event listener breakpoint should not stop if happened inside framework.

If we skip framework with StepInto after a stepping button was hit in front-end,
we should eventually stop somewhere in the user code. On the other hand, in case of
an Event breakpoint we should not stop at all, if the callback is handled inside the framework.

BUG=267592
R=yurys, pfeldman@chromium.org

Review URL: https://codereview.chromium.org/309013005

git-svn-id: svn://svn.chromium.org/blink/trunk@175471 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/LayoutTests/inspector/sources/debugger/frameworks-dom-xhr-event-breakpoints-expected.txt

@@ -22,7 +22,7 @@ Call stack:
     0) testElementClicked (frameworks-dom-xhr-event-breakpoints.html:26)
   * 1) Framework_bound (framework.js:105)
   * 2) Framework_eventListener (framework.js:87)
-    3) addListenerAndClick (frameworks-dom-xhr-event-breakpoints.html:31)
+    3) addListenerAndClick (frameworks-dom-xhr-event-breakpoints.html:35)
     4)  (:1)
 Paused on a "click" Event Listener.
 Debugger was disabled.

third_party/WebKit/LayoutTests/inspector/sources/debugger/frameworks-dom-xhr-event-breakpoints.html

@@ -27,8 +27,13 @@ function addListenerAndClick()
     }
 
     var button = document.getElementById("test");
-    Framework.addEventListener(button, "click", Framework.bind(testElementClicked, null), true);
+    var remover = Framework.addEventListener(button, "click", Framework.empty, true); // Should be ignored.
     button.click();
+    remover();
+
+    remover = Framework.addEventListener(button, "click", Framework.bind(testElementClicked, null), true);
+    button.click();
+    remover();
 }
 
 function test()

third_party/WebKit/LayoutTests/inspector/sources/debugger/frameworks-steppings-expected.txt

+
+Tests stepping into/over/out with framework black-boxing.
+
+Debugger was enabled.
+Set timer for test function.
+Call stack:
+    0) testFunction (frameworks-steppings.html:10)
+
+Executing StepInto...
+Executing StepInto...
+Call stack:
+    0) callback1 (frameworks-steppings.html:12)
+  * 1) Framework.safeRun (framework.js:8)
+    2) testFunction (frameworks-steppings.html:11)
+
+Executing StepInto...
+Call stack:
+    0) callback2 (frameworks-steppings.html:18)
+  * 1) Framework.safeRun (framework.js:8)
+  * 2) Framework.safeRun (framework.js:10)
+    3) callback1 (frameworks-steppings.html:12)
+  * 4) Framework.safeRun (framework.js:8)
+    5) testFunction (frameworks-steppings.html:11)
+
+Executing StepInto...
+Call stack:
+    0) callback2 (frameworks-steppings.html:19)
+  * 1) Framework.safeRun (framework.js:8)
+  * 2) Framework.safeRun (framework.js:10)
+    3) callback1 (frameworks-steppings.html:12)
+  * 4) Framework.safeRun (framework.js:8)
+    5) testFunction (frameworks-steppings.html:11)
+
+Executing StepInto...
+Call stack:
+    0) callback3 (frameworks-steppings.html:24)
+  * 1) Framework.safeRun (framework.js:8)
+  * 2) Framework.safeRun (framework.js:13)
+  * 3) Framework.safeRun (framework.js:10)
+    4) callback2 (frameworks-steppings.html:19)
+  * 5) Framework.safeRun (framework.js:8)
+  * 6) Framework.safeRun (framework.js:10)
+    7) callback1 (frameworks-steppings.html:12)
+  * 8) Framework.safeRun (framework.js:8)
+    9) testFunction (frameworks-steppings.html:11)
+
+Executing StepInto...
+Executing StepInto...
+Executing StepInto...
+Executing StepInto...
+Call stack:
+    0) callback4 (frameworks-steppings.html:32)
+  * 1) Framework_bound (framework.js:105)
+  * 2) Framework_bound (framework.js:105)
+  * 3) Framework_bound (framework.js:105)
+  * 4) Framework.safeRun (framework.js:8)
+    5) callback3 (frameworks-steppings.html:27)
+  * 6) Framework.safeRun (framework.js:8)
+  * 7) Framework.safeRun (framework.js:13)
+  * 8) Framework.safeRun (framework.js:10)
+    9) callback2 (frameworks-steppings.html:19)
+  * 10) Framework.safeRun (framework.js:8)
+  * 11) Framework.safeRun (framework.js:10)
+    12) callback1 (frameworks-steppings.html:12)
+  * 13) Framework.safeRun (framework.js:8)
+    14) testFunction (frameworks-steppings.html:11)
+
+Executing StepInto...
+Call stack:
+    0) callback4 (frameworks-steppings.html:36)
+  * 1) Framework_bound (framework.js:105)
+  * 2) Framework_bound (framework.js:105)
+  * 3) Framework_bound (framework.js:105)
+  * 4) Framework.safeRun (framework.js:8)
+    5) callback3 (frameworks-steppings.html:27)
+  * 6) Framework.safeRun (framework.js:8)
+  * 7) Framework.safeRun (framework.js:13)
+  * 8) Framework.safeRun (framework.js:10)
+    9) callback2 (frameworks-steppings.html:19)
+  * 10) Framework.safeRun (framework.js:8)
+  * 11) Framework.safeRun (framework.js:10)
+    12) callback1 (frameworks-steppings.html:12)
+  * 13) Framework.safeRun (framework.js:8)
+    14) testFunction (frameworks-steppings.html:11)
+
+Executing StepInto...
+Call stack:
+    0) callback4 (frameworks-steppings.html:38)
+  * 1) Framework_bound (framework.js:105)
+  * 2) Framework_bound (framework.js:105)
+  * 3) Framework_bound (framework.js:105)
+  * 4) Framework.safeRun (framework.js:8)
+    5) callback3 (frameworks-steppings.html:27)
+  * 6) Framework.safeRun (framework.js:8)
+  * 7) Framework.safeRun (framework.js:13)
+  * 8) Framework.safeRun (framework.js:10)
+    9) callback2 (frameworks-steppings.html:19)
+  * 10) Framework.safeRun (framework.js:8)
+  * 11) Framework.safeRun (framework.js:10)
+    12) callback1 (frameworks-steppings.html:12)
+  * 13) Framework.safeRun (framework.js:8)
+    14) testFunction (frameworks-steppings.html:11)
+
+Executing StepOut...
+Call stack:
+    0) callback3 (frameworks-steppings.html:28)
+  * 1) Framework.safeRun (framework.js:8)
+  * 2) Framework.safeRun (framework.js:13)
+  * 3) Framework.safeRun (framework.js:10)
+    4) callback2 (frameworks-steppings.html:19)
+  * 5) Framework.safeRun (framework.js:8)
+  * 6) Framework.safeRun (framework.js:10)
+    7) callback1 (frameworks-steppings.html:12)
+  * 8) Framework.safeRun (framework.js:8)
+    9) testFunction (frameworks-steppings.html:11)
+
+Executing StepOver...
+Call stack:
+    0) callback2 (frameworks-steppings.html:20)
+  * 1) Framework.safeRun (framework.js:8)
+  * 2) Framework.safeRun (framework.js:10)
+    3) callback1 (frameworks-steppings.html:12)
+  * 4) Framework.safeRun (framework.js:8)
+    5) testFunction (frameworks-steppings.html:11)
+
+Executing StepInto...
+Call stack:
+    0) callback1 (frameworks-steppings.html:13)
+  * 1) Framework.safeRun (framework.js:8)
+    2) testFunction (frameworks-steppings.html:11)
+
+Debugger was disabled.
+

third_party/WebKit/LayoutTests/inspector/sources/debugger/frameworks-steppings.html

+<html>
+<head>
+<script src="../../../http/tests/inspector/inspector-test.js"></script>
+<script src="../../../http/tests/inspector/debugger-test.js"></script>
+<script src="resources/framework.js"></script>
+<script>
+
+function testFunction()
+{
+    debugger;
+    Framework.safeRun(function callback1() {
+        Framework.safeRun(Framework.empty, callback2);
+    });
+}
+
+function callback2()
+{
+    Framework.safeRun(Framework.empty, Framework.empty); // Should be skipped: all callbacks are inside frameworks.
+    Framework.safeRun(Framework.empty, Framework.throwFrameworkException, callback3); // Should be enough to step into callback3
+}
+
+function callback3()
+{
+    var func = Framework.bind(callback4, null, 1);
+    func = Framework.bind(func, null, 2);
+    func = Framework.bind(func, null, 3);
+    Framework.safeRun(func, Framework.doSomeWork);
+}
+
+function callback4()
+{
+    Framework.safeRun(Framework.doSomeWork, function() {
+        return 0; // Should NOT step into this callback (otherwise too many StepIns)
+    });
+    try {
+        Framework.throwFrameworkException("message");
+    } catch (e) {
+        window.ex = e;
+    }
+}
+
+function test()
+{
+    var frameworkRegexString = "/framework\\.js$";
+    WebInspector.experimentsSettings.frameworksDebuggingSupport.enableForTest();
+    WebInspector.settings.skipStackFramesSwitch.set(true);
+    WebInspector.settings.skipStackFramesPattern.set(frameworkRegexString);
+
+    InspectorTest.setQuiet(true);
+    InspectorTest.startDebuggerTest(step1);
+
+    function step1()
+    {
+        InspectorTest.runTestFunctionAndWaitUntilPaused(didPause);
+    }
+
+    var actions = [
+        "Print", // debugger;
+        "StepInto", "StepInto", "Print", // callback1
+        "StepInto", "Print", // callback2
+        "StepInto", "Print", // callback2, skipped
+        "StepInto", "Print", // callback3
+        "StepInto", "StepInto", "StepInto", "StepInto", "Print", // callback4
+        "StepInto", "Print", // callback4, skipped
+        "StepInto", "Print", // callback4, inside catch
+        "StepOut", "Print", // return to callback3
+        "StepOver", "Print", // return to callback2
+        "StepInto", "Print", // return to callback1
+    ];
+
+    function didPause(callFrames, reason, breakpointIds, asyncStackTrace)
+    {
+        var action = actions.shift();
+        if (action === "Print") {
+            InspectorTest.captureStackTrace(callFrames);
+            InspectorTest.addResult("");
+            while (action === "Print")
+                action = actions.shift();
+        }
+
+        if (!action) {
+            InspectorTest.completeDebuggerTest();
+            return;
+        }
+
+        InspectorTest.addResult("Executing " + action + "...");
+        switch (action) {
+        case "StepInto":
+            WebInspector.panels.sources._stepIntoButton.element.click();
+            break;
+        case "StepOver":
+            WebInspector.panels.sources._stepOverButton.element.click();
+            break;
+        case "StepOut":
+            WebInspector.panels.sources._stepOutButton.element.click();
+            break;
+        default:
+            InspectorTest.addResult("FAIL: Unknown action: " + action);
+            InspectorTest.completeDebuggerTest();
+            return;
+        }
+        InspectorTest.waitUntilResumed(InspectorTest.waitUntilPaused.bind(InspectorTest, didPause));
+    }
+}
+
+</script>
+</head>
+
+<body onload="runTest()">
+<input type='button' onclick='testFunction()' value='Test'/>
+<p>
+Tests stepping into/over/out with framework black-boxing.
+</p>
+</body>
+</html>

third_party/WebKit/Source/bindings/v8/DebuggerScript.js

@@ -205,6 +205,11 @@ DebuggerScript.setPauseOnExceptionsState = function(newState)
         Debug.clearBreakOnUncaughtException();
 }
 
+DebuggerScript.frameCount = function(execState)
+{
+    return execState.frameCount();
+}
+
 DebuggerScript.currentCallFrame = function(execState, data)
 {
     var maximumLimit = data >> 2;

third_party/WebKit/Source/bindings/v8/ScriptDebugServer.cpp

@@ -168,8 +168,7 @@ void ScriptDebugServer::setPauseOnExceptionsState(PauseOnExceptionsState pauseOn
 
 void ScriptDebugServer::setPauseOnNextStatement(bool pause)
 {
-    if (isPaused())
-        return;
+    ASSERT(!isPaused());
     if (pause)
         v8::Debug::DebugBreak(m_isolate);
     else
@@ -314,6 +313,17 @@ bool ScriptDebugServer::setScriptSource(const String& sourceID, const String& ne
     return false;
 }
 
+int ScriptDebugServer::frameCount()
+{
+    ASSERT(isPaused());
+    ASSERT(!m_executionState.IsEmpty());
+    v8::Handle<v8::Value> argv[] = { m_executionState };
+    v8::Handle<v8::Value> result = callDebuggerMethod("frameCount", WTF_ARRAY_LENGTH(argv), argv);
+    if (result->IsInt32())
+        return result->Int32Value();
+    return 0;
+}
+
 PassRefPtr<JavaScriptCallFrame> ScriptDebugServer::wrapCallFrames(int maximumLimit, ScopeInfoDetails scopeDetails)
 {
     const int scopeBits = 2;

third_party/WebKit/Source/bindings/v8/ScriptDebugServer.h

@@ -80,6 +80,7 @@ public:
     ScriptValue currentCallFrames();
     ScriptValue currentCallFramesForAsyncStack();
     PassRefPtr<JavaScriptCallFrame> topCallFrameNoScopes();
+    int frameCount();
 
     class Task {
     public:

third_party/WebKit/Source/core/inspector/InspectorDebuggerAgent.cpp

@@ -76,7 +76,7 @@ static const char skipAllPausesExpiresOnReload[] = "skipAllPausesExpiresOnReload
 
 };
 
-static const int numberOfStepsBeforeStepOut = 20;
+static const int maxSkipStepInCount = 20;
 
 const char InspectorDebuggerAgent::backtraceObjectGroup[] = "backtrace";
 
@@ -105,8 +105,9 @@ InspectorDebuggerAgent::InspectorDebuggerAgent(InjectedScriptManager* injectedSc
     , m_pausedScriptState(nullptr)
     , m_javaScriptPauseScheduled(false)
     , m_debuggerStepScheduled(false)
+    , m_pausingOnNativeEvent(false)
     , m_listener(0)
-    , m_skipStepInCount(numberOfStepsBeforeStepOut)
+    , m_skippedStepInCount(0)
     , m_skipAllPauses(false)
 {
 }
@@ -529,14 +530,29 @@ ScriptDebugListener::SkipPauseRequest InspectorDebuggerAgent::shouldSkipStepPaus
 
     RefPtr<JavaScriptCallFrame> topFrame = scriptDebugServer().topCallFrameNoScopes();
     String scriptUrl = scriptURL(topFrame.get());
-    if (!scriptUrl.isEmpty() && m_cachedSkipStackRegExp->match(scriptUrl) != -1) {
-        if (m_skipStepInCount > 0) {
-            --m_skipStepInCount;
+    if (scriptUrl.isEmpty() || m_cachedSkipStackRegExp->match(scriptUrl) == -1)
+        return ScriptDebugListener::NoSkip;
+
+    if (m_skippedStepInCount == 0) {
+        m_minFrameCountForSkip = scriptDebugServer().frameCount();
+        m_skippedStepInCount = 1;
         return ScriptDebugListener::StepInto;
     }
+
+    if (m_skippedStepInCount < maxSkipStepInCount && topFrame->isAtReturn() && scriptDebugServer().frameCount() <= m_minFrameCountForSkip)
+        m_skippedStepInCount = maxSkipStepInCount;
+
+    if (m_skippedStepInCount >= maxSkipStepInCount) {
+        if (m_pausingOnNativeEvent) {
+            m_pausingOnNativeEvent = false;
+            m_skippedStepInCount = 0;
+            return ScriptDebugListener::Continue;
+        }
         return ScriptDebugListener::StepOut;
     }
-    return ScriptDebugListener::NoSkip;
+
+    ++m_skippedStepInCount;
+    return ScriptDebugListener::StepInto;
 }
 
 PassRefPtr<TypeBuilder::Debugger::Location> InspectorDebuggerAgent::resolveBreakpoint(const String& breakpointId, const String& scriptId, const ScriptBreakpoint& breakpoint, BreakpointSource source)
@@ -629,20 +645,20 @@ void InspectorDebuggerAgent::getFunctionDetails(ErrorString* errorString, const
 
 void InspectorDebuggerAgent::schedulePauseOnNextStatement(InspectorFrontend::Debugger::Reason::Enum breakReason, PassRefPtr<JSONObject> data)
 {
-    if (m_javaScriptPauseScheduled)
+    if (m_javaScriptPauseScheduled || isPaused())
         return;
     m_breakReason = breakReason;
     m_breakAuxData = data;
-    m_debuggerStepScheduled = true;
+    m_pausingOnNativeEvent = true;
     scriptDebugServer().setPauseOnNextStatement(true);
 }
 
 void InspectorDebuggerAgent::cancelPauseOnNextStatement()
 {
-    if (m_javaScriptPauseScheduled)
+    if (m_javaScriptPauseScheduled || isPaused())
         return;
     clearBreakDetails();
-    m_debuggerStepScheduled = false;
+    m_pausingOnNativeEvent = false;
     scriptDebugServer().setPauseOnNextStatement(false);
 }
 
@@ -742,11 +758,10 @@ void InspectorDebuggerAgent::didDeliverMutationRecords()
 
 void InspectorDebuggerAgent::pause(ErrorString*)
 {
-    if (m_javaScriptPauseScheduled)
+    if (m_javaScriptPauseScheduled || isPaused())
         return;
     clearBreakDetails();
     m_javaScriptPauseScheduled = true;
-    m_debuggerStepScheduled = false;
     scriptDebugServer().setPauseOnNextStatement(true);
 }
 
@@ -1111,7 +1126,7 @@ ScriptDebugListener::SkipPauseRequest InspectorDebuggerAgent::didPause(ScriptSta
         result = ScriptDebugListener::NoSkip; // Don't skip explicit breakpoints even if set in frameworks.
     else if (!exception.isEmpty())
         result = shouldSkipExceptionPause();
-    else if (m_debuggerStepScheduled)
+    else if (m_debuggerStepScheduled || m_pausingOnNativeEvent)
         result = shouldSkipStepPause();
     else
         result = ScriptDebugListener::NoSkip;
@@ -1123,8 +1138,6 @@ ScriptDebugListener::SkipPauseRequest InspectorDebuggerAgent::didPause(ScriptSta
     m_pausedScriptState = scriptState;
     m_currentCallStack = callFrames;
 
-    m_skipStepInCount = numberOfStepsBeforeStepOut;
-
     if (!exception.isEmpty()) {
         InjectedScript injectedScript = m_injectedScriptManager->injectedScriptFor(scriptState);
         if (!injectedScript.isEmpty()) {
@@ -1151,6 +1164,8 @@ ScriptDebugListener::SkipPauseRequest InspectorDebuggerAgent::didPause(ScriptSta
     m_frontend->paused(currentCallFrames(), m_breakReason, m_breakAuxData, hitBreakpointIds, currentAsyncStackTrace());
     m_javaScriptPauseScheduled = false;
     m_debuggerStepScheduled = false;
+    m_pausingOnNativeEvent = false;
+    m_skippedStepInCount = 0;
 
     if (!m_continueToLocationBreakpointId.isEmpty()) {
         scriptDebugServer().removeBreakpoint(m_continueToLocationBreakpointId);
@@ -1181,6 +1196,7 @@ void InspectorDebuggerAgent::breakProgram(InspectorFrontend::Debugger::Reason::E
     m_breakReason = breakReason;
     m_breakAuxData = data;
     m_debuggerStepScheduled = false;
+    m_pausingOnNativeEvent = false;
     scriptDebugServer().breakProgram();
 }
 
@@ -1195,6 +1211,7 @@ void InspectorDebuggerAgent::clear()
     clearBreakDetails();
     m_javaScriptPauseScheduled = false;
     m_debuggerStepScheduled = false;
+    m_pausingOnNativeEvent = false;
     ErrorString error;
     setOverlayMessage(&error, 0);
 }

third_party/WebKit/Source/core/inspector/InspectorDebuggerAgent.h

@@ -236,9 +236,11 @@ private:
     RefPtr<JSONObject> m_breakAuxData;
     bool m_javaScriptPauseScheduled;
     bool m_debuggerStepScheduled;
+    bool m_pausingOnNativeEvent;
     Listener* m_listener;
 
-    int m_skipStepInCount;
+    int m_skippedStepInCount;
+    int m_minFrameCountForSkip;
     bool m_skipAllPauses;
     OwnPtr<ScriptRegexp> m_cachedSkipStackRegExp;
     AsyncCallStackTracker m_asyncCallStackTracker;