Turn FileDescriptorInfo a collection class

content/browser/zygote_host/zygote_host_impl_linux.cc

Index: content/browser/zygote_host/zygote_host_impl_linux.cc
diff --git a/content/browser/zygote_host/zygote_host_impl_linux.cc b/content/browser/zygote_host/zygote_host_impl_linux.cc
index 0ead4ce1c33d9005adfed3a95abc88a823ec7b46..ad63ff3aa2df1210602c24bc45398bb7e0135bd3 100644
--- a/content/browser/zygote_host/zygote_host_impl_linux.cc
+++ b/content/browser/zygote_host/zygote_host_impl_linux.cc
@@ -287,10 +287,9 @@ ssize_t ZygoteHostImpl::ReadReply(void* buf, size_t buf_len) {
   return HANDLE_EINTR(read(control_fd_, buf, buf_len));
 }
 
-pid_t ZygoteHostImpl::ForkRequest(
-    const std::vector<std::string>& argv,
-    const std::vector<FileDescriptorInfo>& mapping,
-    const std::string& process_type) {
+pid_t ZygoteHostImpl::ForkRequest(const std::vector<std::string>& argv,
+                                  scoped_ptr<FileDescriptorInfo> mapping,
+                                  const std::string& process_type) {
   DCHECK(init_);
   Pickle pickle;
 
@@ -309,26 +308,19 @@ pid_t ZygoteHostImpl::ForkRequest(
 
   // Fork requests contain one file descriptor for the PID oracle, and one
   // more for each file descriptor mapping for the child process.
-  const size_t num_fds_to_send = 1 + mapping.size();
+  const size_t num_fds_to_send = 1 + mapping->GetMappingSize();
   pickle.WriteInt(num_fds_to_send);
 
   std::vector<int> fds;
-  ScopedVector<base::ScopedFD> autoclose_fds;
 
   // First FD to send is peer_sock.
   fds.push_back(peer_sock.get());
-  autoclose_fds.push_back(new base::ScopedFD(peer_sock.Pass()));
+  mapping->TransferWithoutMapping(peer_sock.Pass());

[mdempsky, 2014/09/24 21:16:44]

This feels a little awkward to me actually.  It seems like a pretty special case
operation, so users otherwise familiar with FileDescriptorInfo probably won't
know what it does off hand. It also means ForkRequest() has to modify the
mapping it's given, not just use it as input.

I'd probably suggest going back to just manually calling peer_sock.reset() below
next to mapping.reset().  That will be the same number of lines here, but it
means you can simplify FileDescriptorInfo's interface/implementation somewhat.

[Hajime Morrita, 2014/09/24 23:08:56]

OK.

Ideally, we should change the zygote IPC protocol to simplify this. But I don't
want to have such a tricky change with this...

 
   // The rest come from mapping.
-  for (std::vector<FileDescriptorInfo>::const_iterator
-       i = mapping.begin(); i != mapping.end(); ++i) {
-    pickle.WriteUInt32(i->id);
-    fds.push_back(i->fd.fd);
-    if (i->fd.auto_close) {
-      // Auto-close means we need to close the FDs after they have been passed
-      // to the other process.
-      autoclose_fds.push_back(new base::ScopedFD(i->fd.fd));
-    }
+  for (size_t i = 0; i < mapping->GetMappingSize(); ++i) {
+    pickle.WriteUInt32(mapping->GetIDAt(i));
+    fds.push_back(mapping->GetFDAt(i));
   }
 
   // Sanity check that we've populated |fds| correctly.
@@ -339,7 +331,7 @@ pid_t ZygoteHostImpl::ForkRequest(
     base::AutoLock lock(control_lock_);
     if (!SendMessage(pickle, &fds))
       return base::kNullProcessHandle;
-    autoclose_fds.clear();
+    mapping.reset();
 
     {
       char buf[sizeof(kZygoteChildPingMessage) + 1];