betterbird: know a composer by its instance alone, and bound a restart's worst case (review of #155)

A composer window whose class is not Betterbird's own was not seen, so a restart could lose its message.
Each step of a restart now has a bound short enough that all of them together, taken in full, stay
under the runtime's 30 s: 3 s for the window tree, 8 s to quit, 3 and 5 s to start, 4 s to see it.

modules/betterbird/README.md

@@ -67,7 +67,7 @@ calendar entry, an account, an address or a credential.**
 | `betterbird_prefs` (r) | named preferences or prefixes (ending in `.` or `*`, at most 200; an empty name, `*` or `.` is refused): value and source (user.js, prefs.js or the default). Withheld without its value: a name that can hold a credential (`password`, `token`, `oauth`, `signon`, …); one that names an account, server, calendar, recent folder or person (`mail.server.`, `mail.identity.`, `mail.last_msg…`, `messenger.account.`, `mail.cloud_files.`, `calendar.registry.`, …); and every `extensions.*` but the theme's id (`extensions.activeThemeID`). In every value answered, the home is `~`, and a mail address, a `user%40domain@host` URI and a host name are masked |
 | `betterbird_user_js` (r; a with `apply`) | the declared user.js against the profile's; `apply` places it |
 | `betterbird_log` (r) | the last lines of the journal of `mesh-betterbird` (or of the reminder test), addresses and credentials masked (the value after `password=`, `token:` and the like, and the token after `Bearer`/`Basic`). A Betterbird the module did not start prints elsewhere, and the answer says where |
-| `betterbird_restart` (a) | ends Betterbird (asked to quit, forced after 10 s), places the user.js where allowed, starts it as `mesh-betterbird`. **Refused while a composer window is open** (X instance `Msgcompose`), because the unsent message would be lost, unless `discard_unsent` is true, which only the operator's yes justifies. It answers within the runtime's 30 s: a start not yet seen after 4 s is said in `note`, and `betterbird_status` shows it |
+| `betterbird_restart` (a) | ends Betterbird (asked to quit, forced after 8 s), places the user.js where allowed, starts it as `mesh-betterbird`. **Refused while a composer window is open** (X instance `Msgcompose`, whatever its class), because the unsent message would be lost, unless `discard_unsent` is true, which only the operator's yes justifies. It answers within the runtime's 30 s: a start not yet seen after 4 s is said in `note`, and `betterbird_status` shows it |
 | `betterbird_check` (r) | installed; one running; profile found; the user.js on the machine, valid, placed, in effect; its output read |
 | `betterbird_reminder_test` (a) | a reminder on demand without the operator's calendar (below) |
 

modules/betterbird/cmd/betterbird-tools/betterbird.go

@@ -676,12 +676,12 @@ type RestartAnswer struct {
 	Unit      string  `json:"unit"`
 }
 
