Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(13)

Issue 1163923005: Fix crash in inspector-protocol/debugger/debugger-pause-dedicated-worker.html (Closed)

Created:
4 years, 11 months ago by yurys
Modified:
4 years, 11 months ago
Reviewers:
sergeyv, pfeldman
CC:
blink-reviews, caseq+blink_chromium.org, arv+blink, vivekg_samsung, vivekg, yurys+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, blink-reviews-bindings_chromium.org, devtools-reviews_chromium.org, apavlov+blink_chromium.org, sergeyv+blink_chromium.org, kozyatinskiy+blink_chromium.org
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Fix crash in inspector-protocol/debugger/debugger-pause-dedicated-worker.html Inspector uses v8 interrupts to execute devtools commands when inspected script is running (potentially for a long time). This is done by first posting command into the debug command queue and provisionally triggering the interrupt. However, inspector itself may be executing JS and in this case we will be running commands in a nested loop. This CL makes sure that no nested command dispatching is allowed on worker thread (but the case when the script is paused). The downside of this change is that if a script evaluated in the console gets into tight loop we won't be able to pause in it. BUG=495719 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=196412

Patch Set 1 #

Total comments: 2

Patch Set 2 : Removed printf #

Unified diffs Side-by-side diffs Delta from patch set Stats (+158 lines, -101 lines) Patch
M Source/bindings/core/v8/MainThreadDebugger.h View 4 chunks +4 lines, -3 lines 0 comments Download
M Source/bindings/core/v8/MainThreadDebugger.cpp View 3 chunks +4 lines, -2 lines 0 comments Download
M Source/bindings/core/v8/V8Debugger.h View 3 chunks +0 lines, -13 lines 0 comments Download
M Source/bindings/core/v8/V8Debugger.cpp View 5 chunks +0 lines, -47 lines 0 comments Download
M Source/core/core.gypi View 1 chunk +2 lines, -0 lines 0 comments Download
A Source/core/inspector/InspectorTaskRunner.h View 1 chunk +46 lines, -0 lines 0 comments Download
A Source/core/inspector/InspectorTaskRunner.cpp View 1 chunk +71 lines, -0 lines 0 comments Download
M Source/core/inspector/WorkerDebuggerAgent.h View 2 chunks +1 line, -4 lines 0 comments Download
M Source/core/inspector/WorkerDebuggerAgent.cpp View 2 chunks +1 line, -29 lines 0 comments Download
M Source/core/inspector/WorkerInspectorController.h View 2 chunks +2 lines, -0 lines 0 comments Download
M Source/core/inspector/WorkerInspectorController.cpp View 6 chunks +24 lines, -1 line 0 comments Download
M Source/web/WebDevToolsAgentImpl.cpp View 3 chunks +3 lines, -2 lines 0 comments Download

Messages

Total messages: 9 (3 generated)
yurys
4 years, 11 months ago (2015-06-03 13:27:03 UTC) #2
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1163923005/1
4 years, 11 months ago (2015-06-03 13:27:29 UTC) #4
sergeyv
lgtm https://codereview.chromium.org/1163923005/diff/1/Source/core/inspector/InspectorDebuggerAgent.cpp File Source/core/inspector/InspectorDebuggerAgent.cpp (right): https://codereview.chromium.org/1163923005/diff/1/Source/core/inspector/InspectorDebuggerAgent.cpp#newcode243 Source/core/inspector/InspectorDebuggerAgent.cpp:243: fprintf(stderr, "InspectorDebuggerAgent::disable %p>>\n", this); lets be silent
4 years, 11 months ago (2015-06-03 14:07:50 UTC) #5
yurys
https://codereview.chromium.org/1163923005/diff/1/Source/core/inspector/InspectorDebuggerAgent.cpp File Source/core/inspector/InspectorDebuggerAgent.cpp (right): https://codereview.chromium.org/1163923005/diff/1/Source/core/inspector/InspectorDebuggerAgent.cpp#newcode243 Source/core/inspector/InspectorDebuggerAgent.cpp:243: fprintf(stderr, "InspectorDebuggerAgent::disable %p>>\n", this); On 2015/06/03 14:07:49, sergeyv wrote: ...
4 years, 11 months ago (2015-06-03 14:29:12 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1163923005/20001
4 years, 11 months ago (2015-06-03 14:29:32 UTC) #8
commit-bot: I haz the power
4 years, 11 months ago (2015-06-03 15:56:01 UTC) #9
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=196412

Powered by Google App Engine
This is Rietveld 408576698