Skip to content

Commit

Permalink
Support for stopping animations that run on UI thread.
Browse files Browse the repository at this point in the history
Summary:This change extends animated native module API with `stopAnimation` method that is responsible for interrupting actively running animation as a reslut of a JS call. In order for the `stopAnimation` to understand `animationId` argument I also had to add `animationId` to `startAnimation` method. As JS thread runs in parallel to the thread which executes the animation there is a chance that JS may call `stopAnimation` after the animation has finished. Because of that we are not doing any checks on the `animationId` parameter passed to `stopAnimation` in native and if the animation does not exists in the registry we ignore that call.

**Test Plan**
Run JS tests: `npm test Libraries/Animated/src/__tests__/AnimatedNative-test.js`
Run java tests: `buck test ReactAndroid/src/test/java/com/facebook/react/animated`
Closes facebook#7058

Differential Revision: D3211906

fb-gh-sync-id: 3761509651de36a550b00d33e2a631c379d3900f
fbshipit-source-id: 3761509651de36a550b00d33e2a631c379d3900f
  • Loading branch information
kmagiera authored and Facebook Github Bot 8 committed Apr 22, 2016
1 parent 63adb48 commit cd11738
Show file tree
Hide file tree
Showing 7 changed files with 143 additions and 13 deletions.
15 changes: 11 additions & 4 deletions Libraries/Animated/src/AnimatedImplementation.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ type AnimationConfig = {
class Animation {
__active: bool;
__isInteraction: bool;
__nativeTag: number;
__nativeId: number;
__onEnd: ?EndCallback;
start(
fromValue: number,
Expand All @@ -91,7 +91,11 @@ class Animation {
previousAnimation: ?Animation,
animatedValue: AnimatedValue
): void {}
stop(): void {}
stop(): void {
if (this.__nativeId) {
NativeAnimatedAPI.stopAnimation(this.__nativeId);
}
}
_getNativeAnimationConfig(): any {
// Subclasses that have corresponding animation implementation done in native
// should override this method
Expand All @@ -105,9 +109,9 @@ class Animation {
}
__startNativeAnimation(animatedValue: AnimatedValue): void {
animatedValue.__makeNative();
this.__nativeTag = NativeAnimatedHelper.generateNewAnimationTag();
this.__nativeId = NativeAnimatedHelper.generateNewAnimationId();
NativeAnimatedAPI.startAnimatingNode(
this.__nativeTag,
this.__nativeId,
animatedValue.__getNativeTag(),
this._getNativeAnimationConfig(),
this.__debouncedOnEnd.bind(this)
Expand Down Expand Up @@ -311,6 +315,7 @@ class TimingAnimation extends Animation {
}

stop(): void {
super.stop();
this.__active = false;
clearTimeout(this._timeout);
window.cancelAnimationFrame(this._animationFrame);
Expand Down Expand Up @@ -381,6 +386,7 @@ class DecayAnimation extends Animation {
}

stop(): void {
super.stop();
this.__active = false;
window.cancelAnimationFrame(this._animationFrame);
this.__debouncedOnEnd({finished: false});
Expand Down Expand Up @@ -595,6 +601,7 @@ class SpringAnimation extends Animation {
}

stop(): void {
super.stop();
this.__active = false;
window.cancelAnimationFrame(this._animationFrame);
this.__debouncedOnEnd({finished: false});
Expand Down
16 changes: 10 additions & 6 deletions Libraries/Animated/src/NativeAnimatedHelper.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ var NativeAnimatedModule = require('NativeModules').NativeAnimatedModule;
var invariant = require('fbjs/lib/invariant');

var __nativeAnimatedNodeTagCount = 1; /* used for animated nodes */
var __nativeAnimationTagCount = 1; /* used for started animations */
var __nativeAnimationIdCount = 1; /* used for started animations */

type EndResult = {finished: bool};
type EndCallback = (result: EndResult) => void;
Expand All @@ -38,9 +38,13 @@ var API = {
assertNativeAnimatedModule();
NativeAnimatedModule.disconnectAnimatedNodes(parentTag, childTag);
},
startAnimatingNode: function(animationTag: number, nodeTag: number, config: Object, endCallback: EndCallback): void {
startAnimatingNode: function(animationId: number, nodeTag: number, config: Object, endCallback: EndCallback): void {
assertNativeAnimatedModule();
NativeAnimatedModule.startAnimatingNode(nodeTag, config, endCallback);
NativeAnimatedModule.startAnimatingNode(animationId, nodeTag, config, endCallback);
},
stopAnimation: function(animationId: number) {
assertNativeAnimatedModule();
NativeAnimatedModule.stopAnimation(animationId);
},
setAnimatedNodeValue: function(nodeTag: number, value: number): void {
assertNativeAnimatedModule();
Expand Down Expand Up @@ -101,8 +105,8 @@ function generateNewNodeTag(): number {
return __nativeAnimatedNodeTagCount++;
}

function generateNewAnimationTag(): number {
return __nativeAnimationTagCount++;
function generateNewAnimationId(): number {
return __nativeAnimationIdCount++;
}

function assertNativeAnimatedModule(): void {
Expand All @@ -114,6 +118,6 @@ module.exports = {
validateProps,
validateStyles,
generateNewNodeTag,
generateNewAnimationTag,
generateNewAnimationId,
assertNativeAnimatedModule,
};
21 changes: 21 additions & 0 deletions Libraries/Animated/src/__tests__/AnimatedNative-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ describe('Animated', () => {
nativeAnimatedModule.connectAnimatedNodes = jest.genMockFunction();
nativeAnimatedModule.disconnectAnimatedNodes = jest.genMockFunction();
nativeAnimatedModule.startAnimatingNode = jest.genMockFunction();
nativeAnimatedModule.stopAnimation = jest.genMockFunction();
nativeAnimatedModule.setAnimatedNodeValue = jest.genMockFunction();
nativeAnimatedModule.connectAnimatedNodeToView = jest.genMockFunction();
nativeAnimatedModule.disconnectAnimatedNodeFromView = jest.genMockFunction();
Expand Down Expand Up @@ -59,6 +60,7 @@ describe('Animated', () => {
expect(nativeAnimatedModule.connectAnimatedNodes.mock.calls.length).toBe(2);

expect(nativeAnimatedModule.startAnimatingNode).toBeCalledWith(
jasmine.any(Number),
jasmine.any(Number),
{type: 'frames', frames: jasmine.any(Array), toValue: jasmine.any(Number)},
jasmine.any(Function)
Expand Down Expand Up @@ -164,6 +166,7 @@ describe('Animated', () => {

var nativeAnimatedModule = require('NativeModules').NativeAnimatedModule;
expect(nativeAnimatedModule.startAnimatingNode).toBeCalledWith(
jasmine.any(Number),
jasmine.any(Number),
{type: 'frames', frames: jasmine.any(Array), toValue: jasmine.any(Number)},
jasmine.any(Function)
Expand Down Expand Up @@ -269,4 +272,22 @@ describe('Animated', () => {
.toBeCalledWith(jasmine.any(Number), { type: 'props', props: { style: jasmine.any(Number) }});
});

it('send stopAnimation command to native', () => {
var value = new Animated.Value(0);
var animation = Animated.timing(value, {toValue: 10, duration: 50, useNativeDriver: true});
var nativeAnimatedModule = require('NativeModules').NativeAnimatedModule;

animation.start();
expect(nativeAnimatedModule.startAnimatingNode).toBeCalledWith(
jasmine.any(Number),
jasmine.any(Number),
{type: 'frames', frames: jasmine.any(Array), toValue: jasmine.any(Number)},
jasmine.any(Function)
);
var animationId = nativeAnimatedModule.startAnimatingNode.mock.calls[0][0];

animation.stop();
expect(nativeAnimatedModule.stopAnimation).toBeCalledWith(animationId);
});

});
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@
*/
/*package*/ abstract class AnimationDriver {

boolean mHasFinished = false;
ValueAnimatedNode mAnimatedValue;
Callback mEndCallback;
/*package*/ boolean mHasFinished = false;
/*package*/ ValueAnimatedNode mAnimatedValue;
/*package*/ Callback mEndCallback;
/*package*/ int mId;

/**
* This method gets called in the main animation loop with a frame time passed down from the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,20 +212,32 @@ public void execute(NativeAnimatedNodesManager animatedNodesManager) {

@ReactMethod
public void startAnimatingNode(
final int animationId,
final int animatedNodeTag,
final ReadableMap animationConfig,
final Callback endCallback) {
mOperations.add(new UIThreadOperation() {
@Override
public void execute(NativeAnimatedNodesManager animatedNodesManager) {
animatedNodesManager.startAnimatingNode(
animationId,
animatedNodeTag,
animationConfig,
endCallback);
}
});
}

@ReactMethod
public void stopAnimation(final int animationId) {
mOperations.add(new UIThreadOperation() {
@Override
public void execute(NativeAnimatedNodesManager animatedNodesManager) {
animatedNodesManager.stopAnimation(animationId);
}
});
}

@ReactMethod
public void connectAnimatedNodes(final int parentNodeTag, final int childNodeTag) {
mOperations.add(new UIThreadOperation() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ public void setAnimatedNodeValue(int tag, double value) {
}

public void startAnimatingNode(
int animationId,
int animatedNodeTag,
ReadableMap animationConfig,
Callback endCallback) {
Expand All @@ -117,11 +118,34 @@ public void startAnimatingNode(
} else {
throw new JSApplicationIllegalArgumentException("Unsupported animation type: " + type);
}
animation.mId = animationId;
animation.mEndCallback = endCallback;
animation.mAnimatedValue = (ValueAnimatedNode) node;
mActiveAnimations.add(animation);
}

public void stopAnimation(int animationId) {
// in most of the cases there should never be more than a few active animations running at the
// same time. Therefore it does not make much sense to create an animationId -> animation
// object map that would require additional memory just to support the use-case of stopping
// an animation
for (int i = 0; i < mActiveAnimations.size(); i++) {
AnimationDriver animation = mActiveAnimations.get(i);
if (animation.mId == animationId) {
// Invoke animation end callback with {finished: false}
WritableMap endCallbackResponse = Arguments.createMap();
endCallbackResponse.putBoolean("finished", false);
animation.mEndCallback.invoke(endCallbackResponse);
mActiveAnimations.remove(i);
return;
}
}
// Do not throw an error in the case animation could not be found. We only keep "active"
// animations in the registry and there is a chance that Animated.js will enqueue a
// stopAnimation call after the animation has ended or the call will reach native thread only
// when the animation is already over.
}

public void connectAnimatedNodes(int parentNodeTag, int childNodeTag) {
AnimatedNode parentNode = mAnimatedNodes.get(parentNodeTag);
if (parentNode == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,12 @@
import org.robolectric.RobolectricTestRunner;

import static org.fest.assertions.api.Assertions.assertThat;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyInt;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;

Expand Down Expand Up @@ -110,6 +113,7 @@ public void testFramesAnimation() {
JavaOnlyArray frames = JavaOnlyArray.of(0d, 0.2d, 0.4d, 0.6d, 0.8d, 1d);
Callback animationCallback = mock(Callback.class);
mNativeAnimatedNodesManager.startAnimatingNode(
1,
1,
JavaOnlyMap.of("type", "frames", "frames", frames, "toValue", 1d),
animationCallback);
Expand Down Expand Up @@ -143,6 +147,7 @@ public void testAnimationCallbackFinish() {
JavaOnlyArray frames = JavaOnlyArray.of(0d, 1d);
Callback animationCallback = mock(Callback.class);
mNativeAnimatedNodesManager.startAnimatingNode(
1,
1,
JavaOnlyMap.of("type", "frames", "frames", frames, "toValue", 1d),
animationCallback);
Expand Down Expand Up @@ -209,11 +214,13 @@ public void testAdditionNode() {
Callback animationCallback = mock(Callback.class);
JavaOnlyArray frames = JavaOnlyArray.of(0d, 1d);
mNativeAnimatedNodesManager.startAnimatingNode(
1,
1,
JavaOnlyMap.of("type", "frames", "frames", frames, "toValue", 101d),
animationCallback);

mNativeAnimatedNodesManager.startAnimatingNode(
2,
2,
JavaOnlyMap.of("type", "frames", "frames", frames, "toValue", 1010d),
animationCallback);
Expand Down Expand Up @@ -258,6 +265,7 @@ public void testViewReceiveUpdatesIfOneOfAnimationHasntStarted() {
Callback animationCallback = mock(Callback.class);
JavaOnlyArray frames = JavaOnlyArray.of(0d, 1d);
mNativeAnimatedNodesManager.startAnimatingNode(
1,
1,
JavaOnlyMap.of("type", "frames", "frames", frames, "toValue", 101d),
animationCallback);
Expand Down Expand Up @@ -304,13 +312,15 @@ public void testViewReceiveUpdatesWhenOneOfAnimationHasFinished() {
// Start animating for the first addition input node, will have 2 frames only
JavaOnlyArray firstFrames = JavaOnlyArray.of(0d, 1d);
mNativeAnimatedNodesManager.startAnimatingNode(
1,
1,
JavaOnlyMap.of("type", "frames", "frames", firstFrames, "toValue", 200d),
animationCallback);

// Start animating for the first addition input node, will have 6 frames
JavaOnlyArray secondFrames = JavaOnlyArray.of(0d, 0.2d, 0.4d, 0.6d, 0.8d, 1d);
mNativeAnimatedNodesManager.startAnimatingNode(
2,
2,
JavaOnlyMap.of("type", "frames", "frames", secondFrames, "toValue", 1010d),
animationCallback);
Expand Down Expand Up @@ -370,11 +380,13 @@ public void testMultiplicationNode() {
Callback animationCallback = mock(Callback.class);
JavaOnlyArray frames = JavaOnlyArray.of(0d, 1d);
mNativeAnimatedNodesManager.startAnimatingNode(
1,
1,
JavaOnlyMap.of("type", "frames", "frames", frames, "toValue", 2d),
animationCallback);

mNativeAnimatedNodesManager.startAnimatingNode(
2,
2,
JavaOnlyMap.of("type", "frames", "frames", frames, "toValue", 10d),
animationCallback);
Expand All @@ -401,4 +413,53 @@ public void testMultiplicationNode() {
mNativeAnimatedNodesManager.runUpdates(nextFrameTime());
verifyNoMoreInteractions(mUIImplementationMock);
}

/**
* This test verifies that when {@link NativeAnimatedModule#stopAnimation} is called the animation
* will no longer be updating the nodes it has been previously attached to and that the animation
* callback will be triggered with {@code {finished: false}}
*/
@Test
public void testHandleStoppingAnimation() {
createSimpleAnimatedViewWithOpacity(1000, 0d);

JavaOnlyArray frames = JavaOnlyArray.of(0d, 0.2d, 0.4d, 0.6d, 0.8d, 1.0d);
Callback animationCallback = mock(Callback.class);
mNativeAnimatedNodesManager.startAnimatingNode(
404,
1,
JavaOnlyMap.of("type", "frames", "frames", frames, "toValue", 1d),
animationCallback);

ArgumentCaptor<ReadableMap> callbackResponseCaptor = ArgumentCaptor.forClass(ReadableMap.class);

reset(animationCallback);
reset(mUIImplementationMock);
mNativeAnimatedNodesManager.runUpdates(nextFrameTime());
mNativeAnimatedNodesManager.runUpdates(nextFrameTime());
verify(mUIImplementationMock, times(2))
.synchronouslyUpdateViewOnUIThread(anyInt(), any(ReactStylesDiffMap.class));
verifyNoMoreInteractions(animationCallback);

reset(animationCallback);
reset(mUIImplementationMock);
mNativeAnimatedNodesManager.stopAnimation(404);
verify(animationCallback).invoke(callbackResponseCaptor.capture());
verifyNoMoreInteractions(animationCallback);
verifyNoMoreInteractions(mUIImplementationMock);

assertThat(callbackResponseCaptor.getValue().hasKey("finished")).isTrue();
assertThat(callbackResponseCaptor.getValue().getBoolean("finished")).isFalse();

reset(animationCallback);
reset(mUIImplementationMock);
// Run "update" loop a few more times -> we expect no further updates nor callback calls to be
// triggered
for (int i = 0; i < 5; i++) {
mNativeAnimatedNodesManager.runUpdates(nextFrameTime());
}

verifyNoMoreInteractions(mUIImplementationMock);
verifyNoMoreInteractions(animationCallback);
}
}

0 comments on commit cd11738

Please sign in to comment.