AppFlowy-IO / AppFlowy-IO/appflowy-editor

Mobile selection auto-scroll can enter feedback loop / ANR when table focus changes

Aperta
#1,215 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub
Lingua principale
Dart
Stelle
684
Fork
329
Metriche di merge delle PR
Nessuna PR unita negli ultimi 30g

Descrizione

### Description

After fixing the table row-height relayout loop from #1213 / #1214, the same mobile table interaction still exposed a second, independent hang in editor auto-scroll.

On a physical Android device, editing a table cell, tapping out of it, then tapping back into the table could ANR the app. With the microtask starvation fixed, the remaining symptom appeared as the page content/table rapidly vibrating until tapping out again, which indicates a repeat auto-scroll feedback loop.

This did not reproduce on the Pixel emulator or Chrome in my testing; it reproduced on a physical Galaxy S23+.

### Environment

- AppFlowy Editor: 6.1.0 / 6.2.0 code path
- Flutter: 3.41.2 stable
- Device: Galaxy S23+ (SM-S916U)
- OS: Android 16
- Renderer: Impeller / Vulkan
- Scenario: mobile editor, table cell focus/defocus, selection/caret update near table content

### Root cause

There are two related problems in the mobile auto-scroll path:

1. The vendored `EdgeDraggingAutoScroller._scroll` in `lib/src/flutter/scrollable_helpers.dart` recursively awaits itself:

```dart
if (_scrolling) {
await _scroll();
}
```

`ScrollPosition.moveTo` can complete synchronously, especially when the scroll position cannot make meaningful progress. In that case, the async recursion can become a tight microtask chain and starve frames/input.

2. Mobile selection changes call `startAutoScroll` even for ordinary caret/table focus updates. Those should be one-shot keep-visible nudges, not repeating auto-scroll. Otherwise, after moving repeat work to frame callbacks, the bug remains visible as rapid content vibration.

### Symbolicated ANR stack

A release APK built with split debug info produced this main-thread Dart stack shape:

```text
ChangeNotifier.notifyListeners
-> _RestorableScrollOffset.didUpdateValue
-> ScrollableState.saveOffset
-> ScrollPosition.didEndScroll
-> ScrollPositionWithSingleContext.jumpTo
-> ViewportOffset.moveTo
-> ScrollPosition.moveTo
-> EdgeDraggingAutoScroller._scroll
-> EdgeDraggingAutoScroller._scroll
-> _microtaskLoop
```

### Expected behavior

Table focus/defocus and caret movement should settle. Auto-scroll should repeat only during actual drag selection and should yield between ticks so frames/input can continue.

### Actual behavior

The editor can enter an auto-scroll feedback loop. On the physical Android device this manifested first as an ANR, and after partial mitigation as visible table/content vibration until tapping out.

### Proposed fix

- Schedule repeated auto-scroll ticks with `SchedulerBinding.scheduleFrameCallback` instead of recursive `await _scroll()`.
- Preserve no-progress termination so auto-scroll stops if `moveTo` does not advance the offset.
- Add a `repeat` flag to the editor auto-scroll service.
- Use repeating auto-scroll only for real mobile drag modes (`leftSelectionHandle`, `rightSelectionHandle`, `cursor`). Use one-shot auto-scroll for ordinary mobile selection/caret updates.

### Verification

On the physical Galaxy S23+:

- The original two-tap table interaction no longer ANRs.
- No new Android Dropbox ANR was written after the fix.
- Screenshots taken shortly after re-entering a table and again two seconds later were stable, with no continuing vibration.
- Manual selection around the table looked stable.

Guida per i contributori

Nessuna guida per i contributori indicizzata per questo repository

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.