fix: html fullscreen transitions stacking (#34908)

shell/browser/api/electron_api_web_contents.cc

@@ -3524,7 +3524,12 @@ void WebContents::EnumerateDirectory(
 
 bool WebContents::IsFullscreenForTabOrPending(
     const content::WebContents* source) {
-  return html_fullscreen_;
+  bool transition_fs = owner_window()
+                           ? owner_window()->fullscreen_transition_state() !=
+                                 NativeWindow::FullScreenTransitionState::NONE
+                           : false;
+
+  return html_fullscreen_ || transition_fs;
 }
 
 bool WebContents::TakeFocus(content::WebContents* source, bool reverse) {
@@ -3846,9 +3851,8 @@ void WebContents::SetHtmlApiFullscreen(bool enter_fullscreen) {
           ? !web_preferences->ShouldDisableHtmlFullscreenWindowResize()
           : true;
 
-  if (html_fullscreenable) {
+  if (html_fullscreenable)
     owner_window_->SetFullScreen(enter_fullscreen);
-  }
 
   UpdateHtmlApiFullscreen(enter_fullscreen);
   native_fullscreen_ = false;

shell/browser/native_window.cc

@@ -718,6 +718,15 @@ std::string NativeWindow::GetAccessibleTitle() {
   return base::UTF16ToUTF8(accessible_title_);
 }
 
+void NativeWindow::HandlePendingFullscreenTransitions() {
+  if (pending_transitions_.empty())
+    return;
+
+  bool next_transition = pending_transitions_.front();
+  pending_transitions_.pop();
+  SetFullScreen(next_transition);
+}
+
 // static
 int32_t NativeWindow::next_id_ = 0;
 

shell/browser/native_window.h

@@ -7,6 +7,7 @@
 
 #include <list>
 #include <memory>
+#include <queue>
 #include <string>
 #include <vector>
 
@@ -317,6 +318,17 @@ class NativeWindow : public base::SupportsUserData,
     observers_.RemoveObserver(obs);
   }
 
+  enum class FullScreenTransitionState { ENTERING, EXITING, NONE };
+
+  // Handle fullscreen transitions.
+  void HandlePendingFullscreenTransitions();
+  void set_fullscreen_transition_state(FullScreenTransitionState state) {
+    fullscreen_transition_state_ = state;
+  }
+  FullScreenTransitionState fullscreen_transition_state() const {
+    return fullscreen_transition_state_;
+  }
+
   views::Widget* widget() const { return widget_.get(); }
   views::View* content_view() const { return content_view_; }
 
@@ -375,6 +387,10 @@ class NativeWindow : public base::SupportsUserData,
   // The "titleBarStyle" option.
   TitleBarStyle title_bar_style_ = TitleBarStyle::kNormal;
 
+  std::queue<bool> pending_transitions_;
+  FullScreenTransitionState fullscreen_transition_state_ =
+      FullScreenTransitionState::NONE;
+
  private:
   std::unique_ptr<views::Widget> widget_;
 

shell/browser/native_window_mac.h

@@ -8,7 +8,6 @@
 #import <Cocoa/Cocoa.h>
 
 #include <memory>
-#include <queue>
 #include <string>
 #include <vector>
 
@@ -174,11 +173,6 @@ class NativeWindowMac : public NativeWindow,
   void SetCollectionBehavior(bool on, NSUInteger flag);
   void SetWindowLevel(int level);
 
-  enum class FullScreenTransitionState { ENTERING, EXITING, NONE };
-
-  // Handle fullscreen transitions.
-  void SetFullScreenTransitionState(FullScreenTransitionState state);
-  void HandlePendingFullscreenTransitions();
   bool HandleDeferredClose();
   void SetHasDeferredWindowClose(bool defer_close) {
     has_deferred_window_close_ = defer_close;
@@ -249,13 +243,6 @@ class NativeWindowMac : public NativeWindow,
   bool zoom_to_page_width_ = false;
   absl::optional<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;
-
   // 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

shell/browser/native_window_mac.mm

@@ -588,11 +588,6 @@ bool NativeWindowMac::IsVisible() {
   return [window_ isVisible] && !occluded && !IsMinimized();
 }
 
-void NativeWindowMac::SetFullScreenTransitionState(
-    FullScreenTransitionState state) {
-  fullscreen_transition_state_ = state;
-}
-
 bool NativeWindowMac::IsEnabled() {
   return [window_ attachedSheet] == nil;
 }
@@ -685,15 +680,6 @@ 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);
-}
-
 bool NativeWindowMac::HandleDeferredClose() {
   if (has_deferred_window_close_) {
     SetHasDeferredWindowClose(false);

shell/browser/ui/cocoa/electron_ns_window_delegate.mm

@@ -238,7 +238,7 @@ using FullScreenTransitionState =
   // Store resizable mask so it can be restored after exiting fullscreen.
   is_resizable_ = shell_->HasStyleMask(NSWindowStyleMaskResizable);
 
-  shell_->SetFullScreenTransitionState(FullScreenTransitionState::ENTERING);
+  shell_->set_fullscreen_transition_state(FullScreenTransitionState::ENTERING);
 
   shell_->NotifyWindowWillEnterFullScreen();
 
@@ -247,7 +247,7 @@ using FullScreenTransitionState =
 }
 
 - (void)windowDidEnterFullScreen:(NSNotification*)notification {
-  shell_->SetFullScreenTransitionState(FullScreenTransitionState::NONE);
+  shell_->set_fullscreen_transition_state(FullScreenTransitionState::NONE);
 
   shell_->NotifyWindowEnterFullScreen();
 
@@ -258,13 +258,13 @@ using FullScreenTransitionState =
 }
 
 - (void)windowWillExitFullScreen:(NSNotification*)notification {
-  shell_->SetFullScreenTransitionState(FullScreenTransitionState::EXITING);
+  shell_->set_fullscreen_transition_state(FullScreenTransitionState::EXITING);
 
   shell_->NotifyWindowWillLeaveFullScreen();
 }
 
 - (void)windowDidExitFullScreen:(NSNotification*)notification {
-  shell_->SetFullScreenTransitionState(FullScreenTransitionState::NONE);
+  shell_->set_fullscreen_transition_state(FullScreenTransitionState::NONE);
 
   shell_->SetResizable(is_resizable_);
   shell_->NotifyWindowLeaveFullScreen();

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

@@ -4981,19 +4981,80 @@ describe('BrowserWindow module', () => {
         expect(w.isFullScreen()).to.be.false('is fullscreen');
       });
 
+      it('handles several HTML fullscreen transitions', async () => {
+        const w = new BrowserWindow();
+        await w.loadFile(path.join(fixtures, 'pages', 'a.html'));
+
+        expect(w.isFullScreen()).to.be.false('is fullscreen');
+
+        const enterFullScreen = emittedOnce(w, 'enter-full-screen');
+        const leaveFullScreen = emittedOnce(w, 'leave-full-screen');
+
+        await w.webContents.executeJavaScript('document.getElementById("div").requestFullscreen()', true);
+        await enterFullScreen;
+        await w.webContents.executeJavaScript('document.exitFullscreen()', true);
+        await leaveFullScreen;
+
+        expect(w.isFullScreen()).to.be.false('is fullscreen');
+
+        await delay();
+
+        await w.webContents.executeJavaScript('document.getElementById("div").requestFullscreen()', true);
+        await enterFullScreen;
+        await w.webContents.executeJavaScript('document.exitFullscreen()', true);
+        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');
 
+        const enterFS = emittedNTimes(w, 'enter-full-screen', 2);
+        const leaveFS = emittedNTimes(w, 'leave-full-screen', 2);
+
         w.setFullScreen(true);
         w.setFullScreen(false);
         w.setFullScreen(true);
+        w.setFullScreen(false);
 
-        const enterFullScreen = emittedNTimes(w, 'enter-full-screen', 2);
-        await enterFullScreen;
+        await Promise.all([enterFS, leaveFS]);
 
-        expect(w.isFullScreen()).to.be.true('not fullscreen');
+        expect(w.isFullScreen()).to.be.false('not fullscreen');
+      });
+
+      it('handles several chromium-initiated transitions in close proximity', async () => {
+        const w = new BrowserWindow();
+        await w.loadFile(path.join(fixtures, 'pages', 'a.html'));
+
+        expect(w.isFullScreen()).to.be.false('is fullscreen');
+
+        let enterCount = 0;
+        let exitCount = 0;
+
+        const done = new Promise<void>(resolve => {
+          const checkDone = () => {
+            if (enterCount === 2 && exitCount === 2) resolve();
+          };
+
+          w.webContents.on('enter-html-full-screen', () => {
+            enterCount++;
+            checkDone();
+          });
+
+          w.webContents.on('leave-html-full-screen', () => {
+            exitCount++;
+            checkDone();
+          });
+        });
+
+        await w.webContents.executeJavaScript('document.getElementById("div").requestFullscreen()', true);
+        await w.webContents.executeJavaScript('document.exitFullscreen()');
+        await w.webContents.executeJavaScript('document.getElementById("div").requestFullscreen()', true);
+        await w.webContents.executeJavaScript('document.exitFullscreen()');
+        await done;
       });
 
       it('does not crash when exiting simpleFullScreen (properties)', async () => {

spec-main/chromium-spec.ts

@@ -10,7 +10,7 @@ import * as url from 'url';
 import * as ChildProcess from 'child_process';
 import { EventEmitter } from 'events';
 import { promisify } from 'util';
-import { ifit, ifdescribe, delay, defer } from './spec-helpers';
+import { ifit, ifdescribe, defer, delay } from './spec-helpers';
 import { AddressInfo } from 'net';
 import { PipeTransport } from './pipe-transport';
 
@@ -1653,7 +1653,7 @@ describe('iframe using HTML fullscreen API while window is OS-fullscreened', ()
     server.close();
   });
 
-  it('can fullscreen from out-of-process iframes (OOPIFs)', async () => {
+  ifit(process.platform !== 'darwin')('can fullscreen from out-of-process iframes (non-macOS)', async () => {
     const fullscreenChange = emittedOnce(ipcMain, 'fullscreenChange');
     const html =
       '<iframe style="width: 0" frameborder=0 src="http://localhost:8989" allowfullscreen></iframe>';
@@ -1677,8 +1677,37 @@ describe('iframe using HTML fullscreen API while window is OS-fullscreened', ()
     expect(width).to.equal(0);
   });
 
+  ifit(process.platform === 'darwin')('can fullscreen from out-of-process iframes (macOS)', async () => {
+    await emittedOnce(w, 'enter-full-screen');
+    const fullscreenChange = emittedOnce(ipcMain, 'fullscreenChange');
+    const html =
+      '<iframe style="width: 0" frameborder=0 src="http://localhost:8989" allowfullscreen></iframe>';
+    w.loadURL(`data:text/html,${html}`);
+    await fullscreenChange;
+
+    const fullscreenWidth = await w.webContents.executeJavaScript(
+      "document.querySelector('iframe').offsetWidth"
+    );
+    expect(fullscreenWidth > 0).to.be.true();
+
+    await w.webContents.executeJavaScript(
+      "document.querySelector('iframe').contentWindow.postMessage('exitFullscreen', '*')"
+    );
+    await emittedOnce(w.webContents, 'leave-html-full-screen');
+
+    const width = await w.webContents.executeJavaScript(
+      "document.querySelector('iframe').offsetWidth"
+    );
+    expect(width).to.equal(0);
+
+    w.setFullScreen(false);
+    await emittedOnce(w, 'leave-full-screen');
+  });
+
   // TODO(jkleinsc) fix this flaky test on WOA
   ifit(process.platform !== 'win32' || process.arch !== 'arm64')('can fullscreen from in-process iframes', async () => {
+    if (process.platform === 'darwin') await emittedOnce(w, 'enter-full-screen');
+
     const fullscreenChange = emittedOnce(ipcMain, 'fullscreenChange');
     w.loadFile(path.join(fixturesPath, 'pages', 'fullscreen-ipif.html'));
     await fullscreenChange;

spec-main/fixtures/webview/fullscreen/frame.html

@@ -2,6 +2,7 @@
   <div id="div">
     WebView
   </div>
+  <video></video>
   <script type="text/javascript" charset="utf-8">
     const {ipcRenderer} = require('electron')
     ipcRenderer.send('webview-ready')

spec-main/webview-spec.ts

@@ -426,11 +426,16 @@ describe('<webview> tag', function () {
           contextIsolation: false
         }
       });
+
       const attachPromise = emittedOnce(w.webContents, 'did-attach-webview');
+      const loadPromise = emittedOnce(w.webContents, 'did-finish-load');
       const readyPromise = emittedOnce(ipcMain, 'webview-ready');
+
       w.loadFile(path.join(__dirname, 'fixtures', 'webview', 'fullscreen', 'main.html'));
+
       const [, webview] = await attachPromise;
-      await readyPromise;
+      await Promise.all([readyPromise, loadPromise]);
+
       return [w, webview];
     };
 
@@ -442,17 +447,38 @@ describe('<webview> tag', function () {
       closeAllWindows();
     });
 
-    it('should make parent frame element fullscreen too', async () => {
+    ifit(process.platform !== 'darwin')('should make parent frame element fullscreen too (non-macOS)', async () => {
       const [w, webview] = await loadWebViewWindow();
       expect(await w.webContents.executeJavaScript('isIframeFullscreen()')).to.be.false();
 
       const parentFullscreen = emittedOnce(ipcMain, 'fullscreenchange');
       await webview.executeJavaScript('document.getElementById("div").requestFullscreen()', true);
       await parentFullscreen;
+
+      expect(await w.webContents.executeJavaScript('isIframeFullscreen()')).to.be.true();
+
+      const close = emittedOnce(w, 'closed');
+      w.close();
+      await close;
+    });
+
+    ifit(process.platform === 'darwin')('should make parent frame element fullscreen too (macOS)', async () => {
+      const [w, webview] = await loadWebViewWindow();
+      expect(await w.webContents.executeJavaScript('isIframeFullscreen()')).to.be.false();
+
+      const parentFullscreen = emittedOnce(ipcMain, 'fullscreenchange');
+      const enterHTMLFS = emittedOnce(w.webContents, 'enter-html-full-screen');
+      const leaveHTMLFS = emittedOnce(w.webContents, 'leave-html-full-screen');
+
+      await webview.executeJavaScript('document.getElementById("div").requestFullscreen()', true);
       expect(await w.webContents.executeJavaScript('isIframeFullscreen()')).to.be.true();
 
+      await webview.executeJavaScript('document.exitFullscreen()');
+      await Promise.all([enterHTMLFS, leaveHTMLFS, parentFullscreen]);
+
+      const close = emittedOnce(w, 'closed');
       w.close();
-      await emittedOnce(w, 'closed');
+      await close;
     });
 
     // FIXME(zcbenz): Fullscreen events do not work on Linux.
@@ -468,8 +494,9 @@ describe('<webview> tag', function () {
       await delay(0);
       expect(w.isFullScreen()).to.be.false();
 
+      const close = emittedOnce(w, 'closed');
       w.close();
-      await emittedOnce(w, 'closed');
+      await close;
     });
 
     // Sending ESC via sendInputEvent only works on Windows.
@@ -485,8 +512,9 @@ describe('<webview> tag', function () {
       await delay(0);
       expect(w.isFullScreen()).to.be.false();
 
+      const close = emittedOnce(w, 'closed');
       w.close();
-      await emittedOnce(w, 'closed');
+      await close;
     });
 
     it('pressing ESC should emit the leave-html-full-screen event', async () => {
@@ -513,11 +541,27 @@ describe('<webview> tag', function () {
       const leaveFSWindow = emittedOnce(w, 'leave-html-full-screen');
       const leaveFSWebview = emittedOnce(webContents, 'leave-html-full-screen');
       webContents.sendInputEvent({ type: 'keyDown', keyCode: 'Escape' });
-      await leaveFSWindow;
       await leaveFSWebview;
+      await leaveFSWindow;
+
+      const close = emittedOnce(w, 'closed');
+      w.close();
+      await close;
+    });
+
+    it('should support user gesture', async () => {
+      const [w, webview] = await loadWebViewWindow();
+
+      const waitForEnterHtmlFullScreen = emittedOnce(webview, 'enter-html-full-screen');
+
+      const jsScript = "document.querySelector('video').webkitRequestFullscreen()";
+      webview.executeJavaScript(jsScript, true);
+
+      await waitForEnterHtmlFullScreen;
 
+      const close = emittedOnce(w, 'closed');
       w.close();
-      await emittedOnce(w, 'closed');
+      await close;
     });
   });
 

spec/fixtures/pages/fullscreen.html

@@ -1 +1 @@
-<video></video>
+<video></video>

spec/webview-spec.js

@@ -922,20 +922,6 @@ describe('<webview> tag', function () {
   });
 
   describe('executeJavaScript', () => {
-    it('should support user gesture', async () => {
-      await loadWebView(webview, {
-        src: `file://${fixtures}/pages/fullscreen.html`
-      });
-
-      // Event handler has to be added before js execution.
-      const waitForEnterHtmlFullScreen = waitForEvent(webview, 'enter-html-full-screen');
-
-      const jsScript = "document.querySelector('video').webkitRequestFullscreen()";
-      webview.executeJavaScript(jsScript, true);
-
-      return waitForEnterHtmlFullScreen;
-    });
-
     it('can return the result of the executed script', async () => {
       await loadWebView(webview, {
         src: 'about:blank'