diff --git a/shell/browser/api/electron_api_base_window.cc b/shell/browser/api/electron_api_base_window.cc index d4a51d41be..4fe4fd3ad6 100644 --- a/shell/browser/api/electron_api_base_window.cc +++ b/shell/browser/api/electron_api_base_window.cc @@ -321,8 +321,8 @@ void BaseWindow::OnWindowLeaveHtmlFullScreen() { Emit("leave-html-full-screen"); } -void BaseWindow::OnWindowAlwaysOnTopChanged() { - Emit("always-on-top-changed", IsAlwaysOnTop()); +void BaseWindow::OnWindowAlwaysOnTopChanged(const bool is_always_on_top) { + Emit("always-on-top-changed", is_always_on_top); } void BaseWindow::OnExecuteAppCommand(const std::string_view command_name) { diff --git a/shell/browser/api/electron_api_base_window.h b/shell/browser/api/electron_api_base_window.h index 83cf03e035..b62d812781 100644 --- a/shell/browser/api/electron_api_base_window.h +++ b/shell/browser/api/electron_api_base_window.h @@ -91,7 +91,7 @@ class BaseWindow : public gin_helper::TrackableObject, void OnWindowLeaveFullScreen() override; void OnWindowEnterHtmlFullScreen() override; void OnWindowLeaveHtmlFullScreen() override; - void OnWindowAlwaysOnTopChanged() override; + void OnWindowAlwaysOnTopChanged(bool is_always_on_top) override; void OnExecuteAppCommand(std::string_view command_name) override; void OnTouchBarItemResult(const std::string& item_id, const base::DictValue& details) override; diff --git a/shell/browser/native_window.cc b/shell/browser/native_window.cc index fbb0974558..c4608a8da1 100644 --- a/shell/browser/native_window.cc +++ b/shell/browser/native_window.cc @@ -596,8 +596,9 @@ void NativeWindow::NotifyWindowLeaveHtmlFullScreen() { observers_.Notify(&NativeWindowObserver::OnWindowLeaveHtmlFullScreen); } -void NativeWindow::NotifyWindowAlwaysOnTopChanged() { - observers_.Notify(&NativeWindowObserver::OnWindowAlwaysOnTopChanged); +void NativeWindow::NotifyWindowAlwaysOnTopChanged(const bool is_always_on_top) { + observers_.Notify(&NativeWindowObserver::OnWindowAlwaysOnTopChanged, + is_always_on_top); } void NativeWindow::NotifyWindowExecuteAppCommand( diff --git a/shell/browser/native_window.h b/shell/browser/native_window.h index a2715b1ba1..22e159cbec 100644 --- a/shell/browser/native_window.h +++ b/shell/browser/native_window.h @@ -332,7 +332,7 @@ class NativeWindow : public views::WidgetDelegate { virtual void NotifyWindowLeaveFullScreen(); void NotifyWindowEnterHtmlFullScreen(); void NotifyWindowLeaveHtmlFullScreen(); - void NotifyWindowAlwaysOnTopChanged(); + void NotifyWindowAlwaysOnTopChanged(bool is_always_on_top); void NotifyWindowExecuteAppCommand(std::string_view command_name); void NotifyTouchBarItemInteraction(const std::string& item_id, base::DictValue details); diff --git a/shell/browser/native_window_mac.mm b/shell/browser/native_window_mac.mm index b8df24768e..8244fe1a2d 100644 --- a/shell/browser/native_window_mac.mm +++ b/shell/browser/native_window_mac.mm @@ -974,10 +974,9 @@ void NativeWindowMac::SetWindowLevel(int unbounded_level) { // a bug of Cocoa or macOS. SetMaximizable(was_maximizable_); - // This must be notified at the very end or IsAlwaysOnTop - // will not yet have been updated to reflect the new status if (did_z_order_level_change) - NativeWindow::NotifyWindowAlwaysOnTopChanged(); + NativeWindow::NotifyWindowAlwaysOnTopChanged(z_order_level != + ui::ZOrderLevel::kNormal); } ui::ZOrderLevel NativeWindowMac::GetZOrderLevel() const { diff --git a/shell/browser/native_window_observer.h b/shell/browser/native_window_observer.h index 957e4f83e4..82bb2aed9c 100644 --- a/shell/browser/native_window_observer.h +++ b/shell/browser/native_window_observer.h @@ -94,7 +94,7 @@ class NativeWindowObserver : public base::CheckedObserver { virtual void OnWindowLeaveFullScreen() {} virtual void OnWindowEnterHtmlFullScreen() {} virtual void OnWindowLeaveHtmlFullScreen() {} - virtual void OnWindowAlwaysOnTopChanged() {} + virtual void OnWindowAlwaysOnTopChanged(bool is_always_on_top) {} virtual void OnTouchBarItemResult(const std::string& item_id, const base::DictValue& details) {} virtual void OnNewWindowForTab() {} diff --git a/shell/browser/native_window_views.cc b/shell/browser/native_window_views.cc index 1c52d002d4..c3f9f7fd48 100644 --- a/shell/browser/native_window_views.cc +++ b/shell/browser/native_window_views.cc @@ -1134,16 +1134,18 @@ bool NativeWindowViews::IsClosable() const { #endif } -void NativeWindowViews::SetAlwaysOnTop(ui::ZOrderLevel z_order, +void NativeWindowViews::SetAlwaysOnTop(const ui::ZOrderLevel z_order, const std::string& level, - int relativeLevel) { - bool level_changed = z_order != widget()->GetZOrderLevel(); + const int relativeLevel) { + const bool level_changed = z_order != widget()->GetZOrderLevel(); + const bool always_on_top = z_order != ui::ZOrderLevel::kNormal; + widget()->SetZOrderLevel(z_order); #if BUILDFLAG(IS_WIN) // Reset the placement flag. behind_task_bar_ = false; - if (z_order != ui::ZOrderLevel::kNormal) { + if (always_on_top) { // On macOS the window is placed behind the Dock for the following levels. // Re-use the same names on Windows to make it easier for the user. static constexpr auto levels = base::MakeFixedFlatSet( @@ -1153,10 +1155,8 @@ void NativeWindowViews::SetAlwaysOnTop(ui::ZOrderLevel z_order, #endif MoveBehindTaskBarIfNeeded(); - // This must be notified at the very end or IsAlwaysOnTop - // will not yet have been updated to reflect the new status if (level_changed) - NativeWindow::NotifyWindowAlwaysOnTopChanged(); + NativeWindow::NotifyWindowAlwaysOnTopChanged(always_on_top); } ui::ZOrderLevel NativeWindowViews::GetZOrderLevel() const { diff --git a/spec/api-browser-window-spec.ts b/spec/api-browser-window-spec.ts index 57d2ebc3ce..d303a98480 100755 --- a/spec/api-browser-window-spec.ts +++ b/spec/api-browser-window-spec.ts @@ -2800,6 +2800,26 @@ describe('BrowserWindow module', () => { describe('BrowserWindow.setAlwaysOnTop(flag, level)', () => { let w: BrowserWindow; + const alwaysOnTopSettleTimeout = 5000; + + const waitForAlwaysOnTop = async (alwaysOnTop: boolean, label: string) => { + try { + await waitUntil(() => w.isAlwaysOnTop() === alwaysOnTop, { + rate: 50, + timeout: alwaysOnTopSettleTimeout + }); + } catch (error) { + throw new Error(`${label}: ${(error as Error).message}`); + } + }; + + const setAlwaysOnTopAndWaitForState = async (alwaysOnTop: boolean, label: string) => { + const alwaysOnTopChanged = once(w, 'always-on-top-changed') as Promise<[any, boolean]>; + w.setAlwaysOnTop(alwaysOnTop); + const [, emittedAlwaysOnTop] = await alwaysOnTopChanged; + expect(emittedAlwaysOnTop).to.equal(alwaysOnTop, `${label}: unexpected event payload`); + await waitForAlwaysOnTop(alwaysOnTop, label); + }; afterEach(closeAllWindows); @@ -2832,13 +2852,20 @@ describe('BrowserWindow module', () => { }); it('causes the right value to be emitted on `always-on-top-changed`', async () => { - const alwaysOnTopChanged = once(w, 'always-on-top-changed') as Promise<[any, boolean]>; expect(w.isAlwaysOnTop()).to.be.false('is alwaysOnTop'); - w.setAlwaysOnTop(true); - const [, alwaysOnTop] = await alwaysOnTopChanged; - expect(alwaysOnTop).to.be.true('is not alwaysOnTop'); + await setAlwaysOnTopAndWaitForState(true, 'single transition'); }); + ifit(process.platform === 'win32')( + 'eventually becomes consistent with the emitted value after enable and disable transitions', + async () => { + expect(w.isAlwaysOnTop()).to.be.false('is alwaysOnTop'); + + await setAlwaysOnTopAndWaitForState(true, 'enable'); + await setAlwaysOnTopAndWaitForState(false, 'disable'); + } + ); + ifit(process.platform === 'darwin')('honors the alwaysOnTop level of a child window', () => { w = new BrowserWindow({ show: false }); const c = new BrowserWindow({ parent: w });