[webkit-reviews] review denied: [Bug 14190] Breakpoints should persist across launches : [Attachment 58250] Proposed patch.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Jun 10 09:32:40 PDT 2010
Pavel Feldman <pfeldman at chromium.org> has denied Pavel Podivilov
<podivilov at chromium.org>'s request for review:
Bug 14190: Breakpoints should persist across launches
https://bugs.webkit.org/show_bug.cgi?id=14190
Attachment 58250: Proposed patch.
https://bugs.webkit.org/attachment.cgi?id=58250&action=review
------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
Almost ready to land. Couple of nits below.
WebCore/inspector/InspectorController.cpp:1753
+ String key = String(breakpointsInspectorSettingPrefix) +
m_mainResource->requestURL();
Please use String::format
WebCore/inspector/InspectorController.cpp:1765
+ sourceBreakpointsFromInspectorObject(breakpointsForURL,
&sourceBreakpoints);
I would declare this as a static method on SourceBreakpoint. No need to add
stuff to WebCore namespace.
WebCore/inspector/ScriptBreakpoint.cpp:2
+ * Copyright (C) 2009 Apple Inc. All rights reserved.
Please use 2010 Google only, Apple has not yet contributed to this file.
WebCore/inspector/front-end/BreakpointManager.js:55
+ this._saveBreakpointOnBackend(breakpoint);
Please make sure one time breakpoints work after your change and are not
persisted.
More information about the webkit-reviews
mailing list