[webkit-reviews] review requested: [Bug 43489] Web Inspector: It would be better to make did* method calls automatically : [Attachment 63591] [patch] second iteration
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Aug 5 07:49:50 PDT 2010
Ilya Tikhonovsky <loislo at chromium.org> has asked for review:
Bug 43489: Web Inspector: It would be better to make did* method calls
automatically
https://bugs.webkit.org/show_bug.cgi?id=43489
Attachment 63591: [patch] second iteration
https://bugs.webkit.org/attachment.cgi?id=63591&action=review
------- Additional Comments from Ilya Tikhonovsky <loislo at chromium.org>
WebCore/ChangeLog:5
+ WebInspector: In the current implementation of inspector agent's
we're calling
typo: agent's -> agents
done.
WebCore/inspector/CodeGeneratorInspector.pm:306
+ push(@function, " protocolError(callId, backendFunctionName,
formatWrongArgumentsCountMessage(args->length() - 1,
$expectedParametersCount));");
I think it should be retportProtocolError according to the style guide
done.
WebCore/inspector/InspectorBackend.cpp:129
+ RefPtr<InspectorArray> result = InspectorArray::create();
this should be *names = InspectorArray::create()
default values are assigning in the generated code.
WebCore/inspector/Inspector.idl:131
+ [handler=DOM, customResponse=didApplyDomChange] void
setTextNodeValue(in long callId, in long nodeId, in String value, out boolean
success);
Would be nice if we could avoid custom responses
will do that in another patch
WebCore/inspector/Inspector.idl:134
+ [handler=DOM] void removeNode(in long callId, in long nodeId, out
long outNodeId);
I think we don't need out outNodeId in this method.
will do that in another patch
WebCore/inspector/InspectorController.cpp:1521
+ void InspectorController::getProfileHeaders(long, RefPtr<InspectorArray>*
headers)
remove first parameter completely here and in other places?
will do that in another patch
WebCore/inspector/InspectorController.cpp:1707
+ *newCallFrames = currentCallFrames();
please add else branch *newCallFrames = InspectorValue::null();
default values are assigning in the generated code.
WebCore/inspector/InspectorDOMAgent.cpp:560
+ {
outNodeId should be removed
will do that in another patch
WebCore/inspector/InspectorDOMAgent.cpp:1140
+ *success = false;
consider generating those initializers in InspectorBackendDispatcher
done.
More information about the webkit-reviews
mailing list