[webkit-reviews] review denied: [Bug 54094] Would like a way to view CoreIPC messages sent between processes : [Attachment 90221] Proposed Core IPC message logging (w/ build flag, corrected changelog style)

[bugzilla-daemon at webkit.org]

Anders Carlsson <andersca at apple.com> has denied Keith Rosenblatt
<keith.rosenblatt at nokia.com>'s request for review:
Bug 54094: Would like a way to view CoreIPC messages sent between processes
https://bugs.webkit.org/show_bug.cgi?id=54094

Attachment 90221: Proposed Core IPC message logging (w/ build flag, corrected
changelog style)
https://bugs.webkit.org/attachment.cgi?id=90221&action=review

------- Additional Comments from Anders Carlsson <andersca at apple.com>
It's great that someone's thinking about CoreIPC message logging. Thanks for
working on this, Keith!

I just talked to Sam Weinig and we'd like to go a somewhat different route:

- As with the rest of the WebKit2 logging, we'd like this to be compiled by
default and controlled by a runtime flag.
- We'd like a solution that's less intrusive and doesn't require adding extra
functions to every object that could receive a message.
- We'd also like to be able to log the message arguments.

A better design would be to just log the messages in the generated message
receiver implementations (For example WebPage::didReceiveWebPageMessage).
Basically, something like:


void WebPage::didReceiveWebPageMessage(CoreIPC::Connection* connection,
CoreIPC::MessageID messageID, CoreIPC::ArgumentDecoder* arguments)
{
    MessageLogger logger(connection->isMessageLoggingEnabled());

    switch (messageID.get<Messages::WebPage::Kind>()) {
    case Messages::WebPage::SetActiveID:
	logger.setMessage(messageID, "WebPage", "SetActive");
	CoreIPC::handleMessage<Messages::WebPage::SetActive>(arguments, this,
&WebPage::setActive, logger);
	return;
    ...
}

The MessageLogger destructor would then output the message if logging was
enabled.

We could add a log member function to our various Arguments classes that would
log the argument values; this would be called from within handleMessage. (I
don't think the argument logging is necessary)

Again, thanks for working on this! I hope you'll like the design as much as we
do :)