fix(tray): one deadline for opening a menu, close it again when it fails

AboutToShow, Event(opened) and GetLayout share a 4 s budget. If the application was told the menu opened and no menu reaches the bar (timeout, bad or empty layout), it is sent Event(0, closed) and the menu is forgotten.

commit: 35d84dc7b30387802d488d757d99cce846decad7
author: brice · 13d ago · 2026-10-09 06:42 UTC
parent: 559fa4c
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

3 files · +122 -15

M internal/tray/control.go +54 -14


@@ -204,6 +204,10 @@ type control struct {
204	204		tipTimer *time.Timer
205	205		hov hover
206	206		menu *lastMenu
	207	+
	208	+	// deadline, when set, ends every call of the sequence in progress
	209	+	// (openMenu): a call waits for the smaller of callTimeout and what is left.
	210	+	deadline time.Time
207	211		}
208	212
209	213		// hover is the icon the pointer rests on, and whether its tooltip is shown.

@@ -432,8 +436,14 @@ func (c control) callOn(dest, path, iface, member, sig string, args func(zerob
432	436		}
433	437		return err
434	438		}
	439	+	wait := callTimeout
	440	+	if !c.deadline.IsZero() {
	441	+	if wait = min(wait, time.Until(c.deadline)); wait <= 0 {
	442	+	return errBudget
	443	+	}
	444	+	}
435	445		args(conn.NewCall(dest, path, iface, member, sig))
436		-	timer := time.AfterFunc(callTimeout, func() { _ = conn.Close() })
	446	+	timer := time.AfterFunc(wait, func() { _ = conn.Close() })
