From 21fcf51000bbdde1a0d6c0d3021666d0d2d01703 Mon Sep 17 00:00:00 2001 From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com> Date: Tue, 2 Aug 2022 11:33:40 -0400 Subject: [PATCH] fix: DCHECK entering fullscreen while loading url (#35164) * fix: DCHECK entering fullscreen while loading url * spec: fixup test Co-authored-by: Shelley Vohr --- 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 cca988ff758f7..4bb0e020aeff3 100644 --- a/shell/browser/api/electron_api_web_contents.cc +++ b/shell/browser/api/electron_api_web_contents.cc @@ -1340,6 +1340,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); @@ -3519,12 +3521,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 46212b1ab58fc..c94417e78b7bd 100644 --- a/shell/browser/native_window.cc +++ b/shell/browser/native_window.cc @@ -719,8 +719,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 7fee40576fdff..935cc9185ad89 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 e4a59c9ca0568..cdf30b2d730f6 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 e36dcee0b58ed..c6f9bce5eb8b4 100644 --- a/spec-main/api-browser-window-spec.ts +++ b/spec-main/api-browser-window-spec.ts @@ -4946,6 +4946,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');