fix(startup): reserve GTK presentation completion
- commit
- 3bc61239b4cc1cc6b33f4d5907d57824e31dee7e
- author
- brice · · 2026-07-14 08:05 UTC
- committer
- brice · · 2026-07-14 09:35 UTC
- parent
- 1a39e17
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +47 -13
M internal/infrastructure/cef/render_handler_adapter.go +1 -1
| @@ -35,7 +35,7 @@ func startupPresentationHooks() cef2gtk.Hooks { | |||
| 35 | 35 | logging.Trace().Mark("first_dmabuf_texture_swap") | |
| 36 | 36 | }, | |
| 37 | 37 | OnFirstPresentation: func() { | |
| 38 | - | logging.Trace().Mark("first_gtk_presentation") | |
| 38 | + | logging.Trace().MarkGTKAfterPaint() | |
| 39 | 39 | }, | |
| 40 | 40 | OnDMABUFUnsupported: func() { | |
| 41 | 41 | logging.Trace().SetIncompleteReason("dmabuf_texture_swap_unavailable") | |
| @@ -35,7 +35,7 @@ func startupPresentationHooks() cef2gtk.Hooks { | |||||
| 35 | logging.Trace().Mark("first_dmabuf_texture_swap") | 35 | logging.Trace().Mark("first_dmabuf_texture_swap") | ||
| 36 | }, | 36 | }, | ||
| 37 | OnFirstPresentation: func() { | 37 | OnFirstPresentation: func() { | ||
| 38 | - | logging.Trace().Mark("first_gtk_presentation") | 38 | + | logging.Trace().MarkGTKAfterPaint() |
| 39 | }, | 39 | }, | ||
| 40 | OnDMABUFUnsupported: func() { | 40 | OnDMABUFUnsupported: func() { | ||
| 41 | logging.Trace().SetIncompleteReason("dmabuf_texture_swap_unavailable") | 41 | logging.Trace().SetIncompleteReason("dmabuf_texture_swap_unavailable") | ||
M internal/logging/startup_trace.go +21 -5
| @@ -121,9 +121,25 @@ func (st *StartupTrace) SetIncompleteReason(reason string) { | |||
| 121 | 121 | } | |
| 122 | 122 | } | |
| 123 | 123 | ||
| 124 | - | // Mark accepts exactly the next startup milestone and returns whether it was | |
| 125 | - | // recorded. It is safe for callbacks from CEF and GTK threads. | |
| 124 | + | // Mark accepts exactly the next non-presentation startup milestone and returns | |
| 125 | + | // whether it was recorded. first_gtk_presentation is reserved for the GTK | |
| 126 | + | // frame-clock after-paint hook through MarkGTKAfterPaint. | |
| 126 | 127 | func (st *StartupTrace) Mark(name string) bool { | |
| 128 | + | if name == "first_gtk_presentation" { | |
| 129 | + | return false | |
| 130 | + | } | |
| 131 | + | return st.mark(name) | |
| 132 | + | } | |
| 133 | + | ||
| 134 | + | // MarkGTKAfterPaint records the final milestone. It is called only from the | |
| 135 | + | // upstream CEF-to-GTK bridge's first frame-clock after-paint callback. | |
| 136 | + | func (st *StartupTrace) MarkGTKAfterPaint() bool { | |
| 137 | + | return st.mark("first_gtk_presentation") | |
| 138 | + | } | |
| 139 | + | ||
| 140 | + | // mark accepts the next milestone and is safe for callbacks from CEF and GTK | |
| 141 | + | // threads. | |
| 142 | + | func (st *StartupTrace) mark(name string) bool { | |
| 127 | 143 | if st == nil || st.now == nil { | |
| 128 | 144 | return false | |
| 129 | 145 | } | |
| @@ -172,9 +188,9 @@ func (st *StartupTrace) emitSummaryLocked() { | |||
| 172 | 188 | Msg("startup_trace: first presentation") | |
| 173 | 189 | } | |
| 174 | 190 | ||
| 175 | - | // Finish is retained for old callers. A startup trace only finishes at the | |
| 176 | - | // actual first GTK presentation milestone. | |
| 177 | - | func (st *StartupTrace) Finish() { _ = st.Mark("first_gtk_presentation") } | |
| 191 | + | // Finish is retained as a harmless compatibility no-op. The only valid source | |
| 192 | + | // for first_gtk_presentation is the upstream GTK frame-clock after-paint hook. | |
| 193 | + | func (*StartupTrace) Finish() {} | |
| 178 | 194 | ||
| 179 | 195 | func (st *StartupTrace) Enabled() bool { return st != nil && st.now != nil } | |
| 180 | 196 | ||
| @@ -121,9 +121,25 @@ func (st *StartupTrace) SetIncompleteReason(reason string) { | |||||
| 121 | } | 121 | } | ||
| 122 | } | 122 | } | ||
| 123 | 123 | ||||
| 124 | - | // Mark accepts exactly the next startup milestone and returns whether it was | 124 | + | // Mark accepts exactly the next non-presentation startup milestone and returns |
| 125 | - | // recorded. It is safe for callbacks from CEF and GTK threads. | 125 | + | // whether it was recorded. first_gtk_presentation is reserved for the GTK |
| 126 | + | // frame-clock after-paint hook through MarkGTKAfterPaint. | |||
| 126 | func (st *StartupTrace) Mark(name string) bool { | 127 | func (st *StartupTrace) Mark(name string) bool { | ||
| 128 | + | if name == "first_gtk_presentation" { | |||
| 129 | + | return false | |||
| 130 | + | } | |||
| 131 | + | return st.mark(name) | |||
| 132 | + | } | |||
| 133 | + | ||||
| 134 | + | // MarkGTKAfterPaint records the final milestone. It is called only from the | |||
| 135 | + | // upstream CEF-to-GTK bridge's first frame-clock after-paint callback. | |||
| 136 | + | func (st *StartupTrace) MarkGTKAfterPaint() bool { | |||
| 137 | + | return st.mark("first_gtk_presentation") | |||
| 138 | + | } | |||
| 139 | + | ||||
| 140 | + | // mark accepts the next milestone and is safe for callbacks from CEF and GTK | |||
| 141 | + | // threads. | |||
| 142 | + | func (st *StartupTrace) mark(name string) bool { | |||
| 127 | if st == nil || st.now == nil { | 143 | if st == nil || st.now == nil { | ||
| 128 | return false | 144 | return false | ||
| 129 | } | 145 | } | ||
| @@ -172,9 +188,9 @@ func (st *StartupTrace) emitSummaryLocked() { | |||||
| 172 | Msg("startup_trace: first presentation") | 188 | Msg("startup_trace: first presentation") | ||
| 173 | } | 189 | } | ||
| 174 | 190 | ||||
| 175 | - | // Finish is retained for old callers. A startup trace only finishes at the | 191 | + | // Finish is retained as a harmless compatibility no-op. The only valid source |
| 176 | - | // actual first GTK presentation milestone. | 192 | + | // for first_gtk_presentation is the upstream GTK frame-clock after-paint hook. |
| 177 | - | func (st *StartupTrace) Finish() { _ = st.Mark("first_gtk_presentation") } | 193 | + | func (*StartupTrace) Finish() {} |
| 178 | 194 | ||||
| 179 | func (st *StartupTrace) Enabled() bool { return st != nil && st.now != nil } | 195 | func (st *StartupTrace) Enabled() bool { return st != nil && st.now != nil } | ||
| 180 | 196 | ||||
M internal/logging/startup_trace_test.go +25 -3
| @@ -27,11 +27,31 @@ func TestStartupTraceAcceptsOnlyOrderedOneShotMilestones(t *testing.T) { | |||
| 27 | 27 | "first_gtk_presentation", | |
| 28 | 28 | } { | |
| 29 | 29 | now = now.Add(time.Millisecond) | |
| 30 | - | require.Truef(t, trace.Mark(name), "milestone %s should be accepted", name) | |
| 30 | + | if name == "first_gtk_presentation" { | |
| 31 | + | require.Truef(t, trace.MarkGTKAfterPaint(), "milestone %s should be accepted", name) | |
| 32 | + | } else { | |
| 33 | + | require.Truef(t, trace.Mark(name), "milestone %s should be accepted", name) | |
| 34 | + | } | |
| 31 | 35 | } | |
| 32 | 36 | require.False(t, trace.Mark("first_gtk_presentation")) | |
| 33 | 37 | } | |
| 34 | 38 | ||
| 39 | + | func TestStartupTraceFinishCannotFabricateFirstGTKPresentation(t *testing.T) { | |
| 40 | + | now := time.Unix(100, 0) | |
| 41 | + | trace := newStartupTrace(func() time.Time { return now }) | |
| 42 | + | ||
| 43 | + | for _, name := range startupMilestoneOrder[:len(startupMilestoneOrder)-1] { | |
| 44 | + | now = now.Add(time.Millisecond) | |
| 45 | + | require.True(t, trace.Mark(name)) | |
| 46 | + | } | |
| 47 | + | ||
| 48 | + | trace.Finish() // Legacy callers do not observe the upstream GTK after-paint. | |
| 49 | + | require.Len(t, trace.milestones, len(startupMilestoneOrder)-1) | |
| 50 | + | require.False(t, trace.Mark("first_gtk_presentation"), "generic callers cannot record the reserved milestone") | |
| 51 | + | require.False(t, trace.summaryEmitted) | |
| 52 | + | require.True(t, trace.MarkGTKAfterPaint(), "only the after-paint hook may record this milestone") | |
| 53 | + | } | |
| 54 | + | ||
| 35 | 55 | func TestStartupTraceEmitsOneNormalSummaryAtFirstPresentation(t *testing.T) { | |
| 36 | 56 | var output bytes.Buffer | |
| 37 | 57 | logger := zerolog.New(&output) | |
| @@ -40,11 +60,13 @@ func TestStartupTraceEmitsOneNormalSummaryAtFirstPresentation(t *testing.T) { | |||
| 40 | 60 | trace.SetBackend("gdk-dmabuf") | |
| 41 | 61 | trace.SetLogger(&logger) | |
| 42 | 62 | ||
| 43 | - | for _, name := range startupMilestoneOrder { | |
| 63 | + | for _, name := range startupMilestoneOrder[:len(startupMilestoneOrder)-1] { | |
| 44 | 64 | now = now.Add(time.Millisecond) | |
| 45 | 65 | require.True(t, trace.Mark(name)) | |
| 46 | 66 | } | |
| 47 | - | trace.Mark("first_gtk_presentation") | |
| 67 | + | now = now.Add(time.Millisecond) | |
| 68 | + | require.True(t, trace.MarkGTKAfterPaint()) | |
| 69 | + | trace.MarkGTKAfterPaint() | |
| 48 | 70 | ||
| 49 | 71 | require.Contains(t, output.String(), `"message":"startup_trace: first presentation"`) | |
| 50 | 72 | require.Contains(t, output.String(), `"backend":"gdk-dmabuf"`) | |
| @@ -27,11 +27,31 @@ func TestStartupTraceAcceptsOnlyOrderedOneShotMilestones(t *testing.T) { | |||||
| 27 | "first_gtk_presentation", | 27 | "first_gtk_presentation", | ||
| 28 | } { | 28 | } { | ||
| 29 | now = now.Add(time.Millisecond) | 29 | now = now.Add(time.Millisecond) | ||
| 30 | - | require.Truef(t, trace.Mark(name), "milestone %s should be accepted", name) | 30 | + | if name == "first_gtk_presentation" { |
| 31 | + | require.Truef(t, trace.MarkGTKAfterPaint(), "milestone %s should be accepted", name) | |||
| 32 | + | } else { | |||
| 33 | + | require.Truef(t, trace.Mark(name), "milestone %s should be accepted", name) | |||
| 34 | + | } | |||
| 31 | } | 35 | } | ||
| 32 | require.False(t, trace.Mark("first_gtk_presentation")) | 36 | require.False(t, trace.Mark("first_gtk_presentation")) | ||
| 33 | } | 37 | } | ||
| 34 | 38 | ||||
| 39 | + | func TestStartupTraceFinishCannotFabricateFirstGTKPresentation(t *testing.T) { | |||
| 40 | + | now := time.Unix(100, 0) | |||
| 41 | + | trace := newStartupTrace(func() time.Time { return now }) | |||
| 42 | + | ||||
| 43 | + | for _, name := range startupMilestoneOrder[:len(startupMilestoneOrder)-1] { | |||
| 44 | + | now = now.Add(time.Millisecond) | |||
| 45 | + | require.True(t, trace.Mark(name)) | |||
| 46 | + | } | |||
| 47 | + | ||||
| 48 | + | trace.Finish() // Legacy callers do not observe the upstream GTK after-paint. | |||
| 49 | + | require.Len(t, trace.milestones, len(startupMilestoneOrder)-1) | |||
| 50 | + | require.False(t, trace.Mark("first_gtk_presentation"), "generic callers cannot record the reserved milestone") | |||
| 51 | + | require.False(t, trace.summaryEmitted) | |||
| 52 | + | require.True(t, trace.MarkGTKAfterPaint(), "only the after-paint hook may record this milestone") | |||
| 53 | + | } | |||
| 54 | + | ||||
| 35 | func TestStartupTraceEmitsOneNormalSummaryAtFirstPresentation(t *testing.T) { | 55 | func TestStartupTraceEmitsOneNormalSummaryAtFirstPresentation(t *testing.T) { | ||
| 36 | var output bytes.Buffer | 56 | var output bytes.Buffer | ||
| 37 | logger := zerolog.New(&output) | 57 | logger := zerolog.New(&output) | ||
| @@ -40,11 +60,13 @@ func TestStartupTraceEmitsOneNormalSummaryAtFirstPresentation(t *testing.T) { | |||||
| 40 | trace.SetBackend("gdk-dmabuf") | 60 | trace.SetBackend("gdk-dmabuf") | ||
| 41 | trace.SetLogger(&logger) | 61 | trace.SetLogger(&logger) | ||
| 42 | 62 | ||||
| 43 | - | for _, name := range startupMilestoneOrder { | 63 | + | for _, name := range startupMilestoneOrder[:len(startupMilestoneOrder)-1] { |
| 44 | now = now.Add(time.Millisecond) | 64 | now = now.Add(time.Millisecond) | ||
| 45 | require.True(t, trace.Mark(name)) | 65 | require.True(t, trace.Mark(name)) | ||
| 46 | } | 66 | } | ||
| 47 | - | trace.Mark("first_gtk_presentation") | 67 | + | now = now.Add(time.Millisecond) |
| 68 | + | require.True(t, trace.MarkGTKAfterPaint()) | |||
| 69 | + | trace.MarkGTKAfterPaint() | |||
| 48 | 70 | ||||
| 49 | require.Contains(t, output.String(), `"message":"startup_trace: first presentation"`) | 71 | require.Contains(t, output.String(), `"message":"startup_trace: first presentation"`) | ||
| 50 | require.Contains(t, output.String(), `"backend":"gdk-dmabuf"`) | 72 | require.Contains(t, output.String(), `"backend":"gdk-dmabuf"`) | ||
M internal/ui/coordinator/content/navigation.go +0 -4
| @@ -288,10 +288,6 @@ func (c *Coordinator) revealIfPending(ctx context.Context, paneID entity.PaneID, | |||
| 288 | 288 | } | |
| 289 | 289 | } | |
| 290 | 290 | ||
| 291 | - | // Mark first_paint and finish startup trace | |
| 292 | - | logging.Trace().Mark("first_paint") | |
| 293 | - | logging.Trace().Finish() | |
| 294 | - | ||
| 295 | 291 | if c.onWebViewShown != nil { | |
| 296 | 292 | c.onWebViewShown(paneID) | |
| 297 | 293 | } | |
| @@ -288,10 +288,6 @@ func (c *Coordinator) revealIfPending(ctx context.Context, paneID entity.PaneID, | |||||
| 288 | } | 288 | } | ||
| 289 | } | 289 | } | ||
| 290 | 290 | ||||
| 291 | - | // Mark first_paint and finish startup trace | |||
| 292 | - | logging.Trace().Mark("first_paint") | |||
| 293 | - | logging.Trace().Finish() | |||
| 294 | - | ||||
| 295 | if c.onWebViewShown != nil { | 291 | if c.onWebViewShown != nil { | ||
| 296 | c.onWebViewShown(paneID) | 292 | c.onWebViewShown(paneID) | ||
| 297 | } | 293 | } | ||