mojo: edk::PeerConnection -> IsolatedConnection

Introduces a public IsolatedConnection helper as a replacement for the
EDK PeerConnection type. A follow-up C will convert existing uses.

Mojo multiprocess tests are converted to use the new type here,
removing the last remnants of EDK dependency from the test multiprocess
test support code.

This also deletes some dead test code and opportunistically converts a
few related uses of the internal EDK ScopedInternalPlatformHandle type
to the newer public PlatformHandle type.

Bug: 844763
Change-Id: Icad97f51d320381483e474c16077ef7844cc2b9d
Reviewed-on: https://chromium-review.googlesource.com/1099269
Commit-Queue: Ken Rockot <rockot@chromium.org>
Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567323}

mojo/edk/embedder/platform_channel_pair_posix_unittest.cc

@@ -156,8 +156,8 @@ TEST_F(PlatformChannelPairPosixTest, SendReceiveFDs) {
           base::CreateAndOpenTemporaryFileInDir(temp_dir.GetPath(), &unused));
       ASSERT_TRUE(fp);
       ASSERT_EQ(j, fwrite(std::string(j, c).data(), 1, j, fp.get()));
-      platform_handles.push_back(
-          test::InternalPlatformHandleFromFILE(std::move(fp)));
+      platform_handles.emplace_back(InternalPlatformHandle(
+          test::PlatformHandleFromFILE(std::move(fp)).ReleaseFD()));
       ASSERT_TRUE(platform_handles.back().is_valid());
     }
 
