Skip to content

Commit

Permalink
Android: Fix native animation crash
Browse files Browse the repository at this point in the history
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 facebook#9887
Closes facebook#10907

Differential Revision: D4340129

Pulled By: lacker

fbshipit-source-id: 69160d9e71281a96a7445d764b4715a3e54c0357
  • Loading branch information
Adam Comella authored and facebook-github-bot committed Dec 16, 2016
1 parent 25cd2c5 commit 1c32385
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 1 deletion.
2 changes: 2 additions & 0 deletions ReactAndroid/src/main/java/com/facebook/react/animated/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,16 @@
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;
import com.facebook.react.bridge.ReadableArray;
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;
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 1c32385

Please sign in to comment.