Add regression tests to ensure that IPC::Listener::OnBadMessageReceived is...

Add regression tests to ensure that IPC::Listener::OnBadMessageReceived is always called for all cases, i.e.
1) using IPC::Channel directly
2) using IPC::ChannelProxy and the IPC is dispatched on the IO thread
2) using IPC::ChannelProxy and the IPC is dispatched on the listener thread

R=tsepez@chromium.org

Review URL: https://codereview.chromium.org/282303014

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@271915 0039d316-1c4b-4281-b951-d872f2087c98

content/browser/renderer_host/render_view_host_unittest.cc

@@ -207,48 +207,6 @@ TEST_F(RenderViewHostTest, DragEnteredFileURLsStillBlocked) {
   EXPECT_FALSE(policy->CanRequestURL(id, sensitive_file_url));
   EXPECT_FALSE(policy->CanReadFile(id, sensitive_file_path));
 }
-/* TODO(jam)
-// The test that follow trigger DCHECKS in debug build.
-#if defined(NDEBUG) && !defined(DCHECK_ALWAYS_ON)
-
-// Test that when we fail to de-serialize a message, RenderViewHost calls the
-// ReceivedBadMessage() handler.
-TEST_F(RenderViewHostTest, BadMessageHandlerRenderViewHost) {
-  EXPECT_EQ(0, process()->bad_msg_count());
-  // craft an incorrect ViewHostMsg_UpdateTargetURL message. The real one has
-  // two payload items but the one we construct has none.
-  IPC::Message message(0, ViewHostMsg_UpdateTargetURL::ID,
-                       IPC::Message::PRIORITY_NORMAL);
-  test_rvh()->OnMessageReceived(message);
-  EXPECT_EQ(1, process()->bad_msg_count());
-}
-
-// Test that when we fail to de-serialize a message, RenderWidgetHost calls the
-// ReceivedBadMessage() handler.
-TEST_F(RenderViewHostTest, BadMessageHandlerRenderWidgetHost) {
-  EXPECT_EQ(0, process()->bad_msg_count());
-  // craft an incorrect ViewHostMsg_UpdateRect message. The real one has
-  // one payload item but the one we construct has none.
-  IPC::Message message(0, ViewHostMsg_UpdateRect::ID,
-                       IPC::Message::PRIORITY_NORMAL);
-  test_rvh()->OnMessageReceived(message);
-  EXPECT_EQ(1, process()->bad_msg_count());
-}
-
-// Test that OnInputEventAck() detects bad messages.
-TEST_F(RenderViewHostTest, BadMessageHandlerInputEventAck) {
-  EXPECT_EQ(0, process()->bad_msg_count());
-  // InputHostMsg_HandleInputEvent_ACK is defined taking 0 params but
-  // the code actually expects it to have at least one int para, this this
-  // bogus message will not fail at de-serialization but should fail in
-  // OnInputEventAck() processing.
-  IPC::Message message(0, InputHostMsg_HandleInputEvent_ACK::ID,
-                       IPC::Message::PRIORITY_NORMAL);
-  test_rvh()->OnMessageReceived(message);
-  EXPECT_EQ(1, process()->bad_msg_count());
-}
-#endif
-*/
 
 TEST_F(RenderViewHostTest, MessageWithBadHistoryItemFiles) {
   base::FilePath file_path;

ipc/ipc_channel_proxy_unittest.cc

@@ -8,47 +8,69 @@
 #include "base/pickle.h"
 #include "base/threading/thread.h"
 #include "ipc/ipc_message.h"
-#include "ipc/ipc_message_macros.h"
 #include "ipc/ipc_test_base.h"
 #include "ipc/message_filter.h"
 
-namespace {
+// Get basic type definitions.
+#define IPC_MESSAGE_IMPL
+#include "ipc/ipc_channel_proxy_unittest_messages.h"
 
-#if defined(IPC_MESSAGE_START)
-#undef IPC_MESSAGE_START
-#endif
+// Generate constructors.
+#include "ipc/struct_constructor_macros.h"
+#include "ipc/ipc_channel_proxy_unittest_messages.h"
 
-enum Command {
-  SEND,
-  QUIT
-};
+// Generate destructors.
+#include "ipc/struct_destructor_macros.h"
+#include "ipc/ipc_channel_proxy_unittest_messages.h"
 
-static void Send(IPC::Sender* sender,
-                 int message_class,
-                 Command command) {
-  const int IPC_MESSAGE_START = message_class;
-  IPC::Message* message = new IPC::Message(0,
-                                           IPC_MESSAGE_ID(),
-                                           IPC::Message::PRIORITY_NORMAL);
-  message->WriteInt(command);
-  sender->Send(message);
-}
+// Generate param traits write methods.
+#include "ipc/param_traits_write_macros.h"
+namespace IPC {
+#include "ipc/ipc_channel_proxy_unittest_messages.h"
+}  // namespace IPC
+
+// Generate param traits read methods.
+#include "ipc/param_traits_read_macros.h"
+namespace IPC {
+#include "ipc/ipc_channel_proxy_unittest_messages.h"
+}  // namespace IPC
+
+// Generate param traits log methods.
+#include "ipc/param_traits_log_macros.h"
+namespace IPC {
+#include "ipc/ipc_channel_proxy_unittest_messages.h"
+}  // namespace IPC
+
+
+namespace {
 
 class QuitListener : public IPC::Listener {
  public:
-  QuitListener() {}
+  QuitListener() : bad_message_received_(false) {}
   virtual ~QuitListener() {}
 
   virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE {
-    PickleIterator iter(message);
+    IPC_BEGIN_MESSAGE_MAP(QuitListener, message)
+      IPC_MESSAGE_HANDLER(WorkerMsg_Quit, OnQuit)
+      IPC_MESSAGE_HANDLER(TestMsg_BadMessage, OnBadMessage)
+    IPC_END_MESSAGE_MAP()
+    return true;
+  }
 
-    int command = SEND;
-    EXPECT_TRUE(iter.ReadInt(&command));
-    if (command == QUIT)
-      base::MessageLoop::current()->QuitWhenIdle();
+  virtual void OnBadMessageReceived(const IPC::Message& message) OVERRIDE {
+    bad_message_received_ = true;
+  }
 
-    return true;
+  void OnQuit() {
+    base::MessageLoop::current()->QuitWhenIdle();
   }
+
+  void OnBadMessage(const BadType& bad_type) {
+    // Should never be called since IPC wouldn't be deserialized correctly.
+    CHECK(false);
+  }
+
+  bool bad_message_received_;
 };
 
 class ChannelReflectorListener : public IPC::Listener {
@@ -62,20 +84,35 @@ class ChannelReflectorListener : public IPC::Listener {
   }
 
   virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE {
-    CHECK(channel_);
+    IPC_BEGIN_MESSAGE_MAP(ChannelReflectorListener, message)
+      IPC_MESSAGE_HANDLER(TestMsg_Bounce, OnTestBounce)
+      IPC_MESSAGE_HANDLER(TestMsg_SendBadMessage, OnSendBadMessage)
+      IPC_MESSAGE_HANDLER(UtilityMsg_Bounce, OnUtilityBounce)
+      IPC_MESSAGE_HANDLER(WorkerMsg_Bounce, OnBounce)
+      IPC_MESSAGE_HANDLER(WorkerMsg_Quit, OnQuit)
+    IPC_END_MESSAGE_MAP()
+    return true;
+  }
 
-    PickleIterator iter(message);
+  void OnTestBounce() {
+    channel_->Send(new TestMsg_Bounce());
+  }
 
-    int command = SEND;
-    EXPECT_TRUE(iter.ReadInt(&command));
-    if (command == QUIT) {
-      channel_->Send(new IPC::Message(message));
-      base::MessageLoop::current()->QuitWhenIdle();
-      return true;
-    }
+  void OnSendBadMessage() {
+    channel_->Send(new TestMsg_BadMessage(BadType()));
+  }
 
-    channel_->Send(new IPC::Message(message));
-    return true;
+  void OnUtilityBounce() {
+    channel_->Send(new UtilityMsg_Bounce());
+  }
+
+  void OnBounce() {
+    channel_->Send(new WorkerMsg_Bounce());
+  }
+
+  void OnQuit() {
+    channel_->Send(new WorkerMsg_Quit());
+    base::MessageLoop::current()->QuitWhenIdle();
   }
 
  private:
@@ -149,7 +186,21 @@ class MessageCountFilter : public IPC::MessageFilter {
       EXPECT_EQ(supported_message_class_, IPC_MESSAGE_CLASS(message));
     }
     ++messages_received_;
-    return message_filtering_enabled_;
+
+    if (!message_filtering_enabled_)
+      return false;
+
+    bool handled = true;
+    IPC_BEGIN_MESSAGE_MAP(MessageCountFilter, message)
+      IPC_MESSAGE_HANDLER(TestMsg_BadMessage, OnBadMessage)
+      IPC_MESSAGE_UNHANDLED(handled = false)
+    IPC_END_MESSAGE_MAP()
+    return handled;
+  }
+
+  void OnBadMessage(const BadType& bad_type) {
+    // Should never be called since IPC wouldn't be deserialized correctly.
+    CHECK(false);
   }
 
   virtual bool GetSupportedMessageClasses(
@@ -207,11 +258,15 @@ class IPCChannelProxyTest : public IPCTestBase {
   }
 
   void SendQuitMessageAndWaitForIdle() {
-    Send(sender(), -1, QUIT);
+    sender()->Send(new WorkerMsg_Quit);
     base::MessageLoop::current()->Run();
     EXPECT_TRUE(WaitForClientShutdown());
   }
 
+  bool DidListenerGetBadMessage() {
+    return listener_->bad_message_received_;
+  }
+
  private:
   scoped_ptr<base::Thread> thread_;
   scoped_ptr<QuitListener> listener_;
@@ -220,20 +275,19 @@ class IPCChannelProxyTest : public IPCTestBase {
 TEST_F(IPCChannelProxyTest, MessageClassFilters) {
   // Construct a filter per message class.
   std::vector<scoped_refptr<MessageCountFilter> > class_filters;
-  for (uint32 i = 0; i < LastIPCMsgStart; ++i) {
-    class_filters.push_back(make_scoped_refptr(
-        new MessageCountFilter(i)));
-    channel_proxy()->AddFilter(class_filters.back().get());
-  }
+  class_filters.push_back(make_scoped_refptr(
+      new MessageCountFilter(TestMsgStart)));
+  class_filters.push_back(make_scoped_refptr(
+      new MessageCountFilter(UtilityMsgStart)));
+  for (size_t i = 0; i < class_filters.size(); ++i)
+    channel_proxy()->AddFilter(class_filters[i].get());
 
   // Send a message for each class; each filter should receive just one message.
-  for (uint32 i = 0; i < LastIPCMsgStart; ++i)
-    Send(sender(), i, SEND);
+  sender()->Send(new TestMsg_Bounce());
+  sender()->Send(new UtilityMsg_Bounce());
 
   // Send some messages not assigned to a specific or valid message class.
-  Send(sender(), -1, SEND);
-  Send(sender(), LastIPCMsgStart, SEND);
-  Send(sender(), LastIPCMsgStart + 1, SEND);
+  sender()->Send(new WorkerMsg_Bounce);
 
   // Each filter should have received just the one sent message of the
   // corresponding class.
@@ -244,9 +298,8 @@ TEST_F(IPCChannelProxyTest, MessageClassFilters) {
 
 TEST_F(IPCChannelProxyTest, GlobalAndMessageClassFilters) {
   // Add a class and global filter.
-  const int kMessageClass = 7;
   scoped_refptr<MessageCountFilter> class_filter(
-      new MessageCountFilter(kMessageClass));
+      new MessageCountFilter(TestMsgStart));
   class_filter->set_message_filtering_enabled(false);
   channel_proxy()->AddFilter(class_filter.get());
 
@@ -254,12 +307,12 @@ TEST_F(IPCChannelProxyTest, GlobalAndMessageClassFilters) {
   global_filter->set_message_filtering_enabled(false);
   channel_proxy()->AddFilter(global_filter.get());
 
-  // A message  of class |kMessageClass| should be seen by both the global
-  // filter and |kMessageClass|-specific filter.
-  Send(sender(), kMessageClass, SEND);
+  // A message  of class Test should be seen by both the global filter and
+  // Test-specific filter.
+  sender()->Send(new TestMsg_Bounce);
 
   // A message of a different class should be seen only by the global filter.
-  Send(sender(), kMessageClass + 1, SEND);
+  sender()->Send(new UtilityMsg_Bounce);
 
   // Flush all messages.
   SendQuitMessageAndWaitForIdle();
@@ -267,16 +320,15 @@ TEST_F(IPCChannelProxyTest, GlobalAndMessageClassFilters) {
   // The class filter should have received only the class-specific message.
   EXPECT_EQ(1U, class_filter->messages_received());
 
-  // The global filter should have received both SEND messages, as well as the
-  // final QUIT message.
+  // The global filter should have received both messages, as well as the final
+  // QUIT message.
   EXPECT_EQ(3U, global_filter->messages_received());
 }
 
 TEST_F(IPCChannelProxyTest, FilterRemoval) {
   // Add a class and global filter.
-  const int kMessageClass = 7;
   scoped_refptr<MessageCountFilter> class_filter(
-      new MessageCountFilter(kMessageClass));
+      new MessageCountFilter(TestMsgStart));
   scoped_refptr<MessageCountFilter> global_filter(new MessageCountFilter());
 
   // Add and remove both types of filters.
@@ -286,8 +338,8 @@ TEST_F(IPCChannelProxyTest, FilterRemoval) {
   channel_proxy()->RemoveFilter(class_filter.get());
 
   // Send some messages; they should not be seen by either filter.
-  Send(sender(), 0, SEND);
-  Send(sender(), kMessageClass, SEND);
+  sender()->Send(new TestMsg_Bounce);
+  sender()->Send(new UtilityMsg_Bounce);
 
   // Ensure that the filters were removed and did not receive any messages.
   SendQuitMessageAndWaitForIdle();
@@ -299,6 +351,77 @@ TEST_F(IPCChannelProxyTest, FilterRemoval) {
   EXPECT_EQ(0U, global_filter->messages_received());
 }
 
+// The test that follow trigger DCHECKS in debug build.
+#if defined(NDEBUG) && !defined(DCHECK_ALWAYS_ON)
+
+TEST_F(IPCChannelProxyTest, BadMessageOnListenerThread) {
+  scoped_refptr<MessageCountFilter> class_filter(
+      new MessageCountFilter(TestMsgStart));
+  class_filter->set_message_filtering_enabled(false);
+  channel_proxy()->AddFilter(class_filter.get());
+
+  sender()->Send(new TestMsg_SendBadMessage());
+
+  SendQuitMessageAndWaitForIdle();
+  EXPECT_TRUE(DidListenerGetBadMessage());
+}
+
+TEST_F(IPCChannelProxyTest, BadMessageOnIPCThread) {
+  scoped_refptr<MessageCountFilter> class_filter(
+      new MessageCountFilter(TestMsgStart));
+  class_filter->set_message_filtering_enabled(true);
+  channel_proxy()->AddFilter(class_filter.get());
+
+  sender()->Send(new TestMsg_SendBadMessage());
+
+  SendQuitMessageAndWaitForIdle();
+  EXPECT_TRUE(DidListenerGetBadMessage());
+}
+
+class IPCChannelBadMessageTest : public IPCTestBase {
+ public:
+  IPCChannelBadMessageTest() {}
+  virtual ~IPCChannelBadMessageTest() {}
+
+  virtual void SetUp() OVERRIDE {
+    IPCTestBase::SetUp();
+
+    Init("ChannelProxyClient");
+
+    listener_.reset(new QuitListener());
+    CreateChannel(listener_.get());
+    ASSERT_TRUE(ConnectChannel());
+
+    ASSERT_TRUE(StartClient());
+  }
+
+  virtual void TearDown() {
+    listener_.reset();
+    IPCTestBase::TearDown();
+  }
+
+  void SendQuitMessageAndWaitForIdle() {
+    sender()->Send(new WorkerMsg_Quit);
+    base::MessageLoop::current()->Run();
+    EXPECT_TRUE(WaitForClientShutdown());
+  }
+
+  bool DidListenerGetBadMessage() {
+    return listener_->bad_message_received_;
+  }
+
+ private:
+  scoped_ptr<QuitListener> listener_;
+};
+
+TEST_F(IPCChannelBadMessageTest, BadMessage) {
+  sender()->Send(new TestMsg_SendBadMessage());
+  SendQuitMessageAndWaitForIdle();
+  EXPECT_TRUE(DidListenerGetBadMessage());
+}
+
+#endif
+
 MULTIPROCESS_IPC_TEST_CLIENT_MAIN(ChannelProxyClient) {
   base::MessageLoopForIO main_message_loop;
   ChannelReflectorListener listener;

ipc/ipc_channel_proxy_unittest_messages.h

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ipc/ipc_message_macros.h"
+
+// Singly-included section for enums and custom IPC traits.
+#ifndef IPC_CHANNEL_PROXY_UNITTEST_MESSAGES_H_
+#define IPC_CHANNEL_PROXY_UNITTEST_MESSAGES_H_
+
+class BadType {
+ public:
+  BadType() {}
+};
+
+namespace IPC {
+
+template <>
+struct ParamTraits<BadType> {
+  static void Write(Message* m, const BadType& p) {}
+  static bool Read(const Message* m, PickleIterator* iter, BadType* r) {
+    return false;
+  }
+  static void Log(const BadType& p, std::string* l) {}
+};
+
+}
+
+#endif  // IPC_CHANNEL_PROXY_UNITTEST_MESSAGES_H_
+
+
+#define IPC_MESSAGE_START TestMsgStart
+IPC_MESSAGE_CONTROL0(TestMsg_Bounce)
+IPC_MESSAGE_CONTROL0(TestMsg_SendBadMessage)
+IPC_MESSAGE_CONTROL1(TestMsg_BadMessage, BadType)
+
+#undef IPC_MESSAGE_START
+#define IPC_MESSAGE_START UtilityMsgStart
+IPC_MESSAGE_CONTROL0(UtilityMsg_Bounce)
+
+#undef IPC_MESSAGE_START
+#define IPC_MESSAGE_START WorkerMsgStart
+IPC_MESSAGE_CONTROL0(WorkerMsg_Bounce)
+IPC_MESSAGE_CONTROL0(WorkerMsg_Quit)