[webkit-reviews] review canceled: [Bug 44338] Web Inspector: inspector protocol should be switched from array to object based representation. : [Attachment 64973] [patch] initial version.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Aug 23 03:18:15 PDT 2010


Ilya Tikhonovsky <loislo at chromium.org> has canceled Ilya Tikhonovsky
<loislo at chromium.org>'s request for review:
Bug 44338: Web Inspector: inspector protocol should be switched from array to
object based representation.
https://bugs.webkit.org/show_bug.cgi?id=44338

Attachment 64973: [patch] initial version.
https://bugs.webkit.org/attachment.cgi?id=64973&action=review

------- Additional Comments from Ilya Tikhonovsky <loislo at chromium.org>

> WebCore/inspector/CodeGeneratorInspector.pm:527
> > +	       this[command] = this.sendMessageToBackend.bind(this,
commandInfo);
> I think _registerDelegate should just take a single argument, commandInfo,
and "command" is just "commandInfo.command".

done.

> WebCore/inspector/CodeGeneratorInspector.pm:540
> > +	       if (args.length == 1 && typeof args[0] === "function")
> Nit: ===

done.

> WebCore/inspector/CodeGeneratorInspector.pm:295
>  +	  push(@function, "    RefPtr<InspectorObject>
requestMessageObject(requestMessageObjectRef);");
> This is not needed.

done

> WebCore/inspector/CodeGeneratorInspector.pm:511
> +		 "\\\"arguments\\\": {$argumentNames}" .
> Unrelated to this issue, but it would be nice if we didn't escape quotes in
the JSON strings in InspectorBackendStub.js but use single quotes instead(
"\"property\"" -> '"property"')

done

> WebCore/inspector/front-end/inspector.js:637
> +	 WebInspector.log("Error: InspectorBackend." + message.command + "
failed.");
> These messages should go into console.error not the Inspector's console.

done

> WebCore/inspector/CodeGeneratorInspector.pm:354
> +	 push(@function, "	  responseMessage->setString(\"type\",
\"response\");");
> It might make sense to extract these string constants and reuse them.

done


InspectorFrontendClientLocal::setAttachedWindow was fixed.
WebInspector_syncDispatch was fixed.


More information about the webkit-reviews mailing list