fix(surface): reject popup grabs without a grabbing parent

Track the open grab chain per connection: a grab needs a grabbing parent
popup and, while a chain is open, its topmost popup. NewPopup also
refuses unconfigured parents, reports the seat bind error as the cause of
a missing seat and keeps positioner destroy errors.

commit: 2671432a50649fce0a897c3f62e4aa99d0283364
author: brice · 13d ago · 2026-10-09 03:55 UTC
parent: 1bb0040
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

5 files · +247 -53

M conn.go +2 -1


@@ -83,7 +83,8 @@ type Conn struct {
83	83		curLock *Lock // the live session lock, if any
84	84		tableBuf []byte
85	85		seat *Seat
86		-	seatErr error // a seat bind failure, reported by the next Dispatch
	86	+	seatErr error // a seat bind failure, reported by the next Dispatch
	87	+	grabTop *Surface // the topmost popup of the open grab chain, nil without one
87	88
88	89		ready bool // Connect finished; announcements go to the Handler
89	90		dispatching bool


@@ -83,7 +83,8 @@ type Conn struct {
83		curLock *Lock // the live session lock, if any	83		curLock *Lock // the live session lock, if any
84		tableBuf []byte	84		tableBuf []byte
85		seat *Seat	85		seat *Seat
86	-	seatErr error // a seat bind failure, reported by the next Dispatch	86	+	seatErr error // a seat bind failure, reported by the next Dispatch
			87	+	grabTop *Surface // the topmost popup of the open grab chain, nil without one
87			88
88		ready bool // Connect finished; announcements go to the Handler	89		ready bool // Connect finished; announcements go to the Handler
89		dispatching bool	90		dispatching bool

M export_test.go +3 -0


@@ -39,3 +39,6 @@ func (s *Surface) MarkPresented() { s.presented = true }
39	39
40	40		// DropFrameCallback marks the pending frame callback as answered.
41	41		func (s *Surface) PendingFrame() bool { return !s.frameReady }
	42	+
	43	+	// SetSeatErr stands in for a failed wl_seat bind.
	44	+	func (c *Conn) SetSeatErr(err error) { c.seatErr = err }


@@ -39,3 +39,6 @@ func (s *Surface) MarkPresented() { s.presented = true }
39		39
40	// DropFrameCallback marks the pending frame callback as answered.	40		// DropFrameCallback marks the pending frame callback as answered.
41	func (s *Surface) PendingFrame() bool { return !s.frameReady }	41		func (s *Surface) PendingFrame() bool { return !s.frameReady }
		42	+
		43	+	// SetSeatErr stands in for a failed wl_seat bind.
		44	+	func (c *Conn) SetSeatErr(err error) { c.seatErr = err }

M popup.go +42 -9


@@ -95,8 +95,7 @@ func (c Conn) newPositioner(p PopupPlacement) (*xdgshell.XdgPositioner, error)
95	95		pos.SetOffset(p.Offset[0], p.Offset[1]),
96	96		} {
97	97		if e != nil {
98		-	_ = pos.Destroy()
99		-	return nil, e
	98	+	return nil, errors.Join(e, pos.Destroy())
100	99		}
101	100		}
102	101		return pos, nil

@@ -108,6 +107,16 @@ func (c Conn) newPositioner(p PopupPlacement) (*xdgshell.XdgPositioner, error)
108	107		// compositor dismisses the popup on an outside click or when the grab is
109	108		// denied, and the popup must then be closed.
110	109		//
	110	+	// The parent must be mapped: configured by the compositor and, for the
	111	+	// compositor to place the popup, presented at least once. NewPopup returns an
	112	+	// error for a parent that has not been configured yet; a compositor dismisses
	113	+	// (PopupDone) a popup whose parent shows nothing.
	114	+	//
	115	+	// A grab needs a seat and the serial of the user action. Its parent must be a
	116	+	// toplevel or layer surface, or the grabbing popup that is the topmost of the
	117	+	// connection's grab chain; only one chain exists at a time. Other grabs return
	118	+	// an error before any request is sent (xdg-shell would raise a protocol error).
	119	+	//
111	120		// Close popups before their parent: Close of a surface with open child popups
112	121		// fails.
113	122		func (c Conn) NewPopup(cfg PopupConfig) (Surface, error) {

@@ -122,6 +131,8 @@ func (c Conn) NewPopup(cfg PopupConfig) (Surface, error) {
122	131		return nil, fmt.Errorf("neferclient: popup parent: %w", ErrClosed)
123	132		case parent.role != roleLayer && parent.role != roleToplevel && parent.role != rolePopup:
124	133		return nil, errors.New("neferclient: popup parent must be a layer surface, a toplevel or a popup")
	134	+	case !parent.configured:
	135	+	return nil, errors.New("neferclient: popup parent is not configured yet: wait for its first Handler.Configure")
125	136		}
126	137		if err := validatePlacement(&cfg.Place); err != nil {
127	138		return nil, err

@@ -132,7 +143,17 @@ func (c Conn) NewPopup(cfg PopupConfig) (Surface, error) {
132	143		}
133	144		c.ensureSeat()
134	145		if c.seat == nil \|\| c.seat.proxy == nil {
135		-	return nil, &CapabilityError{Name: "wl_seat", Cause: errors.New("popup grab needs a seat")}
	146	+	cause := c.seatErr
	147	+	if cause == nil {
	148	+	cause = errors.New("popup grab needs a seat")
	149	+	}
	150	+	return nil, &CapabilityError{Name: "wl_seat", Cause: cause}
	151	+	}
	152	+	if parent.role == rolePopup && !parent.grabbed {
	153	+	return nil, errors.New("neferclient: popup grab needs a grabbing parent popup")
	154	+	}
	155	+	if c.grabTop != nil && parent != c.grabTop {
	156	+	return nil, errors.New("neferclient: popup grab needs the topmost grabbing popup as parent while a grab is open")
136	157		}
137	158		}
138	159		if err := c.ensureXdg(); err != nil {

@@ -162,8 +183,8 @@ func (c Conn) NewPopup(cfg PopupConfig) (Surface, error) {
162	183		parentXdg = parent.xdg
163	184		}
164	185		s.popup, err = s.xdg.GetPopup(parentXdg, pos)
165		-	_ = pos.Destroy() // the compositor copies the state at get_popup
166		-	if err != nil {
	186	+	// The compositor copies the state at get_popup.
	187	+	if err = errors.Join(err, pos.Destroy()); err != nil {
167	188		return nil, err
168	189		}
169	190		if parent.role == roleLayer {

@@ -191,6 +212,7 @@ func (c Conn) NewPopup(cfg PopupConfig) (Surface, error) {
191	212		if err = s.popup.Grab(c.seat.proxy, cfg.Serial); err != nil {
192	213		return nil, err
193	214		}
	215	+	s.grabbed, c.grabTop = true, s
194	216		}
195	217		if err = s.finishSurface(); err != nil {
196	218		return nil, err

@@ -223,9 +245,20 @@ func (s *Surface) Reposition(p PopupPlacement) error {
223	245		return err
224	246		}
225	247		s.repoToken++
226		-	err = s.popup.Reposition(pos, s.repoToken)
227		-	if e := pos.Destroy(); err == nil {
228		-	err = e
	248	+	return errors.Join(s.popup.Reposition(pos, s.repoToken), pos.Destroy())
	249	+	}
	250	+
	251	+	// dropGrab ends the surface's grab: the topmost grabbing popup becomes its
	252	+	// parent, if that one still grabs.
	253	+	func (s *Surface) dropGrab() {
	254	+	if !s.grabbed {
	255	+	return
	256	+	}
	257	+	s.grabbed = false
	258	+	if c := s.c; c.grabTop == s {
	259	+	c.grabTop = nil
	260	+	if p := s.parent; p != nil && p.grabbed {
	261	+	c.grabTop = p
	262	+	}
229	263		}
230		-	return err
231	264		}


@@ -95,8 +95,7 @@ func (c Conn) newPositioner(p PopupPlacement) (*xdgshell.XdgPositioner, error)
95		pos.SetOffset(p.Offset[0], p.Offset[1]),	95		pos.SetOffset(p.Offset[0], p.Offset[1]),
96		} {	96		} {
97		if e != nil {	97		if e != nil {
98	-	_ = pos.Destroy()	98	+	return nil, errors.Join(e, pos.Destroy())
99	-	return nil, e
100		}	99		}
101		}	100		}
102		return pos, nil	101		return pos, nil

@@ -108,6 +107,16 @@ func (c Conn) newPositioner(p PopupPlacement) (*xdgshell.XdgPositioner, error)
108		// compositor dismisses the popup on an outside click or when the grab is	107		// compositor dismisses the popup on an outside click or when the grab is
109		// denied, and the popup must then be closed.	108		// denied, and the popup must then be closed.
110		//	109		//
			110	+	// The parent must be mapped: configured by the compositor and, for the
			111	+	// compositor to place the popup, presented at least once. NewPopup returns an
			112	+	// error for a parent that has not been configured yet; a compositor dismisses
			113	+	// (PopupDone) a popup whose parent shows nothing.
			114	+	//
			115	+	// A grab needs a seat and the serial of the user action. Its parent must be a
			116	+	// toplevel or layer surface, or the grabbing popup that is the topmost of the
			117	+	// connection's grab chain; only one chain exists at a time. Other grabs return
			118	+	// an error before any request is sent (xdg-shell would raise a protocol error).
			119	+	//
111		// Close popups before their parent: Close of a surface with open child popups	120		// Close popups before their parent: Close of a surface with open child popups
112		// fails.	121		// fails.
113		func (c Conn) NewPopup(cfg PopupConfig) (Surface, error) {	122		func (c Conn) NewPopup(cfg PopupConfig) (Surface, error) {

@@ -122,6 +131,8 @@ func (c Conn) NewPopup(cfg PopupConfig) (Surface, error) {
122		return nil, fmt.Errorf("neferclient: popup parent: %w", ErrClosed)	131		return nil, fmt.Errorf("neferclient: popup parent: %w", ErrClosed)
123		case parent.role != roleLayer && parent.role != roleToplevel && parent.role != rolePopup:	132		case parent.role != roleLayer && parent.role != roleToplevel && parent.role != rolePopup:
124		return nil, errors.New("neferclient: popup parent must be a layer surface, a toplevel or a popup")	133		return nil, errors.New("neferclient: popup parent must be a layer surface, a toplevel or a popup")
			134	+	case !parent.configured:
			135	+	return nil, errors.New("neferclient: popup parent is not configured yet: wait for its first Handler.Configure")
125		}	136		}
126		if err := validatePlacement(&cfg.Place); err != nil {	137		if err := validatePlacement(&cfg.Place); err != nil {
127		return nil, err	138		return nil, err

@@ -132,7 +143,17 @@ func (c Conn) NewPopup(cfg PopupConfig) (Surface, error) {
132		}	143		}
133		c.ensureSeat()	144		c.ensureSeat()
134		if c.seat == nil \|\| c.seat.proxy == nil {	145		if c.seat == nil \|\| c.seat.proxy == nil {
135	-	return nil, &CapabilityError{Name: "wl_seat", Cause: errors.New("popup grab needs a seat")}	146	+	cause := c.seatErr
			147	+	if cause == nil {
			148	+	cause = errors.New("popup grab needs a seat")
			149	+	}
			150	+	return nil, &CapabilityError{Name: "wl_seat", Cause: cause}
			151	+	}
			152	+	if parent.role == rolePopup && !parent.grabbed {
			153	+	return nil, errors.New("neferclient: popup grab needs a grabbing parent popup")
			154	+	}
			155	+	if c.grabTop != nil && parent != c.grabTop {
			156	+	return nil, errors.New("neferclient: popup grab needs the topmost grabbing popup as parent while a grab is open")
136		}	157		}
137		}	158		}
138		if err := c.ensureXdg(); err != nil {	159		if err := c.ensureXdg(); err != nil {

@@ -162,8 +183,8 @@ func (c Conn) NewPopup(cfg PopupConfig) (Surface, error) {
162		parentXdg = parent.xdg	183		parentXdg = parent.xdg
163		}	184		}
164		s.popup, err = s.xdg.GetPopup(parentXdg, pos)	185		s.popup, err = s.xdg.GetPopup(parentXdg, pos)
165	-	_ = pos.Destroy() // the compositor copies the state at get_popup	186	+	// The compositor copies the state at get_popup.
166	-	if err != nil {	187	+	if err = errors.Join(err, pos.Destroy()); err != nil {
167		return nil, err	188		return nil, err
168		}	189		}
169		if parent.role == roleLayer {	190		if parent.role == roleLayer {

@@ -191,6 +212,7 @@ func (c Conn) NewPopup(cfg PopupConfig) (Surface, error) {
191		if err = s.popup.Grab(c.seat.proxy, cfg.Serial); err != nil {	212		if err = s.popup.Grab(c.seat.proxy, cfg.Serial); err != nil {
192		return nil, err	213		return nil, err
193		}	214		}
			215	+	s.grabbed, c.grabTop = true, s
194		}	216		}
195		if err = s.finishSurface(); err != nil {	217		if err = s.finishSurface(); err != nil {
196		return nil, err	218		return nil, err

@@ -223,9 +245,20 @@ func (s *Surface) Reposition(p PopupPlacement) error {
223		return err	245		return err
224		}	246		}
225		s.repoToken++	247		s.repoToken++
226	-	err = s.popup.Reposition(pos, s.repoToken)	248	+	return errors.Join(s.popup.Reposition(pos, s.repoToken), pos.Destroy())
227	-	if e := pos.Destroy(); err == nil {	249	+	}
228	-	err = e	250	+
			251	+	// dropGrab ends the surface's grab: the topmost grabbing popup becomes its
			252	+	// parent, if that one still grabs.
			253	+	func (s *Surface) dropGrab() {
			254	+	if !s.grabbed {
			255	+	return
			256	+	}
			257	+	s.grabbed = false
			258	+	if c := s.c; c.grabTop == s {
			259	+	c.grabTop = nil
			260	+	if p := s.parent; p != nil && p.grabbed {
			261	+	c.grabTop = p
			262	+	}
229		}	263		}
230	-	return err
231		}	264		}

M popup_wire_test.go +197 -43


@@ -2,6 +2,9 @@ package neferclient_test
2	2
3	3		import (
4	4		"encoding/binary"
	5	+	"errors"
	6	+	"math"
	7	+	"slices"
5	8		"sync"
6	9		"testing"
7	10

@@ -63,6 +66,28 @@ func (l *popupLog) find(from int, match func(popupReq) bool) int {
63	66		return -1
64	67		}
65	68
	69	+	// findLast returns the index of the last request that matches, or -1. Object
	70	+	// ids are recycled once the compositor confirms a destruction, so a lookup of
	71	+	// a live object must take the newest request.
	72	+	func (l *popupLog) findLast(match func(popupReq) bool) int {
	73	+	l.mu.Lock()
	74	+	defer l.mu.Unlock()
	75	+	for i := len(l.reqs) - 1; i >= 0; i-- {
	76	+	if match(l.reqs[i]) {
	77	+	return i
	78	+	}
	79	+	}
	80	+	return -1
	81	+	}
	82	+
	83	+	// mustFind is find that fails the test when nothing matches.
	84	+	func (l popupLog) mustFind(t testing.T, from int, match func(popupReq) bool, msg string) int {
	85	+	t.Helper()
	86	+	i := l.find(from, match)
	87	+	require.GreaterOrEqual(t, i, 0, msg)
	88	+	return i
	89	+	}
	90	+
66	91		func (l *popupLog) at(i int) popupReq {
67	92		l.mu.Lock()
68	93		defer l.mu.Unlock()

@@ -101,10 +126,13 @@ func newPopupEnv(t testing.T, globals []wireGlobal) popupEnv {
101	126		var err error
102	127		e.top, err = c.NewToplevel("parent", 64, 48)
103	128		require.NoError(t, err)
104		-	if len(globals) > 8 {
	129	+	hasLayer := slices.ContainsFunc(globals, func(g wireGlobal) bool { return g.iface == "zwlr_layer_shell_v1" })
	130	+	e.top.MarkPresentable(64, 48) // stands in for the configure round: popups need a configured parent
	131	+	if hasLayer {
105	132		e.layer, err = c.NewLayerSurface(neferclient.LayerConfig{Level: neferclient.LayerTop,
106	133		Anchors: neferclient.AnchorTop \| neferclient.AnchorLeft, Width: 200, Height: 30})
107	134		require.NoError(t, err)
	135	+	e.layer.MarkPresentable(200, 30)
108	136		}
109	137		require.NoError(t, c.Roundtrip())
110	138		return e

@@ -117,14 +145,43 @@ func (e *popupEnv) wmBase() uint32 {
117	145		}
118	146
119	147		// xdgSurfaceOf returns the xdg_surface id created for wl_surface surf.
	148	+	// getPopup returns the newest xdg_surface.get_popup request of xdg.
	149	+	func (e popupEnv) getPopup(t testing.T, xdg uint32) (int, popupReq) {
	150	+	t.Helper()
	151	+	i := e.log.findLast(e.log.op(xdg, 2))
	152	+	require.GreaterOrEqual(t, i, 0, "xdg_surface.get_popup")
	153	+	return i, e.log.at(i)
	154	+	}
	155	+
120	156		func (e popupEnv) xdgSurfaceOf(t testing.T, surf uint32) uint32 {
121	157		t.Helper()
122	158		wm := e.wmBase()
123		-	i := e.log.find(0, func(r popupReq) bool { return r.obj == wm && r.op == 2 && len(r.body) == 8 && r.u32(1) == surf })
	159	+	i := e.log.findLast(func(r popupReq) bool { return r.obj == wm && r.op == 2 && len(r.body) == 8 && r.u32(1) == surf })
124	160		require.GreaterOrEqual(t, i, 0, "xdg_wm_base.get_xdg_surface")
125	161		return e.log.at(i).u32(0)
126	162		}
127	163
	164	+	// configurePopup has the peer configure popup p and dispatches it, which
	165	+	// makes p a valid parent for further popups.
	166	+	func (e popupEnv) configurePopup(t testing.T, p *neferclient.Surface) {
	167	+	t.Helper()
	168	+	require.NoError(t, e.c.Roundtrip())
	169	+	xdg := e.xdgSurfaceOf(t, p.SurfaceObjectID())
	170	+	_, gp := e.getPopup(t, xdg)
	171	+	e.srv.write(frame(gp.u32(0), 0, appendU32(appendU32(appendU32(appendU32(nil, 0), 0), 60), 40)))
	172	+	e.srv.write(frame(xdg, 0, appendU32(nil, 1)))
	173	+	require.NoError(t, e.c.Roundtrip())
	174	+	dispatchUntil(t, e.c, neferclient.NopHandler{}, func() bool { w, _, _ := p.Size(); return w == 60 })
	175	+	require.NoError(t, e.c.Roundtrip()) // the ack_configure reached the peer
	176	+	}
	177	+
	178	+	// popupID returns the xdg_popup object id of p.
	179	+	func (e popupEnv) popupID(t testing.T, p *neferclient.Surface) uint32 {
	180	+	t.Helper()
	181	+	_, gp := e.getPopup(t, e.xdgSurfaceOf(t, p.SurfaceObjectID()))
	182	+	return gp.u32(0)
	183	+	}
	184	+
128	185		var place = neferclient.PopupPlacement{
129	186		Anchor: neferclient.Rect{X: 10, Y: 0, Width: 20, Height: 30},
130	187		Edge: neferclient.EdgeBottom, Gravity: neferclient.EdgeBottomRight,

@@ -141,29 +198,26 @@ func TestPopupOnLayerParentGrabsBeforeFirstCommit(t *testing.T) {
141	198
142	199		popupSurf := p.SurfaceObjectID()
143	200		xdg := e.xdgSurfaceOf(t, popupSurf)
144		-	getPopup := e.log.find(n, e.log.op(xdg, 2))
145		-	require.GreaterOrEqual(t, getPopup, 0, "xdg_surface.get_popup")
146		-	gp := e.log.at(getPopup)
	201	+	getPopup, gp := e.getPopup(t, xdg)
147	202		popupID, positioner := gp.u32(0), gp.u32(2)
148	203		require.Zero(t, gp.u32(1), "layer parent: null xdg parent")
149	204
150	205		// The layer surface is the one created for e.layer.
151		-	layerID := e.log.at(e.log.find(0, func(r popupReq) bool { return r.op == 0 && len(r.body) > 8 && r.u32(1) == e.layer.SurfaceObjectID() })).u32(0)
152		-	layerPopup := e.log.find(n, e.log.op(layerID, 5))
	206	+	layerID := e.log.at(e.log.mustFind(t, 0, func(r popupReq) bool { return r.op == 0 && len(r.body) > 8 && r.u32(1) == e.layer.SurfaceObjectID() }, "get_layer_surface")).u32(0)
	207	+	layerPopup := e.log.mustFind(t, n, e.log.op(layerID, 5), "layer_surface.get_popup")
153	208		require.Greater(t, layerPopup, getPopup, "layer_surface.get_popup follows xdg_surface.get_popup")
154	209		require.Equal(t, popupID, e.log.at(layerPopup).u32(0))
155	210
156		-	grab := e.log.find(n, e.log.op(popupID, 1))
	211	+	grab := e.log.mustFind(t, n, e.log.op(popupID, 1), "xdg_popup.grab")
157	212		require.Greater(t, grab, layerPopup, "grab follows get_popup")
158	213		require.Equal(t, uint32(42), e.log.at(grab).u32(1))
159	214		require.NotZero(t, e.log.at(grab).u32(0), "seat")
160		-	commit := e.log.find(n, e.log.op(popupSurf, 6))
	215	+	commit := e.log.mustFind(t, n, e.log.op(popupSurf, 6), "wl_surface.commit")
161	216		require.Greater(t, commit, grab, "grab precedes the first commit")
162	217
163	218		// Positioner state.
164	219		pos := func(op uint16) popupReq {
165		-	i := e.log.find(n, e.log.op(positioner, op))
166		-	require.GreaterOrEqual(t, i, 0, "positioner op %d", op)
	220	+	i := e.log.mustFind(t, n, e.log.op(positioner, op), "positioner request")
167	221		require.Less(t, i, getPopup)
168	222		return e.log.at(i)
169	223		}

@@ -184,10 +238,9 @@ func TestPopupOnToplevelParentUsesParentXdgSurface(t *testing.T) {
184	238		require.NoError(t, e.c.Roundtrip())
185	239		parentXdg := e.xdgSurfaceOf(t, e.top.SurfaceObjectID())
186	240		xdg := e.xdgSurfaceOf(t, p.SurfaceObjectID())
187		-	i := e.log.find(n, e.log.op(xdg, 2))
188		-	require.GreaterOrEqual(t, i, 0)
189		-	require.Equal(t, parentXdg, e.log.at(i).u32(1))
190		-	popupID := e.log.at(i).u32(0)
	241	+	_, gp := e.getPopup(t, xdg)
	242	+	require.Equal(t, parentXdg, gp.u32(1))
	243	+	popupID := gp.u32(0)
191	244		require.Negative(t, e.log.find(n, e.log.op(popupID, 1)), "no grab requested")
192	245		}
193	246

@@ -195,11 +248,14 @@ func TestPopupNestedOnPopup(t *testing.T) {
195	248		e := newPopupEnv(t, popupGlobals)
196	249		p, err := e.c.NewPopup(neferclient.PopupConfig{Parent: e.top, Place: place})
197	250		require.NoError(t, err)
	251	+	_, err = e.c.NewPopup(neferclient.PopupConfig{Parent: p, Place: place})
	252	+	require.ErrorContains(t, err, "not configured yet", "a popup parent is mapped only once configured")
	253	+	e.configurePopup(t, p)
198	254		child, err := e.c.NewPopup(neferclient.PopupConfig{Parent: p, Place: place})
199	255		require.NoError(t, err)
200	256		require.NoError(t, e.c.Roundtrip())
201		-	i := e.log.find(0, e.log.op(e.xdgSurfaceOf(t, child.SurfaceObjectID()), 2))
202		-	require.Equal(t, e.xdgSurfaceOf(t, p.SurfaceObjectID()), e.log.at(i).u32(1))
	257	+	_, gp := e.getPopup(t, e.xdgSurfaceOf(t, child.SurfaceObjectID()))
	258	+	require.Equal(t, e.xdgSurfaceOf(t, p.SurfaceObjectID()), gp.u32(1))
203	259		require.Error(t, p.Close(), "popup with an open child")
204	260		require.NoError(t, child.Close())
205	261		require.NoError(t, p.Close())

@@ -211,7 +267,8 @@ func TestPopupConfigureAcksAndDone(t *testing.T) {
211	267		require.NoError(t, err)
212	268		require.NoError(t, e.c.Roundtrip())
213	269		xdg := e.xdgSurfaceOf(t, p.SurfaceObjectID())
214		-	popupID := e.log.at(e.log.find(0, e.log.op(xdg, 2))).u32(0)
	270	+	_, gp := e.getPopup(t, xdg)
	271	+	popupID := gp.u32(0)
215	272
216	273		var w, h int32
217	274		var done bool

@@ -226,8 +283,7 @@ func TestPopupConfigureAcksAndDone(t *testing.T) {
226	283		dispatchUntil(t, e.c, hd, func() bool { return w != 0 })
227	284		require.Equal(t, [2]int32{120, 80}, [2]int32{w, h})
228	285		require.NoError(t, e.c.Roundtrip())
229		-	ack := e.log.find(0, e.log.op(xdg, 4))
230		-	require.GreaterOrEqual(t, ack, 0, "ack_configure")
	286	+	ack := e.log.mustFind(t, 0, e.log.op(xdg, 4), "ack_configure")
231	287		require.Equal(t, uint32(9), e.log.at(ack).u32(0))
232	288		gw, gh, _ := p.Size()
233	289		require.Equal(t, [2]int32{120, 80}, [2]int32{gw, gh})

@@ -237,9 +293,8 @@ func TestPopupConfigureAcksAndDone(t *testing.T) {
237	293		dispatchUntil(t, e.c, hd, func() bool { return done })
238	294		require.NoError(t, p.Close())
239	295		require.NoError(t, e.c.Roundtrip())
240		-	require.NotEmpty(t, e.log.all(e.log.op(popupID, 0)), "xdg_popup.destroy")
241		-	destroyPopup := e.log.find(0, e.log.op(popupID, 0))
242		-	destroyXdg := e.log.find(0, e.log.op(xdg, 0))
	296	+	destroyPopup := e.log.mustFind(t, 0, e.log.op(popupID, 0), "xdg_popup.destroy")
	297	+	destroyXdg := e.log.mustFind(t, 0, e.log.op(xdg, 0), "xdg_surface.destroy")
243	298		require.Less(t, destroyPopup, destroyXdg, "popup destroyed before its xdg_surface")
244	299		}
245	300

@@ -249,19 +304,27 @@ func TestPopupReposition(t *testing.T) {
249	304		require.NoError(t, err)
250	305		require.NoError(t, e.c.Roundtrip())
251	306		xdg := e.xdgSurfaceOf(t, p.SurfaceObjectID())
252		-	popupID := e.log.at(e.log.find(0, e.log.op(xdg, 2))).u32(0)
	307	+	_, gp := e.getPopup(t, xdg)
	308	+	popupID := gp.u32(0)
253	309
254	310		np := place
255	311		np.Anchor.X, np.Width = 50, 90
	312	+	n := e.log.len()
256	313		require.NoError(t, p.Reposition(np))
257	314		require.NoError(t, e.c.Roundtrip())
258		-	i := e.log.find(0, e.log.op(popupID, 2))
259		-	require.GreaterOrEqual(t, i, 0, "xdg_popup.reposition")
	315	+	i := e.log.mustFind(t, n, e.log.op(popupID, 2), "xdg_popup.reposition")
260	316		positioner := e.log.at(i).u32(0)
261		-	require.NotEqual(t, uint32(0), e.log.at(i).u32(1), "token")
262		-	setSize := e.log.at(e.log.find(i-8, e.log.op(positioner, 1)))
	317	+	token := e.log.at(i).u32(1)
	318	+	setSize := e.log.at(e.log.mustFind(t, n, e.log.op(positioner, 1), "positioner.set_size"))
263	319		require.Equal(t, uint32(90), setSize.u32(0))
264		-	require.NotEmpty(t, e.log.all(e.log.op(positioner, 0)), "positioner destroyed")
	320	+	require.Less(t, e.log.mustFind(t, n, e.log.op(positioner, 1), "set_size"), i)
	321	+	require.Greater(t, e.log.mustFind(t, i, e.log.op(positioner, 0), "positioner.destroy"), i)
	322	+
	323	+	n = e.log.len()
	324	+	require.NoError(t, p.Reposition(np))
	325	+	require.NoError(t, e.c.Roundtrip())
	326	+	second := e.log.at(e.log.mustFind(t, n, e.log.op(popupID, 2), "second xdg_popup.reposition"))
	327	+	require.NotEqual(t, token, second.u32(1), "each reposition carries its own token")
265	328
266	329		bad := np
267	330		bad.Width = 0

@@ -289,6 +352,17 @@ func TestPopupInvalidConfigsSendNothing(t *testing.T) {
289	352		closed, err := e.c.NewToplevel("closed", 64, 48)
290	353		require.NoError(t, err)
291	354		require.NoError(t, closed.Close())
	355	+	unconfigured, err := e.c.NewToplevel("unconfigured", 64, 48)
	356	+	require.NoError(t, err)
	357	+	l, err := e.c.Lock()
	358	+	require.NoError(t, err)
	359	+	lockSurf, err := l.NewSurface(7)
	360	+	require.NoError(t, err)
	361	+	lockSurf.MarkPresentable(64, 48)
	362	+	other, _ := connectWire(t, surfaceGlobals, surfaceOutputs)
	363	+	foreign, err := other.NewToplevel("foreign", 64, 48)
	364	+	require.NoError(t, err)
	365	+	foreign.MarkPresentable(64, 48)
292	366		require.NoError(t, e.c.Roundtrip())
293	367		n := e.log.len()
294	368

@@ -297,24 +371,104 @@ func TestPopupInvalidConfigsSendNothing(t *testing.T) {
297	371		f(&cfg)
298	372		return cfg
299	373		}
300		-	for name, cfg := range map[string]neferclient.PopupConfig{
301		-	"nil parent": with(func(c *neferclient.PopupConfig) { c.Parent = nil }),
302		-	"closed parent": with(func(c *neferclient.PopupConfig) { c.Parent = closed }),
303		-	"zero width": with(func(c *neferclient.PopupConfig) { c.Place.Width = 0 }),
304		-	"negative height": with(func(c *neferclient.PopupConfig) { c.Place.Height = -1 }),
305		-	"empty anchor": with(func(c *neferclient.PopupConfig) { c.Place.Anchor.Width = 0 }),
306		-	"bad edge": with(func(c *neferclient.PopupConfig) { c.Place.Edge = 9 }),
307		-	"bad gravity": with(func(c *neferclient.PopupConfig) { c.Place.Gravity = 9 }),
308		-	"bad adjust": with(func(c *neferclient.PopupConfig) { c.Place.Adjust = 64 }),
309		-	"grab no serial": with(func(c *neferclient.PopupConfig) { c.Grab = true }),
	374	+	var capErr *neferclient.CapabilityError
	375	+	for name, tc := range map[string]struct {
	376	+	cfg neferclient.PopupConfig
	377	+	want func(error)
	378	+	}{
	379	+	"nil parent": {with(func(c *neferclient.PopupConfig) { c.Parent = nil }), func(err error) { require.ErrorContains(t, err, "needs a parent surface") }},
	380	+	"foreign parent": {with(func(c *neferclient.PopupConfig) { c.Parent = foreign }), func(err error) { require.ErrorContains(t, err, "of this connection") }},
	381	+	"closed parent": {with(func(c *neferclient.PopupConfig) { c.Parent = closed }), func(err error) { require.ErrorIs(t, err, neferclient.ErrClosed) }},
	382	+	"lock parent": {with(func(c *neferclient.PopupConfig) { c.Parent = lockSurf }), func(err error) { require.ErrorContains(t, err, "must be a layer surface, a toplevel or a popup") }},
	383	+	"unconfigured": {with(func(c *neferclient.PopupConfig) { c.Parent = unconfigured }), func(err error) { require.ErrorContains(t, err, "not configured yet") }},
	384	+	"zero width": {with(func(c *neferclient.PopupConfig) { c.Place.Width = 0 }), func(err error) { require.ErrorContains(t, err, "invalid popup size") }},
	385	+	"negative height": {with(func(c *neferclient.PopupConfig) { c.Place.Height = -1 }), func(err error) { require.ErrorContains(t, err, "invalid popup size") }},
	386	+	"empty anchor": {with(func(c *neferclient.PopupConfig) { c.Place.Anchor.Width = 0 }), func(err error) { require.ErrorContains(t, err, "anchor rectangle") }},
	387	+	"anchor overflow": {with(func(c *neferclient.PopupConfig) {
	388	+	c.Place.Anchor = neferclient.Rect{X: math.MaxInt32, Width: 10, Height: 5}
	389	+	}), func(err error) { require.ErrorContains(t, err, "overflows") }},
	390	+	"bad edge": {with(func(c *neferclient.PopupConfig) { c.Place.Edge = 9 }), func(err error) { require.ErrorContains(t, err, "invalid popup edge") }},
	391	+	"bad gravity": {with(func(c *neferclient.PopupConfig) { c.Place.Gravity = 9 }), func(err error) { require.ErrorContains(t, err, "invalid popup edge") }},
	392	+	"bad adjust": {with(func(c *neferclient.PopupConfig) { c.Place.Adjust = 64 }), func(err error) { require.ErrorContains(t, err, "constraint adjustment") }},
	393	+	"grab no serial": {with(func(c *neferclient.PopupConfig) { c.Grab = true }), func(err error) { require.ErrorContains(t, err, "serial") }},
	394	+	"grab no seat": {with(func(c *neferclient.PopupConfig) { c.Grab, c.Serial = true, 1 }), func(err error) { require.ErrorAs(t, err, &capErr) }},
310	395		} {
311		-	_, err := e.c.NewPopup(cfg)
	396	+	_, err := e.c.NewPopup(tc.cfg)
312	397		require.Error(t, err, name)
	398	+	tc.want(err)
313	399		}
314		-	var ce *neferclient.CapabilityError
315		-	_, err = e.c.NewPopup(with(func(c *neferclient.PopupConfig) { c.Grab, c.Serial = true, 1 }))
316		-	require.ErrorAs(t, err, &ce, "grab without a seat")
317	400		require.NoError(t, e.c.Roundtrip())
318	401		require.Equal(t, n, e.log.len(), "no request was sent")
319	402		require.NoError(t, e.top.Close(), "failed popups left no child behind")
320	403		}
	404	+
	405	+	// A failed seat bind is the cause of the grab's CapabilityError.
	406	+	func TestPopupGrabReportsSeatBindError(t *testing.T) {
	407	+	e := newPopupEnv(t, surfaceGlobals)
	408	+	bindErr := errors.New("bind refused")
	409	+	e.c.SetSeatErr(bindErr)
	410	+	_, err := e.c.NewPopup(neferclient.PopupConfig{Parent: e.top, Place: place, Grab: true, Serial: 1})
	411	+	var ce *neferclient.CapabilityError
	412	+	require.ErrorAs(t, err, &ce)
	413	+	require.Equal(t, "wl_seat", ce.Name)
	414	+	require.ErrorIs(t, err, bindErr)
	415	+	}
	416	+
	417	+	// A grab needs a grabbing parent popup, and while a grab chain is open only
	418	+	// its topmost popup can be the parent of the next grab. Refusals send nothing.
	419	+	func TestPopupGrabChain(t *testing.T) {
	420	+	e := newPopupEnv(t, popupGlobals)
	421	+	grab := func(parent neferclient.Surface) (neferclient.Surface, error) {
	422	+	return e.c.NewPopup(neferclient.PopupConfig{Parent: parent, Place: place, Grab: true, Serial: 7})
	423	+	}
	424	+	refused := func(parent *neferclient.Surface, want string) {
	425	+	t.Helper()
	426	+	require.NoError(t, e.c.Roundtrip()) // earlier requests are logged
	427	+	n := e.log.len()
	428	+	_, err := grab(parent)
	429	+	require.ErrorContains(t, err, want)
	430	+	require.NoError(t, e.c.Roundtrip())
	431	+	require.Equal(t, n, e.log.len(), "no request was sent")
	432	+	}
	433	+
	434	+	plain, err := e.c.NewPopup(neferclient.PopupConfig{Parent: e.layer, Place: place})
	435	+	require.NoError(t, err)
	436	+	e.configurePopup(t, plain)
	437	+	refused(plain, "needs a grabbing parent popup")
	438	+
	439	+	a, err := grab(e.layer)
	440	+	require.NoError(t, err)
	441	+	e.configurePopup(t, a)
	442	+	refused(e.top, "topmost grabbing popup")
	443	+	refused(e.layer, "topmost grabbing popup")
	444	+	refused(plain, "needs a grabbing parent popup")
	445	+
	446	+	b, err := grab(a)
	447	+	require.NoError(t, err)
	448	+	e.configurePopup(t, b)
	449	+	refused(a, "topmost grabbing popup")
	450	+
	451	+	// Closing the topmost popup returns the grab to its parent.
	452	+	require.NoError(t, b.Close())
	453	+	c2, err := grab(a)
	454	+	require.NoError(t, err)
	455	+	require.NoError(t, c2.Close())
	456	+	require.NoError(t, a.Close())
	457	+
	458	+	// A popup_done ends the grab of the dismissed popup, too.
	459	+	a2, err := grab(e.layer)
	460	+	require.NoError(t, err)
	461	+	refused(e.top, "topmost grabbing popup")
	462	+	e.srv.write(frame(e.popupID(t, a2), 1, nil))
	463	+	require.NoError(t, e.c.Roundtrip())
	464	+	var done []neferclient.SurfaceID
	465	+	hd := neferclientmocks.NewMockHandler(t)
	466	+	hd.EXPECT().PopupDone(mock.Anything).Run(func(id neferclient.SurfaceID) { done = append(done, id) }).Return().Once()
	467	+	dispatchUntil(t, e.c, hd, func() bool { return len(done) > 0 })
	468	+	require.Equal(t, []neferclient.SurfaceID{a2.ID()}, done)
	469	+	top, err := grab(e.top)
	470	+	require.NoError(t, err, "the chain is closed after popup_done")
	471	+	require.NoError(t, top.Close())
	472	+	require.NoError(t, a2.Close())
	473	+	require.NoError(t, plain.Close())
	474	+	}


@@ -2,6 +2,9 @@ package neferclient_test
2			2
3		import (	3		import (
4		"encoding/binary"	4		"encoding/binary"
			5	+	"errors"
			6	+	"math"
			7	+	"slices"
5		"sync"	8		"sync"
6		"testing"	9		"testing"
7			10

@@ -63,6 +66,28 @@ func (l *popupLog) find(from int, match func(popupReq) bool) int {
63		return -1	66		return -1
64		}	67		}
65			68
			69	+	// findLast returns the index of the last request that matches, or -1. Object
			70	+	// ids are recycled once the compositor confirms a destruction, so a lookup of
			71	+	// a live object must take the newest request.
			72	+	func (l *popupLog) findLast(match func(popupReq) bool) int {
			73	+	l.mu.Lock()
			74	+	defer l.mu.Unlock()
			75	+	for i := len(l.reqs) - 1; i >= 0; i-- {
			76	+	if match(l.reqs[i]) {
			77	+	return i
			78	+	}
			79	+	}
			80	+	return -1
			81	+	}
			82	+
			83	+	// mustFind is find that fails the test when nothing matches.
			84	+	func (l popupLog) mustFind(t testing.T, from int, match func(popupReq) bool, msg string) int {
			85	+	t.Helper()
			86	+	i := l.find(from, match)
			87	+	require.GreaterOrEqual(t, i, 0, msg)
			88	+	return i
			89	+	}
			90	+
66		func (l *popupLog) at(i int) popupReq {	91		func (l *popupLog) at(i int) popupReq {
67		l.mu.Lock()	92		l.mu.Lock()
68		defer l.mu.Unlock()	93		defer l.mu.Unlock()

@@ -101,10 +126,13 @@ func newPopupEnv(t testing.T, globals []wireGlobal) popupEnv {
101		var err error	126		var err error
102		e.top, err = c.NewToplevel("parent", 64, 48)	127		e.top, err = c.NewToplevel("parent", 64, 48)
103		require.NoError(t, err)	128		require.NoError(t, err)
104	-	if len(globals) > 8 {	129	+	hasLayer := slices.ContainsFunc(globals, func(g wireGlobal) bool { return g.iface == "zwlr_layer_shell_v1" })
			130	+	e.top.MarkPresentable(64, 48) // stands in for the configure round: popups need a configured parent
			131	+	if hasLayer {
105		e.layer, err = c.NewLayerSurface(neferclient.LayerConfig{Level: neferclient.LayerTop,	132		e.layer, err = c.NewLayerSurface(neferclient.LayerConfig{Level: neferclient.LayerTop,
106		Anchors: neferclient.AnchorTop \| neferclient.AnchorLeft, Width: 200, Height: 30})	133		Anchors: neferclient.AnchorTop \| neferclient.AnchorLeft, Width: 200, Height: 30})
107		require.NoError(t, err)	134		require.NoError(t, err)
			135	+	e.layer.MarkPresentable(200, 30)
108		}	136		}
109		require.NoError(t, c.Roundtrip())	137		require.NoError(t, c.Roundtrip())
110		return e	138		return e

@@ -117,14 +145,43 @@ func (e *popupEnv) wmBase() uint32 {
117		}	145		}
118			146
119		// xdgSurfaceOf returns the xdg_surface id created for wl_surface surf.	147		// xdgSurfaceOf returns the xdg_surface id created for wl_surface surf.
			148	+	// getPopup returns the newest xdg_surface.get_popup request of xdg.
			149	+	func (e popupEnv) getPopup(t testing.T, xdg uint32) (int, popupReq) {
			150	+	t.Helper()
			151	+	i := e.log.findLast(e.log.op(xdg, 2))
			152	+	require.GreaterOrEqual(t, i, 0, "xdg_surface.get_popup")
			153	+	return i, e.log.at(i)
			154	+	}
			155	+
120		func (e popupEnv) xdgSurfaceOf(t testing.T, surf uint32) uint32 {	156		func (e popupEnv) xdgSurfaceOf(t testing.T, surf uint32) uint32 {
121		t.Helper()	157		t.Helper()
122		wm := e.wmBase()	158		wm := e.wmBase()
123	-	i := e.log.find(0, func(r popupReq) bool { return r.obj == wm && r.op == 2 && len(r.body) == 8 && r.u32(1) == surf })	159	+	i := e.log.findLast(func(r popupReq) bool { return r.obj == wm && r.op == 2 && len(r.body) == 8 && r.u32(1) == surf })
124		require.GreaterOrEqual(t, i, 0, "xdg_wm_base.get_xdg_surface")	160		require.GreaterOrEqual(t, i, 0, "xdg_wm_base.get_xdg_surface")
125		return e.log.at(i).u32(0)	161		return e.log.at(i).u32(0)
126		}	162		}
127			163
			164	+	// configurePopup has the peer configure popup p and dispatches it, which
			165	+	// makes p a valid parent for further popups.
			166	+	func (e popupEnv) configurePopup(t testing.T, p *neferclient.Surface) {
			167	+	t.Helper()
			168	+	require.NoError(t, e.c.Roundtrip())
			169	+	xdg := e.xdgSurfaceOf(t, p.SurfaceObjectID())
			170	+	_, gp := e.getPopup(t, xdg)
			171	+	e.srv.write(frame(gp.u32(0), 0, appendU32(appendU32(appendU32(appendU32(nil, 0), 0), 60), 40)))
			172	+	e.srv.write(frame(xdg, 0, appendU32(nil, 1)))
			173	+	require.NoError(t, e.c.Roundtrip())
			174	+	dispatchUntil(t, e.c, neferclient.NopHandler{}, func() bool { w, _, _ := p.Size(); return w == 60 })
			175	+	require.NoError(t, e.c.Roundtrip()) // the ack_configure reached the peer
			176	+	}
			177	+
			178	+	// popupID returns the xdg_popup object id of p.
			179	+	func (e popupEnv) popupID(t testing.T, p *neferclient.Surface) uint32 {
			180	+	t.Helper()
			181	+	_, gp := e.getPopup(t, e.xdgSurfaceOf(t, p.SurfaceObjectID()))
			182	+	return gp.u32(0)
			183	+	}
			184	+
128		var place = neferclient.PopupPlacement{	185		var place = neferclient.PopupPlacement{
129		Anchor: neferclient.Rect{X: 10, Y: 0, Width: 20, Height: 30},	186		Anchor: neferclient.Rect{X: 10, Y: 0, Width: 20, Height: 30},
130		Edge: neferclient.EdgeBottom, Gravity: neferclient.EdgeBottomRight,	187		Edge: neferclient.EdgeBottom, Gravity: neferclient.EdgeBottomRight,

@@ -141,29 +198,26 @@ func TestPopupOnLayerParentGrabsBeforeFirstCommit(t *testing.T) {
141			198
142		popupSurf := p.SurfaceObjectID()	199		popupSurf := p.SurfaceObjectID()
143		xdg := e.xdgSurfaceOf(t, popupSurf)	200		xdg := e.xdgSurfaceOf(t, popupSurf)
144	-	getPopup := e.log.find(n, e.log.op(xdg, 2))	201	+	getPopup, gp := e.getPopup(t, xdg)
145	-	require.GreaterOrEqual(t, getPopup, 0, "xdg_surface.get_popup")
146	-	gp := e.log.at(getPopup)
147		popupID, positioner := gp.u32(0), gp.u32(2)	202		popupID, positioner := gp.u32(0), gp.u32(2)
148		require.Zero(t, gp.u32(1), "layer parent: null xdg parent")	203		require.Zero(t, gp.u32(1), "layer parent: null xdg parent")
149			204
150		// The layer surface is the one created for e.layer.	205		// The layer surface is the one created for e.layer.
151	-	layerID := e.log.at(e.log.find(0, func(r popupReq) bool { return r.op == 0 && len(r.body) > 8 && r.u32(1) == e.layer.SurfaceObjectID() })).u32(0)	206	+	layerID := e.log.at(e.log.mustFind(t, 0, func(r popupReq) bool { return r.op == 0 && len(r.body) > 8 && r.u32(1) == e.layer.SurfaceObjectID() }, "get_layer_surface")).u32(0)
152	-	layerPopup := e.log.find(n, e.log.op(layerID, 5))	207	+	layerPopup := e.log.mustFind(t, n, e.log.op(layerID, 5), "layer_surface.get_popup")
153		require.Greater(t, layerPopup, getPopup, "layer_surface.get_popup follows xdg_surface.get_popup")	208		require.Greater(t, layerPopup, getPopup, "layer_surface.get_popup follows xdg_surface.get_popup")
154		require.Equal(t, popupID, e.log.at(layerPopup).u32(0))	209		require.Equal(t, popupID, e.log.at(layerPopup).u32(0))
155			210
156	-	grab := e.log.find(n, e.log.op(popupID, 1))	211	+	grab := e.log.mustFind(t, n, e.log.op(popupID, 1), "xdg_popup.grab")
157		require.Greater(t, grab, layerPopup, "grab follows get_popup")	212		require.Greater(t, grab, layerPopup, "grab follows get_popup")
158		require.Equal(t, uint32(42), e.log.at(grab).u32(1))	213		require.Equal(t, uint32(42), e.log.at(grab).u32(1))
159		require.NotZero(t, e.log.at(grab).u32(0), "seat")	214		require.NotZero(t, e.log.at(grab).u32(0), "seat")
160	-	commit := e.log.find(n, e.log.op(popupSurf, 6))	215	+	commit := e.log.mustFind(t, n, e.log.op(popupSurf, 6), "wl_surface.commit")
161		require.Greater(t, commit, grab, "grab precedes the first commit")	216		require.Greater(t, commit, grab, "grab precedes the first commit")
162			217
163		// Positioner state.	218		// Positioner state.
164		pos := func(op uint16) popupReq {	219		pos := func(op uint16) popupReq {
165	-	i := e.log.find(n, e.log.op(positioner, op))	220	+	i := e.log.mustFind(t, n, e.log.op(positioner, op), "positioner request")
166	-	require.GreaterOrEqual(t, i, 0, "positioner op %d", op)
167		require.Less(t, i, getPopup)	221		require.Less(t, i, getPopup)
168		return e.log.at(i)	222		return e.log.at(i)
169		}	223		}

@@ -184,10 +238,9 @@ func TestPopupOnToplevelParentUsesParentXdgSurface(t *testing.T) {
184		require.NoError(t, e.c.Roundtrip())	238		require.NoError(t, e.c.Roundtrip())
185		parentXdg := e.xdgSurfaceOf(t, e.top.SurfaceObjectID())	239		parentXdg := e.xdgSurfaceOf(t, e.top.SurfaceObjectID())
186		xdg := e.xdgSurfaceOf(t, p.SurfaceObjectID())	240		xdg := e.xdgSurfaceOf(t, p.SurfaceObjectID())
187	-	i := e.log.find(n, e.log.op(xdg, 2))	241	+	_, gp := e.getPopup(t, xdg)
188	-	require.GreaterOrEqual(t, i, 0)	242	+	require.Equal(t, parentXdg, gp.u32(1))
189	-	require.Equal(t, parentXdg, e.log.at(i).u32(1))	243	+	popupID := gp.u32(0)
190	-	popupID := e.log.at(i).u32(0)
191		require.Negative(t, e.log.find(n, e.log.op(popupID, 1)), "no grab requested")	244		require.Negative(t, e.log.find(n, e.log.op(popupID, 1)), "no grab requested")
192		}	245		}
193			246

@@ -195,11 +248,14 @@ func TestPopupNestedOnPopup(t *testing.T) {
195		e := newPopupEnv(t, popupGlobals)	248		e := newPopupEnv(t, popupGlobals)
196		p, err := e.c.NewPopup(neferclient.PopupConfig{Parent: e.top, Place: place})	249		p, err := e.c.NewPopup(neferclient.PopupConfig{Parent: e.top, Place: place})
197		require.NoError(t, err)	250		require.NoError(t, err)
			251	+	_, err = e.c.NewPopup(neferclient.PopupConfig{Parent: p, Place: place})
			252	+	require.ErrorContains(t, err, "not configured yet", "a popup parent is mapped only once configured")
			253	+	e.configurePopup(t, p)
198		child, err := e.c.NewPopup(neferclient.PopupConfig{Parent: p, Place: place})	254		child, err := e.c.NewPopup(neferclient.PopupConfig{Parent: p, Place: place})
199		require.NoError(t, err)	255		require.NoError(t, err)
200		require.NoError(t, e.c.Roundtrip())	256		require.NoError(t, e.c.Roundtrip())
201	-	i := e.log.find(0, e.log.op(e.xdgSurfaceOf(t, child.SurfaceObjectID()), 2))	257	+	_, gp := e.getPopup(t, e.xdgSurfaceOf(t, child.SurfaceObjectID()))
202	-	require.Equal(t, e.xdgSurfaceOf(t, p.SurfaceObjectID()), e.log.at(i).u32(1))	258	+	require.Equal(t, e.xdgSurfaceOf(t, p.SurfaceObjectID()), gp.u32(1))
203		require.Error(t, p.Close(), "popup with an open child")	259		require.Error(t, p.Close(), "popup with an open child")
204		require.NoError(t, child.Close())	260		require.NoError(t, child.Close())
205		require.NoError(t, p.Close())	261		require.NoError(t, p.Close())

@@ -211,7 +267,8 @@ func TestPopupConfigureAcksAndDone(t *testing.T) {
211		require.NoError(t, err)	267		require.NoError(t, err)
212		require.NoError(t, e.c.Roundtrip())	268		require.NoError(t, e.c.Roundtrip())
213		xdg := e.xdgSurfaceOf(t, p.SurfaceObjectID())	269		xdg := e.xdgSurfaceOf(t, p.SurfaceObjectID())
214	-	popupID := e.log.at(e.log.find(0, e.log.op(xdg, 2))).u32(0)	270	+	_, gp := e.getPopup(t, xdg)
			271	+	popupID := gp.u32(0)
215			272
216		var w, h int32	273		var w, h int32
217		var done bool	274		var done bool

@@ -226,8 +283,7 @@ func TestPopupConfigureAcksAndDone(t *testing.T) {
226		dispatchUntil(t, e.c, hd, func() bool { return w != 0 })	283		dispatchUntil(t, e.c, hd, func() bool { return w != 0 })
227		require.Equal(t, [2]int32{120, 80}, [2]int32{w, h})	284		require.Equal(t, [2]int32{120, 80}, [2]int32{w, h})
228		require.NoError(t, e.c.Roundtrip())	285		require.NoError(t, e.c.Roundtrip())
229	-	ack := e.log.find(0, e.log.op(xdg, 4))	286	+	ack := e.log.mustFind(t, 0, e.log.op(xdg, 4), "ack_configure")
230	-	require.GreaterOrEqual(t, ack, 0, "ack_configure")
231		require.Equal(t, uint32(9), e.log.at(ack).u32(0))	287		require.Equal(t, uint32(9), e.log.at(ack).u32(0))
232		gw, gh, _ := p.Size()	288		gw, gh, _ := p.Size()
233		require.Equal(t, [2]int32{120, 80}, [2]int32{gw, gh})	289		require.Equal(t, [2]int32{120, 80}, [2]int32{gw, gh})

@@ -237,9 +293,8 @@ func TestPopupConfigureAcksAndDone(t *testing.T) {
237		dispatchUntil(t, e.c, hd, func() bool { return done })	293		dispatchUntil(t, e.c, hd, func() bool { return done })
238		require.NoError(t, p.Close())	294		require.NoError(t, p.Close())
239		require.NoError(t, e.c.Roundtrip())	295		require.NoError(t, e.c.Roundtrip())
240	-	require.NotEmpty(t, e.log.all(e.log.op(popupID, 0)), "xdg_popup.destroy")	296	+	destroyPopup := e.log.mustFind(t, 0, e.log.op(popupID, 0), "xdg_popup.destroy")
241	-	destroyPopup := e.log.find(0, e.log.op(popupID, 0))	297	+	destroyXdg := e.log.mustFind(t, 0, e.log.op(xdg, 0), "xdg_surface.destroy")
242	-	destroyXdg := e.log.find(0, e.log.op(xdg, 0))
243		require.Less(t, destroyPopup, destroyXdg, "popup destroyed before its xdg_surface")	298		require.Less(t, destroyPopup, destroyXdg, "popup destroyed before its xdg_surface")
244		}	299		}
245			300

@@ -249,19 +304,27 @@ func TestPopupReposition(t *testing.T) {
249		require.NoError(t, err)	304		require.NoError(t, err)
250		require.NoError(t, e.c.Roundtrip())	305		require.NoError(t, e.c.Roundtrip())
251		xdg := e.xdgSurfaceOf(t, p.SurfaceObjectID())	306		xdg := e.xdgSurfaceOf(t, p.SurfaceObjectID())
252	-	popupID := e.log.at(e.log.find(0, e.log.op(xdg, 2))).u32(0)	307	+	_, gp := e.getPopup(t, xdg)
			308	+	popupID := gp.u32(0)
253			309
254		np := place	310		np := place
255		np.Anchor.X, np.Width = 50, 90	311		np.Anchor.X, np.Width = 50, 90
			312	+	n := e.log.len()
256		require.NoError(t, p.Reposition(np))	313		require.NoError(t, p.Reposition(np))
257		require.NoError(t, e.c.Roundtrip())	314		require.NoError(t, e.c.Roundtrip())
258	-	i := e.log.find(0, e.log.op(popupID, 2))	315	+	i := e.log.mustFind(t, n, e.log.op(popupID, 2), "xdg_popup.reposition")
259	-	require.GreaterOrEqual(t, i, 0, "xdg_popup.reposition")
260		positioner := e.log.at(i).u32(0)	316		positioner := e.log.at(i).u32(0)
261	-	require.NotEqual(t, uint32(0), e.log.at(i).u32(1), "token")	317	+	token := e.log.at(i).u32(1)
262	-	setSize := e.log.at(e.log.find(i-8, e.log.op(positioner, 1)))	318	+	setSize := e.log.at(e.log.mustFind(t, n, e.log.op(positioner, 1), "positioner.set_size"))
263		require.Equal(t, uint32(90), setSize.u32(0))	319		require.Equal(t, uint32(90), setSize.u32(0))
264	-	require.NotEmpty(t, e.log.all(e.log.op(positioner, 0)), "positioner destroyed")	320	+	require.Less(t, e.log.mustFind(t, n, e.log.op(positioner, 1), "set_size"), i)
			321	+	require.Greater(t, e.log.mustFind(t, i, e.log.op(positioner, 0), "positioner.destroy"), i)
			322	+
			323	+	n = e.log.len()
			324	+	require.NoError(t, p.Reposition(np))
			325	+	require.NoError(t, e.c.Roundtrip())
			326	+	second := e.log.at(e.log.mustFind(t, n, e.log.op(popupID, 2), "second xdg_popup.reposition"))
			327	+	require.NotEqual(t, token, second.u32(1), "each reposition carries its own token")
265			328
266		bad := np	329		bad := np
267		bad.Width = 0	330		bad.Width = 0

@@ -289,6 +352,17 @@ func TestPopupInvalidConfigsSendNothing(t *testing.T) {
289		closed, err := e.c.NewToplevel("closed", 64, 48)	352		closed, err := e.c.NewToplevel("closed", 64, 48)
290		require.NoError(t, err)	353		require.NoError(t, err)
291		require.NoError(t, closed.Close())	354		require.NoError(t, closed.Close())
			355	+	unconfigured, err := e.c.NewToplevel("unconfigured", 64, 48)
			356	+	require.NoError(t, err)
			357	+	l, err := e.c.Lock()
			358	+	require.NoError(t, err)
			359	+	lockSurf, err := l.NewSurface(7)
			360	+	require.NoError(t, err)
			361	+	lockSurf.MarkPresentable(64, 48)
			362	+	other, _ := connectWire(t, surfaceGlobals, surfaceOutputs)
			363	+	foreign, err := other.NewToplevel("foreign", 64, 48)
			364	+	require.NoError(t, err)
			365	+	foreign.MarkPresentable(64, 48)
292		require.NoError(t, e.c.Roundtrip())	366		require.NoError(t, e.c.Roundtrip())
293		n := e.log.len()	367		n := e.log.len()
294			368

@@ -297,24 +371,104 @@ func TestPopupInvalidConfigsSendNothing(t *testing.T) {
297		f(&cfg)	371		f(&cfg)
298		return cfg	372		return cfg
299		}	373		}
300	-	for name, cfg := range map[string]neferclient.PopupConfig{	374	+	var capErr *neferclient.CapabilityError
301	-	"nil parent": with(func(c *neferclient.PopupConfig) { c.Parent = nil }),	375	+	for name, tc := range map[string]struct {
302	-	"closed parent": with(func(c *neferclient.PopupConfig) { c.Parent = closed }),	376	+	cfg neferclient.PopupConfig
303	-	"zero width": with(func(c *neferclient.PopupConfig) { c.Place.Width = 0 }),	377	+	want func(error)
304	-	"negative height": with(func(c *neferclient.PopupConfig) { c.Place.Height = -1 }),	378	+	}{
305	-	"empty anchor": with(func(c *neferclient.PopupConfig) { c.Place.Anchor.Width = 0 }),	379	+	"nil parent": {with(func(c *neferclient.PopupConfig) { c.Parent = nil }), func(err error) { require.ErrorContains(t, err, "needs a parent surface") }},
306	-	"bad edge": with(func(c *neferclient.PopupConfig) { c.Place.Edge = 9 }),	380	+	"foreign parent": {with(func(c *neferclient.PopupConfig) { c.Parent = foreign }), func(err error) { require.ErrorContains(t, err, "of this connection") }},
307	-	"bad gravity": with(func(c *neferclient.PopupConfig) { c.Place.Gravity = 9 }),	381	+	"closed parent": {with(func(c *neferclient.PopupConfig) { c.Parent = closed }), func(err error) { require.ErrorIs(t, err, neferclient.ErrClosed) }},
308	-	"bad adjust": with(func(c *neferclient.PopupConfig) { c.Place.Adjust = 64 }),	382	+	"lock parent": {with(func(c *neferclient.PopupConfig) { c.Parent = lockSurf }), func(err error) { require.ErrorContains(t, err, "must be a layer surface, a toplevel or a popup") }},
309	-	"grab no serial": with(func(c *neferclient.PopupConfig) { c.Grab = true }),	383	+	"unconfigured": {with(func(c *neferclient.PopupConfig) { c.Parent = unconfigured }), func(err error) { require.ErrorContains(t, err, "not configured yet") }},
			384	+	"zero width": {with(func(c *neferclient.PopupConfig) { c.Place.Width = 0 }), func(err error) { require.ErrorContains(t, err, "invalid popup size") }},
			385	+	"negative height": {with(func(c *neferclient.PopupConfig) { c.Place.Height = -1 }), func(err error) { require.ErrorContains(t, err, "invalid popup size") }},
			386	+	"empty anchor": {with(func(c *neferclient.PopupConfig) { c.Place.Anchor.Width = 0 }), func(err error) { require.ErrorContains(t, err, "anchor rectangle") }},
			387	+	"anchor overflow": {with(func(c *neferclient.PopupConfig) {
			388	+	c.Place.Anchor = neferclient.Rect{X: math.MaxInt32, Width: 10, Height: 5}
			389	+	}), func(err error) { require.ErrorContains(t, err, "overflows") }},
			390	+	"bad edge": {with(func(c *neferclient.PopupConfig) { c.Place.Edge = 9 }), func(err error) { require.ErrorContains(t, err, "invalid popup edge") }},
			391	+	"bad gravity": {with(func(c *neferclient.PopupConfig) { c.Place.Gravity = 9 }), func(err error) { require.ErrorContains(t, err, "invalid popup edge") }},
			392	+	"bad adjust": {with(func(c *neferclient.PopupConfig) { c.Place.Adjust = 64 }), func(err error) { require.ErrorContains(t, err, "constraint adjustment") }},
			393	+	"grab no serial": {with(func(c *neferclient.PopupConfig) { c.Grab = true }), func(err error) { require.ErrorContains(t, err, "serial") }},
			394	+	"grab no seat": {with(func(c *neferclient.PopupConfig) { c.Grab, c.Serial = true, 1 }), func(err error) { require.ErrorAs(t, err, &capErr) }},
310		} {	395		} {
311	-	_, err := e.c.NewPopup(cfg)	396	+	_, err := e.c.NewPopup(tc.cfg)
312		require.Error(t, err, name)	397		require.Error(t, err, name)
			398	+	tc.want(err)
313		}	399		}
314	-	var ce *neferclient.CapabilityError
315	-	_, err = e.c.NewPopup(with(func(c *neferclient.PopupConfig) { c.Grab, c.Serial = true, 1 }))
316	-	require.ErrorAs(t, err, &ce, "grab without a seat")
317		require.NoError(t, e.c.Roundtrip())	400		require.NoError(t, e.c.Roundtrip())
318		require.Equal(t, n, e.log.len(), "no request was sent")	401		require.Equal(t, n, e.log.len(), "no request was sent")
319		require.NoError(t, e.top.Close(), "failed popups left no child behind")	402		require.NoError(t, e.top.Close(), "failed popups left no child behind")
320		}	403		}
			404	+
			405	+	// A failed seat bind is the cause of the grab's CapabilityError.
			406	+	func TestPopupGrabReportsSeatBindError(t *testing.T) {
			407	+	e := newPopupEnv(t, surfaceGlobals)
			408	+	bindErr := errors.New("bind refused")
			409	+	e.c.SetSeatErr(bindErr)
			410	+	_, err := e.c.NewPopup(neferclient.PopupConfig{Parent: e.top, Place: place, Grab: true, Serial: 1})
			411	+	var ce *neferclient.CapabilityError
			412	+	require.ErrorAs(t, err, &ce)
			413	+	require.Equal(t, "wl_seat", ce.Name)
			414	+	require.ErrorIs(t, err, bindErr)
			415	+	}
			416	+
			417	+	// A grab needs a grabbing parent popup, and while a grab chain is open only
			418	+	// its topmost popup can be the parent of the next grab. Refusals send nothing.
			419	+	func TestPopupGrabChain(t *testing.T) {
			420	+	e := newPopupEnv(t, popupGlobals)
			421	+	grab := func(parent neferclient.Surface) (neferclient.Surface, error) {
			422	+	return e.c.NewPopup(neferclient.PopupConfig{Parent: parent, Place: place, Grab: true, Serial: 7})
			423	+	}
			424	+	refused := func(parent *neferclient.Surface, want string) {
			425	+	t.Helper()
			426	+	require.NoError(t, e.c.Roundtrip()) // earlier requests are logged
			427	+	n := e.log.len()
			428	+	_, err := grab(parent)
			429	+	require.ErrorContains(t, err, want)
			430	+	require.NoError(t, e.c.Roundtrip())
			431	+	require.Equal(t, n, e.log.len(), "no request was sent")
			432	+	}
			433	+
			434	+	plain, err := e.c.NewPopup(neferclient.PopupConfig{Parent: e.layer, Place: place})
			435	+	require.NoError(t, err)
			436	+	e.configurePopup(t, plain)
			437	+	refused(plain, "needs a grabbing parent popup")
			438	+
			439	+	a, err := grab(e.layer)
			440	+	require.NoError(t, err)
			441	+	e.configurePopup(t, a)
			442	+	refused(e.top, "topmost grabbing popup")
			443	+	refused(e.layer, "topmost grabbing popup")
			444	+	refused(plain, "needs a grabbing parent popup")
			445	+
			446	+	b, err := grab(a)
			447	+	require.NoError(t, err)
			448	+	e.configurePopup(t, b)
			449	+	refused(a, "topmost grabbing popup")
			450	+
			451	+	// Closing the topmost popup returns the grab to its parent.
			452	+	require.NoError(t, b.Close())
			453	+	c2, err := grab(a)
			454	+	require.NoError(t, err)
			455	+	require.NoError(t, c2.Close())
			456	+	require.NoError(t, a.Close())
			457	+
			458	+	// A popup_done ends the grab of the dismissed popup, too.
			459	+	a2, err := grab(e.layer)
			460	+	require.NoError(t, err)
			461	+	refused(e.top, "topmost grabbing popup")
			462	+	e.srv.write(frame(e.popupID(t, a2), 1, nil))
			463	+	require.NoError(t, e.c.Roundtrip())
			464	+	var done []neferclient.SurfaceID
			465	+	hd := neferclientmocks.NewMockHandler(t)
			466	+	hd.EXPECT().PopupDone(mock.Anything).Run(func(id neferclient.SurfaceID) { done = append(done, id) }).Return().Once()
			467	+	dispatchUntil(t, e.c, hd, func() bool { return len(done) > 0 })
			468	+	require.Equal(t, []neferclient.SurfaceID{a2.ID()}, done)
			469	+	top, err := grab(e.top)
			470	+	require.NoError(t, err, "the chain is closed after popup_done")
			471	+	require.NoError(t, top.Close())
			472	+	require.NoError(t, a2.Close())
			473	+	require.NoError(t, plain.Close())
			474	+	}

M surface.go +3 -0


@@ -108,6 +108,7 @@ type Surface struct {
108	108		parent *Surface // popups: the surface they were created on
109	109		children int // open popups whose parent this surface is
110	110		repoToken uint32 // popups: last xdg_popup.reposition token
	111	+	grabbed bool // popups: an explicit grab was taken and not dismissed yet
111	112
112	113		width, height int32
113	114		scale float64

@@ -573,6 +574,7 @@ func (c Conn) applySurface(ev event, h Handler) {
573	574		case evXdgToplevel, evPopupConfigure:
574	575		s.pendW, s.pendH = ev.a, ev.b
575	576		case evPopupDone:
	577	+	s.dropGrab() // the compositor ended the grab
576	578		h.PopupDone(id)
577	579		case evXdgConfigure:
578	580		if s.pendW > 0 && s.pendH > 0 {

@@ -758,6 +760,7 @@ func (s *Surface) destroy() error {
758	760		s.closed = true
759	761		delete(s.c.surfaces, s.id)
760	762		s.c.seatSurfaceGone(s)
	763	+	s.dropGrab()
761	764		if s.parent != nil {
762	765		s.parent.children--
763	766		s.parent = nil


@@ -108,6 +108,7 @@ type Surface struct {
108	parent *Surface // popups: the surface they were created on	108		parent *Surface // popups: the surface they were created on
109	children int // open popups whose parent this surface is	109		children int // open popups whose parent this surface is
110	repoToken uint32 // popups: last xdg_popup.reposition token	110		repoToken uint32 // popups: last xdg_popup.reposition token
		111	+	grabbed bool // popups: an explicit grab was taken and not dismissed yet
111		112
112	width, height int32	113		width, height int32
113	scale float64	114		scale float64

@@ -573,6 +574,7 @@ func (c Conn) applySurface(ev event, h Handler) {
573	case evXdgToplevel, evPopupConfigure:	574		case evXdgToplevel, evPopupConfigure:
574	s.pendW, s.pendH = ev.a, ev.b	575		s.pendW, s.pendH = ev.a, ev.b
575	case evPopupDone:	576		case evPopupDone:
		577	+	s.dropGrab() // the compositor ended the grab
576	h.PopupDone(id)	578		h.PopupDone(id)
577	case evXdgConfigure:	579		case evXdgConfigure:
578	if s.pendW > 0 && s.pendH > 0 {	580		if s.pendW > 0 && s.pendH > 0 {

@@ -758,6 +760,7 @@ func (s *Surface) destroy() error {
758	s.closed = true	760		s.closed = true
759	delete(s.c.surfaces, s.id)	761		delete(s.c.surfaces, s.id)
760	s.c.seatSurfaceGone(s)	762		s.c.seatSurfaceGone(s)
		763	+	s.dropGrab()
761	if s.parent != nil {	764		if s.parent != nil {
762	s.parent.children--	765		s.parent.children--
763	s.parent = nil	766		s.parent = nil