Merge branch 'nc/abandon-frame': discard late frame callbacks after close
2 files · +6 -8
M surface.go +4 -4
| @@ -724,9 +724,6 @@ func (s *Surface) Close() error { | |||
| 724 | 724 | } | |
| 725 | 725 | ||
| 726 | 726 | func (s *Surface) destroy() error { | |
| 727 | - | // TODO(wlturbo): a frame callback pending here is unregistered locally, so | |
| 728 | - | // its later done/delete_id reaches an unknown object. Fixing it needs | |
| 729 | - | // server-side delete_id handling in wlturbo (see TestCloseWithPendingFrame). | |
| 730 | 727 | s.closed = true | |
| 731 | 728 | delete(s.c.surfaces, s.id) | |
| 732 | 729 | s.c.seatSurfaceGone(s) | |
| @@ -772,7 +769,10 @@ func (s *Surface) destroy() error { | |||
| 772 | 769 | add(s.surf.Destroy()) | |
| 773 | 770 | } | |
| 774 | 771 | if s.cb != nil { | |
| 775 | - | s.c.wlctx.Unregister(s.cb) | |
| 772 | + | // A pending frame callback is owned by the compositor: abandon it so | |
| 773 | + | // its late done and delete_id are discarded. An error only means no | |
| 774 | + | // callback was pending. | |
| 775 | + | _ = s.c.wlctx.Abandon(s.cb) | |
| 776 | 776 | } | |
| 777 | 777 | return errors.Join(errs...) | |
| 778 | 778 | } | |
| @@ -724,9 +724,6 @@ func (s *Surface) Close() error { | |||||
| 724 | } | 724 | } | ||
| 725 | 725 | ||||
| 726 | func (s *Surface) destroy() error { | 726 | func (s *Surface) destroy() error { | ||
| 727 | - | // TODO(wlturbo): a frame callback pending here is unregistered locally, so | |||
| 728 | - | // its later done/delete_id reaches an unknown object. Fixing it needs | |||
| 729 | - | // server-side delete_id handling in wlturbo (see TestCloseWithPendingFrame). | |||
| 730 | s.closed = true | 727 | s.closed = true | ||
| 731 | delete(s.c.surfaces, s.id) | 728 | delete(s.c.surfaces, s.id) | ||
| 732 | s.c.seatSurfaceGone(s) | 729 | s.c.seatSurfaceGone(s) | ||
| @@ -772,7 +769,10 @@ func (s *Surface) destroy() error { | |||||
| 772 | add(s.surf.Destroy()) | 769 | add(s.surf.Destroy()) | ||
| 773 | } | 770 | } | ||
| 774 | if s.cb != nil { | 771 | if s.cb != nil { | ||
| 775 | - | s.c.wlctx.Unregister(s.cb) | 772 | + | // A pending frame callback is owned by the compositor: abandon it so |
| 773 | + | // its late done and delete_id are discarded. An error only means no | |||
| 774 | + | // callback was pending. | |||
| 775 | + | _ = s.c.wlctx.Abandon(s.cb) | |||
| 776 | } | 776 | } | ||
| 777 | return errors.Join(errs...) | 777 | return errors.Join(errs...) | ||
| 778 | } | 778 | } | ||
M surface_wire_test.go +2 -4
| @@ -164,11 +164,9 @@ func TestLockCloseDestroysUnlockedLock(t *testing.T) { | |||
| 164 | 164 | require.NoError(t, l.Close(), "idempotent") | |
| 165 | 165 | } | |
| 166 | 166 | ||
| 167 | - | // GO-002 reproduction, enabled once wlturbo handles delete_id for objects the | |
| 168 | - | // server creates: closing a surface with a pending frame callback must not turn | |
| 169 | - | // the compositor's later callback done and delete_id into a transport error. | |
| 167 | + | // Closing a surface with a pending frame callback must not turn the | |
| 168 | + | // compositor's later callback done and delete_id into a transport error. | |
| 170 | 169 | func TestCloseWithPendingFrame(t *testing.T) { | |
| 171 | - | t.Skip("needs wlturbo server-side delete_id handling") | |
| 172 | 170 | c, srv := connectWire(t, surfaceGlobals, surfaceOutputs) | |
| 173 | 171 | s, err := c.NewToplevel("close", 64, 48) | |
| 174 | 172 | require.NoError(t, err) | |
| @@ -164,11 +164,9 @@ func TestLockCloseDestroysUnlockedLock(t *testing.T) { | |||||
| 164 | require.NoError(t, l.Close(), "idempotent") | 164 | require.NoError(t, l.Close(), "idempotent") | ||
| 165 | } | 165 | } | ||
| 166 | 166 | ||||
| 167 | - | // GO-002 reproduction, enabled once wlturbo handles delete_id for objects the | 167 | + | // Closing a surface with a pending frame callback must not turn the |
| 168 | - | // server creates: closing a surface with a pending frame callback must not turn | 168 | + | // compositor's later callback done and delete_id into a transport error. |
| 169 | - | // the compositor's later callback done and delete_id into a transport error. | |||
| 170 | func TestCloseWithPendingFrame(t *testing.T) { | 169 | func TestCloseWithPendingFrame(t *testing.T) { | ||
| 171 | - | t.Skip("needs wlturbo server-side delete_id handling") | |||
| 172 | c, srv := connectWire(t, surfaceGlobals, surfaceOutputs) | 170 | c, srv := connectWire(t, surfaceGlobals, surfaceOutputs) | ||
| 173 | s, err := c.NewToplevel("close", 64, 48) | 171 | s, err := c.NewToplevel("close", 64, 48) | ||
| 174 | require.NoError(t, err) | 172 | require.NoError(t, err) | ||