See Xcode. Breakpoints should be able to automatically continue after they are hit. This mostly only makes sense after we get Breakpoint Actions, which I will work on next.
<rdar://problem/14815088>
Created attachment 209415 [details] [PATCH] In Progress - Needs to Update LayoutTests This is a first draft. Seems to work pretty well, and the next step is adding BreakpointActions, which I'll do in a follow-up patch. Not marking this for review yet, because I didn't update and write new tests yet.
Created attachment 209417 [details] [IMAGE] Edit Popover with Option to Continue This is what the in progress patch looks like.
Comment on attachment 209415 [details] [PATCH] In Progress - Needs to Update LayoutTests Looking good!
Created attachment 210048 [details] [PATCH] Proposed Fix
Comment on attachment 210048 [details] [PATCH] Proposed Fix Attachment 210048 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/1638369 New failing tests: inspector-protocol/debugger/setBreakpoint-column.html
Created attachment 210061 [details] Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-13 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.4
Comment on attachment 210048 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=210048&action=review > LayoutTests/inspector-protocol/debugger/removeBreakpoint.html:22 > + if (InspectorTest.checkForError(responseObject)) return; I know it is just a test, but this doesn't match WebKit style and is weird for me to read now! There are a few places you do this. (Even though it was my former style 8 years ago…) > Source/WebCore/bindings/js/ScriptDebugServer.cpp:171 > // An empty condition counts as no condition which is equivalent to "true". > if (breaksVector.at(i).condition.isEmpty()) > return true; Maybe this should move to the caller now that you pass out the breakpoint? Or maybe it should move above the new code and not be considered as hitting the breakpoint if the condition is false. > Source/WebInspectorUI/UserInterface/Breakpoint.js:287 > + var optionsRow = table.appendChild(document.createElement("tr")); > + var optionsHeader = optionsRow.appendChild(document.createElement("th")); > + var optionsData = optionsRow.appendChild(document.createElement("td")); It is 2013! <td>! <th>! <tr>! Heh. No worries. > Source/WebInspectorUI/UserInterface/InspectorBackend.js:79 > + agent[domainAndMethod[1]]["supports"] = this._supports.bind(this, method, signature); Sweet!
Comment on attachment 210048 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=210048&action=review >> LayoutTests/inspector-protocol/debugger/removeBreakpoint.html:22 >> + if (InspectorTest.checkForError(responseObject)) return; > > I know it is just a test, but this doesn't match WebKit style and is weird for me to read now! There are a few places you do this. (Even though it was my former style 8 years ago…) Yeah, a newline/indent really breaks up the readability of the test. After thinking about this, we can make InspectorTest.checkForError throw an exception, which would stop further execution. I'll give that a shot! >> Source/WebCore/bindings/js/ScriptDebugServer.cpp:171 >> return true; > > Maybe this should move to the caller now that you pass out the breakpoint? Or maybe it should move above the new code and not be considered as hitting the breakpoint if the condition is false. I think this is fine for now because I suspect all of this code to start going through some changes soon. For now hasBreakpoint returns true/false if the breakpoint was actually triggered or not (including checking the condition), but will always send out the breakpoint that was considered, success or not.
Committed <http://trac.webkit.org/changeset/154910>.