[webkit-reviews] review denied: [Bug 40134] Web Inspector: sendMessageToFrontend implementation required. : [Attachment 58040] [patch] fourth iteration.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jun 7 10:20:06 PDT 2010


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 58040: [patch] fourth iteration.
https://bugs.webkit.org/attachment.cgi?id=58040&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
WebCore/inspector/InspectorClient.h:53
 +	virtual bool sendMessageToFrontend(const String& message) = 0;
const String& (no 'message')



WebCore/inspector/InspectorController.h:139
 +	// transport via InspectorClient. After finish webInspector parameter
should
... After migration, WebInspector parameter should ...


WebKit/cf/WebCoreSupport/WebInspectorClientCF.cpp:116
 +	String dispatchToFrontend("WebInspector.dispatchMessageToFrontend(");
String::format


WebKit/gtk/WebCoreSupport/InspectorClientGtk.cpp:91
 +	m_frontendPage = 0;
What happens when page->inspectorController()->close() is called?


WebKit/gtk/WebCoreSupport/InspectorClientGtk.cpp:127
 +	String dispatchToFrontend("WebInspector.dispatchMessageToFrontend(");
Should we format this message using utility method in WebCore?

WebKit/mac/WebCoreSupport/WebInspectorClient.h:46
 +  
no need in blank lines here and below.

WebKit/mac/WebCoreSupport/WebInspectorClient.h:66
 +	virtual bool sendMessageToFrontend(const WebCore::String& key);
key -> message (or even better, remove it!)

WebCore/inspector/InspectorController.cpp:535
 +	m_client->closeInspectorFrontend();
disconnectFrontend is being called by the WebKit layer. After this call,
InspectorController should not do any attempts to talk to its frontend. You
don't need to call WebCore's disconnectFrontend from WebKit in order to get
immediately back to WebKit and execute closeInspectorFrontend...


More information about the webkit-reviews mailing list