Bug 129655

Summary: Inspector does not restore breakpoints after a page reload
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: Web InspectorAssignee: Blaze Burg <bburg>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, commit-queue, graouts, joepeck, mark.lam, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
reproduction test case
none
the patch
none
the patch (revised) none

Mark Lam
Reported 2014-03-03 20:11:45 PST
Created attachment 225731 [details] reproduction test case Steps to reproduce: 1. Untar the attached test case. You should get a debugger-statement.html and debugger-statement.js. Load debugger-statement.html in the browser. 2. Open the WebInspector. 3. Set breakpoint in debugger-statement.js at lines 2 and 4. 4. Reload the page in the browser. After the reload, the breakpoints are not re-enabled. I put some prints in JSC::Debugger::setBreakpoint() and JSC::Debugger::clearBreakpoint(), and confirmed that the breakpoints were set, and then cleared when I did the reload. However, after the reload, setBreakpoint() was not called to restore the breakpoints. This means the issue happens above JSC.
Attachments
reproduction test case (290 bytes, application/x-gzip)
2014-03-03 20:11 PST, Mark Lam 		no flags
Details
the patch (7.36 KB, patch)
2014-03-04 16:29 PST, Blaze Burg 		no flags
DetailsFormatted DiffDiff
the patch (revised) (7.26 KB, patch)
2014-03-04 19:35 PST, Blaze Burg 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2014-03-03 20:12:16 PST
<rdar://problem/16219327>
Blaze Burg
Comment 2 2014-03-04 16:29:11 PST
Created attachment 225829 [details] the patch
Mark Lam
Comment 3 2014-03-04 16:33:00 PST
Comment on attachment 225829 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=225829&action=review > Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.cpp:723 > + scriptDebugServer().continueProgram(); > + clearDebuggerBreakpointState(); Shouldn't you clear the breakpoints before continuing the program?
Blaze Burg
Comment 4 2014-03-04 16:51:37 PST
(In reply to comment #3) > (From update of attachment 225829 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=225829&action=review > > > Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.cpp:723 > > + scriptDebugServer().continueProgram(); > > + clearDebuggerBreakpointState(); > > Shouldn't you clear the breakpoints before continuing the program? That call is redundant with the clear/continue inside clearDebuggerBreakpointState(), and can be removed.
Joseph Pecoraro
Comment 5 2014-03-04 16:58:16 PST
Comment on attachment 225829 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=225829&action=review > Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.cpp:713 > + ErrorString dummyError; > setOverlayMessage(&dummyError, nullptr); setOverlayMessage currently does nothing. And I wouldn't associate this with "clearDebuggerBreakpointState". Maybe we should just remove it. Otherwise, leave it here and we will clean it up later. >> Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.cpp:723 >> + clearDebuggerBreakpointState(); > > Shouldn't you clear the breakpoints before continuing the program? I agree with Mark's comment. In fact, clearDebuggerBreakpointState already does a continue program, so you can remove the line from here. r=me with that change.
Blaze Burg
Comment 6 2014-03-04 19:35:08 PST
Created attachment 225845 [details] the patch (revised)
WebKit Commit Bot
Comment 7 2014-03-04 20:38:35 PST
Comment on attachment 225845 [details] the patch (revised) Clearing flags on attachment: 225845 Committed r165093: <http://trac.webkit.org/changeset/165093>
WebKit Commit Bot
Comment 8 2014-03-04 20:38:37 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.