Bug 1330496 - Part 1: Remove MOZ_WIN_INHERIT_STD_HANDLES_PRE_VISTA support for inheriting stdout/stderr handles on XP. r=bobowen
authorChris Peterson <cpeterson@mozilla.com>
Tue, 10 Jan 2017 23:50:16 -0800
changeset 374457 605d3b2bd202e7b41e9f6c8ed7a8ce55ad49ff09
parent 374456 9735ab23b94303279482161008ae06b6528fa32b
child 374458 5515c1da32d9ac31948fb44690133afd30bf7491
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbobowen
bugs1330496
milestone53.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1330496 - Part 1: Remove MOZ_WIN_INHERIT_STD_HANDLES_PRE_VISTA support for inheriting stdout/stderr handles on XP. r=bobowen MozReview-Commit-ID: B7qJdK2sjv5
ipc/chromium/src/base/process_util_win.cc
security/sandbox/chromium/sandbox/win/src/broker_services.cc
security/sandbox/modifications-to-chromium-to-reapply-after-upstream-merge.txt
--- a/ipc/chromium/src/base/process_util_win.cc
+++ b/ipc/chromium/src/base/process_util_win.cc
@@ -14,19 +14,16 @@
 #include <winternl.h>
 #include <psapi.h>
 
 #include "base/histogram.h"
 #include "base/logging.h"
 #include "base/win_util.h"
 
 #include <algorithm>
