Skip to content

Commit aa811de

Browse files
Autopilot Botfacebook-github-bot
authored andcommitted
Scope the TextInput spannable cache to the React instance
Summary: WARNING: Generated by Autopilot (alpha) — review carefully, verify the underlying claim before accepting. Agent: React Native Agent (Bugs) | Trajectory: https://www.internalfb.com/intern/devai/devmate/inspector/4f3686f6-6eac-416a-8116-4fd4ee5a627f/ | SC job: https://www.internalfb.com/intern/sandcastle/instance/54043196239422512/ --- On Fabric, `ReactEditText` caches its spannable in `TextLayoutManager` keyed by react tag, and the shadow node's state keeps that tag as `cachedAttributedStringId` to re-measure against. The cache was a single process-global map keyed only by tag. React tags restart for every new React instance, so after a reload the old and new instances can use the same tags in the same map: the new instance can read the old instance's spannable, and when an old `ReactEditText` is finalized it removes the new view's entry. The next cached measure then fails `checkNotNull` in `getOrCreateSpannableForText` with "Required value was null". This change gives each React instance its own spannable cache: - `TextLayoutManager` keeps one map per `ReactApplicationContext`. `FabricUIManager` creates it in its constructor and removes it in `invalidate()`. - `FabricUIManager.measureText` passes its own context, so a cached measure only looks in that instance's map. - `ReactEditText` writes to and evicts from the map of the context it was created with (`ThemedReactContext.reactApplicationContext`). Create, destroy, read and write all resolve the key through one helper, so a view's `ThemedReactContext` and `FabricUIManager`'s `ReactApplicationContext` always hit the same map. After teardown, an old view's set or evict does nothing because its map is gone. - A layout that started before `invalidate()` can still reach `measureText` after the map is removed. In that case the cached measure uses an empty spannable instead of throwing, because the cache-id `MapBuffer` carries only the id and there is nothing to rebuild from. The result belongs to an instance that is going away. A missing entry in a live map still fails `checkNotNull`. This replaces the previous version of this diff, where `finalize()` only evicted the spannable the instance itself had cached. Per-tag eviction still happens in `finalize()`, but now only within the view's own instance, where tags are never reused. I did not move eviction to `onDropViewInstance`: a commit whose layout is still running on a background thread can measure a TextInput after the UI thread has already processed that view's delete, so evicting right away would add a new way to hit the same `checkNotNull`. Changelog: [Android][Fixed] - Fix TextInput measurement crash ("Required value was null") after a React instance reload, by scoping the cached TextInput spannables to the React instance Reviewed By: zeyap Differential Revision: D122870300
1 parent 4e6bf24 commit aa811de

4 files changed

Lines changed: 178 additions & 9 deletions

File tree

‎packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,7 @@ public FabricUIManager(
259259

260260
mViewManagerRegistry = viewManagerRegistry;
261261
mReactApplicationContext.registerComponentCallbacks(viewManagerRegistry);
262+
TextLayoutManager.createSpannableCache(mReactApplicationContext);
262263
}
263264

264265
@Override
@@ -496,6 +497,7 @@ public void invalidate() {
496497
}
497498
mBinding = null;
498499

500+
TextLayoutManager.destroySpannableCache(mReactApplicationContext);
499501
ViewManagerPropertyUpdater.clear();
500502
}
501503

@@ -658,7 +660,8 @@ public long measureText(
658660
? (ReactTextViewManagerCallback) textViewManager
659661
: null,
660662
attachmentsPositions,
661-
mTextEffectRegistry);
663+
mTextEffectRegistry,
664+
mReactApplicationContext);
662665
}
663666

664667
@AnyThread

