Web Inspector: extract breakpoint management code to a helper class and add tests. Our breakpoint management code is quite complex, why don't we move it to a separate class and add a set of unit tests for it? The next step would be to dispatch breakpoint events on uiSourceCode objects directly and thus completely decouple breakpoint management code from the rest of presentation model's code.
Created attachment 103912 [details] Patch
For actually switching to BreakpointHelper in DebuggerPresentationModel see https://bugs.webkit.org/show_bug.cgi?id=66225.
Created attachment 104028 [details] Patch
(In reply to comment #3) > Created an attachment (id=104028) [details] > Patch Split BreakpointHelper into 3 classes and cleanup the code.
Comment on attachment 104028 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=104028&action=review > Source/WebCore/inspector/front-end/DebuggerPresentationModel.js:705 > +WebInspector.BreakpointHelper = function(debuggerModel, breakpoints, breakpointAddedDelegate, breakpointRemovedDelegate) Class with two inner classes deserves to be defined in a separate file. > Source/WebCore/inspector/front-end/DebuggerPresentationModel.js:720 > + registerUISourceCode: function(uiSourceCodeId, rawSourceCode, persistent) Can we pass real instance here? Also, remove <persist>. This could be called uiSourceCodeAdded > Source/WebCore/inspector/front-end/DebuggerPresentationModel.js:722 > + this._uiSourceCodeInfo[uiSourceCodeId] = { rawSourceCode: rawSourceCode, persistent: persistent }; This mapping should be bi-directional, should be defined in the RawSourceCode <-> UISourceCode terms. > Source/WebCore/inspector/front-end/DebuggerPresentationModel.js:745 > + this._uiBreakpoints.addBreakpoint(breakpoint); Either pass all the parameters into addBreakpoint or define a class for a breakpoint.
Created attachment 104324 [details] Patch
Comment on attachment 104324 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=104324&action=review > Source/WebCore/inspector/front-end/BreakpointManager.js:31 > +WebInspector.BreakpointManager = function(debuggerModel, breakpoints, breakpointAddedDelegate, breakpointRemovedDelegate) Please remove the breakpoints parameter and replace it with the "setBreakpointsForTest" method.
Committed r93415: <http://trac.webkit.org/changeset/93415>
The test added by this patch (inspector/debugger/breakpoint-manager.html) is failing everywhere: http://build.webkit.org/results/Windows%20XP%20Debug%20(Tests)/r93465%20(31415)/inspector/debugger/breakpoint-manager-pretty-diff.html http://build.webkit.org/results/Qt%20Linux%20Release/r93416%20(36643)/inspector/debugger/breakpoint-manager-pretty-diff.html http://build.webkit.org/results/SnowLeopard%20Intel%20Debug%20(Tests)/r93461%20(1777)/inspector/debugger/breakpoint-manager-pretty-diff.html I don't even know on which platform this test passes.
> I don't even know on which platform this test passes. I'm rolling it now, sorry for trouble.