Bug 1477402: Wrap WaitForInputIdle with checks for ERROR_NOT_GUI_PROCESS failures; r=agashlin a=lizzard
authorAaron Klotz <aklotz@mozilla.com>
Fri, 20 Jul 2018 15:54:08 -0600
changeset 480645 50dbda5a6c9be4d5935fe0a9d4bf40ea7e5c129f
parent 480644 75b79018fff9e0bf1eaeebec13290aed2a19ee90
child 480646 9913c644c5c01069ccabd0d678954915502c2cff
push id1757
push userffxbld-merge
push dateFri, 24 Aug 2018 17:02:43 +0000
treeherdermozilla-release@736023aebdb1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersagashlin, lizzard
bugs1477402
milestone62.0
Bug 1477402: Wrap WaitForInputIdle with checks for ERROR_NOT_GUI_PROCESS failures; r=agashlin a=lizzard widget/windows/WinUtils.h is getting unwieldy and contains a combination of both header-only and non-header-only code. I thought I'd take the opportunity with this patch to create a new file for self-contained, header-only utility functions, with the hope that we can eventually migrate some stuff out of WinUtils into WinHeaderOnlyUtils in the future.
browser/app/winlauncher/LauncherProcessWin.cpp
toolkit/mozapps/update/updater/updater.cpp
toolkit/xre/nsAppRunner.cpp
widget/windows/WinHeaderOnlyUtils.h
widget/windows/moz.build
xpcom/base/nsWindowsHelpers.h
--- a/browser/app/winlauncher/LauncherProcessWin.cpp
+++ b/browser/app/winlauncher/LauncherProcessWin.cpp
@@ -11,16 +11,17 @@
 #include "mozilla/Attributes.h"
 #include "mozilla/CmdLineAndEnvUtils.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/DynamicallyLinkedFunctionPtr.h"
 #include "mozilla/Maybe.h"
 #include "mozilla/SafeMode.h"
 #include "mozilla/UniquePtr.h"
 #include "mozilla/WindowsVersion.h"
+#include "mozilla/WinHeaderOnlyUtils.h"
 #include "nsWindowsHelpers.h"
 
 #include <windows.h>
 #include <processthreadsapi.h>
 
 #include "DllBlocklistWin.h"
 #include "LaunchUnelevated.h"
 #include "ProcThreadAttributes.h"
@@ -214,17 +215,20 @@ LauncherMain(int argc, wchar_t* argv[])
 
   if (!PostCreationSetup(process.get(), mainThread.get(), isSafeMode.value()) ||
       ::ResumeThread(mainThread.get()) == static_cast<DWORD>(-1)) {
     ShowError();
     ::TerminateProcess(process.get(), 1);
     return 1;
   }
 
+  const DWORD timeout = ::IsDebuggerPresent() ? INFINITE :
+                        kWaitForInputIdleTimeoutMS;
+
   // Keep the current process around until the callback process has created
   // its message queue, to avoid the launched process's windows being forced
   // into the background.
-  ::WaitForInputIdle(process.get(), kWaitForInputIdleTimeoutMS);
+  mozilla::WaitForInputIdle(process.get(), timeout);
 
   return 0;
 }
 
 } // namespace mozilla
--- a/toolkit/mozapps/update/updater/updater.cpp
+++ b/toolkit/mozapps/update/updater/updater.cpp
@@ -50,16 +50,19 @@
 #include "updatecommon.h"
 #ifdef XP_MACOSX
 #include "updaterfileutils_osx.h"
 #endif // XP_MACOSX
 
 #include "mozilla/Compiler.h"
 #include "mozilla/Types.h"
 #include "mozilla/UniquePtr.h"
+#ifdef XP_WIN
+#include "mozilla/WinHeaderOnlyUtils.h"
+#endif // XP_WIN
 
 // Amount of the progress bar to use in each of the 3 update stages,
 // should total 100.0.
 #define PROGRESS_PREPARE_SIZE 20.0f
 #define PROGRESS_EXECUTE_SIZE 75.0f
 #define PROGRESS_FINISH_SIZE   5.0f
 
 // Maximum amount of time in ms to wait for the parent process to close. The 30
@@ -2098,17 +2101,17 @@ LaunchCallbackApp(const NS_tchar *workin
   // Do not allow the callback to run when running an update through the
   // service as session 0.  The unelevated updater.exe will do the launching.
   if (!usingService) {
     HANDLE hProcess;
     if (WinLaunchChild(argv[0], argc, argv, nullptr, &hProcess)) {
       // Keep the current process around until the callback process has created
       // its message queue, to avoid the launched process's windows being forced
       // into the background.
-      WaitForInputIdle(hProcess, kWaitForInputIdleTimeoutMS);
+      mozilla::WaitForInputIdle(hProcess);
       CloseHandle(hProcess);
     }
   }
 #else
 # warning "Need implementaton of LaunchCallbackApp"
 #endif
 }
 
