[webkit-reviews] review denied: [Bug 40134] Web Inspector: sendMessageToFrontend implementation required. : [Attachment 57805] [patch] second iteration. With fixed gtk build and other minor changes.

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Ilya Tikhonovsky
<loislo at chromium.org>'s request for review:
Bug 40134: Web Inspector: sendMessageToFrontend implementation required.
https://bugs.webkit.org/show_bug.cgi?id=40134

Attachment 57805: [patch] second iteration. With fixed gtk build and other
minor changes.
https://bugs.webkit.org/attachment.cgi?id=57805&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
WebCore/ChangeLog:5
 +	    WebInspector: It should be possible to transfer timeline/dom/etc
data from inspected page side to the front-end side in serialized format.
Please wrap it at 80 chars or so.

WebCore/inspector/InspectorController.cpp:429
 +  void InspectorController::connectFrontend(const ScriptObject& webInspector)

I think "attach" would be a better name. Also I would leave a comment saying
that webInspector parameter is going to go away once we migrate to the new
schema.

WebKit/mac/WebCoreSupport/WebInspectorClient.mm:107
 +	return
m_webInspectorFrontendClient->dispatchMessageToFrontend(message);
This one is I think wrong. You are telling client of the inspected page
(InspectorClient) to dispatch message on the frontend via the
InspectorFrontend's client. But methods on client are to be called by WebCore,
not WebKit. You should simply get a pointer to the Page and dispatch the JS
call there.

WebCore/WebCore.Inspector.exp:24
 + 
__ZN7WebCore28InspectorFrontendClientLocal25dispatchMessageToFrontendERKNS_6Str
ingE
This one should not be visible to WebKit. In fact, this entire
InspectorFrontendClientLocal class should go away since it exposes a way too
rich WebCore API to the WebKit.

WebKit/qt/WebCoreSupport/InspectorClientQt.cpp:165
 +	return false;
You plan on implementing these prior to landing, right?