-// composerClass is the X instance of a message being written.
-var composerClass = regexp.MustCompile(`\("Msgcompose" "eu\.betterbird\.Betterbird"\)`)
+// composerClass is the X instance of a message being written, whatever the class beside it.
+var composerClass = regexp.MustCompile(`\("Msgcompose" "[^"]*"\)`)
 
 // composers counts the open message windows, from the X window tree.
 func (m *Machine) composers(s Session) (int, error) {
-	o := m.cmd(5*time.Second, s.Env(), "xwininfo", "-root", "-tree")
+	o := m.cmd(3*time.Second, s.Env(), "xwininfo", "-root", "-tree")
 	if err := failed(o, "xwininfo", "-root", "-tree"); err != nil {
 		return 0, err
 	}
@@ -707,13 +707,23 @@ func (m *Machine) Restart(discard bool) (RestartAnswer, error) {
 		return a, fmt.Errorf("%d composer window(s) are open: an unsent message would be lost. Send or save it, or ask the operator, then call again with discard_unsent", n)
 	}
 	a.Composers = n
-	a.Ended, a.Killed = m.stopProcs(10*time.Second, m.mainProcs())
+	a.Ended, a.Killed = m.stopProcs(8*time.Second, m.mainProcs())
 	if u, err := m.userJS(true); err != nil {
 		a.UserJS = "not placed: " + err.Error()
 	} else {
 		a.UserJS = fmt.Sprintf("placed: %d preference(s) from ~/%s", u.Statements, declared)
 	}
-	if err := m.detach(s, restartAs, startBin); err != nil {
+	// As detach does, with shorter bounds: the worst case of the whole call (3 s for the window tree,
+	// 8 s to quit, 3 + 5 s to start, 4 s to see it) stays under the runtime's 30 s.
+	_ = m.cmd(3*time.Second, s.Env(), "systemctl", "--user", "stop", restartAs+".service")
+	call := []string{"--user", "--collect", "--quiet", "--unit=" + restartAs}
+	for _, kv := range [][2]string{{"DISPLAY", s.Display}, {"XAUTHORITY", s.XAuthority}} {
+		if kv[1] != "" {
+			call = append(call, "--setenv="+kv[0]+"="+kv[1])
+		}
+	}
+	call = append(call, "--", startBin)
+	if err := failed(m.cmd(5*time.Second, s.Env(), "systemd-run", call...), "systemd-run", call...); err != nil {
 		return a, err
 	}
 	for waited := time.Duration(0); waited < 4*time.Second && len(m.mainProcs()) == 0; waited += 250 * time.Millisecond {

modules/betterbird/cmd/betterbird-tools/main.go

@@ -100,7 +100,7 @@ func tools() []stdio.Tool {
 		},
 		{
 			Name: "betterbird_restart",
-			Description: "End Betterbird (asked to quit, forced after 10 s), place the module's user.js when the profile " +
+			Description: "End Betterbird (asked to quit, forced after 8 s), place the module's user.js when the profile " +
 				"allows it, and start Betterbird again in the operator's desktop session under the account's service " +
 				"manager (unit mesh-betterbird), its output in the journal. Refused while a message is being written " +
 				"(a composer window is open), because the unsent message would be lost, unless discard_unsent is true — " +

modules/betterbird/cmd/betterbird-tools/review_test.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"context"
 	"strings"
 	"testing"
 	"time"
@@ -139,3 +140,43 @@ func TestTheReminderTestIsNeverTheOperatorsBetterbirdAndCleansUpWhenItEnds(t *te
 		t.Fatalf("the unit removes its profile when it ends: %v", f.calls)
 	}
 }
+
+func TestAComposerIsKnownByItsInstanceWhateverItsClass(t *testing.T) {
+	f := newFake(t)
+	f.desktopSession()
+	f.betterbird()
+	links(t, nil)
+	f.answer = func(name string, args []string) Output {
+		if name == "xwininfo" {
+			return Output{Stdout: `     0x2a01234 "Write: x": ("Msgcompose" "Betterbird")  900x700+10+10  +10+10` + "\n"}
+		}
+		return Output{}
+	}
+	if _, err := f.Restart(false); err == nil || !strings.Contains(err.Error(), "composer") {
+		t.Fatalf("a composer of another class is still a composer: %v", err)
+	}
+}
+
+func TestARestartsWorstCaseStaysUnderTheRuntimesThirtySeconds(t *testing.T) {
+	f := newFake(t)
+	f.desktopSession()
+	f.betterbird()
+	links(t, nil)
+	f.stubborn[4000] = true
+	total := time.Duration(0)
+	run := f.Run
+	f.Run = func(ctx context.Context, env []string, name string, args ...string) Output {
+		// Every command takes its whole timeout: the worst case.
+		if d, ok := ctx.Deadline(); ok {
+			total += time.Until(d).Round(time.Second)
+		}
+		return run(ctx, env, name, args...)
+	}
+	f.Sleep = func(d time.Duration) { total += d }
+	if _, err := f.Restart(true); err != nil {
+		t.Fatal(err)
+	}
+	if total > 26*time.Second {
+		t.Fatalf("the worst case is %v, over the runtime's 30 s less a margin", total)
+	}
+}