fix: zombie windows when fullscreening and closing (#34390)

Co-authored-by: Shelley Vohr <[email protected]>

shell/browser/native_window_mac.h

@@ -176,6 +176,10 @@ class NativeWindowMac : public NativeWindow,
   // Handle fullscreen transitions.
   void SetFullScreenTransitionState(FullScreenTransitionState state);
   void HandlePendingFullscreenTransitions();
+  bool HandleDeferredClose();
+  void SetHasDeferredWindowClose(bool defer_close) {
+    has_deferred_window_close_ = defer_close;
+  }
 
   enum class VisualEffectState {
     kFollowWindow,
@@ -249,6 +253,12 @@ class NativeWindowMac : public NativeWindow,
   FullScreenTransitionState fullscreen_transition_state_ =
       FullScreenTransitionState::NONE;
 
+  // Trying to close an NSWindow during a fullscreen transition will cause the
+  // window to lock up. Use this to track if CloseWindow was called during a
+  // fullscreen transition, to defer the -[NSWindow close] call until the
+  // transition is complete.
+  bool has_deferred_window_close_ = false;
+
   NSInteger attention_request_id_ = 0;  // identifier from requestUserAttention
 
   // The presentation options before entering kiosk mode.

shell/browser/native_window_mac.mm

@@ -474,6 +474,11 @@ void NativeWindowMac::Close() {
     return;
   }
 
+  if (fullscreen_transition_state() != FullScreenTransitionState::NONE) {
+    SetHasDeferredWindowClose(true);
+    return;
+  }
+
   // If a sheet is attached to the window when we call
   // [window_ performClose:nil], the window won't close properly
   // even after the user has ended the sheet.
@@ -678,6 +683,15 @@ void NativeWindowMac::HandlePendingFullscreenTransitions() {
   SetFullScreen(next_transition);
 }
 
+bool NativeWindowMac::HandleDeferredClose() {
+  if (has_deferred_window_close_) {
+    SetHasDeferredWindowClose(false);
+    Close();
+    return true;
+  }
+  return false;
+}
+
 void NativeWindowMac::SetFullScreen(bool fullscreen) {
   // [NSWindow -toggleFullScreen] is an asynchronous operation, which means
   // that it's possible to call it while a fullscreen transition is currently

shell/browser/ui/cocoa/electron_ns_window_delegate.mm

@@ -248,6 +248,9 @@ using FullScreenTransitionState =
 
   shell_->NotifyWindowEnterFullScreen();
 
+  if (shell_->HandleDeferredClose())
+    return;
+
   shell_->HandlePendingFullscreenTransitions();
 }
 
@@ -263,6 +266,9 @@ using FullScreenTransitionState =
   shell_->SetResizable(is_resizable_);
   shell_->NotifyWindowLeaveFullScreen();
 
+  if (shell_->HandleDeferredClose())
+    return;
+
   shell_->HandlePendingFullscreenTransitions();
 }
 

spec-main/webview-spec.ts

@@ -434,11 +434,12 @@ describe('<webview> tag', function () {
       return [w, webview];
     };
 
-    afterEach(closeAllWindows);
     afterEach(async () => {
       // The leaving animation is un-observable but can interfere with future tests
       // Specifically this is async on macOS but can be on other platforms too
       await delay(1000);
+
+      closeAllWindows();
     });
 
     it('should make parent frame element fullscreen too', async () => {
@@ -449,11 +450,13 @@ describe('<webview> tag', function () {
       await webview.executeJavaScript('document.getElementById("div").requestFullscreen()', true);
       await parentFullscreen;
       expect(await w.webContents.executeJavaScript('isIframeFullscreen()')).to.be.true();
+
+      w.close();
+      await emittedOnce(w, 'closed');
     });
 
     // FIXME(zcbenz): Fullscreen events do not work on Linux.
-    // This test is flaky on arm64 macOS.
-    ifit(process.platform !== 'linux' && process.arch !== 'arm64')('exiting fullscreen should unfullscreen window', async () => {
+    ifit(process.platform !== 'linux')('exiting fullscreen should unfullscreen window', async () => {
       const [w, webview] = await loadWebViewWindow();
       const enterFullScreen = emittedOnce(w, 'enter-full-screen');
       await webview.executeJavaScript('document.getElementById("div").requestFullscreen()', true);
@@ -464,6 +467,9 @@ describe('<webview> tag', function () {
       await leaveFullScreen;
       await delay(0);
       expect(w.isFullScreen()).to.be.false();
+
+      w.close();
+      await emittedOnce(w, 'closed');
     });
 
     // Sending ESC via sendInputEvent only works on Windows.
@@ -478,6 +484,9 @@ describe('<webview> tag', function () {
       await leaveFullScreen;
       await delay(0);
       expect(w.isFullScreen()).to.be.false();
+
+      w.close();
+      await emittedOnce(w, 'closed');
     });
 
     it('pressing ESC should emit the leave-html-full-screen event', async () => {
@@ -506,6 +515,9 @@ describe('<webview> tag', function () {
       webContents.sendInputEvent({ type: 'keyDown', keyCode: 'Escape' });
       await leaveFSWindow;
       await leaveFSWebview;
+
+      w.close();
+      await emittedOnce(w, 'closed');
     });
   });