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

Issue 309013005: DevTools: Event listener breakpoint should not stop if happened inside framework. (Closed)

Created:
6 years, 6 months ago by aandrey

Modified:
6 years, 6 months ago

Reviewers:
pfeldman, yurys

CC:
blink-reviews, caseq+blink_chromium.org, loislo+blink_chromium.org, eustas+blink_chromium.org, malch+blink_chromium.org, blink-reviews-bindings_chromium.org, yurys+blink_chromium.org, lushnikov+blink_chromium.org, abarth-chromium, vsevik+blink_chromium.org, pfeldman+blink_chromium.org, paulirish+reviews_chromium.org, devtools-reviews_chromium.org, arv+blink, sergeyv+blink_chromium.org, aandrey+blink_chromium.org

Base URL:
svn://svn.chromium.org/blink/trunk

Visibility:
Public.

More Reviews

Description

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 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=175471

Patch Set 1 #

Patch Set 2 : one more test #

Total comments: 5

Patch Set 3 : #

Patch Set 4 : addressed #

Created: 6 years, 6 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+309 lines, -21 lines)			Patch
M	LayoutTests/inspector/sources/debugger/frameworks-dom-xhr-event-breakpoints.html	View		1 chunk	+6 lines, -1 line	0 comments	Download
M	LayoutTests/inspector/sources/debugger/frameworks-dom-xhr-event-breakpoints-expected.txt	View		1 chunk	+1 line, -1 line	0 comments	Download
A	LayoutTests/inspector/sources/debugger/frameworks-steppings.html	View	1 2	1 chunk	+115 lines, -0 lines	0 comments	Download
A	LayoutTests/inspector/sources/debugger/frameworks-steppings-expected.txt	View	1	1 chunk	+133 lines, -0 lines	0 comments	Download
M	Source/bindings/v8/DebuggerScript.js	View		1 chunk	+5 lines, -0 lines	0 comments	Download
M	Source/bindings/v8/ScriptDebugServer.h	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	Source/bindings/v8/ScriptDebugServer.cpp	View	1 2 3	2 chunks	+12 lines, -2 lines	0 comments	Download
M	Source/core/inspector/InspectorDebuggerAgent.h	View	1 2 3	1 chunk	+3 lines, -1 line	0 comments	Download
M	Source/core/inspector/InspectorDebuggerAgent.cpp	View	1 2 3	10 chunks	+33 lines, -16 lines	0 comments	Download

Messages

Total messages: 8 (0 generated)

Expand Messages | Collapse Messages

yurys

https://codereview.chromium.org/309013005/diff/20001/Source/core/inspector/InspectorDebuggerAgent.cpp File Source/core/inspector/InspectorDebuggerAgent.cpp (right): https://codereview.chromium.org/309013005/diff/20001/Source/core/inspector/InspectorDebuggerAgent.cpp#newcode543 Source/core/inspector/InspectorDebuggerAgent.cpp:543: m_skippedStepInCount = maxSkipStepInCount; You could change if condition at ...

6 years, 6 months ago (2014-06-03 13:02:11 UTC) #3

yurys

lgtm https://codereview.chromium.org/309013005/diff/20001/Source/bindings/v8/ScriptDebugServer.cpp File Source/bindings/v8/ScriptDebugServer.cpp (right): https://codereview.chromium.org/309013005/diff/20001/Source/bindings/v8/ScriptDebugServer.cpp#newcode320 Source/bindings/v8/ScriptDebugServer.cpp:320: v8::Handle<v8::Function> frameCountFunction = v8::Local<v8::Function>::Cast(m_debuggerScript.newLocal(m_isolate)->Get(v8AtomicString(m_isolate, "frameCount"))); AFAIU this branch ...

6 years, 6 months ago (2014-06-03 13:19:42 UTC) #4

aandrey

https://codereview.chromium.org/309013005/diff/20001/Source/bindings/v8/ScriptDebugServer.cpp File Source/bindings/v8/ScriptDebugServer.cpp (right): https://codereview.chromium.org/309013005/diff/20001/Source/bindings/v8/ScriptDebugServer.cpp#newcode320 Source/bindings/v8/ScriptDebugServer.cpp:320: v8::Handle<v8::Function> frameCountFunction = v8::Local<v8::Function>::Cast(m_debuggerScript.newLocal(m_isolate)->Get(v8AtomicString(m_isolate, "frameCount"))); On 2014/06/03 13:19:43, yurys ...

6 years, 6 months ago (2014-06-04 08:51:27 UTC) #5

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/aandrey@chromium.org/309013005/40001

6 years, 6 months ago (2014-06-04 08:56:44 UTC) #7

Message was sent while issue was closed.

Change committed as 175471

Expand Messages | Collapse Messages