Skip to content

Commit 1066c76

Browse files
shai-almogclaude
andauthored
Fix oversized dialogs in iOS Modern and Android Material themes (#4856)
* 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) <noreply@anthropic.com> * 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 <pkg>` 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 <pkg> -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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent cbe662e commit 1066c76

3 files changed

Lines changed: 105 additions & 24 deletions

File tree

native-themes/android-material/theme.css

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,11 @@
8383
switchTrackOffOutlineColor: "79747e";
8484
switchThumbInsetMM: "0";
8585
/* Dialog sizing/layout constants - see iOS Modern theme for the
86-
rationale. Without these the framework falls back to a wider
87-
default Dialog layout that ballooned Dialog.show() to near-full
88-
screen with the body floating in empty space. */
86+
rationale. dialogPosition is the load-bearing one (routes
87+
Dialog.show() through showPacked so it sizes to content);
88+
without it Form.showDialog's hard-coded 20%/10%/20% screen
89+
margins make even an empty dialog ~60%x70% of the screen. */
90+
dialogPosition: Center;
8991
hideEmptyTitleBool: true;
9092
shrinkPopupTitleBool: true;
9193
dialogButtonCommandsBool: true;

native-themes/ios-modern/theme.css

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,11 +91,16 @@
9191
checkBoxUncheckedIconInt: 59693;
9292
radioCheckedIconInt: 59447;
9393
radioUncheckedIconInt: 59446;
94-
/* Dialog sizing/layout constants the legacy iOS7 theme set and
95-
the modern theme had been missing. Without these the framework
96-
falls back to a wider default Dialog layout that left
97-
Dialog.show("Clicked","Clicked item N","OK",null) ballooning to
98-
near-full screen with the body floating in empty space. */
94+
/* Dialog sizing/layout constants. dialogPosition is the load-bearing
95+
one: without it Dialog.show() falls through to Form.showDialog()
96+
which always positions the dialog with 20%/10%/20% screen-edge
97+
margins (Form.java showDialog), making the dialog ~60%x70% of
98+
the screen regardless of content - so even an empty dialog reads
99+
as huge. With dialogPosition set, Dialog.showImpl routes through
100+
showPacked() which sizes to the content's preferred size. The
101+
boolean constants below tune button-area rendering (compact
102+
title, grid commands) but don't affect outer sizing. */
103+
dialogPosition: Center;
99104
hideEmptyTitleBool: true;
100105
shrinkPopupTitleBool: true;
101106
dialogButtonCommandsBool: true;

scripts/run-android-instrumentation-tests.sh

Lines changed: 90 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,16 @@ ADB_BIN="$(command -v adb)"
139139
ra_log "ADB connected devices:"
140140
"$ADB_BIN" devices -l | sed 's/^/[run-android-instrumentation-tests] /'
141141

142+
# Bump the device-side logcat ring buffer before clearing it. The default on
143+
# Android emulators is 256K-1M per buffer, which is too small for our test
144+
# suite: a single screenshot can emit ~70 base64 chunk lines (~500 bytes
145+
# each), and across 90+ tests the main buffer has been observed to wrap
146+
# mid-suite, dropping a chunk line and causing `Cn1ssChunkTools` to fail
147+
# reassembly with a gap error. 16 MiB is plenty for a single suite run and
148+
# matches what `adb logcat -G` accepts on every supported emulator image.
149+
# Failing to set the buffer is non-fatal — older platforms silently ignore -G.
150+
ra_log "Bumping device logcat ring buffer to 16M (mitigates chunk-line drops)"
151+
"$ADB_BIN" logcat -G 16M >/dev/null 2>&1 || true
142152
ra_log "Clearing logcat buffer"
143153
"$ADB_BIN" logcat -c || true
144154

@@ -370,6 +380,40 @@ if [ "${#FAILED_TESTS[@]}" -gt 0 ] && [ -n "${PACKAGE_NAME:-}" ]; then
370380
if [ "$INSTALL_RC" -ne 0 ]; then
371381
ra_log "WARN: adb install reported failure; the am start below will likely fail too"
372382
fi
383+
384+
# Wait for PackageManager to finish indexing the freshly-installed APK.
385+
# `adb install` returns Success as soon as the APK lands on the device,
386+
# but the launcher Intent isn't resolvable until pm has registered
387+
# every component in the manifest. Without this wait, `am start` in the
388+
# following block races and prints "Activity not started, unable to
389+
# resolve Intent" — exactly the failure the previous version of this
390+
# block kept hitting on busy emulators (see PR #4856 build for the
391+
# observed timing). Poll `cmd package resolve-activity --brief` for
392+
# the same MAIN/LAUNCHER intent we are about to fire; it returns the
393+
# `<pkg>/<.Activity>` triple once the launcher is ready. Cap at 30s
394+
# because indexing usually finishes in < 5s but slow CI runners can
395+
# take longer.
396+
ra_log "Waiting for PackageManager to register launcher activity for $PACKAGE_NAME"
397+
RESOLVE_DEADLINE=$(( $(date +%s) + 30 ))
398+
LAUNCHER_RESOLVED=0
399+
while [ "$(date +%s)" -lt "$RESOLVE_DEADLINE" ]; do
400+
RESOLVE_OUT="$("$ADB_BIN" shell cmd package resolve-activity --brief \
401+
-a android.intent.action.MAIN \
402+
-c android.intent.category.LAUNCHER \
403+
"$PACKAGE_NAME" 2>/dev/null | tr -d '\r')" || RESOLVE_OUT=""
404+
# `resolve-activity --brief` prints the matching component on a line
405+
# of the form `<package>/<activity>`. Newer Android prepends a
406+
# `priority=...` line we want to ignore, so match by substring.
407+
if printf '%s\n' "$RESOLVE_OUT" | grep -q "^${PACKAGE_NAME}/"; then
408+
LAUNCHER_RESOLVED=1
409+
ra_log "PackageManager resolved launcher: $(printf '%s\n' "$RESOLVE_OUT" | grep "^${PACKAGE_NAME}/" | head -n1)"
410+
break
411+
fi
412+
sleep 1
413+
done
414+
if [ "$LAUNCHER_RESOLVED" -eq 0 ]; then
415+
ra_log "WARN: PackageManager did not report launcher activity within 30s; attempting am start anyway"
416+
fi
373417
else
374418
ra_log "ERROR: cannot reinstall — APK not found at $MAIN_APK"
375419
fi
@@ -384,23 +428,53 @@ if [ "${#FAILED_TESTS[@]}" -gt 0 ] && [ -n "${PACKAGE_NAME:-}" ]; then
384428
# (the script would exit at the failing pipeline). Logging both the exit
385429
# code and the raw output is the whole point of this block — see the
386430
# "RIGHT FIX" comment above.
387-
ra_log "Relaunching $PACKAGE_NAME via 'am start -W -a MAIN -c LAUNCHER -p $PACKAGE_NAME'"
388-
if AM_START_OUT="$("$ADB_BIN" shell am start -W \
389-
-a android.intent.action.MAIN \
390-
-c android.intent.category.LAUNCHER \
391-
-p "$PACKAGE_NAME" 2>&1 | tr -d '\r')"; then
392-
AM_START_RC=0
393-
else
394-
AM_START_RC=$?
395-
fi
396-
ra_log "am start exit=${AM_START_RC}, output:"
397-
printf '%s\n' "$AM_START_OUT" | sed 's/^/[run-android-instrumentation-tests] /'
398-
399-
AM_STATUS="$(printf '%s\n' "$AM_START_OUT" \
400-
| awk -F= '/^[[:space:]]*Status[[:space:]]*=/ {print $2; exit}' \
401-
| tr -d '[:space:]')"
431+
#
432+
# Retry the start a few times. Even after `cmd package resolve-activity`
433+
# reports the launcher, `am start` can race a still-completing
434+
# PackageManager scan and return "unable to resolve Intent". Each retry
435+
# waits 2s — empirically enough to clear the race.
436+
AM_STATUS=""
437+
for _amattempt in 1 2 3; do
438+
ra_log "Relaunching $PACKAGE_NAME via 'am start -W -a MAIN -c LAUNCHER -p $PACKAGE_NAME' (attempt ${_amattempt})"
439+
if AM_START_OUT="$("$ADB_BIN" shell am start -W \
440+
-a android.intent.action.MAIN \
441+
-c android.intent.category.LAUNCHER \
442+
-p "$PACKAGE_NAME" 2>&1 | tr -d '\r')"; then
443+
AM_START_RC=0
444+
else
445+
AM_START_RC=$?
446+
fi
447+
ra_log "am start exit=${AM_START_RC}, output:"
448+
printf '%s\n' "$AM_START_OUT" | sed 's/^/[run-android-instrumentation-tests] /'
449+
AM_STATUS="$(printf '%s\n' "$AM_START_OUT" \
450+
| awk -F= '/^[[:space:]]*Status[[:space:]]*=/ {print $2; exit}' \
451+
| tr -d '[:space:]')"
452+
if [ "${AM_STATUS:-}" = "ok" ]; then
453+
break
454+
fi
455+
if [ "$_amattempt" -lt 3 ]; then
456+
ra_log "am start did not report Status=ok (got '${AM_STATUS:-<empty>}'); retrying in 2s"
457+
sleep 2
458+
fi
459+
done
402460
if [ "${AM_STATUS:-}" != "ok" ]; then
403-
ra_log "WARN: am start did not report Status=ok (got '${AM_STATUS:-<empty>}'); the app may not be running"
461+
ra_log "WARN: am start still did not report Status=ok after retries; falling back to monkey launcher"
462+
# `monkey -p <pkg> -c LAUNCHER 1` synthesises a single LAUNCHER intent
463+
# via the system input pipe and bypasses am start's intent-resolution
464+
# path. Some Android versions can't resolve via `am start -p` even
465+
# after pm has indexed, but monkey still works because it asks pm
466+
# directly for the LAUNCHER activity. Treat its output as best-effort:
467+
# the pidof check below is the source of truth for whether the app
468+
# actually came up.
469+
if MONKEY_OUT="$("$ADB_BIN" shell monkey \
470+
-p "$PACKAGE_NAME" \
471+
-c android.intent.category.LAUNCHER 1 2>&1 | tr -d '\r')"; then
472+
MONKEY_RC=0
473+
else
474+
MONKEY_RC=$?
475+
fi
476+
ra_log "monkey launcher exit=${MONKEY_RC}, output:"
477+
printf '%s\n' "$MONKEY_OUT" | sed 's/^/[run-android-instrumentation-tests] /'
404478
fi
405479

406480
# Verify the process is actually up before committing to a 10-minute

0 commit comments

Comments
 (0)