[webkit-reviews] review denied: [Bug 42983] Web Inspector: migrate WebCore from InspectorBackend binding to InspectorBackendDispatcher : [Attachment 62595] [patch] second iteration. fixed windows build.

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Ilya Tikhonovsky
<loislo at chromium.org>'s request for review:
Bug 42983: Web Inspector: migrate WebCore from InspectorBackend binding to
InspectorBackendDispatcher
https://bugs.webkit.org/show_bug.cgi?id=42983

Attachment 62595: [patch] second iteration. fixed windows build.
https://bugs.webkit.org/attachment.cgi?id=62595&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
WebCore/inspector/Inspector.idl:42
 +	    [notify] void didDispatch(out long callId, out String e);
Also, we should not have a generic ack / failure reporting. Each of the backend
methods should ack (either with the results or with failure reporting). The
protocol should say that request with type 'foo' and sequence number 'bar' has
failed.

WebCore/inspector/InspectorController.cpp:2210
 +	m_inspectorBackendDispatcher->dispatch(message, &exception);
Nit: should this guy be static and receive backend instance as an argument to
emphasize its stateless nature?

WebCore/inspector/InspectorController.h:125
 +	void dispatchOnBackend(long callId, const String& message);
This should be dispatchOnBackend(const String& message). We should serialize
all the call data into a single message.

WebCore/inspector/front-end/InspectorBackendStub.js:35
 +	this._registerDelegate("addInspectedNode");
Can you generate these?

WebCore/inspector/front-end/InspectorBackendStub.js:124
 +	    var callbackId = WebInspector.Callback.wrap(function(){});
You don't need this artificial call id / empty response callback anymore. Just
send the message.

WebKit/chromium/src/InspectorFrontendClientImpl.cpp:132
 +	WebVector<WebString> args((size_t)2);
We only allow static_cast<> in such places.

WebKit/chromium/src/InspectorFrontendClientImpl.cpp:136
 +	m_client->sendMessageToAgent(messageData);
I'd introduce new sendWebKitMessageToBackend(string) and use it here. It will
eventually become the main transport for the interaction.