Turn FileDescriptorInfo a collection class

content/browser/android/child_process_launcher_android.cc

 // Copyright (c) 2012 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 "content/browser/android/child_process_launcher_android.h"
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_array.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
@@ skipping 84 matching lines @@
   StartChildProcessCallback* callback =
       reinterpret_cast<StartChildProcessCallback*>(client_context);
   if (handle)
     callback->Run(static_cast<base::ProcessHandle>(handle));
   delete callback;
 }
 
 void StartChildProcess(
     const base::CommandLine::StringVector& argv,
     int child_process_id,
-    const std::vector<content::FileDescriptorInfo>& files_to_register,
+    scoped_ptr<content::FileDescriptorInfo> files_to_register,
     const StartChildProcessCallback& callback) {
   JNIEnv* env = AttachCurrentThread();
   DCHECK(env);
 
   // Create the Command line String[]
   ScopedJavaLocalRef<jobjectArray> j_argv = ToJavaArrayOfStrings(env, argv);
 
-  size_t file_count = files_to_register.size();
+  size_t file_count = files_to_register->GetMappingSize();
   DCHECK(file_count > 0);
 
   ScopedJavaLocalRef<jintArray> j_file_ids(env, env->NewIntArray(file_count));
   base::android::CheckException(env);
   jint* file_ids = env->GetIntArrayElements(j_file_ids.obj(), NULL);
   base::android::CheckException(env);
   ScopedJavaLocalRef<jintArray> j_file_fds(env, env->NewIntArray(file_count));
   base::android::CheckException(env);
   jint* file_fds = env->GetIntArrayElements(j_file_fds.obj(), NULL);
   base::android::CheckException(env);
   ScopedJavaLocalRef<jbooleanArray> j_file_auto_close(
       env, env->NewBooleanArray(file_count));
   base::android::CheckException(env);
   jboolean* file_auto_close =
       env->GetBooleanArrayElements(j_file_auto_close.obj(), NULL);
   base::android::CheckException(env);
   for (size_t i = 0; i < file_count; ++i) {
-    const content::FileDescriptorInfo& fd_info = files_to_register[i];
-    file_ids[i] = fd_info.id;
-    file_fds[i] = fd_info.fd.fd;
-    file_auto_close[i] = fd_info.fd.auto_close;
+    // Owners of passed descriptors can outlive this function and we don't know
+    // when it is safe to close() them. So we pass dup()-ed FD and
+    // let ChildProcessLauncher in java take care of their lifetimes.
+    // TODO(morrita): Drop FileDescriptorInfo.mAutoClose on Java side.
+    file_auto_close[i] = true;  // This indicates ownership transfer.
+    file_ids[i] = files_to_register->GetIDAt(i);
+    file_fds[i] = dup(files_to_register->GetFDAt(i));

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

Needing to dup() even file descriptors that are marked as auto_close seems
unfortunate.  That could maybe be fixed with a TakeOrDupFDAt() function, but I'm
not sure I like that either.  Shrug.

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

Well, lifetime management with different languages is tricky :-(
We could try to simplify FileDescriptInfo.java though.

+    CHECK(0 <= file_fds[i]);

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

FYI, dup() sets errno, so you can use PCHECK() to automatically include the
failure reason in the logs.

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

Done.

   }
   env->ReleaseIntArrayElements(j_file_ids.obj(), file_ids, 0);
   env->ReleaseIntArrayElements(j_file_fds.obj(), file_fds, 0);
   env->ReleaseBooleanArrayElements(j_file_auto_close.obj(), file_auto_close, 0);
 
   Java_ChildProcessLauncher_start(env,
       base::android::GetApplicationContext(),
       j_argv.obj(),
       child_process_id,
       j_file_ids.obj(),
@@ skipping 68 matching lines @@
 jboolean IsSingleProcess(JNIEnv* env, jclass clazz) {
   return base::CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kSingleProcess);
 }
 
 bool RegisterChildProcessLauncher(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }
 
 }  // namespace content