ipc: Implement attachment brokering for SharedMemoryHandle on Windows.

Each SharedMemoryHandle is backed by a HANDLE, and that HANDLE is associated
with a specific process. If a SharedMemoryHandle passed to IPC is associated
with the current process, the IPC stack will automatically broker the handle to
the destination process.

This functionality has been implemented and tested, but is not yet turned on,
because there are a couple of Windows-specific Chrome IPC messages that
intentionally pass a HANDLE associated with another process. I will write a
follow-up CL that turns on this functionality, and removes those IPC messages.

BUG=493414

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

Cr-Commit-Position: refs/heads/master@{#368244}

11 files changed, 138 insertions, 115 deletions

diff --git a/ipc/BUILD.gn b/ipc/BUILD.gn
index d05cc79..1dddc13 100644
--- a/ipc/BUILD.gn
+++ b/ipc/BUILD.gn
@@ -156,7 +156,6 @@ test("ipc_tests") {
     "attachment_broker_mac_unittest.cc",
     "attachment_broker_privileged_mac_unittest.cc",
     "attachment_broker_privileged_win_unittest.cc",
-    "attachment_broker_unprivileged_win_unittest.cc",
     "ipc_channel_posix_unittest.cc",
     "ipc_channel_proxy_unittest.cc",
     "ipc_channel_reader_unittest.cc",
diff --git a/ipc/attachment_broker_privileged_win.cc b/ipc/attachment_broker_privileged_win.cc
index 38b2e65..c749a09 100644
--- a/ipc/attachment_broker_privileged_win.cc
+++ b/ipc/attachment_broker_privileged_win.cc
@@ -23,12 +23,13 @@ bool AttachmentBrokerPrivilegedWin::SendAttachmentToProcess(
     base::ProcessId destination_process) {
   switch (attachment->GetBrokerableType()) {
     case BrokerableAttachment::WIN_HANDLE: {
-      const internal::HandleAttachmentWin* handle_attachment =
-          static_cast<const internal::HandleAttachmentWin*>(attachment.get());
+      internal::HandleAttachmentWin* handle_attachment =
+          static_cast<internal::HandleAttachmentWin*>(attachment.get());
       HandleWireFormat wire_format =
           handle_attachment->GetWireFormat(destination_process);
       HandleWireFormat new_wire_format =
           DuplicateWinHandle(wire_format, base::Process::Current().Pid());
+      handle_attachment->reset_handle_ownership();
       if (new_wire_format.handle == 0)
         return false;
       RouteDuplicatedHandle(new_wire_format);
@@ -107,6 +108,10 @@ AttachmentBrokerPrivilegedWin::DuplicateWinHandle(
   if (source_pid == wire_format.destination_process)
     return wire_format;
 
+  // If the handle is not valid, no additional work is required.
+  if (wire_format.handle == 0)
+    return wire_format;
+
   base::Process source_process =
       base::Process::OpenWithExtraPrivileges(source_pid);
   base::Process dest_process =
diff --git a/ipc/attachment_broker_privileged_win.h b/ipc/attachment_broker_privileged_win.h
index 70a4802..467a977 100644
--- a/ipc/attachment_broker_privileged_win.h
+++ b/ipc/attachment_broker_privileged_win.h
@@ -34,7 +34,7 @@ class IPC_EXPORT AttachmentBrokerPrivilegedWin
   void OnDuplicateWinHandle(const Message& message);
 
   // Duplicates |wire_Format| from |source_process| into its destination
-  // process.
+  // process. Closes the original HANDLE.
   HandleWireFormat DuplicateWinHandle(const HandleWireFormat& wire_format,
                                       base::ProcessId source_process);
 
diff --git a/ipc/attachment_broker_privileged_win_unittest.cc b/ipc/attachment_broker_privileged_win_unittest.cc
index 583050c..7a2be3b 100644
--- a/ipc/attachment_broker_privileged_win_unittest.cc
+++ b/ipc/attachment_broker_privileged_win_unittest.cc
@@ -10,6 +10,9 @@
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/shared_memory.h"
+#include "base/memory/shared_memory_handle.h"
+#include "base/win/scoped_handle.h"
 #include "ipc/attachment_broker_privileged_win.h"
 #include "ipc/attachment_broker_unprivileged_win.h"
 #include "ipc/handle_attachment_win.h"
@@ -21,7 +24,10 @@
 
 namespace {
 
+using base::win::ScopedHandle;
+
 const char kDataBuffer[] = "This is some test data to write to the file.";
+const size_t kSharedMemorySize = 20000;
 
 // Returns the contents of the file represented by |h| as a std::string.
 std::string ReadFromFile(HANDLE h) {
@@ -33,80 +39,107 @@ std::string ReadFromFile(HANDLE h) {
   return success ? std::string(buffer, bytes_read) : std::string();
 }
 
-HANDLE GetHandleFromBrokeredAttachment(
+ScopedHandle GetHandleFromBrokeredAttachment(
     const scoped_refptr<IPC::BrokerableAttachment>& attachment) {
   if (attachment->GetType() !=
       IPC::BrokerableAttachment::TYPE_BROKERABLE_ATTACHMENT) {
     LOG(INFO) << "Attachment type not TYPE_BROKERABLE_ATTACHMENT.";
-    return nullptr;
+    return ScopedHandle(nullptr);
   }
 
   if (attachment->GetBrokerableType() !=
       IPC::BrokerableAttachment::WIN_HANDLE) {
     LOG(INFO) << "Brokerable type not WIN_HANDLE.";
-    return nullptr;
+    return ScopedHandle(nullptr);
   }
 
   IPC::internal::HandleAttachmentWin* received_handle_attachment =
       static_cast<IPC::internal::HandleAttachmentWin*>(attachment.get());
-  return received_handle_attachment->get_handle();
+  ScopedHandle h(received_handle_attachment->get_handle());
+  received_handle_attachment->reset_handle_ownership();
+  return h;
 }
 
 // |message| must be deserializable as a TestHandleWinMsg. Returns the HANDLE,
 // or nullptr if deserialization failed.
-HANDLE GetHandleFromTestHandleWinMsg(const IPC::Message& message) {
+ScopedHandle GetHandleFromTestHandleWinMsg(const IPC::Message& message) {
   // Expect a message with a brokered attachment.
   if (!message.HasBrokerableAttachments()) {
     LOG(INFO) << "Message missing brokerable attachment.";
-    return nullptr;
+    return ScopedHandle(nullptr);
   }
 
   TestHandleWinMsg::Schema::Param p;
   bool success = TestHandleWinMsg::Read(&message, &p);
   if (!success) {
     LOG(INFO) << "Failed to deserialize message.";
-    return nullptr;
+    return ScopedHandle(nullptr);
   }
 
   IPC::HandleWin handle_win = base::get<1>(p);
-  return handle_win.get_handle();
+  return ScopedHandle(handle_win.get_handle());
+}
+
+// Returns a mapped, shared memory region based on the handle in |message|.
+scoped_ptr<base::SharedMemory> GetSharedMemoryFromSharedMemoryHandleMsg1(
+    const IPC::Message& message,
+    size_t size) {
+  // Expect a message with a brokered attachment.
+  if (!message.HasBrokerableAttachments()) {
+    LOG(INFO) << "Message missing brokerable attachment.";
+    return nullptr;
+  }
+
+  TestSharedMemoryHandleMsg1::Schema::Param p;
+  bool success = TestSharedMemoryHandleMsg1::Read(&message, &p);
+  if (!success) {
+    LOG(INFO) << "Failed to deserialize message.";
+    return nullptr;
+  }
+
+  base::SharedMemoryHandle handle = base::get<0>(p);
+  scoped_ptr<base::SharedMemory> shared_memory(
+      new base::SharedMemory(handle, false));
+
+  shared_memory->Map(size);
+  return std::move(shared_memory);
 }
 
 // |message| must be deserializable as a TestTwoHandleWinMsg. Returns the
 // HANDLE, or nullptr if deserialization failed.
-HANDLE GetHandleFromTestTwoHandleWinMsg(const IPC::Message& message,
-                                        int index) {
+bool GetHandleFromTestTwoHandleWinMsg(const IPC::Message& message,
+                                      HANDLE* h1,
+                                      HANDLE* h2) {
   // Expect a message with a brokered attachment.
   if (!message.HasBrokerableAttachments()) {
     LOG(INFO) << "Message missing brokerable attachment.";
-    return nullptr;
+    return false;
   }
 
   TestTwoHandleWinMsg::Schema::Param p;
   bool success = TestTwoHandleWinMsg::Read(&message, &p);
   if (!success) {
     LOG(INFO) << "Failed to deserialize message.";
-    return nullptr;
+    return false;
   }
 
-  IPC::HandleWin handle_win;
-  if (index == 0)
-    handle_win = base::get<0>(p);
-  else if (index == 1)
-    handle_win = base::get<1>(p);
-  return handle_win.get_handle();
+  IPC::HandleWin handle_win = base::get<0>(p);
+  *h1 = handle_win.get_handle();
+  handle_win = base::get<1>(p);
+  *h2 = handle_win.get_handle();
+  return true;
 }
 
 // |message| must be deserializable as a TestHandleWinMsg. Returns true if the
 // attached file HANDLE has contents |kDataBuffer|.
 bool CheckContentsOfTestMessage(const IPC::Message& message) {
-  HANDLE h = GetHandleFromTestHandleWinMsg(message);
-  if (h == nullptr) {
+  ScopedHandle h(GetHandleFromTestHandleWinMsg(message));
+  if (h.Get() == nullptr) {
     LOG(INFO) << "Failed to get handle from TestHandleWinMsg.";
     return false;
   }
 
-  std::string contents = ReadFromFile(h);
+  std::string contents = ReadFromFile(h.Get());
   bool success = (contents == std::string(kDataBuffer));
   if (!success) {
     LOG(INFO) << "Expected contents: " << std::string(kDataBuffer);
@@ -371,12 +404,12 @@ TEST_F(IPCAttachmentBrokerPrivilegedWinTest, SendHandleToSelf) {
   get_broker()->GetAttachmentWithId(*id, &received_attachment);
   ASSERT_NE(received_attachment.get(), nullptr);
 
-  // Check that it's the same entry in the HANDLE table.
-  HANDLE h2 = GetHandleFromBrokeredAttachment(received_attachment);
-  EXPECT_EQ(h2, h);
+  // Check that it's a different entry in the HANDLE table.
+  ScopedHandle h2(GetHandleFromBrokeredAttachment(received_attachment));
+  EXPECT_NE(h2.Get(), h);
 
-  // And still points to the same file.
-  std::string contents = ReadFromFile(h);
+  // But still points to the same file.
+  std::string contents = ReadFromFile(h2.Get());
   EXPECT_EQ(contents, std::string(kDataBuffer));
 
   CommonTearDown();
@@ -435,6 +468,29 @@ TEST_F(IPCAttachmentBrokerPrivilegedWinTest, SendHandleTwice) {
   CommonTearDown();
 }
 
+// An unprivileged process makes a shared memory region and sends it to the
+// privileged process.
+TEST_F(IPCAttachmentBrokerPrivilegedWinTest, DISABLED_SendSharedMemoryHandle) {
+  Init("SendSharedMemoryHandle");
+
+  CommonSetUp();
+  ResultListener result_listener;
+  get_proxy_listener()->set_listener(&result_listener);
+
+  scoped_ptr<base::SharedMemory> shared_memory(new base::SharedMemory);
+  shared_memory->CreateAndMapAnonymous(kSharedMemorySize);
+  memcpy(shared_memory->memory(), kDataBuffer, strlen(kDataBuffer));
+  sender()->Send(new TestSharedMemoryHandleMsg1(shared_memory->handle()));
+  base::MessageLoop::current()->Run();
+
+  // Check the result.
+  ASSERT_EQ(ProxyListener::MESSAGE_RECEIVED,
+            get_proxy_listener()->get_reason());
+  ASSERT_EQ(result_listener.get_result(), RESULT_SUCCESS);
+
+  CommonTearDown();
+}
+
 using OnMessageReceivedCallback = void (*)(IPC::Sender* sender,
                                            const IPC::Message& message);
 
@@ -481,14 +537,14 @@ MULTIPROCESS_IPC_TEST_CLIENT_MAIN(SendHandle) {
 
 void SendHandleWithoutPermissionsCallback(IPC::Sender* sender,
                                           const IPC::Message& message) {
-  HANDLE h = GetHandleFromTestHandleWinMsg(message);
-  if (h != nullptr) {
-    SetFilePointer(h, 0, nullptr, FILE_BEGIN);
+  ScopedHandle h(GetHandleFromTestHandleWinMsg(message));
+  if (h.Get() != nullptr) {
+    SetFilePointer(h.Get(), 0, nullptr, FILE_BEGIN);
 
     char buffer[100];
     DWORD bytes_read;
     BOOL success =
-        ::ReadFile(h, buffer, static_cast<DWORD>(strlen(kDataBuffer)),
+        ::ReadFile(h.Get(), buffer, static_cast<DWORD>(strlen(kDataBuffer)),
                    &bytes_read, nullptr);
     if (!success && GetLastError() == ERROR_ACCESS_DENIED) {
       SendControlMessage(sender, true);
@@ -516,8 +572,8 @@ MULTIPROCESS_IPC_TEST_CLIENT_MAIN(SendHandleToSelf) {
 
 void SendTwoHandlesCallback(IPC::Sender* sender, const IPC::Message& message) {
   // Check for two handles.
-  HANDLE h1 = GetHandleFromTestTwoHandleWinMsg(message, 0);
-  HANDLE h2 = GetHandleFromTestTwoHandleWinMsg(message, 1);
+  HANDLE h1, h2;
+  EXPECT_TRUE(GetHandleFromTestTwoHandleWinMsg(message, &h1, &h2));
   if (h1 == nullptr || h2 == nullptr) {
     SendControlMessage(sender, false);
     return;
@@ -579,4 +635,18 @@ MULTIPROCESS_IPC_TEST_CLIENT_MAIN(SendHandleTwice) {
                                      "SendHandleTwice");
 }
 
+void SendSharedMemoryHandleCallback(IPC::Sender* sender,
+                                    const IPC::Message& message) {
+  scoped_ptr<base::SharedMemory> shared_memory =
+      GetSharedMemoryFromSharedMemoryHandleMsg1(message, kSharedMemorySize);
+  bool success =
+      memcmp(shared_memory->memory(), kDataBuffer, strlen(kDataBuffer)) == 0;
+  SendControlMessage(sender, success);
+}
+
+MULTIPROCESS_IPC_TEST_CLIENT_MAIN(SendSharedMemoryHandle) {
+  return CommonPrivilegedProcessMain(&SendSharedMemoryHandleCallback,
+                                     "SendSharedMemoryHandle");
+}
+
 }  // namespace
diff --git a/ipc/attachment_broker_unprivileged_win_unittest.cc b/ipc/attachment_broker_unprivileged_win_unittest.cc
deleted file mode 100644
index e67c54a..0000000
--- a/ipc/attachment_broker_unprivileged_win_unittest.cc
+++ /dev/null
@@ -1,51 +0,0 @@
-// Copyright 2015 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 "build/build_config.h"
-
-#include <windows.h>
-
-#include "base/memory/ref_counted.h"
-#include "base/memory/scoped_ptr.h"
-#include "base/process/process.h"
-#include "ipc/attachment_broker_messages.h"
-#include "ipc/attachment_broker_unprivileged_win.h"
-#include "ipc/handle_attachment_win.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace IPC {
-
-TEST(AttachmentBrokerUnprivilegedWinTest, ReceiveValidMessage) {
-  HANDLE handle = LongToHandle(8);
-  base::ProcessId destination = base::Process::Current().Pid();
-  scoped_refptr<internal::HandleAttachmentWin> attachment(
-      new internal::HandleAttachmentWin(handle, HandleWin::DUPLICATE));
-  AttachmentBrokerMsg_WinHandleHasBeenDuplicated msg(
-      attachment->GetWireFormat(destination));
-  AttachmentBrokerUnprivilegedWin attachment_broker;
-  EXPECT_TRUE(attachment_broker.OnMessageReceived(msg));
-  EXPECT_EQ(1u, attachment_broker.attachments_.size());
-
-  scoped_refptr<BrokerableAttachment> received_attachment =
-      attachment_broker.attachments_.front();
-  EXPECT_EQ(BrokerableAttachment::WIN_HANDLE,
-            received_attachment->GetBrokerableType());
-  internal::HandleAttachmentWin* received_handle_attachment =
-      static_cast<internal::HandleAttachmentWin*>(received_attachment.get());
-  EXPECT_EQ(handle, received_handle_attachment->get_handle());
-}
-
-TEST(AttachmentBrokerUnprivilegedWinTest, ReceiveInvalidMessage) {
-  HANDLE handle = LongToHandle(8);
-  base::ProcessId destination = base::Process::Current().Pid() + 1;
-  scoped_refptr<internal::HandleAttachmentWin> attachment(
-      new internal::HandleAttachmentWin(handle, HandleWin::DUPLICATE));
-  AttachmentBrokerMsg_WinHandleHasBeenDuplicated msg(
-      attachment->GetWireFormat(destination));
-  AttachmentBrokerUnprivilegedWin attachment_broker;
-  EXPECT_TRUE(attachment_broker.OnMessageReceived(msg));
-  EXPECT_EQ(0u, attachment_broker.attachments_.size());
-}
-
-}  // namespace IPC
diff --git a/ipc/handle_attachment_win.cc b/ipc/handle_attachment_win.cc
index 5afd7fa..39c4ef7 100644
--- a/ipc/handle_attachment_win.cc
+++ b/ipc/handle_attachment_win.cc
@@ -11,18 +11,24 @@ namespace internal {
 
 HandleAttachmentWin::HandleAttachmentWin(const HANDLE& handle,
                                          HandleWin::Permissions permissions)
-    : handle_(handle), permissions_(permissions), owns_handle_(true) {}
+    : handle_(INVALID_HANDLE_VALUE),
+      permissions_(HandleWin::INVALID),
+      owns_handle_(true) {
+  HANDLE duplicated_handle;
+  BOOL result =
+      ::DuplicateHandle(GetCurrentProcess(), handle, GetCurrentProcess(),
+                        &duplicated_handle, 0, FALSE, DUPLICATE_SAME_ACCESS);
+  if (result) {
+    handle_ = duplicated_handle;
+    permissions_ = permissions;
+  }
+}
 
 HandleAttachmentWin::HandleAttachmentWin(const WireFormat& wire_format)
     : BrokerableAttachment(wire_format.attachment_id),
       handle_(LongToHandle(wire_format.handle)),
-      permissions_(wire_format.permissions), owns_handle_(false) {}
-
-HandleAttachmentWin::HandleAttachmentWin(
-    const BrokerableAttachment::AttachmentId& id)
-    : BrokerableAttachment(id),
-      handle_(INVALID_HANDLE_VALUE),
-      permissions_(HandleWin::INVALID), owns_handle_(false) {}
+      permissions_(wire_format.permissions),
+      owns_handle_(true) {}
 
 HandleAttachmentWin::~HandleAttachmentWin() {
   if (handle_ != INVALID_HANDLE_VALUE && owns_handle_)
diff --git a/ipc/handle_attachment_win.h b/ipc/handle_attachment_win.h
index 35807b4..5e04bdb 100644
--- a/ipc/handle_attachment_win.h
+++ b/ipc/handle_attachment_win.h
@@ -58,10 +58,9 @@ class IPC_EXPORT HandleAttachmentWin : public BrokerableAttachment {
   // result. Should only be called by the sender of a Chrome IPC message.
   HandleAttachmentWin(const HANDLE& handle, HandleWin::Permissions permissions);
 
-  // These constructors do not take ownership of the HANDLE, and should only be
-  // called by the receiver of a Chrome IPC message.
+  // This constructor takes ownership of |wire_format.handle| without making a
+  // copy. Should only be called by the receiver of a Chrome IPC message.
   explicit HandleAttachmentWin(const WireFormat& wire_format);
-  explicit HandleAttachmentWin(const BrokerableAttachment::AttachmentId& id);
 
   BrokerableType GetBrokerableType() const override;
 
@@ -71,7 +70,10 @@ class IPC_EXPORT HandleAttachmentWin : public BrokerableAttachment {
   HANDLE get_handle() const { return handle_; }
 
   // The caller of this method has taken ownership of |handle_|.
-  void reset_handle_ownership() { owns_handle_ = false; }
+  void reset_handle_ownership() {
+    owns_handle_ = false;
+    handle_ = INVALID_HANDLE_VALUE;
+  }
 
  private:
   ~HandleAttachmentWin() override;
@@ -81,9 +83,8 @@ class IPC_EXPORT HandleAttachmentWin : public BrokerableAttachment {
   // In the sender process, the attachment owns the HANDLE of a newly created
   // message. The attachment broker will eventually take ownership, and set
   // this member to |false|.
-  // In the destination process, the attachment never owns the Mach port. The
-  // client code that receives the Chrome IPC message is always expected to take
-  // ownership.
+  // In the destination process, the attachment owns the Mach port until a call
+  // to ParamTraits<HandleWin>::Read() takes ownership.
   bool owns_handle_;
 };
 
diff --git a/ipc/handle_win.cc b/ipc/handle_win.cc
index 45fbae4..f964c83 100644
--- a/ipc/handle_win.cc
+++ b/ipc/handle_win.cc
@@ -47,6 +47,7 @@ bool ParamTraits<HandleWin>::Read(const Message* m,
   IPC::internal::HandleAttachmentWin* handle_attachment =
       static_cast<IPC::internal::HandleAttachmentWin*>(brokerable_attachment);
   r->set_handle(handle_attachment->get_handle());
+  handle_attachment->reset_handle_ownership();
   return true;
 }
 
diff --git a/ipc/ipc.gyp b/ipc/ipc.gyp
index bc8659f..05b9989 100644
--- a/ipc/ipc.gyp
+++ b/ipc/ipc.gyp
@@ -55,7 +55,6 @@
         'attachment_broker_mac_unittest.cc',
         'attachment_broker_privileged_mac_unittest.cc',
         'attachment_broker_privileged_win_unittest.cc',
-        'attachment_broker_unprivileged_win_unittest.cc',
         'ipc_channel_posix_unittest.cc',
         'ipc_channel_proxy_unittest.cc',
         'ipc_channel_reader_unittest.cc',
diff --git a/ipc/ipc_message_utils.cc b/ipc/ipc_message_utils.cc
index 60ae58f..ea88b55 100644
--- a/ipc/ipc_message_utils.cc
+++ b/ipc/ipc_message_utils.cc
@@ -660,9 +660,6 @@ void ParamTraits<base::SharedMemoryHandle>::Log(const param_type& p,
 #elif defined(OS_WIN)
 void ParamTraits<base::SharedMemoryHandle>::Write(Message* m,
                                                   const param_type& p) {
-  // Longs on windows are 32 bits.
-  uint32_t pid = p.GetPID();
-  m->WriteUInt32(pid);
   m->WriteBool(p.NeedsBrokering());
 
   if (p.NeedsBrokering()) {
@@ -676,11 +673,6 @@ void ParamTraits<base::SharedMemoryHandle>::Write(Message* m,
 bool ParamTraits<base::SharedMemoryHandle>::Read(const Message* m,
                                                  base::PickleIterator* iter,
                                                  param_type* r) {
-  uint32_t pid_int;
-  if (!iter->ReadUInt32(&pid_int))
-    return false;
-  base::ProcessId pid = pid_int;
-
   bool needs_brokering;
   if (!iter->ReadBool(&needs_brokering))
     return false;
@@ -689,7 +681,8 @@ bool ParamTraits<base::SharedMemoryHandle>::Read(const Message* m,
     HandleWin handle_win;
     if (!ParamTraits<HandleWin>::Read(m, iter, &handle_win))
       return false;
-    *r = base::SharedMemoryHandle(handle_win.get_handle(), pid);
+    *r = base::SharedMemoryHandle(handle_win.get_handle(),
+                                  base::GetCurrentProcId());
     return true;
   }
 
@@ -697,14 +690,12 @@ bool ParamTraits<base::SharedMemoryHandle>::Read(const Message* m,
   if (!iter->ReadInt(&handle_int))
     return false;
   HANDLE handle = LongToHandle(handle_int);
-  *r = base::SharedMemoryHandle(handle, pid);
+  *r = base::SharedMemoryHandle(handle, base::GetCurrentProcId());
   return true;
 }
 
 void ParamTraits<base::SharedMemoryHandle>::Log(const param_type& p,
                                                 std::string* l) {
-  LogParam(p.GetPID(), l);
-  l->append(" ");
   LogParam(p.GetHandle(), l);
   l->append(" needs brokering: ");
   LogParam(p.NeedsBrokering(), l);
diff --git a/ipc/ipc_test_messages.h b/ipc/ipc_test_messages.h
index 62af913..a61d1b0 100644
--- a/ipc/ipc_test_messages.h
+++ b/ipc/ipc_test_messages.h
@@ -9,10 +9,12 @@
 #define IPC_MESSAGE_START IPCTestMsgStart
 
 #if defined(OS_WIN)
+#include "base/memory/shared_memory_handle.h"
 #include "ipc/handle_win.h"
 
 IPC_MESSAGE_CONTROL3(TestHandleWinMsg, int, IPC::HandleWin, int)
 IPC_MESSAGE_CONTROL2(TestTwoHandleWinMsg, IPC::HandleWin, IPC::HandleWin)
+IPC_MESSAGE_CONTROL1(TestSharedMemoryHandleMsg1, base::SharedMemoryHandle)
 #endif  // defined(OS_WIN)
 
 #if defined(OS_MACOSX)