From 3ddba567a876dec355b37ce4817251afdcf0cab3 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Mon, 26 Oct 2020 22:12:09 -0700 Subject: [PATCH] Animated module: don't use random IDs as waiting identifier for queueing Summary: In D24521951 (https://github.com/facebook/react-native/commit/2ff1d4c041ee662871a84363a3f85a8bc9e857ce) 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 --- Libraries/Animated/NativeAnimatedHelper.js | 4 ++-- Libraries/Animated/animations/Animation.js | 13 ++++++++++--- Libraries/Animated/createAnimatedComponent.js | 2 +- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/Libraries/Animated/NativeAnimatedHelper.js b/Libraries/Animated/NativeAnimatedHelper.js index 03af60499f..af78a8d098 100644 --- a/Libraries/Animated/NativeAnimatedHelper.js +++ b/Libraries/Animated/NativeAnimatedHelper.js @@ -53,11 +53,11 @@ const API = { NativeAnimatedModule.getValue(tag, saveValueCallback); } }, - setWaitingForIdentifier: function(id: number): void { + setWaitingForIdentifier: function(id: string): void { waitingForQueuedOperations.add(id); queueOperations = true; }, - unsetWaitingForIdentifier: function(id: number): void { + unsetWaitingForIdentifier: function(id: string): void { waitingForQueuedOperations.delete(id); if (waitingForQueuedOperations.size === 0) { diff --git a/Libraries/Animated/animations/Animation.js b/Libraries/Animated/animations/Animation.js index ef06af5e79..53967d96f9 100644 --- a/Libraries/Animated/animations/Animation.js +++ b/Libraries/Animated/animations/Animation.js @@ -24,6 +24,8 @@ export type AnimationConfig = { iterations?: number, }; +let startNativeAnimationNextId = 1; + // Important note: start() and stop() will only be called at most once. // Once an animation has been stopped or finished its course, it will // not be reused. @@ -57,8 +59,11 @@ class Animation { onEnd && onEnd(result); } __startNativeAnimation(animatedValue: AnimatedValue): void { - const arbitraryValue = Math.random(); - NativeAnimatedHelper.API.setWaitingForIdentifier(arbitraryValue); + const startNativeAnimationWaitId = `${startNativeAnimationNextId}:startAnimation`; + startNativeAnimationNextId += 1; + NativeAnimatedHelper.API.setWaitingForIdentifier( + startNativeAnimationWaitId, + ); try { animatedValue.__makeNative(); this.__nativeId = NativeAnimatedHelper.generateNewAnimationId(); @@ -71,7 +76,9 @@ class Animation { } catch (e) { throw e; } finally { - NativeAnimatedHelper.API.unsetWaitingForIdentifier(arbitraryValue); + NativeAnimatedHelper.API.unsetWaitingForIdentifier( + startNativeAnimationWaitId, + ); } } } diff --git a/Libraries/Animated/createAnimatedComponent.js b/Libraries/Animated/createAnimatedComponent.js index fa62bb3527..230aa16420 100644 --- a/Libraries/Animated/createAnimatedComponent.js +++ b/Libraries/Animated/createAnimatedComponent.js @@ -55,7 +55,7 @@ function createAnimatedComponent( _eventDetachers: Array = []; // Only to be used in this file, and only in Fabric. - _animatedComponentId: number = animatedComponentNextId++; + _animatedComponentId: string = `${animatedComponentNextId++}:animatedComponent`; _attachNativeEvents() { // Make sure to get the scrollable node for components that implement