Bug 1865372: Call nsCocoaWindow::DestroyNativeWindow more often in Destroy. r=mac...
commit477a5e068521556abf914ff54de6229f2f66cb16
authorBrad Werth <bwerth@mozilla.com>
Tue, 30 Jan 2024 18:07:40 +0000 (30 18:07 +0000)
committerBrad Werth <bwerth@mozilla.com>
Tue, 30 Jan 2024 18:07:40 +0000 (30 18:07 +0000)
treecf7ea29b81bdffd0010377ee44390657de8991d9
parentb2f97bc9f123ddf938f73e5f8f505ffcb5532a49
Bug 1865372: Call nsCocoaWindow::DestroyNativeWindow more often in Destroy. r=mac-reviewers,mstange

This seems like the right time to call DestroyNativeWindow since we are
hiding the window and destorying the nsBaseWidget -- there's not much
else that can happen to this window. Making this call here ensures that
the native window is not maintained on the screen waiting for the
destructor to be called during garbage collection.

Unfortunately, this relative simple change introduces the possibility of
a deadlock. It happens like so:

1) Some nsCocoaWindow instance A is in ProcessTransitions, handling a
native fullscreen transition. It claims the static class state lock in
sWindowInNativeTransition.

2) Some *other* nsCocoaWindow instance B is also in ProcessTransitions,
waiting to claim the class state lock. While waiting, it spins a local
run loop.

3) Within that local run loop, instance A receives a message to close
its window, or something else that triggers DestroyNativeWindow.

4) Instance A DestroyNativeWindow attempts to claim class state lock,
can't, and spins an embedded local run loop. This spins forever.

It's not clear exactly what steps would break this deadlock, but it does
seem clear that embedded local run loops are something we need to avoid.
We accomplish this by:

1) Removing all the run loops in DestroyNativeWindow. It does not seem
necessary to ensure that fullscreen transitions are complete before
exiting this function, only that sWindowInNativeTransition is not held.

2) Similarly making DestroyNativeWindow clear the current and pending
transitions, since those will have no meaningful effect when the native
window is destroyed.

3) Changing FinishCurrentTransitionIfMatching to clear out the current
transition immediately, and only dispatching the follow-up call to
ProcessTransitions if there are pending transitions. This is an
alternate way to avoid the dispatch-to-dead-object issue solved by the
changes in Bug 1839537.

Collectively, these changes ensure that the local run loops in
ProcessTransitions will not deadlock, and now there are no other local
run loops that could be nested within them.

Differential Revision: https://phabricator.services.mozilla.com/D197214
widget/cocoa/nsCocoaWindow.h
widget/cocoa/nsCocoaWindow.mm