Mojo: (POSIX) Pass channel handle to child.

R=darin@chromium.org

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

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

mojo/common/test/multiprocess_test_base.cc

@@ -8,6 +8,7 @@
 #include "base/logging.h"
 #include "base/process/kill.h"
 #include "base/process/process_handle.h"
+// TODO(vtl): Remove build_config.h include when fully implemented on Windows.
 #include "build/build_config.h"
 
 namespace mojo {
@@ -28,7 +29,7 @@ void MultiprocessTestBase::SetUp() {
 
 // TODO(vtl): Not implemented on Windows yet.
 #if defined(OS_POSIX)
-  platform_server_channel_ =
+  platform_server_channel =
       system::PlatformServerChannel::Create("TestChannel");
 #endif
 }
@@ -36,13 +37,13 @@ void MultiprocessTestBase::SetUp() {
 void MultiprocessTestBase::TearDown() {
   CHECK_EQ(test_child_handle_, base::kNullProcessHandle);
 
-  platform_server_channel_.reset();
+  platform_server_channel.reset();
 
   MultiProcessTest::TearDown();
 }
 
 void MultiprocessTestBase::StartChild(const std::string& test_child_name) {
-  CHECK(platform_server_channel_.get());
+  CHECK(platform_server_channel.get());
   CHECK(!test_child_name.empty());
   CHECK_EQ(test_child_handle_, base::kNullProcessHandle);
 
@@ -51,7 +52,7 @@ void MultiprocessTestBase::StartChild(const std::string& test_child_name) {
 #if defined(OS_POSIX)
   CommandLine unused(CommandLine::NO_PROGRAM);
   base::FileHandleMappingVector fds_to_map;
-  platform_server_channel_->GetDataNeededToPassClientChannelToChildProcess(
+  platform_server_channel->GetDataNeededToPassClientChannelToChildProcess(
       &unused, &fds_to_map);
   test_child_handle_ = SpawnChild(test_child_main, fds_to_map, false);
 #elif defined(OS_WIN)
@@ -61,7 +62,7 @@ void MultiprocessTestBase::StartChild(const std::string& test_child_name) {
 #endif
 // TODO(vtl): Not implemented on Windows yet.
 #if defined(OS_POSIX)
-  platform_server_channel_->ChildProcessLaunched();
+  platform_server_channel->ChildProcessLaunched();
 #endif
 
   CHECK_NE(test_child_handle_, base::kNullProcessHandle);
@@ -81,14 +82,14 @@ int MultiprocessTestBase::WaitForChildShutdown() {
 
 CommandLine MultiprocessTestBase::MakeCmdLine(const std::string& procname,
                                               bool debug_on_start) {
-  CHECK(platform_server_channel_.get());
+  CHECK(platform_server_channel.get());
 
   CommandLine command_line =
       base::MultiProcessTest::MakeCmdLine(procname, debug_on_start);
 // TODO(vtl): Not implemented on Windows yet.
 #if defined(OS_POSIX)
   base::FileHandleMappingVector unused;
-  platform_server_channel_->GetDataNeededToPassClientChannelToChildProcess(
+  platform_server_channel->GetDataNeededToPassClientChannelToChildProcess(
       &command_line, &unused);
 #endif
   return command_line;
@@ -96,9 +97,19 @@ CommandLine MultiprocessTestBase::MakeCmdLine(const std::string& procname,
 
 // static
 void MultiprocessTestBase::ChildSetup() {
-  // TODO(vtl)
-  NOTIMPLEMENTED();
+  CHECK(CommandLine::InitializedForCurrentProcess());
+// TODO(vtl): Not implemented on Windows yet.
+#if defined(OS_POSIX)
+  platform_client_channel =
+      system::PlatformClientChannel::CreateFromParentProcess(
+          *CommandLine::ForCurrentProcess());
+  CHECK(platform_client_channel.get());
+#endif
 }
 
+// static
+scoped_ptr<system::PlatformClientChannel>
+    MultiprocessTestBase::platform_client_channel;
+
 }  // namespace test
 }  // namespace mojo

mojo/common/test/multiprocess_test_base.h

@@ -39,9 +39,11 @@ class MultiprocessTestBase : public base::MultiProcessTest {
   // For use by |MOJO_MULTIPROCESS_TEST_CHILD_MAIN()| only:
   static void ChildSetup();
 
-  system::PlatformServerChannel* platform_server_channel() {
-    return platform_server_channel_.get();
-  }
+  // For use in the main process:
+  scoped_ptr<system::PlatformServerChannel> platform_server_channel;
+
+  // For use (and only valid) in the child process:
+  static scoped_ptr<system::PlatformClientChannel> platform_client_channel;
 
  private:
   virtual CommandLine MakeCmdLine(const std::string& procname,
@@ -50,8 +52,6 @@ class MultiprocessTestBase : public base::MultiProcessTest {
   // Valid after |StartChild()| and before |WaitForChildShutdown()|.
   base::ProcessHandle test_child_handle_;
 
-  scoped_ptr<system::PlatformServerChannel> platform_server_channel_;
-
   DISALLOW_COPY_AND_ASSIGN(MultiprocessTestBase);
 };
 

mojo/common/test/multiprocess_test_base_unittest.cc

@@ -4,6 +4,10 @@
 
 #include "mojo/common/test/multiprocess_test_base.h"
 
+#include "base/logging.h"
+// TODO(vtl): Remove build_config.h include when fully implemented on Windows.
+#include "build/build_config.h"
+
 namespace mojo {
 namespace {
 
@@ -11,11 +15,22 @@ class MultiprocessTestBaseTest : public test::MultiprocessTestBase {
 };
 
 TEST_F(MultiprocessTestBaseTest, RunChild) {
+// TODO(vtl): Not implemented on Windows yet.
+#if defined(OS_POSIX)
+  EXPECT_TRUE(platform_server_channel.get());
+  EXPECT_TRUE(platform_server_channel->is_valid());
+#endif
   StartChild("RunChild");
   EXPECT_EQ(123, WaitForChildShutdown());
 }
 
 MOJO_MULTIPROCESS_TEST_CHILD_MAIN(RunChild) {
+// TODO(vtl): Not implemented on Windows yet.
+#if defined(OS_POSIX)
+  CHECK(MultiprocessTestBaseTest::platform_client_channel.get());
+  CHECK(MultiprocessTestBaseTest::platform_client_channel->is_valid());
+  // TODO(vtl): Check the client channel.
+#endif
   return 123;
 }
 

mojo/mojo.gyp

@@ -92,6 +92,7 @@
         'system/message_pipe_endpoint.h',
         'system/platform_channel.cc',
         'system/platform_channel.h',
+        'system/platform_channel_handle.cc',
         'system/platform_channel_handle.h',
         'system/platform_channel_posix.cc',
         'system/proxy_message_pipe_endpoint.cc',

mojo/system/platform_channel.cc

@@ -10,9 +10,7 @@ namespace mojo {
 namespace system {
 
 PlatformChannel::~PlatformChannel() {
-  // Implementations must close the handle if necessary (e.g., if no one else
-  // has taken ownership).
-  DCHECK(!is_valid());
+  handle_.CloseIfNecessary();
 }
 
 PlatformChannelHandle PlatformChannel::PassHandle() {
@@ -34,6 +32,7 @@ PlatformServerChannel::PlatformServerChannel(const std::string& name)
 // static
 scoped_ptr<PlatformClientChannel> PlatformClientChannel::CreateFromHandle(
       const PlatformChannelHandle& handle) {
+  DCHECK(handle.is_valid());
   scoped_ptr<PlatformClientChannel> rv(new PlatformClientChannel());
   *rv->mutable_handle() = handle;
   return rv.Pass();

mojo/system/platform_channel_handle.cc

+// Copyright 2013 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 "mojo/system/platform_channel_handle.h"
+
+#include "build/build_config.h"
+#if defined(OS_POSIX)
+#include <unistd.h>
+#elif defined(OS_WIN)
+#include <windows.h>
+#else
+#error "Platform not yet supported."
+#endif
+
+#include "base/logging.h"
+
+namespace mojo {
+namespace system {
+
+void PlatformChannelHandle::CloseIfNecessary() {
+  if (!is_valid())
+    return;
+
+#if defined(OS_POSIX)
+  DPCHECK(close(fd) == 0);
+  fd = -1;
+#elif defined(OS_WIN)
+  DPCHECK(CloseHandle(handle));
+  handle = INVALID_HANDLE_VALUE;
+#else
+#error "Platform not yet supported."
+#endif
+}
+
+}  // namespace system
+}  // namespace mojo

mojo/system/platform_channel_handle.h

@@ -19,6 +19,8 @@ struct PlatformChannelHandle {
   PlatformChannelHandle() : fd(-1) {}
   explicit PlatformChannelHandle(int fd) : fd(fd) {}
 
+  void CloseIfNecessary();
+
   bool is_valid() const { return fd != -1; }
 
   int fd;
@@ -28,6 +30,8 @@ struct PlatformChannelHandle {
   PlatformChannelHandle() : handle(INVALID_HANDLE_VALUE) {}
   explicit PlatformChannelHandle(HANDLE handle) : handle(handle) {}
 
+  void CloseIfNecessary();
+
   bool is_valid() const { return handle != INVALID_HANDLE_VALUE; }
 
   HANDLE handle;

mojo/system/platform_channel_posix.cc

@@ -9,20 +9,27 @@
 #include <sys/types.h>
 #include <unistd.h>
 
+#include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/logging.h"
+#include "base/posix/global_descriptors.h"
+#include "base/strings/string_number_conversions.h"
 
 namespace mojo {
 namespace system {
 
 namespace {
 
-void CloseIfNecessary(PlatformChannelHandle* handle) {
-  if (!handle->is_valid())
-    return;
+const char kMojoChannelDescriptorSwitch[] = "mojo-channel-descriptor";
 
-  PCHECK(close(handle->fd) == 0);
-  *handle = PlatformChannelHandle();
+bool IsTargetDescriptorUsed(
+    const base::FileHandleMappingVector& file_handle_mapping,
+    int target_fd) {
+  for (size_t i = 0; i < file_handle_mapping.size(); i++) {
+    if (file_handle_mapping[i].second == target_fd)
+      return true;
+  }
+  return false;
 }
 
 class PlatformServerChannelPosix : public PlatformServerChannel {
@@ -59,10 +66,7 @@ PlatformServerChannelPosix::PlatformServerChannelPosix(
 }
 
 PlatformServerChannelPosix::~PlatformServerChannelPosix() {
-  if (is_valid())
-    CloseIfNecessary(mutable_handle());
-  if (client_handle_.is_valid())
-    CloseIfNecessary(&client_handle_);
+  client_handle_.CloseIfNecessary();
 }
 
 scoped_ptr<PlatformClientChannel>
@@ -82,13 +86,39 @@ scoped_ptr<PlatformClientChannel>
 void PlatformServerChannelPosix::GetDataNeededToPassClientChannelToChildProcess(
     CommandLine* command_line,
     base::FileHandleMappingVector* file_handle_mapping) const {
-  // TODO(vtl)
-  NOTIMPLEMENTED();
+  DCHECK(command_line);
+  DCHECK(file_handle_mapping);
+  // This is an arbitrary sanity check. (Note that this guarantees that the loop
+  // below will terminate sanely.)
+  CHECK_LT(file_handle_mapping->size(), 1000u);
+
+  DCHECK(client_handle_.is_valid());
+
+  // Find a suitable FD to map our client handle to in the child process.
+  // This has quadratic time complexity in the size of |*file_handle_mapping|,
+  // but |*file_handle_mapping| should be very small (usually/often empty).
+  int target_fd = base::GlobalDescriptors::kBaseDescriptor;
+  while (IsTargetDescriptorUsed(*file_handle_mapping, target_fd))
+    target_fd++;
+
+  file_handle_mapping->push_back(std::pair<int, int>(client_handle_.fd,
+                                                     target_fd));
+  // Log a warning if the command line already has the switch, but "clobber" it
+  // anyway, since it's reasonably likely that all the switches were just copied
+  // from the parent.
+  LOG_IF(WARNING, command_line->HasSwitch(kMojoChannelDescriptorSwitch))
+      << "Child command line already has switch --"
+      << kMojoChannelDescriptorSwitch << "="
+      << command_line->GetSwitchValueASCII(kMojoChannelDescriptorSwitch);
+  // (Any existing switch won't actually be removed from the command line, but
+  // the last one appended takes precedence.)
+  command_line->AppendSwitchASCII(kMojoChannelDescriptorSwitch,
+                                  base::IntToString(target_fd));
 }
 
 void PlatformServerChannelPosix::ChildProcessLaunched() {
-  // TODO(vtl)
-  NOTIMPLEMENTED();
+  DCHECK(client_handle_.is_valid());
+  client_handle_.CloseIfNecessary();
 }
 
 }  // namespace
@@ -110,9 +140,17 @@ scoped_ptr<PlatformServerChannel> PlatformServerChannel::Create(
 scoped_ptr<PlatformClientChannel>
     PlatformClientChannel::CreateFromParentProcess(
         const CommandLine& command_line) {
-  // TODO(vtl)
-  NOTIMPLEMENTED();
+  std::string client_fd_string =
+      command_line.GetSwitchValueASCII(kMojoChannelDescriptorSwitch);
+  int client_fd = -1;
+  if (client_fd_string.empty() ||
+      !base::StringToInt(client_fd_string, &client_fd) ||
+      client_fd < base::GlobalDescriptors::kBaseDescriptor) {
+    LOG(ERROR) << "Missing or invalid --" << kMojoChannelDescriptorSwitch;
     return scoped_ptr<PlatformClientChannel>();
+  }
+
+  return CreateFromHandle(PlatformChannelHandle(client_fd));
 }
 
 }  // namespace system