perf: stop paying for simctl and a deep snapshot before every simulator dump

devices/devicekit/active-app.go

@@ -16,7 +16,20 @@ type ActiveAppInfo struct {
 }
 
 func (c *DeviceKitClient) GetActiveAppInfo() (*ActiveAppInfo, error) {
-	result, err := c.CallRPC("device.apps.foreground", nil)
+	return c.getActiveAppInfo(true)
+}
+
+// GetActiveAppIdentity names the foreground app without asking which view
+// controller is on screen. The agent finds that by snapshotting the hierarchy,
+// which on an app with a large one costs as much as a UI dump; callers that
+// only need to know which app is in front should not pay for it.
+func (c *DeviceKitClient) GetActiveAppIdentity() (*ActiveAppInfo, error) {
+	return c.getActiveAppInfo(false)
+}
+
+func (c *DeviceKitClient) getActiveAppInfo(withViewController bool) (*ActiveAppInfo, error) {
+	// older agents ignore the parameter and always report the view controller
+	result, err := c.CallRPC("device.apps.foreground", map[string]bool{"viewController": withViewController})
 	if err != nil {
 		return nil, fmt.Errorf("failed to get active app info: %w", err)
 	}

devices/ios_device_flutter.go

@@ -68,7 +68,7 @@ func advertisesDartVMService(app installationproxy.AppInfo) bool {
 // determined it says yes with no bundle id, which is the old probe-over-LLDB
 // behaviour.
 func (d *IOSDevice) foregroundFlutterCandidate() (bundleID string, mayBeFlutter bool) {
-	activeApp, err := d.deviceKitClient.GetActiveAppInfo()
+	activeApp, err := d.deviceKitClient.GetActiveAppIdentity()
 	if err != nil || activeApp.ProcessID == 0 {
 		utils.Verbose("flutter: cannot tell the foreground app, probing anyway: %v", err)
 		return "", true

devices/simulator_flutter.go

@@ -9,6 +9,7 @@ import (
 	"path/filepath"
 	"regexp"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/mobile-next/mobilecli/types"
@@ -33,12 +34,8 @@ var vmServiceLineURL = regexp.MustCompile(`http://127\.0\.0\.1:\d+/[A-Za-z0-9_=-
 // tryDumpFlutterSource returns the Flutter render tree for the foreground app,
 // or ok=false to signal the caller should use the accessibility dump.
 func (s *SimulatorDevice) tryDumpFlutterSource() ([]types.ScreenElement, bool) {
-	foreground, err := s.GetForegroundApp()
-	if err != nil {
-		return nil, false
-	}
-	bundleID := foreground.PackageName
-	if !s.isFlutterAppBundle(bundleID) {
+	bundleID, mayBeFlutter := s.foregroundFlutterCandidate()
+	if !mayBeFlutter {
 		return nil, false
 	}
 	uri := s.flutterVMServiceURI(bundleID)
@@ -56,6 +53,69 @@ func (s *SimulatorDevice) tryDumpFlutterSource() ([]types.ScreenElement, bool) {
 	return elements, true
 }
 
+// simulatorFlutterCandidates caches the verdict of foregroundFlutterCandidate,
+// keyed by simulator udid. SimulatorDevice methods run against a copy of the
+// device, so the cache cannot live in the struct the way it does for a real
+// device.
+var (
+	simulatorFlutterMu         sync.Mutex
+	simulatorFlutterCandidates = map[string]flutterCandidate{}
+)
+
+// cachedFlutterCandidate returns the verdict remembered for the process running
+// on a simulator, and whether there is one. A different pid means the app was
+// relaunched or reinstalled, which is a miss: the bundle may be another build.
+func cachedFlutterCandidate(udid string, pid int) (isCandidate bool, ok bool) {
+	if pid == 0 {
+		return false, false
+	}
+
+	simulatorFlutterMu.Lock()
+	defer simulatorFlutterMu.Unlock()
+
+	cached, exists := simulatorFlutterCandidates[udid]
+	if !exists || cached.pid != pid {
+		return false, false
+	}
+	return cached.isCandidate, true
+}
+
+// rememberFlutterCandidate stores the verdict for a process. A pid of 0 means
+// the agent did not report one, and a verdict that cannot be invalidated is
+// worse than no verdict at all, so it is dropped.
+func rememberFlutterCandidate(udid string, pid int, isCandidate bool) {
+	if pid == 0 {
+		return
+	}
+
+	simulatorFlutterMu.Lock()
+	defer simulatorFlutterMu.Unlock()
+	simulatorFlutterCandidates[udid] = flutterCandidate{pid: pid, isCandidate: isCandidate}
+}
+
+// foregroundFlutterCandidate decides whether looking for a Dart VM is worth it,
+// and names the foreground app. Reading the app bundle costs a simctl call, more
+// than the whole accessibility dump of a light screen, and every dump would pay
+// it; the answer cannot change while the process runs, so it is remembered per
+// process id, as on a real device. A relaunched or reinstalled app gets a new
+// pid and is looked at again.
+func (s *SimulatorDevice) foregroundFlutterCandidate() (bundleID string, mayBeFlutter bool) {
+	activeApp, err := s.deviceKitClient.GetActiveAppIdentity()
+	if err != nil || activeApp.BundleID == "" {
+		utils.Verbose("flutter: cannot tell the foreground app, skipping the flutter dump: %v", err)
+		return "", false
+	}
+
+	if isCandidate, ok := cachedFlutterCandidate(s.UDID, activeApp.ProcessID); ok {
+		return activeApp.BundleID, isCandidate
+	}
+
+	isCandidate := s.isFlutterAppBundle(activeApp.BundleID)
+	rememberFlutterCandidate(s.UDID, activeApp.ProcessID, isCandidate)
+
+	return activeApp.BundleID, isCandidate
+}
+
 // isFlutterAppBundle reports whether the installed app embeds Flutter.framework.
 func (s *SimulatorDevice) isFlutterAppBundle(bundleID string) bool {
 	out, err := runSimctl("get_app_container", s.UDID, bundleID, "app")

devices/simulator_flutter_test.go

@@ -0,0 +1,65 @@
+package devices
+
+import "testing"
+
+// A verdict that outlives the process it was taken for is the only way this
+// cache can be wrong, and it would be wrong silently: a Flutter app would be
+// dumped through the accessibility tree, or a native app would be probed for a
+// Dart VM on every dump. The tests below pin the rules that prevent that.
+func TestFlutterCandidateIsRememberedForTheSameProcess(t *testing.T) {
+	t.Cleanup(func() { delete(simulatorFlutterCandidates, "udid-1") })
+
+	if _, ok := cachedFlutterCandidate("udid-1", 100); ok {
+		t.Fatal("an unknown simulator must not have a verdict")
+	}
+
+	rememberFlutterCandidate("udid-1", 100, true)
+
+	isCandidate, ok := cachedFlutterCandidate("udid-1", 100)
+	if !ok || !isCandidate {
+		t.Errorf("the verdict for the same process must be reused, got (%v, %v)", isCandidate, ok)
+	}
+}
+
+// A reinstall can replace a native build with a Flutter one under the same
+// bundle id, so the pid, not the bundle id, is what the verdict is tied to.
+func TestFlutterCandidateIsLookedAtAgainAfterTheAppRestarts(t *testing.T) {
+	t.Cleanup(func() { delete(simulatorFlutterCandidates, "udid-2") })
+
+	rememberFlutterCandidate("udid-2", 100, false)
+
+	if _, ok := cachedFlutterCandidate("udid-2", 101); ok {
+		t.Error("a relaunched or reinstalled app has a new pid and must be looked at again")
+	}
+}
+
+// The agent reports pid 0 when it cannot name the foreground process. Storing
+// a verdict against it would pin an answer that no app relaunch could ever
+// clear, so paying the simctl call again is the cheaper mistake.
+func TestFlutterCandidateIsNotRememberedWithoutAProcessID(t *testing.T) {
+	t.Cleanup(func() { delete(simulatorFlutterCandidates, "udid-3") })
+
+	rememberFlutterCandidate("udid-3", 0, true)
+
+	if _, ok := cachedFlutterCandidate("udid-3", 0); ok {
+		t.Error("a verdict that cannot be invalidated must not be stored")
+	}
+	if _, ok := cachedFlutterCandidate("udid-3", 100); ok {
+		t.Error("a verdict stored without a pid must not be served to a real process")
+	}
+}
+
+// Pids are per simulator, so two booted simulators running different apps can
+// hand out the same number; the udid has to be part of the key.
+func TestFlutterCandidatesAreNotSharedBetweenSimulators(t *testing.T) {
+	t.Cleanup(func() {
+		delete(simulatorFlutterCandidates, "udid-4")
+		delete(simulatorFlutterCandidates, "udid-5")
+	})
+
+	rememberFlutterCandidate("udid-4", 100, true)
+
+	if _, ok := cachedFlutterCandidate("udid-5", 100); ok {
+		t.Error("two simulators can run the same pid; a verdict must not cross over")
+	}
+}