[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