-#include "prenv.h"
-
-#include "mozilla/WindowsVersion.h"
 
 namespace {
 
 // System pagesize. This value remains constant on x86/64 architectures.
 const int PAGESIZE_KB = 4;
 
 // HeapSetInformation function pointer.
 typedef BOOL (WINAPI* HeapSetFn)(HANDLE, HEAP_INFORMATION_CLASS, PVOID, SIZE_T);
@@ -275,72 +272,59 @@ void FreeThreadAttributeList(LPPROC_THRE
 }
 
 bool LaunchApp(const std::wstring& cmdline,
                bool wait, bool start_hidden, ProcessHandle* process_handle) {
 
   // We want to inherit the std handles so dump() statements and assertion
   // messages in the child process can be seen - but we *do not* want to
   // blindly have all handles inherited.  Vista and later has a technique
-  // where only specified handles are inherited - so we use this technique if
-  // we can.  If that technique isn't available (or it fails), we just don't
-  // inherit anything.  This can cause us a problem for Windows XP testing,
-  // because we sometimes need the handles to get inherited for test logging to
-  // work. So we also inherit when a specific environment variable is set.
+  // where only specified handles are inherited - so we use this technique.
+  // If that fails we just don't inherit anything.
   DWORD dwCreationFlags = 0;
   BOOL bInheritHandles = FALSE;
+
   // We use a STARTUPINFOEX, but if we can't do the thread attribute thing, we
   // just pass the size of a STARTUPINFO.
   STARTUPINFOEX startup_info_ex;
   ZeroMemory(&startup_info_ex, sizeof(startup_info_ex));
   STARTUPINFO &startup_info = startup_info_ex.StartupInfo;
   startup_info.cb = sizeof(startup_info);
   startup_info.dwFlags = STARTF_USESHOWWINDOW;
   startup_info.wShowWindow = start_hidden ? SW_HIDE : SW_SHOW;
 
   // Per the comment in CreateThreadAttributeList, lpAttributeList will contain
   // a pointer to handlesToInherit, so make sure they have the same lifetime.
   LPPROC_THREAD_ATTRIBUTE_LIST lpAttributeList = NULL;
   HANDLE handlesToInherit[2];
   int handleCount = 0;
 
-  // Don't even bother trying pre-Vista...
-  if (mozilla::IsVistaOrLater()) {
-    // setup our handle array first - if we end up with no handles that can
-    // be inherited we can avoid trying to do the ThreadAttributeList dance...
-    HANDLE stdOut = ::GetStdHandle(STD_OUTPUT_HANDLE);
-    HANDLE stdErr = ::GetStdHandle(STD_ERROR_HANDLE);
+  // setup our handle array first - if we end up with no handles that can
+  // be inherited we can avoid trying to do the ThreadAttributeList dance...
+  HANDLE stdOut = ::GetStdHandle(STD_OUTPUT_HANDLE);
+  HANDLE stdErr = ::GetStdHandle(STD_ERROR_HANDLE);
 
-    if (IsInheritableHandle(stdOut))
-      handlesToInherit[handleCount++] = stdOut;
-    if (stdErr != stdOut && IsInheritableHandle(stdErr))
-      handlesToInherit[handleCount++] = stdErr;
+  if (IsInheritableHandle(stdOut))
+    handlesToInherit[handleCount++] = stdOut;
+  if (stdErr != stdOut && IsInheritableHandle(stdErr))
+    handlesToInherit[handleCount++] = stdErr;
 
-    if (handleCount) {
-      lpAttributeList = CreateThreadAttributeList(handlesToInherit, handleCount);
-      if (lpAttributeList) {
-        // it's safe to inherit handles, so arrange for that...
-        startup_info.cb = sizeof(startup_info_ex);
-        startup_info.dwFlags |= STARTF_USESTDHANDLES;
-        startup_info.hStdOutput = stdOut;
-        startup_info.hStdError = stdErr;
-        startup_info.hStdInput = INVALID_HANDLE_VALUE;
-        startup_info_ex.lpAttributeList = lpAttributeList;
-        dwCreationFlags |= EXTENDED_STARTUPINFO_PRESENT;
-        bInheritHandles = TRUE;
-      }
+  if (handleCount) {
+    lpAttributeList = CreateThreadAttributeList(handlesToInherit, handleCount);
+    if (lpAttributeList) {
+      // it's safe to inherit handles, so arrange for that...
+      startup_info.cb = sizeof(startup_info_ex);
+      startup_info.dwFlags |= STARTF_USESTDHANDLES;
+      startup_info.hStdOutput = stdOut;
+      startup_info.hStdError = stdErr;
+      startup_info.hStdInput = INVALID_HANDLE_VALUE;
+      startup_info_ex.lpAttributeList = lpAttributeList;
+      dwCreationFlags |= EXTENDED_STARTUPINFO_PRESENT;
+      bInheritHandles = TRUE;
     }
-  } else if (PR_GetEnv("MOZ_WIN_INHERIT_STD_HANDLES_PRE_VISTA")) {
-    // Even if we can't limit what gets inherited, we sometimes want to inherit
-    // stdout/err for testing purposes.
-    startup_info.dwFlags |= STARTF_USESTDHANDLES;
-    startup_info.hStdOutput = ::GetStdHandle(STD_OUTPUT_HANDLE);
-    startup_info.hStdError = ::GetStdHandle(STD_ERROR_HANDLE);
-    startup_info.hStdInput = INVALID_HANDLE_VALUE;
-    bInheritHandles = TRUE;
   }
 
   PROCESS_INFORMATION process_info;
   BOOL createdOK = CreateProcess(NULL,
                      const_cast<wchar_t*>(cmdline.c_str()), NULL, NULL,
                      bInheritHandles, dwCreationFlags, NULL, NULL,
                      &startup_info, &process_info);
   if (lpAttributeList)
--- a/security/sandbox/chromium/sandbox/win/src/broker_services.cc
+++ b/security/sandbox/chromium/sandbox/win/src/broker_services.cc
@@ -404,24 +404,16 @@ ResultCode BrokerServicesBase::SpawnTarg
       startup_info.startup_info()->dwFlags |= STARTF_USESTDHANDLES;
       startup_info.startup_info()->hStdInput = INVALID_HANDLE_VALUE;
       startup_info.startup_info()->hStdOutput = stdout_handle;
       startup_info.startup_info()->hStdError = stderr_handle;
       // Allowing inheritance of handles is only secure now that we
       // have limited which handles will be inherited.
       inherit_handles = true;
     }
-  } else if (getenv("MOZ_WIN_INHERIT_STD_HANDLES_PRE_VISTA")) {
-    // On pre-Vista versions even if we can't limit what gets inherited, we
-    // sometimes want to inherit stdout/err for testing purposes.
-    startup_info.startup_info()->dwFlags |= STARTF_USESTDHANDLES;
-    startup_info.startup_info()->hStdInput = INVALID_HANDLE_VALUE;
-    startup_info.startup_info()->hStdOutput = policy_base->GetStdoutHandle();
-    startup_info.startup_info()->hStdError = policy_base->GetStderrHandle();
-    inherit_handles = true;
   }
 
   // Construct the thread pool here in case it is expensive.
   // The thread pool is shared by all the targets
   if (NULL == thread_pool_)
     thread_pool_ = new Win2kThreadPool();
 
   // Create the TargetProces object and spawn the target suspended. Note that
--- a/security/sandbox/modifications-to-chromium-to-reapply-after-upstream-merge.txt
+++ b/security/sandbox/modifications-to-chromium-to-reapply-after-upstream-merge.txt
@@ -1,9 +1,8 @@
 Please add a link to the bugzilla bug and patch name that should be re-applied.
 Also, please update any existing links to their actual mozilla-central changeset.
 
-https://bugzilla.mozilla.org/show_bug.cgi?id=1287426 bug1287426part4.patch
 https://bugzilla.mozilla.org/show_bug.cgi?id=1287426 bug1287426part5.patch
 https://hg.mozilla.org/mozilla-central/rev/7df8d6639971
 https://bugzilla.mozilla.org/show_bug.cgi?id=1287426 bug1287426part7.patch
 https://bugzilla.mozilla.org/show_bug.cgi?id=1273372 bug1273372part2.patch
 https://bugzilla.mozilla.org/show_bug.cgi?id=1273372 bug1273372part3.patch