437	447		m, err := conn.Call()
438	448		if err == nil && reply != nil {
439	449		reply(m)

@@ -461,6 +471,18 @@ func (c control) callOn(dest, path, iface, member, sig string, args func(zerob
461	471		return err
462	472		}
463	473
	474	+	// errBudget is returned by a call that did not start because the time for its
	475	+	// sequence ran out.
	476	+	var errBudget = errors.New("tray: out of time")
	477	+
	478	+	// menuBudget is how long opening a menu may take in all, whatever the number
	479	+	// of calls it makes. A variable for the tests.
	480	+	var menuBudget = 4 * time.Second
	481	+
	482	+	// closeBudget is the time given to tell an application its menu will not be
	483	+	// shown after all.
	484	+	const closeBudget = time.Second
	485	+
464	486		// transportFailed reports whether err means the call did not complete, as
465	487		// opposed to the application answering with an error.
466	488		func transportFailed(err error) bool {

@@ -470,24 +492,41 @@ func transportFailed(err error) bool {
470	492
471	493		// openMenu shows the application's menu for the press numbered token: its
472	494		// dbusmenu when it has one, else it asks the application to show its own.
	495	+	// The calls share one deadline (menuBudget). When they fail after the
	496	+	// application was told "opened", it is told "closed" and the menu forgotten.
473	497		func (c *control) openMenu(tg Target, token uint32) {
474	498		if tg.Menu == "" \|\| tg.Menu == noMenuPath {
475	499		c.contextMenu(tg)
476	500		return
477	501		}
	502	+	c.menu = nil // an older menu is superseded
	503	+	c.deadline = time.Now().Add(menuBudget)
	504	+	defer func() { c.deadline = time.Time{} }()
	505	+
478	506		// Ignore refusals: many applications do not implement these.
479	507		err := c.callOn(tg.Dest, tg.Menu, menuIface, "AboutToShow", "i", func(e *zerobus.Encoder) { e.Int32(0) }, nil)
480		-	if transportFailed(err) {
	508	+	if err != nil && (transportFailed(err) \|\| errors.Is(err, errBudget)) {
481	509		return
482	510		}
483	511		err = c.menuEvent(tg.Dest, tg.Menu, 0, "opened")
484		-	if transportFailed(err) {
485		-	return
	512	+	if errors.Is(err, errBudget) {
	513	+	return // never sent
486	514		}
	515	+	// From here on the application may believe its menu is open, even if the
	516	+	// call timed out: it may have been delivered.
	517	+	if (err != nil && transportFailed(err)) \|\| !c.sendMenu(tg, token) {
	518	+	c.deadline = time.Now().Add(closeBudget)
	519	+	c.menuEvent(tg.Dest, tg.Menu, 0, "closed")
	520	+	c.menu = nil
	521	+	}
	522	+	}
	523	+
	524	+	// sendMenu reads the menu of tg and writes it for the bar. It reports false
	525	+	// when there is no menu to show.
	526	+	func (c *control) sendMenu(tg Target, token uint32) bool {
487	527		var items []menuItem
488		-	var ids []int32
489	528		var decodeErr error
490		-	err = c.callOn(tg.Dest, tg.Menu, menuIface, "GetLayout", "iias", func(e *zerobus.Encoder) {
	529	+	err := c.callOn(tg.Dest, tg.Menu, menuIface, "GetLayout", "iias", func(e *zerobus.Encoder) {
491	530		e.Int32(0)
492	531		e.Int32(-1)
493	532		a := e.BeginArray('s')

@@ -497,26 +536,27 @@ func (c *control) openMenu(tg Target, token uint32) {
497	536		decodeErr = errors.New("unexpected layout signature " + m.Signature)
498	537		return
499	538		}
500		-	items, ids, decodeErr = decodeLayout(m.Body())
	539	+	items, _, decodeErr = decodeLayout(m.Body())
501	540		})
502	541		if err != nil {
503		-	return
	542	+	return false
504	543		}
505	544		if decodeErr != nil {
506	545		c.log.Warn("tray: unreadable menu", "dest", tg.Dest, "err", decodeErr)
507		-	return
	546	+	return false
508	547		}
509	548		if len(items) == 0 {
510	549		c.log.Debug("tray: the menu is empty", "dest", tg.Dest)
511		-	return
	550	+	return false
512	551		}
513		-	line, ids, encErr := encodeMenu(menuLine{Type: "menu", Col: tg.Start, Width: tg.Width, Click: token, Items: items})
514		-	if encErr != nil {
515		-	c.log.Warn("tray: cannot send the menu", "dest", tg.Dest, "err", encErr)
516		-	return
	552	+	line, ids, err := encodeMenu(menuLine{Type: "menu", Col: tg.Start, Width: tg.Width, Click: token, Items: items})
	553	+	if err != nil {
	554	+	c.log.Warn("tray: cannot send the menu", "dest", tg.Dest, "err", err)
	555	+	return false
517	556		}
518	557		c.menu = &lastMenu{token: token, dest: tg.Dest, path: tg.Menu, ids: ids}
519	558		c.writeLine(line)
	559	+	return true
520	560		}
521	561
522	562		// menuEvent sends a dbusmenu Event with no data.

 	tipTimer *time.Timer
 	hov      hover
 	menu     *lastMenu
+	// deadline, when set, ends every call of the sequence in progress
+	// (openMenu): a call waits for the smaller of callTimeout and what is left.
+	deadline time.Time
 }
 // hover is the icon the pointer rests on, and whether its tooltip is shown.
 		}
 		return err
 	}
+	wait := callTimeout
+	if !c.deadline.IsZero() {
+		if wait = min(wait, time.Until(c.deadline)); wait <= 0 {
+			return errBudget
+		}
+	}
 	args(conn.NewCall(dest, path, iface, member, sig))
-	timer := time.AfterFunc(callTimeout, func() { _ = conn.Close() })
+	timer := time.AfterFunc(wait, func() { _ = conn.Close() })
 	m, err := conn.Call()
 	if err == nil && reply != nil {
 		reply(m)
 	return err
 }
+// errBudget is returned by a call that did not start because the time for its
+// sequence ran out.
+var errBudget = errors.New("tray: out of time")
+// menuBudget is how long opening a menu may take in all, whatever the number
+// of calls it makes. A variable for the tests.
+var menuBudget = 4 * time.Second
+// closeBudget is the time given to tell an application its menu will not be
+// shown after all.
+const closeBudget = time.Second
 // transportFailed reports whether err means the call did not complete, as
 // opposed to the application answering with an error.
 func transportFailed(err error) bool {
 // openMenu shows the application's menu for the press numbered token: its
 // dbusmenu when it has one, else it asks the application to show its own.
+// The calls share one deadline (menuBudget). When they fail after the
+// application was told "opened", it is told "closed" and the menu forgotten.
 func (c *control) openMenu(tg Target, token uint32) {
 	if tg.Menu == "" || tg.Menu == noMenuPath {
 		c.contextMenu(tg)
 		return
 	}
+	c.menu = nil // an older menu is superseded
+	c.deadline = time.Now().Add(menuBudget)
+	defer func() { c.deadline = time.Time{} }()
 	// Ignore refusals: many applications do not implement these.
 	err := c.callOn(tg.Dest, tg.Menu, menuIface, "AboutToShow", "i", func(e *zerobus.Encoder) { e.Int32(0) }, nil)
-	if transportFailed(err) {
+	if err != nil && (transportFailed(err) || errors.Is(err, errBudget)) {
 		return
 	}
 	err = c.menuEvent(tg.Dest, tg.Menu, 0, "opened")
-	if transportFailed(err) {
+	if errors.Is(err, errBudget) {
-		return
+		return // never sent
 	}
+	// From here on the application may believe its menu is open, even if the
+	// call timed out: it may have been delivered.
+	if (err != nil && transportFailed(err)) || !c.sendMenu(tg, token) {
+		c.deadline = time.Now().Add(closeBudget)
+		c.menuEvent(tg.Dest, tg.Menu, 0, "closed")
+		c.menu = nil
+	}
+}
+// sendMenu reads the menu of tg and writes it for the bar. It reports false
+// when there is no menu to show.
+func (c *control) sendMenu(tg Target, token uint32) bool {
 	var items []menuItem
-	var ids []int32
 	var decodeErr error
-	err = c.callOn(tg.Dest, tg.Menu, menuIface, "GetLayout", "iias", func(e *zerobus.Encoder) {
+	err := c.callOn(tg.Dest, tg.Menu, menuIface, "GetLayout", "iias", func(e *zerobus.Encoder) {
 		e.Int32(0)
 		e.Int32(-1)
 		a := e.BeginArray('s')
 			decodeErr = errors.New("unexpected layout signature " + m.Signature)
 			return
 		}
-		items, ids, decodeErr = decodeLayout(m.Body())
+		items, _, decodeErr = decodeLayout(m.Body())
 	})
 	if err != nil {
-		return
+		return false
 	}
 	if decodeErr != nil {
 		c.log.Warn("tray: unreadable menu", "dest", tg.Dest, "err", decodeErr)
-		return
+		return false
 	}
 	if len(items) == 0 {
 		c.log.Debug("tray: the menu is empty", "dest", tg.Dest)
-		return
+		return false
 	}
-	line, ids, encErr := encodeMenu(menuLine{Type: "menu", Col: tg.Start, Width: tg.Width, Click: token, Items: items})
+	line, ids, err := encodeMenu(menuLine{Type: "menu", Col: tg.Start, Width: tg.Width, Click: token, Items: items})
-	if encErr != nil {
+	if err != nil {
-		c.log.Warn("tray: cannot send the menu", "dest", tg.Dest, "err", encErr)
+		c.log.Warn("tray: cannot send the menu", "dest", tg.Dest, "err", err)
-		return
+		return false
 	}
 	c.menu = &lastMenu{token: token, dest: tg.Dest, path: tg.Menu, ids: ids}
 	c.writeLine(line)
+	return true
 }
 // menuEvent sends a dbusmenu Event with no data.

M internal/tray/dbusmenu_test.go +66 -0


@@ -82,6 +82,7 @@ func (f fakeItem) menuCall(m zerobus.Message) bool {
82	82		default:
83	83		return true
84	84		}
	85	+	time.Sleep(f.mslow[m.Member]) // a slow application: the reply is built, not sent yet
85	86		_, err := f.c.Send()
86	87		return err == nil
87	88		}

@@ -325,3 +326,68 @@ func decodeTestLayout(t testing.T, root tnode) ([]menuItem, []int32) {
325	326		<-done
326	327		return items, ids
327	328		}
	329	+
	330	+	// A slow application cannot hold the tray for more than the shared budget, and
	331	+	// it is told the menu is closed when it was told it opened.
	332	+	func TestSlowMenuUsesOneDeadlineAndIsClosed(t *testing.T) {
	333	+	old := menuBudget
	334	+	menuBudget = 1200 * time.Millisecond
	335	+	t.Cleanup(func() { menuBudget = old })
	336	+
	337	+	steam, _, in, out := setup(t)
	338	+	menuReady(t, steam, in, out, sampleLayout()) // the tray knows the menu now
	339	+	in.send(t, "menu-closed 9")
	340	+	expectMenuCall(t, steam, "Event(0,closed)")
	341	+
	342	+	// Every call takes 500 ms: each is under the 2 s call limit, and the
	343	+	// sequence runs out of its 1.2 s while GetLayout is waiting.
	344	+	steam.run(t, func() {
	345	+	steam.mslow = map[string]time.Duration{"AboutToShow": 500 * time.Millisecond, "Event": 500 * time.Millisecond, "GetLayout": 500 * time.Millisecond}
	346	+	})
	347	+	start := time.Now()
	348	+	in.send(t, "click right 0 20")
	349	+	expectMenuCall(t, steam, "AboutToShow(0)")
	350	+	expectMenuCall(t, steam, "Event(0,opened)")
	351	+	expectMenuCall(t, steam, "GetLayout")
	352	+	// GetLayout ran out of budget: the application hears "closed".
	353	+	expectMenuCall(t, steam, "Event(0,closed)")
	354	+	if d := time.Since(start); d > 3*time.Second {
	355	+	t.Fatalf("opening the menu took %v", d)
	356	+	}
	357	+	noControlLineFor(t, out, "menu", 300*time.Millisecond)
	358	+	// The menu is forgotten: a late choice does nothing.
	359	+	in.send(t, "menu-activate 20 1")
	360	+	expectNoMenuCall(t, steam)
	361	+	}
	362	+
	363	+	// An application with an empty menu is also told that it closed.
	364	+	func TestEmptyMenuIsClosedAgain(t *testing.T) {
	365	+	steam, _, in, out := setup(t)
	366	+	menuReady(t, steam, in, out, sampleLayout())
	367	+	in.send(t, "menu-closed 9")
	368	+	expectMenuCall(t, steam, "Event(0,closed)")
	369	+	steam.run(t, func() { steam.menu = &tnode{id: 0} })
	370	+	// The tray still thinks it has a menu path: the layout is just empty.
	371	+	in.send(t, "click right 0 21")
	372	+	for _, want := range []string{"AboutToShow(0)", "Event(0,opened)", "GetLayout", "Event(0,closed)"} {
	373	+	expectMenuCall(t, steam, want)
	374	+	}
	375	+	in.send(t, "menu-activate 21 1")
	376	+	expectNoMenuCall(t, steam)
	377	+	}
	378	+
	379	+	// noControlLineFor fails when a control line of the given type arrives within d.
	380	+	func noControlLineFor(t testing.T, out lines, typ string, d time.Duration) {
	381	+	t.Helper()
	382	+	timeout := time.After(d)
	383	+	for {
	384	+	select {
	385	+	case s, ok := <-out.ch:
	386	+	if ok && strings.HasPrefix(s, controlPrefix) && strings.Contains(s, `"type":"`+typ+`"`) {
	387	+	t.Fatalf("unexpected %s line %q", typ, s)
	388	+	}
	389	+	case <-timeout:
	390	+	return
	391	+	}
	392	+	}
	393	+	}


@@ -82,6 +82,7 @@ func (f fakeItem) menuCall(m zerobus.Message) bool {
82	default:	82		default:
83	return true	83		return true
84	}	84		}
		85	+	time.Sleep(f.mslow[m.Member]) // a slow application: the reply is built, not sent yet
85	_, err := f.c.Send()	86		_, err := f.c.Send()
86	return err == nil	87		return err == nil
87	}	88		}

@@ -325,3 +326,68 @@ func decodeTestLayout(t testing.T, root tnode) ([]menuItem, []int32) {
325	<-done	326		<-done
326	return items, ids	327		return items, ids
327	}	328		}
		329	+
		330	+	// A slow application cannot hold the tray for more than the shared budget, and
		331	+	// it is told the menu is closed when it was told it opened.
		332	+	func TestSlowMenuUsesOneDeadlineAndIsClosed(t *testing.T) {
		333	+	old := menuBudget
		334	+	menuBudget = 1200 * time.Millisecond
		335	+	t.Cleanup(func() { menuBudget = old })
		336	+
		337	+	steam, _, in, out := setup(t)
		338	+	menuReady(t, steam, in, out, sampleLayout()) // the tray knows the menu now
		339	+	in.send(t, "menu-closed 9")
		340	+	expectMenuCall(t, steam, "Event(0,closed)")
		341	+
		342	+	// Every call takes 500 ms: each is under the 2 s call limit, and the
		343	+	// sequence runs out of its 1.2 s while GetLayout is waiting.
		344	+	steam.run(t, func() {
		345	+	steam.mslow = map[string]time.Duration{"AboutToShow": 500 * time.Millisecond, "Event": 500 * time.Millisecond, "GetLayout": 500 * time.Millisecond}
		346	+	})
		347	+	start := time.Now()
		348	+	in.send(t, "click right 0 20")
		349	+	expectMenuCall(t, steam, "AboutToShow(0)")
		350	+	expectMenuCall(t, steam, "Event(0,opened)")
		351	+	expectMenuCall(t, steam, "GetLayout")
		352	+	// GetLayout ran out of budget: the application hears "closed".
		353	+	expectMenuCall(t, steam, "Event(0,closed)")
		354	+	if d := time.Since(start); d > 3*time.Second {
		355	+	t.Fatalf("opening the menu took %v", d)
		356	+	}
		357	+	noControlLineFor(t, out, "menu", 300*time.Millisecond)
		358	+	// The menu is forgotten: a late choice does nothing.
		359	+	in.send(t, "menu-activate 20 1")
		360	+	expectNoMenuCall(t, steam)
		361	+	}
		362	+
		363	+	// An application with an empty menu is also told that it closed.
		364	+	func TestEmptyMenuIsClosedAgain(t *testing.T) {
		365	+	steam, _, in, out := setup(t)
		366	+	menuReady(t, steam, in, out, sampleLayout())
		367	+	in.send(t, "menu-closed 9")
		368	+	expectMenuCall(t, steam, "Event(0,closed)")
		369	+	steam.run(t, func() { steam.menu = &tnode{id: 0} })
		370	+	// The tray still thinks it has a menu path: the layout is just empty.
		371	+	in.send(t, "click right 0 21")
		372	+	for _, want := range []string{"AboutToShow(0)", "Event(0,opened)", "GetLayout", "Event(0,closed)"} {
		373	+	expectMenuCall(t, steam, want)
		374	+	}
		375	+	in.send(t, "menu-activate 21 1")
		376	+	expectNoMenuCall(t, steam)
		377	+	}
		378	+
		379	+	// noControlLineFor fails when a control line of the given type arrives within d.
		380	+	func noControlLineFor(t testing.T, out lines, typ string, d time.Duration) {
		381	+	t.Helper()
		382	+	timeout := time.After(d)
		383	+	for {
		384	+	select {
		385	+	case s, ok := <-out.ch:
		386	+	if ok && strings.HasPrefix(s, controlPrefix) && strings.Contains(s, `"type":"`+typ+`"`) {
		387	+	t.Fatalf("unexpected %s line %q", typ, s)
		388	+	}
		389	+	case <-timeout:
		390	+	return
		391	+	}
		392	+	}
		393	+	}

M internal/tray/tray_test.go +2 -1


@@ -74,7 +74,8 @@ type fakeItem struct {
74	74
75	75		// A dbusmenu at /MenuBar, set through run.
76	76		menu *tnode
77		-	mcalls chan string // the dbusmenu calls it received
	77	+	mcalls chan string // the dbusmenu calls it received
	78	+	mslow map[string]time.Duration // dbusmenu methods it answers only after this long, set through run
78	79		}
79	80
80	81		func newFakeItem(t testing.T, addr, id, iconName string, pixel [4]byte) fakeItem {


@@ -74,7 +74,8 @@ type fakeItem struct {
74			74
75		// A dbusmenu at /MenuBar, set through run.	75		// A dbusmenu at /MenuBar, set through run.
76		menu *tnode	76		menu *tnode
77	-	mcalls chan string // the dbusmenu calls it received	77	+	mcalls chan string // the dbusmenu calls it received
			78	+	mslow map[string]time.Duration // dbusmenu methods it answers only after this long, set through run
78		}	79		}
79			80
80		func newFakeItem(t testing.T, addr, id, iconName string, pixel [4]byte) fakeItem {	81		func newFakeItem(t testing.T, addr, id, iconName string, pixel [4]byte) fakeItem {