From 4b0d49ca7ced1bf580794d6690472642538fcd31 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Sun, 3 May 2026 18:14:07 +0300 Subject: [PATCH 1/2] Fix oversized dialogs in iOS Modern and Android Material themes Dialog.show() was producing ~60%x70% screen-sized dialogs even when empty. Without dialogPosition, Dialog.showImpl falls through to Form.showDialog which applies hard-coded 20%/10%/20% screen-edge margins regardless of content. Setting dialogPosition: Center routes through showPacked so the dialog sizes to its content's preferred size. Co-Authored-By: Claude Opus 4.7 (1M context) --- native-themes/android-material/theme.css | 8 +++++--- native-themes/ios-modern/theme.css | 15 ++++++++++----- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/native-themes/android-material/theme.css b/native-themes/android-material/theme.css index 410a3d868e..f2dcdd6439 100644 --- a/native-themes/android-material/theme.css +++ b/native-themes/android-material/theme.css @@ -83,9 +83,11 @@ switchTrackOffOutlineColor: "79747e"; switchThumbInsetMM: "0"; /* Dialog sizing/layout constants - see iOS Modern theme for the - rationale. Without these the framework falls back to a wider - default Dialog layout that ballooned Dialog.show() to near-full - screen with the body floating in empty space. */ + rationale. dialogPosition is the load-bearing one (routes + Dialog.show() through showPacked so it sizes to content); + without it Form.showDialog's hard-coded 20%/10%/20% screen + margins make even an empty dialog ~60%x70% of the screen. */ + dialogPosition: Center; hideEmptyTitleBool: true; shrinkPopupTitleBool: true; dialogButtonCommandsBool: true; diff --git a/native-themes/ios-modern/theme.css b/native-themes/ios-modern/theme.css index db47f23eb5..8e96338115 100644 --- a/native-themes/ios-modern/theme.css +++ b/native-themes/ios-modern/theme.css @@ -91,11 +91,16 @@ checkBoxUncheckedIconInt: 59693; radioCheckedIconInt: 59447; radioUncheckedIconInt: 59446; - /* Dialog sizing/layout constants the legacy iOS7 theme set and - the modern theme had been missing. Without these the framework - falls back to a wider default Dialog layout that left - Dialog.show("Clicked","Clicked item N","OK",null) ballooning to - near-full screen with the body floating in empty space. */ + /* Dialog sizing/layout constants. dialogPosition is the load-bearing + one: without it Dialog.show() falls through to Form.showDialog() + which always positions the dialog with 20%/10%/20% screen-edge + margins (Form.java showDialog), making the dialog ~60%x70% of + the screen regardless of content - so even an empty dialog reads + as huge. With dialogPosition set, Dialog.showImpl routes through + showPacked() which sizes to the content's preferred size. The + boolean constants below tune button-area rendering (compact + title, grid commands) but don't affect outer sizing. */ + dialogPosition: Center; hideEmptyTitleBool: true; shrinkPopupTitleBool: true; dialogButtonCommandsBool: true; From 6df4f68de320f9492ea19ba599f5affde1546522 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Sun, 3 May 2026 18:56:09 +0300 Subject: [PATCH 2/2] Android CI: harden test retry against PackageManager indexing race The instrumentation test runner already retries decode-only failures (logcat occasionally drops a chunk line, breaking PNG reassembly) by restarting the app and re-emitting from the on-device suite. The retry itself was failing in two ways: 1. After `adb install -r`, `am start -W -a MAIN -c LAUNCHER -p ` returned "Activity not started, unable to resolve Intent" because PackageManager hadn't finished indexing the freshly-installed APK. The script gave up immediately and skipped the 10-minute retry wait, so the failed test never got a second chance. 2. The original logcat capture used the device's default ring buffer (256K-1M), which can wrap mid-suite when 90+ tests each emit ~70 chunk lines. That's the root cause of the decode flakes the retry was supposed to recover from. Changes: - Bump the device-side logcat ring buffer to 16M with `adb logcat -G` before clearing it. Mitigates buffer wrap during long suites. - After `adb install`, poll `cmd package resolve-activity --brief` (max 30s) until pm reports the launcher activity is registered. - Retry `am start` up to 3 times with a 2s backoff to absorb residual indexing race. - Fall back to `monkey -p -c LAUNCHER 1` if `am start` still refuses to resolve the Intent. `pidof` after launch remains the source of truth for whether the app actually came up. Co-Authored-By: Claude Opus 4.7 (1M context) --- scripts/run-android-instrumentation-tests.sh | 106 ++++++++++++++++--- 1 file changed, 90 insertions(+), 16 deletions(-) diff --git a/scripts/run-android-instrumentation-tests.sh b/scripts/run-android-instrumentation-tests.sh index 6823aeb19f..45c05c10b4 100755 --- a/scripts/run-android-instrumentation-tests.sh +++ b/scripts/run-android-instrumentation-tests.sh @@ -139,6 +139,16 @@ ADB_BIN="$(command -v adb)" ra_log "ADB connected devices:" "$ADB_BIN" devices -l | sed 's/^/[run-android-instrumentation-tests] /' +# Bump the device-side logcat ring buffer before clearing it. The default on +# Android emulators is 256K-1M per buffer, which is too small for our test +# suite: a single screenshot can emit ~70 base64 chunk lines (~500 bytes +# each), and across 90+ tests the main buffer has been observed to wrap +# mid-suite, dropping a chunk line and causing `Cn1ssChunkTools` to fail +# reassembly with a gap error. 16 MiB is plenty for a single suite run and +# matches what `adb logcat -G` accepts on every supported emulator image. +# Failing to set the buffer is non-fatal — older platforms silently ignore -G. +ra_log "Bumping device logcat ring buffer to 16M (mitigates chunk-line drops)" +"$ADB_BIN" logcat -G 16M >/dev/null 2>&1 || true ra_log "Clearing logcat buffer" "$ADB_BIN" logcat -c || true @@ -370,6 +380,40 @@ if [ "${#FAILED_TESTS[@]}" -gt 0 ] && [ -n "${PACKAGE_NAME:-}" ]; then if [ "$INSTALL_RC" -ne 0 ]; then ra_log "WARN: adb install reported failure; the am start below will likely fail too" fi + + # Wait for PackageManager to finish indexing the freshly-installed APK. + # `adb install` returns Success as soon as the APK lands on the device, + # but the launcher Intent isn't resolvable until pm has registered + # every component in the manifest. Without this wait, `am start` in the + # following block races and prints "Activity not started, unable to + # resolve Intent" — exactly the failure the previous version of this + # block kept hitting on busy emulators (see PR #4856 build for the + # observed timing). Poll `cmd package resolve-activity --brief` for + # the same MAIN/LAUNCHER intent we are about to fire; it returns the + # `/<.Activity>` triple once the launcher is ready. Cap at 30s + # because indexing usually finishes in < 5s but slow CI runners can + # take longer. + ra_log "Waiting for PackageManager to register launcher activity for $PACKAGE_NAME" + RESOLVE_DEADLINE=$(( $(date +%s) + 30 )) + LAUNCHER_RESOLVED=0 + while [ "$(date +%s)" -lt "$RESOLVE_DEADLINE" ]; do + RESOLVE_OUT="$("$ADB_BIN" shell cmd package resolve-activity --brief \ + -a android.intent.action.MAIN \ + -c android.intent.category.LAUNCHER \ + "$PACKAGE_NAME" 2>/dev/null | tr -d '\r')" || RESOLVE_OUT="" + # `resolve-activity --brief` prints the matching component on a line + # of the form `/`. Newer Android prepends a + # `priority=...` line we want to ignore, so match by substring. + if printf '%s\n' "$RESOLVE_OUT" | grep -q "^${PACKAGE_NAME}/"; then + LAUNCHER_RESOLVED=1 + ra_log "PackageManager resolved launcher: $(printf '%s\n' "$RESOLVE_OUT" | grep "^${PACKAGE_NAME}/" | head -n1)" + break + fi + sleep 1 + done + if [ "$LAUNCHER_RESOLVED" -eq 0 ]; then + ra_log "WARN: PackageManager did not report launcher activity within 30s; attempting am start anyway" + fi else ra_log "ERROR: cannot reinstall — APK not found at $MAIN_APK" fi @@ -384,23 +428,53 @@ if [ "${#FAILED_TESTS[@]}" -gt 0 ] && [ -n "${PACKAGE_NAME:-}" ]; then # (the script would exit at the failing pipeline). Logging both the exit # code and the raw output is the whole point of this block — see the # "RIGHT FIX" comment above. - ra_log "Relaunching $PACKAGE_NAME via 'am start -W -a MAIN -c LAUNCHER -p $PACKAGE_NAME'" - if AM_START_OUT="$("$ADB_BIN" shell am start -W \ - -a android.intent.action.MAIN \ - -c android.intent.category.LAUNCHER \ - -p "$PACKAGE_NAME" 2>&1 | tr -d '\r')"; then - AM_START_RC=0 - else - AM_START_RC=$? - fi - ra_log "am start exit=${AM_START_RC}, output:" - printf '%s\n' "$AM_START_OUT" | sed 's/^/[run-android-instrumentation-tests] /' - - AM_STATUS="$(printf '%s\n' "$AM_START_OUT" \ - | awk -F= '/^[[:space:]]*Status[[:space:]]*=/ {print $2; exit}' \ - | tr -d '[:space:]')" + # + # Retry the start a few times. Even after `cmd package resolve-activity` + # reports the launcher, `am start` can race a still-completing + # PackageManager scan and return "unable to resolve Intent". Each retry + # waits 2s — empirically enough to clear the race. + AM_STATUS="" + for _amattempt in 1 2 3; do + ra_log "Relaunching $PACKAGE_NAME via 'am start -W -a MAIN -c LAUNCHER -p $PACKAGE_NAME' (attempt ${_amattempt})" + if AM_START_OUT="$("$ADB_BIN" shell am start -W \ + -a android.intent.action.MAIN \ + -c android.intent.category.LAUNCHER \ + -p "$PACKAGE_NAME" 2>&1 | tr -d '\r')"; then + AM_START_RC=0 + else + AM_START_RC=$? + fi + ra_log "am start exit=${AM_START_RC}, output:" + printf '%s\n' "$AM_START_OUT" | sed 's/^/[run-android-instrumentation-tests] /' + AM_STATUS="$(printf '%s\n' "$AM_START_OUT" \ + | awk -F= '/^[[:space:]]*Status[[:space:]]*=/ {print $2; exit}' \ + | tr -d '[:space:]')" + if [ "${AM_STATUS:-}" = "ok" ]; then + break + fi + if [ "$_amattempt" -lt 3 ]; then + ra_log "am start did not report Status=ok (got '${AM_STATUS:-}'); retrying in 2s" + sleep 2 + fi + done if [ "${AM_STATUS:-}" != "ok" ]; then - ra_log "WARN: am start did not report Status=ok (got '${AM_STATUS:-}'); the app may not be running" + ra_log "WARN: am start still did not report Status=ok after retries; falling back to monkey launcher" + # `monkey -p -c LAUNCHER 1` synthesises a single LAUNCHER intent + # via the system input pipe and bypasses am start's intent-resolution + # path. Some Android versions can't resolve via `am start -p` even + # after pm has indexed, but monkey still works because it asks pm + # directly for the LAUNCHER activity. Treat its output as best-effort: + # the pidof check below is the source of truth for whether the app + # actually came up. + if MONKEY_OUT="$("$ADB_BIN" shell monkey \ + -p "$PACKAGE_NAME" \ + -c android.intent.category.LAUNCHER 1 2>&1 | tr -d '\r')"; then + MONKEY_RC=0 + else + MONKEY_RC=$? + fi + ra_log "monkey launcher exit=${MONKEY_RC}, output:" + printf '%s\n' "$MONKEY_OUT" | sed 's/^/[run-android-instrumentation-tests] /' fi # Verify the process is actually up before committing to a 10-minute