Skip to content

Commit 98291bc

Browse files
hryhoriiK97meta-codesync[bot]
authored andcommitted
Look up Android image resources without throwing an exception (#59456)
Summary: In Release builds on Android, an image from the bundle (`<Image source={require('./icon.png')} />`) is a drawable resource, and its source is the resource name, for example `assets_icon`. `ImageSource` turns that name into a URI with `ResourceDrawableIdHelper.getResourceDrawableId`. The same lookup runs for `defaultSource` and `loadingIndicatorSource`, `Image.getSize` of a bundled image, and TextInput's `inlineImageLeft`. Images loaded from Metro or from a bundle outside the APK have a URI scheme and don't use it. **Current behavior.** `getResourceDrawableId` first tries the name as a numeric resource id with `toInt()` and catches the `NumberFormatException`. Resource names are not numbers, so every such image source throws an exception and fills in a stack trace as deep as the mount call stack. This happens before the cache of looked-up names, so names that were already looked up throw too. On a Samsung Galaxy A14 the lookup takes about 0.24 ms for each image source, almost all of it in the exception. **With this change.** The name is parsed with `toIntOrNull()`, which accepts the same numbers as `toInt()` and returns `null` instead of throwing. Numeric names still return the same id, and other names are still looked up as drawables, so every name gives the same result as before. ## Changelog: [ANDROID] [CHANGED] - Images from the app bundle look up their resource without throwing an exception Pull Request resolved: #59456 Test Plan: - The pinned ktfmt (`--dry-run --set-exit-if-changed`) passes on both files. - Added `ResourceDrawableIdHelperTest` (5 tests: empty name, numeric names, a name looked up as a drawable after normalization, a number too big for an `Int`, an unknown name). It passes with this change and with the old code, so the results are the same. - **Device:** Samsung Galaxy A14 (SM-A145R, Android 15), RNTester built with Gradle in Release (`assembleRelease`, arm64-v8a) and compiled with `cmd package compile -m speed`. The scenario is a local Playground screen, not part of this PR: a long feed with avatars and photos from the bundle that scrolls by itself. - **A/B setup:** a single APK with a local switch (an `adb shell setprop`, not part of this PR) that runs either the old `toInt()` with `try`/`catch` or `toIntOrNull()`, and times every call. Launches alternate A B B A A B B A, 4 per arm. Every launch made 1,300 lookups. | | before | this PR | change | |---|--:|--:|--:| | Time per `getResourceDrawableId` | 239–243 µs | 16.4–17.3 µs | −93% | - **Perfetto** (30 s of the same feed): `ReactImageManager.setSource` went from 3.5% to 1.6% of the main thread's samples, and the samples in exception handling under it from 237 to 0. - **Rendering:** screenshots of the feed in both arms are byte-identical. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Reviewed By: javache Differential Revision: D127722121 Pulled By: Abbondanzo fbshipit-source-id: 3fadd8cfe62f335ffcfd917f903e51ef940082c7
1 parent ed15270 commit 98291bc

2 files changed

Lines changed: 79 additions & 5 deletions

File tree

‎packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/imagehelper/ResourceDrawableIdHelper.kt‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,10 @@ public object ResourceDrawableIdHelper {
3131
}
3232
val normalizedName = name.lowercase().replace("-", "_")
3333

34-
// name could be a resource id.
35-
try {
36-
return normalizedName.toInt()
37-
} catch (e: NumberFormatException) {
38-
// Do nothing.
34+
// name could be a resource id. Most names are not numbers, so this must not throw: an
35+
// exception would build a stack trace for every image source.
36+
normalizedName.toIntOrNull()?.let {
37+
return it
3938
}
4039

4140
synchronized(this) {
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
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.imagehelper
9+
10+
import android.content.Context
11+
import org.assertj.core.api.Assertions.assertThat
12+
import org.junit.After
13+
import org.junit.Before
14+
import org.junit.Test
15+
import org.junit.runner.RunWith
16+
import org.robolectric.RobolectricTestRunner
17+
import org.robolectric.RuntimeEnvironment
18+
19+
@RunWith(RobolectricTestRunner::class)
20+
class ResourceDrawableIdHelperTest {
21+
22+
private lateinit var context: Context
23+
24+
@Before
25+
fun setUp() {
26+
context = RuntimeEnvironment.getApplication()
27+
ResourceDrawableIdHelper.clear()
28+
}
29+
30+
@After
31+
fun tearDown() {
32+
ResourceDrawableIdHelper.clear()
33+
}
34+
35+
@Test
36+
fun returnsZeroForMissingName() {
37+
assertThat(ResourceDrawableIdHelper.getResourceDrawableId(context, null)).isEqualTo(0)
38+
assertThat(ResourceDrawableIdHelper.getResourceDrawableId(context, "")).isEqualTo(0)
39+
}
40+
41+
@Test
42+
fun returnsNumericNameAsId() {
43+
assertThat(ResourceDrawableIdHelper.getResourceDrawableId(context, "123")).isEqualTo(123)
44+
assertThat(ResourceDrawableIdHelper.getResourceDrawableId(context, "+42")).isEqualTo(42)
45+
}
46+
47+
@Test
48+
fun looksUpNonNumericNameAsDrawable() {
49+
val expected =
50+
context.resources.getIdentifier(
51+
"redbox_top_border_background",
52+
"drawable",
53+
context.packageName,
54+
)
55+
assertThat(expected).isGreaterThan(0)
56+
assertThat(
57+
ResourceDrawableIdHelper.getResourceDrawableId(
58+
context,
59+
"Redbox-Top-Border-Background",
60+
)
61+
)
62+
.isEqualTo(expected)
63+
}
64+
65+
@Test
66+
fun looksUpNumberThatDoesNotFitInAnIntAsDrawable() {
67+
assertThat(ResourceDrawableIdHelper.getResourceDrawableId(context, "99999999999")).isEqualTo(0)
68+
}
69+
70+
@Test
71+
fun returnsZeroForUnknownDrawable() {
72+
assertThat(ResourceDrawableIdHelper.getResourceDrawableId(context, "unknown_image"))
73+
.isEqualTo(0)
74+
}
75+
}

0 commit comments

Comments
 (0)