Repository navigation
Commit 307162b
Scope the TextInput spannable cache to the React instance (#58869)
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/dea12476-b7e6-441b-8ebb-bd79b2de520a/ | SC job: https://www.internalfb.com/intern/sandcastle/instance/49539596621165635/
---
Pull Request resolved: #58869
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 returns a 0x0 measurement 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: D1228703001 parent 99d327c commit 307162b
4 files changed
Lines changed: 281 additions & 40 deletions
File tree
- packages/react-native/ReactAndroid/src
- main/java/com/facebook/react
- fabric
- views
- textinput
- text
- test/java/com/facebook/react/views/textinput
Lines changed: 6 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
259 | 259 | | |
260 | 260 | | |
261 | 261 | | |
| 262 | + | |
262 | 263 | | |
263 | 264 | | |
264 | 265 | | |
| |||
496 | 497 | | |
497 | 498 | | |
498 | 499 | | |
| 500 | + | |
499 | 501 | | |
500 | 502 | | |
501 | 503 | | |
| |||
566 | 568 | | |
567 | 569 | | |
568 | 570 | | |
569 | | - | |
| 571 | + | |
| 572 | + | |
570 | 573 | | |
571 | 574 | | |
572 | 575 | | |
| |||
658 | 661 | | |
659 | 662 | | |
660 | 663 | | |
661 | | - | |
| 664 | + | |
| 665 | + | |
662 | 666 | | |
663 | 667 | | |
664 | 668 | | |
| |||
Lines changed: 72 additions & 36 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
28 | 28 | | |
29 | 29 | | |
30 | 30 | | |
| 31 | + | |
31 | 32 | | |
32 | 33 | | |
| 34 | + | |
33 | 35 | | |
34 | 36 | | |
35 | 37 | | |
| |||
42 | 44 | | |
43 | 45 | | |
44 | 46 | | |
| 47 | + | |
45 | 48 | | |
46 | 49 | | |
47 | 50 | | |
| |||
115 | 118 | | |
116 | 119 | | |
117 | 120 | | |
118 | | - | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
119 | 126 | | |
120 | 127 | | |
121 | 128 | | |
| |||
180 | 187 | | |
181 | 188 | | |
182 | 189 | | |
183 | | - | |
184 | | - | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
185 | 215 | | |
186 | 216 | | |
187 | | - | |
188 | | - | |
| 217 | + | |
| 218 | + | |
189 | 219 | | |
190 | 220 | | |
191 | 221 | | |
| |||
765 | 795 | | |
766 | 796 | | |
767 | 797 | | |
768 | | - | |
769 | | - | |
770 | | - | |
771 | | - | |
772 | | - | |
773 | | - | |
774 | | - | |
775 | | - | |
776 | | - | |
777 | | - | |
778 | | - | |
779 | | - | |
780 | | - | |
781 | | - | |
782 | | - | |
783 | | - | |
784 | | - | |
785 | | - | |
786 | | - | |
| 798 | + | |
| 799 | + | |
| 800 | + | |
| 801 | + | |
| 802 | + | |
| 803 | + | |
| 804 | + | |
| 805 | + | |
| 806 | + | |
787 | 807 | | |
788 | 808 | | |
789 | 809 | | |
| |||
1091 | 1111 | | |
1092 | 1112 | | |
1093 | 1113 | | |
1094 | | - | |
1095 | | - | |
1096 | | - | |
1097 | | - | |
1098 | | - | |
1099 | | - | |
1100 | | - | |
1101 | | - | |
1102 | | - | |
1103 | | - | |
| 1114 | + | |
| 1115 | + | |
| 1116 | + | |
1104 | 1117 | | |
1105 | 1118 | | |
| 1119 | + | |
| 1120 | + | |
| 1121 | + | |
| 1122 | + | |
| 1123 | + | |
| 1124 | + | |
| 1125 | + | |
1106 | 1126 | | |
1107 | 1127 | | |
| 1128 | + | |
| 1129 | + | |
| 1130 | + | |
| 1131 | + | |
| 1132 | + | |
| 1133 | + | |
| 1134 | + | |
| 1135 | + | |
1108 | 1136 | | |
1109 | 1137 | | |
1110 | 1138 | | |
| |||
1463 | 1491 | | |
1464 | 1492 | | |
1465 | 1493 | | |
| 1494 | + | |
1466 | 1495 | | |
1467 | 1496 | | |
1468 | 1497 | | |
| |||
1476 | 1505 | | |
1477 | 1506 | | |
1478 | 1507 | | |
| 1508 | + | |
1479 | 1509 | | |
1480 | 1510 | | |
1481 | 1511 | | |
| |||
1492 | 1522 | | |
1493 | 1523 | | |
1494 | 1524 | | |
| 1525 | + | |
1495 | 1526 | | |
1496 | 1527 | | |
1497 | 1528 | | |
| |||
1506 | 1537 | | |
1507 | 1538 | | |
1508 | 1539 | | |
1509 | | - | |
| 1540 | + | |
| 1541 | + | |
1510 | 1542 | | |
1511 | 1543 | | |
1512 | 1544 | | |
| |||
1775 | 1807 | | |
1776 | 1808 | | |
1777 | 1809 | | |
| 1810 | + | |
1778 | 1811 | | |
1779 | 1812 | | |
1780 | 1813 | | |
| |||
1785 | 1818 | | |
1786 | 1819 | | |
1787 | 1820 | | |
| 1821 | + | |
1788 | 1822 | | |
1789 | 1823 | | |
1790 | 1824 | | |
| |||
1798 | 1832 | | |
1799 | 1833 | | |
1800 | 1834 | | |
| 1835 | + | |
1801 | 1836 | | |
1802 | 1837 | | |
1803 | 1838 | | |
| |||
1811 | 1846 | | |
1812 | 1847 | | |
1813 | 1848 | | |
1814 | | - | |
| 1849 | + | |
| 1850 | + | |
1815 | 1851 | | |
1816 | 1852 | | |
1817 | 1853 | | |
| |||
Lines changed: 11 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
| 12 | + | |
12 | 13 | | |
13 | 14 | | |
14 | 15 | | |
| |||
46 | 47 | | |
47 | 48 | | |
48 | 49 | | |
| 50 | + | |
49 | 51 | | |
50 | 52 | | |
51 | 53 | | |
| |||
292 | 294 | | |
293 | 295 | | |
294 | 296 | | |
295 | | - | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
296 | 305 | | |
297 | 306 | | |
298 | 307 | | |
| |||
1220 | 1229 | | |
1221 | 1230 | | |
1222 | 1231 | | |
1223 | | - | |
| 1232 | + | |
1224 | 1233 | | |
1225 | 1234 | | |
1226 | 1235 | | |
| |||
0 commit comments