Closed Bug 1477402 Opened Last year Closed Last year

WaitForInputIdle may return WAIT_FAILED in timing-sensitive cases


(Core :: Widget: Win32, defect, P1)




Tracking Status
firefox62 --- fixed
firefox63 --- fixed


(Reporter: aklotz, Assigned: aklotz)




(1 file, 1 obsolete file)

I was encountering some problems with the launcher process, and it turns out that if the message queue isn't created yet, WaitForInputIdle can return WAIT_FAILED with last error == ERROR_NOT_GUI_PROCESS.

I think we still want to use that API for waiting for a child process to be ready (and we know that this works when the function actually succeeds), so I'd like to just create a wrapper for it that monitors for that condition and does a brief sleep before retrying.
Whoops, missed a file.
Attachment #8993822 - Attachment is obsolete: true
Attachment #8993822 - Flags: review?(agashlin)
Attachment #8993823 - Flags: review?(agashlin)
Comment on attachment 8993823 [details] [diff] [review]
Create and use a wrapper for WaitForInputIdle (r2)

Looks good, a few nits:

> r=mhowell
Just in case...

> -  DWORD timeout = ::IsDebuggerPresent() ? INFINITE : kWaitForInputIdleTimeoutMS;
This isn't in mozilla-central to be patched out, at the moment.

> +inline bool
> +WaitForInputIdle(HANDLE aProcess, DWORD aTimeoutMs = kWaitForInputIdleTimeoutMS)
I know we're not using it currently, but the return value for mozilla::WaitForInputIdle is now opposite the sense of ::WaitForInputIdle (false for failure instead of 0 for wait succeeded), it might be helpful to document it.

I ran it through try (with a manual patch for the IsDebuggerPresent line):
Attachment #8993823 - Flags: review?(agashlin) → review+
Bug 1477402: Wrap WaitForInputIdle with checks for ERROR_NOT_GUI_PROCESS failures; r=agashlin
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Comment on attachment 8993823 [details] [diff] [review]
Create and use a wrapper for WaitForInputIdle (r2)

Approval Request Comment
[Feature/Bug causing the regression]: bug 1451366
[User impact if declined]: Some windows might not be shown in the foreground when a new Firefox process is started by the updater or by the profile manager
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: None
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Really simple patch, verified in Nightly
[String changes made/needed]: None
Attachment #8993823 - Flags: approval-mozilla-beta?
Comment on attachment 8993823 [details] [diff] [review]
Create and use a wrapper for WaitForInputIdle (r2)

Fix for regression from 62, verified in nightly, let's uplift for beta 13.
Attachment #8993823 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Marking qe- based on c#7.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.