[webkit-reviews] review granted: [Bug 52615] Web Inspector: set breakpoints by line:column : [Attachment 79254] Patch.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Jan 24 04:36:51 PST 2011
Yury Semikhatsky <yurys at chromium.org> has granted Pavel Podivilov
<podivilov at chromium.org>'s request for review:
Bug 52615: Web Inspector: set breakpoints by line:column
https://bugs.webkit.org/show_bug.cgi?id=52615
Attachment 79254: Patch.
https://bugs.webkit.org/attachment.cgi?id=79254&action=review
------- Additional Comments from Yury Semikhatsky <yurys at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=79254&action=review
Please address the comments before landing.
> Source/WebCore/bindings/js/ScriptDebugServer.cpp:153
> + *actualColumnNumber = 1;
Please file a bug against JSC and add FIXME comment here.
> Source/WebCore/bindings/js/ScriptDebugServer.h:138
> + typedef HashMap<long, ScriptBreakpoint> ScriptBreakpoints;
ScriptBreakpoints -> LineToScriptBreakpointMap
> Source/WebCore/bindings/js/ScriptDebugServer.h:139
> + typedef HashMap<intptr_t, ScriptBreakpoints> BreakpointsMap;
BreakpointsMap -> SourceIdToBreakpointsMap, I'd also recommend renaming
corresponding fields as m_breakpoints sounds too general.
> Source/WebCore/inspector/InspectorDebuggerAgent.h:100
> + typedef HashMap<Location, ScriptBreakpoint> ScriptBreakpoints;
ScriptBreakpoints -> LocationToBreakpointMap
More information about the webkit-reviews
mailing list