fix(android): target the selected device in adb reads; don't strand nav mode

Review fixes:
- ForegroundPackage/FocusedWindowPackage now take a serial and pass -s, so the
  foreground/scope guard works when several devices are attached (the --device
  path). Previously they ran bare `adb shell`, which errors with multiple
  devices, silently disabling app-scope enforcement. The sidecar client passes
  its serial through.
- Extract an adbArgs helper and route every adb call through it, removing four
  duplicated serial-arg builders.
- ForceThreeButtonNav now decides what to restore before changing anything: if
  the current mode is unknown or already 3-button it leaves nav untouched,
  instead of switching and then stranding the device in 3-button. Logic split
  into the pure navModeToRestore, now unit tested.
This commit is contained in:
pj committed 2026-06-10 21:19:30 +05:30
1 parent 542bf11a02
commit c48b13f304
3 files changed
+111 -56

No files matched your search

+41 -36
View File
@@ -63,6 +63,17 @@ func EnsureDevice(ctx context.Context, serial, avdName string, stdout io.Writer)
// Every step is best effort: some OEM builds restrict or kill these commands // Every step is best effort: some OEM builds restrict or kill these commands
// (e.g. HyperOS SIGKILLs `svc power stayon`), and none is required for a run to // (e.g. HyperOS SIGKILLs `svc power stayon`), and none is required for a run to
// proceed, so a failure is logged and skipped rather than aborting the run. // proceed, so a failure is logged and skipped rather than aborting the run.
// adbArgs prepends the device selector when a serial is set, so every adb
// invocation targets the chosen device. Without it, `adb` fails on a host with
// more than one device attached, which silently disables anything that reads
// adb output (the foreground/scope guard).
func adbArgs(serial string, args ...string) []string {
if serial == "" {
return args
}
return append([]string{"-s", serial}, args...)
}
func PrepareDevice(ctx context.Context, serial string, stdout io.Writer) error { func PrepareDevice(ctx context.Context, serial string, stdout io.Writer) error {
adb, err := AdbBinary() adb, err := AdbBinary()
if err != nil { if err != nil {
@@ -73,11 +84,7 @@ func PrepareDevice(ctx context.Context, serial string, stdout io.Writer) error {
{"input", "keyevent", "KEYCODE_WAKEUP"}, {"input", "keyevent", "KEYCODE_WAKEUP"},
{"wm", "dismiss-keyguard"}, {"wm", "dismiss-keyguard"},
}, antiFreezeCommands()...) { }, antiFreezeCommands()...) {
args := []string{} args := adbArgs(serial, append([]string{"shell"}, shellCommand...)...)
if serial != "" {
args = append(args, "-s", serial)
}
args = append(append(args, "shell"), shellCommand...)
if err := exec.CommandContext(ctx, adb, args...).Run(); err != nil { if err := exec.CommandContext(ctx, adb, args...).Run(); err != nil {
fmt.Fprintf(stdout, "device prep: skipping `adb %s` (%v)\n", strings.Join(shellCommand, " "), err) fmt.Fprintf(stdout, "device prep: skipping `adb %s` (%v)\n", strings.Join(shellCommand, " "), err)
} }
@@ -121,16 +128,10 @@ func ReinstallApp(ctx context.Context, serial, bundleID, apkPath string, stdout
if err != nil { if err != nil {
return err return err
} }
withSerial := func(args ...string) []string { if output, err := exec.CommandContext(ctx, adb, adbArgs(serial, "uninstall", bundleID)...).CombinedOutput(); err != nil {
if serial != "" {
return append([]string{"-s", serial}, args...)
}
return args
}
if output, err := exec.CommandContext(ctx, adb, withSerial("uninstall", bundleID)...).CombinedOutput(); err != nil {
fmt.Fprintf(stdout, "clear-state: uninstall %s skipped (%v: %s)\n", bundleID, err, strings.TrimSpace(string(output))) fmt.Fprintf(stdout, "clear-state: uninstall %s skipped (%v: %s)\n", bundleID, err, strings.TrimSpace(string(output)))
} }
if output, err := exec.CommandContext(ctx, adb, withSerial("install", "-r", apkPath)...).CombinedOutput(); err != nil { if output, err := exec.CommandContext(ctx, adb, adbArgs(serial, "install", "-r", apkPath)...).CombinedOutput(); err != nil {
return fmt.Errorf("install %s: %w: %s", apkPath, err, strings.TrimSpace(string(output))) return fmt.Errorf("install %s: %w: %s", apkPath, err, strings.TrimSpace(string(output)))
} }
return nil return nil
@@ -157,30 +158,39 @@ func ForceThreeButtonNav(ctx context.Context, serial string, stdout io.Writer) f
if err != nil { if err != nil {
return func() {} return func() {}
} }
original := enabledNavOverlay(ctx, adb, serial) // Decide before changing anything: if the current mode is unknown (an OEM
// overlay, or a parse failure) or already 3-button, there is nothing to
// restore, so leave navigation untouched rather than stranding the device in
// 3-button after the run.
restore := navModeToRestore(enabledNavOverlay(ctx, adb, serial))
if restore == "" {
return func() {}
}
if err := navOverlayCommand(ctx, adb, serial, threeButtonNavOverlay).Run(); err != nil { if err := navOverlayCommand(ctx, adb, serial, threeButtonNavOverlay).Run(); err != nil {
fmt.Fprintf(stdout, "device prep: skipping 3-button nav (%v)\n", err) fmt.Fprintf(stdout, "device prep: skipping 3-button nav (%v)\n", err)
return func() {} return func() {}
} }
if original == "" || original == threeButtonNavOverlay {
return func() {}
}
return func() { return func() {
if err := navOverlayCommand(context.Background(), adb, serial, original).Run(); err != nil { if err := navOverlayCommand(context.Background(), adb, serial, restore).Run(); err != nil {
fmt.Fprintf(stdout, "device prep: could not restore nav mode %s (%v)\n", original, err) fmt.Fprintf(stdout, "device prep: could not restore nav mode %s (%v)\n", restore, err)
} }
} }
} }
// navModeToRestore returns the navigation overlay to restore after forcing
// 3-button nav, or "" when nothing should change: an unknown current mode (not
// restorable) or one that is already 3-button.
func navModeToRestore(original string) string {
if original == "" || original == threeButtonNavOverlay {
return ""
}
return original
}
// enabledNavOverlay returns the currently active navigation-mode overlay, or "" // enabledNavOverlay returns the currently active navigation-mode overlay, or ""
// when it cannot be determined. // when it cannot be determined.
func enabledNavOverlay(ctx context.Context, adb, serial string) string { func enabledNavOverlay(ctx context.Context, adb, serial string) string {
args := []string{} output, err := exec.CommandContext(ctx, adb, adbArgs(serial, "shell", "cmd", "overlay", "list")...).Output()
if serial != "" {
args = append(args, "-s", serial)
}
args = append(args, "shell", "cmd", "overlay", "list")
output, err := exec.CommandContext(ctx, adb, args...).Output()
if err != nil { if err != nil {
return "" return ""
} }
@@ -204,12 +214,7 @@ func parseEnabledNavOverlay(overlayList string) string {
} }
func navOverlayCommand(ctx context.Context, adb, serial, overlay string) *exec.Cmd { func navOverlayCommand(ctx context.Context, adb, serial, overlay string) *exec.Cmd {
args := []string{} return exec.CommandContext(ctx, adb, adbArgs(serial, "shell", "cmd", "overlay", "enable-exclusive", overlay)...)
if serial != "" {
args = append(args, "-s", serial)
}
args = append(args, "shell", "cmd", "overlay", "enable-exclusive", overlay)
return exec.CommandContext(ctx, adb, args...)
} }
// AdbReverse sets up adb reverse forwarding for a local abstract socket. // AdbReverse sets up adb reverse forwarding for a local abstract socket.
@@ -418,13 +423,13 @@ func waitForBoot(ctx context.Context, timeout time.Duration) error {
} }
// ForegroundPackage returns the package of the currently resumed activity on // ForegroundPackage returns the package of the currently resumed activity on
// the connected device, or "" when it cannot be determined. // the given device, or "" when it cannot be determined.
func ForegroundPackage(ctx context.Context) (string, error) { func ForegroundPackage(ctx context.Context, serial string) (string, error) {
adb, err := AdbBinary() adb, err := AdbBinary()
if err != nil { if err != nil {
return "", err return "", err
} }
output, err := exec.CommandContext(ctx, adb, "shell", "dumpsys", "activity", "activities").Output() output, err := exec.CommandContext(ctx, adb, adbArgs(serial, "shell", "dumpsys", "activity", "activities")...).Output()
if err != nil { if err != nil {
return "", err return "", err
} }
@@ -436,7 +441,7 @@ func ForegroundPackage(ctx context.Context) (string, error) {
// Unlike ForegroundPackage, this reflects what is actually on screen: // Unlike ForegroundPackage, this reflects what is actually on screen:
// ResumedActivity flips to a newly launched app before its first frame renders, // ResumedActivity flips to a newly launched app before its first frame renders,
// while mCurrentFocus only names the app once its window is up. // while mCurrentFocus only names the app once its window is up.
func FocusedWindowPackage(ctx context.Context) (string, error) { func FocusedWindowPackage(ctx context.Context, serial string) (string, error) {
adb, err := AdbBinary() adb, err := AdbBinary()
if err != nil { if err != nil {
return "", err return "", err
@@ -444,7 +449,7 @@ func FocusedWindowPackage(ctx context.Context) (string, error) {
// Grep the focus line on-device: the full dumpsys window output is large and // Grep the focus line on-device: the full dumpsys window output is large and
// this runs on the per-step scope guard, so transferring it whole would add // this runs on the per-step scope guard, so transferring it whole would add
// latency to every step. // latency to every step.
output, err := exec.CommandContext(ctx, adb, "shell", "dumpsys window | grep mCurrentFocus").Output() output, err := exec.CommandContext(ctx, adb, adbArgs(serial, "shell", "dumpsys window | grep mCurrentFocus")...).Output()
if err != nil { if err != nil {
return "", err return "", err
} }
+68 -18
View File
@@ -2,6 +2,7 @@ package android
import ( import (
"reflect" "reflect"
"slices"
"strings" "strings"
"testing" "testing"
) )
@@ -191,6 +192,16 @@ func TestParseFocusedWindowPackage(t *testing.T) {
dumpsys: " mCurrentFocus=Window{885e289 u0 NotificationShade}", dumpsys: " mCurrentFocus=Window{885e289 u0 NotificationShade}",
want: "com.android.systemui", want: "com.android.systemui",
}, },
{
name: "quick settings focused",
dumpsys: " mCurrentFocus=Window{abc u0 QuickSettings}",
want: "com.android.systemui",
},
{
name: "volume dialog focused",
dumpsys: " mCurrentFocus=Window{abc u0 VolumeUiDialog}",
want: "com.android.systemui",
},
} }
for _, tc := range cases { for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) { t.Run(tc.name, func(t *testing.T) {
@@ -217,11 +228,21 @@ func TestParseEnabledNavOverlay(t *testing.T) {
listing: "[x] com.android.internal.systemui.navbar.threebutton\n[ ] com.android.internal.systemui.navbar.gestural", listing: "[x] com.android.internal.systemui.navbar.threebutton\n[ ] com.android.internal.systemui.navbar.gestural",
want: "com.android.internal.systemui.navbar.threebutton", want: "com.android.internal.systemui.navbar.threebutton",
}, },
{
name: "two-button active",
listing: "[x] com.android.internal.systemui.navbar.twobutton\n[ ] com.android.internal.systemui.navbar.gestural",
want: "com.android.internal.systemui.navbar.twobutton",
},
{ {
name: "ignores enabled non-nav overlays", name: "ignores enabled non-nav overlays",
listing: "[x] com.some.other.overlay\n[ ] com.android.internal.systemui.navbar.gestural", listing: "[x] com.some.other.overlay\n[ ] com.android.internal.systemui.navbar.gestural",
want: "", want: "",
}, },
{
name: "no overlay enabled",
listing: "[ ] com.android.internal.systemui.navbar.gestural\n[ ] com.android.internal.systemui.navbar.threebutton",
want: "",
},
} }
for _, tc := range cases { for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) { t.Run(tc.name, func(t *testing.T) {
@@ -234,29 +255,58 @@ func TestParseEnabledNavOverlay(t *testing.T) {
func TestAntiFreezeCommands_DisablesFreezersAndExemptsDriver(t *testing.T) { func TestAntiFreezeCommands_DisablesFreezersAndExemptsDriver(t *testing.T) {
commands := antiFreezeCommands() commands := antiFreezeCommands()
joined := make([]string, len(commands)) has := func(want ...string) bool {
for i, c := range commands { return slices.ContainsFunc(commands, func(c []string) bool { return slices.Equal(c, want) })
joined[i] = strings.Join(c, " ") }
for _, want := range [][]string{
{"device_config", "set_sync_disabled_for_tests", "persistent"},
{"device_config", "put", "activity_manager_native_boot", "use_freezer", "false"},
{"settings", "put", "global", "settings_enable_monitor_phantom_procs", "false"},
{"dumpsys", "deviceidle", "disable"},
} {
if !has(want...) {
t.Errorf("anti-freeze commands missing exact command %v", want)
}
} }
all := strings.Join(joined, "\n")
wantContains := []string{ // The freezer exemption must be one device_config command whose final
"device_config set_sync_disabled_for_tests persistent", // argument lists every driver package, not just the package string
"device_config put activity_manager_native_boot use_freezer false", // appearing somewhere among the commands.
"settings put global settings_enable_monitor_phantom_procs false", exemption := findCommand(commands, "device_config", "put", "activity_manager_native_boot", "freeze_exempt_inst_pkg")
"dumpsys deviceidle disable", if exemption == nil {
} t.Fatalf("no freeze_exempt_inst_pkg command found in %v", commands)
for _, want := range wantContains {
if !strings.Contains(all, want) {
t.Errorf("anti-freeze commands missing %q\ngot:\n%s", want, all)
}
} }
value := exemption[len(exemption)-1]
for _, pkg := range driverPackages { for _, pkg := range driverPackages {
if !strings.Contains(all, "freeze_exempt_inst_pkg") || !strings.Contains(all, pkg) { if !strings.Contains(value, pkg) {
t.Errorf("driver package %q not exempted from freezer\ngot:\n%s", pkg, all) t.Errorf("freeze_exempt_inst_pkg value %q missing driver package %q", value, pkg)
} }
if !strings.Contains(all, "deviceidle whitelist +"+pkg) { if !has("dumpsys", "deviceidle", "whitelist", "+"+pkg) {
t.Errorf("driver package %q not whitelisted from doze\ngot:\n%s", pkg, all) t.Errorf("driver package %q not whitelisted from doze", pkg)
}
}
}
// findCommand returns the first command whose leading tokens equal prefix.
func findCommand(commands [][]string, prefix ...string) []string {
for _, c := range commands {
if len(c) >= len(prefix) && slices.Equal(c[:len(prefix)], prefix) {
return c
}
}
return nil
}
func TestNavModeToRestore(t *testing.T) {
cases := map[string]string{
"com.android.internal.systemui.navbar.gestural": "com.android.internal.systemui.navbar.gestural",
"com.android.internal.systemui.navbar.twobutton": "com.android.internal.systemui.navbar.twobutton",
"com.android.internal.systemui.navbar.threebutton": "", // already 3-button: nothing to change
"": "", // unknown current mode: must not switch what cannot be restored
}
for original, want := range cases {
if got := navModeToRestore(original); got != want {
t.Errorf("navModeToRestore(%q) = %q, want %q", original, got, want)
} }
} }
} }
+2 -2
View File
@@ -54,7 +54,7 @@ func (c *Client) ForegroundApp(ctx context.Context) (string, error) {
if c.platform != "android" { if c.platform != "android" {
return "", nil return "", nil
} }
return android.ForegroundPackage(ctx) return android.ForegroundPackage(ctx, c.serial)
} }
// FocusedWindowApp reports the package owning the focused window. Only Android // FocusedWindowApp reports the package owning the focused window. Only Android
@@ -64,7 +64,7 @@ func (c *Client) FocusedWindowApp(ctx context.Context) (string, error) {
if c.platform != "android" { if c.platform != "android" {
return "", nil return "", nil
} }
return android.FocusedWindowPackage(ctx) return android.FocusedWindowPackage(ctx, c.serial)
} }
// Dial connects to the sidecar gRPC server at the given address. // Dial connects to the sidecar gRPC server at the given address.