fix: handle async nature of [NSWindow -toggleFullScreen] (#28773)

shell/browser/native_window.cc

@@ -143,7 +143,7 @@ void NativeWindow::InitFromOptions(const gin_helper::Dictionary& options) {
     fullscreenable = false;
 #endif
   }
-  // Overriden by 'fullscreenable'.
+  // Overridden by 'fullscreenable'.
   options.Get(options::kFullScreenable, &fullscreenable);
   SetFullScreenable(fullscreenable);
   if (fullscreen) {

shell/browser/native_window_mac.h

@@ -8,6 +8,7 @@
 #import <Cocoa/Cocoa.h>
 
 #include <memory>
+#include <queue>
 #include <string>
 #include <tuple>
 #include <vector>
@@ -159,11 +160,16 @@ class NativeWindowMac : public NativeWindow, public ui::NativeThemeObserver {
 
   // Custom traffic light positioning
   void RedrawTrafficLights() override;
-  void SetExitingFullScreen(bool flag);
   void SetTrafficLightPosition(const gfx::Point& position) override;
   gfx::Point GetTrafficLightPosition() const override;
   void OnNativeThemeUpdated(ui::NativeTheme* observed_theme) override;
 
+  enum class FullScreenTransitionState { ENTERING, EXITING, NONE };
+
+  // Handle fullscreen transitions.
+  void SetFullScreenTransitionState(FullScreenTransitionState state);
+  void HandlePendingFullscreenTransitions();
+
   enum class VisualEffectState {
     FOLLOW_WINDOW,
     ACTIVE,
@@ -183,7 +189,6 @@ class NativeWindowMac : public NativeWindow, public ui::NativeThemeObserver {
   bool zoom_to_page_width() const { return zoom_to_page_width_; }
   bool fullscreen_window_title() const { return fullscreen_window_title_; }
   bool always_simple_fullscreen() const { return always_simple_fullscreen_; }
-  bool exiting_fullscreen() const { return exiting_fullscreen_; }
 
  protected:
   // views::WidgetDelegate:
@@ -216,13 +221,18 @@ class NativeWindowMac : public NativeWindow, public ui::NativeThemeObserver {
   std::unique_ptr<RootViewMac> root_view_;
 
   bool is_kiosk_ = false;
-  bool was_fullscreen_ = false;
   bool zoom_to_page_width_ = false;
   bool fullscreen_window_title_ = false;
   bool resizable_ = true;
-  bool exiting_fullscreen_ = false;
   gfx::Point traffic_light_position_;
 
+  std::queue<bool> pending_transitions_;
+  FullScreenTransitionState fullscreen_transition_state() const {
+    return fullscreen_transition_state_;
+  }
+  FullScreenTransitionState fullscreen_transition_state_ =
+      FullScreenTransitionState::NONE;
+
   NSInteger attention_request_id_ = 0;  // identifier from requestUserAttention
 
   // The presentation options before entering kiosk mode.

shell/browser/native_window_mac.mm

@@ -569,7 +569,7 @@ void NativeWindowMac::RedrawTrafficLights() {
 
   // Hide the container when exiting fullscreen, otherwise traffic light buttons
   // jump
-  if (exiting_fullscreen_) {
+  if (fullscreen_transition_state_ == FullScreenTransitionState::EXITING) {
     [titleBarContainerView setHidden:YES];
     return;
   }
@@ -719,16 +719,17 @@ bool NativeWindowMac::IsVisible() {
   return [window_ isVisible] && !occluded && !IsMinimized();
 }
 
-void NativeWindowMac::SetExitingFullScreen(bool flag) {
-  exiting_fullscreen_ = flag;
-}
-
 void NativeWindowMac::OnNativeThemeUpdated(ui::NativeTheme* observed_theme) {
   base::PostTask(
       FROM_HERE, {content::BrowserThread::UI},
       base::BindOnce(&NativeWindow::RedrawTrafficLights, GetWeakPtr()));
 }
 
+void NativeWindowMac::SetFullScreenTransitionState(
+    FullScreenTransitionState state) {
+  fullscreen_transition_state_ = state;
+}
+
 bool NativeWindowMac::IsEnabled() {
   return [window_ attachedSheet] == nil;
 }
@@ -793,13 +794,48 @@ bool NativeWindowMac::IsMinimized() {
   return [window_ isMiniaturized];
 }
 
+void NativeWindowMac::HandlePendingFullscreenTransitions() {
+  if (pending_transitions_.empty())
+    return;
+
+  bool next_transition = pending_transitions_.front();
+  pending_transitions_.pop();
+  SetFullScreen(next_transition);
+}
+
 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
+  // in process. This can create weird behavior (incl. phantom windows),
+  // so we want to schedule a transition for when the current one has completed.
+  if (fullscreen_transition_state() != FullScreenTransitionState::NONE) {
+    if (!pending_transitions_.empty()) {
+      bool last_pending = pending_transitions_.back();
+      // Only push new transitions if they're different than the last transition
+      // in the queue.
+      if (last_pending != fullscreen)
+        pending_transitions_.push(fullscreen);
+    } else {
+      pending_transitions_.push(fullscreen);
+    }
+    return;
+  }
+
   if (fullscreen == IsFullscreen())
     return;
 
   // Take note of the current window size
   if (IsNormal())
     original_frame_ = [window_ frame];
+
+  // This needs to be set here because it can be the case that
+  // SetFullScreen is called by a user before windowWillEnterFullScreen
+  // or windowWillExitFullScreen are invoked, and so a potential transition
+  // could be dropped.
+  fullscreen_transition_state_ = fullscreen
+                                     ? FullScreenTransitionState::ENTERING
+                                     : FullScreenTransitionState::EXITING;
+
   [window_ toggleFullScreenMode:nil];
 }
 
@@ -1186,14 +1222,11 @@ void NativeWindowMac::SetKiosk(bool kiosk) {
         NSApplicationPresentationDisableHideApplication;
     [NSApp setPresentationOptions:options];
     is_kiosk_ = true;
-    was_fullscreen_ = IsFullscreen();
-    if (!was_fullscreen_)
-      SetFullScreen(true);
+    SetFullScreen(true);
   } else if (!kiosk && is_kiosk_) {
-    is_kiosk_ = false;
-    if (!was_fullscreen_)
-      SetFullScreen(false);
     [NSApp setPresentationOptions:kiosk_options_];
+    is_kiosk_ = false;
+    SetFullScreen(false);
   }
 }
 

shell/browser/ui/cocoa/electron_ns_window.mm

@@ -223,11 +223,23 @@ bool ScopedDisableResize::disable_resize_ = false;
   if (is_simple_fs || always_simple_fs) {
     shell_->SetSimpleFullScreen(!is_simple_fs);
   } else {
-    bool maximizable = shell_->IsMaximizable();
-    [super toggleFullScreen:sender];
+    if (shell_->IsVisible()) {
+      // Until 10.13, AppKit would obey a call to -toggleFullScreen: made inside
+      // windowDidEnterFullScreen & windowDidExitFullScreen. Starting in 10.13,
+      // it behaves as though the transition is still in progress and just emits
+      // "not in a fullscreen state" when trying to exit fullscreen in the same
+      // runloop that entered it. To handle this, invoke -toggleFullScreen:
+      // asynchronously.
+      [super performSelector:@selector(toggleFullScreen:)
+                  withObject:nil
+                  afterDelay:0];
+    } else {
+      [super toggleFullScreen:sender];
+    }
 
     // Exiting fullscreen causes Cocoa to redraw the NSWindow, which resets
     // the enabled state for NSWindowZoomButton. We need to persist it.
+    bool maximizable = shell_->IsMaximizable();
     shell_->SetMaximizable(maximizable);
   }
 }

shell/browser/ui/cocoa/electron_ns_window_delegate.mm

@@ -17,6 +17,8 @@
 #include "ui/views/widget/native_widget_mac.h"
 
 using TitleBarStyle = electron::NativeWindowMac::TitleBarStyle;
+using FullScreenTransitionState =
+    electron::NativeWindowMac::FullScreenTransitionState;
 
 @implementation ElectronNSWindowDelegate
 
@@ -212,7 +214,9 @@ using TitleBarStyle = electron::NativeWindowMac::TitleBarStyle;
 }
 
 - (void)windowWillEnterFullScreen:(NSNotification*)notification {
-  // Setting resizable to true before entering fullscreen
+  shell_->SetFullScreenTransitionState(FullScreenTransitionState::ENTERING);
+
+  // Setting resizable to true before entering fullscreen.
   is_resizable_ = shell_->IsResizable();
   shell_->SetResizable(true);
   // Hide the native toolbar before entering fullscreen, so there is no visual
@@ -224,6 +228,8 @@ using TitleBarStyle = electron::NativeWindowMac::TitleBarStyle;
 }
 
 - (void)windowDidEnterFullScreen:(NSNotification*)notification {
+  shell_->SetFullScreenTransitionState(FullScreenTransitionState::NONE);
+
   shell_->NotifyWindowEnterFullScreen();
 
   // For frameless window we don't show set title for normal mode since the
@@ -252,9 +258,13 @@ using TitleBarStyle = electron::NativeWindowMac::TitleBarStyle;
     [window setTitlebarAppearsTransparent:NO];
     shell_->SetStyleMask(true, NSWindowStyleMaskFullSizeContentView);
   }
+
+  shell_->HandlePendingFullscreenTransitions();
 }
 
 - (void)windowWillExitFullScreen:(NSNotification*)notification {
+  shell_->SetFullScreenTransitionState(FullScreenTransitionState::EXITING);
+
   // Restore the titlebar visibility.
   NSWindow* window = shell_->GetNativeWindow().GetNativeNSWindow();
   if ((shell_->transparent() || !shell_->has_frame()) &&
@@ -268,19 +278,23 @@ using TitleBarStyle = electron::NativeWindowMac::TitleBarStyle;
     shell_->SetStyleMask(false, NSWindowStyleMaskFullSizeContentView);
     [window setTitlebarAppearsTransparent:YES];
   }
-  shell_->SetExitingFullScreen(true);
+
   if (shell_->title_bar_style() == TitleBarStyle::HIDDEN) {
     shell_->RedrawTrafficLights();
   }
 }
 
 - (void)windowDidExitFullScreen:(NSNotification*)notification {
+  shell_->SetFullScreenTransitionState(FullScreenTransitionState::NONE);
+
   shell_->SetResizable(is_resizable_);
   shell_->NotifyWindowLeaveFullScreen();
-  shell_->SetExitingFullScreen(false);
+
   if (shell_->title_bar_style() == TitleBarStyle::HIDDEN) {
     shell_->RedrawTrafficLights();
   }
+
+  shell_->HandlePendingFullscreenTransitions();
 }
 
 - (void)windowWillClose:(NSNotification*)notification {

spec-main/api-browser-window-spec.ts

@@ -8,7 +8,7 @@ import * as http from 'http';
 import { AddressInfo } from 'net';
 import { app, BrowserWindow, BrowserView, ipcMain, OnBeforeSendHeadersListenerDetails, protocol, screen, webContents, session, WebContents } from 'electron/main';
 
-import { emittedOnce, emittedUntil } from './events-helpers';
+import { emittedOnce, emittedUntil, emittedNTimes } from './events-helpers';
 import { ifit, ifdescribe, defer, delay } from './spec-helpers';
 import { closeWindow, closeAllWindows } from './window-helpers';
 
@@ -4046,6 +4046,42 @@ describe('BrowserWindow module', () => {
         expect(w.isFullScreen()).to.be.false('isFullScreen');
       });
 
+      it('handles several transitions starting with fullscreen', async () => {
+        const w = new BrowserWindow({ fullscreen: true, show: true });
+
+        expect(w.isFullScreen()).to.be.true('not fullscreen');
+
+        w.setFullScreen(false);
+        w.setFullScreen(true);
+
+        const enterFullScreen = emittedNTimes(w, 'enter-full-screen', 2);
+        await enterFullScreen;
+
+        expect(w.isFullScreen()).to.be.true('not fullscreen');
+
+        await delay();
+        const leaveFullScreen = emittedOnce(w, 'leave-full-screen');
+        w.setFullScreen(false);
+        await leaveFullScreen;
+
+        expect(w.isFullScreen()).to.be.false('is fullscreen');
+      });
+
+      it('handles several transitions in close proximity', async () => {
+        const w = new BrowserWindow();
+
+        expect(w.isFullScreen()).to.be.false('is fullscreen');
+
+        w.setFullScreen(true);
+        w.setFullScreen(false);
+        w.setFullScreen(true);
+
+        const enterFullScreen = emittedNTimes(w, 'enter-full-screen', 2);
+        await enterFullScreen;
+
+        expect(w.isFullScreen()).to.be.true('not fullscreen');
+      });
+
       it('does not crash when exiting simpleFullScreen (properties)', async () => {
         const w = new BrowserWindow();
         w.setSimpleFullScreen(true);

spec-main/chromium-spec.ts

@@ -1502,8 +1502,8 @@ describe('iframe using HTML fullscreen API while window is OS-fullscreened', ()
   });
 
   afterEach(async () => {
-    await closeAllWindows()
-    ;(w as any) = null;
+    await closeAllWindows();
+    (w as any) = null;
     server.close();
   });