[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