diff --git a/go/display/webkit/pkg/window/mock_test.go b/go/display/webkit/pkg/window/mock_test.go index ca7f74b..6e3f467 100644 --- a/go/display/webkit/pkg/window/mock_test.go +++ b/go/display/webkit/pkg/window/mock_test.go @@ -1,6 +1,24 @@ package window +import "sync" + +// The mock is driven from more than one goroutine, so it is guarded like the +// thing it stands in for. +// +// A real platform window is called from wherever the Service happens to be: +// taskEvalJS runs ExecJS on the caller's goroutine while a test — or another +// task — reads the window's state on its own. mockWindow began as a bag of +// plain fields, which made that a data race rather than a design: `go test +// ./display/webkit/pkg/window/ -race` failed in TestTaskEvalJS_Good, one +// goroutine appending to execJSCalls inside ExecJS while the test read the same +// slice. +// +// One mutex per window covers every field rather than only the slice that +// happened to be caught. Guarding one field would leave the same defect latent +// in every other, waiting for the next test that drives the Service +// concurrently — and the cost here is a test double taking an uncontended lock. type mockPlatform struct { + mu sync.Mutex windows []*mockWindow } @@ -18,19 +36,28 @@ func (m *mockPlatform) CreateWindow(options PlatformWindowOptions) PlatformWindo if options.JS != "" { w.execJSCalls = append(w.execJSCalls, options.JS) } + + m.mu.Lock() + defer m.mu.Unlock() m.windows = append(m.windows, w) + return w } func (m *mockPlatform) GetWindows() []PlatformWindow { + m.mu.Lock() + defer m.mu.Unlock() + out := make([]PlatformWindow, len(m.windows)) for i, w := range m.windows { out[i] = w } + return out } type mockWindow struct { + mu sync.Mutex name, title, url, html string width, height, x, y int maximised, focused bool @@ -50,71 +77,171 @@ type mockWindow struct { closeBehavior CloseBehavior } -func (w *mockWindow) Name() string { return w.name } -func (w *mockWindow) Title() string { return w.title } -func (w *mockWindow) Position() (int, int) { return w.x, w.y } -func (w *mockWindow) Size() (int, int) { return w.width, w.height } -func (w *mockWindow) IsMaximised() bool { return w.maximised } -func (w *mockWindow) IsFocused() bool { return w.focused } -func (w *mockWindow) IsVisible() bool { return w.visible } -func (w *mockWindow) IsFullscreen() bool { return w.fullscreened } -func (w *mockWindow) IsMinimised() bool { return w.minimised } -func (w *mockWindow) IsAlwaysOnTop() bool { return w.alwaysOnTop } -func (w *mockWindow) GetBounds() (int, int, int, int) { return w.x, w.y, w.width, w.height } +func (w *mockWindow) Name() string { w.mu.Lock(); defer w.mu.Unlock(); return w.name } +func (w *mockWindow) Title() string { w.mu.Lock(); defer w.mu.Unlock(); return w.title } +func (w *mockWindow) Position() (int, int) { + w.mu.Lock() + defer w.mu.Unlock() + return w.x, w.y +} + +func (w *mockWindow) Size() (int, int) { + w.mu.Lock() + defer w.mu.Unlock() + return w.width, w.height +} +func (w *mockWindow) IsMaximised() bool { w.mu.Lock(); defer w.mu.Unlock(); return w.maximised } +func (w *mockWindow) IsFocused() bool { w.mu.Lock(); defer w.mu.Unlock(); return w.focused } +func (w *mockWindow) IsVisible() bool { w.mu.Lock(); defer w.mu.Unlock(); return w.visible } +func (w *mockWindow) IsFullscreen() bool { w.mu.Lock(); defer w.mu.Unlock(); return w.fullscreened } +func (w *mockWindow) IsMinimised() bool { w.mu.Lock(); defer w.mu.Unlock(); return w.minimised } +func (w *mockWindow) IsAlwaysOnTop() bool { w.mu.Lock(); defer w.mu.Unlock(); return w.alwaysOnTop } +func (w *mockWindow) GetBounds() (int, int, int, int) { + w.mu.Lock() + defer w.mu.Unlock() + return w.x, w.y, w.width, w.height +} + func (w *mockWindow) GetZoom() float64 { + w.mu.Lock() + defer w.mu.Unlock() if w.zoom == 0 { return 1.0 } + return w.zoom } -func (w *mockWindow) GetOpacity() float64 { return w.opacity } -func (w *mockWindow) SetTitle(title string) { w.title = title } -func (w *mockWindow) SetPosition(x, y int) { w.x = x; w.y = y } -func (w *mockWindow) SetSize(width, height int) { w.width = width; w.height = height } -func (w *mockWindow) SetBackgroundColour(r, g, b, a uint8) { w.backgroundColour = [4]uint8{r, g, b, a} } -func (w *mockWindow) SetVisibility(visible bool) { w.visible = visible } -func (w *mockWindow) SetAlwaysOnTop(alwaysOnTop bool) { w.alwaysOnTop = alwaysOnTop } -func (w *mockWindow) SetOpacity(opacity float64) { w.opacity = opacity } +func (w *mockWindow) GetOpacity() float64 { w.mu.Lock(); defer w.mu.Unlock(); return w.opacity } +func (w *mockWindow) SetTitle(title string) { w.mu.Lock(); defer w.mu.Unlock(); w.title = title } +func (w *mockWindow) SetPosition(x, y int) { + w.mu.Lock() + defer w.mu.Unlock() + w.x, w.y = x, y +} + +func (w *mockWindow) SetSize(width, height int) { + w.mu.Lock() + defer w.mu.Unlock() + w.width, w.height = width, height +} + +func (w *mockWindow) SetBackgroundColour(r, g, b, a uint8) { + w.mu.Lock() + defer w.mu.Unlock() + w.backgroundColour = [4]uint8{r, g, b, a} +} + +func (w *mockWindow) SetVisibility(visible bool) { + w.mu.Lock() + defer w.mu.Unlock() + w.visible = visible +} + +func (w *mockWindow) SetAlwaysOnTop(alwaysOnTop bool) { + w.mu.Lock() + defer w.mu.Unlock() + w.alwaysOnTop = alwaysOnTop +} + +func (w *mockWindow) SetOpacity(opacity float64) { + w.mu.Lock() + defer w.mu.Unlock() + w.opacity = opacity +} + func (w *mockWindow) SetBounds(x, y, width, height int) { + w.mu.Lock() + defer w.mu.Unlock() w.x = x w.y = y w.width = width w.height = height } -func (w *mockWindow) SetURL(url string) { w.url = url } -func (w *mockWindow) SetHTML(html string) { w.html = html } -func (w *mockWindow) SetZoom(magnification float64) { w.zoom = magnification } -func (w *mockWindow) SetContentProtection(protection bool) { w.contentProtection = protection } -func (w *mockWindow) Maximise() { w.maximised = true } -func (w *mockWindow) Restore() { w.maximised = false } -func (w *mockWindow) Minimise() { w.minimised = true } -func (w *mockWindow) Focus() { w.focused = true } -func (w *mockWindow) Close() { w.closed = true } -func (w *mockWindow) Show() { w.visible = true } -func (w *mockWindow) Hide() { w.visible = false } -func (w *mockWindow) Fullscreen() { w.fullscreened = true } -func (w *mockWindow) UnFullscreen() { w.fullscreened = false } -func (w *mockWindow) ToggleFullscreen() { w.fullscreened = !w.fullscreened } -func (w *mockWindow) ToggleMaximise() { w.maximised = !w.maximised } -func (w *mockWindow) ExecJS(js string) { w.execJSCalls = append(w.execJSCalls, js) } -func (w *mockWindow) Flash(enabled bool) { w.flashed = enabled } -func (w *mockWindow) Print() resultFailure { return nil } -func (w *mockWindow) OpenDevTools() { w.devToolsOpen = true } -func (w *mockWindow) CloseDevTools() { w.devToolsOpen = false } +func (w *mockWindow) SetURL(url string) { w.mu.Lock(); defer w.mu.Unlock(); w.url = url } +func (w *mockWindow) SetHTML(html string) { w.mu.Lock(); defer w.mu.Unlock(); w.html = html } +func (w *mockWindow) SetZoom(magnification float64) { + w.mu.Lock() + defer w.mu.Unlock() + w.zoom = magnification +} + +func (w *mockWindow) SetContentProtection(protection bool) { + w.mu.Lock() + defer w.mu.Unlock() + w.contentProtection = protection +} +func (w *mockWindow) Maximise() { w.mu.Lock(); defer w.mu.Unlock(); w.maximised = true } +func (w *mockWindow) Restore() { w.mu.Lock(); defer w.mu.Unlock(); w.maximised = false } +func (w *mockWindow) Minimise() { w.mu.Lock(); defer w.mu.Unlock(); w.minimised = true } +func (w *mockWindow) Focus() { w.mu.Lock(); defer w.mu.Unlock(); w.focused = true } +func (w *mockWindow) Close() { w.mu.Lock(); defer w.mu.Unlock(); w.closed = true } +func (w *mockWindow) Show() { w.mu.Lock(); defer w.mu.Unlock(); w.visible = true } +func (w *mockWindow) Hide() { w.mu.Lock(); defer w.mu.Unlock(); w.visible = false } +func (w *mockWindow) Fullscreen() { w.mu.Lock(); defer w.mu.Unlock(); w.fullscreened = true } +func (w *mockWindow) UnFullscreen() { w.mu.Lock(); defer w.mu.Unlock(); w.fullscreened = false } +func (w *mockWindow) ToggleFullscreen() { + w.mu.Lock() + defer w.mu.Unlock() + w.fullscreened = !w.fullscreened +} + +func (w *mockWindow) ToggleMaximise() { + w.mu.Lock() + defer w.mu.Unlock() + w.maximised = !w.maximised +} + +func (w *mockWindow) ExecJS(js string) { + w.mu.Lock() + defer w.mu.Unlock() + w.execJSCalls = append(w.execJSCalls, js) +} +func (w *mockWindow) Flash(enabled bool) { w.mu.Lock(); defer w.mu.Unlock(); w.flashed = enabled } +func (w *mockWindow) Print() resultFailure { return nil } +func (w *mockWindow) OpenDevTools() { w.mu.Lock(); defer w.mu.Unlock(); w.devToolsOpen = true } +func (w *mockWindow) CloseDevTools() { w.mu.Lock(); defer w.mu.Unlock(); w.devToolsOpen = false } func (w *mockWindow) OnWindowEvent(handler func(WindowEvent)) { + w.mu.Lock() + defer w.mu.Unlock() w.eventHandlers = append(w.eventHandlers, handler) } + func (w *mockWindow) OnFileDrop(handler func(paths []string, target *DropTarget)) { + w.mu.Lock() + defer w.mu.Unlock() w.fileDropHandlers = append(w.fileDropHandlers, handler) } func (w *mockWindow) SetCloseBehavior(behavior CloseBehavior) { + w.mu.Lock() + defer w.mu.Unlock() w.closeBehavior = behavior } +// execJSCallsSnapshot is how a test reads what was executed. +// +// A copy, taken under the lock, because the slice it copies is appended to by +// whichever goroutine the Service ran ExecJS on — reading the field directly is +// the race this file exists to have fixed. +func (w *mockWindow) execJSCallsSnapshot() []string { + w.mu.Lock() + defer w.mu.Unlock() + + return append([]string(nil), w.execJSCalls...) +} + // emit fires a test event to all registered handlers. +// +// The handlers are copied under the lock and called outside it: a handler is +// free to call back into the window it was registered on, and holding the lock +// across the call would deadlock on the first one that does. func (w *mockWindow) emit(e WindowEvent) { - for _, h := range w.eventHandlers { + w.mu.Lock() + handlers := make([]func(WindowEvent), len(w.eventHandlers)) + copy(handlers, w.eventHandlers) + w.mu.Unlock() + + for _, h := range handlers { h(e) } } @@ -122,8 +249,16 @@ func (w *mockWindow) emit(e WindowEvent) { // emitFileDrop simulates a file drop on the window. Pass nil target // for legacy zero-context drops, or a DropTarget with the element // metadata the consumer expects to receive. +// +// Handlers are copied and called outside the lock, for the reason {@see emit} +// gives. func (w *mockWindow) emitFileDrop(paths []string, target *DropTarget) { - for _, h := range w.fileDropHandlers { + w.mu.Lock() + handlers := make([]func(paths []string, target *DropTarget), len(w.fileDropHandlers)) + copy(handlers, w.fileDropHandlers) + w.mu.Unlock() + + for _, h := range handlers { h(paths, target) } } @@ -135,7 +270,8 @@ func (w *mockWindow) emitFileDrop(paths []string, target *DropTarget) { // app.Event.On (Wails-only). type recordingBinder struct { mockPlatform - bindings []recordedBinding + bindingsMu sync.Mutex + bindings []recordedBinding } type recordedBinding struct { @@ -145,5 +281,7 @@ type recordedBinding struct { // BindCustomEvent satisfies CustomEventBinder. func (r *recordingBinder) BindCustomEvent(name string, cb func(data any)) { + r.bindingsMu.Lock() + defer r.bindingsMu.Unlock() r.bindings = append(r.bindings, recordedBinding{name: name, cb: cb}) } diff --git a/go/display/webkit/pkg/window/service_test.go b/go/display/webkit/pkg/window/service_test.go index 65cab63..a3e458a 100644 --- a/go/display/webkit/pkg/window/service_test.go +++ b/go/display/webkit/pkg/window/service_test.go @@ -717,8 +717,7 @@ func TestTaskExecJS_Good(t *core.T) { pw, ok := svc.Manager().Get("test") core.RequireTrue(t, ok) - mw := pw.(*mockWindow) - core.AssertContains(t, mw.execJSCalls, "document.title = 'Ready'") + core.AssertContains(t, pw.(*mockWindow).execJSCallsSnapshot(), "document.title = 'Ready'") } func TestTaskExecJS_Bad(t *core.T) { @@ -756,9 +755,11 @@ func TestTaskEvalJS_Good(t *core.T) { if !ok { continue } - mw := pw.(*mockWindow) - if len(mw.execJSCalls) > 0 { - reqID = extractEvalReqID(mw.execJSCalls[len(mw.execJSCalls)-1]) + // Through the snapshot, not the field: taskEvalJS is running on the + // goroutine above and appending to that slice as this loop reads it. + calls := pw.(*mockWindow).execJSCallsSnapshot() + if len(calls) > 0 { + reqID = extractEvalReqID(calls[len(calls)-1]) } } core.RequireTrue(t, reqID != "")