Add IPC::ForwardingMessageFilter.

Add IPC::ForwardingMessageFilter.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=147708

[nduca, 2012-07-20 06:27:58]

Here we go. I gussed at a location and style, hope I'm not too far off base.

[darin (slow to review), 2012-07-20 06:57:30]

Looks good...

http://codereview.chromium.org/10796057/diff/1/ipc/ipc_forwarding_message_fil...
File ipc/ipc_forwarding_message_filter.cc (right):

http://codereview.chromium.org/10796057/diff/1/ipc/ipc_forwarding_message_fil...
ipc/ipc_forwarding_message_filter.cc:8: #include
"ipc/ipc_forwarding_message_filter.h"
nit: this include should be listed first.

http://codereview.chromium.org/10796057/diff/1/ipc/ipc_forwarding_message_fil...
ipc/ipc_forwarding_message_filter.cc:58: }
nit: "}  // namespace IPC"

http://codereview.chromium.org/10796057/diff/1/ipc/ipc_forwarding_message_fil...
File ipc/ipc_forwarding_message_filter.h (right):

http://codereview.chromium.org/10796057/diff/1/ipc/ipc_forwarding_message_fil...
ipc/ipc_forwarding_message_filter.h:30: public:
nit: indent "public" and "protected" by one space

http://codereview.chromium.org/10796057/diff/1/ipc/ipc_forwarding_message_fil...
ipc/ipc_forwarding_message_filter.h:31: // Called when
ViewMsg_UpdateVSyncParameters message is intercepted.
nit: update/delete comment

http://codereview.chromium.org/10796057/diff/1/ipc/ipc_forwarding_message_fil...
ipc/ipc_forwarding_message_filter.h:42: ForwardingMessageFilter(const
std::set<int>& message_ids_to_filter,
nit: an int array might be a bit friendlier to consumers.  most consumers will
just have a static list of message IDs they want filtered, so passing an array
is a nice way to express that:

  const uint32 kMessagesToFilter[] = {
    ViewMsg_UpdateVSyncParameters::ID
  };
  ForwardingMessageFilter(kMessagesToFilter, arraysize(kMessagesToFilter),
                          ...);

http://codereview.chromium.org/10796057/diff/1/ipc/ipc_forwarding_message_fil...
ipc/ipc_forwarding_message_filter.h:43: base::MessageLoopProxy* target_loop,
nit: consider using base::TaskRunner now instead to make things even more
generic.

[nduca, 2012-07-20 07:24:30]

* done. Hopefully I caught everything, I have to admit, I'm getting sleepy :)

[darin (slow to review), 2012-07-20 16:53:53]

LGTM

http://codereview.chromium.org/10796057/diff/5003/ipc/ipc_forwarding_message_...
File ipc/ipc_forwarding_message_filter.h (right):

http://codereview.chromium.org/10796057/diff/5003/ipc/ipc_forwarding_message_...
ipc/ipc_forwarding_message_filter.h:29: class Client {
Just one more thing:  Thoughts on using base::Callback<void(const
IPC::Message&)> here instead?  That just gives the consumer a bit more
flexibility.

[nduca, 2012-07-20 17:13:58]

Happy to do that as well if thats your preference. I'll give it a shot.

[jam, 2012-07-20 18:09:44]

just nits (don't need me to review since darin already did)

http://codereview.chromium.org/10796057/diff/5003/ipc/ipc_forwarding_message_...
File ipc/ipc_forwarding_message_filter.h (right):

http://codereview.chromium.org/10796057/diff/5003/ipc/ipc_forwarding_message_...
ipc/ipc_forwarding_message_filter.h:32: virtual void OnMessageReceived(const
IPC::Message& message) = 0;
nit: get rid of "IPC::" in this file and the cc

http://codereview.chromium.org/10796057/diff/5003/ipc/ipc_forwarding_message_...
ipc/ipc_forwarding_message_filter.h:38: // This fitler will intercept
|message_ids_to_filter| and post
nit: filter

http://codereview.chromium.org/10796057/diff/5003/ipc/ipc_forwarding_message_...
ipc/ipc_forwarding_message_filter.h:57:
DISALLOW_COPY_AND_ASSIGN(ForwardingMessageFilter);
nit: this usually goes at the end of the class

[commit-bot: I haz the power, 2012-07-20 18:52:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nduca@chromium.org/10796057/3002

[commit-bot: I haz the power, 2012-07-20 20:14:29]

Change committed as 147708