ipc: Update Mach port attachment ownership semantics.

Previously, in the receiving process, the Mach port attachment did not own the
underlying Mach port, and the receiver of the message was expected to take
ownership.

Now, the Mach port attachment has ownership, and the first call to
ParamTraits<MachPortMac>::Read() takes ownership of the Mach port. This relies
on the assumption that ParamTraits<MachPortMac>::Read() is only called once on
any given Chrome IPC message parameter.

This change has two benefits. First, Mach port attachments now behave
identically between sender and receiver processes, so tests that stub out the
Chrome IPC stack can still send IPC::Messages that contain Mach port
attachments. Second, if ParamTraits<MachPortMac>::Read() is never called, the
Mach port is not leaked.

BUG=

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

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

4 files changed, 12 insertions, 16 deletions

diff --git a/ipc/attachment_broker_mac_unittest.cc b/ipc/attachment_broker_mac_unittest.cc
index be4f3c5..bf994b7 100644
--- a/ipc/attachment_broker_mac_unittest.cc
+++ b/ipc/attachment_broker_mac_unittest.cc
@@ -76,8 +76,10 @@ base::mac::ScopedMachSendRight GetMachPortFromBrokeredAttachment(
 
   IPC::internal::MachPortAttachmentMac* received_mach_port_attachment =
       static_cast<IPC::internal::MachPortAttachmentMac*>(attachment.get());
-  return base::mac::ScopedMachSendRight(
+  base::mac::ScopedMachSendRight send_right(
       received_mach_port_attachment->get_mach_port());
+  received_mach_port_attachment->reset_mach_port_ownership();
+  return send_right;
 }
 
 // Makes a Mach port backed SharedMemory region and fills it with |contents|.
diff --git a/ipc/mach_port_attachment_mac.cc b/ipc/mach_port_attachment_mac.cc
index 7a1d40b..65baa34 100644
--- a/ipc/mach_port_attachment_mac.cc
+++ b/ipc/mach_port_attachment_mac.cc
@@ -24,13 +24,7 @@ MachPortAttachmentMac::MachPortAttachmentMac(mach_port_t mach_port)
 MachPortAttachmentMac::MachPortAttachmentMac(const WireFormat& wire_format)
     : BrokerableAttachment(wire_format.attachment_id),
       mach_port_(static_cast<mach_port_t>(wire_format.mach_port)),
-      owns_mach_port_(false) {}
-
-MachPortAttachmentMac::MachPortAttachmentMac(
-    const BrokerableAttachment::AttachmentId& id)
-    : BrokerableAttachment(id),
-      mach_port_(MACH_PORT_NULL),
-      owns_mach_port_(false) {}
+      owns_mach_port_(true) {}
 
 MachPortAttachmentMac::~MachPortAttachmentMac() {
   if (mach_port_ != MACH_PORT_NULL && owns_mach_port_) {
diff --git a/ipc/mach_port_attachment_mac.h b/ipc/mach_port_attachment_mac.h
index 7b2465c..244014e 100644
--- a/ipc/mach_port_attachment_mac.h
+++ b/ipc/mach_port_attachment_mac.h
@@ -51,10 +51,10 @@ class IPC_EXPORT MachPortAttachmentMac : public BrokerableAttachment {
   // IPC message.
   explicit MachPortAttachmentMac(mach_port_t mach_port);
 
-  // These constructors do not take ownership of |mach_port|, and should only be
-  // called by the receiver of a Chrome IPC message.
+  // This constructor takes ownership of |wire_format.mach_port|, but does not
+  // modify its ref count. Should only be called by the receiver of a Chrome IPC
+  // message.
   explicit MachPortAttachmentMac(const WireFormat& wire_format);
-  explicit MachPortAttachmentMac(const BrokerableAttachment::AttachmentId& id);
 
   BrokerableType GetBrokerableType() const override;
 
@@ -71,11 +71,10 @@ class IPC_EXPORT MachPortAttachmentMac : public BrokerableAttachment {
   const mach_port_t mach_port_;
 
   // In the sender process, the attachment owns the Mach port 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.
+  // message. The attachment broker will eventually take ownership of
+  // |mach_port_|.
+  // In the destination process, the attachment owns |mach_port_| until
+  // ParamTraits<MachPortMac>::Read() is called, which takes ownership.
   bool owns_mach_port_;
   DISALLOW_COPY_AND_ASSIGN(MachPortAttachmentMac);
 };
diff --git a/ipc/mach_port_mac.cc b/ipc/mach_port_mac.cc
index 96f4aca..51a5bd7 100644
--- a/ipc/mach_port_mac.cc
+++ b/ipc/mach_port_mac.cc
@@ -37,6 +37,7 @@ bool ParamTraits<MachPortMac>::Read(const Message* m,
   IPC::internal::MachPortAttachmentMac* mach_port_attachment =
       static_cast<IPC::internal::MachPortAttachmentMac*>(brokerable_attachment);
   r->set_mach_port(mach_port_attachment->get_mach_port());
+  mach_port_attachment->reset_mach_port_ownership();
   return true;
 }