From 1c32385344a57c6638c58260a939ef1d7c429d67 Mon Sep 17 00:00:00 2001 From: Adam Comella Date: Fri, 16 Dec 2016 14:25:30 -0800 Subject: [PATCH] Android: Fix native animation crash Summary: An exception is thrown when the native animation code attempts to play an animation on a view that hasn't been created yet. This can happen because views are created in batches. If this particular view didn't make it into a batch yet, the view won't exist and an exception will be thrown when attempting to start an animation on it. This change eats the exception rather than crashing. The impact is that the app may drop one or more frames of the animation. **Notes** I'm not familiar enough with the Android native animation code to know whether or not this is a good fix. My team is using this change in our app because dropping animation frames is better than crashing the app. [This is the code](https://github.com/facebook/react-native/blob/c612c615449b8645976429ad1c0661a4f6e56115/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIViewOperationQueue.java#L874-L892) that is creating the views in batches. Hopefully my PR at least provides some insight into the cause of the bug. This may fix #9887 Closes https://github.com/facebook/react-native/pull/10907 Differential Revision: D4340129 Pulled By: lacker fbshipit-source-id: 69160d9e71281a96a7445d764b4715a3e54c0357 --- .../main/java/com/facebook/react/animated/BUCK | 2 ++ .../animated/NativeAnimatedNodesManager.java | 15 ++++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/animated/BUCK b/ReactAndroid/src/main/java/com/facebook/react/animated/BUCK index af3a76e0cec5a9..ac827a56ee1b05 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/animated/BUCK +++ b/ReactAndroid/src/main/java/com/facebook/react/animated/BUCK @@ -6,10 +6,12 @@ android_library( '*.java', ]), deps = [ + react_native_dep('libraries/fbcore/src/main/java/com/facebook/common/logging:logging'), react_native_dep('third-party/android/support/v4:lib-support-v4'), react_native_dep('third-party/java/infer-annotations:infer-annotations'), react_native_dep('third-party/java/jsr-305:jsr-305'), react_native_target('java/com/facebook/react/bridge:bridge'), + react_native_target('java/com/facebook/react/common:common'), react_native_target('java/com/facebook/react/module/annotations:annotations'), react_native_target('java/com/facebook/react/modules/core:core'), react_native_target('java/com/facebook/react/uimanager/annotations:annotations'), diff --git a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java index 95d90549b5f118..0e53f59ef6e7c7 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java @@ -12,6 +12,7 @@ import android.util.SparseArray; import com.facebook.infer.annotation.Assertions; +import com.facebook.common.logging.FLog; import com.facebook.react.bridge.Arguments; import com.facebook.react.bridge.Callback; import com.facebook.react.bridge.JSApplicationIllegalArgumentException; @@ -19,6 +20,8 @@ import com.facebook.react.bridge.ReadableMap; import com.facebook.react.bridge.UiThreadUtil; import com.facebook.react.bridge.WritableMap; +import com.facebook.react.common.ReactConstants; +import com.facebook.react.uimanager.IllegalViewOperationException; import com.facebook.react.uimanager.UIImplementation; import com.facebook.react.uimanager.UIManagerModule; import com.facebook.react.uimanager.events.Event; @@ -446,7 +449,17 @@ public void runUpdates(long frameTimeNanos) { nextNode.update(); if (nextNode instanceof PropsAnimatedNode) { // Send property updates to native view manager - ((PropsAnimatedNode) nextNode).updateView(mUIImplementation); + try { + ((PropsAnimatedNode) nextNode).updateView(mUIImplementation); + } catch (IllegalViewOperationException e) { + // An exception is thrown if the view hasn't been created yet. This can happen because views are + // created in batches. If this particular view didn't make it into a batch yet, the view won't + // exist and an exception will be thrown when attempting to start an animation on it. + // + // Eat the exception rather than crashing. The impact is that we may drop one or more frames of the + // animation. + FLog.e(ReactConstants.TAG, "Native animation workaround, frame lost as result of race condition", e); + } } if (nextNode instanceof ValueAnimatedNode) { // Potentially send events to JS when the node's value is updated