Closed Bug 1477402 Opened 6 years ago Closed 6 years ago

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: bugzilla, Assigned: bugzilla)

References

(Regression)

Details

(Keywords: regression)

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.
Attached patch Create and use a wrapper for WaitForInputIdle (obsolete) (deleted) — Splinter Review
Attachment #8993822 - Flags: review?(agashlin)
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
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
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-
Keywords: regression
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: