From 7588bb7425522959206daaa25d7e1f36c7015934 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Mon, 1 Aug 2022 22:52:58 +0200 Subject: [PATCH] fix: DCHECK entering fullscreen while loading url (#35111) * fix: DCHECK entering fullscreen while loading url * spec: fixup test --- shell/browser/api/electron_api_web_contents.cc | 15 ++++++++++----- shell/browser/native_window.cc | 4 +++- shell/browser/native_window.h | 16 ++++++++++++++-- .../ui/cocoa/electron_ns_window_delegate.mm | 2 +- spec-main/api-browser-window-spec.ts | 17 +++++++++++++++++ 5 files changed, 45 insertions(+), 9 deletions(-) diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc index 243db0489238d..b7a03e906e2a1 100644 --- a/shell/browser/api/electron_api_web_contents.cc +++ b/shell/browser/api/electron_api_web_contents.cc @@ -1327,6 +1327,8 @@ void WebContents::OnEnterFullscreenModeForTab( return; } + owner_window()->set_fullscreen_transition_type( + NativeWindow::FullScreenTransitionType::HTML); exclusive_access_manager_->fullscreen_controller()->EnterFullscreenModeForTab( requesting_frame, options.display_id); @@ -3537,12 +3539,15 @@ void WebContents::EnumerateDirectory( bool WebContents::IsFullscreenForTabOrPending( const content::WebContents* source) { - bool transition_fs = owner_window() - ? owner_window()->fullscreen_transition_state() != - NativeWindow::FullScreenTransitionState::NONE - : false; + if (!owner_window()) + return html_fullscreen_; + + bool in_transition = owner_window()->fullscreen_transition_state() != + NativeWindow::FullScreenTransitionState::NONE; + bool is_html_transition = owner_window()->fullscreen_transition_type() == + NativeWindow::FullScreenTransitionType::HTML; - return html_fullscreen_ || transition_fs; + return html_fullscreen_ || (in_transition && is_html_transition); } bool WebContents::TakeFocus(content::WebContents* source, bool reverse) { diff --git a/shell/browser/native_window.cc b/shell/browser/native_window.cc index 67439e818700e..931e7759073cb 100644 --- a/shell/browser/native_window.cc +++ b/shell/browser/native_window.cc @@ -718,8 +718,10 @@ std::string NativeWindow::GetAccessibleTitle() { } void NativeWindow::HandlePendingFullscreenTransitions() { - if (pending_transitions_.empty()) + if (pending_transitions_.empty()) { + set_fullscreen_transition_type(FullScreenTransitionType::NONE); return; + } bool next_transition = pending_transitions_.front(); pending_transitions_.pop(); diff --git a/shell/browser/native_window.h b/shell/browser/native_window.h index 13de702f18863..6568f03127bf3 100644 --- a/shell/browser/native_window.h +++ b/shell/browser/native_window.h @@ -318,10 +318,11 @@ class NativeWindow : public base::SupportsUserData, observers_.RemoveObserver(obs); } - enum class FullScreenTransitionState { ENTERING, EXITING, NONE }; - // Handle fullscreen transitions. void HandlePendingFullscreenTransitions(); + + enum class FullScreenTransitionState { ENTERING, EXITING, NONE }; + void set_fullscreen_transition_state(FullScreenTransitionState state) { fullscreen_transition_state_ = state; } @@ -329,6 +330,15 @@ class NativeWindow : public base::SupportsUserData, return fullscreen_transition_state_; } + enum class FullScreenTransitionType { HTML, NATIVE, NONE }; + + void set_fullscreen_transition_type(FullScreenTransitionType type) { + fullscreen_transition_type_ = type; + } + FullScreenTransitionType fullscreen_transition_type() const { + return fullscreen_transition_type_; + } + views::Widget* widget() const { return widget_.get(); } views::View* content_view() const { return content_view_; } @@ -390,6 +400,8 @@ class NativeWindow : public base::SupportsUserData, std::queue pending_transitions_; FullScreenTransitionState fullscreen_transition_state_ = FullScreenTransitionState::NONE; + FullScreenTransitionType fullscreen_transition_type_ = + FullScreenTransitionType::NONE; private: std::unique_ptr widget_; diff --git a/shell/browser/ui/cocoa/electron_ns_window_delegate.mm b/shell/browser/ui/cocoa/electron_ns_window_delegate.mm index 362f464f91c82..243c2d3993dc3 100644 --- a/shell/browser/ui/cocoa/electron_ns_window_delegate.mm +++ b/shell/browser/ui/cocoa/electron_ns_window_delegate.mm @@ -19,7 +19,7 @@ using TitleBarStyle = electron::NativeWindowMac::TitleBarStyle; using FullScreenTransitionState = - electron::NativeWindowMac::FullScreenTransitionState; + electron::NativeWindow::FullScreenTransitionState; @implementation ElectronNSWindowDelegate diff --git a/spec-main/api-browser-window-spec.ts b/spec-main/api-browser-window-spec.ts index 993c6ed2c3800..2ab728239b196 100644 --- a/spec-main/api-browser-window-spec.ts +++ b/spec-main/api-browser-window-spec.ts @@ -4954,6 +4954,23 @@ describe('BrowserWindow module', () => { await leaveFullScreen; }); + it('should be able to load a URL while transitioning to fullscreen', async () => { + const w = new BrowserWindow({ fullscreen: true }); + w.loadFile(path.join(fixtures, 'pages', 'c.html')); + + const load = emittedOnce(w.webContents, 'did-finish-load'); + const enterFS = emittedOnce(w, 'enter-full-screen'); + + await Promise.all([enterFS, load]); + expect(w.fullScreen).to.be.true(); + + await delay(); + + const leaveFullScreen = emittedOnce(w, 'leave-full-screen'); + w.setFullScreen(false); + await leaveFullScreen; + }); + it('can be changed with setFullScreen method', async () => { const w = new BrowserWindow(); const enterFullScreen = emittedOnce(w, 'enter-full-screen');