Crash in PointerInputFilter when used inside AdapterList [157998762]

Fixed

Bug

Status Update

No update yet.

Description

an...@google.com

created issue #1

Jun 2, 2020 05:35PM

I reproduced one annoying bug, and I also saw that a lot of people facing something similar in slack, and some other channels.
Basically what happens is when you have some postponed loading like asynchronously loading the image inside an item of AdapterList and you scroll sometimes it crashes and you there is no workaround.
It is not stable reproducible but I come up with a simple repro app(and you can see the logs here as well):

https://paste.googleplex.com/6333037623115776. we need to start scrolling in different directions and apparently it will crash.
What happens as I understand:
In PointerInputEventProcessor you first collect all PointerInputFilters which were hit(for example in our case is and AdapterList itself and the clickable item inside it)
Then inside AdapterList onDrag callback we synchronously subcompose which means that the AdapterList's item could recompose and apply a new modifier(for example update the background color).
And then we execute dispatchChanges on a modifier which was already detached.
We just discussed it during the sync, but seems like the simplest workaround is to add inside Node.dispatchChanges() something like

if (!pointerInputFilter.isAttached) {
return false
}

Comments

an...@google.com <an...@google.com> #2Jun 3, 2020 03:59PM

Related CLs

These are the CLs related to this project that were merged before filing this bug, mostly in chronological order.

Adding support for multiple text input systems. aosp/2406439
Copy EditProcessor, EditingBuffer and EditCommand to foundation demo app. aosp/2401358
WIP BasicTextInputPlugin, BasicTextField2, TextFieldState aosp/2450923
Stabilize PlatformTextInput API to allow use from Foundation. aosp/2465156
Make LocalPlatformTextInputPluginRegistry non-experimental. aosp/2466795
Always use EditProcessor.update method for setText semantics action in BTF2. aosp/2485833
Move BasicTextField2 prototype and tests to foundation. aosp/2475185
Disable a broken test in BasicTextField2 aosp/2487271
Continue expanding BTF2 API with DecorationBox aosp/2480257
Attempt to fix a flaky test for BasicTextField selection aosp/2493878
Add semantics action for inserting text at cursor. aosp/2481301
Move most BasicTextField2 decoration box modifiers to a Modifier.Node. aosp/2494377
KeyEvent support for BasicTextField2 aosp/2485061
Kick-off using Modifier Nodes in BasicTextField2 core modifiers aosp/2497299
Add Scroll capability to BasicTextField2 aosp/2506069
Simplify the BasicTextField2 package structure. aosp/2513916
Add some hooks to integration test full pipeline from BTF2 to platform. aosp/2515797
Implement basic TextFieldState.edit. aosp/2500547
Move filter from property on TextFieldState to param on BasicTextField2. aosp/2512127
Address nit feedback on aosp/2512127 and aosp/2500547. aosp/2516839
Give TextFieldState a Saver object and rememberTextFieldState. aosp/2520395
Make TextEditFilter use MutableTextFieldValueWithSelection. aosp/2513918
KeyboardActions in BasicTextField2 aosp/2518475
Add keyboard visibility controls to TextInputSession aosp/2518476
Add change tracking to MutableTextFieldValue. aosp/2513915
Improve TextFieldKeyEventTest for BasicTextField2 aosp/2513576
Disable some BasicTextField2 tests on API21 aosp/2524362
Replace MutableTextFieldValueWithSelection.resetTo with revertAllChanges. aosp/2526345
Add the ability for TextEditFilters to specify default keyboard options. aosp/2523357
Combine min/max lines into a single parameter. aosp/2523359
- Revert "Combine min/max lines into a single parameter." aosp/2528167
- Revert "Revert "Combine min/max lines into a single parameter."" aosp/2528168
Make TextRange.constrain public. aosp/2528163
Introduce BasicSecureTextField in text2 aosp/2519909
Remove the system settings check from BasicSecureTextField. aosp/2528482
Get TextFieldValue out of public BasicTextField2 API. aosp/2526539
Rename selection to selectionInChars. aosp/2528164

sh...@google.com <sh...@google.com> Jun 3, 2020 10:55PM

Accepted by sh...@google.com.

sh...@google.com <sh...@google.com> #3Jun 4, 2020 10:35PM

Project: platform/frameworks/support
Branch: androidx-main

commit 8243c6ccdf7a97c51f3424d8a3872767d309cf57
Author: Zach Klippenstein <klippenstein@google.com>
Date: Fri Apr 07 11:14:52 2023

Misc polish for BTF2 APIs.

- Renamed TextFieldEditResult to TextEditResult to be more concise and
match TextEditFilter.
- Added placeCursorAfter{Char|Codepoint}At methods to both
TextEditResult and MutableTextFieldValueWithSelection.
- Added more docs.

