From 7da0c2a07a8bfea91b3cebd88dddf7ee232c5cac Mon Sep 17 00:00:00 2001 From: Zoltan Papp Date: Mon, 14 Sep 2026 18:06:35 +0200 Subject: [PATCH] [client] Fix the Windows tray deadlock on re-entrant window creation (#7449) (#7537) * [client] Fix the Windows tray deadlock on re-entrant window creation The Wails systray runs the left-click handler synchronously inside the tray window procedure, and creating a window on a running app pumps a nested Win32 message loop while WebView2 initialises. ensureWindow held the non-reentrant createMu across that creation, so the second button-up of a double click re-entered ShowWindow from the pump and blocked the main thread on its own lock. A goroutine holding createMu while the main thread pumped, and the Open* dialogs holding mu across NewWithOptions, Show, Hide and InvokeSync, exposed the same inversion. WindowManager now serialises creation with a per-slot creating flag and queues the callers' operations until the window exists, and no Wails call runs while mu is held. The tray click and second-instance handlers call ShowWindow off the message loop. * [client] Serialize window operations while a slot is being created Callers arriving after the window is published but before the creator has drained the queue took the existing-window fast path and could run ahead of older queued operations, so a newer SetURL could be overwritten by an older one. withWindow now queues every caller while the creating flag is set and clears the flag only once the queue is seen empty under the lock. A factory panic or a nil window left the creating flag set and the slot dead; creation and drain now reset that state on early exit. hideOtherWindows records the windows it hid only when no restore ran in between, tracked by a generation counter, and re-shows them otherwise, so a restore racing the hide cannot strand hidden windows. * [misc] Run the client/ui subpackage tests in CI The three test workflows filtered the package list with a `/client/ui` prefix match, which dropped the subpackages along with the package that cannot compile without a frontend build. `services`, `preferences`, `i18n` and `authsession` all carry Go-side unit tests that never ran, including the window manager re-entrancy regression test. Anchor the pattern so only `client/ui` itself is excluded. The linux leg keeps the prefix match on 386, where only the 64-bit gtk4/webkitgtk dev packages are installed and the Wails application package would fail to link, and the alpine container job keeps it for the same reason. * [misc] Run the client/ui subpackage tests on a gtk4 4.10 runner The previous commit let the subpackages into the linux client job, where client/ui/services failed to build: the wails runtime's linux cgo layer uses GtkFileDialog, which arrived in gtk4 4.10, and the job's ubuntu-22.04 runner ships 4.6. Move them to their own job pinned to ubuntu-24.04 and restore the linux client job's original exclusion, leaving the 386 and privileged legs on the runner they have used since 2024. The new job needs no build cache, sudo or privileged tag, so it stays a few seconds long. Darwin and Windows keep the anchored pattern from the previous commit and already run these tests green, including the window manager re-entrancy regression test on the platform the deadlock was reported on. * [client] Defer a window close that lands while the window is still being created WindowManager publishes a dialog's slot only after the factory returns, and on Windows the factory blocks in the WebView2 embed pump. A Close* arriving in that gap found a nil slot and returned without doing anything, so the dialog appeared afterwards for a flow that had already been cancelled. The pre-fix Open* dialog functions held mu across the whole creation, which blocked a concurrent Close* until the slot was set; removing that lock hold reopened this gap. Close* now goes through closeWindow: while the slot is being created it records a closer in pendingClose, and finishCreation runs that closer before any queued operation, so a window that is going away is never shown and Wails never sees a Show on a destroyed window, which would recreate it. Ops queued behind a close are dropped; windowOp carries no factory, so they cannot be replayed into a new creation, and the frontend callers reissue on the next state change. The browser-login slot uses the same restoring closer from both CloseBrowserLogin and CloseRenewFlow, since the popup's WindowClosing hook only restores on a user close. Where two closers race one creation the first registered wins, so a later caller cannot replace a restoring closer with one that does not restore. (cherry picked from commit 2d28f9002a7a43f6a3da322b643efb6ab5242fe5) --- .github/workflows/golang-test-darwin.yml | 12 +- .github/workflows/golang-test-linux.yml | 39 +- .github/workflows/golang-test-windows.yml | 12 +- client/ui/main.go | 2 +- client/ui/services/windowmanager.go | 640 +++++++++++++--------- client/ui/services/windowmanager_test.go | 350 ++++++++++++ client/ui/tray_click_windows.go | 2 +- 7 files changed, 793 insertions(+), 264 deletions(-) create mode 100644 client/ui/services/windowmanager_test.go diff --git a/.github/workflows/golang-test-darwin.yml b/.github/workflows/golang-test-darwin.yml index c17d8e775..dec57dc80 100644 --- a/.github/workflows/golang-test-darwin.yml +++ b/.github/workflows/golang-test-darwin.yml @@ -46,15 +46,17 @@ jobs: run: git --no-pager diff --exit-code - name: Test - # Exclude client/ui: its main.go uses //go:embed all:frontend/dist, - # which fails to compile until the frontend has been built. The Wails UI - # has no Go-side unit tests, and its release pipeline runs `pnpm build` - # before goreleaser. + # Exclude the client/ui package itself: its main.go uses //go:embed + # all:frontend/dist, which fails to compile until the frontend has been + # built, and its release pipeline runs `pnpm build` before goreleaser. + # The pattern is anchored so the subpackages (services, preferences, + # i18n, authsession) still run: they hold Go-side unit tests and need no + # frontend bundle. # `go list -e` lets the listing succeed even though the embed fails to # resolve; the grep then drops the broken package by path. Without -e, # go list aborts with empty stdout and `go test` falls back to the repo # root, which has no Go files. - run: NETBIRD_STORE_ENGINE=${{ matrix.store }} CI=true go test -coverprofile=coverage.txt -tags 'devcert privileged' -exec 'sudo --preserve-env=CI,NETBIRD_STORE_ENGINE' -timeout 5m -p 1 $(go list -e ./... | grep -v -e /management -e /signal -e /relay -e /proxy -e /combined -e /client/ui -e /client/testutil/privileged) + run: NETBIRD_STORE_ENGINE=${{ matrix.store }} CI=true go test -coverprofile=coverage.txt -tags 'devcert privileged' -exec 'sudo --preserve-env=CI,NETBIRD_STORE_ENGINE' -timeout 5m -p 1 $(go list -e ./... | grep -v -e /management -e /signal -e /relay -e /proxy -e /combined -e '/client/ui$' -e /client/testutil/privileged) - name: Upload coverage reports to Codecov uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f #v7.0.0 diff --git a/.github/workflows/golang-test-linux.yml b/.github/workflows/golang-test-linux.yml index f24dfbe9d..9e3caa17a 100644 --- a/.github/workflows/golang-test-linux.yml +++ b/.github/workflows/golang-test-linux.yml @@ -160,9 +160,10 @@ jobs: - name: Test # Exclude client/ui: its main.go uses //go:embed all:frontend/dist, - # which fails to compile until the frontend has been built. The Wails UI - # has no Go-side unit tests, and its release pipeline runs `pnpm build` - # before goreleaser. + # which fails to compile until the frontend has been built, and its + # release pipeline runs `pnpm build` before goreleaser. The subpackages + # go with it because this runner's gtk4 is older than the wails runtime + # needs; the Client UI / Unit job below covers them instead. # `go list -e` lets the listing succeed even though the embed fails to # resolve; the grep then drops the broken package by path. Without -e, # go list aborts with empty stdout and `go test` falls back to the repo @@ -177,6 +178,35 @@ jobs: slug: netbirdio/netbird flags: unit,client + test_client_ui: + name: "Client UI / Unit" + # Pinned to 24.04 rather than the 22.04 the other client jobs use: the wails + # runtime's linux cgo layer needs GtkFileDialog, which arrived in gtk4 4.10, + # and jammy ships 4.6. Not ubuntu-latest, so a runner image rollover cannot + # move this out from under us. + runs-on: ubuntu-24.04 + steps: + - name: Checkout code + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false + + - name: Install Go + uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0 + with: + go-version-file: "go.mod" + cache: false + + - name: Install dependencies + run: sudo apt update && sudo apt install -y -q libgtk-4-dev libwebkitgtk-6.0-dev libsoup-3.0-dev + + - name: Test + # client/ui itself stays out: its main.go embeds all:frontend/dist, + # which only exists after `pnpm build`. The subpackages carry the + # Go-side unit tests, including the window manager re-entrancy + # regression test, and need no frontend bundle. + run: CGO_ENABLED=1 go test -timeout 5m ./client/ui/authsession/... ./client/ui/i18n/... ./client/ui/preferences/... ./client/ui/services/... + test_client_on_docker: name: "Client (Docker) / Unit" needs: [build-cache] @@ -211,6 +241,9 @@ jobs: ${{ runner.os }}-gotest-cache- - name: Run tests in container + # Unlike the native job above, this one drops all of client/ui including + # the subpackages: the alpine container has no gtk4/webkitgtk, so the + # Wails application package they import would fail to link. env: HOST_GOCACHE: ${{ steps.go-env.outputs.cache_dir }} HOST_GOMODCACHE: ${{ steps.go-env.outputs.modcache_dir }} diff --git a/.github/workflows/golang-test-windows.yml b/.github/workflows/golang-test-windows.yml index fb7b745d2..ca300f7df 100644 --- a/.github/workflows/golang-test-windows.yml +++ b/.github/workflows/golang-test-windows.yml @@ -66,15 +66,17 @@ jobs: - run: PsExec64 -s -w ${{ github.workspace }} C:\hostedtoolcache\windows\go\${{ steps.go.outputs.go-version }}\x64\bin\go.exe env -w GOCACHE=${{ env.modcache }} - run: PsExec64 -s -w ${{ github.workspace }} C:\hostedtoolcache\windows\go\${{ steps.go.outputs.go-version }}\x64\bin\go.exe mod tidy - name: Generate test script - # Exclude client/ui: its main.go uses //go:embed all:frontend/dist, - # which fails to compile until the frontend has been built. The Wails UI - # has no Go-side unit tests, and its release pipeline runs `pnpm build` - # before goreleaser. + # Exclude the client/ui package itself: its main.go uses //go:embed + # all:frontend/dist, which fails to compile until the frontend has been + # built, and its release pipeline runs `pnpm build` before goreleaser. + # The pattern is anchored so the subpackages (services, preferences, + # i18n, authsession) still run: they hold Go-side unit tests and need no + # frontend bundle. # `go list -e` lets the listing succeed even though the embed fails to # resolve; the Where-Object pipeline then drops the broken package by # path. Without -e, go list aborts with empty stdout. run: | - $packages = go list -e ./... | Where-Object { $_ -notmatch '/management' } | Where-Object { $_ -notmatch '/relay' } | Where-Object { $_ -notmatch '/signal' } | Where-Object { $_ -notmatch '/proxy' } | Where-Object { $_ -notmatch '/combined' } | Where-Object { $_ -notmatch '/client/ui' } + $packages = go list -e ./... | Where-Object { $_ -notmatch '/management' } | Where-Object { $_ -notmatch '/relay' } | Where-Object { $_ -notmatch '/signal' } | Where-Object { $_ -notmatch '/proxy' } | Where-Object { $_ -notmatch '/combined' } | Where-Object { $_ -notmatch '/client/ui$' } $goExe = "C:\hostedtoolcache\windows\go\${{ steps.go.outputs.go-version }}\x64\bin\go.exe" $cmd = "$goExe test -tags `"devcert privileged`" -timeout 10m -p 1 $($packages -join ' ') > test-out.txt 2>&1" Set-Content -Path "${{ github.workspace }}\run-tests.cmd" -Value $cmd diff --git a/client/ui/main.go b/client/ui/main.go index 5652efcf2..1a866e3a5 100644 --- a/client/ui/main.go +++ b/client/ui/main.go @@ -100,7 +100,7 @@ func main() { var tray *Tray app := newApplication(func() { if tray != nil { - tray.ShowWindow() + go tray.ShowWindow() } }) diff --git a/client/ui/services/windowmanager.go b/client/ui/services/windowmanager.go index 94dba6038..5c2e36785 100644 --- a/client/ui/services/windowmanager.go +++ b/client/ui/services/windowmanager.go @@ -21,6 +21,10 @@ type LanguageSubscriber interface { Subscribe() (<-chan preferences.UIPreferences, func()) } +type windowOp func(w *application.WebviewWindow, created bool) + +type windowCloser func(w *application.WebviewWindow) + // EventTriggerLogin asks the frontend's startLogin() to begin an SSO flow. const EventTriggerLogin = "trigger-login" @@ -36,6 +40,16 @@ const paintedFallback = 2 * time.Second const headlessTeardownDelay = 2 * time.Second +const ( + windowMain = "main" + windowSettings = "settings" + windowBrowserLogin = "browser-login" + windowSessionExpiration = "session-expiration" + windowInstallProgress = "install-progress" + windowWelcome = "welcome" + windowError = "error" +) + var WindowBackgroundColour = application.NewRGB(24, 26, 29) // bg-nb-gray-950 // WindowHeight is shared by the main and Settings windows. @@ -116,8 +130,11 @@ type WindowManager struct { // hiddenForLogin holds windows hidden while the BrowserLogin popup is open, restored on close. hiddenForLogin []application.Window mu sync.Mutex - createMu sync.Mutex newMain func(startURL string) *application.WebviewWindow + creating map[string]bool + pendingOps map[string][]windowOp + pendingClose map[string]windowCloser + restoreGen uint64 ready map[uint]bool showPending map[uint]bool pendingTab map[uint]string @@ -137,6 +154,9 @@ func NewWindowManager(app *application.App, mainWindow *application.WebviewWindo translator: translator, prefs: prefs, linuxIcon: linuxIcon, + creating: map[string]bool{}, + pendingOps: map[string][]windowOp{}, + pendingClose: map[string]windowCloser{}, ready: map[uint]bool{}, showPending: map[uint]bool{}, pendingTab: map[uint]string{}, @@ -165,7 +185,7 @@ func NewWindowManager(app *application.App, mainWindow *application.WebviewWindo func (s *WindowManager) newSettingsWindow() *application.WebviewWindow { w := s.app.Window.NewWithOptions(application.WebviewWindowOptions{ - Name: "settings", + Name: windowSettings, Title: s.title("window.title.settings"), Width: 900, Height: WindowHeight, @@ -186,6 +206,7 @@ func (s *WindowManager) newSettingsWindow() *application.WebviewWindow { s.forgetWindowLocked(w) s.mu.Unlock() }) + s.armReady(w) return w } @@ -197,63 +218,68 @@ func (s *WindowManager) OpenSettings(tab string) { target = "general" } - w, _ := s.ensureWindow(&s.settings, s.newSettingsWindow) + s.withWindow(windowSettings, &s.settings, s.newSettingsWindow, func(w *application.WebviewWindow, _ bool) { + s.mu.Lock() + ready := s.ready[w.ID()] + if !ready { + s.pendingTab[w.ID()] = target + } + s.mu.Unlock() - s.mu.Lock() - ready := s.ready[w.ID()] - if !ready { - s.pendingTab[w.ID()] = target - } - s.mu.Unlock() - - if ready { - s.app.Event.Emit(EventSettingsOpen, target) - } - s.showWhenReady(w) + if ready { + s.app.Event.Emit(EventSettingsOpen, target) + } + s.showWhenReady(w) + }) } // OpenBrowserLogin shows the SSO popup, creating it on first use. func (s *WindowManager) OpenBrowserLogin(uri string) { - s.mu.Lock() - defer s.mu.Unlock() - if s.browserLogin == nil { - startURL := "/#/dialog/browser-login" - if uri != "" { - startURL = "/#/dialog/browser-login?uri=" + url.QueryEscape(uri) - } - s.hideOtherWindowsLocked("browser-login") - opts := DialogWindowOptions("browser-login", s.title("window.title.signIn"), startURL, s.linuxIcon) - // Not always-on-top: it would obscure the browser tab the user logs in through. - opts.AlwaysOnTop = false - opts.InitialPosition = application.WindowCentered - // Open on the active (where users cursor is) display, like the session-expiration dialog. - opts.Screen = s.getScreenBasedOnCursorPosition() - s.browserLogin = s.app.Window.NewWithOptions(opts) - bl := s.browserLogin - bl.OnWindowEvent(events.Common.WindowClosing, func(_ *application.WindowEvent) { - s.mu.Lock() - // Only a live user red-X still has this registered; programmatic closers - // nil s.browserLogin first and clean up themselves. Guarding here stops a - // stale close event from wiping a replacement popup's state. - userClosed := s.browserLogin == bl - if userClosed { - s.browserLogin = nil - s.restoreHiddenWindowsLocked() - } - s.mu.Unlock() - if userClosed { - s.app.Event.Emit(EventBrowserLoginCancel) - } - }) - s.centerOnCursorScreen(s.browserLogin) - return - } + startURL := "/#/dialog/browser-login" if uri != "" { - s.browserLogin.SetURL("/#/dialog/browser-login?uri=" + url.QueryEscape(uri)) + startURL = "/#/dialog/browser-login?uri=" + url.QueryEscape(uri) } - s.centerOnCursorScreen(s.browserLogin) - s.browserLogin.Show() - s.browserLogin.Focus() + s.withWindow(windowBrowserLogin, &s.browserLogin, func() *application.WebviewWindow { + return s.newBrowserLoginWindow(startURL) + }, func(w *application.WebviewWindow, created bool) { + if created { + s.centerOnCursorScreen(w) + return + } + if uri != "" { + w.SetURL(startURL) + } + s.centerOnCursorScreen(w) + w.Show() + w.Focus() + }) +} + +func (s *WindowManager) newBrowserLoginWindow(startURL string) *application.WebviewWindow { + s.hideOtherWindows(windowBrowserLogin) + opts := DialogWindowOptions(windowBrowserLogin, s.title("window.title.signIn"), startURL, s.linuxIcon) + // Not always-on-top: it would obscure the browser tab the user logs in through. + opts.AlwaysOnTop = false + opts.InitialPosition = application.WindowCentered + // Open on the active (where users cursor is) display, like the session-expiration dialog. + opts.Screen = s.getScreenBasedOnCursorPosition() + w := s.app.Window.NewWithOptions(opts) + w.OnWindowEvent(events.Common.WindowClosing, func(_ *application.WindowEvent) { + s.mu.Lock() + // Only a live user red-X still has this registered; programmatic closers + // nil s.browserLogin first and clean up themselves. Guarding here stops a + // stale close event from wiping a replacement popup's state. + userClosed := s.browserLogin == w + if userClosed { + s.browserLogin = nil + } + s.mu.Unlock() + if userClosed { + s.restoreHiddenWindows() + s.app.Event.Emit(EventBrowserLoginCancel) + } + }) + return w } // BrowserLoginWindow returns the live SSO popup, or nil. While non-nil it is the @@ -273,71 +299,62 @@ func (s *WindowManager) InstallProgressWindow() *application.WebviewWindow { } func (s *WindowManager) CloseBrowserLogin() { - s.mu.Lock() - w := s.browserLogin - s.browserLogin = nil // The WindowClosing hook no-ops on a programmatic close, so restore here — // but only if a popup was actually open. The frontend calls this even when no // popup was ever shown (e.g. resetDialog() after an early RequestExtend failure, // or connection.ts's catch path), and hiddenForLogin is shared with // OpenInstallProgress, so an unconditional restore could re-show windows a // still-running install-progress is hiding. - if w != nil { - s.restoreHiddenWindowsLocked() - } - s.mu.Unlock() - if w != nil { - w.Close() - } + s.closeWindow(windowBrowserLogin, &s.browserLogin, s.restoreAndClose) } // OpenSessionExpiration shows the countdown warning on the cursor's display; seconds seeds // the countdown and deadlineUnixMilli (0 when unknown) is the absolute deadline the dialog // compares renewal snapshots against. Singleton, destroyed on close. func (s *WindowManager) OpenSessionExpiration(seconds int, deadlineUnixMilli int64) { - s.mu.Lock() - defer s.mu.Unlock() startURL := "/#/dialog/session-expiration?seconds=" + strconv.Itoa(seconds) if deadlineUnixMilli > 0 { startURL += "&deadline=" + strconv.FormatInt(deadlineUnixMilli, 10) } - if s.sessionExpiration == nil { - opts := DialogWindowOptions("session-expiration", s.title("window.title.sessionExpiration"), startURL, s.linuxIcon) - opts.Screen = s.getScreenBasedOnCursorPosition() - opts.InitialPosition = application.WindowCentered - s.sessionExpiration = s.app.Window.NewWithOptions(opts) - s.sessionExpiration.OnWindowEvent(events.Common.WindowClosing, func(_ *application.WindowEvent) { - s.mu.Lock() + s.withWindow(windowSessionExpiration, &s.sessionExpiration, func() *application.WebviewWindow { + return s.newSessionExpirationWindow(startURL) + }, func(w *application.WebviewWindow, created bool) { + if created { + s.centerOnCursorScreen(w) + return + } + w.SetURL(startURL) + s.centerOnCursorScreen(w) + w.Show() + w.Focus() + }) +} + +func (s *WindowManager) newSessionExpirationWindow(startURL string) *application.WebviewWindow { + opts := DialogWindowOptions(windowSessionExpiration, s.title("window.title.sessionExpiration"), startURL, s.linuxIcon) + opts.Screen = s.getScreenBasedOnCursorPosition() + opts.InitialPosition = application.WindowCentered + w := s.app.Window.NewWithOptions(opts) + w.OnWindowEvent(events.Common.WindowClosing, func(_ *application.WindowEvent) { + s.mu.Lock() + if s.sessionExpiration == w { s.sessionExpiration = nil - s.mu.Unlock() - }) - s.centerOnCursorScreen(s.sessionExpiration) - return - } - s.sessionExpiration.SetURL(startURL) - s.centerOnCursorScreen(s.sessionExpiration) - s.sessionExpiration.Show() - s.sessionExpiration.Focus() + } + s.mu.Unlock() + }) + return w } func (s *WindowManager) CloseSessionExpiration() { - s.mu.Lock() - w := s.sessionExpiration - s.sessionExpiration = nil - s.mu.Unlock() - if w != nil { - w.Close() - } + s.closeWindow(windowSessionExpiration, &s.sessionExpiration, closeOnly) } // CloseRenewFlow tears down the SSO session-renewal UI in a single call: it // closes the browser-login popup and the session-expiration window together. func (s *WindowManager) CloseRenewFlow() { s.mu.Lock() - bl := s.browserLogin - se := s.sessionExpiration - s.browserLogin = nil - s.sessionExpiration = nil + bl := s.takeWindowLocked(windowBrowserLogin, &s.browserLogin, s.restoreAndClose) + se := s.takeWindowLocked(windowSessionExpiration, &s.sessionExpiration, closeOnly) if se != nil { kept := s.hiddenForLogin[:0] for _, w := range s.hiddenForLogin { @@ -347,9 +364,9 @@ func (s *WindowManager) CloseRenewFlow() { } s.hiddenForLogin = kept } - s.restoreHiddenWindowsLocked() s.mu.Unlock() + s.restoreHiddenWindows() // Close after unlock so the re-entrant handlers can take s.mu. if bl != nil { bl.Close() @@ -362,73 +379,70 @@ func (s *WindowManager) CloseRenewFlow() { // OpenInstallProgress shows the install-progress window and hides the rest for the duration // (restored on close). It owns its own result polling since the daemon restarts mid-install. func (s *WindowManager) OpenInstallProgress(version string) { - s.mu.Lock() - defer s.mu.Unlock() startURL := "/#/dialog/install-progress" if version != "" { startURL = "/#/dialog/install-progress?version=" + url.QueryEscape(version) } - if s.installProgress == nil { - s.hideOtherWindowsLocked("install-progress") - s.installProgress = s.app.Window.NewWithOptions( - DialogWindowOptions("install-progress", s.title("window.title.updating"), startURL, s.linuxIcon), - ) - s.installProgress.OnWindowEvent(events.Common.WindowClosing, func(_ *application.WindowEvent) { - s.mu.Lock() + s.withWindow(windowInstallProgress, &s.installProgress, func() *application.WebviewWindow { + return s.newInstallProgressWindow(startURL) + }, func(w *application.WebviewWindow, created bool) { + if !created { + w.SetURL(startURL) + w.Show() + w.Focus() + } + s.centerWhenReady(w) + }) +} + +func (s *WindowManager) newInstallProgressWindow(startURL string) *application.WebviewWindow { + s.hideOtherWindows(windowInstallProgress) + w := s.app.Window.NewWithOptions( + DialogWindowOptions(windowInstallProgress, s.title("window.title.updating"), startURL, s.linuxIcon), + ) + w.OnWindowEvent(events.Common.WindowClosing, func(_ *application.WindowEvent) { + s.mu.Lock() + if s.installProgress == w { s.installProgress = nil - s.restoreHiddenWindowsLocked() - s.mu.Unlock() - }) - s.centerWhenReady(s.installProgress) - return - } - s.installProgress.SetURL(startURL) - s.installProgress.Show() - s.installProgress.Focus() - s.centerWhenReady(s.installProgress) + } + s.mu.Unlock() + s.restoreHiddenWindows() + }) + return w } func (s *WindowManager) CloseInstallProgress() { - s.mu.Lock() - w := s.installProgress - s.installProgress = nil - s.mu.Unlock() - if w != nil { - w.Close() - } + s.closeWindow(windowInstallProgress, &s.installProgress, closeOnly) } // OpenWelcome shows the first-launch onboarding window. Singleton, destroyed on close. func (s *WindowManager) OpenWelcome() { - s.mu.Lock() - defer s.mu.Unlock() - if s.welcome == nil { - opts := DialogWindowOptions("welcome", s.title("window.title.welcome"), "/#/dialog/welcome", s.linuxIcon) - opts.Width = 420 - opts.InitialPosition = application.WindowCentered - s.welcome = s.app.Window.NewWithOptions(opts) - w := s.welcome - w.OnWindowEvent(events.Common.WindowClosing, func(_ *application.WindowEvent) { - s.mu.Lock() + s.withWindow(windowWelcome, &s.welcome, s.newWelcomeWindow, func(w *application.WebviewWindow, created bool) { + if !created { + w.Show() + w.Focus() + } + s.centerWhenReady(w) + }) +} + +func (s *WindowManager) newWelcomeWindow() *application.WebviewWindow { + opts := DialogWindowOptions(windowWelcome, s.title("window.title.welcome"), "/#/dialog/welcome", s.linuxIcon) + opts.Width = 420 + opts.InitialPosition = application.WindowCentered + w := s.app.Window.NewWithOptions(opts) + w.OnWindowEvent(events.Common.WindowClosing, func(_ *application.WindowEvent) { + s.mu.Lock() + if s.welcome == w { s.welcome = nil - s.mu.Unlock() - }) - s.centerWhenReady(s.welcome) - return - } - s.welcome.Show() - s.welcome.Focus() - s.centerWhenReady(s.welcome) + } + s.mu.Unlock() + }) + return w } func (s *WindowManager) CloseWelcome() { - s.mu.Lock() - w := s.welcome - s.welcome = nil - s.mu.Unlock() - if w != nil { - w.Close() - } + s.closeWindow(windowWelcome, &s.welcome, closeOnly) } // OpenError shows the custom error dialog; title/message/command are pre-localised @@ -439,35 +453,35 @@ func (s *WindowManager) OpenError(title, message, command string) { if ShuttingDown() { return } - s.mu.Lock() - defer s.mu.Unlock() startURL := errorDialogURL(title, message, command) - if s.errorDialog == nil { - s.errorDialog = s.app.Window.NewWithOptions( - DialogWindowOptions("error", s.title("window.title.error"), startURL, s.linuxIcon), - ) - s.errorDialog.OnWindowEvent(events.Common.WindowClosing, func(_ *application.WindowEvent) { - s.mu.Lock() + s.withWindow(windowError, &s.errorDialog, func() *application.WebviewWindow { + return s.newErrorWindow(startURL) + }, func(w *application.WebviewWindow, created bool) { + if !created { + w.SetURL(startURL) + w.Show() + w.Focus() + } + s.centerWhenReady(w) + }) +} + +func (s *WindowManager) newErrorWindow(startURL string) *application.WebviewWindow { + w := s.app.Window.NewWithOptions( + DialogWindowOptions(windowError, s.title("window.title.error"), startURL, s.linuxIcon), + ) + w.OnWindowEvent(events.Common.WindowClosing, func(_ *application.WindowEvent) { + s.mu.Lock() + if s.errorDialog == w { s.errorDialog = nil - s.mu.Unlock() - }) - s.centerWhenReady(s.errorDialog) - return - } - s.errorDialog.SetURL(startURL) - s.errorDialog.Show() - s.errorDialog.Focus() - s.centerWhenReady(s.errorDialog) + } + s.mu.Unlock() + }) + return w } func (s *WindowManager) CloseError() { - s.mu.Lock() - w := s.errorDialog - s.errorDialog = nil - s.mu.Unlock() - if w != nil { - w.Close() - } + s.closeWindow(windowError, &s.errorDialog, closeOnly) } // OpenMain brings the main window forward; the welcome handoff uses it instead of the tray. @@ -478,65 +492,171 @@ func (s *WindowManager) OpenMain() { // ShowMain brings the main window forward (re-centering on minimal WMs). The single entry // point every surface (tray, SIGUSR1, welcome) should use so centering applies uniformly. func (s *WindowManager) ShowMain() { - s.showWhenReady(s.MainWindow()) + s.ensureMain("/", func(w *application.WebviewWindow, _ bool) { + s.showWhenReady(w) + }) } // ShowMainAndEmit brings the main window forward and emits event once its frontend is ready. func (s *WindowManager) ShowMainAndEmit(event string) { - w := s.MainWindow() - if w == nil { - return - } + s.ensureMain("/", func(w *application.WebviewWindow, _ bool) { + id := w.ID() + s.mu.Lock() + ready := s.ready[id] + if !ready { + s.pendingEmits[id] = append(s.pendingEmits[id], event) + } + s.mu.Unlock() - id := w.ID() - s.mu.Lock() - ready := s.ready[id] - if !ready { - s.pendingEmits[id] = append(s.pendingEmits[id], event) - } - s.mu.Unlock() - - s.showWhenReady(w) - if ready { - s.app.Event.Emit(event) - } + s.showWhenReady(w) + if ready { + s.app.Event.Emit(event) + } + }) } func (s *WindowManager) MainWindow() *application.WebviewWindow { - w, _ := s.ensureMain("/") - return w + s.mu.Lock() + defer s.mu.Unlock() + return s.mainWindow } -func (s *WindowManager) ensureMain(startURL string) (*application.WebviewWindow, bool) { +func (s *WindowManager) ensureMain(startURL string, op windowOp) { s.mu.Lock() factory := s.newMain s.mu.Unlock() if factory == nil { - return s.ensureWindow(&s.mainWindow, nil) + s.withWindow(windowMain, &s.mainWindow, nil, op) + return } - return s.ensureWindow(&s.mainWindow, func() *application.WebviewWindow { - return factory(startURL) - }) + s.withWindow(windowMain, &s.mainWindow, func() *application.WebviewWindow { + w := factory(startURL) + s.armReady(w) + return w + }, op) } -func (s *WindowManager) ensureWindow(slot **application.WebviewWindow, factory func() *application.WebviewWindow) (*application.WebviewWindow, bool) { - s.createMu.Lock() - defer s.createMu.Unlock() - +func (s *WindowManager) withWindow(name string, slot **application.WebviewWindow, factory func() *application.WebviewWindow, op windowOp) { s.mu.Lock() - w := *slot - s.mu.Unlock() - if w != nil || factory == nil { - return w, false + if s.creating[name] { + s.pendingOps[name] = append(s.pendingOps[name], op) + s.mu.Unlock() + return } + if w := *slot; w != nil { + s.mu.Unlock() + op(w, false) + return + } + if factory == nil { + s.mu.Unlock() + return + } + s.creating[name] = true + s.mu.Unlock() - w = factory() - s.armReady(w) + w := s.createWindow(name, slot, factory) + if w == nil { + return + } + s.finishCreation(name, slot, w, op) +} +func (s *WindowManager) createWindow(name string, slot **application.WebviewWindow, factory func() *application.WebviewWindow) *application.WebviewWindow { + created := false + defer func() { + if created { + return + } + s.mu.Lock() + s.releaseCreationLocked(name) + s.mu.Unlock() + }() + + w := factory() + if w == nil { + return nil + } s.mu.Lock() *slot = w s.mu.Unlock() - return w, true + created = true + return w +} + +func (s *WindowManager) finishCreation(name string, slot **application.WebviewWindow, w *application.WebviewWindow, op windowOp) { + finished := false + defer func() { + if finished { + return + } + s.mu.Lock() + s.releaseCreationLocked(name) + s.mu.Unlock() + }() + + created := true + for { + s.mu.Lock() + if closer := s.pendingClose[name]; closer != nil { + if *slot == w { + *slot = nil + } + s.releaseCreationLocked(name) + finished = true + s.mu.Unlock() + closer(w) + return + } + var next windowOp + switch { + case created: + next = op + case len(s.pendingOps[name]) > 0: + next = s.pendingOps[name][0] + s.pendingOps[name] = s.pendingOps[name][1:] + default: + s.releaseCreationLocked(name) + finished = true + s.mu.Unlock() + return + } + s.mu.Unlock() + next(w, created) + created = false + } +} + +func (s *WindowManager) closeWindow(name string, slot **application.WebviewWindow, closer windowCloser) { + s.mu.Lock() + w := s.takeWindowLocked(name, slot, closer) + s.mu.Unlock() + if w != nil { + closer(w) + } +} + +func (s *WindowManager) takeWindowLocked(name string, slot **application.WebviewWindow, closer windowCloser) *application.WebviewWindow { + if s.creating[name] { + if s.pendingClose[name] == nil { + s.pendingClose[name] = closer + } + return nil + } + w := *slot + *slot = nil + return w +} + +func (s *WindowManager) releaseCreationLocked(name string) { + delete(s.creating, name) + delete(s.pendingOps, name) + delete(s.pendingClose, name) +} + +func (s *WindowManager) restoreAndClose(w *application.WebviewWindow) { + s.restoreHiddenWindows() + w.Close() } func (s *WindowManager) armReady(w *application.WebviewWindow) { @@ -576,24 +696,21 @@ func (s *WindowManager) watchTriggerLogin() { return } - w, created := s.ensureMain("/") - if w == nil { - return - } + s.ensureMain("/", func(w *application.WebviewWindow, created bool) { + s.mu.Lock() + if created { + s.headlessMain = true + } + pending := !s.ready[w.ID()] + if pending { + s.pendingEmits[w.ID()] = append(s.pendingEmits[w.ID()], EventTriggerLogin) + } + s.mu.Unlock() - s.mu.Lock() - if created { - s.headlessMain = true - } - pending := !s.ready[w.ID()] - if pending { - s.pendingEmits[w.ID()] = append(s.pendingEmits[w.ID()], EventTriggerLogin) - } - s.mu.Unlock() - - if !pending { - s.app.Event.Emit(EventTriggerLogin) - } + if !pending { + s.app.Event.Emit(EventTriggerLogin) + } + }) }) s.app.Event.On(EventBrowserLoginCancel, func(_ *application.CustomEvent) { @@ -664,9 +781,9 @@ func (s *WindowManager) windowByName(name string) *application.WebviewWindow { s.mu.Lock() defer s.mu.Unlock() switch name { - case "main": + case windowMain: return s.mainWindow - case "settings": + case windowSettings: return s.settings default: return nil @@ -741,14 +858,12 @@ func (s *WindowManager) showNow(w *application.WebviewWindow) { } func (s *WindowManager) ShowMainAt(url string) { - w, created := s.ensureMain(url) - if w == nil { - return - } - if !created { - w.SetURL(url) - } - s.showWhenReady(w) + s.ensureMain(url, func(w *application.WebviewWindow, created bool) { + if !created { + w.SetURL(url) + } + s.showWhenReady(w) + }) } func (s *WindowManager) SetMainFactory(f func(startURL string) *application.WebviewWindow) { @@ -868,39 +983,61 @@ func (s *WindowManager) retitleAll() { } } -// hideOtherWindowsLocked hides every visible window except keepName, recording -// them in hiddenForLogin for restoreHiddenWindowsLocked. Caller must hold s.mu. -func (s *WindowManager) hideOtherWindowsLocked(keepName string) { +func (s *WindowManager) hideOtherWindows(keepName string) { + s.mu.Lock() + gen := s.restoreGen + s.mu.Unlock() + + var hidden []application.Window for _, w := range s.app.Window.GetAll() { - if w == nil || w.Name() == keepName { - continue - } - if !w.IsVisible() { + if w == nil || w.Name() == keepName || !w.IsVisible() { continue } w.Hide() - s.hiddenForLogin = append(s.hiddenForLogin, w) + hidden = append(hidden, w) + } + if len(hidden) == 0 { + return + } + + s.mu.Lock() + restored := s.restoreGen != gen + if !restored { + s.hiddenForLogin = append(s.hiddenForLogin, hidden...) + } + s.mu.Unlock() + if !restored { + return + } + for _, w := range hidden { + w.Show() } } -// restoreHiddenWindowsLocked re-shows windows hidden by hideOtherWindowsLocked -// (caller holds s.mu). If the main window was among them, raiseToForeground -// lifts it above the SSO browser, which still owns the foreground — a plain -// Show/Focus would be demoted to a taskbar flash and leave it stranded behind. -func (s *WindowManager) restoreHiddenWindowsLocked() { +// restoreHiddenWindows re-shows windows hidden by hideOtherWindows. If the main +// window was among them, raiseToForeground lifts it above the SSO browser, which +// still owns the foreground — a plain Show/Focus would be demoted to a taskbar +// flash and leave it stranded behind. +func (s *WindowManager) restoreHiddenWindows() { + s.mu.Lock() + hidden := s.hiddenForLogin + s.hiddenForLogin = nil + s.restoreGen++ + mainWindow := s.mainWindow + s.mu.Unlock() + mainRestored := false - for _, w := range s.hiddenForLogin { + for _, w := range hidden { if w == nil { continue } w.Show() - if w == s.mainWindow { + if w == mainWindow { mainRestored = true } } - s.hiddenForLogin = nil - if mainRestored && s.mainWindow != nil { - raiseToForeground(s.mainWindow) + if mainRestored && mainWindow != nil { + raiseToForeground(mainWindow) } } @@ -915,8 +1052,11 @@ func (s *WindowManager) getScreenBasedOnCursorPosition() *application.Screen { return sc } } - if s.mainWindow != nil { - if sc, err := s.mainWindow.GetScreen(); err == nil { + s.mu.Lock() + mainWindow := s.mainWindow + s.mu.Unlock() + if mainWindow != nil { + if sc, err := mainWindow.GetScreen(); err == nil { return sc } } @@ -944,3 +1084,5 @@ func errorDialogURL(title, message, command string) string { // u32ptr returns a pointer to v, for the optional *uint32 Wails theme fields. func u32ptr(v uint32) *uint32 { return &v } + +func closeOnly(w *application.WebviewWindow) { w.Close() } diff --git a/client/ui/services/windowmanager_test.go b/client/ui/services/windowmanager_test.go new file mode 100644 index 000000000..13c8548ab --- /dev/null +++ b/client/ui/services/windowmanager_test.go @@ -0,0 +1,350 @@ +//go:build !android && !ios && !freebsd && !js + +package services + +import ( + "fmt" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/require" + "github.com/wailsapp/wails/v3/pkg/application" +) + +func newTestWindowManager() *WindowManager { + return &WindowManager{ + creating: map[string]bool{}, + pendingOps: map[string][]windowOp{}, + pendingClose: map[string]windowCloser{}, + } +} + +func waitDone(t *testing.T, done <-chan struct{}, msg string) { + t.Helper() + select { + case <-done: + case <-time.After(5 * time.Second): + t.Fatal(msg) + } +} + +func TestWithWindowReusesExistingWindow(t *testing.T) { + s := newTestWindowManager() + existing := &application.WebviewWindow{} + slot := existing + factoryCalls := 0 + var got *application.WebviewWindow + created := true + s.withWindow(windowMain, &slot, func() *application.WebviewWindow { + factoryCalls++ + return &application.WebviewWindow{} + }, func(w *application.WebviewWindow, c bool) { + got, created = w, c + }) + require.Equal(t, 0, factoryCalls) + require.Same(t, existing, got) + require.False(t, created) +} + +func TestWithWindowNilFactoryWithoutWindowSkipsOp(t *testing.T) { + s := newTestWindowManager() + var slot *application.WebviewWindow + opCalls := 0 + s.withWindow(windowMain, &slot, nil, func(*application.WebviewWindow, bool) { + opCalls++ + }) + require.Equal(t, 0, opCalls) + require.Nil(t, slot) +} + +func TestWithWindowReentrantCallDuringCreationIsQueued(t *testing.T) { + s := newTestWindowManager() + var slot *application.WebviewWindow + factoryCalls := 0 + var order []string + var factory func() *application.WebviewWindow + factory = func() *application.WebviewWindow { + factoryCalls++ + // Simulates the Windows message pump re-entering the tray click handler + // while WebView2 is still initialising the window being created. + s.withWindow(windowMain, &slot, factory, func(_ *application.WebviewWindow, created bool) { + order = append(order, fmt.Sprintf("reentrant:%v", created)) + }) + return &application.WebviewWindow{} + } + + done := make(chan struct{}) + go func() { + defer close(done) + s.withWindow(windowMain, &slot, factory, func(_ *application.WebviewWindow, created bool) { + order = append(order, fmt.Sprintf("outer:%v", created)) + }) + }() + waitDone(t, done, "withWindow deadlocked on a re-entrant call during creation") + + require.Equal(t, 1, factoryCalls) + require.Equal(t, []string{"outer:true", "reentrant:false"}, order) + require.NotNil(t, slot) + require.Empty(t, s.creating) + require.Empty(t, s.pendingOps) +} + +func TestWithWindowConcurrentCallersShareOneCreation(t *testing.T) { + s := newTestWindowManager() + var slot *application.WebviewWindow + factoryEntered := make(chan struct{}) + release := make(chan struct{}) + var factoryCalls, opCalls atomic.Int32 + factory := func() *application.WebviewWindow { + factoryCalls.Add(1) + close(factoryEntered) + <-release + return &application.WebviewWindow{} + } + op := func(*application.WebviewWindow, bool) { opCalls.Add(1) } + + first := make(chan struct{}) + go func() { + defer close(first) + s.withWindow(windowSettings, &slot, factory, op) + }() + <-factoryEntered + + second := make(chan struct{}) + go func() { + defer close(second) + s.withWindow(windowSettings, &slot, factory, op) + }() + waitDone(t, second, "second caller blocked while the window was being created") + require.Equal(t, int32(0), opCalls.Load()) + + close(release) + waitDone(t, first, "creator did not finish") + + require.Equal(t, int32(1), factoryCalls.Load()) + require.Equal(t, int32(2), opCalls.Load()) + require.NotNil(t, slot) +} + +func TestWithWindowOpsQueuedDuringCreationRunInArrivalOrder(t *testing.T) { + s := newTestWindowManager() + var slot *application.WebviewWindow + var order []string + record := func(label string) windowOp { + return func(_ *application.WebviewWindow, created bool) { + order = append(order, fmt.Sprintf("%s:%v", label, created)) + } + } + var factory func() *application.WebviewWindow + factory = func() *application.WebviewWindow { + s.withWindow(windowMain, &slot, factory, func(w *application.WebviewWindow, created bool) { + record("a")(w, created) + // Arrives while the creator is still draining the queue: it must not + // jump ahead of "b" through the existing-window fast path. + s.withWindow(windowMain, &slot, factory, record("c")) + }) + s.withWindow(windowMain, &slot, factory, record("b")) + return &application.WebviewWindow{} + } + + done := make(chan struct{}) + go func() { + defer close(done) + s.withWindow(windowMain, &slot, factory, record("outer")) + }() + waitDone(t, done, "withWindow deadlocked while draining queued operations") + + require.Equal(t, []string{"outer:true", "a:false", "b:false", "c:false"}, order) + require.Empty(t, s.creating) + require.Empty(t, s.pendingOps) +} + +func TestWithWindowFactoryPanicReleasesCreation(t *testing.T) { + s := newTestWindowManager() + var slot *application.WebviewWindow + func() { + defer func() { require.NotNil(t, recover()) }() + s.withWindow(windowMain, &slot, func() *application.WebviewWindow { + panic("factory failed") + }, func(*application.WebviewWindow, bool) {}) + }() + require.Empty(t, s.creating) + require.Empty(t, s.pendingOps) + require.Nil(t, slot) + + created := false + s.withWindow(windowMain, &slot, func() *application.WebviewWindow { + return &application.WebviewWindow{} + }, func(_ *application.WebviewWindow, c bool) { + created = c + }) + require.True(t, created) + require.NotNil(t, slot) +} + +func TestWithWindowNilFromFactoryReleasesCreation(t *testing.T) { + s := newTestWindowManager() + var slot *application.WebviewWindow + opCalls := 0 + s.withWindow(windowMain, &slot, func() *application.WebviewWindow { + return nil + }, func(*application.WebviewWindow, bool) { + opCalls++ + }) + require.Equal(t, 0, opCalls) + require.Empty(t, s.creating) + require.Nil(t, slot) +} + +func TestCloseWindowDuringCreationDefersCloseAndSkipsOps(t *testing.T) { + s := newTestWindowManager() + var slot *application.WebviewWindow + created := &application.WebviewWindow{} + opCalls, closeCalls := 0, 0 + var closed *application.WebviewWindow + s.withWindow(windowError, &slot, func() *application.WebviewWindow { + s.closeWindow(windowError, &slot, func(w *application.WebviewWindow) { + closeCalls++ + closed = w + }) + require.Equal(t, 0, closeCalls) + return created + }, func(*application.WebviewWindow, bool) { + opCalls++ + }) + require.Equal(t, 0, opCalls) + require.Equal(t, 1, closeCalls) + require.Same(t, created, closed) + require.Nil(t, slot) + require.Empty(t, s.creating) + require.Empty(t, s.pendingOps) + require.Empty(t, s.pendingClose) + + factoryCalls := 0 + reopened := false + s.withWindow(windowError, &slot, func() *application.WebviewWindow { + factoryCalls++ + return &application.WebviewWindow{} + }, func(_ *application.WebviewWindow, c bool) { + reopened = c + }) + require.Equal(t, 1, factoryCalls) + require.True(t, reopened) + require.NotNil(t, slot) +} + +func TestCloseWindowDuringDrainStopsRemainingOps(t *testing.T) { + s := newTestWindowManager() + var slot *application.WebviewWindow + var order []string + closeCalls := 0 + var factory func() *application.WebviewWindow + factory = func() *application.WebviewWindow { + s.withWindow(windowWelcome, &slot, factory, func(*application.WebviewWindow, bool) { + order = append(order, "a") + s.closeWindow(windowWelcome, &slot, func(*application.WebviewWindow) { closeCalls++ }) + s.withWindow(windowWelcome, &slot, factory, func(*application.WebviewWindow, bool) { + order = append(order, "c") + }) + }) + s.withWindow(windowWelcome, &slot, factory, func(*application.WebviewWindow, bool) { + order = append(order, "b") + }) + return &application.WebviewWindow{} + } + s.withWindow(windowWelcome, &slot, factory, func(*application.WebviewWindow, bool) { + order = append(order, "outer") + }) + + // "b" was queued before the close and "c" after it; a close supersedes both + // rather than showing a window that is about to be destroyed. + require.Equal(t, []string{"outer", "a"}, order) + require.Equal(t, 1, closeCalls) + require.Nil(t, slot) + require.Empty(t, s.creating) + require.Empty(t, s.pendingOps) + require.Empty(t, s.pendingClose) +} + +func TestCloseWindowWithoutWindowSkipsCloser(t *testing.T) { + s := newTestWindowManager() + var slot *application.WebviewWindow + calls := 0 + s.closeWindow(windowBrowserLogin, &slot, func(*application.WebviewWindow) { calls++ }) + require.Equal(t, 0, calls) + require.Nil(t, slot) + require.Empty(t, s.pendingClose) +} + +func TestCloseWindowWithExistingWindowRunsCloser(t *testing.T) { + s := newTestWindowManager() + existing := &application.WebviewWindow{} + slot := existing + var got *application.WebviewWindow + s.closeWindow(windowError, &slot, func(w *application.WebviewWindow) { got = w }) + require.Same(t, existing, got) + require.Nil(t, slot) + require.Empty(t, s.pendingClose) +} + +func TestWithWindowNilFromFactoryDropsPendingClose(t *testing.T) { + s := newTestWindowManager() + var slot *application.WebviewWindow + closeCalls := 0 + s.withWindow(windowError, &slot, func() *application.WebviewWindow { + s.closeWindow(windowError, &slot, func(*application.WebviewWindow) { closeCalls++ }) + return nil + }, func(*application.WebviewWindow, bool) {}) + require.Equal(t, 0, closeCalls) + require.Nil(t, slot) + require.Empty(t, s.creating) + require.Empty(t, s.pendingClose) +} + +func TestWithWindowFactoryPanicDropsPendingClose(t *testing.T) { + s := newTestWindowManager() + var slot *application.WebviewWindow + closeCalls := 0 + func() { + defer func() { require.NotNil(t, recover()) }() + s.withWindow(windowError, &slot, func() *application.WebviewWindow { + s.closeWindow(windowError, &slot, func(*application.WebviewWindow) { closeCalls++ }) + panic("factory failed") + }, func(*application.WebviewWindow, bool) {}) + }() + require.Equal(t, 0, closeCalls) + require.Empty(t, s.creating) + require.Empty(t, s.pendingClose) +} + +func TestCloseWindowKeepsFirstDeferredCloser(t *testing.T) { + s := newTestWindowManager() + var slot *application.WebviewWindow + var ran []string + s.withWindow(windowError, &slot, func() *application.WebviewWindow { + s.closeWindow(windowError, &slot, func(*application.WebviewWindow) { ran = append(ran, "first") }) + s.closeWindow(windowError, &slot, func(*application.WebviewWindow) { ran = append(ran, "second") }) + return &application.WebviewWindow{} + }, func(*application.WebviewWindow, bool) {}) + require.Equal(t, []string{"first"}, ran) + require.Nil(t, slot) + require.Empty(t, s.pendingClose) +} + +func TestCloseRenewFlowDuringBrowserLoginCreationRestoresHiddenWindows(t *testing.T) { + s := newTestWindowManager() + s.withWindow(windowBrowserLogin, &s.browserLogin, func() *application.WebviewWindow { + s.CloseRenewFlow() + // Seeded after the call so the deferred closer, not CloseRenewFlow's own + // immediate restore, is what has to drain it. A nil entry is skipped by + // restoreHiddenWindows, so no Wails window is needed. + s.hiddenForLogin = []application.Window{nil} + return &application.WebviewWindow{} + }, func(*application.WebviewWindow, bool) {}) + + require.Nil(t, s.browserLogin) + require.Empty(t, s.hiddenForLogin) + require.Empty(t, s.creating) + require.Empty(t, s.pendingClose) +} diff --git a/client/ui/tray_click_windows.go b/client/ui/tray_click_windows.go index 17a6dc5df..04c66dd8a 100644 --- a/client/ui/tray_click_windows.go +++ b/client/ui/tray_click_windows.go @@ -4,5 +4,5 @@ package main // Open application window on left click, right click opens the tray menu func bindTrayClick(t *Tray) { - t.tray.OnClick(func() { t.ShowWindow() }) + t.tray.OnClick(func() { go t.ShowWindow() }) }