fix(ios): find a popover dismiss region in one query instead of reading every element

.github/workflows/ios.yml

@@ -200,6 +200,9 @@ jobs:
             -only-testing:AgentDeviceRunnerUITests/RunnerTests/testAlertHittableProbeCompletingAfterDeadlineLeavesTheOriginalUntouched \
             -only-testing:AgentDeviceRunnerUITests/RunnerTests/testAlertActivationIgnoresAnAppThatNeverSettlesBeforeTheDeadline \
             -only-testing:AgentDeviceRunnerUITests/RunnerTests/testAlertActivationDoesNotWaitOutANotificationBanner \
+            -only-testing:AgentDeviceRunnerUITests/RunnerTests/testAlertResolutionWithoutAnAlertDoesNotReadEveryElementOfTheScreen \
+            -only-testing:AgentDeviceRunnerUITests/RunnerTests/testAlertResolutionFindsADismissPopupMarkerOnACrowdedScreen \
+            -only-testing:AgentDeviceRunnerUITests/RunnerTests/testAlertResolutionFindsAWindowThatIsItselfTheDismissPopupMarker \
             -only-testing:AgentDeviceRunnerUITests/RunnerTests/testSystemModalProbeSliceSharesAndClampsToPlanDeadline \
             -only-testing:AgentDeviceRunnerUITests/RunnerTests/testDispatchRecoverySkipsBookkeepingWhileXCTestChannelOccupied \
             -only-testing:AgentDeviceRunnerUITests/RunnerTests/testBoundedSystemModalProbeTimeoutRecoversThenReleasesOnDrain \

CHANGELOG.md

@@ -8,6 +8,12 @@
   the parser read only the old element, so every row resolved no stack at all. Both spellings now
   parse through the same `id`/`ref` resolution, so a profile recorded with an older Xcode reports
   what it did before. (#2860)
+- Fixed (ios): `alert get`, `accept`, or `dismiss` with no alert on screen no longer reads every
+  element of the app to look for a popover's dismiss region. That walk cost one XCTest round trip
+  per element, plus XCTest's retry cycle for each element that vanished mid-walk. On a loading
+  WebView it outran the 10 s alert budget and kept the runner's main thread busy for more than 30 s
+  after the command failed, so later commands failed with `RUNNER_BUSY`. The dismiss region is now
+  found with one predicate query per window set. (#2491)
 - Changed (apple): a read-only runner command is resent inside the same request only when the
   runner refused it as `RUNNER_BUSY`. Before, any `COMMAND_FAILED` carrying `details.retriable:
   true` was sent up to three times. That flag tells a caller's own poll, such as `wait`, to try

apple/runner/AgentDeviceRunner/AgentDeviceRunner/AgentDeviceRunnerApp.m

@@ -295,6 +295,24 @@ - (void)viewDidLoad {
     ]];
   }
 
+  if ([NSProcessInfo.processInfo.arguments containsObject:@"--agent-device-crowded-screen"]) {
+    for (NSUInteger row = 0; row < 500; row++) {
+      UILabel *rowLabel = [[UILabel alloc] initWithFrame:CGRectMake(0, 60 + (row % 30) * 24, 400, 16)];
+      rowLabel.text = [NSString stringWithFormat:@"Crowded row %lu", (unsigned long)row];
+      rowLabel.accessibilityIdentifier = [NSString stringWithFormat:@"agent-device-crowded-row-%lu", (unsigned long)row];
+      [self.view addSubview:rowLabel];
+    }
+  }
+
+  BOOL markedButton = [NSProcessInfo.processInfo.arguments containsObject:@"--agent-device-dismiss-popup"];
+  if (markedButton || [NSProcessInfo.processInfo.arguments containsObject:@"--agent-device-dismiss-popup-window"]) {
+    UIButton *dismissRegion = [UIButton buttonWithType:UIButtonTypeSystem];
+    dismissRegion.accessibilityIdentifier = markedButton ? @" Dismiss Popup " : @"agent-device-close-popover";
+    [dismissRegion setTitle:@"Close popover" forState:UIControlStateNormal];
+    dismissRegion.frame = CGRectMake(40, 40, 200, 44);
+    [self.view addSubview:dismissRegion];
+  }
+
   if ([NSProcessInfo.processInfo.arguments containsObject:@"--agent-device-selector-read-regression"]) {
     NSString *const duplicateIdentifier = @"agent-device-selector-read-duplicate";
 
@@ -338,6 +356,11 @@ - (void)scene:(UIScene *)scene
 
   self.window = [[UIWindow alloc] initWithWindowScene:(UIWindowScene *)scene];
   self.window.rootViewController = [[AgentDeviceRunnerViewController alloc] init];
+#if TARGET_OS_IOS
+  if ([NSProcessInfo.processInfo.arguments containsObject:@"--agent-device-dismiss-popup-window"]) {
+    self.window.accessibilityIdentifier = @"Dismiss popup";
+  }
+#endif
   [self.window makeKeyAndVisible];
 }
 

apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Alert.swift

@@ -161,22 +161,26 @@ extension RunnerTests {
     elements.first { isVisibleElement($0) }
   }
 
