Merge sanitized crash stability history
28 files · +1038 -88
M internal/infrastructure/cef/app.go +18 -0
| @@ -448,12 +448,30 @@ func isBrowseRelaunchCommandLine(commandLine purecef.CommandLine) bool { | |||
| 448 | 448 | return len(args) >= 2 && args[1] == "browse" | |
| 449 | 449 | } | |
| 450 | 450 | ||
| 451 | + | func isGPUProcessRelaunchCommandLine(commandLine purecef.CommandLine) bool { | |
| 452 | + | if commandLine == nil { | |
| 453 | + | return false | |
| 454 | + | } | |
| 455 | + | for _, arg := range parseRelaunchCommandLineArgs(commandLine.GetCommandLineString()) { | |
| 456 | + | if arg == "--type=gpu-process" { | |
| 457 | + | return true | |
| 458 | + | } | |
| 459 | + | } | |
| 460 | + | return false | |
| 461 | + | } | |
| 462 | + | ||
| 451 | 463 | func parseBrowseURLFromRelaunchCommandLine(commandLine purecef.CommandLine) string { | |
| 452 | 464 | browseURL, _ := parseBrowseRelaunchCommandLine(commandLine) | |
| 453 | 465 | return browseURL | |
| 454 | 466 | } | |
| 455 | 467 | ||
| 456 | 468 | func (h *dumberBPH) OnAlreadyRunningAppRelaunch(commandLine purecef.CommandLine, _ string) int32 { | |
| 469 | + | if isGPUProcessRelaunchCommandLine(commandLine) { | |
| 470 | + | if h != nil && h.engine != nil { | |
| 471 | + | h.engine.recordGPURelaunch() | |
| 472 | + | } | |
| 473 | + | return 0 | |
| 474 | + | } | |
| 457 | 475 | if browseURL, ok := parseBrowseRelaunchCommandLine(commandLine); ok { | |
| 458 | 476 | if h != nil && h.engine != nil { | |
| 459 | 477 | if handler := h.engine.alreadyRunningAppRelaunchCallback(); handler != nil { | |
| @@ -448,12 +448,30 @@ func isBrowseRelaunchCommandLine(commandLine purecef.CommandLine) bool { | |||||
| 448 | return len(args) >= 2 && args[1] == "browse" | 448 | return len(args) >= 2 && args[1] == "browse" | ||
| 449 | } | 449 | } | ||
| 450 | 450 | ||||
| 451 | + | func isGPUProcessRelaunchCommandLine(commandLine purecef.CommandLine) bool { | |||
| 452 | + | if commandLine == nil { | |||
| 453 | + | return false | |||
| 454 | + | } | |||
| 455 | + | for _, arg := range parseRelaunchCommandLineArgs(commandLine.GetCommandLineString()) { | |||
| 456 | + | if arg == "--type=gpu-process" { | |||
| 457 | + | return true | |||
| 458 | + | } | |||
| 459 | + | } | |||
| 460 | + | return false | |||
| 461 | + | } | |||
| 462 | + | ||||
| 451 | func parseBrowseURLFromRelaunchCommandLine(commandLine purecef.CommandLine) string { | 463 | func parseBrowseURLFromRelaunchCommandLine(commandLine purecef.CommandLine) string { | ||
| 452 | browseURL, _ := parseBrowseRelaunchCommandLine(commandLine) | 464 | browseURL, _ := parseBrowseRelaunchCommandLine(commandLine) | ||
| 453 | return browseURL | 465 | return browseURL | ||
| 454 | } | 466 | } | ||
| 455 | 467 | ||||
| 456 | func (h *dumberBPH) OnAlreadyRunningAppRelaunch(commandLine purecef.CommandLine, _ string) int32 { | 468 | func (h *dumberBPH) OnAlreadyRunningAppRelaunch(commandLine purecef.CommandLine, _ string) int32 { | ||
| 469 | + | if isGPUProcessRelaunchCommandLine(commandLine) { | |||
| 470 | + | if h != nil && h.engine != nil { | |||
| 471 | + | h.engine.recordGPURelaunch() | |||
| 472 | + | } | |||
| 473 | + | return 0 | |||
| 474 | + | } | |||
| 457 | if browseURL, ok := parseBrowseRelaunchCommandLine(commandLine); ok { | 475 | if browseURL, ok := parseBrowseRelaunchCommandLine(commandLine); ok { | ||
| 458 | if h != nil && h.engine != nil { | 476 | if h != nil && h.engine != nil { | ||
| 459 | if handler := h.engine.alreadyRunningAppRelaunchCallback(); handler != nil { | 477 | if handler := h.engine.alreadyRunningAppRelaunchCallback(); handler != nil { | ||
M internal/infrastructure/cef/engine.go +13 -0
| @@ -31,6 +31,7 @@ type Engine struct { | |||
| 31 | 31 | pool *WebViewPool | |
| 32 | 32 | profileLogDir string | |
| 33 | 33 | runtimeCEFDir string | |
| 34 | + | stateRoot string | |
| 34 | 35 | renderStackPlan cef2gtk.RenderStackPlan | |
| 35 | 36 | applicationScaleMu sync.RWMutex | |
| 36 | 37 | applicationScale float64 | |
| @@ -54,6 +55,7 @@ type Engine struct { | |||
| 54 | 55 | downloadHandler *downloadHandler | |
| 55 | 56 | alreadyRunningAppRelaunchMu sync.RWMutex | |
| 56 | 57 | alreadyRunningAppRelaunchHandler func(string) | |
| 58 | + | gpuRelaunchCount atomic.Int32 | |
| 57 | 59 | ||
| 58 | 60 | // activeWebViews tracks all live webviews for CSS broadcast. | |
| 59 | 61 | activeWebViews sync.Map // map[port.WebViewID]*WebView | |
| @@ -145,6 +147,17 @@ func (e *Engine) alreadyRunningAppRelaunchCallback() func(string) { | |||
| 145 | 147 | return e.alreadyRunningAppRelaunchHandler | |
| 146 | 148 | } | |
| 147 | 149 | ||
| 150 | + | // recordGPURelaunch asks the next engine start to use Chromium's conservative | |
| 151 | + | // defaults after two GPU-process relaunch attempts in one running instance. | |
| 152 | + | func (e *Engine) recordGPURelaunch() { | |
| 153 | + | if e == nil || e.gpuRelaunchCount.Add(1) != 2 { | |
| 154 | + | return | |
| 155 | + | } | |
| 156 | + | if err := writeNextStartSafetyMarker(e.stateRoot); err != nil { | |
| 157 | + | logging.FromContext(e.ctx).Warn().Err(err).Msg("cef: failed to mark next start safe after repeated GPU relaunch") | |
| 158 | + | } | |
| 159 | + | } | |
| 160 | + | ||
| 148 | 161 | // registerWebView adds a webview to the active tracking map. | |
| 149 | 162 | func (e *Engine) registerWebView(wv *WebView) { | |
| 150 | 163 | e.activeWebViews.Store(wv.id, wv) | |
| @@ -31,6 +31,7 @@ type Engine struct { | |||||
| 31 | pool *WebViewPool | 31 | pool *WebViewPool | ||
| 32 | profileLogDir string | 32 | profileLogDir string | ||
| 33 | runtimeCEFDir string | 33 | runtimeCEFDir string | ||
| 34 | + | stateRoot string | |||
| 34 | renderStackPlan cef2gtk.RenderStackPlan | 35 | renderStackPlan cef2gtk.RenderStackPlan | ||
| 35 | applicationScaleMu sync.RWMutex | 36 | applicationScaleMu sync.RWMutex | ||
| 36 | applicationScale float64 | 37 | applicationScale float64 | ||
| @@ -54,6 +55,7 @@ type Engine struct { | |||||
| 54 | downloadHandler *downloadHandler | 55 | downloadHandler *downloadHandler | ||
| 55 | alreadyRunningAppRelaunchMu sync.RWMutex | 56 | alreadyRunningAppRelaunchMu sync.RWMutex | ||
| 56 | alreadyRunningAppRelaunchHandler func(string) | 57 | alreadyRunningAppRelaunchHandler func(string) | ||
| 58 | + | gpuRelaunchCount atomic.Int32 | |||
| 57 | 59 | ||||
| 58 | // activeWebViews tracks all live webviews for CSS broadcast. | 60 | // activeWebViews tracks all live webviews for CSS broadcast. | ||
| 59 | activeWebViews sync.Map // map[port.WebViewID]*WebView | 61 | activeWebViews sync.Map // map[port.WebViewID]*WebView | ||
| @@ -145,6 +147,17 @@ func (e *Engine) alreadyRunningAppRelaunchCallback() func(string) { | |||||
| 145 | return e.alreadyRunningAppRelaunchHandler | 147 | return e.alreadyRunningAppRelaunchHandler | ||
| 146 | } | 148 | } | ||
| 147 | 149 | ||||
| 150 | + | // recordGPURelaunch asks the next engine start to use Chromium's conservative | |||
| 151 | + | // defaults after two GPU-process relaunch attempts in one running instance. | |||
| 152 | + | func (e *Engine) recordGPURelaunch() { | |||
| 153 | + | if e == nil || e.gpuRelaunchCount.Add(1) != 2 { | |||
| 154 | + | return | |||
| 155 | + | } | |||
| 156 | + | if err := writeNextStartSafetyMarker(e.stateRoot); err != nil { | |||
| 157 | + | logging.FromContext(e.ctx).Warn().Err(err).Msg("cef: failed to mark next start safe after repeated GPU relaunch") | |||
| 158 | + | } | |||
| 159 | + | } | |||
| 160 | + | ||||
| 148 | // registerWebView adds a webview to the active tracking map. | 161 | // registerWebView adds a webview to the active tracking map. | ||
| 149 | func (e *Engine) registerWebView(wv *WebView) { | 162 | func (e *Engine) registerWebView(wv *WebView) { | ||
| 150 | e.activeWebViews.Store(wv.id, wv) | 163 | e.activeWebViews.Store(wv.id, wv) | ||
M internal/infrastructure/cef/engine_init.go +5 -0
| @@ -45,6 +45,10 @@ func NewEngine( | |||
| 45 | 45 | ) (*Engine, error) { | |
| 46 | 46 | logger := logging.FromContext(ctx) | |
| 47 | 47 | stateRoot := resolvedStateRoot(paths.StateRoot, opts) | |
| 48 | + | if consumeNextStartSafetyMarker(stateRoot) { | |
| 49 | + | applyNextStartSafetyEnvironment() | |
| 50 | + | logger.Warn().Str("state_root", stateRoot).Msg("cef: applying next-start GPU safety recovery") | |
| 51 | + | } | |
| 48 | 52 | cleanStaleSingletonLocks(logger, stateRoot) | |
| 49 | 53 | windowlessFrameRate := normalizedWindowlessFrameRate(cfg.WindowlessFrameRate) | |
| 50 | 54 | renderStackPlan, err := resolveCEFRenderStackPlan(cfg.RenderStack) | |
| @@ -67,6 +71,7 @@ func NewEngine( | |||
| 67 | 71 | ctx: ctx, | |
| 68 | 72 | profileLogDir: paths.ProfileLogDir, | |
| 69 | 73 | runtimeCEFDir: settings.CEFDir, | |
| 74 | + | stateRoot: stateRoot, | |
| 70 | 75 | renderStackPlan: renderStackPlan, | |
| 71 | 76 | applicationScale: normalizedApplicationScale(cfg.ApplicationScale), | |
| 72 | 77 | registerHandlers: deps.RegisterHandlers, | |
| @@ -45,6 +45,10 @@ func NewEngine( | |||||
| 45 | ) (*Engine, error) { | 45 | ) (*Engine, error) { | ||
| 46 | logger := logging.FromContext(ctx) | 46 | logger := logging.FromContext(ctx) | ||
| 47 | stateRoot := resolvedStateRoot(paths.StateRoot, opts) | 47 | stateRoot := resolvedStateRoot(paths.StateRoot, opts) | ||
| 48 | + | if consumeNextStartSafetyMarker(stateRoot) { | |||
| 49 | + | applyNextStartSafetyEnvironment() | |||
| 50 | + | logger.Warn().Str("state_root", stateRoot).Msg("cef: applying next-start GPU safety recovery") | |||
| 51 | + | } | |||
| 48 | cleanStaleSingletonLocks(logger, stateRoot) | 52 | cleanStaleSingletonLocks(logger, stateRoot) | ||
| 49 | windowlessFrameRate := normalizedWindowlessFrameRate(cfg.WindowlessFrameRate) | 53 | windowlessFrameRate := normalizedWindowlessFrameRate(cfg.WindowlessFrameRate) | ||
| 50 | renderStackPlan, err := resolveCEFRenderStackPlan(cfg.RenderStack) | 54 | renderStackPlan, err := resolveCEFRenderStackPlan(cfg.RenderStack) | ||
| @@ -67,6 +71,7 @@ func NewEngine( | |||||
| 67 | ctx: ctx, | 71 | ctx: ctx, | ||
| 68 | profileLogDir: paths.ProfileLogDir, | 72 | profileLogDir: paths.ProfileLogDir, | ||
| 69 | runtimeCEFDir: settings.CEFDir, | 73 | runtimeCEFDir: settings.CEFDir, | ||
| 74 | + | stateRoot: stateRoot, | |||
| 70 | renderStackPlan: renderStackPlan, | 75 | renderStackPlan: renderStackPlan, | ||
| 71 | applicationScale: normalizedApplicationScale(cfg.ApplicationScale), | 76 | applicationScale: normalizedApplicationScale(cfg.ApplicationScale), | ||
| 72 | registerHandlers: deps.RegisterHandlers, | 77 | registerHandlers: deps.RegisterHandlers, | ||
A internal/infrastructure/cef/restart_safety.go +68 -0
| @@ -0,0 +1,68 @@ | |||
| 1 | + | package cef | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "errors" | |
| 5 | + | "os" | |
| 6 | + | "path/filepath" | |
| 7 | + | ) | |
| 8 | + | ||
| 9 | + | const ( | |
| 10 | + | nextStartSafetyMarkerFile = "cef-next-start-safe" | |
| 11 | + | safetyMarkerDirPerm = 0o700 | |
| 12 | + | safetyMarkerFilePerm = 0o600 | |
| 13 | + | ) | |
| 14 | + | ||
| 15 | + | // writeNextStartSafetyMarker records a one-shot recovery request for the next | |
| 16 | + | // CEF start. The marker lives beside CEF's root cache so it follows explicit | |
| 17 | + | // per-session cache roots as well as the normal profile. | |
| 18 | + | func writeNextStartSafetyMarker(stateRoot string) error { | |
| 19 | + | if stateRoot == "" { | |
| 20 | + | return errors.New("CEF state root is empty") | |
| 21 | + | } | |
| 22 | + | if err := os.MkdirAll(stateRoot, safetyMarkerDirPerm); err != nil { | |
| 23 | + | return err | |
| 24 | + | } | |
| 25 | + | ||
| 26 | + | marker := filepath.Join(stateRoot, nextStartSafetyMarkerFile) | |
| 27 | + | temporary, err := os.CreateTemp(stateRoot, ".cef-next-start-safe-*") | |
| 28 | + | if err != nil { | |
| 29 | + | return err | |
| 30 | + | } | |
| 31 | + | temporaryName := temporary.Name() | |
| 32 | + | defer func() { _ = os.Remove(temporaryName) }() | |
| 33 | + | if err := temporary.Chmod(safetyMarkerFilePerm); err != nil { | |
| 34 | + | _ = temporary.Close() | |
| 35 | + | return err | |
| 36 | + | } | |
| 37 | + | if _, err := temporary.WriteString("safe\n"); err != nil { | |
| 38 | + | _ = temporary.Close() | |
| 39 | + | return err | |
| 40 | + | } | |
| 41 | + | if err := temporary.Close(); err != nil { | |
| 42 | + | return err | |
| 43 | + | } | |
| 44 | + | return os.Rename(temporaryName, marker) | |
| 45 | + | } | |
| 46 | + | ||
| 47 | + | // consumeNextStartSafetyMarker consumes a one-shot marker. It deliberately | |
| 48 | + | // does not follow symlinks, so an untrusted cache entry cannot redirect the | |
| 49 | + | // recovery check outside the CEF state root. | |
| 50 | + | func consumeNextStartSafetyMarker(stateRoot string) bool { | |
| 51 | + | if stateRoot == "" { | |
| 52 | + | return false | |
| 53 | + | } | |
| 54 | + | marker := filepath.Join(stateRoot, nextStartSafetyMarkerFile) | |
| 55 | + | info, err := os.Lstat(marker) | |
| 56 | + | if err != nil || !info.Mode().IsRegular() { | |
| 57 | + | return false | |
| 58 | + | } | |
| 59 | + | return os.Remove(marker) == nil | |
| 60 | + | } | |
| 61 | + | ||
| 62 | + | // applyNextStartSafetyEnvironment leaves the Vulkan render stack intact while | |
| 63 | + | // returning Chromium to its hardware-decode safety defaults for one start. | |
| 64 | + | func applyNextStartSafetyEnvironment() { | |
| 65 | + | _ = os.Setenv(cefEnableVAAPIEnvVar, "0") | |
| 66 | + | _ = os.Unsetenv(cefChromiumFlagsEnvVar) | |
| 67 | + | _ = os.Unsetenv(cefRenderNodeEnvVar) | |
| 68 | + | } | |
| @@ -0,0 +1,68 @@ | |||||
| 1 | + | package cef | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "errors" | |||
| 5 | + | "os" | |||
| 6 | + | "path/filepath" | |||
| 7 | + | ) | |||
| 8 | + | ||||
| 9 | + | const ( | |||
| 10 | + | nextStartSafetyMarkerFile = "cef-next-start-safe" | |||
| 11 | + | safetyMarkerDirPerm = 0o700 | |||
| 12 | + | safetyMarkerFilePerm = 0o600 | |||
| 13 | + | ) | |||
| 14 | + | ||||
| 15 | + | // writeNextStartSafetyMarker records a one-shot recovery request for the next | |||
| 16 | + | // CEF start. The marker lives beside CEF's root cache so it follows explicit | |||
| 17 | + | // per-session cache roots as well as the normal profile. | |||
| 18 | + | func writeNextStartSafetyMarker(stateRoot string) error { | |||
| 19 | + | if stateRoot == "" { | |||
| 20 | + | return errors.New("CEF state root is empty") | |||
| 21 | + | } | |||
| 22 | + | if err := os.MkdirAll(stateRoot, safetyMarkerDirPerm); err != nil { | |||
| 23 | + | return err | |||
| 24 | + | } | |||
| 25 | + | ||||
| 26 | + | marker := filepath.Join(stateRoot, nextStartSafetyMarkerFile) | |||
| 27 | + | temporary, err := os.CreateTemp(stateRoot, ".cef-next-start-safe-*") | |||
| 28 | + | if err != nil { | |||
| 29 | + | return err | |||
| 30 | + | } | |||
| 31 | + | temporaryName := temporary.Name() | |||
| 32 | + | defer func() { _ = os.Remove(temporaryName) }() | |||
| 33 | + | if err := temporary.Chmod(safetyMarkerFilePerm); err != nil { | |||
| 34 | + | _ = temporary.Close() | |||
| 35 | + | return err | |||
| 36 | + | } | |||
| 37 | + | if _, err := temporary.WriteString("safe\n"); err != nil { | |||
| 38 | + | _ = temporary.Close() | |||
| 39 | + | return err | |||
| 40 | + | } | |||
| 41 | + | if err := temporary.Close(); err != nil { | |||
| 42 | + | return err | |||
| 43 | + | } | |||
| 44 | + | return os.Rename(temporaryName, marker) | |||
| 45 | + | } | |||
| 46 | + | ||||
| 47 | + | // consumeNextStartSafetyMarker consumes a one-shot marker. It deliberately | |||
| 48 | + | // does not follow symlinks, so an untrusted cache entry cannot redirect the | |||
| 49 | + | // recovery check outside the CEF state root. | |||
| 50 | + | func consumeNextStartSafetyMarker(stateRoot string) bool { | |||
| 51 | + | if stateRoot == "" { | |||
| 52 | + | return false | |||
| 53 | + | } | |||
| 54 | + | marker := filepath.Join(stateRoot, nextStartSafetyMarkerFile) | |||
| 55 | + | info, err := os.Lstat(marker) | |||
| 56 | + | if err != nil || !info.Mode().IsRegular() { | |||
| 57 | + | return false | |||
| 58 | + | } | |||
| 59 | + | return os.Remove(marker) == nil | |||
| 60 | + | } | |||
| 61 | + | ||||
| 62 | + | // applyNextStartSafetyEnvironment leaves the Vulkan render stack intact while | |||
| 63 | + | // returning Chromium to its hardware-decode safety defaults for one start. | |||
| 64 | + | func applyNextStartSafetyEnvironment() { | |||
| 65 | + | _ = os.Setenv(cefEnableVAAPIEnvVar, "0") | |||
| 66 | + | _ = os.Unsetenv(cefChromiumFlagsEnvVar) | |||
| 67 | + | _ = os.Unsetenv(cefRenderNodeEnvVar) | |||
| 68 | + | } | |||
A internal/infrastructure/cef/restart_safety_test.go +38 -0
| @@ -0,0 +1,38 @@ | |||
| 1 | + | package cef | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "os" | |
| 5 | + | "path/filepath" | |
| 6 | + | "testing" | |
| 7 | + | ||
| 8 | + | "github.com/stretchr/testify/require" | |
| 9 | + | ) | |
| 10 | + | ||
| 11 | + | func TestConsumeNextStartSafetyMarkerDisablesVAAPIWithoutChangingVulkanStack(t *testing.T) { | |
| 12 | + | root := t.TempDir() | |
| 13 | + | t.Setenv(cefEnableVAAPIEnvVar, "1") | |
| 14 | + | t.Setenv(cefChromiumFlagsEnvVar, "--ignore-gpu-blocklist --disable-gpu-driver-bug-workaround") | |
| 15 | + | t.Setenv(dumberRenderStackEnvVar, renderStackVulkanDMABUF) | |
| 16 | + | ||
| 17 | + | require.NoError(t, writeNextStartSafetyMarker(root)) | |
| 18 | + | require.True(t, consumeNextStartSafetyMarker(root)) | |
| 19 | + | applyNextStartSafetyEnvironment() | |
| 20 | + | ||
| 21 | + | require.Equal(t, "0", os.Getenv(cefEnableVAAPIEnvVar)) | |
| 22 | + | require.Empty(t, os.Getenv(cefChromiumFlagsEnvVar)) | |
| 23 | + | require.Equal(t, renderStackVulkanDMABUF, os.Getenv(dumberRenderStackEnvVar)) | |
| 24 | + | _, err := os.Stat(filepath.Join(root, nextStartSafetyMarkerFile)) | |
| 25 | + | require.ErrorIs(t, err, os.ErrNotExist) | |
| 26 | + | } | |
| 27 | + | ||
| 28 | + | func TestGPUProcessRelaunchMarksNextStartSafeAfterRepeatedRelaunch(t *testing.T) { | |
| 29 | + | root := t.TempDir() | |
| 30 | + | engine := &Engine{stateRoot: root} | |
| 31 | + | handler := &dumberBPH{engine: engine} | |
| 32 | + | commandLine := relaunchCommandLineStub{commandLineString: "dumber --type=gpu-process"} | |
| 33 | + | ||
| 34 | + | require.Zero(t, handler.OnAlreadyRunningAppRelaunch(commandLine, "")) | |
| 35 | + | require.NoFileExists(t, filepath.Join(root, nextStartSafetyMarkerFile)) | |
| 36 | + | require.Zero(t, handler.OnAlreadyRunningAppRelaunch(commandLine, "")) | |
| 37 | + | require.FileExists(t, filepath.Join(root, nextStartSafetyMarkerFile)) | |
| 38 | + | } | |
| @@ -0,0 +1,38 @@ | |||||
| 1 | + | package cef | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "os" | |||
| 5 | + | "path/filepath" | |||
| 6 | + | "testing" | |||
| 7 | + | ||||
| 8 | + | "github.com/stretchr/testify/require" | |||
| 9 | + | ) | |||
| 10 | + | ||||
| 11 | + | func TestConsumeNextStartSafetyMarkerDisablesVAAPIWithoutChangingVulkanStack(t *testing.T) { | |||
| 12 | + | root := t.TempDir() | |||
| 13 | + | t.Setenv(cefEnableVAAPIEnvVar, "1") | |||
| 14 | + | t.Setenv(cefChromiumFlagsEnvVar, "--ignore-gpu-blocklist --disable-gpu-driver-bug-workaround") | |||
| 15 | + | t.Setenv(dumberRenderStackEnvVar, renderStackVulkanDMABUF) | |||
| 16 | + | ||||
| 17 | + | require.NoError(t, writeNextStartSafetyMarker(root)) | |||
| 18 | + | require.True(t, consumeNextStartSafetyMarker(root)) | |||
| 19 | + | applyNextStartSafetyEnvironment() | |||
| 20 | + | ||||
| 21 | + | require.Equal(t, "0", os.Getenv(cefEnableVAAPIEnvVar)) | |||
| 22 | + | require.Empty(t, os.Getenv(cefChromiumFlagsEnvVar)) | |||
| 23 | + | require.Equal(t, renderStackVulkanDMABUF, os.Getenv(dumberRenderStackEnvVar)) | |||
| 24 | + | _, err := os.Stat(filepath.Join(root, nextStartSafetyMarkerFile)) | |||
| 25 | + | require.ErrorIs(t, err, os.ErrNotExist) | |||
| 26 | + | } | |||
| 27 | + | ||||
| 28 | + | func TestGPUProcessRelaunchMarksNextStartSafeAfterRepeatedRelaunch(t *testing.T) { | |||
| 29 | + | root := t.TempDir() | |||
| 30 | + | engine := &Engine{stateRoot: root} | |||
| 31 | + | handler := &dumberBPH{engine: engine} | |||
| 32 | + | commandLine := relaunchCommandLineStub{commandLineString: "dumber --type=gpu-process"} | |||
| 33 | + | ||||
| 34 | + | require.Zero(t, handler.OnAlreadyRunningAppRelaunch(commandLine, "")) | |||
| 35 | + | require.NoFileExists(t, filepath.Join(root, nextStartSafetyMarkerFile)) | |||
| 36 | + | require.Zero(t, handler.OnAlreadyRunningAppRelaunch(commandLine, "")) | |||
| 37 | + | require.FileExists(t, filepath.Join(root, nextStartSafetyMarkerFile)) | |||
| 38 | + | } | |||
M internal/infrastructure/cef/webview.go +36 -7
| @@ -39,6 +39,10 @@ var ( | |||
| 39 | 39 | // errDestroyed is returned when an operation is attempted on a destroyed WebView. | |
| 40 | 40 | var errDestroyed = errors.New("cef: webview is destroyed") | |
| 41 | 41 | ||
| 42 | + | // unrefTickCallback is the canonical puregotk/purego callback-slot release. | |
| 43 | + | // It remains injectable so lifecycle tests can prove exactly-once ownership. | |
| 44 | + | var unrefTickCallback = glib.UnrefCallback | |
| 45 | + | ||
| 42 | 46 | // errNoBrowser is returned when the browser has not been created yet. | |
| 43 | 47 | var errNoBrowser = errors.New("cef: browser not yet created") | |
| 44 | 48 | ||
| @@ -107,11 +111,19 @@ type WebView struct { | |||
| 107 | 111 | viewportSyncReason string | |
| 108 | 112 | viewportMapFunc func(gtk.Widget) | |
| 109 | 113 | viewportShowFunc func(gtk.Widget) | |
| 114 | + | viewportHideFunc func(gtk.Widget) | |
| 115 | + | viewportUnmapFunc func(gtk.Widget) | |
| 110 | 116 | viewportRealizeFunc func(gtk.Widget) | |
| 111 | 117 | viewportMapSignalID uint | |
| 112 | 118 | viewportShowSignalID uint | |
| 119 | + | viewportHideSignalID uint | |
| 120 | + | viewportUnmapSignalID uint | |
| 113 | 121 | viewportRealizeSignalID uint | |
| 114 | 122 | viewportResizePulseSeq atomic.Uint64 | |
| 123 | + | // effectiveVisibility records the last CEF WasHidden state dispatched. It | |
| 124 | + | // is guarded by mu because GTK and CEF UI work cross threads. | |
| 125 | + | effectiveVisibilityKnown bool | |
| 126 | + | effectiveVisible bool | |
| 115 | 127 | ||
| 116 | 128 | adaptiveWindowlessFrameRate bool | |
| 117 | 129 | windowlessFrameRateMax int32 | |
| @@ -2067,12 +2079,14 @@ func (wv *WebView) startBeginFrameLoop() { | |||
| 2067 | 2079 | cb := new(gtk.TickCallback) | |
| 2068 | 2080 | *cb = func(_, _, _ uintptr) bool { | |
| 2069 | 2081 | if wv.destroyed.Load() { | |
| 2082 | + | wv.releaseBeginFrameTickCallback() | |
| 2070 | 2083 | return false | |
| 2071 | 2084 | } | |
| 2072 | 2085 | wv.mu.RLock() | |
| 2073 | 2086 | host := wv.host | |
| 2074 | 2087 | wv.mu.RUnlock() | |
| 2075 | 2088 | if host == nil { | |
| 2089 | + | wv.releaseBeginFrameTickCallback() | |
| 2076 | 2090 | return false | |
| 2077 | 2091 | } | |
| 2078 | 2092 | host.SendExternalBeginFrame() | |
| @@ -2095,19 +2109,34 @@ func (wv *WebView) startBeginFrameLoop() { | |||
| 2095 | 2109 | } | |
| 2096 | 2110 | ||
| 2097 | 2111 | func (wv *WebView) stopBeginFrameLoop() { | |
| 2098 | - | if wv.viewBridge == nil || wv.viewBridge.Widget() == nil { | |
| 2099 | - | return | |
| 2100 | - | } | |
| 2101 | - | ||
| 2102 | 2112 | wv.mu.Lock() | |
| 2103 | 2113 | tickID := wv.beginFrameTickID | |
| 2114 | + | callback := wv.beginFrameTick | |
| 2104 | 2115 | wv.beginFrameTickID = 0 | |
| 2105 | 2116 | wv.beginFrameTick = nil | |
| 2106 | - | widget := wv.viewBridge.Widget() | |
| 2117 | + | bridge := wv.viewBridge | |
| 2107 | 2118 | wv.mu.Unlock() | |
| 2108 | 2119 | ||
| 2109 | - | if tickID != 0 { | |
| 2110 | - | widget.RemoveTickCallback(tickID) | |
| 2120 | + | if tickID != 0 && bridge != nil { | |
| 2121 | + | if widget := bridge.Widget(); widget != nil { | |
| 2122 | + | widget.RemoveTickCallback(tickID) | |
| 2123 | + | } | |
| 2124 | + | } | |
| 2125 | + | if callback != nil { | |
| 2126 | + | _ = unrefTickCallback(callback) | |
| 2127 | + | } | |
| 2128 | + | } | |
| 2129 | + | ||
| 2130 | + | // releaseBeginFrameTickCallback releases the purego callback slot after GTK | |
| 2131 | + | // removes a source because its callback returned false. | |
| 2132 | + | func (wv *WebView) releaseBeginFrameTickCallback() { | |
| 2133 | + | wv.mu.Lock() | |
| 2134 | + | callback := wv.beginFrameTick | |
| 2135 | + | wv.beginFrameTickID = 0 | |
| 2136 | + | wv.beginFrameTick = nil | |
| 2137 | + | wv.mu.Unlock() | |
| 2138 | + | if callback != nil { | |
| 2139 | + | _ = unrefTickCallback(callback) | |
| 2111 | 2140 | } | |
| 2112 | 2141 | } | |
| 2113 | 2142 | ||
| @@ -39,6 +39,10 @@ var ( | |||||
| 39 | // errDestroyed is returned when an operation is attempted on a destroyed WebView. | 39 | // errDestroyed is returned when an operation is attempted on a destroyed WebView. | ||
| 40 | var errDestroyed = errors.New("cef: webview is destroyed") | 40 | var errDestroyed = errors.New("cef: webview is destroyed") | ||
| 41 | 41 | ||||
| 42 | + | // unrefTickCallback is the canonical puregotk/purego callback-slot release. | |||
| 43 | + | // It remains injectable so lifecycle tests can prove exactly-once ownership. | |||
| 44 | + | var unrefTickCallback = glib.UnrefCallback | |||
| 45 | + | ||||
| 42 | // errNoBrowser is returned when the browser has not been created yet. | 46 | // errNoBrowser is returned when the browser has not been created yet. | ||
| 43 | var errNoBrowser = errors.New("cef: browser not yet created") | 47 | var errNoBrowser = errors.New("cef: browser not yet created") | ||
| 44 | 48 | ||||
| @@ -107,11 +111,19 @@ type WebView struct { | |||||
| 107 | viewportSyncReason string | 111 | viewportSyncReason string | ||
| 108 | viewportMapFunc func(gtk.Widget) | 112 | viewportMapFunc func(gtk.Widget) | ||
| 109 | viewportShowFunc func(gtk.Widget) | 113 | viewportShowFunc func(gtk.Widget) | ||
| 114 | + | viewportHideFunc func(gtk.Widget) | |||
| 115 | + | viewportUnmapFunc func(gtk.Widget) | |||
| 110 | viewportRealizeFunc func(gtk.Widget) | 116 | viewportRealizeFunc func(gtk.Widget) | ||
| 111 | viewportMapSignalID uint | 117 | viewportMapSignalID uint | ||
| 112 | viewportShowSignalID uint | 118 | viewportShowSignalID uint | ||
| 119 | + | viewportHideSignalID uint | |||
| 120 | + | viewportUnmapSignalID uint | |||
| 113 | viewportRealizeSignalID uint | 121 | viewportRealizeSignalID uint | ||
| 114 | viewportResizePulseSeq atomic.Uint64 | 122 | viewportResizePulseSeq atomic.Uint64 | ||
| 123 | + | // effectiveVisibility records the last CEF WasHidden state dispatched. It | |||
| 124 | + | // is guarded by mu because GTK and CEF UI work cross threads. | |||
| 125 | + | effectiveVisibilityKnown bool | |||
| 126 | + | effectiveVisible bool | |||
| 115 | 127 | ||||
| 116 | adaptiveWindowlessFrameRate bool | 128 | adaptiveWindowlessFrameRate bool | ||
| 117 | windowlessFrameRateMax int32 | 129 | windowlessFrameRateMax int32 | ||
| @@ -2067,12 +2079,14 @@ func (wv *WebView) startBeginFrameLoop() { | |||||
| 2067 | cb := new(gtk.TickCallback) | 2079 | cb := new(gtk.TickCallback) | ||
| 2068 | *cb = func(_, _, _ uintptr) bool { | 2080 | *cb = func(_, _, _ uintptr) bool { | ||
| 2069 | if wv.destroyed.Load() { | 2081 | if wv.destroyed.Load() { | ||
| 2082 | + | wv.releaseBeginFrameTickCallback() | |||
| 2070 | return false | 2083 | return false | ||
| 2071 | } | 2084 | } | ||
| 2072 | wv.mu.RLock() | 2085 | wv.mu.RLock() | ||
| 2073 | host := wv.host | 2086 | host := wv.host | ||
| 2074 | wv.mu.RUnlock() | 2087 | wv.mu.RUnlock() | ||
| 2075 | if host == nil { | 2088 | if host == nil { | ||
| 2089 | + | wv.releaseBeginFrameTickCallback() | |||
| 2076 | return false | 2090 | return false | ||
| 2077 | } | 2091 | } | ||
| 2078 | host.SendExternalBeginFrame() | 2092 | host.SendExternalBeginFrame() | ||
| @@ -2095,19 +2109,34 @@ func (wv *WebView) startBeginFrameLoop() { | |||||
| 2095 | } | 2109 | } | ||
| 2096 | 2110 | ||||
| 2097 | func (wv *WebView) stopBeginFrameLoop() { | 2111 | func (wv *WebView) stopBeginFrameLoop() { | ||
| 2098 | - | if wv.viewBridge == nil || wv.viewBridge.Widget() == nil { | |||
| 2099 | - | return | |||
| 2100 | - | } | |||
| 2101 | - | ||||
| 2102 | wv.mu.Lock() | 2112 | wv.mu.Lock() | ||
| 2103 | tickID := wv.beginFrameTickID | 2113 | tickID := wv.beginFrameTickID | ||
| 2114 | + | callback := wv.beginFrameTick | |||
| 2104 | wv.beginFrameTickID = 0 | 2115 | wv.beginFrameTickID = 0 | ||
| 2105 | wv.beginFrameTick = nil | 2116 | wv.beginFrameTick = nil | ||
| 2106 | - | widget := wv.viewBridge.Widget() | 2117 | + | bridge := wv.viewBridge |
| 2107 | wv.mu.Unlock() | 2118 | wv.mu.Unlock() | ||
| 2108 | 2119 | ||||
| 2109 | - | if tickID != 0 { | 2120 | + | if tickID != 0 && bridge != nil { |
| 2110 | - | widget.RemoveTickCallback(tickID) | 2121 | + | if widget := bridge.Widget(); widget != nil { |
| 2122 | + | widget.RemoveTickCallback(tickID) | |||
| 2123 | + | } | |||
| 2124 | + | } | |||
| 2125 | + | if callback != nil { | |||
| 2126 | + | _ = unrefTickCallback(callback) | |||
| 2127 | + | } | |||
| 2128 | + | } | |||
| 2129 | + | ||||
| 2130 | + | // releaseBeginFrameTickCallback releases the purego callback slot after GTK | |||
| 2131 | + | // removes a source because its callback returned false. | |||
| 2132 | + | func (wv *WebView) releaseBeginFrameTickCallback() { | |||
| 2133 | + | wv.mu.Lock() | |||
| 2134 | + | callback := wv.beginFrameTick | |||
| 2135 | + | wv.beginFrameTickID = 0 | |||
| 2136 | + | wv.beginFrameTick = nil | |||
| 2137 | + | wv.mu.Unlock() | |||
| 2138 | + | if callback != nil { | |||
| 2139 | + | _ = unrefTickCallback(callback) | |||
| 2111 | } | 2140 | } | ||
| 2112 | } | 2141 | } | ||
| 2113 | 2142 | ||||
A internal/infrastructure/cef/webview_begin_frame_cleanup_test.go +32 -0
| @@ -0,0 +1,32 @@ | |||
| 1 | + | package cef | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "testing" | |
| 5 | + | ||
| 6 | + | "github.com/bnema/puregotk/v4/gtk" | |
| 7 | + | ) | |
| 8 | + | ||
| 9 | + | func TestStopBeginFrameLoopReleasesCallbackSlotWithoutWidget(t *testing.T) { | |
| 10 | + | callback := new(gtk.TickCallback) | |
| 11 | + | wv := &WebView{beginFrameTick: callback, beginFrameTickID: 17} | |
| 12 | + | oldUnref := unrefTickCallback | |
| 13 | + | defer func() { unrefTickCallback = oldUnref }() | |
| 14 | + | var unrefs int | |
| 15 | + | unrefTickCallback = func(got any) error { | |
| 16 | + | if got != callback { | |
| 17 | + | t.Fatalf("unref callback = %p, want %p", got, callback) | |
| 18 | + | } | |
| 19 | + | unrefs++ | |
| 20 | + | return nil | |
| 21 | + | } | |
| 22 | + | ||
| 23 | + | wv.stopBeginFrameLoop() | |
| 24 | + | wv.stopBeginFrameLoop() | |
| 25 | + | ||
| 26 | + | if wv.beginFrameTick != nil || wv.beginFrameTickID != 0 { | |
| 27 | + | t.Fatal("stop must release the BeginFrame callback slot even after the GTK widget is gone") | |
| 28 | + | } | |
| 29 | + | if unrefs != 1 { | |
| 30 | + | t.Fatalf("callback unrefs = %d, want 1", unrefs) | |
| 31 | + | } | |
| 32 | + | } | |
| @@ -0,0 +1,32 @@ | |||||
| 1 | + | package cef | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "testing" | |||
| 5 | + | ||||
| 6 | + | "github.com/bnema/puregotk/v4/gtk" | |||
| 7 | + | ) | |||
| 8 | + | ||||
| 9 | + | func TestStopBeginFrameLoopReleasesCallbackSlotWithoutWidget(t *testing.T) { | |||
| 10 | + | callback := new(gtk.TickCallback) | |||
| 11 | + | wv := &WebView{beginFrameTick: callback, beginFrameTickID: 17} | |||
| 12 | + | oldUnref := unrefTickCallback | |||
| 13 | + | defer func() { unrefTickCallback = oldUnref }() | |||
| 14 | + | var unrefs int | |||
| 15 | + | unrefTickCallback = func(got any) error { | |||
| 16 | + | if got != callback { | |||
| 17 | + | t.Fatalf("unref callback = %p, want %p", got, callback) | |||
| 18 | + | } | |||
| 19 | + | unrefs++ | |||
| 20 | + | return nil | |||
| 21 | + | } | |||
| 22 | + | ||||
| 23 | + | wv.stopBeginFrameLoop() | |||
| 24 | + | wv.stopBeginFrameLoop() | |||
| 25 | + | ||||
| 26 | + | if wv.beginFrameTick != nil || wv.beginFrameTickID != 0 { | |||
| 27 | + | t.Fatal("stop must release the BeginFrame callback slot even after the GTK widget is gone") | |||
| 28 | + | } | |||
| 29 | + | if unrefs != 1 { | |||
| 30 | + | t.Fatalf("callback unrefs = %d, want 1", unrefs) | |||
| 31 | + | } | |||
| 32 | + | } | |||
M internal/infrastructure/cef/webview_handlers.go +2 -3
| @@ -819,9 +819,8 @@ func (h *handlerSet) attachAfterCreatedBrowser( | |||
| 819 | 819 | }) | |
| 820 | 820 | } | |
| 821 | 821 | ||
| 822 | - | // Mark browser as visible — CEF OSR starts in hidden state and suppresses | |
| 823 | - | // painting/caret updates until explicitly told the browser is shown. | |
| 824 | - | host.WasHidden(0) | |
| 822 | + | // CEF OSR starts hidden. SyncViewport reads GTK's mapped, ancestor-aware | |
| 823 | + | // effective state on the main context and reports that truthful initial state. | |
| 825 | 824 | if h.wv != nil { | |
| 826 | 825 | if h.wv.ctx != nil { | |
| 827 | 826 | logging.FromContext(h.wv.ctx).Info(). | |
| @@ -819,9 +819,8 @@ func (h *handlerSet) attachAfterCreatedBrowser( | |||||
| 819 | }) | 819 | }) | ||
| 820 | } | 820 | } | ||
| 821 | 821 | ||||
| 822 | - | // Mark browser as visible — CEF OSR starts in hidden state and suppresses | 822 | + | // CEF OSR starts hidden. SyncViewport reads GTK's mapped, ancestor-aware |
| 823 | - | // painting/caret updates until explicitly told the browser is shown. | 823 | + | // effective state on the main context and reports that truthful initial state. |
| 824 | - | host.WasHidden(0) | |||
| 825 | if h.wv != nil { | 824 | if h.wv != nil { | ||
| 826 | if h.wv.ctx != nil { | 825 | if h.wv.ctx != nil { | ||
| 827 | logging.FromContext(h.wv.ctx).Info(). | 826 | logging.FromContext(h.wv.ctx).Info(). | ||
M internal/infrastructure/cef/webview_viewport.go +75 -2
| @@ -24,6 +24,38 @@ type pendingBrowserCreateObservedSizeBridge interface { | |||
| 24 | 24 | RefreshObservedSizeOnGTKThread() (int32, int32) | |
| 25 | 25 | } | |
| 26 | 26 | ||
| 27 | + | // viewportWidgetVisibility is the effective GTK state of a widget in its | |
| 28 | + | // hierarchy. Gtk.Widget.IsVisible already includes ancestor visibility; mapped | |
| 29 | + | // is checked separately because a visible widget can still be unmapped. | |
| 30 | + | type viewportWidgetVisibility struct { | |
| 31 | + | visible bool | |
| 32 | + | mapped bool | |
| 33 | + | } | |
| 34 | + | ||
| 35 | + | func effectiveViewportVisibility(states []viewportWidgetVisibility) bool { | |
| 36 | + | if len(states) == 0 { | |
| 37 | + | return false | |
| 38 | + | } | |
| 39 | + | for _, state := range states { | |
| 40 | + | if !state.visible || !state.mapped { | |
| 41 | + | return false | |
| 42 | + | } | |
| 43 | + | } | |
| 44 | + | return true | |
| 45 | + | } | |
| 46 | + | ||
| 47 | + | func effectiveViewportWidgetVisibility(widget *gtk.Widget) bool { | |
| 48 | + | if widget == nil { | |
| 49 | + | return false | |
| 50 | + | } | |
| 51 | + | // IsVisible is GTK's ancestor-aware visibility predicate. Pairing it with | |
| 52 | + | // GetMapped makes hidden/unmapped ancestors effective CEF-hidden states. | |
| 53 | + | return effectiveViewportVisibility([]viewportWidgetVisibility{{ | |
| 54 | + | visible: widget.IsVisible(), | |
| 55 | + | mapped: widget.GetMapped(), | |
| 56 | + | }}) | |
| 57 | + | } | |
| 58 | + | ||
| 27 | 59 | const ( | |
| 28 | 60 | // pendingBrowserCreateObservedSizeRetryDelay spaces retries while waiting | |
| 29 | 61 | // for the GTK bridge to observe the widget's allocated size. | |
| @@ -129,7 +161,7 @@ func (wv *WebView) syncViewportNowOnGTK(ctx context.Context, reason string) bool | |||
| 129 | 161 | } | |
| 130 | 162 | ||
| 131 | 163 | widget := wv.viewBridge.Widget() | |
| 132 | - | visible := widget != nil && widget.IsVisible() | |
| 164 | + | visible := effectiveViewportWidgetVisibility(widget) | |
| 133 | 165 | wv.notifyViewportSyncOnCEFUIThread(host, visible) | |
| 134 | 166 | wv.syncZoomForBackingScaleOnCEFUIThread(host, reason) | |
| 135 | 167 | logging.FromContext(ctx).Debug(). | |
| @@ -437,12 +469,20 @@ func (wv *WebView) installViewportSyncHooksOnGTKThread() { | |||
| 437 | 469 | wv.viewportShowFunc = func(gtk.Widget) { | |
| 438 | 470 | wv.SyncViewport(wv.ctx, "gtk-show") | |
| 439 | 471 | } | |
| 472 | + | wv.viewportHideFunc = func(gtk.Widget) { | |
| 473 | + | wv.SyncViewport(wv.ctx, "gtk-hide") | |
| 474 | + | } | |
| 475 | + | wv.viewportUnmapFunc = func(gtk.Widget) { | |
| 476 | + | wv.SyncViewport(wv.ctx, "gtk-unmap") | |
| 477 | + | } | |
| 440 | 478 | wv.viewportRealizeFunc = func(gtk.Widget) { | |
| 441 | 479 | wv.SyncViewport(wv.ctx, "gtk-realize") | |
| 442 | 480 | } | |
| 443 | 481 | ||
| 444 | 482 | wv.viewportMapSignalID = widget.ConnectMap(&wv.viewportMapFunc) | |
| 445 | 483 | wv.viewportShowSignalID = widget.ConnectShow(&wv.viewportShowFunc) | |
| 484 | + | wv.viewportHideSignalID = widget.ConnectHide(&wv.viewportHideFunc) | |
| 485 | + | wv.viewportUnmapSignalID = widget.ConnectUnmap(&wv.viewportUnmapFunc) | |
| 446 | 486 | wv.viewportRealizeSignalID = widget.ConnectRealize(&wv.viewportRealizeFunc) | |
| 447 | 487 | } | |
| 448 | 488 | ||
| @@ -463,6 +503,14 @@ func (wv *WebView) disconnectViewportSyncHooksOnGTKThread() { | |||
| 463 | 503 | gobject.SignalHandlerDisconnect(obj, wv.viewportShowSignalID) | |
| 464 | 504 | wv.viewportShowSignalID = 0 | |
| 465 | 505 | } | |
| 506 | + | if wv.viewportHideSignalID != 0 { | |
| 507 | + | gobject.SignalHandlerDisconnect(obj, wv.viewportHideSignalID) | |
| 508 | + | wv.viewportHideSignalID = 0 | |
| 509 | + | } | |
| 510 | + | if wv.viewportUnmapSignalID != 0 { | |
| 511 | + | gobject.SignalHandlerDisconnect(obj, wv.viewportUnmapSignalID) | |
| 512 | + | wv.viewportUnmapSignalID = 0 | |
| 513 | + | } | |
| 466 | 514 | if wv.viewportRealizeSignalID != 0 { | |
| 467 | 515 | gobject.SignalHandlerDisconnect(obj, wv.viewportRealizeSignalID) | |
| 468 | 516 | wv.viewportRealizeSignalID = 0 | |
| @@ -471,6 +519,8 @@ func (wv *WebView) disconnectViewportSyncHooksOnGTKThread() { | |||
| 471 | 519 | } | |
| 472 | 520 | wv.viewportMapFunc = nil | |
| 473 | 521 | wv.viewportShowFunc = nil | |
| 522 | + | wv.viewportHideFunc = nil | |
| 523 | + | wv.viewportUnmapFunc = nil | |
| 474 | 524 | wv.viewportRealizeFunc = nil | |
| 475 | 525 | } | |
| 476 | 526 | ||
| @@ -488,7 +538,8 @@ func (wv *WebView) notifyViewportSyncOnCEFUIThread(host viewportSyncBrowserHost, | |||
| 488 | 538 | if currentHost != host { | |
| 489 | 539 | return | |
| 490 | 540 | } | |
| 491 | - | notifyBrowserViewportSync(host, visible) | |
| 541 | + | wv.applyEffectiveVisibility(host, visible) | |
| 542 | + | notifyBrowserViewportResize(host) | |
| 492 | 543 | })) | |
| 493 | 544 | if task == nil { | |
| 494 | 545 | return | |
| @@ -526,6 +577,28 @@ func (wv *WebView) syncZoomForBackingScaleOnCEFUIThread(host purecef.BrowserHost | |||
| 526 | 577 | cefPostDelayedTask(purecef.ThreadIDTidUi, task, 0) | |
| 527 | 578 | } | |
| 528 | 579 | ||
| 580 | + | // applyEffectiveVisibility reports the initial effective visibility and later | |
| 581 | + | // transitions exactly once. CEF OSR begins hidden, so even an initially hidden | |
| 582 | + | // GTK widget must be explicitly reported to keep both lifecycles aligned. | |
| 583 | + | func (wv *WebView) applyEffectiveVisibility(host viewportSyncBrowserHost, visible bool) { | |
| 584 | + | if wv == nil || host == nil || wv.destroyed.Load() { | |
| 585 | + | return | |
| 586 | + | } | |
| 587 | + | wv.mu.Lock() | |
| 588 | + | if wv.effectiveVisibilityKnown && wv.effectiveVisible == visible { | |
| 589 | + | wv.mu.Unlock() | |
| 590 | + | return | |
| 591 | + | } | |
| 592 | + | wv.effectiveVisibilityKnown = true | |
| 593 | + | wv.effectiveVisible = visible | |
| 594 | + | wv.mu.Unlock() | |
| 595 | + | if visible { | |
| 596 | + | host.WasHidden(0) | |
| 597 | + | return | |
| 598 | + | } | |
| 599 | + | host.WasHidden(1) | |
| 600 | + | } | |
| 601 | + | ||
| 529 | 602 | func notifyBrowserViewportSync(host viewportSyncBrowserHost, visible bool) { | |
| 530 | 603 | if host == nil { | |
| 531 | 604 | return | |
| @@ -24,6 +24,38 @@ type pendingBrowserCreateObservedSizeBridge interface { | |||||
| 24 | RefreshObservedSizeOnGTKThread() (int32, int32) | 24 | RefreshObservedSizeOnGTKThread() (int32, int32) | ||
| 25 | } | 25 | } | ||
| 26 | 26 | ||||
| 27 | + | // viewportWidgetVisibility is the effective GTK state of a widget in its | |||
| 28 | + | // hierarchy. Gtk.Widget.IsVisible already includes ancestor visibility; mapped | |||
| 29 | + | // is checked separately because a visible widget can still be unmapped. | |||
| 30 | + | type viewportWidgetVisibility struct { | |||
| 31 | + | visible bool | |||
| 32 | + | mapped bool | |||
| 33 | + | } | |||
| 34 | + | ||||
| 35 | + | func effectiveViewportVisibility(states []viewportWidgetVisibility) bool { | |||
| 36 | + | if len(states) == 0 { | |||
| 37 | + | return false | |||
| 38 | + | } | |||
| 39 | + | for _, state := range states { | |||
| 40 | + | if !state.visible || !state.mapped { | |||
| 41 | + | return false | |||
| 42 | + | } | |||
| 43 | + | } | |||
| 44 | + | return true | |||
| 45 | + | } | |||
| 46 | + | ||||
| 47 | + | func effectiveViewportWidgetVisibility(widget *gtk.Widget) bool { | |||
| 48 | + | if widget == nil { | |||
| 49 | + | return false | |||
| 50 | + | } | |||
| 51 | + | // IsVisible is GTK's ancestor-aware visibility predicate. Pairing it with | |||
| 52 | + | // GetMapped makes hidden/unmapped ancestors effective CEF-hidden states. | |||
| 53 | + | return effectiveViewportVisibility([]viewportWidgetVisibility{{ | |||
| 54 | + | visible: widget.IsVisible(), | |||
| 55 | + | mapped: widget.GetMapped(), | |||
| 56 | + | }}) | |||
| 57 | + | } | |||
| 58 | + | ||||
| 27 | const ( | 59 | const ( | ||
| 28 | // pendingBrowserCreateObservedSizeRetryDelay spaces retries while waiting | 60 | // pendingBrowserCreateObservedSizeRetryDelay spaces retries while waiting | ||
| 29 | // for the GTK bridge to observe the widget's allocated size. | 61 | // for the GTK bridge to observe the widget's allocated size. | ||
| @@ -129,7 +161,7 @@ func (wv *WebView) syncViewportNowOnGTK(ctx context.Context, reason string) bool | |||||
| 129 | } | 161 | } | ||
| 130 | 162 | ||||
| 131 | widget := wv.viewBridge.Widget() | 163 | widget := wv.viewBridge.Widget() | ||
| 132 | - | visible := widget != nil && widget.IsVisible() | 164 | + | visible := effectiveViewportWidgetVisibility(widget) |
| 133 | wv.notifyViewportSyncOnCEFUIThread(host, visible) | 165 | wv.notifyViewportSyncOnCEFUIThread(host, visible) | ||
| 134 | wv.syncZoomForBackingScaleOnCEFUIThread(host, reason) | 166 | wv.syncZoomForBackingScaleOnCEFUIThread(host, reason) | ||
| 135 | logging.FromContext(ctx).Debug(). | 167 | logging.FromContext(ctx).Debug(). | ||
| @@ -437,12 +469,20 @@ func (wv *WebView) installViewportSyncHooksOnGTKThread() { | |||||
| 437 | wv.viewportShowFunc = func(gtk.Widget) { | 469 | wv.viewportShowFunc = func(gtk.Widget) { | ||
| 438 | wv.SyncViewport(wv.ctx, "gtk-show") | 470 | wv.SyncViewport(wv.ctx, "gtk-show") | ||
| 439 | } | 471 | } | ||
| 472 | + | wv.viewportHideFunc = func(gtk.Widget) { | |||
| 473 | + | wv.SyncViewport(wv.ctx, "gtk-hide") | |||
| 474 | + | } | |||
| 475 | + | wv.viewportUnmapFunc = func(gtk.Widget) { | |||
| 476 | + | wv.SyncViewport(wv.ctx, "gtk-unmap") | |||
| 477 | + | } | |||
| 440 | wv.viewportRealizeFunc = func(gtk.Widget) { | 478 | wv.viewportRealizeFunc = func(gtk.Widget) { | ||
| 441 | wv.SyncViewport(wv.ctx, "gtk-realize") | 479 | wv.SyncViewport(wv.ctx, "gtk-realize") | ||
| 442 | } | 480 | } | ||
| 443 | 481 | ||||
| 444 | wv.viewportMapSignalID = widget.ConnectMap(&wv.viewportMapFunc) | 482 | wv.viewportMapSignalID = widget.ConnectMap(&wv.viewportMapFunc) | ||
| 445 | wv.viewportShowSignalID = widget.ConnectShow(&wv.viewportShowFunc) | 483 | wv.viewportShowSignalID = widget.ConnectShow(&wv.viewportShowFunc) | ||
| 484 | + | wv.viewportHideSignalID = widget.ConnectHide(&wv.viewportHideFunc) | |||
| 485 | + | wv.viewportUnmapSignalID = widget.ConnectUnmap(&wv.viewportUnmapFunc) | |||
| 446 | wv.viewportRealizeSignalID = widget.ConnectRealize(&wv.viewportRealizeFunc) | 486 | wv.viewportRealizeSignalID = widget.ConnectRealize(&wv.viewportRealizeFunc) | ||
| 447 | } | 487 | } | ||
| 448 | 488 | ||||
| @@ -463,6 +503,14 @@ func (wv *WebView) disconnectViewportSyncHooksOnGTKThread() { | |||||
| 463 | gobject.SignalHandlerDisconnect(obj, wv.viewportShowSignalID) | 503 | gobject.SignalHandlerDisconnect(obj, wv.viewportShowSignalID) | ||
| 464 | wv.viewportShowSignalID = 0 | 504 | wv.viewportShowSignalID = 0 | ||
| 465 | } | 505 | } | ||
| 506 | + | if wv.viewportHideSignalID != 0 { | |||
| 507 | + | gobject.SignalHandlerDisconnect(obj, wv.viewportHideSignalID) | |||
| 508 | + | wv.viewportHideSignalID = 0 | |||
| 509 | + | } | |||
| 510 | + | if wv.viewportUnmapSignalID != 0 { | |||
| 511 | + | gobject.SignalHandlerDisconnect(obj, wv.viewportUnmapSignalID) | |||
| 512 | + | wv.viewportUnmapSignalID = 0 | |||
| 513 | + | } | |||
| 466 | if wv.viewportRealizeSignalID != 0 { | 514 | if wv.viewportRealizeSignalID != 0 { | ||
| 467 | gobject.SignalHandlerDisconnect(obj, wv.viewportRealizeSignalID) | 515 | gobject.SignalHandlerDisconnect(obj, wv.viewportRealizeSignalID) | ||
| 468 | wv.viewportRealizeSignalID = 0 | 516 | wv.viewportRealizeSignalID = 0 | ||
| @@ -471,6 +519,8 @@ func (wv *WebView) disconnectViewportSyncHooksOnGTKThread() { | |||||
| 471 | } | 519 | } | ||
| 472 | wv.viewportMapFunc = nil | 520 | wv.viewportMapFunc = nil | ||
| 473 | wv.viewportShowFunc = nil | 521 | wv.viewportShowFunc = nil | ||
| 522 | + | wv.viewportHideFunc = nil | |||
| 523 | + | wv.viewportUnmapFunc = nil | |||
| 474 | wv.viewportRealizeFunc = nil | 524 | wv.viewportRealizeFunc = nil | ||
| 475 | } | 525 | } | ||
| 476 | 526 | ||||
| @@ -488,7 +538,8 @@ func (wv *WebView) notifyViewportSyncOnCEFUIThread(host viewportSyncBrowserHost, | |||||
| 488 | if currentHost != host { | 538 | if currentHost != host { | ||
| 489 | return | 539 | return | ||
| 490 | } | 540 | } | ||
| 491 | - | notifyBrowserViewportSync(host, visible) | 541 | + | wv.applyEffectiveVisibility(host, visible) |
| 542 | + | notifyBrowserViewportResize(host) | |||
| 492 | })) | 543 | })) | ||
| 493 | if task == nil { | 544 | if task == nil { | ||
| 494 | return | 545 | return | ||
| @@ -526,6 +577,28 @@ func (wv *WebView) syncZoomForBackingScaleOnCEFUIThread(host purecef.BrowserHost | |||||
| 526 | cefPostDelayedTask(purecef.ThreadIDTidUi, task, 0) | 577 | cefPostDelayedTask(purecef.ThreadIDTidUi, task, 0) | ||
| 527 | } | 578 | } | ||
| 528 | 579 | ||||
| 580 | + | // applyEffectiveVisibility reports the initial effective visibility and later | |||
| 581 | + | // transitions exactly once. CEF OSR begins hidden, so even an initially hidden | |||
| 582 | + | // GTK widget must be explicitly reported to keep both lifecycles aligned. | |||
| 583 | + | func (wv *WebView) applyEffectiveVisibility(host viewportSyncBrowserHost, visible bool) { | |||
| 584 | + | if wv == nil || host == nil || wv.destroyed.Load() { | |||
| 585 | + | return | |||
| 586 | + | } | |||
| 587 | + | wv.mu.Lock() | |||
| 588 | + | if wv.effectiveVisibilityKnown && wv.effectiveVisible == visible { | |||
| 589 | + | wv.mu.Unlock() | |||
| 590 | + | return | |||
| 591 | + | } | |||
| 592 | + | wv.effectiveVisibilityKnown = true | |||
| 593 | + | wv.effectiveVisible = visible | |||
| 594 | + | wv.mu.Unlock() | |||
| 595 | + | if visible { | |||
| 596 | + | host.WasHidden(0) | |||
| 597 | + | return | |||
| 598 | + | } | |||
| 599 | + | host.WasHidden(1) | |||
| 600 | + | } | |||
| 601 | + | ||||
| 529 | func notifyBrowserViewportSync(host viewportSyncBrowserHost, visible bool) { | 602 | func notifyBrowserViewportSync(host viewportSyncBrowserHost, visible bool) { | ||
| 530 | if host == nil { | 603 | if host == nil { | ||
| 531 | return | 604 | return | ||
M internal/infrastructure/cef/webview_viewport_test.go +0 -1
| @@ -149,7 +149,6 @@ func TestNotifyViewportSyncOnCEFUIThread_RemainsViewportOnlyAcrossRepeatedResize | |||
| 149 | 149 | scheduled.Execute() | |
| 150 | 150 | ||
| 151 | 151 | require.Equal(t, []string{ | |
| 152 | - | "WasHidden", | |
| 153 | 152 | "NotifyScreenInfoChanged", | |
| 154 | 153 | "WasResized", | |
| 155 | 154 | "Invalidate", | |
| @@ -149,7 +149,6 @@ func TestNotifyViewportSyncOnCEFUIThread_RemainsViewportOnlyAcrossRepeatedResize | |||||
| 149 | scheduled.Execute() | 149 | scheduled.Execute() | ||
| 150 | 150 | ||||
| 151 | require.Equal(t, []string{ | 151 | require.Equal(t, []string{ | ||
| 152 | - | "WasHidden", | |||
| 153 | "NotifyScreenInfoChanged", | 152 | "NotifyScreenInfoChanged", | ||
| 154 | "WasResized", | 153 | "WasResized", | ||
| 155 | "Invalidate", | 154 | "Invalidate", | ||
A internal/infrastructure/cef/webview_visibility_test.go +45 -0
| @@ -0,0 +1,45 @@ | |||
| 1 | + | package cef | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "testing" | |
| 5 | + | ||
| 6 | + | "github.com/stretchr/testify/require" | |
| 7 | + | ) | |
| 8 | + | ||
| 9 | + | func TestEffectiveViewportVisibilityRequiresMappedVisibleAncestors(t *testing.T) { | |
| 10 | + | tests := []struct { | |
| 11 | + | name string | |
| 12 | + | states []viewportWidgetVisibility | |
| 13 | + | visible bool | |
| 14 | + | }{ | |
| 15 | + | {name: "mapped visible widget", states: []viewportWidgetVisibility{{visible: true, mapped: true}}, visible: true}, | |
| 16 | + | {name: "unmapped widget", states: []viewportWidgetVisibility{{visible: true, mapped: false}}}, | |
| 17 | + | {name: "hidden widget", states: []viewportWidgetVisibility{{visible: false, mapped: true}}}, | |
| 18 | + | {name: "hidden ancestor", states: []viewportWidgetVisibility{{visible: true, mapped: true}, {visible: false, mapped: true}}}, | |
| 19 | + | {name: "unmapped ancestor", states: []viewportWidgetVisibility{{visible: true, mapped: true}, {visible: true, mapped: false}}}, | |
| 20 | + | } | |
| 21 | + | for _, tt := range tests { | |
| 22 | + | t.Run(tt.name, func(t *testing.T) { | |
| 23 | + | require.Equal(t, tt.visible, effectiveViewportVisibility(tt.states)) | |
| 24 | + | }) | |
| 25 | + | } | |
| 26 | + | } | |
| 27 | + | ||
| 28 | + | func TestWebViewEffectiveVisibilitySendsInitialStateAndDeduplicatesTransitions(t *testing.T) { | |
| 29 | + | host := &viewportSyncOrderHost{} | |
| 30 | + | wv := &WebView{} | |
| 31 | + | ||
| 32 | + | wv.applyEffectiveVisibility(host, false) | |
| 33 | + | require.Equal(t, []string{"WasHidden", "WasHidden(nonzero)"}, host.calls) | |
| 34 | + | ||
| 35 | + | host.calls = nil | |
| 36 | + | wv.applyEffectiveVisibility(host, false) | |
| 37 | + | require.Empty(t, host.calls, "unchanged hidden state must not be resent") | |
| 38 | + | ||
| 39 | + | wv.applyEffectiveVisibility(host, true) | |
| 40 | + | require.Equal(t, []string{"WasHidden"}, host.calls) | |
| 41 | + | ||
| 42 | + | host.calls = nil | |
| 43 | + | wv.applyEffectiveVisibility(host, true) | |
| 44 | + | require.Empty(t, host.calls, "unchanged visible state must not be resent") | |
| 45 | + | } | |
| @@ -0,0 +1,45 @@ | |||||
| 1 | + | package cef | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "testing" | |||
| 5 | + | ||||
| 6 | + | "github.com/stretchr/testify/require" | |||
| 7 | + | ) | |||
| 8 | + | ||||
| 9 | + | func TestEffectiveViewportVisibilityRequiresMappedVisibleAncestors(t *testing.T) { | |||
| 10 | + | tests := []struct { | |||
| 11 | + | name string | |||
| 12 | + | states []viewportWidgetVisibility | |||
| 13 | + | visible bool | |||
| 14 | + | }{ | |||
| 15 | + | {name: "mapped visible widget", states: []viewportWidgetVisibility{{visible: true, mapped: true}}, visible: true}, | |||
| 16 | + | {name: "unmapped widget", states: []viewportWidgetVisibility{{visible: true, mapped: false}}}, | |||
| 17 | + | {name: "hidden widget", states: []viewportWidgetVisibility{{visible: false, mapped: true}}}, | |||
| 18 | + | {name: "hidden ancestor", states: []viewportWidgetVisibility{{visible: true, mapped: true}, {visible: false, mapped: true}}}, | |||
| 19 | + | {name: "unmapped ancestor", states: []viewportWidgetVisibility{{visible: true, mapped: true}, {visible: true, mapped: false}}}, | |||
| 20 | + | } | |||
| 21 | + | for _, tt := range tests { | |||
| 22 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 23 | + | require.Equal(t, tt.visible, effectiveViewportVisibility(tt.states)) | |||
| 24 | + | }) | |||
| 25 | + | } | |||
| 26 | + | } | |||
| 27 | + | ||||
| 28 | + | func TestWebViewEffectiveVisibilitySendsInitialStateAndDeduplicatesTransitions(t *testing.T) { | |||
| 29 | + | host := &viewportSyncOrderHost{} | |||
| 30 | + | wv := &WebView{} | |||
| 31 | + | ||||
| 32 | + | wv.applyEffectiveVisibility(host, false) | |||
| 33 | + | require.Equal(t, []string{"WasHidden", "WasHidden(nonzero)"}, host.calls) | |||
| 34 | + | ||||
| 35 | + | host.calls = nil | |||
| 36 | + | wv.applyEffectiveVisibility(host, false) | |||
| 37 | + | require.Empty(t, host.calls, "unchanged hidden state must not be resent") | |||
| 38 | + | ||||
| 39 | + | wv.applyEffectiveVisibility(host, true) | |||
| 40 | + | require.Equal(t, []string{"WasHidden"}, host.calls) | |||
| 41 | + | ||||
| 42 | + | host.calls = nil | |||
| 43 | + | wv.applyEffectiveVisibility(host, true) | |||
| 44 | + | require.Empty(t, host.calls, "unchanged visible state must not be resent") | |||
| 45 | + | } | |||
M internal/ui/app.go +36 -13
| @@ -57,7 +57,10 @@ func gtkApplicationFlags() gio.ApplicationFlags { | |||
| 57 | 57 | return gio.GApplicationNonUniqueValue | |
| 58 | 58 | } | |
| 59 | 59 | ||
| 60 | - | var adwaitaInitOnce sync.Once | |
| 60 | + | var ( | |
| 61 | + | adwaitaInitOnce sync.Once | |
| 62 | + | unrefTickCallback = glib.UnrefCallback | |
| 63 | + | ) | |
| 61 | 64 | ||
| 62 | 65 | // EnsureAdwaitaInitialized initializes libadwaita and GTK exactly once. | |
| 63 | 66 | func EnsureAdwaitaInitialized() { | |
| @@ -167,9 +170,12 @@ type floatingWorkspaceSession struct { | |||
| 167 | 170 | omnibox *component.Omnibox | |
| 168 | 171 | omniboxWidget layout.Widget | |
| 169 | 172 | resizeWatcherActive bool | |
| 170 | - | resizeTickID uint | |
| 171 | - | appliedWidth int | |
| 172 | - | appliedHeight int | |
| 173 | + | // resizeTickCallback keeps the purego callback live until its GTK source is | |
| 174 | + | // removed. It is cleared exactly once with resizeTickID. | |
| 175 | + | resizeTickCallback *gtk.TickCallback | |
| 176 | + | resizeTickID uint | |
| 177 | + | appliedWidth int | |
| 178 | + | appliedHeight int | |
| 173 | 179 | } | |
| 174 | 180 | ||
| 175 | 181 | type floatingSessionKey struct { | |
| @@ -4594,22 +4600,22 @@ func (a *App) startFloatingResizeWatcher(session *floatingWorkspaceSession) { | |||
| 4594 | 4600 | ||
| 4595 | 4601 | session.resizeWatcherActive = true | |
| 4596 | 4602 | paneID := session.paneID | |
| 4597 | - | tickCallback := gtk.TickCallback(func(_ uintptr, _ uintptr, _ uintptr) bool { | |
| 4603 | + | callback := new(gtk.TickCallback) | |
| 4604 | + | *callback = func(_ uintptr, _ uintptr, _ uintptr) bool { | |
| 4598 | 4605 | liveSession := a.floatingSessionByPaneID(paneID) | |
| 4599 | 4606 | if liveSession == nil { | |
| 4600 | - | session.resizeWatcherActive = false | |
| 4601 | - | session.resizeTickID = 0 | |
| 4607 | + | session.releaseResizeTickCallback() | |
| 4602 | 4608 | return false | |
| 4603 | 4609 | } | |
| 4604 | 4610 | ||
| 4605 | 4611 | keepRunning := a.handleFloatingViewportTick(liveSession) | |
| 4606 | 4612 | if !keepRunning { | |
| 4607 | - | liveSession.resizeWatcherActive = false | |
| 4608 | - | liveSession.resizeTickID = 0 | |
| 4613 | + | liveSession.releaseResizeTickCallback() | |
| 4609 | 4614 | } | |
| 4610 | 4615 | return keepRunning | |
| 4611 | - | }) | |
| 4612 | - | session.resizeTickID = overlayWidget.AddTickCallback(&tickCallback, 0, nil) | |
| 4616 | + | } | |
| 4617 | + | session.resizeTickCallback = callback | |
| 4618 | + | session.resizeTickID = overlayWidget.AddTickCallback(callback, 0, nil) | |
| 4613 | 4619 | } | |
| 4614 | 4620 | ||
| 4615 | 4621 | func (a *App) stopFloatingResizeWatcher(session *floatingWorkspaceSession) { | |
| @@ -4617,13 +4623,30 @@ func (a *App) stopFloatingResizeWatcher(session *floatingWorkspaceSession) { | |||
| 4617 | 4623 | return | |
| 4618 | 4624 | } | |
| 4619 | 4625 | ||
| 4620 | - | if session.resizeTickID != 0 && session.overlay != nil { | |
| 4626 | + | tickID := session.resizeTickID | |
| 4627 | + | session.releaseResizeTickCallback() | |
| 4628 | + | if tickID != 0 && session.overlay != nil { | |
| 4621 | 4629 | if overlayWidget := session.overlay.GtkWidget(); overlayWidget != nil { | |
| 4622 | - | overlayWidget.RemoveTickCallback(session.resizeTickID) | |
| 4630 | + | overlayWidget.RemoveTickCallback(tickID) | |
| 4623 | 4631 | } | |
| 4624 | 4632 | } | |
| 4633 | + | } | |
| 4634 | + | ||
| 4635 | + | // releaseResizeTickCallback releases the raw purego callback slot. It is safe | |
| 4636 | + | // to call after GTK has already removed a callback that returned false. | |
| 4637 | + | func (session *floatingWorkspaceSession) releaseResizeTickCallback() { | |
| 4638 | + | if session == nil { | |
| 4639 | + | return | |
| 4640 | + | } | |
| 4641 | + | callback := session.resizeTickCallback | |
| 4625 | 4642 | session.resizeTickID = 0 | |
| 4643 | + | session.resizeTickCallback = nil | |
| 4626 | 4644 | session.resizeWatcherActive = false | |
| 4645 | + | if callback != nil { | |
| 4646 | + | // AddTickCallback creates a purego function-pointer slot. Clearing the Go | |
| 4647 | + | // pointer alone cannot return that finite runtime slot to the ledger. | |
| 4648 | + | _ = unrefTickCallback(callback) | |
| 4649 | + | } | |
| 4627 | 4650 | } | |
| 4628 | 4651 | ||
| 4629 | 4652 | func (a *App) hideFloatingSession(ctx context.Context, session *floatingWorkspaceSession) { | |
| @@ -57,7 +57,10 @@ func gtkApplicationFlags() gio.ApplicationFlags { | |||||
| 57 | return gio.GApplicationNonUniqueValue | 57 | return gio.GApplicationNonUniqueValue | ||
| 58 | } | 58 | } | ||
| 59 | 59 | ||||
| 60 | - | var adwaitaInitOnce sync.Once | 60 | + | var ( |
| 61 | + | adwaitaInitOnce sync.Once | |||
| 62 | + | unrefTickCallback = glib.UnrefCallback | |||
| 63 | + | ) | |||
| 61 | 64 | ||||
| 62 | // EnsureAdwaitaInitialized initializes libadwaita and GTK exactly once. | 65 | // EnsureAdwaitaInitialized initializes libadwaita and GTK exactly once. | ||
| 63 | func EnsureAdwaitaInitialized() { | 66 | func EnsureAdwaitaInitialized() { | ||
| @@ -167,9 +170,12 @@ type floatingWorkspaceSession struct { | |||||
| 167 | omnibox *component.Omnibox | 170 | omnibox *component.Omnibox | ||
| 168 | omniboxWidget layout.Widget | 171 | omniboxWidget layout.Widget | ||
| 169 | resizeWatcherActive bool | 172 | resizeWatcherActive bool | ||
| 170 | - | resizeTickID uint | 173 | + | // resizeTickCallback keeps the purego callback live until its GTK source is |
| 171 | - | appliedWidth int | 174 | + | // removed. It is cleared exactly once with resizeTickID. |
| 172 | - | appliedHeight int | 175 | + | resizeTickCallback *gtk.TickCallback |
| 176 | + | resizeTickID uint | |||
| 177 | + | appliedWidth int | |||
| 178 | + | appliedHeight int | |||
| 173 | } | 179 | } | ||
| 174 | 180 | ||||
| 175 | type floatingSessionKey struct { | 181 | type floatingSessionKey struct { | ||
| @@ -4594,22 +4600,22 @@ func (a *App) startFloatingResizeWatcher(session *floatingWorkspaceSession) { | |||||
| 4594 | 4600 | ||||
| 4595 | session.resizeWatcherActive = true | 4601 | session.resizeWatcherActive = true | ||
| 4596 | paneID := session.paneID | 4602 | paneID := session.paneID | ||
| 4597 | - | tickCallback := gtk.TickCallback(func(_ uintptr, _ uintptr, _ uintptr) bool { | 4603 | + | callback := new(gtk.TickCallback) |
| 4604 | + | *callback = func(_ uintptr, _ uintptr, _ uintptr) bool { | |||
| 4598 | liveSession := a.floatingSessionByPaneID(paneID) | 4605 | liveSession := a.floatingSessionByPaneID(paneID) | ||
| 4599 | if liveSession == nil { | 4606 | if liveSession == nil { | ||
| 4600 | - | session.resizeWatcherActive = false | 4607 | + | session.releaseResizeTickCallback() |
| 4601 | - | session.resizeTickID = 0 | |||
| 4602 | return false | 4608 | return false | ||
| 4603 | } | 4609 | } | ||
| 4604 | 4610 | ||||
| 4605 | keepRunning := a.handleFloatingViewportTick(liveSession) | 4611 | keepRunning := a.handleFloatingViewportTick(liveSession) | ||
| 4606 | if !keepRunning { | 4612 | if !keepRunning { | ||
| 4607 | - | liveSession.resizeWatcherActive = false | 4613 | + | liveSession.releaseResizeTickCallback() |
| 4608 | - | liveSession.resizeTickID = 0 | |||
| 4609 | } | 4614 | } | ||
| 4610 | return keepRunning | 4615 | return keepRunning | ||
| 4611 | - | }) | 4616 | + | } |
| 4612 | - | session.resizeTickID = overlayWidget.AddTickCallback(&tickCallback, 0, nil) | 4617 | + | session.resizeTickCallback = callback |
| 4618 | + | session.resizeTickID = overlayWidget.AddTickCallback(callback, 0, nil) | |||
| 4613 | } | 4619 | } | ||
| 4614 | 4620 | ||||
| 4615 | func (a *App) stopFloatingResizeWatcher(session *floatingWorkspaceSession) { | 4621 | func (a *App) stopFloatingResizeWatcher(session *floatingWorkspaceSession) { | ||
| @@ -4617,13 +4623,30 @@ func (a *App) stopFloatingResizeWatcher(session *floatingWorkspaceSession) { | |||||
| 4617 | return | 4623 | return | ||
| 4618 | } | 4624 | } | ||
| 4619 | 4625 | ||||
| 4620 | - | if session.resizeTickID != 0 && session.overlay != nil { | 4626 | + | tickID := session.resizeTickID |
| 4627 | + | session.releaseResizeTickCallback() | |||
| 4628 | + | if tickID != 0 && session.overlay != nil { | |||
| 4621 | if overlayWidget := session.overlay.GtkWidget(); overlayWidget != nil { | 4629 | if overlayWidget := session.overlay.GtkWidget(); overlayWidget != nil { | ||
| 4622 | - | overlayWidget.RemoveTickCallback(session.resizeTickID) | 4630 | + | overlayWidget.RemoveTickCallback(tickID) |
| 4623 | } | 4631 | } | ||
| 4624 | } | 4632 | } | ||
| 4633 | + | } | |||
| 4634 | + | ||||
| 4635 | + | // releaseResizeTickCallback releases the raw purego callback slot. It is safe | |||
| 4636 | + | // to call after GTK has already removed a callback that returned false. | |||
| 4637 | + | func (session *floatingWorkspaceSession) releaseResizeTickCallback() { | |||
| 4638 | + | if session == nil { | |||
| 4639 | + | return | |||
| 4640 | + | } | |||
| 4641 | + | callback := session.resizeTickCallback | |||
| 4625 | session.resizeTickID = 0 | 4642 | session.resizeTickID = 0 | ||
| 4643 | + | session.resizeTickCallback = nil | |||
| 4626 | session.resizeWatcherActive = false | 4644 | session.resizeWatcherActive = false | ||
| 4645 | + | if callback != nil { | |||
| 4646 | + | // AddTickCallback creates a purego function-pointer slot. Clearing the Go | |||
| 4647 | + | // pointer alone cannot return that finite runtime slot to the ledger. | |||
| 4648 | + | _ = unrefTickCallback(callback) | |||
| 4649 | + | } | |||
| 4627 | } | 4650 | } | ||
| 4628 | 4651 | ||||
| 4629 | func (a *App) hideFloatingSession(ctx context.Context, session *floatingWorkspaceSession) { | 4652 | func (a *App) hideFloatingSession(ctx context.Context, session *floatingWorkspaceSession) { | ||
A internal/ui/component/omnibox_callback_pressure_stress_test.go +155 -0
| @@ -0,0 +1,155 @@ | |||
| 1 | + | package component | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "bufio" | |
| 5 | + | "encoding/json" | |
| 6 | + | "os" | |
| 7 | + | "testing" | |
| 8 | + | "time" | |
| 9 | + | ||
| 10 | + | "github.com/bnema/purego" | |
| 11 | + | "github.com/bnema/puregotk/v4/glib" | |
| 12 | + | "github.com/bnema/puregotk/v4/gobject" | |
| 13 | + | "github.com/bnema/puregotk/v4/gtk" | |
| 14 | + | "github.com/stretchr/testify/require" | |
| 15 | + | ) | |
| 16 | + | ||
| 17 | + | const gtkCallbackStressIterations = 3_200 | |
| 18 | + | ||
| 19 | + | type callbackLedgerEvent struct { | |
| 20 | + | Event string `json:"event"` | |
| 21 | + | Type string `json:"type"` | |
| 22 | + | } | |
| 23 | + | ||
| 24 | + | // runGTKCallbackLifecycleStress exercises real GTK motion controllers and raw | |
| 25 | + | // tick callbacks while p3 and p0 are repeatedly removed/rebuilt. It is never | |
| 26 | + | // opt-in: GTK init is skipped only when the host has no usable native display. | |
| 27 | + | func runGTKCallbackLifecycleStress(t *testing.T) { | |
| 28 | + | t.Helper() | |
| 29 | + | ledgerPath := t.TempDir() + "/purego-callback-ledger.jsonl" | |
| 30 | + | t.Setenv("PUREGO_CALLBACK_LEDGER", "1") | |
| 31 | + | t.Setenv("PUREGO_CALLBACK_LEDGER_FILE", ledgerPath) | |
| 32 | + | if !gtk.InitCheck() { | |
| 33 | + | t.Skip("GTK native display prerequisite unavailable (gtk.InitCheck returned false)") | |
| 34 | + | } | |
| 35 | + | ||
| 36 | + | mainContext := glib.MainContextDefault() | |
| 37 | + | require.NotNil(t, mainContext) | |
| 38 | + | done := make(chan struct{}) | |
| 39 | + | stress := glib.SourceFunc(func(uintptr) bool { | |
| 40 | + | defer close(done) | |
| 41 | + | runGTKCallbackLifecycleStressOnMain(t, mainContext, ledgerPath) | |
| 42 | + | return false | |
| 43 | + | }) | |
| 44 | + | // Invoke guarantees that the entire GTK mutation sequence is owned by the | |
| 45 | + | // default context, whether the test thread owns it or a native GTK loop does. | |
| 46 | + | mainContext.Invoke(&stress, 0) | |
| 47 | + | select { | |
| 48 | + | case <-done: | |
| 49 | + | case <-time.After(30 * time.Second): | |
| 50 | + | t.Fatal("GTK main context did not execute callback lifecycle stress") | |
| 51 | + | } | |
| 52 | + | } | |
| 53 | + | ||
| 54 | + | func runGTKCallbackLifecycleStressOnMain(t *testing.T, mainContext *glib.MainContext, ledgerPath string) { | |
| 55 | + | t.Helper() | |
| 56 | + | assertMainContextOwner := func() { | |
| 57 | + | t.Helper() | |
| 58 | + | require.True(t, mainContext.IsOwner(), "GTK mutation must own the default GLib main context") | |
| 59 | + | } | |
| 60 | + | ||
| 61 | + | assertMainContextOwner() | |
| 62 | + | root := gtk.NewBox(gtk.OrientationVerticalValue, 0) | |
| 63 | + | assertMainContextOwner() | |
| 64 | + | p0 := gtk.NewBox(gtk.OrientationVerticalValue, 0) | |
| 65 | + | assertMainContextOwner() | |
| 66 | + | p3 := gtk.NewBox(gtk.OrientationVerticalValue, 0) | |
| 67 | + | require.NotNil(t, root) | |
| 68 | + | require.NotNil(t, p0) | |
| 69 | + | require.NotNil(t, p3) | |
| 70 | + | assertMainContextOwner() | |
| 71 | + | root.Append(&p0.Widget) | |
| 72 | + | assertMainContextOwner() | |
| 73 | + | root.Append(&p3.Widget) | |
| 74 | + | ledgerStart := len(readCallbackLedgerEvents(t, ledgerPath)) | |
| 75 | + | ||
| 76 | + | enters, leaves := 0, 0 | |
| 77 | + | for i := 0; i < gtkCallbackStressIterations; i++ { | |
| 78 | + | pane := p0 | |
| 79 | + | if i%2 != 0 { | |
| 80 | + | pane = p3 | |
| 81 | + | } | |
| 82 | + | assertMainContextOwner() | |
| 83 | + | controller := gtk.NewEventControllerMotion() | |
| 84 | + | require.NotNil(t, controller) | |
| 85 | + | enter := func(_ gtk.EventControllerMotion, _ float64, _ float64) { enters++ } | |
| 86 | + | leave := func(_ gtk.EventControllerMotion) { leaves++ } | |
| 87 | + | enterID := controller.ConnectEnter(&enter) | |
| 88 | + | leaveID := controller.ConnectLeave(&leave) | |
| 89 | + | assertMainContextOwner() | |
| 90 | + | pane.AddController(&controller.EventController) | |
| 91 | + | ||
| 92 | + | // Invoke the real connected handler functions in GTK enter/leave order | |
| 93 | + | // while the alternate pane is closed and the workspace rebuilt. | |
| 94 | + | enter(*controller, 0, 0) | |
| 95 | + | leave(*controller) | |
| 96 | + | ||
| 97 | + | tick := gtk.TickCallback(func(uintptr, uintptr, uintptr) bool { return false }) | |
| 98 | + | tickCallback := glib.NewCallback(&tick) | |
| 99 | + | assertMainContextOwner() | |
| 100 | + | tickID := pane.AddTickCallback(&tick, 0, nil) | |
| 101 | + | require.NotZero(t, tickID) | |
| 102 | + | assertMainContextOwner() | |
| 103 | + | pane.RemoveTickCallback(tickID) | |
| 104 | + | require.NoError(t, purego.UnrefCallback(tickCallback), "raw tick slot must be released after GTK removal") | |
| 105 | + | ||
| 106 | + | assertMainContextOwner() | |
| 107 | + | gobject.SignalHandlerDisconnect(gobject.ObjectNewFromInternalPtr(controller.GoPointer()), enterID) | |
| 108 | + | assertMainContextOwner() | |
| 109 | + | gobject.SignalHandlerDisconnect(gobject.ObjectNewFromInternalPtr(controller.GoPointer()), leaveID) | |
| 110 | + | assertMainContextOwner() | |
| 111 | + | pane.RemoveController(&controller.EventController) | |
| 112 | + | ||
| 113 | + | // Alternate p3 <-> p0 close/rebuild mutations on the same main context. | |
| 114 | + | assertMainContextOwner() | |
| 115 | + | root.Remove(&pane.Widget) | |
| 116 | + | assertMainContextOwner() | |
| 117 | + | root.Append(&pane.Widget) | |
| 118 | + | } | |
| 119 | + | require.Equal(t, gtkCallbackStressIterations, enters) | |
| 120 | + | require.Equal(t, gtkCallbackStressIterations, leaves) | |
| 121 | + | ||
| 122 | + | events := readCallbackLedgerEvents(t, ledgerPath)[ledgerStart:] | |
| 123 | + | var tickReleases, tickReuses int | |
| 124 | + | for _, event := range events { | |
| 125 | + | switch event.Event { | |
| 126 | + | case "release-fnptr", "release": | |
| 127 | + | tickReleases++ | |
| 128 | + | case "reuse": | |
| 129 | + | tickReuses++ | |
| 130 | + | case "release-miss", "release-fnptr-miss", "exhausted": | |
| 131 | + | t.Fatalf("callback ledger recorded %s for tick callback", event.Event) | |
| 132 | + | } | |
| 133 | + | } | |
| 134 | + | require.Equal(t, gtkCallbackStressIterations, tickReleases, "every raw GTK tick slot must be released exactly once") | |
| 135 | + | // The first motion-controller connection may initialize one shared GTK | |
| 136 | + | // trampoline after the baseline; every subsequent raw tick slot is reused. | |
| 137 | + | require.GreaterOrEqual(t, tickReuses, gtkCallbackStressIterations-2, "released raw tick slots must be reused") | |
| 138 | + | } | |
| 139 | + | ||
| 140 | + | func readCallbackLedgerEvents(t *testing.T, path string) []callbackLedgerEvent { | |
| 141 | + | t.Helper() | |
| 142 | + | file, err := os.Open(path) | |
| 143 | + | require.NoError(t, err) | |
| 144 | + | defer file.Close() | |
| 145 | + | ||
| 146 | + | var events []callbackLedgerEvent | |
| 147 | + | scanner := bufio.NewScanner(file) | |
| 148 | + | for scanner.Scan() { | |
| 149 | + | var event callbackLedgerEvent | |
| 150 | + | require.NoError(t, json.Unmarshal(scanner.Bytes(), &event)) | |
| 151 | + | events = append(events, event) | |
| 152 | + | } | |
| 153 | + | require.NoError(t, scanner.Err()) | |
| 154 | + | return events | |
| 155 | + | } | |
| @@ -0,0 +1,155 @@ | |||||
| 1 | + | package component | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "bufio" | |||
| 5 | + | "encoding/json" | |||
| 6 | + | "os" | |||
| 7 | + | "testing" | |||
| 8 | + | "time" | |||
| 9 | + | ||||
| 10 | + | "github.com/bnema/purego" | |||
| 11 | + | "github.com/bnema/puregotk/v4/glib" | |||
| 12 | + | "github.com/bnema/puregotk/v4/gobject" | |||
| 13 | + | "github.com/bnema/puregotk/v4/gtk" | |||
| 14 | + | "github.com/stretchr/testify/require" | |||
| 15 | + | ) | |||
| 16 | + | ||||
| 17 | + | const gtkCallbackStressIterations = 3_200 | |||
| 18 | + | ||||
| 19 | + | type callbackLedgerEvent struct { | |||
| 20 | + | Event string `json:"event"` | |||
| 21 | + | Type string `json:"type"` | |||
| 22 | + | } | |||
| 23 | + | ||||
| 24 | + | // runGTKCallbackLifecycleStress exercises real GTK motion controllers and raw | |||
| 25 | + | // tick callbacks while p3 and p0 are repeatedly removed/rebuilt. It is never | |||
| 26 | + | // opt-in: GTK init is skipped only when the host has no usable native display. | |||
| 27 | + | func runGTKCallbackLifecycleStress(t *testing.T) { | |||
| 28 | + | t.Helper() | |||
| 29 | + | ledgerPath := t.TempDir() + "/purego-callback-ledger.jsonl" | |||
| 30 | + | t.Setenv("PUREGO_CALLBACK_LEDGER", "1") | |||
| 31 | + | t.Setenv("PUREGO_CALLBACK_LEDGER_FILE", ledgerPath) | |||
| 32 | + | if !gtk.InitCheck() { | |||
| 33 | + | t.Skip("GTK native display prerequisite unavailable (gtk.InitCheck returned false)") | |||
| 34 | + | } | |||
| 35 | + | ||||
| 36 | + | mainContext := glib.MainContextDefault() | |||
| 37 | + | require.NotNil(t, mainContext) | |||
| 38 | + | done := make(chan struct{}) | |||
| 39 | + | stress := glib.SourceFunc(func(uintptr) bool { | |||
| 40 | + | defer close(done) | |||
| 41 | + | runGTKCallbackLifecycleStressOnMain(t, mainContext, ledgerPath) | |||
| 42 | + | return false | |||
| 43 | + | }) | |||
| 44 | + | // Invoke guarantees that the entire GTK mutation sequence is owned by the | |||
| 45 | + | // default context, whether the test thread owns it or a native GTK loop does. | |||
| 46 | + | mainContext.Invoke(&stress, 0) | |||
| 47 | + | select { | |||
| 48 | + | case <-done: | |||
| 49 | + | case <-time.After(30 * time.Second): | |||
| 50 | + | t.Fatal("GTK main context did not execute callback lifecycle stress") | |||
| 51 | + | } | |||
| 52 | + | } | |||
| 53 | + | ||||
| 54 | + | func runGTKCallbackLifecycleStressOnMain(t *testing.T, mainContext *glib.MainContext, ledgerPath string) { | |||
| 55 | + | t.Helper() | |||
| 56 | + | assertMainContextOwner := func() { | |||
| 57 | + | t.Helper() | |||
| 58 | + | require.True(t, mainContext.IsOwner(), "GTK mutation must own the default GLib main context") | |||
| 59 | + | } | |||
| 60 | + | ||||
| 61 | + | assertMainContextOwner() | |||
| 62 | + | root := gtk.NewBox(gtk.OrientationVerticalValue, 0) | |||
| 63 | + | assertMainContextOwner() | |||
| 64 | + | p0 := gtk.NewBox(gtk.OrientationVerticalValue, 0) | |||
| 65 | + | assertMainContextOwner() | |||
| 66 | + | p3 := gtk.NewBox(gtk.OrientationVerticalValue, 0) | |||
| 67 | + | require.NotNil(t, root) | |||
| 68 | + | require.NotNil(t, p0) | |||
| 69 | + | require.NotNil(t, p3) | |||
| 70 | + | assertMainContextOwner() | |||
| 71 | + | root.Append(&p0.Widget) | |||
| 72 | + | assertMainContextOwner() | |||
| 73 | + | root.Append(&p3.Widget) | |||
| 74 | + | ledgerStart := len(readCallbackLedgerEvents(t, ledgerPath)) | |||
| 75 | + | ||||
| 76 | + | enters, leaves := 0, 0 | |||
| 77 | + | for i := 0; i < gtkCallbackStressIterations; i++ { | |||
| 78 | + | pane := p0 | |||
| 79 | + | if i%2 != 0 { | |||
| 80 | + | pane = p3 | |||
| 81 | + | } | |||
| 82 | + | assertMainContextOwner() | |||
| 83 | + | controller := gtk.NewEventControllerMotion() | |||
| 84 | + | require.NotNil(t, controller) | |||
| 85 | + | enter := func(_ gtk.EventControllerMotion, _ float64, _ float64) { enters++ } | |||
| 86 | + | leave := func(_ gtk.EventControllerMotion) { leaves++ } | |||
| 87 | + | enterID := controller.ConnectEnter(&enter) | |||
| 88 | + | leaveID := controller.ConnectLeave(&leave) | |||
| 89 | + | assertMainContextOwner() | |||
| 90 | + | pane.AddController(&controller.EventController) | |||
| 91 | + | ||||
| 92 | + | // Invoke the real connected handler functions in GTK enter/leave order | |||
| 93 | + | // while the alternate pane is closed and the workspace rebuilt. | |||
| 94 | + | enter(*controller, 0, 0) | |||
| 95 | + | leave(*controller) | |||
| 96 | + | ||||
| 97 | + | tick := gtk.TickCallback(func(uintptr, uintptr, uintptr) bool { return false }) | |||
| 98 | + | tickCallback := glib.NewCallback(&tick) | |||
| 99 | + | assertMainContextOwner() | |||
| 100 | + | tickID := pane.AddTickCallback(&tick, 0, nil) | |||
| 101 | + | require.NotZero(t, tickID) | |||
| 102 | + | assertMainContextOwner() | |||
| 103 | + | pane.RemoveTickCallback(tickID) | |||
| 104 | + | require.NoError(t, purego.UnrefCallback(tickCallback), "raw tick slot must be released after GTK removal") | |||
| 105 | + | ||||
| 106 | + | assertMainContextOwner() | |||
| 107 | + | gobject.SignalHandlerDisconnect(gobject.ObjectNewFromInternalPtr(controller.GoPointer()), enterID) | |||
| 108 | + | assertMainContextOwner() | |||
| 109 | + | gobject.SignalHandlerDisconnect(gobject.ObjectNewFromInternalPtr(controller.GoPointer()), leaveID) | |||
| 110 | + | assertMainContextOwner() | |||
| 111 | + | pane.RemoveController(&controller.EventController) | |||
| 112 | + | ||||
| 113 | + | // Alternate p3 <-> p0 close/rebuild mutations on the same main context. | |||
| 114 | + | assertMainContextOwner() | |||
| 115 | + | root.Remove(&pane.Widget) | |||
| 116 | + | assertMainContextOwner() | |||
| 117 | + | root.Append(&pane.Widget) | |||
| 118 | + | } | |||
| 119 | + | require.Equal(t, gtkCallbackStressIterations, enters) | |||
| 120 | + | require.Equal(t, gtkCallbackStressIterations, leaves) | |||
| 121 | + | ||||
| 122 | + | events := readCallbackLedgerEvents(t, ledgerPath)[ledgerStart:] | |||
| 123 | + | var tickReleases, tickReuses int | |||
| 124 | + | for _, event := range events { | |||
| 125 | + | switch event.Event { | |||
| 126 | + | case "release-fnptr", "release": | |||
| 127 | + | tickReleases++ | |||
| 128 | + | case "reuse": | |||
| 129 | + | tickReuses++ | |||
| 130 | + | case "release-miss", "release-fnptr-miss", "exhausted": | |||
| 131 | + | t.Fatalf("callback ledger recorded %s for tick callback", event.Event) | |||
| 132 | + | } | |||
| 133 | + | } | |||
| 134 | + | require.Equal(t, gtkCallbackStressIterations, tickReleases, "every raw GTK tick slot must be released exactly once") | |||
| 135 | + | // The first motion-controller connection may initialize one shared GTK | |||
| 136 | + | // trampoline after the baseline; every subsequent raw tick slot is reused. | |||
| 137 | + | require.GreaterOrEqual(t, tickReuses, gtkCallbackStressIterations-2, "released raw tick slots must be reused") | |||
| 138 | + | } | |||
| 139 | + | ||||
| 140 | + | func readCallbackLedgerEvents(t *testing.T, path string) []callbackLedgerEvent { | |||
| 141 | + | t.Helper() | |||
| 142 | + | file, err := os.Open(path) | |||
| 143 | + | require.NoError(t, err) | |||
| 144 | + | defer file.Close() | |||
| 145 | + | ||||
| 146 | + | var events []callbackLedgerEvent | |||
| 147 | + | scanner := bufio.NewScanner(file) | |||
| 148 | + | for scanner.Scan() { | |||
| 149 | + | var event callbackLedgerEvent | |||
| 150 | + | require.NoError(t, json.Unmarshal(scanner.Bytes(), &event)) | |||
| 151 | + | events = append(events, event) | |||
| 152 | + | } | |||
| 153 | + | require.NoError(t, scanner.Err()) | |||
| 154 | + | return events | |||
| 155 | + | } | |||
M internal/ui/component/omnibox_callback_pressure_test.go +2 -8
| @@ -1,13 +1,7 @@ | |||
| 1 | 1 | package component | |
| 2 | 2 | ||
| 3 | - | import ( | |
| 4 | - | "os" | |
| 5 | - | "testing" | |
| 6 | - | ) | |
| 3 | + | import "testing" | |
| 7 | 4 | ||
| 8 | 5 | func TestOmniboxCallbackPressure_NoCrashSignal(t *testing.T) { | |
| 9 | - | if os.Getenv("OMNIBOX_STRESS") != "1" { | |
| 10 | - | t.Skip("skipping: set OMNIBOX_STRESS=1 to run with GTK stress harness") | |
| 11 | - | } | |
| 12 | - | // TODO: wire GTK stress harness and assert no abrupt crash markers | |
| 6 | + | runGTKCallbackLifecycleStress(t) | |
| 13 | 7 | } | |
| @@ -1,13 +1,7 @@ | |||||
| 1 | package component | 1 | package component | ||
| 2 | 2 | ||||
| 3 | - | import ( | 3 | + | import "testing" |
| 4 | - | "os" | |||
| 5 | - | "testing" | |||
| 6 | - | ) | |||
| 7 | 4 | ||||
| 8 | func TestOmniboxCallbackPressure_NoCrashSignal(t *testing.T) { | 5 | func TestOmniboxCallbackPressure_NoCrashSignal(t *testing.T) { | ||
| 9 | - | if os.Getenv("OMNIBOX_STRESS") != "1" { | 6 | + | runGTKCallbackLifecycleStress(t) |
| 10 | - | t.Skip("skipping: set OMNIBOX_STRESS=1 to run with GTK stress harness") | |||
| 11 | - | } | |||
| 12 | - | // TODO: wire GTK stress harness and assert no abrupt crash markers | |||
| 13 | } | 7 | } | ||
M internal/ui/component/pane_view.go +5 -0
| @@ -516,6 +516,7 @@ func (pv *PaneView) Cleanup() { | |||
| 516 | 516 | pv.onFocusIn = nil | |
| 517 | 517 | pv.onFocusOut = nil | |
| 518 | 518 | pv.onHover = nil | |
| 519 | + | pv.onMouseMotion = nil | |
| 519 | 520 | ||
| 520 | 521 | // Detach hover handler if present | |
| 521 | 522 | if pv.hoverHandler != nil { | |
| @@ -530,6 +531,10 @@ func (pv *PaneView) Cleanup() { | |||
| 530 | 531 | } | |
| 531 | 532 | ||
| 532 | 533 | // Clear other overlay children | |
| 534 | + | if pv.loading != nil { | |
| 535 | + | pv.overlay.RemoveOverlay(pv.loading.Widget()) | |
| 536 | + | pv.loading = nil | |
| 537 | + | } | |
| 533 | 538 | if pv.progressBar != nil { | |
| 534 | 539 | pv.overlay.RemoveOverlay(pv.progressBar.Widget()) | |
| 535 | 540 | pv.progressBar = nil | |
| @@ -516,6 +516,7 @@ func (pv *PaneView) Cleanup() { | |||||
| 516 | pv.onFocusIn = nil | 516 | pv.onFocusIn = nil | ||
| 517 | pv.onFocusOut = nil | 517 | pv.onFocusOut = nil | ||
| 518 | pv.onHover = nil | 518 | pv.onHover = nil | ||
| 519 | + | pv.onMouseMotion = nil | |||
| 519 | 520 | ||||
| 520 | // Detach hover handler if present | 521 | // Detach hover handler if present | ||
| 521 | if pv.hoverHandler != nil { | 522 | if pv.hoverHandler != nil { | ||
| @@ -530,6 +531,10 @@ func (pv *PaneView) Cleanup() { | |||||
| 530 | } | 531 | } | ||
| 531 | 532 | ||||
| 532 | // Clear other overlay children | 533 | // Clear other overlay children | ||
| 534 | + | if pv.loading != nil { | |||
| 535 | + | pv.overlay.RemoveOverlay(pv.loading.Widget()) | |||
| 536 | + | pv.loading = nil | |||
| 537 | + | } | |||
| 533 | if pv.progressBar != nil { | 538 | if pv.progressBar != nil { | ||
| 534 | pv.overlay.RemoveOverlay(pv.progressBar.Widget()) | 539 | pv.overlay.RemoveOverlay(pv.progressBar.Widget()) | ||
| 535 | pv.progressBar = nil | 540 | pv.progressBar = nil | ||
A internal/ui/component/pane_view_cleanup_test.go +13 -0
| @@ -0,0 +1,13 @@ | |||
| 1 | + | package component | |
| 2 | + | ||
| 3 | + | import "testing" | |
| 4 | + | ||
| 5 | + | func TestPaneViewCleanupClearsMouseMotionCallback(t *testing.T) { | |
| 6 | + | pv := &PaneView{onMouseMotion: func() { t.Fatal("stale callback invoked") }} | |
| 7 | + | ||
| 8 | + | pv.Cleanup() | |
| 9 | + | ||
| 10 | + | if pv.onMouseMotion != nil { | |
| 11 | + | t.Fatal("Cleanup must release the mouse-motion callback slot") | |
| 12 | + | } | |
| 13 | + | } | |
| @@ -0,0 +1,13 @@ | |||||
| 1 | + | package component | |||
| 2 | + | ||||
| 3 | + | import "testing" | |||
| 4 | + | ||||
| 5 | + | func TestPaneViewCleanupClearsMouseMotionCallback(t *testing.T) { | |||
| 6 | + | pv := &PaneView{onMouseMotion: func() { t.Fatal("stale callback invoked") }} | |||
| 7 | + | ||||
| 8 | + | pv.Cleanup() | |||
| 9 | + | ||||
| 10 | + | if pv.onMouseMotion != nil { | |||
| 11 | + | t.Fatal("Cleanup must release the mouse-motion callback slot") | |||
| 12 | + | } | |||
| 13 | + | } | |||
M internal/ui/component/workspace_view.go +8 -0
| @@ -192,6 +192,14 @@ func (wv *WorkspaceView) SetWorkspace(ctx context.Context, ws *entity.Workspace) | |||
| 192 | 192 | wv.mu.Lock() | |
| 193 | 193 | defer wv.mu.Unlock() | |
| 194 | 194 | ||
| 195 | + | // Release previous views before dropping their ownership. This also detaches | |
| 196 | + | // hover controllers and callback closures before their widgets are rebuilt. | |
| 197 | + | for _, pv := range wv.paneViews { | |
| 198 | + | if pv != nil { | |
| 199 | + | pv.Cleanup() | |
| 200 | + | } | |
| 201 | + | } | |
| 202 | + | ||
| 195 | 203 | // Clear previous state | |
| 196 | 204 | wv.workspace = ws | |
| 197 | 205 | wv.paneViews = make(map[entity.PaneID]*PaneView) | |
| @@ -192,6 +192,14 @@ func (wv *WorkspaceView) SetWorkspace(ctx context.Context, ws *entity.Workspace) | |||||
| 192 | wv.mu.Lock() | 192 | wv.mu.Lock() | ||
| 193 | defer wv.mu.Unlock() | 193 | defer wv.mu.Unlock() | ||
| 194 | 194 | ||||
| 195 | + | // Release previous views before dropping their ownership. This also detaches | |||
| 196 | + | // hover controllers and callback closures before their widgets are rebuilt. | |||
| 197 | + | for _, pv := range wv.paneViews { | |||
| 198 | + | if pv != nil { | |||
| 199 | + | pv.Cleanup() | |||
| 200 | + | } | |||
| 201 | + | } | |||
| 202 | + | ||||
| 195 | // Clear previous state | 203 | // Clear previous state | ||
| 196 | wv.workspace = ws | 204 | wv.workspace = ws | ||
| 197 | wv.paneViews = make(map[entity.PaneID]*PaneView) | 205 | wv.paneViews = make(map[entity.PaneID]*PaneView) | ||
A internal/ui/floating_pane_callback_cleanup_test.go +32 -0
| @@ -0,0 +1,32 @@ | |||
| 1 | + | package ui | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "testing" | |
| 5 | + | ||
| 6 | + | "github.com/bnema/puregotk/v4/gtk" | |
| 7 | + | ) | |
| 8 | + | ||
| 9 | + | func TestStopFloatingResizeWatcherReleasesRawCallbackSlot(t *testing.T) { | |
| 10 | + | callback := new(gtk.TickCallback) | |
| 11 | + | session := &floatingWorkspaceSession{resizeTickCallback: callback, resizeTickID: 17, resizeWatcherActive: true} | |
| 12 | + | oldUnref := unrefTickCallback | |
| 13 | + | defer func() { unrefTickCallback = oldUnref }() | |
| 14 | + | var unrefs int | |
| 15 | + | unrefTickCallback = func(got any) error { | |
| 16 | + | if got != callback { | |
| 17 | + | t.Fatalf("unref callback = %p, want %p", got, callback) | |
| 18 | + | } | |
| 19 | + | unrefs++ | |
| 20 | + | return nil | |
| 21 | + | } | |
| 22 | + | ||
| 23 | + | (&App{}).stopFloatingResizeWatcher(session) | |
| 24 | + | (&App{}).stopFloatingResizeWatcher(session) | |
| 25 | + | ||
| 26 | + | if session.resizeTickCallback != nil || session.resizeTickID != 0 || session.resizeWatcherActive { | |
| 27 | + | t.Fatal("stop must release the floating resize callback slot exactly once") | |
| 28 | + | } | |
| 29 | + | if unrefs != 1 { | |
| 30 | + | t.Fatalf("callback unrefs = %d, want 1", unrefs) | |
| 31 | + | } | |
| 32 | + | } | |
| @@ -0,0 +1,32 @@ | |||||
| 1 | + | package ui | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "testing" | |||
| 5 | + | ||||
| 6 | + | "github.com/bnema/puregotk/v4/gtk" | |||
| 7 | + | ) | |||
| 8 | + | ||||
| 9 | + | func TestStopFloatingResizeWatcherReleasesRawCallbackSlot(t *testing.T) { | |||
| 10 | + | callback := new(gtk.TickCallback) | |||
| 11 | + | session := &floatingWorkspaceSession{resizeTickCallback: callback, resizeTickID: 17, resizeWatcherActive: true} | |||
| 12 | + | oldUnref := unrefTickCallback | |||
| 13 | + | defer func() { unrefTickCallback = oldUnref }() | |||
| 14 | + | var unrefs int | |||
| 15 | + | unrefTickCallback = func(got any) error { | |||
| 16 | + | if got != callback { | |||
| 17 | + | t.Fatalf("unref callback = %p, want %p", got, callback) | |||
| 18 | + | } | |||
| 19 | + | unrefs++ | |||
| 20 | + | return nil | |||
| 21 | + | } | |||
| 22 | + | ||||
| 23 | + | (&App{}).stopFloatingResizeWatcher(session) | |||
| 24 | + | (&App{}).stopFloatingResizeWatcher(session) | |||
| 25 | + | ||||
| 26 | + | if session.resizeTickCallback != nil || session.resizeTickID != 0 || session.resizeWatcherActive { | |||
| 27 | + | t.Fatal("stop must release the floating resize callback slot exactly once") | |||
| 28 | + | } | |||
| 29 | + | if unrefs != 1 { | |||
| 30 | + | t.Fatalf("callback unrefs = %d, want 1", unrefs) | |||
| 31 | + | } | |||
| 32 | + | } | |||
M internal/ui/input/hover.go +119 -41
| @@ -7,6 +7,8 @@ import ( | |||
| 7 | 7 | ||
| 8 | 8 | "github.com/bnema/dumber/internal/domain/entity" | |
| 9 | 9 | "github.com/bnema/dumber/internal/logging" | |
| 10 | + | "github.com/bnema/puregotk/v4/glib" | |
| 11 | + | "github.com/bnema/puregotk/v4/gobject" | |
| 10 | 12 | "github.com/bnema/puregotk/v4/gtk" | |
| 11 | 13 | ) | |
| 12 | 14 | ||
| @@ -24,19 +26,28 @@ type MotionCallback func() | |||
| 24 | 26 | // HoverHandler handles mouse hover events for focus-follows-mouse behavior. | |
| 25 | 27 | // It uses a debounce timer to avoid rapid focus switches. | |
| 26 | 28 | type HoverHandler struct { | |
| 27 | - | motionCtrl *gtk.EventControllerMotion | |
| 28 | - | paneID entity.PaneID | |
| 29 | - | onEnter HoverCallback | |
| 30 | - | onMotion MotionCallback | |
| 29 | + | attachedWidget *gtk.Widget | |
| 30 | + | motionCtrl *gtk.EventControllerMotion | |
| 31 | + | enterSignalID uint | |
| 32 | + | leaveSignalID uint | |
| 33 | + | motionSignalID uint | |
| 34 | + | paneID entity.PaneID | |
| 35 | + | onEnter HoverCallback | |
| 36 | + | onMotion MotionCallback | |
| 31 | 37 | ||
| 32 | 38 | // Callback retention: must stay reachable by Go GC. | |
| 33 | 39 | enterCb func(gtk.EventControllerMotion, float64, float64) | |
| 34 | 40 | leaveCb func(gtk.EventControllerMotion) | |
| 35 | 41 | motionCb func(gtk.EventControllerMotion, float64, float64) | |
| 36 | 42 | ||
| 37 | - | timer *time.Timer | |
| 38 | - | timerMu sync.Mutex | |
| 39 | - | canceled bool | |
| 43 | + | timerMu sync.Mutex | |
| 44 | + | sourceID uint | |
| 45 | + | generation uint64 | |
| 46 | + | detached bool | |
| 47 | + | schedule func(uint, func(uintptr) bool) uint | |
| 48 | + | removeSource func(uint) bool | |
| 49 | + | disconnectSignal func(*gtk.EventControllerMotion, uint) | |
| 50 | + | removeController func(*gtk.Widget, *gtk.EventController) | |
| 40 | 51 | ||
| 41 | 52 | ctx context.Context | |
| 42 | 53 | } | |
| @@ -47,8 +58,12 @@ func NewHoverHandler(ctx context.Context, paneID entity.PaneID) *HoverHandler { | |||
| 47 | 58 | log.Debug().Str("pane_id", string(paneID)).Msg("creating hover handler") | |
| 48 | 59 | ||
| 49 | 60 | return &HoverHandler{ | |
| 50 | - | ctx: ctx, | |
| 51 | - | paneID: paneID, | |
| 61 | + | ctx: ctx, | |
| 62 | + | paneID: paneID, | |
| 63 | + | schedule: scheduleHoverSource, | |
| 64 | + | removeSource: glib.SourceRemove, | |
| 65 | + | disconnectSignal: disconnectHoverSignal, | |
| 66 | + | removeController: removeHoverController, | |
| 52 | 67 | } | |
| 53 | 68 | } | |
| 54 | 69 | ||
| @@ -73,6 +88,15 @@ func (h *HoverHandler) AttachTo(widget *gtk.Widget) { | |||
| 73 | 88 | return | |
| 74 | 89 | } | |
| 75 | 90 | ||
| 91 | + | // Reattachment must not leave a controller owned by the previous widget. | |
| 92 | + | if h.motionCtrl != nil || h.attachedWidget != nil { | |
| 93 | + | h.Detach() | |
| 94 | + | } | |
| 95 | + | h.timerMu.Lock() | |
| 96 | + | h.detached = false | |
| 97 | + | h.attachedWidget = widget | |
| 98 | + | h.timerMu.Unlock() | |
| 99 | + | ||
| 76 | 100 | h.motionCtrl = gtk.NewEventControllerMotion() | |
| 77 | 101 | if h.motionCtrl == nil { | |
| 78 | 102 | log.Error().Msg("failed to create motion controller") | |
| @@ -83,13 +107,13 @@ func (h *HoverHandler) AttachTo(widget *gtk.Widget) { | |||
| 83 | 107 | h.enterCb = func(_ gtk.EventControllerMotion, _ float64, _ float64) { | |
| 84 | 108 | h.handleEnter() | |
| 85 | 109 | } | |
| 86 | - | h.motionCtrl.ConnectEnter(&h.enterCb) | |
| 110 | + | h.enterSignalID = h.motionCtrl.ConnectEnter(&h.enterCb) | |
| 87 | 111 | ||
| 88 | 112 | // Connect leave handler to cancel pending focus | |
| 89 | 113 | h.leaveCb = func(_ gtk.EventControllerMotion) { | |
| 90 | 114 | h.handleLeave() | |
| 91 | 115 | } | |
| 92 | - | h.motionCtrl.ConnectLeave(&h.leaveCb) | |
| 116 | + | h.leaveSignalID = h.motionCtrl.ConnectLeave(&h.leaveCb) | |
| 93 | 117 | ||
| 94 | 118 | // Connect motion handler to detect intentional mouse movement. | |
| 95 | 119 | // Unlike enter/leave, motion only fires when the cursor physically moves | |
| @@ -99,7 +123,7 @@ func (h *HoverHandler) AttachTo(widget *gtk.Widget) { | |||
| 99 | 123 | h.onMotion() | |
| 100 | 124 | } | |
| 101 | 125 | } | |
| 102 | - | h.motionCtrl.ConnectMotion(&h.motionCb) | |
| 126 | + | h.motionSignalID = h.motionCtrl.ConnectMotion(&h.motionCb) | |
| 103 | 127 | ||
| 104 | 128 | // Add controller to widget | |
| 105 | 129 | widget.AddController(&h.motionCtrl.EventController) | |
| @@ -107,32 +131,49 @@ func (h *HoverHandler) AttachTo(widget *gtk.Widget) { | |||
| 107 | 131 | log.Debug().Str("pane_id", string(h.paneID)).Msg("hover handler attached to widget") | |
| 108 | 132 | } | |
| 109 | 133 | ||
| 134 | + | // scheduleHoverSource schedules a one-shot callback on GTK's owning GLib main | |
| 135 | + | // context. GTK state must never be touched by a Go timer goroutine. | |
| 136 | + | func scheduleHoverSource(delay uint, fn func(uintptr) bool) uint { | |
| 137 | + | cb := glib.SourceFunc(fn) | |
| 138 | + | return glib.TimeoutAdd(delay, &cb, 0) | |
| 139 | + | } | |
| 140 | + | ||
| 110 | 141 | // handleEnter processes the mouse enter event with debouncing. | |
| 111 | 142 | func (h *HoverHandler) handleEnter() { | |
| 112 | 143 | h.timerMu.Lock() | |
| 113 | - | defer h.timerMu.Unlock() | |
| 114 | - | ||
| 115 | - | // Cancel any existing timer | |
| 116 | - | if h.timer != nil { | |
| 117 | - | h.timer.Stop() | |
| 144 | + | if h.detached { | |
| 145 | + | h.timerMu.Unlock() | |
| 146 | + | return | |
| 118 | 147 | } | |
| 119 | - | ||
| 120 | - | h.canceled = false | |
| 121 | - | ||
| 122 | - | // Start new debounce timer | |
| 123 | - | h.timer = time.AfterFunc(HoverFocusDelay, func() { | |
| 148 | + | h.cancelSourceLocked() | |
| 149 | + | h.generation++ | |
| 150 | + | generation := h.generation | |
| 151 | + | schedule := h.schedule | |
| 152 | + | h.sourceID = schedule(uint(HoverFocusDelay.Milliseconds()), func(_ uintptr) bool { | |
| 124 | 153 | h.timerMu.Lock() | |
| 125 | - | canceled := h.canceled | |
| 126 | - | h.timerMu.Unlock() | |
| 127 | - | ||
| 128 | - | if canceled { | |
| 129 | - | return | |
| 154 | + | if h.detached || h.generation != generation { | |
| 155 | + | h.timerMu.Unlock() | |
| 156 | + | return false | |
| 130 | 157 | } | |
| 158 | + | h.sourceID = 0 | |
| 159 | + | callback := h.onEnter | |
| 160 | + | paneID := h.paneID | |
| 161 | + | h.timerMu.Unlock() | |
| 131 | 162 | ||
| 132 | - | if h.onEnter != nil { | |
| 133 | - | h.onEnter(h.paneID) | |
| 163 | + | if callback != nil { | |
| 164 | + | callback(paneID) | |
| 134 | 165 | } | |
| 166 | + | return false | |
| 135 | 167 | }) | |
| 168 | + | h.timerMu.Unlock() | |
| 169 | + | } | |
| 170 | + | ||
| 171 | + | func (h *HoverHandler) cancelSourceLocked() { | |
| 172 | + | if h.sourceID == 0 { | |
| 173 | + | return | |
| 174 | + | } | |
| 175 | + | h.removeSource(h.sourceID) | |
| 176 | + | h.sourceID = 0 | |
| 136 | 177 | } | |
| 137 | 178 | ||
| 138 | 179 | // handleLeave cancels any pending focus switch. | |
| @@ -146,24 +187,61 @@ func (h *HoverHandler) Cancel() { | |||
| 146 | 187 | h.timerMu.Lock() | |
| 147 | 188 | defer h.timerMu.Unlock() | |
| 148 | 189 | ||
| 149 | - | h.canceled = true | |
| 150 | - | if h.timer != nil { | |
| 151 | - | h.timer.Stop() | |
| 152 | - | h.timer = nil | |
| 153 | - | } | |
| 190 | + | h.generation++ | |
| 191 | + | h.cancelSourceLocked() | |
| 154 | 192 | } | |
| 155 | 193 | ||
| 156 | - | // Detach removes the hover handler. | |
| 194 | + | // Detach removes the hover handler and invalidates callbacks already queued on | |
| 195 | + | // the GLib main context. The generation check is required because removing a | |
| 196 | + | // source can race with dispatch. | |
| 157 | 197 | func (h *HoverHandler) Detach() { | |
| 158 | 198 | h.timerMu.Lock() | |
| 159 | - | if h.timer != nil { | |
| 160 | - | h.timer.Stop() | |
| 161 | - | h.timer = nil | |
| 162 | - | } | |
| 163 | - | h.timerMu.Unlock() | |
| 164 | - | ||
| 199 | + | h.detached = true | |
| 200 | + | h.generation++ | |
| 201 | + | h.cancelSourceLocked() | |
| 202 | + | h.onEnter = nil | |
| 203 | + | h.onMotion = nil | |
| 204 | + | widget := h.attachedWidget | |
| 205 | + | controller := h.motionCtrl | |
| 206 | + | enterID := h.enterSignalID | |
| 207 | + | leaveID := h.leaveSignalID | |
| 208 | + | motionID := h.motionSignalID | |
| 209 | + | h.attachedWidget = nil | |
| 165 | 210 | h.motionCtrl = nil | |
| 211 | + | h.enterSignalID = 0 | |
| 212 | + | h.leaveSignalID = 0 | |
| 213 | + | h.motionSignalID = 0 | |
| 166 | 214 | h.enterCb = nil | |
| 167 | 215 | h.leaveCb = nil | |
| 168 | 216 | h.motionCb = nil | |
| 217 | + | disconnectSignal := h.disconnectSignal | |
| 218 | + | removeController := h.removeController | |
| 219 | + | h.timerMu.Unlock() | |
| 220 | + | ||
| 221 | + | if controller == nil { | |
| 222 | + | return | |
| 223 | + | } | |
| 224 | + | if disconnectSignal != nil { | |
| 225 | + | for _, id := range []uint{enterID, leaveID, motionID} { | |
| 226 | + | if id != 0 { | |
| 227 | + | disconnectSignal(controller, id) | |
| 228 | + | } | |
| 229 | + | } | |
| 230 | + | } | |
| 231 | + | if widget != nil && removeController != nil { | |
| 232 | + | removeController(widget, &controller.EventController) | |
| 233 | + | } | |
| 234 | + | } | |
| 235 | + | ||
| 236 | + | func disconnectHoverSignal(controller *gtk.EventControllerMotion, id uint) { | |
| 237 | + | if controller == nil || id == 0 || controller.GoPointer() == 0 { | |
| 238 | + | return | |
| 239 | + | } | |
| 240 | + | gobject.SignalHandlerDisconnect(gobject.ObjectNewFromInternalPtr(controller.GoPointer()), id) | |
| 241 | + | } | |
| 242 | + | ||
| 243 | + | func removeHoverController(widget *gtk.Widget, controller *gtk.EventController) { | |
| 244 | + | if widget != nil && controller != nil { | |
| 245 | + | widget.RemoveController(controller) | |
| 246 | + | } | |
| 169 | 247 | } | |
| @@ -7,6 +7,8 @@ import ( | |||||
| 7 | 7 | ||||
| 8 | "github.com/bnema/dumber/internal/domain/entity" | 8 | "github.com/bnema/dumber/internal/domain/entity" | ||
| 9 | "github.com/bnema/dumber/internal/logging" | 9 | "github.com/bnema/dumber/internal/logging" | ||
| 10 | + | "github.com/bnema/puregotk/v4/glib" | |||
| 11 | + | "github.com/bnema/puregotk/v4/gobject" | |||
| 10 | "github.com/bnema/puregotk/v4/gtk" | 12 | "github.com/bnema/puregotk/v4/gtk" | ||
| 11 | ) | 13 | ) | ||
| 12 | 14 | ||||
| @@ -24,19 +26,28 @@ type MotionCallback func() | |||||
| 24 | // HoverHandler handles mouse hover events for focus-follows-mouse behavior. | 26 | // HoverHandler handles mouse hover events for focus-follows-mouse behavior. | ||
| 25 | // It uses a debounce timer to avoid rapid focus switches. | 27 | // It uses a debounce timer to avoid rapid focus switches. | ||
| 26 | type HoverHandler struct { | 28 | type HoverHandler struct { | ||
| 27 | - | motionCtrl *gtk.EventControllerMotion | 29 | + | attachedWidget *gtk.Widget |
| 28 | - | paneID entity.PaneID | 30 | + | motionCtrl *gtk.EventControllerMotion |
| 29 | - | onEnter HoverCallback | 31 | + | enterSignalID uint |
| 30 | - | onMotion MotionCallback | 32 | + | leaveSignalID uint |
| 33 | + | motionSignalID uint | |||
| 34 | + | paneID entity.PaneID | |||
| 35 | + | onEnter HoverCallback | |||
| 36 | + | onMotion MotionCallback | |||
| 31 | 37 | ||||
| 32 | // Callback retention: must stay reachable by Go GC. | 38 | // Callback retention: must stay reachable by Go GC. | ||
| 33 | enterCb func(gtk.EventControllerMotion, float64, float64) | 39 | enterCb func(gtk.EventControllerMotion, float64, float64) | ||
| 34 | leaveCb func(gtk.EventControllerMotion) | 40 | leaveCb func(gtk.EventControllerMotion) | ||
| 35 | motionCb func(gtk.EventControllerMotion, float64, float64) | 41 | motionCb func(gtk.EventControllerMotion, float64, float64) | ||
| 36 | 42 | ||||
| 37 | - | timer *time.Timer | 43 | + | timerMu sync.Mutex |
| 38 | - | timerMu sync.Mutex | 44 | + | sourceID uint |
| 39 | - | canceled bool | 45 | + | generation uint64 |
| 46 | + | detached bool | |||
| 47 | + | schedule func(uint, func(uintptr) bool) uint | |||
| 48 | + | removeSource func(uint) bool | |||
| 49 | + | disconnectSignal func(*gtk.EventControllerMotion, uint) | |||
| 50 | + | removeController func(*gtk.Widget, *gtk.EventController) | |||
| 40 | 51 | ||||
| 41 | ctx context.Context | 52 | ctx context.Context | ||
| 42 | } | 53 | } | ||
| @@ -47,8 +58,12 @@ func NewHoverHandler(ctx context.Context, paneID entity.PaneID) *HoverHandler { | |||||
| 47 | log.Debug().Str("pane_id", string(paneID)).Msg("creating hover handler") | 58 | log.Debug().Str("pane_id", string(paneID)).Msg("creating hover handler") | ||
| 48 | 59 | ||||
| 49 | return &HoverHandler{ | 60 | return &HoverHandler{ | ||
| 50 | - | ctx: ctx, | 61 | + | ctx: ctx, |
| 51 | - | paneID: paneID, | 62 | + | paneID: paneID, |
| 63 | + | schedule: scheduleHoverSource, | |||
| 64 | + | removeSource: glib.SourceRemove, | |||
| 65 | + | disconnectSignal: disconnectHoverSignal, | |||
| 66 | + | removeController: removeHoverController, | |||
| 52 | } | 67 | } | ||
| 53 | } | 68 | } | ||
| 54 | 69 | ||||
| @@ -73,6 +88,15 @@ func (h *HoverHandler) AttachTo(widget *gtk.Widget) { | |||||
| 73 | return | 88 | return | ||
| 74 | } | 89 | } | ||
| 75 | 90 | ||||
| 91 | + | // Reattachment must not leave a controller owned by the previous widget. | |||
| 92 | + | if h.motionCtrl != nil || h.attachedWidget != nil { | |||
| 93 | + | h.Detach() | |||
| 94 | + | } | |||
| 95 | + | h.timerMu.Lock() | |||
| 96 | + | h.detached = false | |||
| 97 | + | h.attachedWidget = widget | |||
| 98 | + | h.timerMu.Unlock() | |||
| 99 | + | ||||
| 76 | h.motionCtrl = gtk.NewEventControllerMotion() | 100 | h.motionCtrl = gtk.NewEventControllerMotion() | ||
| 77 | if h.motionCtrl == nil { | 101 | if h.motionCtrl == nil { | ||
| 78 | log.Error().Msg("failed to create motion controller") | 102 | log.Error().Msg("failed to create motion controller") | ||
| @@ -83,13 +107,13 @@ func (h *HoverHandler) AttachTo(widget *gtk.Widget) { | |||||
| 83 | h.enterCb = func(_ gtk.EventControllerMotion, _ float64, _ float64) { | 107 | h.enterCb = func(_ gtk.EventControllerMotion, _ float64, _ float64) { | ||
| 84 | h.handleEnter() | 108 | h.handleEnter() | ||
| 85 | } | 109 | } | ||
| 86 | - | h.motionCtrl.ConnectEnter(&h.enterCb) | 110 | + | h.enterSignalID = h.motionCtrl.ConnectEnter(&h.enterCb) |
| 87 | 111 | ||||
| 88 | // Connect leave handler to cancel pending focus | 112 | // Connect leave handler to cancel pending focus | ||
| 89 | h.leaveCb = func(_ gtk.EventControllerMotion) { | 113 | h.leaveCb = func(_ gtk.EventControllerMotion) { | ||
| 90 | h.handleLeave() | 114 | h.handleLeave() | ||
| 91 | } | 115 | } | ||
| 92 | - | h.motionCtrl.ConnectLeave(&h.leaveCb) | 116 | + | h.leaveSignalID = h.motionCtrl.ConnectLeave(&h.leaveCb) |
| 93 | 117 | ||||
| 94 | // Connect motion handler to detect intentional mouse movement. | 118 | // Connect motion handler to detect intentional mouse movement. | ||
| 95 | // Unlike enter/leave, motion only fires when the cursor physically moves | 119 | // Unlike enter/leave, motion only fires when the cursor physically moves | ||
| @@ -99,7 +123,7 @@ func (h *HoverHandler) AttachTo(widget *gtk.Widget) { | |||||
| 99 | h.onMotion() | 123 | h.onMotion() | ||
| 100 | } | 124 | } | ||
| 101 | } | 125 | } | ||
| 102 | - | h.motionCtrl.ConnectMotion(&h.motionCb) | 126 | + | h.motionSignalID = h.motionCtrl.ConnectMotion(&h.motionCb) |
| 103 | 127 | ||||
| 104 | // Add controller to widget | 128 | // Add controller to widget | ||
| 105 | widget.AddController(&h.motionCtrl.EventController) | 129 | widget.AddController(&h.motionCtrl.EventController) | ||
| @@ -107,32 +131,49 @@ func (h *HoverHandler) AttachTo(widget *gtk.Widget) { | |||||
| 107 | log.Debug().Str("pane_id", string(h.paneID)).Msg("hover handler attached to widget") | 131 | log.Debug().Str("pane_id", string(h.paneID)).Msg("hover handler attached to widget") | ||
| 108 | } | 132 | } | ||
| 109 | 133 | ||||
| 134 | + | // scheduleHoverSource schedules a one-shot callback on GTK's owning GLib main | |||
| 135 | + | // context. GTK state must never be touched by a Go timer goroutine. | |||
| 136 | + | func scheduleHoverSource(delay uint, fn func(uintptr) bool) uint { | |||
| 137 | + | cb := glib.SourceFunc(fn) | |||
| 138 | + | return glib.TimeoutAdd(delay, &cb, 0) | |||
| 139 | + | } | |||
| 140 | + | ||||
| 110 | // handleEnter processes the mouse enter event with debouncing. | 141 | // handleEnter processes the mouse enter event with debouncing. | ||
| 111 | func (h *HoverHandler) handleEnter() { | 142 | func (h *HoverHandler) handleEnter() { | ||
| 112 | h.timerMu.Lock() | 143 | h.timerMu.Lock() | ||
| 113 | - | defer h.timerMu.Unlock() | 144 | + | if h.detached { |
| 114 | - | 145 | + | h.timerMu.Unlock() | |
| 115 | - | // Cancel any existing timer | 146 | + | return |
| 116 | - | if h.timer != nil { | |||
| 117 | - | h.timer.Stop() | |||
| 118 | } | 147 | } | ||
| 119 | - | 148 | + | h.cancelSourceLocked() | |
| 120 | - | h.canceled = false | 149 | + | h.generation++ |
| 121 | - | 150 | + | generation := h.generation | |
| 122 | - | // Start new debounce timer | 151 | + | schedule := h.schedule |
| 123 | - | h.timer = time.AfterFunc(HoverFocusDelay, func() { | 152 | + | h.sourceID = schedule(uint(HoverFocusDelay.Milliseconds()), func(_ uintptr) bool { |
| 124 | h.timerMu.Lock() | 153 | h.timerMu.Lock() | ||
| 125 | - | canceled := h.canceled | 154 | + | if h.detached || h.generation != generation { |
| 126 | - | h.timerMu.Unlock() | 155 | + | h.timerMu.Unlock() |
| 127 | - | 156 | + | return false | |
| 128 | - | if canceled { | |||
| 129 | - | return | |||
| 130 | } | 157 | } | ||
| 158 | + | h.sourceID = 0 | |||
| 159 | + | callback := h.onEnter | |||
| 160 | + | paneID := h.paneID | |||
| 161 | + | h.timerMu.Unlock() | |||
| 131 | 162 | ||||
| 132 | - | if h.onEnter != nil { | 163 | + | if callback != nil { |
| 133 | - | h.onEnter(h.paneID) | 164 | + | callback(paneID) |
| 134 | } | 165 | } | ||
| 166 | + | return false | |||
| 135 | }) | 167 | }) | ||
| 168 | + | h.timerMu.Unlock() | |||
| 169 | + | } | |||
| 170 | + | ||||
| 171 | + | func (h *HoverHandler) cancelSourceLocked() { | |||
| 172 | + | if h.sourceID == 0 { | |||
| 173 | + | return | |||
| 174 | + | } | |||
| 175 | + | h.removeSource(h.sourceID) | |||
| 176 | + | h.sourceID = 0 | |||
| 136 | } | 177 | } | ||
| 137 | 178 | ||||
| 138 | // handleLeave cancels any pending focus switch. | 179 | // handleLeave cancels any pending focus switch. | ||
| @@ -146,24 +187,61 @@ func (h *HoverHandler) Cancel() { | |||||
| 146 | h.timerMu.Lock() | 187 | h.timerMu.Lock() | ||
| 147 | defer h.timerMu.Unlock() | 188 | defer h.timerMu.Unlock() | ||
| 148 | 189 | ||||
| 149 | - | h.canceled = true | 190 | + | h.generation++ |
| 150 | - | if h.timer != nil { | 191 | + | h.cancelSourceLocked() |
| 151 | - | h.timer.Stop() | |||
| 152 | - | h.timer = nil | |||
| 153 | - | } | |||
| 154 | } | 192 | } | ||
| 155 | 193 | ||||
| 156 | - | // Detach removes the hover handler. | 194 | + | // Detach removes the hover handler and invalidates callbacks already queued on |
| 195 | + | // the GLib main context. The generation check is required because removing a | |||
| 196 | + | // source can race with dispatch. | |||
| 157 | func (h *HoverHandler) Detach() { | 197 | func (h *HoverHandler) Detach() { | ||
| 158 | h.timerMu.Lock() | 198 | h.timerMu.Lock() | ||
| 159 | - | if h.timer != nil { | 199 | + | h.detached = true |
| 160 | - | h.timer.Stop() | 200 | + | h.generation++ |
| 161 | - | h.timer = nil | 201 | + | h.cancelSourceLocked() |
| 162 | - | } | 202 | + | h.onEnter = nil |
| 163 | - | h.timerMu.Unlock() | 203 | + | h.onMotion = nil |
| 164 | - | 204 | + | widget := h.attachedWidget | |
| 205 | + | controller := h.motionCtrl | |||
| 206 | + | enterID := h.enterSignalID | |||
| 207 | + | leaveID := h.leaveSignalID | |||
| 208 | + | motionID := h.motionSignalID | |||
| 209 | + | h.attachedWidget = nil | |||
| 165 | h.motionCtrl = nil | 210 | h.motionCtrl = nil | ||
| 211 | + | h.enterSignalID = 0 | |||
| 212 | + | h.leaveSignalID = 0 | |||
| 213 | + | h.motionSignalID = 0 | |||
| 166 | h.enterCb = nil | 214 | h.enterCb = nil | ||
| 167 | h.leaveCb = nil | 215 | h.leaveCb = nil | ||
| 168 | h.motionCb = nil | 216 | h.motionCb = nil | ||
| 217 | + | disconnectSignal := h.disconnectSignal | |||
| 218 | + | removeController := h.removeController | |||
| 219 | + | h.timerMu.Unlock() | |||
| 220 | + | ||||
| 221 | + | if controller == nil { | |||
| 222 | + | return | |||
| 223 | + | } | |||
| 224 | + | if disconnectSignal != nil { | |||
| 225 | + | for _, id := range []uint{enterID, leaveID, motionID} { | |||
| 226 | + | if id != 0 { | |||
| 227 | + | disconnectSignal(controller, id) | |||
| 228 | + | } | |||
| 229 | + | } | |||
| 230 | + | } | |||
| 231 | + | if widget != nil && removeController != nil { | |||
| 232 | + | removeController(widget, &controller.EventController) | |||
| 233 | + | } | |||
| 234 | + | } | |||
| 235 | + | ||||
| 236 | + | func disconnectHoverSignal(controller *gtk.EventControllerMotion, id uint) { | |||
| 237 | + | if controller == nil || id == 0 || controller.GoPointer() == 0 { | |||
| 238 | + | return | |||
| 239 | + | } | |||
| 240 | + | gobject.SignalHandlerDisconnect(gobject.ObjectNewFromInternalPtr(controller.GoPointer()), id) | |||
| 241 | + | } | |||
| 242 | + | ||||
| 243 | + | func removeHoverController(widget *gtk.Widget, controller *gtk.EventController) { | |||
| 244 | + | if widget != nil && controller != nil { | |||
| 245 | + | widget.RemoveController(controller) | |||
| 246 | + | } | |||
| 169 | } | 247 | } | ||
A internal/ui/input/hover_test.go +90 -0
| @@ -0,0 +1,90 @@ | |||
| 1 | + | package input | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "reflect" | |
| 6 | + | "testing" | |
| 7 | + | ||
| 8 | + | "github.com/bnema/dumber/internal/domain/entity" | |
| 9 | + | "github.com/bnema/puregotk/v4/gtk" | |
| 10 | + | ) | |
| 11 | + | ||
| 12 | + | func TestHoverHandlerSchedulesCallbackOnGLibAndRejectsStaleGeneration(t *testing.T) { | |
| 13 | + | h := NewHoverHandler(context.Background(), entity.PaneID("pane-1")) | |
| 14 | + | var scheduled func(uintptr) bool | |
| 15 | + | h.schedule = func(_ uint, fn func(uintptr) bool) uint { | |
| 16 | + | scheduled = fn | |
| 17 | + | return 7 | |
| 18 | + | } | |
| 19 | + | ||
| 20 | + | var calls int | |
| 21 | + | h.SetOnEnter(func(entity.PaneID) { calls++ }) | |
| 22 | + | h.handleEnter() | |
| 23 | + | first := scheduled | |
| 24 | + | h.handleEnter() | |
| 25 | + | if first(0) { | |
| 26 | + | t.Fatal("hover source must be one-shot") | |
| 27 | + | } | |
| 28 | + | if calls != 0 { | |
| 29 | + | t.Fatalf("stale hover callback ran %d times", calls) | |
| 30 | + | } | |
| 31 | + | if scheduled == nil { | |
| 32 | + | t.Fatal("current hover callback was not scheduled") | |
| 33 | + | } | |
| 34 | + | if scheduled(0) { | |
| 35 | + | t.Fatal("hover source must be one-shot") | |
| 36 | + | } | |
| 37 | + | if calls != 1 { | |
| 38 | + | t.Fatalf("current hover callback ran %d times, want 1", calls) | |
| 39 | + | } | |
| 40 | + | } | |
| 41 | + | ||
| 42 | + | func TestHoverHandlerDetachRemovesNativeControllerAndSignalsExactlyOnce(t *testing.T) { | |
| 43 | + | widget := >k.Widget{} | |
| 44 | + | controller := >k.EventControllerMotion{} | |
| 45 | + | h := NewHoverHandler(context.Background(), entity.PaneID("pane-1")) | |
| 46 | + | h.attachedWidget = widget | |
| 47 | + | h.motionCtrl = controller | |
| 48 | + | h.enterSignalID = 1 | |
| 49 | + | h.leaveSignalID = 2 | |
| 50 | + | h.motionSignalID = 3 | |
| 51 | + | var disconnected []uint | |
| 52 | + | var removed int | |
| 53 | + | h.disconnectSignal = func(_ *gtk.EventControllerMotion, id uint) { disconnected = append(disconnected, id) } | |
| 54 | + | h.removeController = func(gotWidget *gtk.Widget, gotController *gtk.EventController) { | |
| 55 | + | if gotWidget != widget || gotController != &controller.EventController { | |
| 56 | + | t.Fatal("detach removed the wrong native controller") | |
| 57 | + | } | |
| 58 | + | removed++ | |
| 59 | + | } | |
| 60 | + | ||
| 61 | + | h.Detach() | |
| 62 | + | h.Detach() | |
| 63 | + | ||
| 64 | + | if got, want := disconnected, []uint{1, 2, 3}; !reflect.DeepEqual(got, want) { | |
| 65 | + | t.Fatalf("disconnected signals = %v, want %v", got, want) | |
| 66 | + | } | |
| 67 | + | if removed != 1 { | |
| 68 | + | t.Fatalf("native controllers removed = %d, want 1", removed) | |
| 69 | + | } | |
| 70 | + | if h.attachedWidget != nil || h.motionCtrl != nil || h.enterCb != nil || h.leaveCb != nil || h.motionCb != nil { | |
| 71 | + | t.Fatal("detach must clear native and callback ownership") | |
| 72 | + | } | |
| 73 | + | } | |
| 74 | + | ||
| 75 | + | func TestHoverHandlerDetachInvalidatesScheduledCallbackAndRemovesSource(t *testing.T) { | |
| 76 | + | h := NewHoverHandler(context.Background(), entity.PaneID("pane-1")) | |
| 77 | + | var scheduled func(uintptr) bool | |
| 78 | + | var removed []uint | |
| 79 | + | h.schedule = func(_ uint, fn func(uintptr) bool) uint { scheduled = fn; return 9 } | |
| 80 | + | h.removeSource = func(id uint) bool { removed = append(removed, id); return true } | |
| 81 | + | h.SetOnEnter(func(entity.PaneID) { t.Fatal("detached hover callback ran") }) | |
| 82 | + | h.handleEnter() | |
| 83 | + | h.Detach() | |
| 84 | + | if len(removed) != 1 || removed[0] != 9 { | |
| 85 | + | t.Fatalf("removed sources = %v, want [9]", removed) | |
| 86 | + | } | |
| 87 | + | if scheduled == nil || scheduled(0) { | |
| 88 | + | t.Fatal("detached callback must be rejected and one-shot") | |
| 89 | + | } | |
| 90 | + | } | |
| @@ -0,0 +1,90 @@ | |||||
| 1 | + | package input | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "reflect" | |||
| 6 | + | "testing" | |||
| 7 | + | ||||
| 8 | + | "github.com/bnema/dumber/internal/domain/entity" | |||
| 9 | + | "github.com/bnema/puregotk/v4/gtk" | |||
| 10 | + | ) | |||
| 11 | + | ||||
| 12 | + | func TestHoverHandlerSchedulesCallbackOnGLibAndRejectsStaleGeneration(t *testing.T) { | |||
| 13 | + | h := NewHoverHandler(context.Background(), entity.PaneID("pane-1")) | |||
| 14 | + | var scheduled func(uintptr) bool | |||
| 15 | + | h.schedule = func(_ uint, fn func(uintptr) bool) uint { | |||
| 16 | + | scheduled = fn | |||
| 17 | + | return 7 | |||
| 18 | + | } | |||
| 19 | + | ||||
| 20 | + | var calls int | |||
| 21 | + | h.SetOnEnter(func(entity.PaneID) { calls++ }) | |||
| 22 | + | h.handleEnter() | |||
| 23 | + | first := scheduled | |||
| 24 | + | h.handleEnter() | |||
| 25 | + | if first(0) { | |||
| 26 | + | t.Fatal("hover source must be one-shot") | |||
| 27 | + | } | |||
| 28 | + | if calls != 0 { | |||
| 29 | + | t.Fatalf("stale hover callback ran %d times", calls) | |||
| 30 | + | } | |||
| 31 | + | if scheduled == nil { | |||
| 32 | + | t.Fatal("current hover callback was not scheduled") | |||
| 33 | + | } | |||
| 34 | + | if scheduled(0) { | |||
| 35 | + | t.Fatal("hover source must be one-shot") | |||
| 36 | + | } | |||
| 37 | + | if calls != 1 { | |||
| 38 | + | t.Fatalf("current hover callback ran %d times, want 1", calls) | |||
| 39 | + | } | |||
| 40 | + | } | |||
| 41 | + | ||||
| 42 | + | func TestHoverHandlerDetachRemovesNativeControllerAndSignalsExactlyOnce(t *testing.T) { | |||
| 43 | + | widget := >k.Widget{} | |||
| 44 | + | controller := >k.EventControllerMotion{} | |||
| 45 | + | h := NewHoverHandler(context.Background(), entity.PaneID("pane-1")) | |||
| 46 | + | h.attachedWidget = widget | |||
| 47 | + | h.motionCtrl = controller | |||
| 48 | + | h.enterSignalID = 1 | |||
| 49 | + | h.leaveSignalID = 2 | |||
| 50 | + | h.motionSignalID = 3 | |||
| 51 | + | var disconnected []uint | |||
| 52 | + | var removed int | |||
| 53 | + | h.disconnectSignal = func(_ *gtk.EventControllerMotion, id uint) { disconnected = append(disconnected, id) } | |||
| 54 | + | h.removeController = func(gotWidget *gtk.Widget, gotController *gtk.EventController) { | |||
| 55 | + | if gotWidget != widget || gotController != &controller.EventController { | |||
| 56 | + | t.Fatal("detach removed the wrong native controller") | |||
| 57 | + | } | |||
| 58 | + | removed++ | |||
| 59 | + | } | |||
| 60 | + | ||||
| 61 | + | h.Detach() | |||
| 62 | + | h.Detach() | |||
| 63 | + | ||||
| 64 | + | if got, want := disconnected, []uint{1, 2, 3}; !reflect.DeepEqual(got, want) { | |||
| 65 | + | t.Fatalf("disconnected signals = %v, want %v", got, want) | |||
| 66 | + | } | |||
| 67 | + | if removed != 1 { | |||
| 68 | + | t.Fatalf("native controllers removed = %d, want 1", removed) | |||
| 69 | + | } | |||
| 70 | + | if h.attachedWidget != nil || h.motionCtrl != nil || h.enterCb != nil || h.leaveCb != nil || h.motionCb != nil { | |||
| 71 | + | t.Fatal("detach must clear native and callback ownership") | |||
| 72 | + | } | |||
| 73 | + | } | |||
| 74 | + | ||||
| 75 | + | func TestHoverHandlerDetachInvalidatesScheduledCallbackAndRemovesSource(t *testing.T) { | |||
| 76 | + | h := NewHoverHandler(context.Background(), entity.PaneID("pane-1")) | |||
| 77 | + | var scheduled func(uintptr) bool | |||
| 78 | + | var removed []uint | |||
| 79 | + | h.schedule = func(_ uint, fn func(uintptr) bool) uint { scheduled = fn; return 9 } | |||
| 80 | + | h.removeSource = func(id uint) bool { removed = append(removed, id); return true } | |||
| 81 | + | h.SetOnEnter(func(entity.PaneID) { t.Fatal("detached hover callback ran") }) | |||
| 82 | + | h.handleEnter() | |||
| 83 | + | h.Detach() | |||
| 84 | + | if len(removed) != 1 || removed[0] != 9 { | |||
| 85 | + | t.Fatalf("removed sources = %v, want [9]", removed) | |||
| 86 | + | } | |||
| 87 | + | if scheduled == nil || scheduled(0) { | |||
| 88 | + | t.Fatal("detached callback must be rejected and one-shot") | |||
| 89 | + | } | |||
| 90 | + | } | |||
M internal/ui/layout/gtk_widgets.go +49 -5
| @@ -1,7 +1,10 @@ | |||
| 1 | 1 | package layout | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "sync" | |
| 5 | + | ||
| 4 | 6 | "github.com/bnema/puregotk/v4/gdk" | |
| 7 | + | "github.com/bnema/puregotk/v4/glib" | |
| 5 | 8 | "github.com/bnema/puregotk/v4/gobject" | |
| 6 | 9 | "github.com/bnema/puregotk/v4/graphene" | |
| 7 | 10 | "github.com/bnema/puregotk/v4/gtk" | |
| @@ -82,8 +85,14 @@ func (w *gtkWidget) ComputePoint(target Widget) (x, y float64, ok bool) { | |||
| 82 | 85 | // gtkPaned wraps gtk.Paned to implement PanedWidget. | |
| 83 | 86 | type gtkPaned struct { | |
| 84 | 87 | inner *gtk.Paned | |
| 88 | + | ||
| 89 | + | tickMu sync.Mutex | |
| 90 | + | tickCallbacks map[uint]*gtk.TickCallback | |
| 85 | 91 | } | |
| 86 | 92 | ||
| 93 | + | // unrefTickCallback is the canonical puregotk/purego callback-slot release. | |
| 94 | + | var unrefTickCallback = glib.UnrefCallback | |
| 95 | + | ||
| 87 | 96 | func (p *gtkPaned) Show() { p.inner.Show() } | |
| 88 | 97 | func (p *gtkPaned) Hide() { p.inner.Hide() } | |
| 89 | 98 | func (p *gtkPaned) SetVisible(visible bool) { p.inner.SetVisible(visible) } | |
| @@ -176,10 +185,45 @@ func (p *gtkPaned) ConnectNotifyPosition(callback func()) uint { | |||
| 176 | 185 | } | |
| 177 | 186 | ||
| 178 | 187 | func (p *gtkPaned) AddTickCallback(callback func() bool) uint { | |
| 179 | - | cb := gtk.TickCallback(func(widget uintptr, frameClock uintptr, userData uintptr) bool { | |
| 180 | - | return callback() | |
| 181 | - | }) | |
| 182 | - | return p.inner.AddTickCallback(&cb, 0, nil) | |
| 188 | + | cb := new(gtk.TickCallback) | |
| 189 | + | var id uint | |
| 190 | + | *cb = func(_ uintptr, _ uintptr, _ uintptr) bool { | |
| 191 | + | keepRunning := callback() | |
| 192 | + | if !keepRunning { | |
| 193 | + | // GTK removes a false-returning source itself; release its matching | |
| 194 | + | // purego slot exactly once rather than merely dropping the Go pointer. | |
| 195 | + | p.releaseTickCallback(id) | |
| 196 | + | } | |
| 197 | + | return keepRunning | |
| 198 | + | } | |
| 199 | + | id = p.inner.AddTickCallback(cb, 0, nil) | |
| 200 | + | if id != 0 { | |
| 201 | + | p.tickMu.Lock() | |
| 202 | + | if p.tickCallbacks == nil { | |
| 203 | + | p.tickCallbacks = make(map[uint]*gtk.TickCallback) | |
| 204 | + | } | |
| 205 | + | p.tickCallbacks[id] = cb | |
| 206 | + | p.tickMu.Unlock() | |
| 207 | + | } | |
| 208 | + | return id | |
| 209 | + | } | |
| 210 | + | ||
| 211 | + | func (p *gtkPaned) RemoveTickCallback(id uint) { | |
| 212 | + | p.inner.RemoveTickCallback(id) | |
| 213 | + | p.releaseTickCallback(id) | |
| 214 | + | } | |
| 215 | + | ||
| 216 | + | func (p *gtkPaned) releaseTickCallback(id uint) { | |
| 217 | + | if id == 0 { | |
| 218 | + | return | |
| 219 | + | } | |
| 220 | + | p.tickMu.Lock() | |
| 221 | + | callback := p.tickCallbacks[id] | |
| 222 | + | delete(p.tickCallbacks, id) | |
| 223 | + | p.tickMu.Unlock() | |
| 224 | + | if callback != nil { | |
| 225 | + | _ = unrefTickCallback(callback) | |
| 226 | + | } | |
| 183 | 227 | } | |
| 184 | 228 | ||
| 185 | 229 | func (p *gtkPaned) GetAllocatedWidth() int { return p.inner.GetAllocatedWidth() } | |
| @@ -779,7 +823,7 @@ func (f *GtkWidgetFactory) NewPaned(orientation Orientation) PanedWidget { | |||
| 779 | 823 | paned := gtk.NewPaned(orientation) | |
| 780 | 824 | paned.SetHexpand(true) | |
| 781 | 825 | paned.SetVexpand(true) | |
| 782 | - | return >kPaned{inner: paned} | |
| 826 | + | return >kPaned{inner: paned, tickCallbacks: make(map[uint]*gtk.TickCallback)} | |
| 783 | 827 | } | |
| 784 | 828 | ||
| 785 | 829 | func (f *GtkWidgetFactory) NewBox(orientation Orientation, spacing int) BoxWidget { | |
| @@ -1,7 +1,10 @@ | |||||
| 1 | package layout | 1 | package layout | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "sync" | |||
| 5 | + | ||||
| 4 | "github.com/bnema/puregotk/v4/gdk" | 6 | "github.com/bnema/puregotk/v4/gdk" | ||
| 7 | + | "github.com/bnema/puregotk/v4/glib" | |||
| 5 | "github.com/bnema/puregotk/v4/gobject" | 8 | "github.com/bnema/puregotk/v4/gobject" | ||
| 6 | "github.com/bnema/puregotk/v4/graphene" | 9 | "github.com/bnema/puregotk/v4/graphene" | ||
| 7 | "github.com/bnema/puregotk/v4/gtk" | 10 | "github.com/bnema/puregotk/v4/gtk" | ||
| @@ -82,8 +85,14 @@ func (w *gtkWidget) ComputePoint(target Widget) (x, y float64, ok bool) { | |||||
| 82 | // gtkPaned wraps gtk.Paned to implement PanedWidget. | 85 | // gtkPaned wraps gtk.Paned to implement PanedWidget. | ||
| 83 | type gtkPaned struct { | 86 | type gtkPaned struct { | ||
| 84 | inner *gtk.Paned | 87 | inner *gtk.Paned | ||
| 88 | + | ||||
| 89 | + | tickMu sync.Mutex | |||
| 90 | + | tickCallbacks map[uint]*gtk.TickCallback | |||
| 85 | } | 91 | } | ||
| 86 | 92 | ||||
| 93 | + | // unrefTickCallback is the canonical puregotk/purego callback-slot release. | |||
| 94 | + | var unrefTickCallback = glib.UnrefCallback | |||
| 95 | + | ||||
| 87 | func (p *gtkPaned) Show() { p.inner.Show() } | 96 | func (p *gtkPaned) Show() { p.inner.Show() } | ||
| 88 | func (p *gtkPaned) Hide() { p.inner.Hide() } | 97 | func (p *gtkPaned) Hide() { p.inner.Hide() } | ||
| 89 | func (p *gtkPaned) SetVisible(visible bool) { p.inner.SetVisible(visible) } | 98 | func (p *gtkPaned) SetVisible(visible bool) { p.inner.SetVisible(visible) } | ||
| @@ -176,10 +185,45 @@ func (p *gtkPaned) ConnectNotifyPosition(callback func()) uint { | |||||
| 176 | } | 185 | } | ||
| 177 | 186 | ||||
| 178 | func (p *gtkPaned) AddTickCallback(callback func() bool) uint { | 187 | func (p *gtkPaned) AddTickCallback(callback func() bool) uint { | ||
| 179 | - | cb := gtk.TickCallback(func(widget uintptr, frameClock uintptr, userData uintptr) bool { | 188 | + | cb := new(gtk.TickCallback) |
| 180 | - | return callback() | 189 | + | var id uint |
| 181 | - | }) | 190 | + | *cb = func(_ uintptr, _ uintptr, _ uintptr) bool { |
| 182 | - | return p.inner.AddTickCallback(&cb, 0, nil) | 191 | + | keepRunning := callback() |
| 192 | + | if !keepRunning { | |||
| 193 | + | // GTK removes a false-returning source itself; release its matching | |||
| 194 | + | // purego slot exactly once rather than merely dropping the Go pointer. | |||
| 195 | + | p.releaseTickCallback(id) | |||
| 196 | + | } | |||
| 197 | + | return keepRunning | |||
| 198 | + | } | |||
| 199 | + | id = p.inner.AddTickCallback(cb, 0, nil) | |||
| 200 | + | if id != 0 { | |||
| 201 | + | p.tickMu.Lock() | |||
| 202 | + | if p.tickCallbacks == nil { | |||
| 203 | + | p.tickCallbacks = make(map[uint]*gtk.TickCallback) | |||
| 204 | + | } | |||
| 205 | + | p.tickCallbacks[id] = cb | |||
| 206 | + | p.tickMu.Unlock() | |||
| 207 | + | } | |||
| 208 | + | return id | |||
| 209 | + | } | |||
| 210 | + | ||||
| 211 | + | func (p *gtkPaned) RemoveTickCallback(id uint) { | |||
| 212 | + | p.inner.RemoveTickCallback(id) | |||
| 213 | + | p.releaseTickCallback(id) | |||
| 214 | + | } | |||
| 215 | + | ||||
| 216 | + | func (p *gtkPaned) releaseTickCallback(id uint) { | |||
| 217 | + | if id == 0 { | |||
| 218 | + | return | |||
| 219 | + | } | |||
| 220 | + | p.tickMu.Lock() | |||
| 221 | + | callback := p.tickCallbacks[id] | |||
| 222 | + | delete(p.tickCallbacks, id) | |||
| 223 | + | p.tickMu.Unlock() | |||
| 224 | + | if callback != nil { | |||
| 225 | + | _ = unrefTickCallback(callback) | |||
| 226 | + | } | |||
| 183 | } | 227 | } | ||
| 184 | 228 | ||||
| 185 | func (p *gtkPaned) GetAllocatedWidth() int { return p.inner.GetAllocatedWidth() } | 229 | func (p *gtkPaned) GetAllocatedWidth() int { return p.inner.GetAllocatedWidth() } | ||
| @@ -779,7 +823,7 @@ func (f *GtkWidgetFactory) NewPaned(orientation Orientation) PanedWidget { | |||||
| 779 | paned := gtk.NewPaned(orientation) | 823 | paned := gtk.NewPaned(orientation) | ||
| 780 | paned.SetHexpand(true) | 824 | paned.SetHexpand(true) | ||
| 781 | paned.SetVexpand(true) | 825 | paned.SetVexpand(true) | ||
| 782 | - | return >kPaned{inner: paned} | 826 | + | return >kPaned{inner: paned, tickCallbacks: make(map[uint]*gtk.TickCallback)} |
| 783 | } | 827 | } | ||
| 784 | 828 | ||||
| 785 | func (f *GtkWidgetFactory) NewBox(orientation Orientation, spacing int) BoxWidget { | 829 | func (f *GtkWidgetFactory) NewBox(orientation Orientation, spacing int) BoxWidget { | ||
A internal/ui/layout/gtk_widgets_tick_cleanup_test.go +29 -0
| @@ -0,0 +1,29 @@ | |||
| 1 | + | package layout | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "testing" | |
| 5 | + | ||
| 6 | + | "github.com/bnema/puregotk/v4/gtk" | |
| 7 | + | ) | |
| 8 | + | ||
| 9 | + | func TestGtkPanedReleaseTickCallbackUnrefsRawSlotExactlyOnce(t *testing.T) { | |
| 10 | + | callback := new(gtk.TickCallback) | |
| 11 | + | paned := >kPaned{tickCallbacks: map[uint]*gtk.TickCallback{7: callback}} | |
| 12 | + | oldUnref := unrefTickCallback | |
| 13 | + | defer func() { unrefTickCallback = oldUnref }() | |
| 14 | + | var unrefs int | |
| 15 | + | unrefTickCallback = func(got any) error { | |
| 16 | + | if got != callback { | |
| 17 | + | t.Fatalf("unref callback = %p, want %p", got, callback) | |
| 18 | + | } | |
| 19 | + | unrefs++ | |
| 20 | + | return nil | |
| 21 | + | } | |
| 22 | + | ||
| 23 | + | paned.releaseTickCallback(7) | |
| 24 | + | paned.releaseTickCallback(7) | |
| 25 | + | ||
| 26 | + | if unrefs != 1 { | |
| 27 | + | t.Fatalf("callback unrefs = %d, want 1", unrefs) | |
| 28 | + | } | |
| 29 | + | } | |
| @@ -0,0 +1,29 @@ | |||||
| 1 | + | package layout | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "testing" | |||
| 5 | + | ||||
| 6 | + | "github.com/bnema/puregotk/v4/gtk" | |||
| 7 | + | ) | |||
| 8 | + | ||||
| 9 | + | func TestGtkPanedReleaseTickCallbackUnrefsRawSlotExactlyOnce(t *testing.T) { | |||
| 10 | + | callback := new(gtk.TickCallback) | |||
| 11 | + | paned := >kPaned{tickCallbacks: map[uint]*gtk.TickCallback{7: callback}} | |||
| 12 | + | oldUnref := unrefTickCallback | |||
| 13 | + | defer func() { unrefTickCallback = oldUnref }() | |||
| 14 | + | var unrefs int | |||
| 15 | + | unrefTickCallback = func(got any) error { | |||
| 16 | + | if got != callback { | |||
| 17 | + | t.Fatalf("unref callback = %p, want %p", got, callback) | |||
| 18 | + | } | |||
| 19 | + | unrefs++ | |||
| 20 | + | return nil | |||
| 21 | + | } | |||
| 22 | + | ||||
| 23 | + | paned.releaseTickCallback(7) | |||
| 24 | + | paned.releaseTickCallback(7) | |||
| 25 | + | ||||
| 26 | + | if unrefs != 1 { | |||
| 27 | + | t.Fatalf("callback unrefs = %d, want 1", unrefs) | |||
| 28 | + | } | |||
| 29 | + | } | |||
M internal/ui/layout/mocks/widgets.go +40 -0
| @@ -2425,6 +2425,46 @@ func (_c *MockPanedWidget_RemoveCssClass_Call) RunAndReturn(run func(cssClass st | |||
| 2425 | 2425 | return _c | |
| 2426 | 2426 | } | |
| 2427 | 2427 | ||
| 2428 | + | // RemoveTickCallback provides a mock function for the type MockPanedWidget | |
| 2429 | + | func (_mock *MockPanedWidget) RemoveTickCallback(id uint) { | |
| 2430 | + | _mock.Called(id) | |
| 2431 | + | return | |
| 2432 | + | } | |
| 2433 | + | ||
| 2434 | + | // MockPanedWidget_RemoveTickCallback_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'RemoveTickCallback' | |
| 2435 | + | type MockPanedWidget_RemoveTickCallback_Call struct { | |
| 2436 | + | *mock.Call | |
| 2437 | + | } | |
| 2438 | + | ||
| 2439 | + | // RemoveTickCallback is a helper method to define mock.On call | |
| 2440 | + | // - id uint | |
| 2441 | + | func (_e *MockPanedWidget_Expecter) RemoveTickCallback(id any) *MockPanedWidget_RemoveTickCallback_Call { | |
| 2442 | + | return &MockPanedWidget_RemoveTickCallback_Call{Call: _e.mock.On("RemoveTickCallback", id)} | |
| 2443 | + | } | |
| 2444 | + | ||
| 2445 | + | func (_c *MockPanedWidget_RemoveTickCallback_Call) Run(run func(id uint)) *MockPanedWidget_RemoveTickCallback_Call { | |
| 2446 | + | _c.Call.Run(func(args mock.Arguments) { | |
| 2447 | + | var arg0 uint | |
| 2448 | + | if args[0] != nil { | |
| 2449 | + | arg0 = args[0].(uint) | |
| 2450 | + | } | |
| 2451 | + | run( | |
| 2452 | + | arg0, | |
| 2453 | + | ) | |
| 2454 | + | }) | |
| 2455 | + | return _c | |
| 2456 | + | } | |
| 2457 | + | ||
| 2458 | + | func (_c *MockPanedWidget_RemoveTickCallback_Call) Return() *MockPanedWidget_RemoveTickCallback_Call { | |
| 2459 | + | _c.Call.Return() | |
| 2460 | + | return _c | |
| 2461 | + | } | |
| 2462 | + | ||
| 2463 | + | func (_c *MockPanedWidget_RemoveTickCallback_Call) RunAndReturn(run func(id uint)) *MockPanedWidget_RemoveTickCallback_Call { | |
| 2464 | + | _c.Run(run) | |
| 2465 | + | return _c | |
| 2466 | + | } | |
| 2467 | + | ||
| 2428 | 2468 | // SetCanFocus provides a mock function for the type MockPanedWidget | |
| 2429 | 2469 | func (_mock *MockPanedWidget) SetCanFocus(canFocus bool) { | |
| 2430 | 2470 | _mock.Called(canFocus) | |
| @@ -2425,6 +2425,46 @@ func (_c *MockPanedWidget_RemoveCssClass_Call) RunAndReturn(run func(cssClass st | |||||
| 2425 | return _c | 2425 | return _c | ||
| 2426 | } | 2426 | } | ||
| 2427 | 2427 | ||||
| 2428 | + | // RemoveTickCallback provides a mock function for the type MockPanedWidget | |||
| 2429 | + | func (_mock *MockPanedWidget) RemoveTickCallback(id uint) { | |||
| 2430 | + | _mock.Called(id) | |||
| 2431 | + | return | |||
| 2432 | + | } | |||
| 2433 | + | ||||
| 2434 | + | // MockPanedWidget_RemoveTickCallback_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'RemoveTickCallback' | |||
| 2435 | + | type MockPanedWidget_RemoveTickCallback_Call struct { | |||
| 2436 | + | *mock.Call | |||
| 2437 | + | } | |||
| 2438 | + | ||||
| 2439 | + | // RemoveTickCallback is a helper method to define mock.On call | |||
| 2440 | + | // - id uint | |||
| 2441 | + | func (_e *MockPanedWidget_Expecter) RemoveTickCallback(id any) *MockPanedWidget_RemoveTickCallback_Call { | |||
| 2442 | + | return &MockPanedWidget_RemoveTickCallback_Call{Call: _e.mock.On("RemoveTickCallback", id)} | |||
| 2443 | + | } | |||
| 2444 | + | ||||
| 2445 | + | func (_c *MockPanedWidget_RemoveTickCallback_Call) Run(run func(id uint)) *MockPanedWidget_RemoveTickCallback_Call { | |||
| 2446 | + | _c.Call.Run(func(args mock.Arguments) { | |||
| 2447 | + | var arg0 uint | |||
| 2448 | + | if args[0] != nil { | |||
| 2449 | + | arg0 = args[0].(uint) | |||
| 2450 | + | } | |||
| 2451 | + | run( | |||
| 2452 | + | arg0, | |||
| 2453 | + | ) | |||
| 2454 | + | }) | |||
| 2455 | + | return _c | |||
| 2456 | + | } | |||
| 2457 | + | ||||
| 2458 | + | func (_c *MockPanedWidget_RemoveTickCallback_Call) Return() *MockPanedWidget_RemoveTickCallback_Call { | |||
| 2459 | + | _c.Call.Return() | |||
| 2460 | + | return _c | |||
| 2461 | + | } | |||
| 2462 | + | ||||
| 2463 | + | func (_c *MockPanedWidget_RemoveTickCallback_Call) RunAndReturn(run func(id uint)) *MockPanedWidget_RemoveTickCallback_Call { | |||
| 2464 | + | _c.Run(run) | |||
| 2465 | + | return _c | |||
| 2466 | + | } | |||
| 2467 | + | ||||
| 2428 | // SetCanFocus provides a mock function for the type MockPanedWidget | 2468 | // SetCanFocus provides a mock function for the type MockPanedWidget | ||
| 2429 | func (_mock *MockPanedWidget) SetCanFocus(canFocus bool) { | 2469 | func (_mock *MockPanedWidget) SetCanFocus(canFocus bool) { | ||
| 2430 | _mock.Called(canFocus) | 2470 | _mock.Called(canFocus) | ||
M internal/ui/layout/split.go +22 -1
| @@ -23,6 +23,8 @@ type SplitView struct { | |||
| 23 | 23 | onRatioChanged func(ratio float64) | |
| 24 | 24 | pendingNotifyRatio float64 | |
| 25 | 25 | notifyDebounceTimer *time.Timer | |
| 26 | + | tickCallbackID uint | |
| 27 | + | cleanedUp bool | |
| 26 | 28 | ||
| 27 | 29 | hasAppliedRatio bool | |
| 28 | 30 | suppressNotifyUntil time.Time | |
| @@ -188,7 +190,7 @@ func NewSplitView( | |||
| 188 | 190 | // Add tick callback to retry applying ratio every frame until successful. | |
| 189 | 191 | // This handles cases where allocation isn't ready even after Map signal. | |
| 190 | 192 | frames := 0 | |
| 191 | - | paned.AddTickCallback(func() bool { | |
| 193 | + | sv.tickCallbackID = paned.AddTickCallback(func() bool { | |
| 192 | 194 | frames++ | |
| 193 | 195 | if sv.ApplyRatio() { | |
| 194 | 196 | sv.logger.Debug(). | |
| @@ -213,6 +215,25 @@ func NewSplitView( | |||
| 213 | 215 | // The ratio is clamped to the range [0.0, 1.0]. | |
| 214 | 216 | // Note: This sets the position based on ratio; actual pixel position | |
| 215 | 217 | // depends on the allocated size of the paned widget. | |
| 218 | + | // Cleanup releases callbacks owned by this split. It is idempotent because | |
| 219 | + | // tree rebuild and GTK teardown can both reach the same view. | |
| 220 | + | func (sv *SplitView) Cleanup() { | |
| 221 | + | sv.mu.Lock() | |
| 222 | + | defer sv.mu.Unlock() | |
| 223 | + | if sv.cleanedUp { | |
| 224 | + | return | |
| 225 | + | } | |
| 226 | + | sv.cleanedUp = true | |
| 227 | + | if sv.notifyDebounceTimer != nil { | |
| 228 | + | sv.notifyDebounceTimer.Stop() | |
| 229 | + | sv.notifyDebounceTimer = nil | |
| 230 | + | } | |
| 231 | + | if sv.tickCallbackID != 0 { | |
| 232 | + | sv.paned.RemoveTickCallback(sv.tickCallbackID) | |
| 233 | + | sv.tickCallbackID = 0 | |
| 234 | + | } | |
| 235 | + | } | |
| 236 | + | ||
| 216 | 237 | func (sv *SplitView) SetRatio(ratio float64) { | |
| 217 | 238 | sv.mu.Lock() | |
| 218 | 239 | defer sv.mu.Unlock() | |
| @@ -23,6 +23,8 @@ type SplitView struct { | |||||
| 23 | onRatioChanged func(ratio float64) | 23 | onRatioChanged func(ratio float64) | ||
| 24 | pendingNotifyRatio float64 | 24 | pendingNotifyRatio float64 | ||
| 25 | notifyDebounceTimer *time.Timer | 25 | notifyDebounceTimer *time.Timer | ||
| 26 | + | tickCallbackID uint | |||
| 27 | + | cleanedUp bool | |||
| 26 | 28 | ||||
| 27 | hasAppliedRatio bool | 29 | hasAppliedRatio bool | ||
| 28 | suppressNotifyUntil time.Time | 30 | suppressNotifyUntil time.Time | ||
| @@ -188,7 +190,7 @@ func NewSplitView( | |||||
| 188 | // Add tick callback to retry applying ratio every frame until successful. | 190 | // Add tick callback to retry applying ratio every frame until successful. | ||
| 189 | // This handles cases where allocation isn't ready even after Map signal. | 191 | // This handles cases where allocation isn't ready even after Map signal. | ||
| 190 | frames := 0 | 192 | frames := 0 | ||
| 191 | - | paned.AddTickCallback(func() bool { | 193 | + | sv.tickCallbackID = paned.AddTickCallback(func() bool { |
| 192 | frames++ | 194 | frames++ | ||
| 193 | if sv.ApplyRatio() { | 195 | if sv.ApplyRatio() { | ||
| 194 | sv.logger.Debug(). | 196 | sv.logger.Debug(). | ||
| @@ -213,6 +215,25 @@ func NewSplitView( | |||||
| 213 | // The ratio is clamped to the range [0.0, 1.0]. | 215 | // The ratio is clamped to the range [0.0, 1.0]. | ||
| 214 | // Note: This sets the position based on ratio; actual pixel position | 216 | // Note: This sets the position based on ratio; actual pixel position | ||
| 215 | // depends on the allocated size of the paned widget. | 217 | // depends on the allocated size of the paned widget. | ||
| 218 | + | // Cleanup releases callbacks owned by this split. It is idempotent because | |||
| 219 | + | // tree rebuild and GTK teardown can both reach the same view. | |||
| 220 | + | func (sv *SplitView) Cleanup() { | |||
| 221 | + | sv.mu.Lock() | |||
| 222 | + | defer sv.mu.Unlock() | |||
| 223 | + | if sv.cleanedUp { | |||
| 224 | + | return | |||
| 225 | + | } | |||
| 226 | + | sv.cleanedUp = true | |||
| 227 | + | if sv.notifyDebounceTimer != nil { | |||
| 228 | + | sv.notifyDebounceTimer.Stop() | |||
| 229 | + | sv.notifyDebounceTimer = nil | |||
| 230 | + | } | |||
| 231 | + | if sv.tickCallbackID != 0 { | |||
| 232 | + | sv.paned.RemoveTickCallback(sv.tickCallbackID) | |||
| 233 | + | sv.tickCallbackID = 0 | |||
| 234 | + | } | |||
| 235 | + | } | |||
| 236 | + | ||||
| 216 | func (sv *SplitView) SetRatio(ratio float64) { | 237 | func (sv *SplitView) SetRatio(ratio float64) { | ||
| 217 | sv.mu.Lock() | 238 | sv.mu.Lock() | ||
| 218 | defer sv.mu.Unlock() | 239 | defer sv.mu.Unlock() | ||
A internal/ui/layout/split_cleanup_test.go +29 -0
| @@ -0,0 +1,29 @@ | |||
| 1 | + | package layout_test | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "testing" | |
| 6 | + | ||
| 7 | + | "github.com/stretchr/testify/mock" | |
| 8 | + | ||
| 9 | + | "github.com/bnema/dumber/internal/ui/layout" | |
| 10 | + | "github.com/bnema/dumber/internal/ui/layout/mocks" | |
| 11 | + | ) | |
| 12 | + | ||
| 13 | + | func TestSplitViewCleanupReleasesTickCallbackExactlyOnce(t *testing.T) { | |
| 14 | + | factory := mocks.NewMockWidgetFactory(t) | |
| 15 | + | paned := mocks.NewMockPanedWidget(t) | |
| 16 | + | factory.EXPECT().NewPaned(layout.OrientationHorizontal).Return(paned).Once() | |
| 17 | + | paned.EXPECT().SetResizeStartChild(true).Once() | |
| 18 | + | paned.EXPECT().SetResizeEndChild(true).Once() | |
| 19 | + | paned.EXPECT().SetVisible(true).Once() | |
| 20 | + | paned.EXPECT().GetAllocatedWidth().Return(0).Once() | |
| 21 | + | paned.EXPECT().ConnectNotifyPosition(mock.Anything).Return(uint(0)).Once() | |
| 22 | + | paned.EXPECT().ConnectMap(mock.Anything).Return(uint(0)).Once() | |
| 23 | + | paned.EXPECT().AddTickCallback(mock.Anything).Return(uint(42)).Once() | |
| 24 | + | paned.EXPECT().RemoveTickCallback(uint(42)).Once() | |
| 25 | + | ||
| 26 | + | sv := layout.NewSplitView(context.Background(), factory, layout.OrientationHorizontal, nil, nil, 0.5) | |
| 27 | + | sv.Cleanup() | |
| 28 | + | sv.Cleanup() | |
| 29 | + | } | |
| @@ -0,0 +1,29 @@ | |||||
| 1 | + | package layout_test | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "testing" | |||
| 6 | + | ||||
| 7 | + | "github.com/stretchr/testify/mock" | |||
| 8 | + | ||||
| 9 | + | "github.com/bnema/dumber/internal/ui/layout" | |||
| 10 | + | "github.com/bnema/dumber/internal/ui/layout/mocks" | |||
| 11 | + | ) | |||
| 12 | + | ||||
| 13 | + | func TestSplitViewCleanupReleasesTickCallbackExactlyOnce(t *testing.T) { | |||
| 14 | + | factory := mocks.NewMockWidgetFactory(t) | |||
| 15 | + | paned := mocks.NewMockPanedWidget(t) | |||
| 16 | + | factory.EXPECT().NewPaned(layout.OrientationHorizontal).Return(paned).Once() | |||
| 17 | + | paned.EXPECT().SetResizeStartChild(true).Once() | |||
| 18 | + | paned.EXPECT().SetResizeEndChild(true).Once() | |||
| 19 | + | paned.EXPECT().SetVisible(true).Once() | |||
| 20 | + | paned.EXPECT().GetAllocatedWidth().Return(0).Once() | |||
| 21 | + | paned.EXPECT().ConnectNotifyPosition(mock.Anything).Return(uint(0)).Once() | |||
| 22 | + | paned.EXPECT().ConnectMap(mock.Anything).Return(uint(0)).Once() | |||
| 23 | + | paned.EXPECT().AddTickCallback(mock.Anything).Return(uint(42)).Once() | |||
| 24 | + | paned.EXPECT().RemoveTickCallback(uint(42)).Once() | |||
| 25 | + | ||||
| 26 | + | sv := layout.NewSplitView(context.Background(), factory, layout.OrientationHorizontal, nil, nil, 0.5) | |||
| 27 | + | sv.Cleanup() | |||
| 28 | + | sv.Cleanup() | |||
| 29 | + | } | |||
M internal/ui/layout/tree.go +24 -5
| @@ -33,6 +33,9 @@ type TreeRenderer struct { | |||
| 33 | 33 | logger zerolog.Logger | |
| 34 | 34 | nodeToWidget map[string]Widget | |
| 35 | 35 | splitOrientation map[string]Orientation | |
| 36 | + | // splitViews owns the SplitView callback registrations created while | |
| 37 | + | // rendering. Widgets alone cannot release their tick callbacks. | |
| 38 | + | splitViews map[string]*SplitView | |
| 36 | 39 | // paneToStack maps pane IDs to their containing StackedView. | |
| 37 | 40 | // Every leaf pane is wrapped in a StackedView for easy stacking later. | |
| 38 | 41 | paneToStack map[string]*StackedView | |
| @@ -52,6 +55,7 @@ func NewTreeRenderer(ctx context.Context, factory WidgetFactory, paneViewFactory | |||
| 52 | 55 | logger: log.With().Str("component", "tree-renderer").Logger(), | |
| 53 | 56 | nodeToWidget: make(map[string]Widget), | |
| 54 | 57 | splitOrientation: make(map[string]Orientation), | |
| 58 | + | splitViews: make(map[string]*SplitView), | |
| 55 | 59 | paneToStack: make(map[string]*StackedView), | |
| 56 | 60 | } | |
| 57 | 61 | } | |
| @@ -73,10 +77,7 @@ func (tr *TreeRenderer) Build(ctx context.Context, root *entity.PaneNode) (Widge | |||
| 73 | 77 | tr.mu.Lock() | |
| 74 | 78 | defer tr.mu.Unlock() | |
| 75 | 79 | ||
| 76 | - | // Clear previous mappings | |
| 77 | - | tr.nodeToWidget = make(map[string]Widget) | |
| 78 | - | tr.splitOrientation = make(map[string]Orientation) | |
| 79 | - | tr.paneToStack = make(map[string]*StackedView) | |
| 80 | + | tr.clearInternal() | |
| 80 | 81 | ||
| 81 | 82 | return tr.renderNode(ctx, root) | |
| 82 | 83 | } | |
| @@ -189,6 +190,7 @@ func (tr *TreeRenderer) renderSplit(ctx context.Context, node *entity.PaneNode) | |||
| 189 | 190 | } | |
| 190 | 191 | }) | |
| 191 | 192 | tr.splitOrientation[node.ID] = orientation | |
| 193 | + | tr.splitViews[node.ID] = splitView | |
| 192 | 194 | ||
| 193 | 195 | return splitView.Widget(), nil | |
| 194 | 196 | } | |
| @@ -264,13 +266,26 @@ func (tr *TreeRenderer) NodeCount() int { | |||
| 264 | 266 | return len(tr.nodeToWidget) | |
| 265 | 267 | } | |
| 266 | 268 | ||
| 267 | - | // Clear removes all node-to-widget mappings. | |
| 269 | + | // Clear releases all renderer-owned split callbacks and removes mappings. | |
| 268 | 270 | func (tr *TreeRenderer) Clear() { | |
| 269 | 271 | tr.mu.Lock() | |
| 270 | 272 | defer tr.mu.Unlock() | |
| 271 | 273 | ||
| 274 | + | tr.clearInternal() | |
| 275 | + | } | |
| 276 | + | ||
| 277 | + | // clearInternal releases renderer-owned views and resets the lookup maps. | |
| 278 | + | // The caller must hold tr.mu. | |
| 279 | + | func (tr *TreeRenderer) clearInternal() { | |
| 280 | + | for _, splitView := range tr.splitViews { | |
| 281 | + | if splitView != nil { | |
| 282 | + | splitView.Cleanup() | |
| 283 | + | } | |
| 284 | + | } | |
| 272 | 285 | tr.nodeToWidget = make(map[string]Widget) | |
| 273 | 286 | tr.splitOrientation = make(map[string]Orientation) | |
| 287 | + | tr.splitViews = make(map[string]*SplitView) | |
| 288 | + | tr.paneToStack = make(map[string]*StackedView) | |
| 274 | 289 | } | |
| 275 | 290 | ||
| 276 | 291 | // RegisterWidget adds or updates a node-to-widget mapping. | |
| @@ -297,8 +312,12 @@ func (tr *TreeRenderer) UnregisterWidget(nodeID string) { | |||
| 297 | 312 | tr.mu.Lock() | |
| 298 | 313 | defer tr.mu.Unlock() | |
| 299 | 314 | ||
| 315 | + | if splitView := tr.splitViews[nodeID]; splitView != nil { | |
| 316 | + | splitView.Cleanup() | |
| 317 | + | } | |
| 300 | 318 | delete(tr.nodeToWidget, nodeID) | |
| 301 | 319 | delete(tr.splitOrientation, nodeID) | |
| 320 | + | delete(tr.splitViews, nodeID) | |
| 302 | 321 | } | |
| 303 | 322 | ||
| 304 | 323 | // UpdateSplitRatio updates the ratio of a split view by node ID. | |
| @@ -33,6 +33,9 @@ type TreeRenderer struct { | |||||
| 33 | logger zerolog.Logger | 33 | logger zerolog.Logger | ||
| 34 | nodeToWidget map[string]Widget | 34 | nodeToWidget map[string]Widget | ||
| 35 | splitOrientation map[string]Orientation | 35 | splitOrientation map[string]Orientation | ||
| 36 | + | // splitViews owns the SplitView callback registrations created while | |||
| 37 | + | // rendering. Widgets alone cannot release their tick callbacks. | |||
| 38 | + | splitViews map[string]*SplitView | |||
| 36 | // paneToStack maps pane IDs to their containing StackedView. | 39 | // paneToStack maps pane IDs to their containing StackedView. | ||
| 37 | // Every leaf pane is wrapped in a StackedView for easy stacking later. | 40 | // Every leaf pane is wrapped in a StackedView for easy stacking later. | ||
| 38 | paneToStack map[string]*StackedView | 41 | paneToStack map[string]*StackedView | ||
| @@ -52,6 +55,7 @@ func NewTreeRenderer(ctx context.Context, factory WidgetFactory, paneViewFactory | |||||
| 52 | logger: log.With().Str("component", "tree-renderer").Logger(), | 55 | logger: log.With().Str("component", "tree-renderer").Logger(), | ||
| 53 | nodeToWidget: make(map[string]Widget), | 56 | nodeToWidget: make(map[string]Widget), | ||
| 54 | splitOrientation: make(map[string]Orientation), | 57 | splitOrientation: make(map[string]Orientation), | ||
| 58 | + | splitViews: make(map[string]*SplitView), | |||
| 55 | paneToStack: make(map[string]*StackedView), | 59 | paneToStack: make(map[string]*StackedView), | ||
| 56 | } | 60 | } | ||
| 57 | } | 61 | } | ||
| @@ -73,10 +77,7 @@ func (tr *TreeRenderer) Build(ctx context.Context, root *entity.PaneNode) (Widge | |||||
| 73 | tr.mu.Lock() | 77 | tr.mu.Lock() | ||
| 74 | defer tr.mu.Unlock() | 78 | defer tr.mu.Unlock() | ||
| 75 | 79 | ||||
| 76 | - | // Clear previous mappings | 80 | + | tr.clearInternal() |
| 77 | - | tr.nodeToWidget = make(map[string]Widget) | |||
| 78 | - | tr.splitOrientation = make(map[string]Orientation) | |||
| 79 | - | tr.paneToStack = make(map[string]*StackedView) | |||
| 80 | 81 | ||||
| 81 | return tr.renderNode(ctx, root) | 82 | return tr.renderNode(ctx, root) | ||
| 82 | } | 83 | } | ||
| @@ -189,6 +190,7 @@ func (tr *TreeRenderer) renderSplit(ctx context.Context, node *entity.PaneNode) | |||||
| 189 | } | 190 | } | ||
| 190 | }) | 191 | }) | ||
| 191 | tr.splitOrientation[node.ID] = orientation | 192 | tr.splitOrientation[node.ID] = orientation | ||
| 193 | + | tr.splitViews[node.ID] = splitView | |||
| 192 | 194 | ||||
| 193 | return splitView.Widget(), nil | 195 | return splitView.Widget(), nil | ||
| 194 | } | 196 | } | ||
| @@ -264,13 +266,26 @@ func (tr *TreeRenderer) NodeCount() int { | |||||
| 264 | return len(tr.nodeToWidget) | 266 | return len(tr.nodeToWidget) | ||
| 265 | } | 267 | } | ||
| 266 | 268 | ||||
| 267 | - | // Clear removes all node-to-widget mappings. | 269 | + | // Clear releases all renderer-owned split callbacks and removes mappings. |
| 268 | func (tr *TreeRenderer) Clear() { | 270 | func (tr *TreeRenderer) Clear() { | ||
| 269 | tr.mu.Lock() | 271 | tr.mu.Lock() | ||
| 270 | defer tr.mu.Unlock() | 272 | defer tr.mu.Unlock() | ||
| 271 | 273 | ||||
| 274 | + | tr.clearInternal() | |||
| 275 | + | } | |||
| 276 | + | ||||
| 277 | + | // clearInternal releases renderer-owned views and resets the lookup maps. | |||
| 278 | + | // The caller must hold tr.mu. | |||
| 279 | + | func (tr *TreeRenderer) clearInternal() { | |||
| 280 | + | for _, splitView := range tr.splitViews { | |||
| 281 | + | if splitView != nil { | |||
| 282 | + | splitView.Cleanup() | |||
| 283 | + | } | |||
| 284 | + | } | |||
| 272 | tr.nodeToWidget = make(map[string]Widget) | 285 | tr.nodeToWidget = make(map[string]Widget) | ||
| 273 | tr.splitOrientation = make(map[string]Orientation) | 286 | tr.splitOrientation = make(map[string]Orientation) | ||
| 287 | + | tr.splitViews = make(map[string]*SplitView) | |||
| 288 | + | tr.paneToStack = make(map[string]*StackedView) | |||
| 274 | } | 289 | } | ||
| 275 | 290 | ||||
| 276 | // RegisterWidget adds or updates a node-to-widget mapping. | 291 | // RegisterWidget adds or updates a node-to-widget mapping. | ||
| @@ -297,8 +312,12 @@ func (tr *TreeRenderer) UnregisterWidget(nodeID string) { | |||||
| 297 | tr.mu.Lock() | 312 | tr.mu.Lock() | ||
| 298 | defer tr.mu.Unlock() | 313 | defer tr.mu.Unlock() | ||
| 299 | 314 | ||||
| 315 | + | if splitView := tr.splitViews[nodeID]; splitView != nil { | |||
| 316 | + | splitView.Cleanup() | |||
| 317 | + | } | |||
| 300 | delete(tr.nodeToWidget, nodeID) | 318 | delete(tr.nodeToWidget, nodeID) | ||
| 301 | delete(tr.splitOrientation, nodeID) | 319 | delete(tr.splitOrientation, nodeID) | ||
| 320 | + | delete(tr.splitViews, nodeID) | |||
| 302 | } | 321 | } | ||
| 303 | 322 | ||||
| 304 | // UpdateSplitRatio updates the ratio of a split view by node ID. | 323 | // UpdateSplitRatio updates the ratio of a split view by node ID. | ||
A internal/ui/layout/tree_cleanup_test.go +48 -0
| @@ -0,0 +1,48 @@ | |||
| 1 | + | package layout_test | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "testing" | |
| 6 | + | ||
| 7 | + | "github.com/bnema/dumber/internal/domain/entity" | |
| 8 | + | "github.com/bnema/dumber/internal/ui/layout" | |
| 9 | + | "github.com/bnema/dumber/internal/ui/layout/mocks" | |
| 10 | + | "github.com/stretchr/testify/mock" | |
| 11 | + | "github.com/stretchr/testify/require" | |
| 12 | + | ) | |
| 13 | + | ||
| 14 | + | func TestTreeRendererClearReleasesOwnedSplitTickExactlyOnce(t *testing.T) { | |
| 15 | + | ctx := context.Background() | |
| 16 | + | factory := mocks.NewMockWidgetFactory(t) | |
| 17 | + | paneFactory := mocks.NewMockPaneViewFactory(t) | |
| 18 | + | paned := mocks.NewMockPanedWidget(t) | |
| 19 | + | leftStack, leftContainer := setupStackedLeafMocks(t, factory) | |
| 20 | + | rightStack, rightContainer := setupStackedLeafMocks(t, factory) | |
| 21 | + | ||
| 22 | + | left := &entity.PaneNode{ID: "left", Pane: entity.NewPane("left")} | |
| 23 | + | right := &entity.PaneNode{ID: "right", Pane: entity.NewPane("right")} | |
| 24 | + | root := &entity.PaneNode{ID: "split", SplitDir: entity.SplitHorizontal, SplitRatio: 0.5, Children: []*entity.PaneNode{left, right}} | |
| 25 | + | ||
| 26 | + | paneFactory.EXPECT().CreatePaneView(left).Return(leftContainer).Once() | |
| 27 | + | paneFactory.EXPECT().CreatePaneView(right).Return(rightContainer).Once() | |
| 28 | + | factory.EXPECT().NewPaned(layout.OrientationHorizontal).Return(paned).Once() | |
| 29 | + | paned.EXPECT().SetResizeStartChild(true).Once() | |
| 30 | + | paned.EXPECT().SetResizeEndChild(true).Once() | |
| 31 | + | paned.EXPECT().SetVisible(true).Once() | |
| 32 | + | leftStack.EXPECT().SetVisible(true).Once() | |
| 33 | + | rightStack.EXPECT().SetVisible(true).Once() | |
| 34 | + | paned.EXPECT().SetStartChild(leftStack).Once() | |
| 35 | + | paned.EXPECT().SetEndChild(rightStack).Once() | |
| 36 | + | paned.EXPECT().ConnectNotifyPosition(mock.Anything).Return(uint(0)).Once() | |
| 37 | + | paned.EXPECT().GetAllocatedWidth().Return(0).Once() | |
| 38 | + | paned.EXPECT().ConnectMap(mock.Anything).Return(uint(0)).Once() | |
| 39 | + | paned.EXPECT().AddTickCallback(mock.Anything).Return(uint(42)).Once() | |
| 40 | + | paned.EXPECT().RemoveTickCallback(uint(42)).Once() | |
| 41 | + | ||
| 42 | + | renderer := layout.NewTreeRenderer(ctx, factory, paneFactory) | |
| 43 | + | _, err := renderer.Build(ctx, root) | |
| 44 | + | require.NoError(t, err) | |
| 45 | + | ||
| 46 | + | renderer.Clear() | |
| 47 | + | renderer.Clear() | |
| 48 | + | } | |
| @@ -0,0 +1,48 @@ | |||||
| 1 | + | package layout_test | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "testing" | |||
| 6 | + | ||||
| 7 | + | "github.com/bnema/dumber/internal/domain/entity" | |||
| 8 | + | "github.com/bnema/dumber/internal/ui/layout" | |||
| 9 | + | "github.com/bnema/dumber/internal/ui/layout/mocks" | |||
| 10 | + | "github.com/stretchr/testify/mock" | |||
| 11 | + | "github.com/stretchr/testify/require" | |||
| 12 | + | ) | |||
| 13 | + | ||||
| 14 | + | func TestTreeRendererClearReleasesOwnedSplitTickExactlyOnce(t *testing.T) { | |||
| 15 | + | ctx := context.Background() | |||
| 16 | + | factory := mocks.NewMockWidgetFactory(t) | |||
| 17 | + | paneFactory := mocks.NewMockPaneViewFactory(t) | |||
| 18 | + | paned := mocks.NewMockPanedWidget(t) | |||
| 19 | + | leftStack, leftContainer := setupStackedLeafMocks(t, factory) | |||
| 20 | + | rightStack, rightContainer := setupStackedLeafMocks(t, factory) | |||
| 21 | + | ||||
| 22 | + | left := &entity.PaneNode{ID: "left", Pane: entity.NewPane("left")} | |||
| 23 | + | right := &entity.PaneNode{ID: "right", Pane: entity.NewPane("right")} | |||
| 24 | + | root := &entity.PaneNode{ID: "split", SplitDir: entity.SplitHorizontal, SplitRatio: 0.5, Children: []*entity.PaneNode{left, right}} | |||
| 25 | + | ||||
| 26 | + | paneFactory.EXPECT().CreatePaneView(left).Return(leftContainer).Once() | |||
| 27 | + | paneFactory.EXPECT().CreatePaneView(right).Return(rightContainer).Once() | |||
| 28 | + | factory.EXPECT().NewPaned(layout.OrientationHorizontal).Return(paned).Once() | |||
| 29 | + | paned.EXPECT().SetResizeStartChild(true).Once() | |||
| 30 | + | paned.EXPECT().SetResizeEndChild(true).Once() | |||
| 31 | + | paned.EXPECT().SetVisible(true).Once() | |||
| 32 | + | leftStack.EXPECT().SetVisible(true).Once() | |||
| 33 | + | rightStack.EXPECT().SetVisible(true).Once() | |||
| 34 | + | paned.EXPECT().SetStartChild(leftStack).Once() | |||
| 35 | + | paned.EXPECT().SetEndChild(rightStack).Once() | |||
| 36 | + | paned.EXPECT().ConnectNotifyPosition(mock.Anything).Return(uint(0)).Once() | |||
| 37 | + | paned.EXPECT().GetAllocatedWidth().Return(0).Once() | |||
| 38 | + | paned.EXPECT().ConnectMap(mock.Anything).Return(uint(0)).Once() | |||
| 39 | + | paned.EXPECT().AddTickCallback(mock.Anything).Return(uint(42)).Once() | |||
| 40 | + | paned.EXPECT().RemoveTickCallback(uint(42)).Once() | |||
| 41 | + | ||||
| 42 | + | renderer := layout.NewTreeRenderer(ctx, factory, paneFactory) | |||
| 43 | + | _, err := renderer.Build(ctx, root) | |||
| 44 | + | require.NoError(t, err) | |||
| 45 | + | ||||
| 46 | + | renderer.Clear() | |||
| 47 | + | renderer.Clear() | |||
| 48 | + | } | |||
M internal/ui/layout/widgets.go +5 -2
| @@ -99,9 +99,12 @@ type PanedWidget interface { | |||
| 99 | 99 | ConnectMap(callback func()) uint | |
| 100 | 100 | ConnectNotifyPosition(callback func()) uint | |
| 101 | 101 | ||
| 102 | - | // Tick callback for frame-based updates (returns signal ID) | |
| 103 | - | // Callback returns true to continue, false to stop | |
| 102 | + | // Tick callback for frame-based updates (returns signal ID). | |
| 103 | + | // Callback returns true to continue, false to stop. | |
| 104 | 104 | AddTickCallback(callback func() bool) uint | |
| 105 | + | // RemoveTickCallback releases an active callback slot. It is safe to call | |
| 106 | + | // once after the callback has stopped itself. | |
| 107 | + | RemoveTickCallback(id uint) | |
| 105 | 108 | ||
| 106 | 109 | // Handle appearance | |
| 107 | 110 | SetWideHandle(wide bool) | |
| @@ -99,9 +99,12 @@ type PanedWidget interface { | |||||
| 99 | ConnectMap(callback func()) uint | 99 | ConnectMap(callback func()) uint | ||
| 100 | ConnectNotifyPosition(callback func()) uint | 100 | ConnectNotifyPosition(callback func()) uint | ||
| 101 | 101 | ||||
| 102 | - | // Tick callback for frame-based updates (returns signal ID) | 102 | + | // Tick callback for frame-based updates (returns signal ID). |
| 103 | - | // Callback returns true to continue, false to stop | 103 | + | // Callback returns true to continue, false to stop. |
| 104 | AddTickCallback(callback func() bool) uint | 104 | AddTickCallback(callback func() bool) uint | ||
| 105 | + | // RemoveTickCallback releases an active callback slot. It is safe to call | |||
| 106 | + | // once after the callback has stopped itself. | |||
| 107 | + | RemoveTickCallback(id uint) | |||
| 105 | 108 | ||||
| 106 | // Handle appearance | 109 | // Handle appearance | ||
| 107 | SetWideHandle(wide bool) | 110 | SetWideHandle(wide bool) | ||