Animated module: don't use random IDs as waiting identifier for queueing

Summary:
In D24521951 (2ff1d4c041) I refactored the JS-side queueing for NativeAnimated API calls, and used randomized IDs for queueing. This could cause bugs or unexpected behavior, and potentially crashes, if there's ever a collision in random numbers generated or
a collision between random number and one of the deterministic numbers generated in createAnimatedComponent.

In this diff I make both of them namespaced with a string, and deterministic, to eliminate any potential collisions. This could also be slightly faster (but not meaningfully) since we're not relying on Math.random.

Changelog: [Internal]

Reviewed By: yungsters

Differential Revision: D24553557

fbshipit-source-id: 8b765e21597ad4f8e641453c1f9f90bdf1ee022f
This commit is contained in:
Joshua Gross 2020-10-26 22:12:09 -07:00 коммит произвёл Facebook GitHub Bot
Родитель e9116519c8
Коммит 3ddba567a8
3 изменённых файлов: 13 добавлений и 6 удалений

Просмотреть файл

@ -53,11 +53,11 @@ const API = {
NativeAnimatedModule.getValue(tag, saveValueCallback); NativeAnimatedModule.getValue(tag, saveValueCallback);
} }
}, },
setWaitingForIdentifier: function(id: number): void { setWaitingForIdentifier: function(id: string): void {
waitingForQueuedOperations.add(id); waitingForQueuedOperations.add(id);
queueOperations = true; queueOperations = true;
}, },
unsetWaitingForIdentifier: function(id: number): void { unsetWaitingForIdentifier: function(id: string): void {
waitingForQueuedOperations.delete(id); waitingForQueuedOperations.delete(id);
if (waitingForQueuedOperations.size === 0) { if (waitingForQueuedOperations.size === 0) {

Просмотреть файл

@ -24,6 +24,8 @@ export type AnimationConfig = {
iterations?: number, iterations?: number,
}; };
let startNativeAnimationNextId = 1;
// Important note: start() and stop() will only be called at most once. // Important note: start() and stop() will only be called at most once.
// Once an animation has been stopped or finished its course, it will // Once an animation has been stopped or finished its course, it will
// not be reused. // not be reused.
@ -57,8 +59,11 @@ class Animation {
onEnd && onEnd(result); onEnd && onEnd(result);
} }
__startNativeAnimation(animatedValue: AnimatedValue): void { __startNativeAnimation(animatedValue: AnimatedValue): void {
const arbitraryValue = Math.random(); const startNativeAnimationWaitId = `${startNativeAnimationNextId}:startAnimation`;
NativeAnimatedHelper.API.setWaitingForIdentifier(arbitraryValue); startNativeAnimationNextId += 1;
NativeAnimatedHelper.API.setWaitingForIdentifier(
startNativeAnimationWaitId,
);
try { try {
animatedValue.__makeNative(); animatedValue.__makeNative();
this.__nativeId = NativeAnimatedHelper.generateNewAnimationId(); this.__nativeId = NativeAnimatedHelper.generateNewAnimationId();
@ -71,7 +76,9 @@ class Animation {
} catch (e) { } catch (e) {
throw e; throw e;
} finally { } finally {
NativeAnimatedHelper.API.unsetWaitingForIdentifier(arbitraryValue); NativeAnimatedHelper.API.unsetWaitingForIdentifier(
startNativeAnimationWaitId,
);
} }
} }
} }

Просмотреть файл

@ -55,7 +55,7 @@ function createAnimatedComponent<Props: {+[string]: mixed, ...}, Instance>(
_eventDetachers: Array<Function> = []; _eventDetachers: Array<Function> = [];
// Only to be used in this file, and only in Fabric. // Only to be used in this file, and only in Fabric.
_animatedComponentId: number = animatedComponentNextId++; _animatedComponentId: string = `${animatedComponentNextId++}:animatedComponent`;
_attachNativeEvents() { _attachNativeEvents() {
// Make sure to get the scrollable node for components that implement // Make sure to get the scrollable node for components that implement