From 5f2ec00c0f0f054abc0dcadf5fb57bf1a7d2d346 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Tue, 7 Jun 2022 18:59:50 +0200 Subject: [PATCH] fix: html fullscreen transitions stacking --- .../browser/api/electron_api_web_contents.cc | 10 ++- shell/browser/native_window.cc | 9 +++ shell/browser/native_window.h | 16 +++++ shell/browser/native_window_mac.h | 13 ---- shell/browser/native_window_mac.mm | 14 ---- .../ui/cocoa/electron_ns_window_delegate.mm | 8 +-- spec-main/api-browser-window-spec.ts | 67 ++++++++++++++++++- spec-main/chromium-spec.ts | 33 ++++++++- .../fixtures/webview/fullscreen/frame.html | 1 + spec-main/webview-spec.ts | 58 ++++++++++++++-- spec/fixtures/pages/fullscreen.html | 2 +- spec/webview-spec.js | 14 ---- 12 files changed, 184 insertions(+), 61 deletions(-) diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc index dffb80de27128..4bb29525e2304 100644 --- a/shell/browser/api/electron_api_web_contents.cc +++ b/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; diff --git a/shell/browser/native_window.cc b/shell/browser/native_window.cc index 1ca962fa92746..46212b1ab58fc 100644 --- a/shell/browser/native_window.cc +++ b/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; diff --git a/shell/browser/native_window.h b/shell/browser/native_window.h index 762552656a5df..7fee40576fdff 100644 --- a/shell/browser/native_window.h +++ b/shell/browser/native_window.h @@ -7,6 +7,7 @@ #include #include +#include #include #include @@ -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 pending_transitions_; + FullScreenTransitionState fullscreen_transition_state_ = + FullScreenTransitionState::NONE; + private: std::unique_ptr widget_; diff --git a/shell/browser/native_window_mac.h b/shell/browser/native_window_mac.h index e9c6245085caa..2e6b45375bbd4 100644 --- a/shell/browser/native_window_mac.h +++ b/shell/browser/native_window_mac.h @@ -8,7 +8,6 @@ #import #include -#include #include #include @@ -173,11 +172,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; @@ -248,13 +242,6 @@ class NativeWindowMac : public NativeWindow, bool zoom_to_page_width_ = false; absl::optional traffic_light_position_; - std::queue 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 diff --git a/shell/browser/native_window_mac.mm b/shell/browser/native_window_mac.mm index a740aa263b327..a4858ff76a23e 100644 --- a/shell/browser/native_window_mac.mm +++ b/shell/browser/native_window_mac.mm @@ -586,11 +586,6 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) { return [window_ isVisible] && !occluded && !IsMinimized(); } -void NativeWindowMac::SetFullScreenTransitionState( - FullScreenTransitionState state) { - fullscreen_transition_state_ = state; -} - bool NativeWindowMac::IsEnabled() { return [window_ attachedSheet] == nil; } @@ -674,15 +669,6 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) { 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); diff --git a/shell/browser/ui/cocoa/electron_ns_window_delegate.mm b/shell/browser/ui/cocoa/electron_ns_window_delegate.mm index 79b2bc8faecc1..5243065940251 100644 --- a/shell/browser/ui/cocoa/electron_ns_window_delegate.mm +++ b/shell/browser/ui/cocoa/electron_ns_window_delegate.mm @@ -235,7 +235,7 @@ - (void)windowDidEndLiveResize:(NSNotification*)notification { } - (void)windowWillEnterFullScreen:(NSNotification*)notification { - shell_->SetFullScreenTransitionState(FullScreenTransitionState::ENTERING); + shell_->set_fullscreen_transition_state(FullScreenTransitionState::ENTERING); shell_->NotifyWindowWillEnterFullScreen(); @@ -245,7 +245,7 @@ - (void)windowWillEnterFullScreen:(NSNotification*)notification { } - (void)windowDidEnterFullScreen:(NSNotification*)notification { - shell_->SetFullScreenTransitionState(FullScreenTransitionState::NONE); + shell_->set_fullscreen_transition_state(FullScreenTransitionState::NONE); shell_->NotifyWindowEnterFullScreen(); @@ -256,13 +256,13 @@ - (void)windowDidEnterFullScreen:(NSNotification*)notification { } - (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(); diff --git a/spec-main/api-browser-window-spec.ts b/spec-main/api-browser-window-spec.ts index 9d48cb77e6cb6..08d7dd8f36f7c 100644 --- a/spec-main/api-browser-window-spec.ts +++ b/spec-main/api-browser-window-spec.ts @@ -4957,19 +4957,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(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 () => { diff --git a/spec-main/chromium-spec.ts b/spec-main/chromium-spec.ts index 0776d13bf53ee..4eccfe439d05e 100644 --- a/spec-main/chromium-spec.ts +++ b/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 = ''; @@ -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 = + ''; + 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; diff --git a/spec-main/fixtures/webview/fullscreen/frame.html b/spec-main/fixtures/webview/fullscreen/frame.html index c92571eef4a8b..d7307f0b83bc4 100644 --- a/spec-main/fixtures/webview/fullscreen/frame.html +++ b/spec-main/fixtures/webview/fullscreen/frame.html @@ -2,6 +2,7 @@
WebView
+