‎packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.kt‎

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import com.facebook.common.logging.FLog
3030
import com.facebook.infer.annotation.Assertions
3131
import com.facebook.react.bridge.JavaOnlyArray
3232
import com.facebook.react.bridge.JavaOnlyMap
33+
import com.facebook.react.bridge.ReactContext
3334
import com.facebook.react.bridge.ReadableMap
3435
import com.facebook.react.bridge.WritableArray
3536
import com.facebook.react.common.ReactConstants
@@ -42,6 +43,7 @@ import com.facebook.react.uimanager.PixelUtil
4243
import com.facebook.react.uimanager.PixelUtil.dpToPx
4344
import com.facebook.react.uimanager.PixelUtil.pxToDp
4445
import com.facebook.react.uimanager.ReactAccessibilityDelegate
46+
import com.facebook.react.uimanager.ThemedReactContext
4547
import com.facebook.react.util.AndroidVersion.VERSION_CODE_VANILLA_ICE_CREAM
4648
import com.facebook.react.views.text.internal.span.CustomLetterSpacingSpan
4749
import com.facebook.react.views.text.internal.span.CustomLineHeightSpan
@@ -115,7 +117,11 @@ internal object TextLayoutManager {
115117

116118
private const val TEXT_WIDTH_MODE_LONGEST_LINE = "longest-line"
117119

118-
private val tagToSpannableCache = ConcurrentHashMap<Int, Spannable>()
120+
// TextInput spannables keyed by react tag, one map per React instance. Tags restart from the same
121+
// value in every instance, so entries must not be shared across instances. FabricUIManager
122+
// creates and destroys the map for its ReactApplicationContext.
123+
private val tagToSpannableCaches =
124+
ConcurrentHashMap<ReactContext, ConcurrentHashMap<Int, Spannable>>()
119125

120126
// These wrappers mirror Android 15 APIs but use reflection because some internal targets still
121127
// compile against Android 14. They return null when the API is unavailable or cannot be invoked.
@@ -180,12 +186,30 @@ internal object TextLayoutManager {
180186
null
181187
}
182188

183-
fun setCachedSpannableForTag(reactTag: Int, sp: Spannable) {
184-
tagToSpannableCache[reactTag] = sp
189+
// Views hold a ThemedReactContext while FabricUIManager holds the ReactApplicationContext it
190+
// wraps. Every cache access goes through this so both resolve to the same key.
191+
private fun spannableCacheKey(reactContext: ReactContext): ReactContext =
192+
if (reactContext is ThemedReactContext) reactContext.reactApplicationContext else reactContext
193+
194+
@JvmStatic
195+
fun createSpannableCache(reactContext: ReactContext) {
196+
tagToSpannableCaches[spannableCacheKey(reactContext)] = ConcurrentHashMap()
197+
}
198+
199+
@JvmStatic
200+
fun destroySpannableCache(reactContext: ReactContext) {
201+
tagToSpannableCaches.remove(spannableCacheKey(reactContext))
202+
}
203+
204+
private fun getSpannableCache(reactContext: ReactContext): ConcurrentHashMap<Int, Spannable>? =
205+
tagToSpannableCaches[spannableCacheKey(reactContext)]
206+
207+
fun setCachedSpannableForTag(reactContext: ReactContext, reactTag: Int, sp: Spannable) {
208+
getSpannableCache(reactContext)?.put(reactTag, sp)
185209
}
186210

187-
fun deleteCachedSpannableForTag(reactTag: Int) {
188-
tagToSpannableCache.remove(reactTag)
211+
fun deleteCachedSpannableForTag(reactContext: ReactContext, reactTag: Int) {
212+
getSpannableCache(reactContext)?.remove(reactTag)
189213
}
190214

191215
fun isRTL(attributedString: MapBuffer): Boolean {
@@ -765,11 +789,15 @@ internal object TextLayoutManager {
765789
attributedString: MapBuffer,
766790
reactTextViewManagerCallback: ReactTextViewManagerCallback?,
767791
textEffectRegistry: TextEffectRegistry?,
792+
spannableCacheOwner: ReactContext? = null,
768793
): Spannable {
769794
val text: Spannable?
770795
if (attributedString.contains(AS_KEY_CACHE_ID)) {
771796
val cacheId = attributedString.getInt(AS_KEY_CACHE_ID)
772-
text = checkNotNull(tagToSpannableCache[cacheId])
797+
val cache = getSpannableCache(checkNotNull(spannableCacheOwner))
798+
// FabricUIManager.invalidate() can remove the cache while a layout that started before it is
799+
// still measuring on another thread. That layout's result is discarded with the instance.
800+
text = if (cache == null) SpannableString("") else checkNotNull(cache[cacheId])
773801
} else {
774802
text =
775803
createSpannableFromAttributedString(
@@ -1091,6 +1119,7 @@ internal object TextLayoutManager {
10911119
heightYogaMeasureMode: YogaMeasureMode,
10921120
reactTextViewManagerCallback: ReactTextViewManagerCallback?,
10931121
textEffectRegistry: TextEffectRegistry? = null,
1122+
spannableCacheOwner: ReactContext? = null,
10941123
): Layout {
10951124
val text =
10961125
getOrCreateSpannableForText(
@@ -1099,6 +1128,7 @@ internal object TextLayoutManager {
10991128
attributedString,
11001129
reactTextViewManagerCallback,
11011130
textEffectRegistry,
1131+
spannableCacheOwner,
11021132
)
11031133

11041134
val paint: TextPaint
@@ -1463,6 +1493,7 @@ internal object TextLayoutManager {
14631493
reactTextViewManagerCallback: ReactTextViewManagerCallback?,
14641494
attachmentsPositions: FloatArray?,
14651495
textEffectRegistry: TextEffectRegistry? = null,
1496+
spannableCacheOwner: ReactContext? = null,
14661497
): Long =
14671498
measureText(
14681499
assets,
@@ -1476,6 +1507,7 @@ internal object TextLayoutManager {
14761507
reactTextViewManagerCallback,
14771508
attachmentsPositions,
14781509
textEffectRegistry,
1510+
spannableCacheOwner,
14791511
)
14801512

14811513
@JvmStatic
@@ -1492,6 +1524,7 @@ internal object TextLayoutManager {
14921524
reactTextViewManagerCallback: ReactTextViewManagerCallback?,
14931525
attachmentsPositions: FloatArray?,
14941526
textEffectRegistry: TextEffectRegistry? = null,
1527+
spannableCacheOwner: ReactContext? = null,
14951528
): Long {
14961529
// TODO(5578671): Handle text direction (see View#getTextDirectionHeuristic)
14971530
val layout =
@@ -1506,6 +1539,7 @@ internal object TextLayoutManager {
15061539
heightYogaMeasureMode,
15071540
reactTextViewManagerCallback,
15081541
textEffectRegistry,
1542+
spannableCacheOwner,
15091543
)
15101544

15111545
val maximumNumberOfLines =

‎packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,7 @@ public open class ReactEditText public constructor(context: Context) : AppCompat
292292
if (DEBUG_MODE) {
293293
FLog.e(TAG, "finalize[$id] delete cached spannable")
294294
}
295-
TextLayoutManager.deleteCachedSpannableForTag(id)
295+
TextLayoutManager.deleteCachedSpannableForTag(UIManagerHelper.getReactContext(this), id)
296296
}
297297

298298
// After the text changes inside an EditText, TextView checks if a layout() has been requested.
@@ -1220,7 +1220,7 @@ public open class ReactEditText public constructor(context: Context) : AppCompat
12201220
sb.length,
12211221
Spannable.SPAN_INCLUSIVE_INCLUSIVE,
12221222
)
1223-
TextLayoutManager.setCachedSpannableForTag(id, sb)
1223+
TextLayoutManager.setCachedSpannableForTag(UIManagerHelper.getReactContext(this), id, sb)
12241224
}
12251225

12261226
public fun setEventDispatcher(eventDispatcher: EventDispatcher?) {
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
package com.facebook.react.views.textinput
9+
10+
import android.text.SpannableString
11+
import android.util.DisplayMetrics
12+
import android.view.Gravity
13+
import androidx.core.content.res.ResourcesCompat.ID_NULL
14+
import com.facebook.react.bridge.ReactApplicationContext
15+
import com.facebook.react.common.annotations.UnstableReactNativeAPI
16+
import com.facebook.react.common.mapbuffer.WritableMapBuffer
17+
import com.facebook.react.internal.featureflags.ReactNativeFeatureFlagsForTests
18+
import com.facebook.react.uimanager.DisplayMetricsHolder
19+
import com.facebook.react.uimanager.StateWrapper
20+
import com.facebook.react.uimanager.ThemedReactContext
21+
import com.facebook.react.views.text.ReactTextUpdate
22+
import com.facebook.react.views.text.TextLayoutManager
23+
import org.assertj.core.api.Assertions.assertThat
24+
import org.assertj.core.api.Assertions.assertThatThrownBy
25+
import org.junit.After
26+
import org.junit.Before
27+
import org.junit.Test
28+
import org.junit.runner.RunWith
29+
import org.mockito.kotlin.mock
30+
import org.robolectric.RobolectricTestRunner
31+
import org.robolectric.RuntimeEnvironment
32+
33+
@RunWith(RobolectricTestRunner::class)
34+
class ReactEditTextSpannableCacheTest {
35+
36+
private lateinit var manager: ReactTextInputManager
37+
private val oldRuntime = mock<ReactApplicationContext>()
38+
private val newRuntime = mock<ReactApplicationContext>()
39+
40+
@Before
41+
fun setup() {
42+
ReactNativeFeatureFlagsForTests.setUp()
43+
manager = ReactTextInputManager()
44+
DisplayMetricsHolder.setScreenDisplayMetrics(DisplayMetrics())
45+
}
46+
47+
@After
48+
fun tearDown() {
49+
TextLayoutManager.destroySpannableCache(oldRuntime)
50+
TextLayoutManager.destroySpannableCache(newRuntime)
51+
}
52+
53+
@Test
54+
fun `a view from a destroyed runtime does not evict a new runtime's spannable for the same tag`() {
55+
// Mirrors FabricUIManager's constructor and invalidate() across a React instance reload.
56+
TextLayoutManager.createSpannableCache(oldRuntime)
57+
val staleView = createViewWithCachedText(oldRuntime, "stale")
58+
TextLayoutManager.destroySpannableCache(oldRuntime)
59+
TextLayoutManager.createSpannableCache(newRuntime)
60+
61+
assertThatThrownBy { cachedSpannable(newRuntime) }
62+
.isInstanceOf(IllegalStateException::class.java)
63+
64+
val liveView = createViewWithCachedText(newRuntime, "live")
65+
runFinalizer(staleView)
66+
67+
assertThat(cachedSpannable(newRuntime).toString()).isEqualTo("live")
68+
69+
runFinalizer(liveView)
70+
71+
assertThatThrownBy { cachedSpannable(newRuntime) }
72+
.isInstanceOf(IllegalStateException::class.java)
73+
}
74+
75+
@Test
76+
fun `a view's spannable lands in the cache registered for its ReactApplicationContext`() {
77+
TextLayoutManager.createSpannableCache(newRuntime)
78+
val view = createViewWithCachedText(newRuntime, "live")
79+
80+
assertThat(view.context).isInstanceOf(ThemedReactContext::class.java)
81+
assertThat(cachedSpannable(newRuntime).toString()).isEqualTo("live")
82+
}
83+
84+
@Test
85+
fun `measuring after the runtime's cache is destroyed returns an empty spannable`() {
86+
TextLayoutManager.createSpannableCache(oldRuntime)
87+
createViewWithCachedText(oldRuntime, "stale")
88+
TextLayoutManager.destroySpannableCache(oldRuntime)
89+
90+
assertThat(cachedSpannable(oldRuntime).toString()).isEmpty()
91+
}
92+
93+
private fun createViewWithCachedText(
94+
runtime: ReactApplicationContext,
95+
text: String,
96+
): ReactEditText {
97+
val themedContext =
98+
ThemedReactContext(runtime, RuntimeEnvironment.getApplication(), null, ID_NULL)
99+
val view = manager.createViewInstance(themedContext)
100+
view.id = TAG
101+
view.stateWrapper = mock<StateWrapper>()
102+
view.maybeSetTextFromState(
103+
ReactTextUpdate(
104+
SpannableString(text),
105+
0,
106+
view.gravity and Gravity.HORIZONTAL_GRAVITY_MASK,
107+
0,
108+
0,
109+
),
110+
)
111+
return view
112+
}
113+
114+
private fun runFinalizer(view: ReactEditText) {
115+
ReactEditText::class.java.getDeclaredMethod("finalize").apply { isAccessible = true }(view)
116+
}
117+
118+
@OptIn(UnstableReactNativeAPI::class)
119+
private fun cachedSpannable(runtime: ReactApplicationContext) =
120+
TextLayoutManager.getOrCreateSpannableForText(
121+
RuntimeEnvironment.getApplication().assets,
122+
0,
123+
WritableMapBuffer().put(TextLayoutManager.AS_KEY_CACHE_ID, TAG),
124+
null,
125+
null,
126+
runtime,
127+
)
128+
129+
private companion object {
130+
const val TAG = 42
131+
}
132+
}

0 commit comments

Comments
 (0)