+  /// The marker is matched inside XCTest's query, so the screen is read once per query. Reading each
+  /// descendant instead costs one round trip per element, and on a screen whose tree changes while
+  /// it is read (a loading web view) each vanished element adds XCTest's retry cycle. `containing`
+  /// also matches a window that is itself the marker.
   private func firstDismissPopupWindow(in app: XCUIApplication) -> XCUIElement? {
-    safeElementsQuery {
-      app.windows.allElementsBoundByIndex
-    }.first { window in
-      if !isVisibleElement(window) { return false }
-      if isDismissPopupMarker(window.label) || isDismissPopupMarker(window.identifier) {
-        return true
-      }
-      return safeElementsQuery {
-        window.descendants(matching: .any).allElementsBoundByIndex
-      }.contains { descendant in
-        isDismissPopupMarker(descendant.label) || isDismissPopupMarker(descendant.identifier)
-      }
-    }
+    firstExistingElement(in: safeElementsQuery {
+      app.windows.containing(Self.dismissPopupMarker).allElementsBoundByIndex
+    })
   }
 
+  /// The one definition of a popover's dismiss region: a label or identifier that reads "dismiss
+  /// popup", in any case, with any surrounding whitespace. XCTest queries take it as a format predicate.
+  private static let dismissPopupMarkerPattern = #"\s*dismiss popup\s*"#
+  private static let dismissPopupMarker = NSPredicate(
+    format: "label MATCHES[c] %@ OR identifier MATCHES[c] %@",
+    dismissPopupMarkerPattern,
+    dismissPopupMarkerPattern
+  )
+  private static let dismissPopupMarkerText = NSPredicate(format: "SELF MATCHES[c] %@", dismissPopupMarkerPattern)
+
   private func chooseAlertButton(_ buttons: [XCUIElement], action: String) -> XCUIElement? {
     if action == "accept" {
       if let accept = buttons.first(where: { isAcceptButton($0.label) }) {
@@ -285,7 +289,7 @@ extension RunnerTests {
     return hittable
   }
 
-  private func isDismissPopupMarker(_ label: String) -> Bool {
-    label.trimmingCharacters(in: .whitespacesAndNewlines).caseInsensitiveCompare("dismiss popup") == .orderedSame
+  func isDismissPopupMarker(_ text: String) -> Bool {
+    Self.dismissPopupMarkerText.evaluate(with: text)
   }
 }

apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+AlertObservation.swift

@@ -68,9 +68,8 @@ extension RunnerTests {
   }
 
   private func containsDismissPopupMarker(_ snapshot: XCUIElementSnapshot) -> Bool {
-    [snapshot.label, snapshot.identifier].contains {
-      $0.trimmingCharacters(in: .whitespacesAndNewlines).caseInsensitiveCompare("dismiss popup") == .orderedSame
-    } || snapshot.children.contains { containsDismissPopupMarker($0) }
+    [snapshot.label, snapshot.identifier].contains { isDismissPopupMarker($0) } ||
+      snapshot.children.contains { containsDismissPopupMarker($0) }
   }
 
   private func alertPresentation(_ snapshot: XCUIElementSnapshot) -> RunnerAlertPresentation {

apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+AlertTests.swift

@@ -7,3 +7,53 @@ extension RunnerTests {
   }
 }
 #endif
+
+#if AGENT_DEVICE_RUNNER_UNIT_TESTS && os(iOS)
+extension RunnerTests {
+  func testAlertResolutionWithoutAnAlertDoesNotReadEveryElementOfTheScreen() throws {
+    launchCrowdedScreen(extraArguments: [])
+    defer { terminateCrowdedScreen() }
+
+    let startedAt = Date()
+    XCTAssertNil(resolveAlert(app: app, deadline: startedAt.addingTimeInterval(RunnerTests.defaultAlertCommandTimeout)))
+    // Half the command's own budget: an absent alert answers ALERT_NOT_FOUND well inside it on a
+    // contended host, while a read per element spends it many times over on this screen.
+    XCTAssertLessThan(Date().timeIntervalSince(startedAt), RunnerTests.defaultAlertCommandTimeout / 2)
+  }
+
+  func testAlertResolutionFindsADismissPopupMarkerOnACrowdedScreen() throws {
+    launchCrowdedScreen(extraArguments: ["--agent-device-dismiss-popup"])
+    defer { terminateCrowdedScreen() }
+
+    let alert = try XCTUnwrap(
+      resolveAlert(app: app, deadline: Date().addingTimeInterval(RunnerTests.defaultAlertCommandTimeout))
+    )
+    XCTAssertEqual(alert.source, .dismissPopup)
+    XCTAssertEqual(alert.root.elementType, .window)
+    XCTAssertTrue(alert.buttons.contains { $0.identifier == " Dismiss Popup " })
+  }
+
+  func testAlertResolutionFindsAWindowThatIsItselfTheDismissPopupMarker() throws {
+    launchCrowdedScreen(extraArguments: ["--agent-device-dismiss-popup-window"])
+    defer { terminateCrowdedScreen() }
+
+    let alert = try XCTUnwrap(
+      resolveAlert(app: app, deadline: Date().addingTimeInterval(RunnerTests.defaultAlertCommandTimeout))
+    )
+    XCTAssertEqual(alert.source, .dismissPopup)
+    XCTAssertEqual(alert.root.identifier, "Dismiss popup")
+    XCTAssertTrue(alert.buttons.contains { $0.identifier == "agent-device-close-popover" })
+  }
+
+  private func launchCrowdedScreen(extraArguments: [String]) {
+    app.launchArguments = ["--agent-device-crowded-screen"] + extraArguments
+    app.launch()
+    XCTAssertTrue(app.staticTexts["agent-device-crowded-row-499"].waitForExistence(timeout: appExistenceTimeout))
+  }
+
+  private func terminateCrowdedScreen() {
+    invalidateCachedTarget(reason: "unit_test_cleanup")
+    app.terminate()
+  }
+}
+#endif