Bug:

b/277380808

Test: existing coverage
Relnote: n/a
Change-Id: Ic2d4a7d907d9a3c091a2ef4131b906af701b6e42

M compose/foundation/foundation/api/public_plus_experimental_current.txt
M compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/text2/BasicTextField2FilterDemos.kt
M compose/foundation/foundation/samples/src/main/java/androidx/compose/foundation/samples/BasicTextField2Samples.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/text2/BasicTextField2Test.kt
M compose/foundation/foundation/src/androidMain/kotlin/androidx/compose/foundation/text2/BasicSecureTextField.kt
M compose/foundation/foundation/src/androidMain/kotlin/androidx/compose/foundation/text2/input/AllCapsFilter.kt
M compose/foundation/foundation/src/androidMain/kotlin/androidx/compose/foundation/text2/input/MaxLengthFilter.kt
M compose/foundation/foundation/src/androidMain/kotlin/androidx/compose/foundation/text2/input/TextEditFilter.kt
A compose/foundation/foundation/src/androidMain/kotlin/androidx/compose/foundation/text2/input/TextEditResult.kt
M compose/foundation/foundation/src/androidMain/kotlin/androidx/compose/foundation/text2/input/TextFieldBufferWithSelection.kt
D compose/foundation/foundation/src/androidMain/kotlin/androidx/compose/foundation/text2/input/TextFieldEditResult.kt
M compose/foundation/foundation/src/androidMain/kotlin/androidx/compose/foundation/text2/input/TextFieldState.kt
M compose/foundation/foundation/src/androidMain/kotlin/androidx/compose/foundation/text2/input/internal/EditProcessor.kt

https://android-review.googlesource.com/2528481

ap...@google.com <ap...@google.com> #4Jun 9, 2020 09:43PM

Marked as fixed.

Project: platform/frameworks/support
Branch: androidx-main

commit 72eedc538254914ad5ba7581af02c10f8294492a
Author: Halil Ozercan <halilibo@google.com>
Date: Sat Apr 08 05:20:32 2023

Remove secureContent from BasicTextField2

secureContent is currently doing an unnecessary plumbing for BasicSecureTextField. Although we have plans for it to have more responsibilities in the future, today it's just a confusing API with little usefulness.

Bug:

b/277380808

Test: BasicSecureTextFieldTest
Relnote: N/A
Change-Id: I3151c97b7ad723f231156ec7f3887c7fa1fecaca

M compose/foundation/foundation/api/public_plus_experimental_current.txt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/text2/BasicSecureTextFieldTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/text2/BasicTextField2SemanticsTest.kt
M compose/foundation/foundation/src/androidMain/kotlin/androidx/compose/foundation/text2/BasicSecureTextField.kt
M compose/foundation/foundation/src/androidMain/kotlin/androidx/compose/foundation/text2/BasicTextField2.kt
M compose/foundation/foundation/src/androidMain/kotlin/androidx/compose/foundation/text2/input/internal/TextFieldDecoratorModifier.kt

https://android-review.googlesource.com/2529280

ap...@google.com <ap...@google.com> #5Jun 10, 2020 06:22PM

Project: platform/frameworks/support
Branch: androidx-main

commit aa91a7917ed8e4e4782e42267d177797e2b22532
Author: Halil Ozercan <halilibo@google.com>
Date: Tue Apr 11 19:12:42 2023

Delegate pointer events from TextFieldDecoratorModifierNode

Many complex pointer event handlers use PointerInputScope. We are migrating TextFieldDecoratorModifier to `SuspendingPointerInputModifierNode`, using delegation to take advantage of existing node implementation with minimal effect on our exiting `Modifier.Node`.

`detectTapAndPress` only consumes press and tap events which would make it less annoying to put `BasicTextField2` in a scrollable container.

Bug:

b/277380808

Test: BasicTextField2Test
Change-Id: I82783dd967c186c0147eface0698258000cb840d

M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/text2/BasicTextField2Test.kt
M compose/foundation/foundation/src/androidMain/kotlin/androidx/compose/foundation/text2/input/internal/TextFieldDecoratorModifier.kt

https://android-review.googlesource.com/2531362

Issue 157998762

Description

Issue summary

Comments

an...@google.com <an...@google.com> #2Jun 3, 2020 03:59PM

Related CLs

sh...@google.com <sh...@google.com> Jun 3, 2020 10:55PM

sh...@google.com <sh...@google.com> #3Jun 4, 2020 10:35PM

ap...@google.com <ap...@google.com> #4Jun 9, 2020 09:43PM

ap...@google.com <ap...@google.com> #5Jun 10, 2020 06:22PM

Add comment

Issue metadata