Add check on invalid file descriptor at both broker and renderer sides.

content/renderer/pepper_plugin_delegate_impl_unittest.cc

Index: content/renderer/pepper_plugin_delegate_impl_unittest.cc
diff --git a/content/renderer/pepper_plugin_delegate_impl_unittest.cc b/content/renderer/pepper_plugin_delegate_impl_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..d1b8f7e85b5e100825a8276aa0db46522f0bd466
--- /dev/null
+++ b/content/renderer/pepper_plugin_delegate_impl_unittest.cc
@@ -0,0 +1,54 @@
+// Copyright (c) 2011 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 "content/renderer/pepper_plugin_delegate_impl.h"
+
+#if defined(OS_POSIX)
+#include <fcntl.h>
+#include <sys/socket.h>
+#endif  // defined(OS_POSIX));
+
+#include "content/test/mock_render_process.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class PepperPluginDelegateImplTest : public ::testing::Test {
+ protected:
+  MessageLoopForIO message_loop_;
+  // We need a render process for ppapi::proxy::ProxyChannel to work.
+  MockRenderProcess mock_process_;
+};
+
+TEST_F(PepperPluginDelegateImplTest, BrokerDispatcherWrapperInitFailure) {
+  // Try to initialize BrokerDispatcherWrapper with invalid ChannelHandle.

[ddorwin, 2011/11/07 23:54:07]

This is a test-level comment and should be above the TEST_F.

[xhwang, 2011/11/08 00:45:39]

Done.

+  // Initialization should fail.
+  BrokerDispatcherWrapper dispatcher_wrapper;
+  base::ProcessHandle broker_process_handle = base::kNullProcessHandle;
+  IPC::ChannelHandle invalid_channel;  // Invalid by default.
+
+  // An invalide handle should result in a failure (false) without a CHECK, such

[ddorwin, 2011/11/07 23:54:07]

invalid
"CHECK" is misleading since it's a LOG(FATAL). Crash might be better but isn't
entirely accurate.

[xhwang, 2011/11/08 00:45:39]

Done.

+  // as the one in CreatePipe().  Call it twice because without the invalid
+  // handle check, the posix code would hit a one-time path due to a static
+  // variable and cause a LOG(FATAL).

[ddorwin, 2011/11/07 23:54:07]

... and not go through the LOG(FATAL) path.

[xhwang, 2011/11/08 00:45:39]

Done.

+  EXPECT_FALSE(dispatcher_wrapper.Init(broker_process_handle, invalid_channel));
+  EXPECT_FALSE(dispatcher_wrapper.Init(broker_process_handle, invalid_channel));
+
+  // On valid ChannelHandle, initialization should succeed.

[ddorwin, 2011/11/07 23:54:07]

Success should be a different test - something like
BrokerDispatcherWrapperInitSuccess.

[xhwang, 2011/11/08 00:45:39]

Done.

+  const char kChannelName[] = "/tmp/PepperPluginDelegateImplTestChannelName";

[ddorwin, 2011/11/07 23:54:07]

Is this a file or just a string? This is weird on Windows too.

[xhwang, 2011/11/08 00:45:39]

Done.

+#if defined(OS_POSIX)
+  int fds[2] = {-1, -1};

[ddorwin, 2011/11/07 23:54:07]

Is there a CreateSocketPair() or something like that we can use instead?

[xhwang, 2011/11/08 00:45:39]

bool SocketPair(int* fd1, int* fd2) is defined in ipc_channel_posix.cc but it's
mostly local and not declared in header files.

+  ASSERT_EQ(0, socketpair(AF_UNIX, SOCK_STREAM, 0, fds));
+  // Channel::ChannelImpl::CreatePipe needs the fd to be non-blocking.
+  ASSERT_EQ(0, fcntl(fds[1], F_SETFL, O_NONBLOCK));
+  base::FileDescriptor file_descriptor(fds[1], true);  // Auto close.
+  IPC::ChannelHandle valid_channel(kChannelName, file_descriptor);
+#else
+  IPC::ChannelHandle valid_channel(kChannelName);
+#endif  // defined(OS_POSIX));
+
+  EXPECT_TRUE(dispatcher_wrapper.Init(broker_process_handle, valid_channel));
+
+#if defined(OS_POSIX)
+  EXPECT_EQ(0, ::close(fds[0]));
+#endif  // defined(OS_POSIX));
+}