Skip to content

Commit ade41c8

Browse files
javachefacebook-github-bot
javache
authored and
facebook-github-bot
committed
Make missing parent view state in updateLayout a soft error (facebook#49951)
Summary: Pull Request resolved: facebook#49951 We previously fixed Differentiator generating an incorrect parentTag (facebook#48055), but this can lead to crashes in Android UI due to reordering that happens in the Android mounting layer. While we have an experiment to disable this reordering (facebook#46702) this currently has a negative performance impact which needs to be addressed. As a mitigation, we can make the lookup of parentTag's ViewManager state nullable. We only require this to support `needsCustomLayoutForChildren`, which is not commonly used, and seems acceptable to drop in this scenario. Changelog: [Android][Changed] Do not crash when parent view state can't be found Reviewed By: NickGerleman Differential Revision: D70966621 fbshipit-source-id: 33d0b6a90860788a4c9a8c6cea36c2c72c1392e1
1 parent 1482dd9 commit ade41c8

File tree

2 files changed

+46
-21
lines changed

2 files changed

+46
-21
lines changed

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

+9-2
Original file line numberDiff line numberDiff line change
@@ -833,9 +833,16 @@ public void updateLayout(
833833
parent.requestLayout();
834834
}
835835

836-
ViewState parentViewState = getViewState(parentTag);
836+
// TODO T212247085: Make this non-nullable again after rolling out
837+
// disableMountItemReorderingAndroid
838+
ViewState parentViewState = getNullableViewState(parentTag);
837839
IViewGroupManager<?> parentViewManager = null;
838-
if (parentViewState.mViewManager != null) {
840+
if (parentViewState == null) {
841+
ReactSoftExceptionLogger.logSoftException(
842+
ReactSoftExceptionLogger.Categories.SURFACE_MOUNTING_MANAGER_MISSING_VIEWSTATE,
843+
new ReactNoCrashSoftException(
844+
"Unable to find viewState for tag: " + parentTag + " for updateLayout"));
845+
} else if (parentViewState.mViewManager != null) {
839846
parentViewManager = (IViewGroupManager) parentViewState.mViewManager;
840847
}
841848
if (parentViewManager == null || !parentViewManager.needsCustomLayoutForChildren()) {

‎packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/IntBufferBatchMountItem.java

+37-19
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags;
2323
import com.facebook.react.uimanager.StateWrapper;
2424
import com.facebook.systrace.Systrace;
25+
import java.util.Locale;
2526

2627
/**
2728
* This class represents a batch of {@link MountItem}s, represented directly as int buffers to
@@ -199,7 +200,7 @@ public boolean isBatchEmpty() {
199200
public String toString() {
200201
try {
201202
StringBuilder s = new StringBuilder();
202-
s.append(String.format("IntBufferBatchMountItem [surface:%d]:\n", mSurfaceId));
203+
s.append(String.format(Locale.ROOT, "IntBufferBatchMountItem [surface:%d]:\n", mSurfaceId));
203204
int i = 0, j = 0;
204205
while (i < mIntBufferLen) {
205206
int rawType = mIntBuffer[i++];
@@ -211,49 +212,65 @@ public String toString() {
211212
j += 3;
212213
s.append(
213214
String.format(
215+
Locale.ROOT,
214216
"CREATE [%d] - layoutable:%d - %s\n",
215-
mIntBuffer[i++], mIntBuffer[i++], componentName));
217+
mIntBuffer[i++],
218+
mIntBuffer[i++],
219+
componentName));
216220
} else if (type == INSTRUCTION_DELETE) {
217-
s.append(String.format("DELETE [%d]\n", mIntBuffer[i++]));
221+
s.append(String.format(Locale.ROOT, "DELETE [%d]\n", mIntBuffer[i++]));
218222
} else if (type == INSTRUCTION_INSERT) {
219223
s.append(
220224
String.format(
221-
"INSERT [%d]->[%d] @%d\n", mIntBuffer[i++], mIntBuffer[i++], mIntBuffer[i++]));
225+
Locale.ROOT,
226+
"INSERT [%d]->[%d] @%d\n",
227+
mIntBuffer[i++],
228+
mIntBuffer[i++],
229+
mIntBuffer[i++]));
222230
} else if (type == INSTRUCTION_REMOVE) {
223231
s.append(
224232
String.format(
225-
"REMOVE [%d]->[%d] @%d\n", mIntBuffer[i++], mIntBuffer[i++], mIntBuffer[i++]));
233+
Locale.ROOT,
234+
"REMOVE [%d]->[%d] @%d\n",
235+
mIntBuffer[i++],
236+
mIntBuffer[i++],
237+
mIntBuffer[i++]));
226238
} else if (type == INSTRUCTION_UPDATE_PROPS) {
227239
Object props = mObjBuffer[j++];
228240
String propsString =
229241
IS_DEVELOPMENT_ENVIRONMENT
230242
? (props != null ? props.toString() : "<null>")
231243
: "<hidden>";
232-
s.append(String.format("UPDATE PROPS [%d]: %s\n", mIntBuffer[i++], propsString));
244+
s.append(
245+
String.format(
246+
Locale.ROOT, "UPDATE PROPS [%d]: %s\n", mIntBuffer[i++], propsString));
233247
} else if (type == INSTRUCTION_UPDATE_STATE) {
234248
StateWrapper state = (StateWrapper) mObjBuffer[j++];
235249
String stateString =
236250
IS_DEVELOPMENT_ENVIRONMENT
237251
? (state != null ? state.toString() : "<null>")
238252
: "<hidden>";
239-
s.append(String.format("UPDATE STATE [%d]: %s\n", mIntBuffer[i++], stateString));
253+
s.append(
254+
String.format(
255+
Locale.ROOT, "UPDATE STATE [%d]: %s\n", mIntBuffer[i++], stateString));
240256
} else if (type == INSTRUCTION_UPDATE_LAYOUT) {
241-
int reactTag = mIntBuffer[i++];
242-
int parentTag = mIntBuffer[i++];
243-
int x = mIntBuffer[i++];
244-
int y = mIntBuffer[i++];
245-
int w = mIntBuffer[i++];
246-
int h = mIntBuffer[i++];
247-
int displayType = mIntBuffer[i++];
248-
int layoutDirection = mIntBuffer[i++];
249257
s.append(
250258
String.format(
251-
"UPDATE LAYOUT [%d]->[%d]: x:%d y:%d w:%d h:%d displayType:%d layoutDirection:"
252-
+ " %d\n",
253-
parentTag, reactTag, x, y, w, h, displayType, layoutDirection));
259+
Locale.ROOT,
260+
"UPDATE LAYOUT [%d]->[%d]: x:%d y:%d w:%d h:%d displayType:%d"
261+
+ " layoutDirection:%d\n",
262+
mIntBuffer[i++],
263+
mIntBuffer[i++],
264+
mIntBuffer[i++],
265+
mIntBuffer[i++],
266+
mIntBuffer[i++],
267+
mIntBuffer[i++],
268+
mIntBuffer[i++],
269+
mIntBuffer[i++]));
254270
} else if (type == INSTRUCTION_UPDATE_PADDING) {
255271
s.append(
256272
String.format(
273+
Locale.ROOT,
257274
"UPDATE PADDING [%d]: top:%d right:%d bottom:%d left:%d\n",
258275
mIntBuffer[i++],
259276
mIntBuffer[i++],
@@ -263,6 +280,7 @@ public String toString() {
263280
} else if (type == INSTRUCTION_UPDATE_OVERFLOW_INSET) {
264281
s.append(
265282
String.format(
283+
Locale.ROOT,
266284
"UPDATE OVERFLOWINSET [%d]: left:%d top:%d right:%d bottom:%d\n",
267285
mIntBuffer[i++],
268286
mIntBuffer[i++],
@@ -271,7 +289,7 @@ public String toString() {
271289
mIntBuffer[i++]));
272290
} else if (type == INSTRUCTION_UPDATE_EVENT_EMITTER) {
273291
j += 1;
274-
s.append(String.format("UPDATE EVENTEMITTER [%d]\n", mIntBuffer[i++]));
292+
s.append(String.format(Locale.ROOT, "UPDATE EVENTEMITTER [%d]\n", mIntBuffer[i++]));
275293
} else {
276294
FLog.e(TAG, "String so far: " + s.toString());
277295
throw new IllegalArgumentException(

0 commit comments

Comments
 (0)