--- a/toolkit/xre/nsAppRunner.cpp
+++ b/toolkit/xre/nsAppRunner.cpp
@@ -99,16 +99,17 @@
 
 #ifdef XP_WIN
 #include "nsIWinAppHelper.h"
 #include <windows.h>
 #include <intrin.h>
 #include <math.h>
 #include "cairo/cairo-features.h"
 #include "mozilla/WindowsDllBlocklist.h"
+#include "mozilla/WinHeaderOnlyUtils.h"
 #include "mozilla/mscom/MainThreadRuntime.h"
 #include "mozilla/widget/AudioSession.h"
 
 #ifndef PROCESS_DEP_ENABLE
 #define PROCESS_DEP_ENABLE 0x1
 #endif
 #endif
 
@@ -1850,17 +1851,17 @@ static nsresult LaunchChild(nsINativeApp
     return rv;
 
   HANDLE hProcess;
   if (!WinLaunchChild(exePath.get(), gRestartArgc, gRestartArgv, nullptr, &hProcess))
     return NS_ERROR_FAILURE;
   // Keep the current process around until the restarted process has created
   // its message queue, to avoid the launched process's windows being forced
   // into the background.
-  ::WaitForInputIdle(hProcess, kWaitForInputIdleTimeoutMS);
+  mozilla::WaitForInputIdle(hProcess);
   ::CloseHandle(hProcess);
 
 #else
   nsAutoCString exePath;
   rv = lf->GetNativePath(exePath);
   if (NS_FAILED(rv))
     return rv;
 
new file mode 100644
--- /dev/null
+++ b/widget/windows/WinHeaderOnlyUtils.h
@@ -0,0 +1,68 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
+
+#ifndef mozilla_WinHeaderOnlyUtils_h
+#define mozilla_WinHeaderOnlyUtils_h
+
+#include <windows.h>
+
+/**
+ * This header is intended for self-contained, header-only, utility code for
+ * Win32. It may be used outside of xul.dll, in places such as firefox.exe or
+ * mozglue.dll. If your code creates dependencies on Mozilla libraries, you
+ * should put it elsewhere.
+ */
+
+namespace mozilla {
+
+// How long to wait for a created process to become available for input,
+// to prevent that process's windows being forced to the background.
+// This is used across update, restart, and the launcher.
+const DWORD kWaitForInputIdleTimeoutMS = 10*1000;
+
+/**
+ * Wait for a child GUI process to become "idle." Idle means that the process
+ * has created its message queue and has begun waiting for user input.
+ *
+ * Note that this must only be used when the child process is going to display
+ * GUI! Otherwise you're going to be waiting for a very long time ;-)
+ *
+ * @return true if we successfully waited for input idle;
+ *         false if we timed out or failed to wait.
+ */
+inline bool
+WaitForInputIdle(HANDLE aProcess, DWORD aTimeoutMs = kWaitForInputIdleTimeoutMS)
+{
+  const DWORD kSleepTimeMs = 10;
+  const DWORD waitStart = aTimeoutMs == INFINITE ? 0 : ::GetTickCount();
+  DWORD elapsed = 0;
+
+  while (true) {
+    if (aTimeoutMs != INFINITE) {
+      elapsed = ::GetTickCount() - waitStart;
+    }
+
+    if (elapsed >= aTimeoutMs) {
+      return false;
+    }
+
+    DWORD waitResult = ::WaitForInputIdle(aProcess, aTimeoutMs - elapsed);
+    if (!waitResult) {
+      return true;
+    }
+
+    if (waitResult == WAIT_FAILED && ::GetLastError() == ERROR_NOT_GUI_PROCESS) {
+      ::Sleep(kSleepTimeMs);
+      continue;
+    }
+
+    return false;
+  }
+}
+
+} // namespace mozilla
+
+#endif // mozilla_WinHeaderOnlyUtils_h
--- a/widget/windows/moz.build
+++ b/widget/windows/moz.build
@@ -13,16 +13,20 @@ with Files("*CompositorWidget*"):
 TEST_DIRS += ['tests']
 
 EXPORTS += [
     'nsdefs.h',
     'WindowHook.h',
     'WinUtils.h',
 ]
 
+EXPORTS.mozilla += [
+    'WinHeaderOnlyUtils.h',
+]
+
 EXPORTS.mozilla.widget += [
     'AudioSession.h',
     'CompositorWidgetChild.h',
     'CompositorWidgetParent.h',
     'InProcessWinCompositorWidget.h',
     'PDFiumChild.h',
     'PDFiumEngineShim.h',
     'PDFiumParent.h',
--- a/xpcom/base/nsWindowsHelpers.h
+++ b/xpcom/base/nsWindowsHelpers.h
@@ -363,14 +363,9 @@ LoadLibrarySystem32(LPCWSTR aModule)
   if (!ConstructSystem32Path(aModule, systemPath, MAX_PATH + 1)) {
     return NULL;
   }
   return LoadLibraryW(systemPath);
 }
 
 }
 
-// How long to wait for a created process to become available for input,
-// to prevent that process's windows being forced to the background.
-// This is used across update, restart, and the launcher.
-const DWORD kWaitForInputIdleTimeoutMS = 10*1000;
-
 #endif