[webkit-reviews] review granted: [Bug 28799] REGRESSION(r47686-r47783): Debugger no longer works on reload : [Attachment 49150] [PATCH] Proposed change.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Feb 21 11:24:48 PST 2010


Timothy Hatcher <timothy at hatcher.name> has granted Pavel Feldman
<pfeldman at chromium.org>'s request for review:
Bug 28799: REGRESSION(r47686-r47783): Debugger no longer works on reload
https://bugs.webkit.org/show_bug.cgi?id=28799

Attachment 49150: [PATCH] Proposed change.
https://bugs.webkit.org/attachment.cgi?id=49150&action=review

------- Additional Comments from Timothy Hatcher <timothy at hatcher.name>
Some general comments:

1) It would be nice if common parts of ScriptDebugger.h were shared. This goes
for other bindings/{js, v8} Script* files too. We should use an approach like
the other platform files. Share a common header with common implementation and
have platform specific implementations. That would be more maintainable than
complete file forks.

2) JavaScriptDebugServer should have no dependancy on Inspector. So
InspectorBreakpoint should be ScriptBreakpoint.

3) Like we talked about on IRC, I would like to see the Script* files moved to
inspector/, and adopt the platform approch I mentioned in point 1.

This patch looks fine as long as you rename InspectorBreakpoint before landing.


More information about the webkit-reviews mailing list