[webkit-reviews] review denied: [Bug 34326] [Chromium] Upstream the rest of DevTools WebKit API implementation : [Attachment 47842] patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Feb 1 07:58:47 PST 2010
Pavel Feldman <pfeldman at chromium.org> has denied Yury Semikhatsky
<yurys at chromium.org>'s request for review:
Bug 34326: [Chromium] Upstream the rest of DevTools WebKit API implementation
https://bugs.webkit.org/show_bug.cgi?id=34326
Attachment 47842: patch
https://bugs.webkit.org/attachment.cgi?id=47842&action=review
------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
Bunch of style nits, other than that r+.
> + 'src/DebuggerAgent.h',
> + 'src/DebuggerAgentImpl.cpp',
> + 'src/DebuggerAgentImpl.h',
> + 'src/DebuggerAgentManager.cpp',
> + 'src/DebuggerAgentManager.h',
> + 'src/DevToolsRPC.h',
> + 'src/DevToolsRPCJS.h',
Indent.
> +
> +#define APU_AGENT_DELEGATE_STRUCT(METHOD0, METHOD1, METHOD2, METHOD3,
MEHTOD4, METHOD5) \
> + /* Sends a json object to apu. */ \
> + METHOD1(dispatchToApu, String /* data */)
> +
4 space indent?
> +
> + v8::Handle<v8::Value> args[] = {
> + functionNameWrapper,
> + jsonArgsWrapper,
> + callIdWrapper
> + };
4 space indent?
> + v8::Handle<v8::Value> args[] = {
> + v8::Local<v8::Value>()
Indent.
> +WebCore::Page* DebuggerAgentImpl::page()
> +{
> + return m_webViewImpl->page();
> +}
Indent.
> +
> +#include "webkit/glue/glue_util.h"
> +#include "webkit/glue/webkit_glue.h"
> +
Does this compile?
> + virtual void dispatchOnInjectedScript(
> + int callId,
> + int injectedScriptId,
> + const WebCore::String& functionName,
> + const WebCore::String& jsonArgs,
> + bool async);
Use single line here in and cpp.
> +WebDevToolsFrontend* WebDevToolsFrontend::create(
> + WebView* view,
> + WebDevToolsFrontendClient* client,
> + const WebString& applicationLocale)
> +{
> + return new WebDevToolsFrontendImpl(
> + static_cast<WebViewImpl*>(view),
> + client,
> + applicationLocale);
> +}
Why wrapping?
More information about the webkit-reviews
mailing list