Skip to content

Commit

Permalink
Bug 1877749: Prevent nsCocoaWindow from forgetting transitions during…
Browse files Browse the repository at this point in the history
… transitory window rebuilds. r=mstange

DestroyNativeWindow() is called for permanent window destruction, but it
is also called for transitory window recreated in HideWindowChrome().
When the nsCocoaWindow itself is also expected to be destroyed, it's
useful to clear out transitions. But when the nsCocoaWindow is expected
to persist (with a new mWindow instance), it's unhelpful to clear the
transitions, because emulated fullscreen relies on transition
continuity.

This change further simplifies DestroyNativeWindow so it does only the
bare-minimum, always-needed things before forgetting mWindow and its
delegate. The higher-level concern of clearing out transitions is
factored out into a new function CancelAllTransitions, which is invoked
by callers when appropriate.

Differential Revision: https://phabricator.services.mozilla.com/D200215
  • Loading branch information
bradwerth committed Feb 6, 2024
1 parent e68568b commit b4e4ce3
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 10 deletions.
4 changes: 4 additions & 0 deletions widget/cocoa/nsCocoaWindow.h
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,10 @@ class nsCocoaWindow final : public nsBaseWidget, public nsPIWidgetCocoa {
void QueueTransition(const TransitionType& aTransition);
void ProcessTransitions();

// Call this to stop all transition processing, which is useful during
// window closing and shutdown.
void CancelAllTransitions();

bool mInProcessTransitions = false;

// While running an emulated fullscreen transition, we want to suppress
Expand Down
26 changes: 16 additions & 10 deletions widget/cocoa/nsCocoaWindow.mm
Original file line number Diff line number Diff line change
Expand Up @@ -178,16 +178,10 @@ static void RollUpPopups(nsIRollupListener::AllowAnimations aAllowAnimations =
MOZ_ASSERT(mWindowMadeHere,
"We shouldn't be trying to destroy a window we didn't create.");

// Clear our current and pending transitions. This simplifies our
// reasoning about what happens next, and ensures that whatever is
// currently happening won't trigger another call to
// ProcessTransitions().
mTransitionCurrent.reset();
mIsTransitionCurrentAdded = false;
std::queue<TransitionType>().swap(mTransitionsPending);

// Clear our class state, which will ensure that other nsCocoaWindow
// instances are not waiting for us to finish a native transition.
// Clear our class state that is keyed off of mWindow. It's our last
// chance! This ensures that other nsCocoaWindow instances are not waiting
// for us to finish a native transition that will have no listener once
// we clear our delegate.
EndOurNativeTransition();

[mWindow releaseJSObjects];
Expand Down Expand Up @@ -224,6 +218,7 @@ static void RollUpPopups(nsIRollupListener::AllowAnimations aAllowAnimations =
}

if (mWindow && mWindowMadeHere) {
CancelAllTransitions();
DestroyNativeWindow();
}

Expand Down Expand Up @@ -653,6 +648,7 @@ static unsigned int WindowMaskForBorderStyle(BorderStyle aBorderStyle) {
// at least one object holding a reference to ourselves is usually waiting
// to be garbage-collected.
if (mWindow && mWindowMadeHere) {
CancelAllTransitions();
DestroyNativeWindow();
}
}
Expand Down Expand Up @@ -1958,6 +1954,16 @@ static bool AlwaysUsesNativeFullScreen() {
NS_OBJC_END_TRY_IGNORE_BLOCK;
}

void nsCocoaWindow::CancelAllTransitions() {
// Clear our current and pending transitions. This simplifies our
// reasoning about what happens next, and ensures that whatever is
// currently happening won't trigger another call to
// ProcessTransitions().
mTransitionCurrent.reset();
mIsTransitionCurrentAdded = false;
std::queue<TransitionType>().swap(mTransitionsPending);
}

void nsCocoaWindow::FinishCurrentTransitionIfMatching(
const TransitionType& aTransition) {
// We've just finished some transition activity, and we're not sure whether it
Expand Down

0 comments on commit b4e4ce3

Please sign in to comment.