1879608 - Parent windows may lock up if embedded Gecko window is reparented while file dialog is open

Status: RESOLVED WONTFIX

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

Description

[Ray Kraesig [:rkraesig]]

This (mostly-theoretical) concern was discussed in D200742.

With the move to async, it becomes slightly less theoretical: a host program can't guarantee that a Gecko window which it plans to reparent isn't about to open a file dialog. (The Windows file dialog might or might not store the window it disabled so as to correctly reenable that window on exit; but if the file dialog crashes, the question is moot.)

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1858225

Comment 2

[Ray Kraesig [:rkraesig]]

Attachment: Bug 1879608 - [1/1] also manually disable the root window when opening a file-dialog r?#win-reviewers

Eliminate a possible race-condition wherein multiple file-dialogs fight
over disabling the root window, possibly reenabling it early.

Comment 3

[Ray Kraesig [:rkraesig]]

On reanalysis, my initial patch didn't constitute a solution... and can't really be levered into becoming one.

I don't see a way to solve this, as written, without either a) an unenforceable requirement on the host application to ask before reparenting ancestors of a Gecko window's HWND to new root windows, or b) hooking all window-reparent events in the host application and checking whether they'd affect any Gecko windows which might have file-dialogs open.

I believe the latter is technically feasible, but it's a lot of present-day effort and future maintenance work for a use-case which not only hasn't been requested, but quite plausibly won't be before Microsoft reworks the file-dialog implementation again.

If you have such a use-case, we're willing to revisit this to find a reasonable solution. In the meantime, I plan to land the above patch — which does at least ameliorate this slightly — and then close this bug as RESOLVED WONTFIX.

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1858225

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b3d4baf33ad6
[1/1] also manually disable the root window when opening a file-dialog  r=win-reviewers,handyman

Comment 6

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/b3d4baf33ad6

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:rkraesig, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox124 to wontfix.

For more information, please visit BugBot documentation.

Comment 8

[Ray Kraesig [:rkraesig]]

No, the patch isn't important enough to uplift.

:diannaS: Setting this to WONTFIX was intentional (see comment 3). Was that causing issues?

Comment 9

[Dianna Smith [:diannaS]]

My apologies, makes sense now that i read that comment again. I only changed it when i changed the status/milestone for backtracking purposes.
Ill change it back to a wont fix resolution, but i think the status/milestone should stay as something being introduced in 125.