Fix ProcessUtilTest.FDRemapping on Android.

base/test/multiprocess_test_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 "base/test/multiprocess_test.h"
 
 #include <unistd.h>
 
+#include "base/hash_tables.h"
 #include "base/logging.h"
+#include "base/posix/eintr_wrapper.h"
 #include "base/process.h"
 #include "testing/multiprocess_func_list.h"
 
 namespace base {
 
 // A very basic implementation for android. On Android tests can run in an APK
 // and we don't have an executable to exec*. This implementation does the bare
 // minimum to execute the method specified by procname (in the child process).
-//  - File descriptors are not closed and hence |fds_to_map| is ignored.
 //  - |debug_on_start| is ignored.
 ProcessHandle MultiProcessTest::SpawnChildImpl(
     const std::string& procname,
-    const FileHandleMappingVector& fds_to_map,
+    const FileHandleMappingVector& fds_to_remap,
     bool debug_on_start) {
   pid_t pid = fork();
 
   if (pid < 0) {
-    DPLOG(ERROR) << "fork";
+    PLOG(ERROR) << "fork";
     return kNullProcessHandle;
-  } else if (pid == 0) {
-    // Child process.
-    _exit(multi_process_function_list::InvokeChildProcessTest(procname));
   }
-
-  // Parent process.
-  return pid;
+  if (pid > 0)
+    // Parent process.

[Paweł Hajdan Jr., 2012/11/26 18:18:41]

nit: If you lay it out like that use braces {} for the "if".

[Philippe, 2012/11/26 18:30:44]

Done.

+    return pid;
+  // Child process.
+  std::hash_set<int> fds_to_keep_open;
+  for (FileHandleMappingVector::const_iterator it = fds_to_remap.begin();
+       it != fds_to_remap.end(); ++it) {
+    fds_to_keep_open.insert(it->first);
+  }
+  // Keep stdin, stdout and stderr open since this is not meant to spawn a
+  // daemon.
+  const int kFdForAndroidLogging = 3;  // FD used by __android_log_write().
+  for (int fd = kFdForAndroidLogging + 1; fd < getdtablesize(); ++fd)
+    if (fds_to_keep_open.find(fd) == fds_to_keep_open.end())
+      HANDLE_EINTR(close(fd));

[Paweł Hajdan Jr., 2012/11/26 18:18:41]

nit: This is a macro, so use {} around it.

[Philippe, 2012/11/26 18:30:44]

I think that such macros should be written in a safe way (e.g. by using do {}
while (0)) so that they don't cause subtle bugs but I agree that it's better not
to take any risk.

+  for (FileHandleMappingVector::const_iterator it = fds_to_remap.begin();
+       it != fds_to_remap.end(); ++it) {
+    const int old_fd = it->first;

[Paweł Hajdan Jr., 2012/11/26 18:18:41]

nit: No need for "const int" if this is not really a constant.

[Philippe, 2012/11/26 18:30:44]

Done.

+    const int new_fd = it->second;
+    if (dup2(old_fd, new_fd) < 0) {
+      PLOG(ERROR) << "dup2";

[Paweł Hajdan Jr., 2012/11/26 18:18:41]

Why do you make that non-fatal?

[Philippe, 2012/11/26 18:30:44]

Done.

+      continue;
+    }
+    HANDLE_EINTR(close(old_fd));
+  }
+  _exit(multi_process_function_list::InvokeChildProcessTest(procname));
+  return 0;
 }
 
 }  // namespace base