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)
Tracking
()
RESOLVED
FIXED
mozilla63
People
(Reporter: bugzilla, Assigned: bugzilla)
References
(Regression)
Details
(Keywords: regression)
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
agashlin
:
review+
lizzard
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•6 years ago
|
||
Attachment #8993822 -
Flags: review?(agashlin)
Assignee | ||
Comment 2•6 years ago
|
||
Whoops, missed a file.
Attachment #8993822 -
Attachment is obsolete: true
Attachment #8993822 -
Flags: review?(agashlin)
Attachment #8993823 -
Flags: review?(agashlin)
Assignee | ||
Comment 3•6 years ago
|
||
Comment 4•6 years ago
|
||
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+
Assignee | ||
Comment 5•6 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/fef4321d9525fa0ba17dedc3085b246f9596e77b
Bug 1477402: Wrap WaitForInputIdle with checks for ERROR_NOT_GUI_PROCESS failures; r=agashlin
Comment 6•6 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Assignee | ||
Comment 7•6 years ago
|
||
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?
status-firefox62:
--- → affected
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+
Comment 9•6 years ago
|
||
bugherder uplift |
Updated•5 years ago
|
Keywords: regression
Updated•3 years ago
|
Has Regression Range: --- → yes
You need to log in
before you can comment on or make changes to this bug.
Description
•