[webkit-reviews] review denied: [Bug 47173] Web Inspector: extract Inspector Instrumentation API as a class : [Attachment 69953] Comments addressed.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Oct 6 14:02:11 PDT 2010


Pavel Feldman <pfeldman at chromium.org> has denied Pavel Podivilov
<podivilov at chromium.org>'s request for review:
Bug 47173: Web Inspector: extract Inspector Instrumentation API as a class
https://bugs.webkit.org/show_bug.cgi?id=47173

Attachment 69953: Comments addressed.
https://bugs.webkit.org/attachment.cgi?id=69953&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=69953&action=review

> WebCore/ChangeLog:27
> +	   (WebCore::InspectorInstrumentation::willInsertDOMNodeImpl):

You should nuke these scary verbose lines.

> WebCore/bindings/v8/V8Proxy.cpp:414
> +    InspectorInstrumentation::didEvaluateScript(m_frame, cookie);

I think cookie should be the first parameter in all the calls.

> WebCore/inspector/InspectorInstrumentation.cpp:-67
> -    if (!inspectorController->hasFrontend())

Why did these go? Please explain in order to get r+.

> WebCore/inspector/InspectorInstrumentation.h:116
> +    static InspectorInstrumentationCookie
willCallFunctionImpl(InspectorController*, const String& scriptName, int
scriptLine);

This is crazy amount of methods, really easy to make a mistake here. Can we cut
the number?


More information about the webkit-reviews mailing list