Improve process launch handle sharing API.

base/process/launch.h

Index: base/process/launch.h
diff --git a/base/process/launch.h b/base/process/launch.h
index 00e85f58f1a1db4f3334dc6443aab4a047cc0264..59a2807b9147a82584dd75c63870778dc9f5d027 100644
--- a/base/process/launch.h
+++ b/base/process/launch.h
@@ -35,13 +35,13 @@ namespace base {
 
 class CommandLine;
 
-#if defined(OS_WIN)
+#if defined(OS_POSIX)
+typedef std::vector<std::pair<int, int>> FileHandleMappingVector;

[Joe Mason, 2017/07/04 18:30:34]

From the CL desc: "Name base::HandleToInheritVector only defined on Posix."

That seems to be the opposite of what the code says here. HandlesToInheritVector
is NOT defined on POSIX, but FileHandleMappingVector IS.

[brettw, 2017/07/05 19:13:51]

Will update the CL description. I put the vector definition back after the
FUCHSIA thing landed in the middle of this patch.

+#elif defined(OS_WIN)
 typedef std::vector<HANDLE> HandlesToInheritVector;
 #elif defined(OS_FUCHSIA)

[Joe Mason, 2017/07/04 18:26:54]

Is OS_POSIX also defined when OS_FUCHSIA is? (I can't imagine OS_WIN overlaps
with OS_FUCHSIA...)

If so, this #elif will never be hit. But if not I don't see how the "#else  //
!defined(OS_WIN)" below can work since FileHandleMappingVector is used in that
block.

[brettw, 2017/07/05 19:13:51]

Looks like it is, I reordered.

 typedef std::vector<mx_handle_t> HandlesToInheritVector;
 #endif
-// TODO(viettrungluu): Only define this on POSIX?
-typedef std::vector<std::pair<int, int> > FileHandleMappingVector;
 
 // Options for launching a subprocess that are passed to LaunchProcess().
 // The default constructor constructs the object with default options.
@@ -77,17 +77,31 @@ struct BASE_EXPORT LaunchOptions {
 #if defined(OS_WIN)
   bool start_hidden = false;
 
-  // If non-null, inherit exactly the list of handles in this vector (these
-  // handles must be inheritable).
-  HandlesToInheritVector* handles_to_inherit = nullptr;
-
-  // If true, the new process inherits handles from the parent. In production
-  // code this flag should be used only when running short-lived, trusted
-  // binaries, because open handles from other libraries and subsystems will
-  // leak to the child process, causing errors such as open socket hangs.
-  // Note: If |handles_to_inherit| is non-null, this flag is ignored and only
-  // those handles will be inherited.
-  bool inherit_handles = false;
+  // Windows can inherit handles when it launches child processes.
+  // See https://blogs.msdn.microsoft.com/oldnewthing/20111216-00/?p=8873
+  // for a good overview of Windows handle inheritance.
+  //
+  // Implementation note: it might be nice to implement in terms of
+  // base::Optional<>, but then the natural default state (vector not present)
+  // would be "all inheritable handles" while we want "no inheritance."
+  enum class Inherit {
+    // Only those handles in |handles_to_inherit| vector are inherited. If the
+    // vector is empty, no handles are inherited. The handles in the vector must
+    // all be inheritable.
+    kSpecific,
+
+    // All handles in the current process which are inheritable are inherited.
+    // In production code this flag should be used only when running
+    // short-lived, trusted binaries, because open handles from other libraries
+    // and subsystems will leak to the child process, causing errors such as
+    // open socket hangs. There are also race conditions that can cause handle
+    // over-sharing.
+    //
+    // |handles_to_inherit| must be null.
+    kAll
+  };
+  Inherit inherit_mode = Inherit::kSpecific;
+  HandlesToInheritVector handles_to_inherit;
 
   // If non-null, runs as if the user represented by the token had launched it.
   // Whether the application is visible on the interactive desktop depends on
@@ -106,10 +120,16 @@ struct BASE_EXPORT LaunchOptions {
   // the job object fails.
   HANDLE job_handle = nullptr;
 
-  // Handles for the redirection of stdin, stdout and stderr. The handles must
-  // be inheritable. Caller should either set all three of them or none (i.e.
-  // there is no way to redirect stderr without redirecting stdin). The
-  // |inherit_handles| flag must be set to true when redirecting stdio stream.
+  // Handles for the redirection of stdin, stdout and stderr. The caller should
+  // either set all three of them or none (i.e. there is no way to redirect
+  // stderr without redirecting stdin).
+  //
+  // The handles must be inheritable. Pseudo handles are used when stdout and
+  // stderr redirect to the console. In that case, GetFileType() will return
+  // FILE_TYPE_CHAR and they're automatically inherited by child processes. See
+  // https://msdn.microsoft.com/en-us/library/windows/desktop/ms682075.aspx
+  // Otherwise, the caller must ensure that the |inherit_mode| and/or
+  // |handles_to_inherit| set so that the handles are inherited.
   HANDLE stdin_handle = nullptr;
   HANDLE stdout_handle = nullptr;
   HANDLE stderr_handle = nullptr;
@@ -128,11 +148,9 @@ struct BASE_EXPORT LaunchOptions {
   // |environ|.
   bool clear_environ = false;
 
-  // If non-null, remap file descriptors according to the mapping of
-  // src fd->dest fd to propagate FDs into the child process.
-  // This pointer is owned by the caller and must live through the
-  // call to LaunchProcess().
-  const FileHandleMappingVector* fds_to_remap = nullptr;
+  // Remap file descriptors according to the mapping of src_fd->dest_fd to
+  // propagate FDs into the child process.
+  FileHandleMappingVector fds_to_remap;
 
   // Each element is an RLIMIT_* constant that should be raised to its
   // rlim_max.  This pointer is owned by the caller and must live through