@@ -180,8 +180,10 @@ TEST_F(PlatformChannelPairPosixTest, SendReceiveFDs) {
     EXPECT_EQ(i, received_handles.size());
 
     for (size_t j = 0; j < received_handles.size(); j++) {
-      base::ScopedFILE fp(test::FILEFromInternalPlatformHandle(
-          std::move(received_handles.front()), "rb"));
+      base::ScopedFILE fp(test::FILEFromPlatformHandle(
+          PlatformHandle(
+              base::ScopedFD(received_handles.front().release().handle)),
+          "rb"));
       received_handles.pop_front();
       ASSERT_TRUE(fp);
       rewind(fp.get());
@@ -213,7 +215,8 @@ TEST_F(PlatformChannelPairPosixTest, AppendReceivedFDs) {
     ASSERT_EQ(file_contents.size(),
               fwrite(file_contents.data(), 1, file_contents.size(), fp.get()));
     std::vector<ScopedInternalPlatformHandle> platform_handles(1);
-    platform_handles[0] = test::InternalPlatformHandleFromFILE(std::move(fp));
+    platform_handles[0].reset(InternalPlatformHandle(
+        test::PlatformHandleFromFILE(std::move(fp)).ReleaseFD()));
     ASSERT_TRUE(platform_handles.back().is_valid());
 
     // Send the FD (+ "hello").
@@ -241,8 +244,9 @@ TEST_F(PlatformChannelPairPosixTest, AppendReceivedFDs) {
   EXPECT_TRUE(received_handles[1].is_valid());
 
   {
-    base::ScopedFILE fp(test::FILEFromInternalPlatformHandle(
-        std::move(received_handles[1]), "rb"));
+    base::ScopedFILE fp(test::FILEFromPlatformHandle(
+        PlatformHandle(base::ScopedFD(received_handles[1].release().handle)),
+        "rb"));
     ASSERT_TRUE(fp);
     rewind(fp.get());
     char read_buf[100];

mojo/edk/system/multiprocess_message_pipe_unittest.cc

@@ -32,6 +32,7 @@
 #include "mojo/public/c/system/functions.h"
 #include "mojo/public/c/system/types.h"
 #include "mojo/public/cpp/system/message_pipe.h"
+#include "mojo/public/cpp/system/platform_handle.h"
 #include "mojo/public/cpp/system/simple_watcher.h"
 #include "mojo/public/cpp/system/wait.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -431,13 +432,10 @@ DEFINE_TEST_CLIENT_WITH_PIPE(CheckInternalPlatformHandleFile,
   CHECK_GT(num_handles, 0);
 
   for (int i = 0; i < num_handles; ++i) {
-    ScopedInternalPlatformHandle h;
-    CHECK_EQ(PassWrappedInternalPlatformHandle(handles[i], &h), MOJO_RESULT_OK);
+    PlatformHandle h = UnwrapPlatformHandle(ScopedHandle(Handle(handles[i])));
     CHECK(h.is_valid());
-    MojoClose(handles[i]);
 
-    base::ScopedFILE fp(
-        test::FILEFromInternalPlatformHandle(std::move(h), "r"));
+    base::ScopedFILE fp = test::FILEFromPlatformHandle(std::move(h), "r");
     CHECK(fp);
     std::string fread_buffer(100, '\0');
     size_t bytes_read =
@@ -470,13 +468,10 @@ TEST_P(MultiprocessMessagePipeTestWithPipeCount,
       CHECK_EQ(fwrite(&world[0], 1, world.size(), fp.get()), world.size());
       fflush(fp.get());
       rewind(fp.get());
-      MojoHandle handle;
-      ASSERT_EQ(CreateInternalPlatformHandleWrapper(
-                    ScopedInternalPlatformHandle(
-                        test::InternalPlatformHandleFromFILE(std::move(fp))),
-                    &handle),
-                MOJO_RESULT_OK);
-      handles.push_back(handle);
+      ScopedHandle handle =
+          WrapPlatformHandle(test::PlatformHandleFromFILE(std::move(fp)));
+      ASSERT_TRUE(handle.is_valid());
+      handles.push_back(handle.release().value());
     }
 
     char message[128];

mojo/edk/system/platform_handle_dispatcher_unittest.cc

@@ -12,14 +12,39 @@
 #include "base/files/scoped_file.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/memory/ref_counted.h"
+#include "mojo/edk/embedder/platform_handle_utils.h"
 #include "mojo/edk/embedder/scoped_platform_handle.h"
 #include "mojo/edk/test/test_utils.h"
+#include "mojo/public/cpp/system/platform_handle.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace mojo {
 namespace edk {
 namespace {
 
+ScopedInternalPlatformHandle PlatformHandleToScopedInternalPlatformHandle(
+    PlatformHandle platform_handle) {
+  MojoPlatformHandle handle;
+  PlatformHandleToMojoPlatformHandle(std::move(platform_handle), &handle);
+  ScopedInternalPlatformHandle internal_handle;
+  MojoResult result = MojoPlatformHandleToScopedInternalPlatformHandle(
+      &handle, &internal_handle);
+  if (result != MOJO_RESULT_OK)
+    return ScopedInternalPlatformHandle();
+  return internal_handle;
+}
+
+PlatformHandle ScopedInternalPlatformHandleToPlatformHandle(
+    ScopedInternalPlatformHandle internal_handle) {
+  MojoPlatformHandle handle;
+  handle.struct_size = sizeof(handle);
+  MojoResult result = ScopedInternalPlatformHandleToMojoPlatformHandle(
+      std::move(internal_handle), &handle);
+  if (result != MOJO_RESULT_OK)
+    return PlatformHandle();
+  return MojoPlatformHandleToPlatformHandle(&handle);
+}
+
 TEST(PlatformHandleDispatcherTest, Basic) {
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
@@ -33,20 +58,21 @@ TEST(PlatformHandleDispatcherTest, Basic) {
   EXPECT_EQ(sizeof(kHelloWorld),
             fwrite(kHelloWorld, 1, sizeof(kHelloWorld), fp.get()));
 
-  ScopedInternalPlatformHandle h(
-      test::InternalPlatformHandleFromFILE(std::move(fp)));
+  PlatformHandle h = test::PlatformHandleFromFILE(std::move(fp));
   EXPECT_FALSE(fp);
   ASSERT_TRUE(h.is_valid());
 
   scoped_refptr<PlatformHandleDispatcher> dispatcher =
-      PlatformHandleDispatcher::Create(std::move(h));
+      PlatformHandleDispatcher::Create(
+          PlatformHandleToScopedInternalPlatformHandle(std::move(h)));
   EXPECT_FALSE(h.is_valid());
   EXPECT_EQ(Dispatcher::Type::PLATFORM_HANDLE, dispatcher->GetType());
 
-  h = dispatcher->PassInternalPlatformHandle();
+  h = ScopedInternalPlatformHandleToPlatformHandle(
+      dispatcher->PassInternalPlatformHandle());
   EXPECT_TRUE(h.is_valid());
 
-  fp = test::FILEFromInternalPlatformHandle(std::move(h), "rb");
+  fp = test::FILEFromPlatformHandle(std::move(h), "rb");
   EXPECT_FALSE(h.is_valid());
   EXPECT_TRUE(fp);
 
@@ -57,8 +83,8 @@ TEST(PlatformHandleDispatcherTest, Basic) {
   EXPECT_STREQ(kHelloWorld, read_buffer);
 
   // Try getting the handle again. (It should fail cleanly.)
-  h = dispatcher->PassInternalPlatformHandle();
-  EXPECT_FALSE(h.is_valid());
+  auto internal_handle = dispatcher->PassInternalPlatformHandle();
+  EXPECT_FALSE(internal_handle.is_valid());
 
   EXPECT_EQ(MOJO_RESULT_OK, dispatcher->Close());
 }
@@ -76,7 +102,8 @@ TEST(PlatformHandleDispatcherTest, Serialization) {
 
   scoped_refptr<PlatformHandleDispatcher> dispatcher =
       PlatformHandleDispatcher::Create(
-          test::InternalPlatformHandleFromFILE(std::move(fp)));
+          PlatformHandleToScopedInternalPlatformHandle(
+              test::PlatformHandleFromFILE(std::move(fp))));
 
   uint32_t num_bytes = 0;
   uint32_t num_ports = 0;
@@ -110,8 +137,10 @@ TEST(PlatformHandleDispatcherTest, Serialization) {
   EXPECT_FALSE(received_handle.is_valid());
   EXPECT_TRUE(dispatcher->GetType() == Dispatcher::Type::PLATFORM_HANDLE);
 
-  fp = test::FILEFromInternalPlatformHandle(
-      dispatcher->PassInternalPlatformHandle(), "rb");
+  fp = test::FILEFromPlatformHandle(
+      ScopedInternalPlatformHandleToPlatformHandle(
+          dispatcher->PassInternalPlatformHandle()),
+      "rb");
   EXPECT_TRUE(fp);
 
   rewind(fp.get());

mojo/edk/test/BUILD.gn

@@ -18,10 +18,8 @@ static_library("test_support") {
     ]
   }
 
-  if (is_fuchsia) {
-    sources += [ "test_utils_fuchsia.cc" ]
-  } else if (is_posix) {
-    sources += [ "test_utils_posix.cc" ]
+  if (is_fuchsia || is_posix) {
+    sources += [ "test_utils.cc" ]
   }
 
   public_deps = [

mojo/edk/test/mojo_test_base.cc

@@ -47,15 +47,14 @@ MojoTestBase::~MojoTestBase() {}
 
 MojoTestBase::ClientController& MojoTestBase::StartClient(
     const std::string& client_name) {
-  clients_.push_back(std::make_unique<ClientController>(
-      client_name, this, process_error_callback_, launch_type_));
+  clients_.push_back(
+      std::make_unique<ClientController>(client_name, this, launch_type_));
   return *clients_.back();
 }
 
 MojoTestBase::ClientController::ClientController(
     const std::string& client_name,
     MojoTestBase* test,
-    const ProcessErrorCallback& process_error_callback,
     LaunchType launch_type) {
 #if !defined(OS_IOS)
 #if defined(OS_MACOSX)
@@ -67,7 +66,6 @@ MojoTestBase::ClientController::ClientController(
   // launch and child pid registration.
   base::AutoLock lock(g_mach_broker->GetLock());
 #endif
-  helper_.set_process_error_callback(process_error_callback);
   pipe_ = helper_.StartChild(client_name, launch_type);
 #if defined(OS_MACOSX)
   g_mach_broker->AddPlaceholderForPid(helper_.test_child().Handle());

mojo/edk/test/mojo_test_base.h

@@ -14,7 +14,6 @@
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
-#include "mojo/edk/embedder/embedder.h"
 #include "mojo/edk/test/multiprocess_test_helper.h"
 #include "mojo/public/c/system/trap.h"
 #include "mojo/public/c/system/types.h"
@@ -37,7 +36,6 @@ class MojoTestBase : public testing::Test {
    public:
     ClientController(const std::string& client_name,
                      MojoTestBase* test,
-                     const ProcessErrorCallback& process_error_callback,
                      LaunchType launch_type);
     ~ClientController();
 
@@ -57,13 +55,6 @@ class MojoTestBase : public testing::Test {
     DISALLOW_COPY_AND_ASSIGN(ClientController);
   };
 
-  // Set the callback to handle bad messages received from test client
-  // processes. This can be set to a different callback before starting each
-  // client.
-  void set_process_error_callback(const ProcessErrorCallback& callback) {
-    process_error_callback_ = callback;
-  }
-
   ClientController& StartClient(const std::string& client_name);
 
   template <typename HandlerFunc>
@@ -175,8 +166,6 @@ class MojoTestBase : public testing::Test {
 
   std::vector<std::unique_ptr<ClientController>> clients_;
 
-  ProcessErrorCallback process_error_callback_;
-
   LaunchType launch_type_ = LaunchType::CHILD;
 
   DISALLOW_COPY_AND_ASSIGN(MojoTestBase);

mojo/edk/test/multiprocess_test_helper.cc

@@ -26,17 +26,12 @@
 #include "base/task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
-#include "mojo/edk/embedder/embedder.h"
-#include "mojo/edk/embedder/named_platform_handle.h"
-#include "mojo/edk/embedder/named_platform_handle_utils.h"
-#include "mojo/edk/embedder/peer_connection.h"
-#include "mojo/edk/embedder/platform_channel_pair.h"
-#include "mojo/edk/embedder/platform_handle_utils.h"
 #include "mojo/public/cpp/platform/named_platform_channel.h"
 #include "mojo/public/cpp/platform/platform_channel.h"
 #include "mojo/public/cpp/platform/platform_channel_endpoint.h"
 #include "mojo/public/cpp/platform/platform_channel_server_endpoint.h"
 #include "mojo/public/cpp/system/invitation.h"
+#include "mojo/public/cpp/system/isolated_connection.h"
 #include "mojo/public/cpp/system/platform_handle.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -56,7 +51,7 @@ const char kRunAsBrokerClient[] = "run-as-broker-client";
 const char kTestChildMessagePipeName[] = "test_pipe";
 
 // For use (and only valid) in a test child process:
-base::LazyInstance<PeerConnection>::Leaky g_child_peer_connection;
+base::LazyInstance<IsolatedConnection>::Leaky g_child_isolated_connection;
 
 template <typename Func>
 int RunClientFunction(Func handler, bool pass_pipe_ownership_to_main) {
@@ -68,20 +63,6 @@ int RunClientFunction(Func handler, bool pass_pipe_ownership_to_main) {
   return handler(pipe_handle.value());
 }
 
-// TODO(https://844763): Clean this up. Some test code still relies on old EDK
-// APIs and thus internal platform handle types. We try to use the new public
-// types as much as possible, so this does a reliable conversion from the new
-// type to the old type.
-ScopedInternalPlatformHandle PlatformHandleToScopedInternalPlatformHandle(
-    PlatformHandle handle) {
-  MojoPlatformHandle platform_handle;
-  PlatformHandleToMojoPlatformHandle(std::move(handle), &platform_handle);
-  ScopedInternalPlatformHandle edk_handle;
-  MojoPlatformHandleToScopedInternalPlatformHandle(&platform_handle,
-                                                   &edk_handle);
-  return edk_handle;
-}
-
 }  // namespace
 
 MultiprocessTestHelper::MultiprocessTestHelper() {}
@@ -193,27 +174,17 @@ ScopedMessagePipeHandle MultiprocessTestHelper::StartChildWithExtraSwitch(
     command_line.AppendSwitch(kRunAsBrokerClient);
   } else if (launch_type == LaunchType::PEER ||
              launch_type == LaunchType::NAMED_PEER) {
-    peer_connection_ = std::make_unique<PeerConnection>();
-
-    // TODO(https://844763): Either move peer connection into the C API or
-    // (preferably) get rid of it altogether. For now we do this dance to get
-    // the new public handle types to work with the peer connection API.
-    ScopedInternalPlatformHandle local_handle;
+    isolated_connection_ = std::make_unique<IsolatedConnection>();
     if (local_channel_endpoint.is_valid()) {
-      local_handle = PlatformHandleToScopedInternalPlatformHandle(
-          local_channel_endpoint.TakePlatformHandle());
+      pipe = isolated_connection_->Connect(std::move(local_channel_endpoint));
     } else {
 #if defined(OS_POSIX) || defined(OS_WIN)
       DCHECK(server_endpoint.is_valid());
-      local_handle = PlatformHandleToScopedInternalPlatformHandle(
-          server_endpoint.TakePlatformHandle());
-      local_handle.get().needs_connection = true;
+      pipe = isolated_connection_->Connect(std::move(server_endpoint));
 #else
       NOTREACHED();
 #endif
     }
-    pipe = peer_connection_->Connect(
-        ConnectionParams(TransportProtocol::kLegacy, std::move(local_handle)));
   }
 
   test_child_ =
@@ -225,12 +196,12 @@ ScopedMessagePipeHandle MultiprocessTestHelper::StartChildWithExtraSwitch(
     DCHECK(local_channel_endpoint.is_valid());
     OutgoingInvitation::Send(std::move(child_invitation), test_child_.Handle(),
                              std::move(local_channel_endpoint),
-                             process_error_callback_);
+                             mojo::ProcessErrorCallback());
   } else if (launch_type == LaunchType::NAMED_CHILD) {
     DCHECK(server_endpoint.is_valid());
     OutgoingInvitation::Send(std::move(child_invitation), test_child_.Handle(),
                              std::move(server_endpoint),
-                             process_error_callback_);
+                             mojo::ProcessErrorCallback());
   }
 
   CHECK(test_child_.IsValid());
@@ -275,14 +246,11 @@ void MultiprocessTestHelper::ChildSetup() {
     primordial_pipe = invitation.ExtractMessagePipe(kTestChildMessagePipeName);
   } else {
     if (!named_pipe.empty()) {
-      NamedPlatformHandle pipe_name(named_pipe);
-      primordial_pipe = g_child_peer_connection.Get().Connect(ConnectionParams(
-          TransportProtocol::kLegacy, CreateClientHandle(pipe_name)));
+      primordial_pipe = g_child_isolated_connection.Get().Connect(
+          NamedPlatformChannel::ConnectToServer(named_pipe));
     } else {
-      primordial_pipe = g_child_peer_connection.Get().Connect(ConnectionParams(
-          TransportProtocol::kLegacy,
-          PlatformChannelPair::PassClientHandleFromParentProcess(
-              command_line)));
+      primordial_pipe = g_child_isolated_connection.Get().Connect(
+          PlatformChannel::RecoverPassedEndpointFromCommandLine(command_line));
     }
   }
 }

mojo/edk/test/multiprocess_test_helper.h

@@ -12,16 +12,14 @@
 #include "base/process/process.h"
 #include "base/test/multiprocess_test.h"
 #include "base/test/test_timeouts.h"
-#include "mojo/edk/embedder/embedder.h"
 #include "mojo/public/cpp/system/message_pipe.h"
 #include "testing/multiprocess_func_list.h"
 
 namespace mojo {
 
-namespace edk {
-
-class PeerConnection;
+class IsolatedConnection;
 
+namespace edk {
 namespace test {
 
 class MultiprocessTestHelper {
@@ -63,10 +61,6 @@ class MultiprocessTestHelper {
       const std::string& switch_value,
       LaunchType launch_type);
 
-  void set_process_error_callback(const ProcessErrorCallback& callback) {
-    process_error_callback_ = callback;
-  }
-
   // Wait for the child process to terminate.
   // Returns the exit code of the child process. Note that, though it's declared
   // to be an |int|, the exit code is subject to mangling by the OS. E.g., we
@@ -96,9 +90,7 @@ class MultiprocessTestHelper {
   // Valid after |StartChild()| and before |WaitForChildShutdown()|.
   base::Process test_child_;
 
-  ProcessErrorCallback process_error_callback_;
-
-  std::unique_ptr<PeerConnection> peer_connection_;
+  std::unique_ptr<IsolatedConnection> isolated_connection_;
 
   DISALLOW_COPY_AND_ASSIGN(MultiprocessTestHelper);
 };

mojo/edk/test/multiprocess_test_helper_unittest.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/edk/test/multiprocess_test_helper.h"
-
-#include <stddef.h>
-
-#include <utility>
-
-#include "base/logging.h"
-#include "build/build_config.h"
-#include "mojo/edk/embedder/scoped_platform_handle.h"
-#include "mojo/edk/system/test_utils.h"
-#include "mojo/edk/test/test_utils.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-#if defined(OS_POSIX)
-#include <fcntl.h>
-#endif
-
-namespace mojo {
-namespace edk {
-namespace test {
-namespace {
-
-bool IsNonBlocking(const InternalPlatformHandle& handle) {
-#if defined(OS_WIN)
-  // Haven't figured out a way to query whether a HANDLE was created with
-  // FILE_FLAG_OVERLAPPED.
-  return true;
-#else
-  return fcntl(handle.handle, F_GETFL) & O_NONBLOCK;
-#endif
-}
-
-bool WriteByte(const InternalPlatformHandle& handle, char c) {
-  size_t bytes_written = 0;
-  BlockingWrite(handle, &c, 1, &bytes_written);
-  return bytes_written == 1;
-}
-
-bool ReadByte(const InternalPlatformHandle& handle, char* c) {
-  size_t bytes_read = 0;
-  BlockingRead(handle, c, 1, &bytes_read);
-  return bytes_read == 1;
-}
-
-using MultiprocessTestHelperTest = testing::Test;
-
-TEST_F(MultiprocessTestHelperTest, RunChild) {
-  MultiprocessTestHelper helper;
-  EXPECT_TRUE(helper.server_platform_handle.is_valid());
-
-  helper.StartChild("RunChild");
-  EXPECT_EQ(123, helper.WaitForChildShutdown());
-}
-
-MOJO_MULTIPROCESS_TEST_CHILD_MAIN(RunChild) {
-  CHECK(MultiprocessTestHelper::client_platform_handle.is_valid());
-  return 123;
-}
-
-TEST_F(MultiprocessTestHelperTest, TestChildMainNotFound) {
-  MultiprocessTestHelper helper;
-  helper.StartChild("NoSuchTestChildMain");
-  int result = helper.WaitForChildShutdown();
-  EXPECT_FALSE(result >= 0 && result <= 127);
-}
-
-TEST_F(MultiprocessTestHelperTest, PassedChannel) {
-  MultiprocessTestHelper helper;
-  EXPECT_TRUE(helper.server_platform_handle.is_valid());
-  helper.StartChild("PassedChannel");
-
-  // Take ownership of the handle.
-  ScopedInternalPlatformHandle handle =
-      std::move(helper.server_platform_handle);
-
-  // The handle should be non-blocking.
-  EXPECT_TRUE(IsNonBlocking(handle.get()));
-
-  // Write a byte.
-  const char c = 'X';
-  EXPECT_TRUE(WriteByte(handle.get(), c));
-
-  // It'll echo it back to us, incremented.
-  char d = 0;
-  EXPECT_TRUE(ReadByte(handle.get(), &d));
-  EXPECT_EQ(c + 1, d);
-
-  // And return it, incremented again.
-  EXPECT_EQ(c + 2, helper.WaitForChildShutdown());
-}
-
-MOJO_MULTIPROCESS_TEST_CHILD_MAIN(PassedChannel) {
-  CHECK(MultiprocessTestHelper::client_platform_handle.is_valid());
-
-  // Take ownership of the handle.
-  ScopedInternalPlatformHandle handle =
-      std::move(MultiprocessTestHelper::client_platform_handle);
-
-  // The handle should be non-blocking.
-  EXPECT_TRUE(IsNonBlocking(handle.get()));
-
-  // Read a byte.
-  char c = 0;
-  EXPECT_TRUE(ReadByte(handle.get(), &c));
-
-  // Write it back, incremented.
-  c++;
-  EXPECT_TRUE(WriteByte(handle.get(), c));
-
-  // And return it, incremented again.
-  c++;
-  return static_cast<int>(c);
-}
-
-TEST_F(MultiprocessTestHelperTest, ChildTestPasses) {
-  MultiprocessTestHelper helper;
-  EXPECT_TRUE(helper.server_platform_handle.is_valid());
-  helper.StartChild("ChildTestPasses");
-  EXPECT_TRUE(helper.WaitForChildTestShutdown());
-}
-
-MOJO_MULTIPROCESS_TEST_CHILD_TEST(ChildTestPasses) {
-  ASSERT_TRUE(MultiprocessTestHelper::client_platform_handle.is_valid());
-  EXPECT_TRUE(
-      IsNonBlocking(MultiprocessTestHelper::client_platform_handle.get()));
-}
-
-TEST_F(MultiprocessTestHelperTest, ChildTestFailsAssert) {
-  MultiprocessTestHelper helper;
-  EXPECT_TRUE(helper.server_platform_handle.is_valid());
-  helper.StartChild("ChildTestFailsAssert");
-  EXPECT_FALSE(helper.WaitForChildTestShutdown());
-}
-
-MOJO_MULTIPROCESS_TEST_CHILD_TEST(ChildTestFailsAssert) {
-  ASSERT_FALSE(MultiprocessTestHelper::client_platform_handle.is_valid())
-      << "DISREGARD: Expected failure in child process";
-  ASSERT_FALSE(
-      IsNonBlocking(MultiprocessTestHelper::client_platform_handle.get()))
-      << "Not reached";
-  CHECK(false) << "Not reached";
-}
-
-TEST_F(MultiprocessTestHelperTest, ChildTestFailsExpect) {
-  MultiprocessTestHelper helper;
-  EXPECT_TRUE(helper.server_platform_handle.is_valid());
-  helper.StartChild("ChildTestFailsExpect");
-  EXPECT_FALSE(helper.WaitForChildTestShutdown());
-}
-
-MOJO_MULTIPROCESS_TEST_CHILD_TEST(ChildTestFailsExpect) {
-  EXPECT_FALSE(MultiprocessTestHelper::client_platform_handle.is_valid())
-      << "DISREGARD: Expected failure #1 in child process";
-  EXPECT_FALSE(
-      IsNonBlocking(MultiprocessTestHelper::client_platform_handle.get()))
-      << "DISREGARD: Expected failure #2 in child process";
-}
-
-}  // namespace
-}  // namespace test
-}  // namespace edk
-}  // namespace mojo

mojo/edk/test/test_utils.cc

+// 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 "mojo/edk/test/test_utils.h"
+
+#include <fcntl.h>
+#include <stddef.h>
+#include <unistd.h>
+
+#include "base/posix/eintr_wrapper.h"
+
+namespace mojo {
+namespace edk {
+namespace test {
+
+PlatformHandle PlatformHandleFromFILE(base::ScopedFILE fp) {
+  CHECK(fp);
+  int rv = HANDLE_EINTR(dup(fileno(fp.get())));
+  PCHECK(rv != -1) << "dup";
+  return PlatformHandle(base::ScopedFD(rv));
+}
+
+base::ScopedFILE FILEFromPlatformHandle(PlatformHandle h, const char* mode) {
+  CHECK(h.is_valid());
+  base::ScopedFILE rv(fdopen(h.ReleaseFD(), mode));
+  PCHECK(rv) << "fdopen";
+  return rv;
+}
+
+}  // namespace test
+}  // namespace edk
+}  // namespace mojo

mojo/edk/test/test_utils.h

@@ -11,43 +11,18 @@
 #include <string>
 
 #include "base/files/scoped_file.h"
-#include "mojo/edk/embedder/platform_handle.h"
 #include "mojo/edk/embedder/scoped_platform_handle.h"
+#include "mojo/public/cpp/platform/platform_handle.h"
 
 namespace mojo {
 namespace edk {
 namespace test {
 
-// On success, |bytes_written| is updated to the number of bytes written;
-// otherwise it is untouched.
-bool BlockingWrite(const InternalPlatformHandle& handle,
-                   const void* buffer,
-                   size_t bytes_to_write,
-                   size_t* bytes_written);
-
-// On success, |bytes_read| is updated to the number of bytes read; otherwise it
-// is untouched.
-bool BlockingRead(const InternalPlatformHandle& handle,
-                  void* buffer,
-                  size_t buffer_size,
-                  size_t* bytes_read);
-
-// If the read is done successfully or would block, the function returns true
-// and updates |bytes_read| to the number of bytes read (0 if the read would
-// block); otherwise it returns false and leaves |bytes_read| untouched.
-// |handle| must already be in non-blocking mode.
-bool NonBlockingRead(const InternalPlatformHandle& handle,
-                     void* buffer,
-                     size_t buffer_size,
-                     size_t* bytes_read);
-
-// Gets a (scoped) |InternalPlatformHandle| from the given (scoped) |FILE|.
-ScopedInternalPlatformHandle InternalPlatformHandleFromFILE(
-    base::ScopedFILE fp);
+// Gets a (scoped) |PlatformHandle| from the given (scoped) |FILE|.
+PlatformHandle PlatformHandleFromFILE(base::ScopedFILE fp);
 
 // Gets a (scoped) |FILE| from a (scoped) |InternalPlatformHandle|.
-base::ScopedFILE FILEFromInternalPlatformHandle(ScopedInternalPlatformHandle h,
-                                                const char* mode);
+base::ScopedFILE FILEFromPlatformHandle(PlatformHandle h, const char* mode);
 
 }  // namespace test
 }  // namespace edk

mojo/edk/test/test_utils_fuchsia.cc

-// Copyright 2017 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/edk/test/test_utils.h"
-
-#include <fcntl.h>
-#include <stddef.h>
-#include <unistd.h>
-
-#include "base/files/scoped_file.h"
-#include "base/logging.h"
-#include "base/posix/eintr_wrapper.h"
-#include "mojo/edk/embedder/scoped_platform_handle.h"
-
-namespace mojo {
-namespace edk {
-namespace test {
-
-// TODO(fuchsia): Merge Fuchsia's InternalPlatformHandle with the POSIX one and
-// use the POSIX-generic versions of these. See crbug.com/754029.
-
-bool BlockingWrite(const InternalPlatformHandle& handle,
-                   const void* buffer,
-                   size_t bytes_to_write,
-                   size_t* bytes_written) {
-  int original_flags = fcntl(handle.as_fd(), F_GETFL);
-  if (original_flags == -1 ||
-      fcntl(handle.as_fd(), F_SETFL, original_flags & (~O_NONBLOCK)) != 0) {
-    return false;
-  }
-
-  ssize_t result = HANDLE_EINTR(write(handle.as_fd(), buffer, bytes_to_write));
-
-  fcntl(handle.as_fd(), F_SETFL, original_flags);
-
-  if (result < 0)
-    return false;
-
-  *bytes_written = result;
-  return true;
-}
-
-bool BlockingRead(const InternalPlatformHandle& handle,
-                  void* buffer,
-                  size_t buffer_size,
-                  size_t* bytes_read) {
-  int original_flags = fcntl(handle.as_fd(), F_GETFL);
-  if (original_flags == -1 ||
-      fcntl(handle.as_fd(), F_SETFL, original_flags & (~O_NONBLOCK)) != 0) {
-    return false;
-  }
-
-  ssize_t result = HANDLE_EINTR(read(handle.as_fd(), buffer, buffer_size));
-
-  fcntl(handle.as_fd(), F_SETFL, original_flags);
-
-  if (result < 0)
-    return false;
-
-  *bytes_read = result;
-  return true;
-}
-
-bool NonBlockingRead(const InternalPlatformHandle& handle,
-                     void* buffer,
-                     size_t buffer_size,
-                     size_t* bytes_read) {
-  ssize_t result = HANDLE_EINTR(read(handle.as_fd(), buffer, buffer_size));
-
-  if (result < 0) {
-    if (errno != EAGAIN && errno != EWOULDBLOCK)
-      return false;
-
-    *bytes_read = 0;
-  } else {
-    *bytes_read = result;
-  }
-
-  return true;
-}
-
-ScopedInternalPlatformHandle InternalPlatformHandleFromFILE(
-    base::ScopedFILE fp) {
-  CHECK(fp);
-  int rv = HANDLE_EINTR(dup(fileno(fp.get())));
-  PCHECK(rv != -1) << "dup";
-  return ScopedInternalPlatformHandle(InternalPlatformHandle::ForFd(rv));
-}
-
-base::ScopedFILE FILEFromInternalPlatformHandle(ScopedInternalPlatformHandle h,
-                                                const char* mode) {
-  CHECK(h.get().is_valid_fd());
-  base::ScopedFILE rv(fdopen(h.release().as_fd(), mode));
-  PCHECK(rv) << "fdopen";
-  return rv;
-}
-
-}  // namespace test
-}  // namespace edk
-}  // namespace mojo

mojo/edk/test/test_utils_posix.cc

-// 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 "mojo/edk/test/test_utils.h"
-
-#include <fcntl.h>
-#include <stddef.h>
-#include <unistd.h>
-
-#include "base/posix/eintr_wrapper.h"
-
-namespace mojo {
-namespace edk {
-namespace test {
-
-bool BlockingWrite(const InternalPlatformHandle& handle,
-                   const void* buffer,
-                   size_t bytes_to_write,
-                   size_t* bytes_written) {
-  int original_flags = fcntl(handle.handle, F_GETFL);
-  if (original_flags == -1 ||
-      fcntl(handle.handle, F_SETFL, original_flags & (~O_NONBLOCK)) != 0) {
-    return false;
-  }
-
-  ssize_t result = HANDLE_EINTR(write(handle.handle, buffer, bytes_to_write));
-
-  fcntl(handle.handle, F_SETFL, original_flags);
-
-  if (result < 0)
-    return false;
-
-  *bytes_written = result;
-  return true;
-}
-
-bool BlockingRead(const InternalPlatformHandle& handle,
-                  void* buffer,
-                  size_t buffer_size,
-                  size_t* bytes_read) {
-  int original_flags = fcntl(handle.handle, F_GETFL);
-  if (original_flags == -1 ||
-      fcntl(handle.handle, F_SETFL, original_flags & (~O_NONBLOCK)) != 0) {
-    return false;
-  }
-
-  ssize_t result = HANDLE_EINTR(read(handle.handle, buffer, buffer_size));
-
-  fcntl(handle.handle, F_SETFL, original_flags);
-
-  if (result < 0)
-    return false;
-
-  *bytes_read = result;
-  return true;
-}
-
-bool NonBlockingRead(const InternalPlatformHandle& handle,
-                     void* buffer,
-                     size_t buffer_size,
-                     size_t* bytes_read) {
-  ssize_t result = HANDLE_EINTR(read(handle.handle, buffer, buffer_size));
-
-  if (result < 0) {
-    if (errno != EAGAIN && errno != EWOULDBLOCK)
-      return false;
-
-    *bytes_read = 0;
-  } else {
-    *bytes_read = result;
-  }
-
-  return true;
-}
-
-ScopedInternalPlatformHandle InternalPlatformHandleFromFILE(
-    base::ScopedFILE fp) {
-  CHECK(fp);
-  int rv = HANDLE_EINTR(dup(fileno(fp.get())));
-  PCHECK(rv != -1) << "dup";
-  return ScopedInternalPlatformHandle(InternalPlatformHandle(rv));
-}
-
-base::ScopedFILE FILEFromInternalPlatformHandle(ScopedInternalPlatformHandle h,
-                                                const char* mode) {
-  CHECK(h.is_valid());
-  base::ScopedFILE rv(fdopen(h.release().handle, mode));
-  PCHECK(rv) << "fdopen";
-  return rv;
-}
-
-}  // namespace test
-}  // namespace edk
-}  // namespace mojo

mojo/edk/test/test_utils_win.cc

@@ -14,73 +14,7 @@ namespace mojo {
 namespace edk {
 namespace test {
 
-bool BlockingWrite(const InternalPlatformHandle& handle,
-                   const void* buffer,
-                   size_t bytes_to_write,
-                   size_t* bytes_written) {
-  OVERLAPPED overlapped = {0};
-  DWORD bytes_written_dword = 0;
-
-  if (!WriteFile(handle.handle, buffer, static_cast<DWORD>(bytes_to_write),
-                 &bytes_written_dword, &overlapped)) {
-    if (GetLastError() != ERROR_IO_PENDING ||
-        !GetOverlappedResult(handle.handle, &overlapped, &bytes_written_dword,
-                             TRUE)) {
-      return false;
-    }
-  }
-
-  *bytes_written = bytes_written_dword;
-  return true;
-}
-
-bool BlockingRead(const InternalPlatformHandle& handle,
-                  void* buffer,
-                  size_t buffer_size,
-                  size_t* bytes_read) {
-  OVERLAPPED overlapped = {0};
-  DWORD bytes_read_dword = 0;
-
-  if (!ReadFile(handle.handle, buffer, static_cast<DWORD>(buffer_size),
-                &bytes_read_dword, &overlapped)) {
-    if (GetLastError() != ERROR_IO_PENDING ||
-        !GetOverlappedResult(handle.handle, &overlapped, &bytes_read_dword,
-                             TRUE)) {
-      return false;
-    }
-  }
-
-  *bytes_read = bytes_read_dword;
-  return true;
-}
-
-bool NonBlockingRead(const InternalPlatformHandle& handle,
-                     void* buffer,
-                     size_t buffer_size,
-                     size_t* bytes_read) {
-  OVERLAPPED overlapped = {0};
-  DWORD bytes_read_dword = 0;
-
-  if (!ReadFile(handle.handle, buffer, static_cast<DWORD>(buffer_size),
-                &bytes_read_dword, &overlapped)) {
-    if (GetLastError() != ERROR_IO_PENDING)
-      return false;
-
-    CancelIo(handle.handle);
-
-    if (!GetOverlappedResult(handle.handle, &overlapped, &bytes_read_dword,
-                             TRUE)) {
-      *bytes_read = 0;
-      return true;
-    }
-  }
-
-  *bytes_read = bytes_read_dword;
-  return true;
-}
-
-ScopedInternalPlatformHandle InternalPlatformHandleFromFILE(
-    base::ScopedFILE fp) {
+PlatformHandle PlatformHandleFromFILE(base::ScopedFILE fp) {
   CHECK(fp);
 
   HANDLE rv = INVALID_HANDLE_VALUE;
@@ -89,11 +23,10 @@ ScopedInternalPlatformHandle InternalPlatformHandleFromFILE(
       reinterpret_cast<HANDLE>(_get_osfhandle(_fileno(fp.get()))),
       GetCurrentProcess(), &rv, 0, TRUE, DUPLICATE_SAME_ACCESS))
       << "DuplicateHandle";
-  return ScopedInternalPlatformHandle(InternalPlatformHandle(rv));
+  return PlatformHandle(base::win::ScopedHandle(rv));
 }
 
-base::ScopedFILE FILEFromInternalPlatformHandle(ScopedInternalPlatformHandle h,
-                                                const char* mode) {
+base::ScopedFILE FILEFromPlatformHandle(PlatformHandle h, const char* mode) {
   CHECK(h.is_valid());
   // Microsoft's documentation for |_open_osfhandle()| only discusses these
   // flags (and |_O_WTEXT|). Hmmm.
@@ -105,7 +38,7 @@ base::ScopedFILE FILEFromInternalPlatformHandle(ScopedInternalPlatformHandle h,
   if (strchr(mode, 't'))
     flags |= _O_TEXT;
   base::ScopedFILE rv(_fdopen(
-      _open_osfhandle(reinterpret_cast<intptr_t>(h.release().handle), flags),
+      _open_osfhandle(reinterpret_cast<intptr_t>(h.ReleaseHandle()), flags),
       mode));
   PCHECK(rv) << "_fdopen";
   return rv;

mojo/public/cpp/system/BUILD.gn

@@ -40,6 +40,8 @@ component("system") {
     "handle_signals_state.h",
     "invitation.cc",
     "invitation.h",
+    "isolated_connection.cc",
+    "isolated_connection.h",
     "message.h",
     "message_pipe.cc",
     "message_pipe.h",

mojo/public/cpp/system/isolated_connection.cc

+// Copyright 2018 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/public/cpp/system/isolated_connection.h"
+
+#include "mojo/public/cpp/platform/platform_channel.h"
+#include "mojo/public/cpp/system/invitation.h"
+
+namespace mojo {
+
+IsolatedConnection::IsolatedConnection()
+    : token_(base::UnguessableToken::Create()) {}
+
+IsolatedConnection::~IsolatedConnection() {
+  // We send a dummy invitation over a temporary channel, re-using |token_| as
+  // the name. This ensures that the connection set up by Connect(), if any,
+  // will be replaced with a short-lived, self-terminating connection.
+  //
+  // This is a bit of a hack since Mojo does not provide any API for explicitly
+  // terminating isolated connections, but this is a decision made to minimize
+  // the API surface dedicated to isolated connections in anticipation of the
+  // concept being deprecated eventually.
+  PlatformChannel channel;
+  OutgoingInvitation::SendIsolated(channel.TakeLocalEndpoint(),
+                                   token_.ToString());
+}
+
+ScopedMessagePipeHandle IsolatedConnection::Connect(
+    PlatformChannelEndpoint endpoint) {
+  return OutgoingInvitation::SendIsolated(std::move(endpoint),
+                                          token_.ToString());
+}
+
+ScopedMessagePipeHandle IsolatedConnection::Connect(
+    PlatformChannelServerEndpoint endpoint) {
+  return OutgoingInvitation::SendIsolated(std::move(endpoint),
+                                          token_.ToString());
+}
+
+}  // namespace mojo

mojo/public/cpp/system/isolated_connection.h

+// Copyright 2018 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.
+
+#ifndef MOJO_PUBLIC_CPP_SYSTEM_ISOLATED_CONNECTION_H_
+#define MOJO_PUBLIC_CPP_SYSTEM_ISOLATED_CONNECTION_H_
+
+#include "base/macros.h"
+#include "base/unguessable_token.h"
+#include "mojo/public/cpp/platform/platform_channel_endpoint.h"
+#include "mojo/public/cpp/platform/platform_channel_server_endpoint.h"
+#include "mojo/public/cpp/system/message_pipe.h"
+#include "mojo/public/cpp/system/system_export.h"
+
+namespace mojo {
+
+// IsolatedConnection establishes a one-off Mojo IPC connection between two
+// processes. Unlike more common connections established by invitation
+// (see OutgoingInvitation and IncomingInvitation), isolated connections
+// do not result in the two processes becoming part of the same connected
+// graph of processes. As such, any message pipe established over this
+// connection can only be used for direct IPC between the two processes in
+// question.
+//
+// This means that if one of the processes sends a Mojo handle (e.g. another
+// message pipe endpoint) to the other process, the receiving process cannot
+// pass that handle to yet another process in its own graph. This limitation is
+// subtle and can be difficult to work around, so use of IsolatedConnection
+// should be rare.
+//
+// This is primarily useful when you already have two established Mojo process
+// graphs isolated form each other, and you want to do some IPC between two
+// processes, one in each graph.
+//
+// A connection established via |Connect()|, and any opened message pipes
+// spanning that connection, will remain valid and connected as long as this
+// object remains alive.
+class MOJO_CPP_SYSTEM_EXPORT IsolatedConnection {
+ public:
+  IsolatedConnection();
+  ~IsolatedConnection();
+
+  // Connects to a process at the other end of the channel. Returns a primordial
+  // message pipe that can be used for Mojo IPC. The connection
+  // will be connected to a corresponding peer pipe in the remote process.
+  ScopedMessagePipeHandle Connect(PlatformChannelEndpoint endpoint);
+
+  // Same as above but works with a server endpoint. The corresponding client
+  // could use the above signature with NamedPlatformChannel::ConnectToServer.
+  ScopedMessagePipeHandle Connect(PlatformChannelServerEndpoint endpoint);
+
+ private:
+  const base::UnguessableToken token_;
+
+  DISALLOW_COPY_AND_ASSIGN(IsolatedConnection);
+};
+
+}  // namespace mojo
+
+#endif  // MOJO_PUBLIC_CPP_SYSTEM_ISOLATED_CONNECTION_H_