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..10a9fcd6406fb58033472ef7f0d29c3c41c7a5a6
--- /dev/null
+++ b/content/renderer/pepper_plugin_delegate_impl_unittest.cc
@@ -0,0 +1,49 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.

[Paweł Hajdan Jr., 2011/11/04 08:05:15]

nit: 2011 not 2009.

[xhwang, 2011/11/04 15:44:48]

Done.

+// 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"
+#include "content/test/mock_render_process.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class PepperPluginDelegateImplTest : public ::testing::Test {
+ public:
+  PepperPluginDelegateImplTest() {}

[Paweł Hajdan Jr., 2011/11/04 08:05:15]

nit: Is the ctor needed here? Is the dtor needed?

[xhwang, 2011/11/04 15:44:48]

Done.

+  virtual ~PepperPluginDelegateImplTest() {}
+
+  virtual void SetUp() {
+    mock_process_.reset(new MockRenderProcess);
+  }
+
+  virtual void TearDown() {
+    mock_process_.reset();
+  }
+
+ protected:
+  MessageLoopForIO message_loop_;
+  // ppapi::proxy::ProxyChannel needs to be initialized on the render process.
+  scoped_ptr<MockRenderProcess> mock_process_;

[Paweł Hajdan Jr., 2011/11/04 08:05:15]

nit: I think you can just declare MockRenderProcess mock_process_, without
scoped_ptr and SetUp and TearDown. The entire test fixture is created fresh and
destroyed for each test, i.e. ctor, SetUp, test body, TearDown, dtor, ctor,
SetUp, test body, TearDown, dtor instead of ctor, SetUp, test body, TearDown,
SetUp, test body, TearDown, dtor

[xhwang, 2011/11/04 15:44:48]

Great to know that.  Thanks!

+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(PepperPluginDelegateImplTest);

[Paweł Hajdan Jr., 2011/11/04 08:05:15]

nit: No need for this in test fixtures.

[xhwang, 2011/11/04 15:44:48]

Done.

+};
+
+// Try to initialize BrokerDispatcherWrapper with invalid ChannelHandle.
+// Initialization should fail.
+TEST_F(PepperPluginDelegateImplTest, BrokerDispatcherWrapperInitFailure) {
+  BrokerDispatcherWrapper dispatcher_wrapper;
+  base::ProcessHandle broker_process_handle = base::kNullProcessHandle;
+  IPC::ChannelHandle channel_handle; // Invalid by default.

[Paweł Hajdan Jr., 2011/11/04 08:05:15]

nit: Two spaces between code and comment please.

[xhwang, 2011/11/04 15:44:48]

Done.

+
+  // Try BrokerDispatcherWrapper.Init() TWICE to trigger the LOG(FATAL)
+  // of "Denying attempt to reuse initial IPC channel for ..." in
+  // IPC::Channel::ChannelImpl::CreatePipe() in case invalid ChannelHandle is
+  // not checked properly.  If invalid ChannelHandle is not checked and passed
+  // all the way down to IPC::Channel::ChannelImpl::CreatePipe(), CreatePipe
+  // will be called in client mode with channel_handle.socket.fd == -1.  There
+  // is a static variable "bool used_initial_channel = false" in this function.
+  // In the first test it will be set to true.  In the second test the
+  // LOG(FATAL) error will be triggered.
+  EXPECT_FALSE(dispatcher_wrapper.Init(broker_process_handle, channel_handle));
+  EXPECT_FALSE(dispatcher_wrapper.Init(broker_process_handle, channel_handle));
+}