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 09a589506..74a87b4df 100644 --- a/client/ui/main.go +++ b/client/ui/main.go @@ -101,7 +101,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 1ba2ffdc0..24319dae0 100644 --- a/client/ui/services/windowmanager.go +++ b/client/ui/services/windowmanager.go @@ -22,6 +22,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" @@ -37,6 +41,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" +) + // Window background per effective appearance. Both match the body background // (bg-nb-gray DEFAULT) in globals.css so opaque native pixels and the webview // paint the same surface; keep the three in sync. @@ -202,8 +216,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 @@ -223,6 +240,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{}, @@ -252,7 +272,7 @@ func NewWindowManager(app *application.App, mainWindow *application.WebviewWindo func (s *WindowManager) newSettingsWindow() *application.WebviewWindow { a := CurrentAppearance() w := s.app.Window.NewWithOptions(application.WebviewWindowOptions{ - Name: "settings", + Name: windowSettings, Title: s.title("window.title.settings"), Width: 900, Height: WindowHeight, @@ -273,6 +293,7 @@ func (s *WindowManager) newSettingsWindow() *application.WebviewWindow { s.forgetWindowLocked(w) s.mu.Unlock() }) + s.armReady(w) return w } @@ -284,63 +305,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 @@ -360,71 +386,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 { @@ -434,9 +451,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() @@ -449,73 +466,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 @@ -526,35 +540,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. @@ -565,65 +579,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) { @@ -663,24 +783,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) { @@ -751,9 +868,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 @@ -828,14 +945,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) { @@ -955,39 +1070,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) } } @@ -1002,8 +1139,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 } } @@ -1031,3 +1171,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() }) }