IPC: Generalize FileDescriptorSet to MessageAttachmentSet

This is a preparation for crbug.com/448190, where we're going
to allow Mojo MessagePipe being sent over Chrome IPC.

The plan is that we'll extend existing FileDescriptorSet to
carry both platform handles and Mojo's MessagePipes, then
let ChannelMojo take care of these handles.

As the first step, this change renames FileDescriptorSet to
MessageAttachmentSet and moves it out from the OS_POSIX guard.
Although this change doesn't do anything significant, we'll
generalize it to carry both platform files and mojo handles
eventually.

R=agl@chromium.org, viettrungluu@chromium.org
BUG=448190

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

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

ipc/BUILD.gn

@@ -4,8 +4,6 @@
 
 component("ipc") {
   sources = [
-    "file_descriptor_set_posix.cc",
-    "file_descriptor_set_posix.h",
     "ipc_channel.cc",
     "ipc_channel.h",
     "ipc_channel_factory.cc",
@@ -31,6 +29,8 @@ component("ipc") {
     "ipc_logging.h",
     "ipc_message.cc",
     "ipc_message.h",
+    "ipc_message_attachment_set.cc",
+    "ipc_message_attachment_set.h",
     "ipc_message_macros.h",
     "ipc_message_start.h",
     "ipc_message_utils.cc",
@@ -86,10 +86,10 @@ component("ipc") {
 if (!is_android) {
   test("ipc_tests") {
     sources = [
-      "file_descriptor_set_posix_unittest.cc",
       "ipc_channel_posix_unittest.cc",
       "ipc_channel_unittest.cc",
       "ipc_fuzzing_tests.cc",
+      "ipc_message_attachment_set_posix_unittest.cc",
       "ipc_message_unittest.cc",
       "ipc_message_utils_unittest.cc",
       "ipc_send_fds_test.cc",

ipc/ipc.gyp

@@ -44,11 +44,11 @@
         '..'
       ],
       'sources': [
-        'file_descriptor_set_posix_unittest.cc',
         'ipc_channel_posix_unittest.cc',
         'ipc_channel_proxy_unittest.cc',
         'ipc_channel_unittest.cc',
         'ipc_fuzzing_tests.cc',
+        'ipc_message_attachment_set_posix_unittest.cc',
         'ipc_message_unittest.cc',
         'ipc_message_utils_unittest.cc',
         'ipc_send_fds_test.cc',

ipc/ipc.gypi

@@ -11,8 +11,6 @@
       # This part is shared between the targets defined below.
       ['ipc_target==1', {
         'sources': [
-          'file_descriptor_set_posix.cc',
-          'file_descriptor_set_posix.h',
           'ipc_channel.cc',
           'ipc_channel.h',
           'ipc_channel_factory.cc',
@@ -38,6 +36,8 @@
           'ipc_logging.h',
           'ipc_message.cc',
           'ipc_message.h',
+          'ipc_message_attachment_set.cc',
+          'ipc_message_attachment_set.h',
           'ipc_message_macros.h',
           'ipc_message_start.h',
           'ipc_message_utils.cc',

ipc/ipc_channel_nacl.cc

@@ -16,9 +16,9 @@
 #include "base/synchronization/lock.h"
 #include "base/task_runner_util.h"
 #include "base/threading/simple_thread.h"
-#include "ipc/file_descriptor_set_posix.h"
 #include "ipc/ipc_listener.h"
 #include "ipc/ipc_logging.h"
+#include "ipc/ipc_message_attachment_set.h"
 #include "native_client/src/public/imc_syscalls.h"
 #include "native_client/src/public/imc_types.h"
 
@@ -37,7 +37,7 @@ bool ReadDataOnReaderThread(int pipe, MessageContents* contents) {
     return false;
 
   contents->data.resize(Channel::kReadBufferSize);
-  contents->fds.resize(FileDescriptorSet::kMaxDescriptorsPerMessage);
+  contents->fds.resize(MessageAttachmentSet::kMaxDescriptorsPerMessage);
 
   NaClAbiNaClImcMsgIoVec iov = { &contents->data[0], contents->data.size() };
   NaClAbiNaClImcMsgHdr msg = {
@@ -275,10 +275,10 @@ bool ChannelNacl::ProcessOutgoingMessages() {
     linked_ptr<Message> msg = output_queue_.front();
     output_queue_.pop_front();
 
-    int fds[FileDescriptorSet::kMaxDescriptorsPerMessage];
-    const size_t num_fds = msg->file_descriptor_set()->size();
-    DCHECK(num_fds <= FileDescriptorSet::kMaxDescriptorsPerMessage);
-    msg->file_descriptor_set()->PeekDescriptors(fds);
+    int fds[MessageAttachmentSet::kMaxDescriptorsPerMessage];
+    const size_t num_fds = msg->attachment_set()->size();
+    DCHECK(num_fds <= MessageAttachmentSet::kMaxDescriptorsPerMessage);
+    msg->attachment_set()->PeekDescriptors(fds);
 
     NaClAbiNaClImcMsgIoVec iov = {
       const_cast<void*>(msg->data()), msg->size()
@@ -298,7 +298,7 @@ bool ChannelNacl::ProcessOutgoingMessages() {
                   << msg->size();
       return false;
     } else {
-      msg->file_descriptor_set()->CommitAll();
+      msg->attachment_set()->CommitAll();
     }
 
     // Message sent OK!
@@ -352,8 +352,7 @@ bool ChannelNacl::WillDispatchInputMessage(Message* msg) {
   // The shenaniganery below with &foo.front() requires input_fds_ to have
   // contiguous underlying storage (such as a simple array or a std::vector).
   // This is why the header warns not to make input_fds_ a deque<>.
-  msg->file_descriptor_set()->AddDescriptorsToOwn(&input_fds_.front(),
-                                                  header_fds);
+  msg->attachment_set()->AddDescriptorsToOwn(&input_fds_.front(), header_fds);
   input_fds_.clear();
   return true;
 }

ipc/ipc_channel_posix.cc

@@ -37,10 +37,10 @@
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "base/synchronization/lock.h"
-#include "ipc/file_descriptor_set_posix.h"
 #include "ipc/ipc_descriptors.h"
 #include "ipc/ipc_listener.h"
 #include "ipc/ipc_logging.h"
+#include "ipc/ipc_message_attachment_set.h"
 #include "ipc/ipc_message_utils.h"
 #include "ipc/ipc_switches.h"
 #include "ipc/unix_domain_socket_util.h"
@@ -395,13 +395,13 @@ void ChannelPosix::CloseFileDescriptors(Message* msg) {
   // descriptor. For more information, see:
   // http://crbug.com/298276
   std::vector<int> to_close;
-  msg->file_descriptor_set()->ReleaseFDsToClose(&to_close);
+  msg->attachment_set()->ReleaseFDsToClose(&to_close);
   for (size_t i = 0; i < to_close.size(); i++) {
     fds_to_close_.insert(to_close[i]);
     QueueCloseFDMessage(to_close[i], 2);
   }
 #else
-  msg->file_descriptor_set()->CommitAll();
+  msg->attachment_set()->CommitAll();
 #endif
 }
 
@@ -428,20 +428,19 @@ bool ChannelPosix::ProcessOutgoingMessages() {
     struct iovec iov = {const_cast<char*>(out_bytes), amt_to_write};
     msgh.msg_iov = &iov;
     msgh.msg_iovlen = 1;
-    char buf[CMSG_SPACE(
-        sizeof(int) * FileDescriptorSet::kMaxDescriptorsPerMessage)];
+    char buf[CMSG_SPACE(sizeof(int) *
+                        MessageAttachmentSet::kMaxDescriptorsPerMessage)];
 
     ssize_t bytes_written = 1;
     int fd_written = -1;
 
-    if (message_send_bytes_written_ == 0 &&
-        !msg->file_descriptor_set()->empty()) {
+    if (message_send_bytes_written_ == 0 && !msg->attachment_set()->empty()) {
       // This is the first chunk of a message which has descriptors to send
       struct cmsghdr *cmsg;
-      const unsigned num_fds = msg->file_descriptor_set()->size();
+      const unsigned num_fds = msg->attachment_set()->size();
 
-      DCHECK(num_fds <= FileDescriptorSet::kMaxDescriptorsPerMessage);
-      if (msg->file_descriptor_set()->ContainsDirectoryDescriptor()) {
+      DCHECK(num_fds <= MessageAttachmentSet::kMaxDescriptorsPerMessage);
+      if (msg->attachment_set()->ContainsDirectoryDescriptor()) {
         LOG(FATAL) << "Panic: attempting to transport directory descriptor over"
                       " IPC. Aborting to maintain sandbox isolation.";
         // If you have hit this then something tried to send a file descriptor
@@ -457,7 +456,7 @@ bool ChannelPosix::ProcessOutgoingMessages() {
       cmsg->cmsg_level = SOL_SOCKET;
       cmsg->cmsg_type = SCM_RIGHTS;
       cmsg->cmsg_len = CMSG_LEN(sizeof(int) * num_fds);
-      msg->file_descriptor_set()->PeekDescriptors(
+      msg->attachment_set()->PeekDescriptors(
           reinterpret_cast<int*>(CMSG_DATA(cmsg)));
       msgh.msg_controllen = cmsg->cmsg_len;
 
@@ -488,7 +487,7 @@ bool ChannelPosix::ProcessOutgoingMessages() {
       fd_written = pipe_.get();
 #if defined(IPC_USES_READWRITE)
       if ((mode_ & MODE_CLIENT_FLAG) && IsHelloMessage(*msg)) {
-        DCHECK_EQ(msg->file_descriptor_set()->size(), 1U);
+        DCHECK_EQ(msg->attachment_set()->size(), 1U);
       }
       if (!msgh.msg_controllen) {
         bytes_written =
@@ -795,7 +794,7 @@ void ChannelPosix::QueueHelloMessage() {
     if (!msg->WriteBorrowingFile(remote_fd_pipe_.get())) {
       NOTREACHED() << "Unable to pickle hello message file descriptors";
     }
-    DCHECK_EQ(msg->file_descriptor_set()->size(), 1U);
+    DCHECK_EQ(msg->attachment_set()->size(), 1U);
   }
 #endif  // IPC_USES_READWRITE
   output_queue_.push(msg.release());
@@ -903,7 +902,7 @@ bool ChannelPosix::WillDispatchInputMessage(Message* msg) {
       error = "Message needs unreceived descriptors";
   }
 
-  if (header_fds > FileDescriptorSet::kMaxDescriptorsPerMessage)
+  if (header_fds > MessageAttachmentSet::kMaxDescriptorsPerMessage)
     error = "Message requires an excessive number of descriptors";
 
   if (error) {
@@ -919,8 +918,7 @@ bool ChannelPosix::WillDispatchInputMessage(Message* msg) {
   // The shenaniganery below with &foo.front() requires input_fds_ to have
   // contiguous underlying storage (such as a simple array or a std::vector).
   // This is why the header warns not to make input_fds_ a deque<>.
-  msg->file_descriptor_set()->AddDescriptorsToOwn(&input_fds_.front(),
-                                                  header_fds);
+  msg->attachment_set()->AddDescriptorsToOwn(&input_fds_.front(), header_fds);
   input_fds_.erase(input_fds_.begin(), input_fds_.begin() + header_fds);
   return true;
 }
@@ -1013,7 +1011,7 @@ void ChannelPosix::HandleInternalMessage(const Message& msg) {
         // With IPC_USES_READWRITE, the Hello message from the client to the
         // server also contains the fd_pipe_, which  will be used for all
         // subsequent file descriptor passing.
-        DCHECK_EQ(msg.file_descriptor_set()->size(), 1U);
+        DCHECK_EQ(msg.attachment_set()->size(), 1U);
         base::ScopedFD descriptor;
         if (!msg.ReadFile(&iter, &descriptor)) {
           NOTREACHED();

ipc/ipc_channel_posix.h

@@ -17,8 +17,8 @@
 #include "base/files/scoped_file.h"
 #include "base/message_loop/message_loop.h"
 #include "base/process/process.h"
-#include "ipc/file_descriptor_set_posix.h"
 #include "ipc/ipc_channel_reader.h"
+#include "ipc/ipc_message_attachment_set.h"
 
 #if !defined(OS_MACOSX)
 // On Linux, the seccomp sandbox makes it very expensive to call
@@ -178,7 +178,7 @@ class IPC_EXPORT ChannelPosix : public Channel,
   // message has no payload and a full complement of descriptors.
   static const size_t kMaxReadFDs =
       (Channel::kReadBufferSize / sizeof(IPC::Message::Header)) *
-      FileDescriptorSet::kMaxDescriptorsPerMessage;
+      MessageAttachmentSet::kMaxDescriptorsPerMessage;
 
   // Buffer size for file descriptors used for recvmsg. On Mac the CMSG macros
   // don't seem to be constant so we have to pick a "large enough" value.

ipc/ipc_message.cc

@@ -7,10 +7,10 @@
 #include "base/atomic_sequence_num.h"
 #include "base/logging.h"
 #include "build/build_config.h"
+#include "ipc/ipc_message_attachment_set.h"
 
 #if defined(OS_POSIX)
 #include "base/file_descriptor_posix.h"
-#include "ipc/file_descriptor_set_posix.h"
 #endif
 
 namespace {
@@ -71,7 +71,7 @@ Message::Message(const char* data, int data_len) : Pickle(data, data_len) {
 Message::Message(const Message& other) : Pickle(other) {
   Init();
 #if defined(OS_POSIX)
-  file_descriptor_set_ = other.file_descriptor_set_;
+  attachment_set_ = other.attachment_set_;
 #endif
 }
 
@@ -87,7 +87,7 @@ void Message::Init() {
 Message& Message::operator=(const Message& other) {
   *static_cast<Pickle*>(this) = other;
 #if defined(OS_POSIX)
-  file_descriptor_set_ = other.file_descriptor_set_;
+  attachment_set_ = other.attachment_set_;
 #endif
   return *this;
 }
@@ -101,6 +101,11 @@ void Message::SetHeaderValues(int32 routing, uint32 type, uint32 flags) {
   header()->flags = flags;
 }
 
+void Message::EnsureMessageAttachmentSet() {
+  if (attachment_set_.get() == NULL)
+    attachment_set_ = new MessageAttachmentSet;
+}
+
 #ifdef IPC_MESSAGE_LOG_ENABLED
 void Message::set_sent_time(int64 time) {
   DCHECK((header()->flags & HAS_SENT_TIME_BIT) == 0);
@@ -126,15 +131,15 @@ void Message::set_received_time(int64 time) const {
 bool Message::WriteFile(base::ScopedFD descriptor) {
   // We write the index of the descriptor so that we don't have to
   // keep the current descriptor as extra decoding state when deserialising.
-  WriteInt(file_descriptor_set()->size());
-  return file_descriptor_set()->AddToOwn(descriptor.Pass());
+  WriteInt(attachment_set()->size());
+  return attachment_set()->AddToOwn(descriptor.Pass());
 }
 
 bool Message::WriteBorrowingFile(const base::PlatformFile& descriptor) {
   // We write the index of the descriptor so that we don't have to
   // keep the current descriptor as extra decoding state when deserialising.
-  WriteInt(file_descriptor_set()->size());
-  return file_descriptor_set()->AddToBorrow(descriptor);
+  WriteInt(attachment_set()->size());
+  return attachment_set()->AddToBorrow(descriptor);
 }
 
 bool Message::ReadFile(PickleIterator* iter, base::ScopedFD* descriptor) const {
@@ -142,12 +147,11 @@ bool Message::ReadFile(PickleIterator* iter, base::ScopedFD* descriptor) const {
   if (!iter->ReadInt(&descriptor_index))
     return false;
 
-  FileDescriptorSet* file_descriptor_set = file_descriptor_set_.get();
-  if (!file_descriptor_set)
+  MessageAttachmentSet* attachment_set = attachment_set_.get();
+  if (!attachment_set)
     return false;
 
-  base::PlatformFile file =
-      file_descriptor_set->TakeDescriptorAt(descriptor_index);
+  base::PlatformFile file = attachment_set->TakeDescriptorAt(descriptor_index);
   if (file < 0)
     return false;
 
@@ -156,12 +160,7 @@ bool Message::ReadFile(PickleIterator* iter, base::ScopedFD* descriptor) const {
 }
 
 bool Message::HasFileDescriptors() const {
-  return file_descriptor_set_.get() && !file_descriptor_set_->empty();
-}
-
-void Message::EnsureFileDescriptorSet() {
-  if (file_descriptor_set_.get() == NULL)
-    file_descriptor_set_ = new FileDescriptorSet;
+  return attachment_set_.get() && !attachment_set_->empty();
 }
 
 #endif

ipc/ipc_message.h

@@ -21,13 +21,12 @@
 #include "base/memory/ref_counted.h"
 #endif
 
-class FileDescriptorSet;
-
 namespace IPC {
 
 //------------------------------------------------------------------------------
 
 struct LogData;
+class MessageAttachmentSet;
 
 class IPC_EXPORT Message : public Pickle {
  public:
@@ -250,21 +249,19 @@ class IPC_EXPORT Message : public Pickle {
   // Used internally to support IPC::Listener::OnBadMessageReceived.
   mutable bool dispatch_error_;
 
-#if defined(OS_POSIX)
   // The set of file descriptors associated with this message.
-  scoped_refptr<FileDescriptorSet> file_descriptor_set_;
+  scoped_refptr<MessageAttachmentSet> attachment_set_;
 
-  // Ensure that a FileDescriptorSet is allocated
-  void EnsureFileDescriptorSet();
+  // Ensure that a MessageAttachmentSet is allocated
+  void EnsureMessageAttachmentSet();
 
-  FileDescriptorSet* file_descriptor_set() {
-    EnsureFileDescriptorSet();
-    return file_descriptor_set_.get();
+  MessageAttachmentSet* attachment_set() {
+    EnsureMessageAttachmentSet();
+    return attachment_set_.get();
   }
-  const FileDescriptorSet* file_descriptor_set() const {
-    return file_descriptor_set_.get();
+  const MessageAttachmentSet* attachment_set() const {
+    return attachment_set_.get();
   }
-#endif
 
 #ifdef IPC_MESSAGE_LOG_ENABLED
   // Used for logging.

ipc/file_descriptor_set_posix.cc → ipc/ipc_message_attachment_set.cc

@@ -2,20 +2,24 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "ipc/file_descriptor_set_posix.h"
+#include "ipc/ipc_message_attachment_set.h"
 
+#include "base/logging.h"
+#include "base/posix/eintr_wrapper.h"
+
+#if defined(OS_POSIX)
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <unistd.h>
+#endif // OS_POSIX
 
-#include "base/logging.h"
-#include "base/posix/eintr_wrapper.h"
+namespace IPC {
 
-FileDescriptorSet::FileDescriptorSet()
+MessageAttachmentSet::MessageAttachmentSet()
     : consumed_descriptor_highwater_(0) {
 }
 
-FileDescriptorSet::~FileDescriptorSet() {
+MessageAttachmentSet::~MessageAttachmentSet() {
   if (consumed_descriptor_highwater_ == size())
     return;
 
@@ -27,15 +31,25 @@ FileDescriptorSet::~FileDescriptorSet() {
   // (which could a DOS against the browser by a rogue renderer) then all
   // the descriptors have their close flag set and we free all the extra
   // kernel resources.
-  LOG(WARNING) << "FileDescriptorSet destroyed with unconsumed descriptors: "
+  LOG(WARNING) << "MessageAttachmentSet destroyed with unconsumed descriptors: "
                << consumed_descriptor_highwater_ << "/" << size();
 }
 
-bool FileDescriptorSet::AddToBorrow(base::PlatformFile fd) {
+unsigned MessageAttachmentSet::size() const {
+#if defined(OS_POSIX)
+  return descriptors_.size();
+#else
+  return 0;
+#endif
+}
+
+#if defined(OS_POSIX)
+
+bool MessageAttachmentSet::AddToBorrow(base::PlatformFile fd) {
   DCHECK_EQ(consumed_descriptor_highwater_, 0u);
 
   if (size() == kMaxDescriptorsPerMessage) {
-    DLOG(WARNING) << "Cannot add file descriptor. FileDescriptorSet full.";
+    DLOG(WARNING) << "Cannot add file descriptor. MessageAttachmentSet full.";
     return false;
   }
 
@@ -43,11 +57,11 @@ bool FileDescriptorSet::AddToBorrow(base::PlatformFile fd) {
   return true;
 }
 
-bool FileDescriptorSet::AddToOwn(base::ScopedFD fd) {
+bool MessageAttachmentSet::AddToOwn(base::ScopedFD fd) {
   DCHECK_EQ(consumed_descriptor_highwater_, 0u);
 
   if (size() == kMaxDescriptorsPerMessage) {
-    DLOG(WARNING) << "Cannot add file descriptor. FileDescriptorSet full.";
+    DLOG(WARNING) << "Cannot add file descriptor. MessageAttachmentSet full.";
     return false;
   }
 
@@ -57,14 +71,12 @@ bool FileDescriptorSet::AddToOwn(base::ScopedFD fd) {
   return true;
 }
 
-base::PlatformFile FileDescriptorSet::TakeDescriptorAt(unsigned index) {
+base::PlatformFile MessageAttachmentSet::TakeDescriptorAt(unsigned index) {
   if (index >= size()) {
-    DLOG(WARNING) << "Accessing out of bound index:"
-                  << index << "/" << size();
+    DLOG(WARNING) << "Accessing out of bound index:" << index << "/" << size();
     return -1;
   }
 
-
   // We should always walk the descriptors in order, so it's reasonable to
   // enforce this. Consider the case where a compromised renderer sends us
   // the following message:
@@ -102,8 +114,7 @@ base::PlatformFile FileDescriptorSet::TakeDescriptorAt(unsigned index) {
   // changed to exercise with own-able descriptors.
   for (ScopedVector<base::ScopedFD>::const_iterator i =
            owned_descriptors_.begin();
-       i != owned_descriptors_.end();
-       ++i) {
+       i != owned_descriptors_.end(); ++i) {
     if ((*i)->get() == file) {
       ignore_result((*i)->release());
       break;
@@ -113,16 +124,15 @@ base::PlatformFile FileDescriptorSet::TakeDescriptorAt(unsigned index) {
   return file;
 }
 
-void FileDescriptorSet::PeekDescriptors(base::PlatformFile* buffer) const {
+void MessageAttachmentSet::PeekDescriptors(base::PlatformFile* buffer) const {
   std::copy(descriptors_.begin(), descriptors_.end(), buffer);
 }
 
-bool FileDescriptorSet::ContainsDirectoryDescriptor() const {
+bool MessageAttachmentSet::ContainsDirectoryDescriptor() const {
   struct stat st;
 
   for (std::vector<base::PlatformFile>::const_iterator i = descriptors_.begin();
-       i != descriptors_.end();
-       ++i) {
+       i != descriptors_.end(); ++i) {
     if (fstat(*i, &st) == 0 && S_ISDIR(st.st_mode))
       return true;
   }
@@ -130,25 +140,24 @@ bool FileDescriptorSet::ContainsDirectoryDescriptor() const {
   return false;
 }
 
-void FileDescriptorSet::CommitAll() {
+void MessageAttachmentSet::CommitAll() {
   descriptors_.clear();
   owned_descriptors_.clear();
   consumed_descriptor_highwater_ = 0;
 }
 
-void FileDescriptorSet::ReleaseFDsToClose(
+void MessageAttachmentSet::ReleaseFDsToClose(
     std::vector<base::PlatformFile>* fds) {
   for (ScopedVector<base::ScopedFD>::iterator i = owned_descriptors_.begin();
-       i != owned_descriptors_.end();
-       ++i) {
+       i != owned_descriptors_.end(); ++i) {
     fds->push_back((*i)->release());
   }
 
   CommitAll();
 }
 
-void FileDescriptorSet::AddDescriptorsToOwn(const base::PlatformFile* buffer,
-                                            unsigned count) {
+void MessageAttachmentSet::AddDescriptorsToOwn(const base::PlatformFile* buffer,
+                                               unsigned count) {
   DCHECK(count <= kMaxDescriptorsPerMessage);
   DCHECK_EQ(size(), 0u);
   DCHECK_EQ(consumed_descriptor_highwater_, 0u);
@@ -160,3 +169,9 @@ void FileDescriptorSet::AddDescriptorsToOwn(const base::PlatformFile* buffer,
     owned_descriptors_.push_back(new base::ScopedFD(buffer[i]));
   }
 }
+
+#endif  // OS_POSIX
+
+}  // namespace IPC
+
+

ipc/file_descriptor_set_posix.h → ipc/ipc_message_attachment_set.h

@@ -2,27 +2,38 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef IPC_FILE_DESCRIPTOR_SET_POSIX_H_
-#define IPC_FILE_DESCRIPTOR_SET_POSIX_H_
+#ifndef IPC_IPC_MESSAGE_ATTACHMENT_SET_H_
+#define IPC_IPC_MESSAGE_ATTACHMENT_SET_H_
 
 #include <vector>
 
 #include "base/basictypes.h"
-#include "base/files/file.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_vector.h"
 #include "ipc/ipc_export.h"
 
+#if defined(OS_POSIX)
+#include "base/files/file.h"
+#endif
+
+namespace IPC {
+
 // -----------------------------------------------------------------------------
-// A FileDescriptorSet is an ordered set of POSIX file descriptors. These are
+// A MessageAttachmentSet is an ordered set of POSIX file descriptors. These are
 // associated with IPC messages so that descriptors can be transmitted over a
 // UNIX domain socket.
 // -----------------------------------------------------------------------------
-class IPC_EXPORT FileDescriptorSet
-    : public base::RefCountedThreadSafe<FileDescriptorSet> {
+class IPC_EXPORT MessageAttachmentSet
+    : public base::RefCountedThreadSafe<MessageAttachmentSet> {
  public:
-  FileDescriptorSet();
+  MessageAttachmentSet();
 
+  // Return the number of descriptors
+  unsigned size() const;
+  // Return true if no unconsumed descriptors remain
+  bool empty() const { return 0 == size(); }
+
+#if defined(OS_POSIX)
   // This is the maximum number of descriptors per message. We need to know this
   // because the control message kernel interface has to be given a buffer which
   // is large enough to store all the descriptor numbers. Otherwise the kernel
@@ -30,7 +41,7 @@ class IPC_EXPORT FileDescriptorSet
   // lost.
   //
   // In debugging mode, it's a fatal error to try and add more than this number
-  // of descriptors to a FileDescriptorSet.
+  // of descriptors to a MessageAttachmentSet.
   static const size_t kMaxDescriptorsPerMessage = 7;
 
   // ---------------------------------------------------------------------------
@@ -43,15 +54,9 @@ class IPC_EXPORT FileDescriptorSet
   bool AddToOwn(base::ScopedFD fd);
 
   // ---------------------------------------------------------------------------
-
-
   // ---------------------------------------------------------------------------
   // Interfaces for accessing during message deserialisation...
 
-  // Return the number of descriptors
-  unsigned size() const { return descriptors_.size(); }
-  // Return true if no unconsumed descriptors remain
-  bool empty() const { return 0 == size(); }
   // Take the nth descriptor from the beginning of the set,
   // transferring the ownership of the descriptor taken. Code using this
   // /must/ access the descriptors in order, and must do it at most once.
@@ -63,7 +68,6 @@ class IPC_EXPORT FileDescriptorSet
 
   // ---------------------------------------------------------------------------
 
-
   // ---------------------------------------------------------------------------
   // Interfaces for transmission...
 
@@ -84,7 +88,6 @@ class IPC_EXPORT FileDescriptorSet
 
   // ---------------------------------------------------------------------------
 
-
   // ---------------------------------------------------------------------------
   // Interfaces for receiving...
 
@@ -93,19 +96,23 @@ class IPC_EXPORT FileDescriptorSet
   // unconsumed descriptors are closed on destruction.
   void AddDescriptorsToOwn(const base::PlatformFile* buffer, unsigned count);
 
+#endif  // OS_POSIX
+
   // ---------------------------------------------------------------------------
 
  private:
-  friend class base::RefCountedThreadSafe<FileDescriptorSet>;
+  friend class base::RefCountedThreadSafe<MessageAttachmentSet>;
 
-  ~FileDescriptorSet();
+  ~MessageAttachmentSet();
 
+#if defined(OS_POSIX)
   // A vector of descriptors and close flags. If this message is sent, then
   // these descriptors are sent as control data. After sending, any descriptors
   // with a true flag are closed. If this message has been received, then these
   // are the descriptors which were received and all close flags are true.
   std::vector<base::PlatformFile> descriptors_;
   ScopedVector<base::ScopedFD> owned_descriptors_;
+#endif
 
   // This contains the index of the next descriptor which should be consumed.
   // It's used in a couple of ways. Firstly, at destruction we can check that
@@ -113,7 +120,9 @@ class IPC_EXPORT FileDescriptorSet
   // can check that they are read in order.
   mutable unsigned consumed_descriptor_highwater_;
 
-  DISALLOW_COPY_AND_ASSIGN(FileDescriptorSet);
+  DISALLOW_COPY_AND_ASSIGN(MessageAttachmentSet);
 };
 
-#endif  // IPC_FILE_DESCRIPTOR_SET_POSIX_H_
+}  // namespace IPC
+
+#endif  // IPC_IPC_MESSAGE_ATTACHMENT_SET_H_

ipc/file_descriptor_set_posix_unittest.cc → ipc/ipc_message_attachment_set_posix_unittest.cc

@@ -4,15 +4,16 @@
 
 // This test is POSIX only.
 
-#include "ipc/file_descriptor_set_posix.h"
+#include "ipc/ipc_message_attachment_set.h"
 
-#include <unistd.h>
 #include <fcntl.h>
+#include <unistd.h>
 
 #include "base/basictypes.h"
 #include "base/posix/eintr_wrapper.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+namespace IPC {
 namespace {
 
 // Get a safe file descriptor for test purposes.
@@ -31,13 +32,13 @@ bool VerifyClosed(int fd) {
   return true;
 }
 
-// The FileDescriptorSet will try and close some of the descriptor numbers
+// The MessageAttachmentSet will try and close some of the descriptor numbers
 // which we given it. This is the base descriptor value. It's great enough such
 // that no real descriptor will accidently be closed.
 static const int kFDBase = 50000;
 
-TEST(FileDescriptorSet, BasicAdd) {
-  scoped_refptr<FileDescriptorSet> set(new FileDescriptorSet);
+TEST(MessageAttachmentSet, BasicAdd) {
+  scoped_refptr<MessageAttachmentSet> set(new MessageAttachmentSet);
 
   ASSERT_EQ(set->size(), 0u);
   ASSERT_TRUE(set->empty());
@@ -50,8 +51,8 @@ TEST(FileDescriptorSet, BasicAdd) {
   set->CommitAll();
 }
 
-TEST(FileDescriptorSet, BasicAddAndClose) {
-  scoped_refptr<FileDescriptorSet> set(new FileDescriptorSet);
+TEST(MessageAttachmentSet, BasicAddAndClose) {
+  scoped_refptr<MessageAttachmentSet> set(new MessageAttachmentSet);
 
   ASSERT_EQ(set->size(), 0u);
   ASSERT_TRUE(set->empty());
@@ -64,10 +65,10 @@ TEST(FileDescriptorSet, BasicAddAndClose) {
 
   ASSERT_TRUE(VerifyClosed(fd));
 }
-TEST(FileDescriptorSet, MaxSize) {
-  scoped_refptr<FileDescriptorSet> set(new FileDescriptorSet);
+TEST(MessageAttachmentSet, MaxSize) {
+  scoped_refptr<MessageAttachmentSet> set(new MessageAttachmentSet);
 
-  for (size_t i = 0; i < FileDescriptorSet::kMaxDescriptorsPerMessage; ++i)
+  for (size_t i = 0; i < MessageAttachmentSet::kMaxDescriptorsPerMessage; ++i)
     ASSERT_TRUE(set->AddToBorrow(kFDBase + 1 + i));
 
   ASSERT_TRUE(!set->AddToBorrow(kFDBase));
@@ -75,8 +76,8 @@ TEST(FileDescriptorSet, MaxSize) {
   set->CommitAll();
 }
 
-TEST(FileDescriptorSet, SetDescriptors) {
-  scoped_refptr<FileDescriptorSet> set(new FileDescriptorSet);
+TEST(MessageAttachmentSet, SetDescriptors) {
+  scoped_refptr<MessageAttachmentSet> set(new MessageAttachmentSet);
 
   ASSERT_TRUE(set->empty());
   set->AddDescriptorsToOwn(NULL, 0);
@@ -93,8 +94,8 @@ TEST(FileDescriptorSet, SetDescriptors) {
   ASSERT_TRUE(VerifyClosed(fd));
 }
 
-TEST(FileDescriptorSet, PeekDescriptors) {
-  scoped_refptr<FileDescriptorSet> set(new FileDescriptorSet);
+TEST(MessageAttachmentSet, PeekDescriptors) {
+  scoped_refptr<MessageAttachmentSet> set(new MessageAttachmentSet);
 
   set->PeekDescriptors(NULL);
   ASSERT_TRUE(set->AddToBorrow(kFDBase));
@@ -107,8 +108,8 @@ TEST(FileDescriptorSet, PeekDescriptors) {
   ASSERT_TRUE(set->empty());
 }
 
-TEST(FileDescriptorSet, WalkInOrder) {
-  scoped_refptr<FileDescriptorSet> set(new FileDescriptorSet);
+TEST(MessageAttachmentSet, WalkInOrder) {
+  scoped_refptr<MessageAttachmentSet> set(new MessageAttachmentSet);
 
   // TODO(morrita): This test is wrong. TakeDescriptorAt() shouldn't be
   // used to retrieve borrowed descriptors. That never happens in production.
@@ -123,8 +124,8 @@ TEST(FileDescriptorSet, WalkInOrder) {
   set->CommitAll();
 }
 
-TEST(FileDescriptorSet, WalkWrongOrder) {
-  scoped_refptr<FileDescriptorSet> set(new FileDescriptorSet);
+TEST(MessageAttachmentSet, WalkWrongOrder) {
+  scoped_refptr<MessageAttachmentSet> set(new MessageAttachmentSet);
 
   // TODO(morrita): This test is wrong. TakeDescriptorAt() shouldn't be
   // used to retrieve borrowed descriptors. That never happens in production.
@@ -138,8 +139,8 @@ TEST(FileDescriptorSet, WalkWrongOrder) {
   set->CommitAll();
 }
 
-TEST(FileDescriptorSet, WalkCycle) {
-  scoped_refptr<FileDescriptorSet> set(new FileDescriptorSet);
+TEST(MessageAttachmentSet, WalkCycle) {
+  scoped_refptr<MessageAttachmentSet> set(new MessageAttachmentSet);
 
   // TODO(morrita): This test is wrong. TakeDescriptorAt() shouldn't be
   // used to retrieve borrowed descriptors. That never happens in production.
@@ -160,8 +161,8 @@ TEST(FileDescriptorSet, WalkCycle) {
   set->CommitAll();
 }
 
-TEST(FileDescriptorSet, DontClose) {
-  scoped_refptr<FileDescriptorSet> set(new FileDescriptorSet);
+TEST(MessageAttachmentSet, DontClose) {
+  scoped_refptr<MessageAttachmentSet> set(new MessageAttachmentSet);
 
   const int fd = GetSafeFd();
   ASSERT_TRUE(set->AddToBorrow(fd));
@@ -170,8 +171,8 @@ TEST(FileDescriptorSet, DontClose) {
   ASSERT_FALSE(VerifyClosed(fd));
 }
 
-TEST(FileDescriptorSet, DoClose) {
-  scoped_refptr<FileDescriptorSet> set(new FileDescriptorSet);
+TEST(MessageAttachmentSet, DoClose) {
+  scoped_refptr<MessageAttachmentSet> set(new MessageAttachmentSet);
 
   const int fd = GetSafeFd();
   ASSERT_TRUE(set->AddToOwn(base::ScopedFD(fd)));
@@ -181,3 +182,4 @@ TEST(FileDescriptorSet, DoClose) {
 }
 
 }  // namespace
+}  // namespace IPC

ipc/ipc_message_utils.cc

@@ -13,10 +13,9 @@
 #include "base/time/time.h"
 #include "base/values.h"
 #include "ipc/ipc_channel_handle.h"
+#include "ipc/ipc_message_attachment_set.h"
 
-#if defined(OS_POSIX)
-#include "ipc/file_descriptor_set_posix.h"
-#elif defined(OS_WIN)
+#if defined(OS_WIN)
 #include <tchar.h>
 #endif
 

ipc/ipc_send_fds_test.cc

@@ -23,12 +23,12 @@ extern "C" {
 #include "base/pickle.h"
 #include "base/posix/eintr_wrapper.h"
 #include "base/synchronization/waitable_event.h"
+#include "ipc/ipc_message_attachment_set.h"
 #include "ipc/ipc_message_utils.h"
 #include "ipc/ipc_test_base.h"
 
 #if defined(OS_POSIX)
 #include "base/macros.h"
-#include "ipc/file_descriptor_set_posix.h"
 #endif
 
 namespace {
@@ -38,8 +38,9 @@ const unsigned kNumMessages = 20;
 const char* kDevZeroPath = "/dev/zero";
 
 #if defined(OS_POSIX)
-COMPILE_ASSERT(kNumFDsToSend == FileDescriptorSet::kMaxDescriptorsPerMessage,
-  num_fds_to_send_must_be_the_same_as_the_max_desc_per_message);
+COMPILE_ASSERT(kNumFDsToSend ==
+                   IPC::MessageAttachmentSet::kMaxDescriptorsPerMessage,
+               num_fds_to_send_must_be_the_same_as_the_max_desc_per_message);
 #endif
 
 class MyChannelDescriptorListenerBase : public IPC::Listener {

ipc/mojo/ipc_channel_mojo.cc

@@ -9,16 +9,13 @@
 #include "base/lazy_instance.h"
 #include "ipc/ipc_listener.h"
 #include "ipc/ipc_logging.h"
+#include "ipc/ipc_message_attachment_set.h"
 #include "ipc/ipc_message_macros.h"
 #include "ipc/mojo/client_channel.mojom.h"
 #include "ipc/mojo/ipc_mojo_bootstrap.h"
 #include "mojo/edk/embedder/embedder.h"
 #include "mojo/public/cpp/bindings/error_handler.h"
 
-#if defined(OS_POSIX) && !defined(OS_NACL)
-#include "ipc/file_descriptor_set_posix.h"
-#endif
-
 namespace IPC {
 
 namespace {
@@ -342,7 +339,7 @@ base::ScopedFD ChannelMojo::TakeClientFileDescriptor() {
 }
 
 // static
-MojoResult ChannelMojo::WriteToFileDescriptorSet(
+MojoResult ChannelMojo::WriteToMessageAttachmentSet(
     const std::vector<MojoHandle>& handle_buffer,
     Message* message) {
   for (size_t i = 0; i < handle_buffer.size(); ++i) {
@@ -355,7 +352,7 @@ MojoResult ChannelMojo::WriteToFileDescriptorSet(
       return unwrap_result;
     }
 
-    bool ok = message->file_descriptor_set()->AddToOwn(
+    bool ok = message->attachment_set()->AddToOwn(
         base::ScopedFD(platform_handle.release().fd));
     DCHECK(ok);
   }
@@ -364,14 +361,14 @@ MojoResult ChannelMojo::WriteToFileDescriptorSet(
 }
 
 // static
-MojoResult ChannelMojo::ReadFromFileDescriptorSet(
+MojoResult ChannelMojo::ReadFromMessageAttachmentSet(
     Message* message,
     std::vector<MojoHandle>* handles) {
   // We dup() the handles in IPC::Message to transmit.
-  // IPC::FileDescriptorSet has intricate lifecycle semantics
+  // IPC::MessageAttachmentSet has intricate lifecycle semantics
   // of FDs, so just to dup()-and-own them is the safest option.
   if (message->HasFileDescriptors()) {
-    FileDescriptorSet* fdset = message->file_descriptor_set();
+    MessageAttachmentSet* fdset = message->attachment_set();
     std::vector<base::PlatformFile> fds_to_send(fdset->size());
     fdset->PeekDescriptors(&fds_to_send[0]);
     for (size_t i = 0; i < fds_to_send.size(); ++i) {

ipc/mojo/ipc_channel_mojo.h

@@ -95,11 +95,12 @@ class IPC_MOJO_EXPORT ChannelMojo
 
   // These access protected API of IPC::Message, which has ChannelMojo
   // as a friend class.
-  static MojoResult WriteToFileDescriptorSet(
+  static MojoResult WriteToMessageAttachmentSet(
       const std::vector<MojoHandle>& handle_buffer,
       Message* message);
-  static MojoResult ReadFromFileDescriptorSet(Message* message,
-                                              std::vector<MojoHandle>* handles);
+  static MojoResult ReadFromMessageAttachmentSet(
+      Message* message,
+      std::vector<MojoHandle>* handles);
 
 #endif  // defined(OS_POSIX) && !defined(OS_NACL)
 

ipc/mojo/ipc_message_pipe_reader.cc

@@ -46,7 +46,7 @@ bool MessagePipeReader::Send(scoped_ptr<Message> message) {
   std::vector<MojoHandle> handles;
   MojoResult result = MOJO_RESULT_OK;
 #if defined(OS_POSIX) && !defined(OS_NACL)
-  result = ChannelMojo::ReadFromFileDescriptorSet(message.get(), &handles);
+  result = ChannelMojo::ReadFromMessageAttachmentSet(message.get(), &handles);
 #endif
   if (result == MOJO_RESULT_OK) {
     result = MojoWriteMessage(handle(),
@@ -74,7 +74,7 @@ void MessagePipeReader::OnMessageReceived() {
   TakeHandleBuffer(&handle_buffer);
 #if defined(OS_POSIX) && !defined(OS_NACL)
   MojoResult write_result =
-      ChannelMojo::WriteToFileDescriptorSet(handle_buffer, &message);
+      ChannelMojo::WriteToMessageAttachmentSet(handle_buffer, &message);
   if (write_result != MOJO_RESULT_OK) {
     CloseWithError(write_result);
     return;