IPC: Get rid of FileDescriptor usage from FileDescriptorSet and Message

This is a step toward to killing FileDescriptor.
This change lets FiileDescriptorSet have both Files (for owning fds)
and PlatformFiles (for non-owning fds). Doing this, we no longer
need FileDescriptor which provides |auto_close| flag.

BUG=415294
TEST=ipc_tests, ipc_mojo_unittests
R=agl@chromium.org, jam@hcromium.org, viettrungluu@chromium.org

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

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

4 files changed, 20 insertions, 13 deletions

diff --git a/ipc/mojo/ipc_channel_mojo.cc b/ipc/mojo/ipc_channel_mojo.cc
index e61a90e..72bc29b 100644
--- a/ipc/mojo/ipc_channel_mojo.cc
+++ b/ipc/mojo/ipc_channel_mojo.cc
@@ -229,7 +229,8 @@ MojoResult ChannelMojo::WriteToFileDescriptorSet(
       return unwrap_result;
     }
 
-    bool ok = message->file_descriptor_set()->Add(platform_handle.release().fd);
+    bool ok = message->file_descriptor_set()->AddToOwn(
+        base::ScopedFD(platform_handle.release().fd));
     DCHECK(ok);
   }
 
@@ -238,17 +239,20 @@ MojoResult ChannelMojo::WriteToFileDescriptorSet(
 
 // static
 MojoResult ChannelMojo::ReadFromFileDescriptorSet(
-    const Message& message,
+    Message* message,
     std::vector<MojoHandle>* handles) {
   // We dup() the handles in IPC::Message to transmit.
   // IPC::FileDescriptorSet has intricate lifecycle semantics
   // of FDs, so just to dup()-and-own them is the safest option.
-  if (message.HasFileDescriptors()) {
-    const FileDescriptorSet* fdset = message.file_descriptor_set();
-    for (size_t i = 0; i < fdset->size(); ++i) {
-      int fd_to_send = dup(fdset->GetDescriptorAt(i));
+  if (message->HasFileDescriptors()) {
+    FileDescriptorSet* fdset = message->file_descriptor_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) {
+      int fd_to_send = dup(fds_to_send[i]);
       if (-1 == fd_to_send) {
         DPLOG(WARNING) << "Failed to dup FD to transmit.";
+        fdset->CommitAll();
         return MOJO_RESULT_UNKNOWN;
       }
 
@@ -260,11 +264,14 @@ MojoResult ChannelMojo::ReadFromFileDescriptorSet(
       if (MOJO_RESULT_OK != wrap_result) {
         DLOG(WARNING) << "Pipe failed to wrap handles. Closing: "
                       << wrap_result;
+        fdset->CommitAll();
         return wrap_result;
       }
 
       handles->push_back(wrapped_handle);
     }
+
+    fdset->CommitAll();
   }
 
   return MOJO_RESULT_OK;
diff --git a/ipc/mojo/ipc_channel_mojo.h b/ipc/mojo/ipc_channel_mojo.h
index 9964a8f..e3ecb99 100644
--- a/ipc/mojo/ipc_channel_mojo.h
+++ b/ipc/mojo/ipc_channel_mojo.h
@@ -100,7 +100,7 @@ class IPC_MOJO_EXPORT ChannelMojo : public Channel,
   static MojoResult WriteToFileDescriptorSet(
       const std::vector<MojoHandle>& handle_buffer,
       Message* message);
-  static MojoResult ReadFromFileDescriptorSet(const Message& message,
+  static MojoResult ReadFromFileDescriptorSet(Message* message,
                                               std::vector<MojoHandle>* handles);
 
 #endif  // defined(OS_POSIX) && !defined(OS_NACL)
diff --git a/ipc/mojo/ipc_channel_mojo_readers.cc b/ipc/mojo/ipc_channel_mojo_readers.cc
index 7af96f8..227a946 100644
--- a/ipc/mojo/ipc_channel_mojo_readers.cc
+++ b/ipc/mojo/ipc_channel_mojo_readers.cc
@@ -138,7 +138,7 @@ bool MessageReader::Send(scoped_ptr<Message> message) {
   std::vector<MojoHandle> handles;
 #if defined(OS_POSIX) && !defined(OS_NACL)
   MojoResult read_result =
-      ChannelMojo::ReadFromFileDescriptorSet(*message, &handles);
+      ChannelMojo::ReadFromFileDescriptorSet(message.get(), &handles);
   if (read_result != MOJO_RESULT_OK) {
     std::for_each(handles.begin(), handles.end(), &MojoClose);
     CloseWithError(read_result);
diff --git a/ipc/mojo/ipc_channel_mojo_unittest.cc b/ipc/mojo/ipc_channel_mojo_unittest.cc
index 0763533..57a32a4 100644
--- a/ipc/mojo/ipc_channel_mojo_unittest.cc
+++ b/ipc/mojo/ipc_channel_mojo_unittest.cc
@@ -302,10 +302,11 @@ class ListenerThatExpectsFile : public IPC::Listener {
 
   virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE {
     PickleIterator iter(message);
-    base::FileDescriptor desc;
-    EXPECT_TRUE(message.ReadFileDescriptor(&iter, &desc));
+
+    base::ScopedFD fd;
+    EXPECT_TRUE(message.ReadFile(&iter, &fd));
+    base::File file(fd.release());
     std::string content(GetSendingFileContent().size(), ' ');
-    base::File file(desc.fd);
     file.Read(0, &content[0], content.size());
     EXPECT_EQ(content, GetSendingFileContent());
     base::MessageLoop::current()->Quit();
@@ -334,8 +335,7 @@ class ListenerThatExpectsFile : public IPC::Listener {
     file.Flush();
     IPC::Message* message = new IPC::Message(
         0, 2, IPC::Message::PRIORITY_NORMAL);
-    message->WriteFileDescriptor(
-        base::FileDescriptor(file.TakePlatformFile(), false));
+    message->WriteFile(base::ScopedFD(file.TakePlatformFile()));
     ASSERT_TRUE(sender->Send(message));
   }