Closed Bug 1477402 Opened Last year Closed Last year

WaitForInputIdle may return WAIT_FAILED in timing-sensitive cases

Categories

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

Unspecified
Windows
defect

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox62 --- fixed
firefox63 --- fixed

People

(Reporter: aklotz, Assigned: aklotz)

References

Details

Attachments

(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):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=910aaa2b6870e1104d149eb1d4abaa2b19029df4
Attachment #8993823 - Flags: review?(agashlin) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/fef4321d9525fa0ba17dedc3085b246f9596e77b
Bug 1477402: Wrap WaitForInputIdle with checks for ERROR_NOT_GUI_PROCESS failures; r=agashlin
https://hg.mozilla.org/mozilla-central/rev/fef4321d9525
Status: ASSIGNED → RESOLVED
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.