Fix js sync on animation end in Animation Backend (#55566)

Summary:
Pull Request resolved: #55566

In Animation Backend, we allow animation frameowrks to request a js-thread sync of the current animation state. It is used by c++ Animated, and is meant to serve as a way to push animation changes to react through RSNRU, after the animation finsishes. This way we ensure that subsequent rerenders of the component don't bring back the old style value.

This approach is currently broken when the animation performs any main-thread commits, as in this case the `runtimeShadowNodeReference_` is not copied to new node revisions, so the js-thread sync commit cannot use RSNRU properly. The bug was not visible, because we don't clean up the registry in that case, we only do it for react commits.

This PR fixes the issue for the case when `updateRuntimeShadowNodeReferencesOnCommitThread` is enabled, as this fixes the RSNRU propagation, so we can clean-up the registry. If the flag is disabled, we don't cleanup the registry, as we want the next react commit to make sure the animation state is not overwritten.

# Changelog

[General][Added] - test for the Animation Backend js sync
[General][Changed] - TesterAnimationChoreographer changes the thread_local RSNRU flag when running animation update, to better simulate the real application use-case
[General][Changed] - AnimationBackend now cleans-up the AnimatedPropsRegistry after the js sync when `updateRuntimeShadowNodeReferencesOnCommitThread` is enabled

Reviewed By: zeyap

Differential Revision: D93414839

fbshipit-source-id: 1c2566d117c2da563ae35e95cefc5ea08313fc7d

Author: Bartlomiej Bloniarz

packages/react-native/Libraries/Animated/__tests__/AnimatedBackend-itest.js

@@ -4,7 +4,7 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  *
- * @fantom_flags useSharedAnimatedBackend:true
+ * @fantom_flags useSharedAnimatedBackend:true updateRuntimeShadowNodeReferencesOnCommitThread:*
  * @flow strict-local
  * @format
  */
@@ -15,8 +15,8 @@ import type {HostInstance} from 'react-native';
 
 import ensureInstance from '../../../src/private/__tests__/utilities/ensureInstance';
 import * as Fantom from '@react-native/fantom';
-import {createRef, useEffect, useState} from 'react';
-import {Animated, useAnimatedValue} from 'react-native';
+import {createRef, memo, useEffect, useMemo, useState} from 'react';
+import {Animated, View, useAnimatedValue} from 'react-native';
 import {allowStyleProp} from 'react-native/Libraries/Animated/NativeAnimatedAllowlist';
 import ReactNativeElement from 'react-native/src/private/webapis/dom/nodes/ReactNativeElement';
 
@@ -468,3 +468,90 @@ test('animate width, height and opacity at once', () => {
     root.getRenderedOutput({props: ['width', 'height', 'opacity']}).toJSX(),
   ).toEqual(<rn-view height="200.000000" opacity="0.5" width="200.000000" />);
 });
+
+test('animate width with memo and rerender (js sync test)', () => {
+  const viewRef = createRef<HostInstance>();
+  allowStyleProp('width');
+
+  let _widthAnimation;
+  let _setState;
+
+  function useAnimation() {
+    const animatedValue = useAnimatedValue(100);
+
+    useEffect(() => {
+      const animation = Animated.timing(animatedValue, {
+        toValue: 200,
+        duration: 1000,
+        useNativeDriver: true,
+      });
+      _widthAnimation = animation;
+      animation.start();
+
+      return () => {
+        animation.stop();
+      };
+    }, [animatedValue]);
+
+    return animatedValue;
+  }
+
+  const AnimatedComponent = memo(() => {
+    const animatedValue = useAnimation();
+
+    const animatedStyle = useMemo(() => {
+      return {
+        width: animatedValue,
+      };
+    }, [animatedValue]);
+
+    return (
+      <Animated.View
+        ref={viewRef}
+        style={[{backgroundColor: 'green', height: 100}, animatedStyle]}
+      />
+    );
+  });
+
+  function MyApp() {
+    const [state, setState] = useState(0);
+    _setState = setState;
+
+    return (
+      <>
+        <View key={state} />
+        <AnimatedComponent />
+      </>
+    );
+  }
+
+  const root = Fantom.createRoot();
+
+  Fantom.runTask(() => {
+    root.render(<MyApp />);
+  });
+
+  expect(root.getRenderedOutput({props: ['width', 'height']}).toJSX()).toEqual(
+    <rn-view height="100.000000" width="100.000000" />,
+  );
+
+  Fantom.unstable_produceFramesForDuration(1000);
+
+  // TODO: this shouldn't be necessary since animation should be stopped after duration
+  Fantom.runTask(() => {
+    _widthAnimation?.stop();
+  });
+
+  expect(root.getRenderedOutput({props: ['width']}).toJSX()).toEqual(
+    <rn-view width="200.000000" />,
+  );
+
+  // Trigger rerender after animation completes to see if animation state gets overwritten
+  Fantom.runTask(() => {
+    _setState(s => 1 - s);
+  });
+
+  expect(root.getRenderedOutput({props: ['width']}).toJSX()).toEqual(
+    <rn-view width="200.000000" />,
+  );
+});

packages/react-native/ReactCommon/react/renderer/animationbackend/AnimationBackend.cpp

@@ -9,6 +9,7 @@
 #include "AnimatedPropsRegistry.h"
 
 #include <react/debug/react_native_assert.h>
+#include <react/featureflags/ReactNativeFeatureFlags.h>
 #include <react/renderer/animationbackend/AnimatedPropsSerializer.h>
 #include <react/renderer/graphics/Color.h>
 #include <chrono>
@@ -152,8 +153,7 @@ void AnimationBackend::commitUpdates(
                         return shadowNode.clone(
                             {.props = newProps,
                              .children = fragment.children,
-                             .state = shadowNode.getState(),
-                             .runtimeShadowNodeReference = false});
+                             .state = shadowNode.getState()});
                       }));
             },
             {.mountSynchronously = true});
@@ -177,24 +177,45 @@ void AnimationBackend::requestAsyncFlushForSurfaces(
   react_native_assert(
       jsInvoker_ != nullptr ||
       surfaces.empty() && "jsInvoker_ was not provided");
+  std::weak_ptr<AnimatedPropsRegistry> weakAnimatedPropsRegistry =
+      animatedPropsRegistry_;
   for (const auto& surfaceId : surfaces) {
     // perform an empty commit on the js thread, to force the commit hook to
     // push updated shadow nodes to react through RSNRU
-    jsInvoker_->invokeAsync([weakUIManager = uiManager_, surfaceId]() {
-      auto uiManager = weakUIManager.lock();
-      if (!uiManager) {
-        return;
-      }
-      uiManager->getShadowTreeRegistry().visit(
-          surfaceId, [](const ShadowTree& shadowTree) {
-            shadowTree.commit(
-                [](const RootShadowNode& oldRootShadowNode) {
-                  return std::static_pointer_cast<RootShadowNode>(
-                      oldRootShadowNode.ShadowNode::clone({}));
-                },
-                {.source = ShadowTreeCommitSource::AnimationEndSync});
-          });
-    });
+    jsInvoker_->invokeAsync(
+        [weakUIManager = uiManager_, surfaceId, weakAnimatedPropsRegistry]() {
+          auto uiManager = weakUIManager.lock();
+          if (!uiManager) {
+            return;
+          }
+          uiManager->getShadowTreeRegistry().visit(
+              surfaceId,
+              [weakAnimatedPropsRegistry](const ShadowTree& shadowTree) {
+                auto result = shadowTree.commit(
+                    [weakAnimatedPropsRegistry](
+                        const RootShadowNode& oldRootShadowNode) {
+                      return std::static_pointer_cast<RootShadowNode>(
+                          oldRootShadowNode.ShadowNode::clone({}));
+                    },
+                    {.source = ShadowTreeCommitSource::AnimationEndSync});
+                // To clear the registry, the updates neeed to be propagated to
+                // React with RSNRU. Without
+                // updateRuntimeShadowNodeReferencesOnCommitThread this won't
+                // happen if we do any commits on the main thread, since the
+                // runtimeShadowNodeReference_ is not propagated to nodes cloned
+                // outside of the JS thread. So when the flag is disabled we
+                // keep the updates in the registry and we will reapply them in
+                // a commit hook triggered by a rerender.
+                if (result == ShadowTree::CommitStatus::Succeeded &&
+                    ReactNativeFeatureFlags::
+                        updateRuntimeShadowNodeReferencesOnCommitThread()) {
+                  if (auto animatedPropsRegistry =
+                          weakAnimatedPropsRegistry.lock()) {
+                    animatedPropsRegistry->clear(shadowTree.getSurfaceId());
+                  }
+                }
+              });
+        });
   }
 }
 

private/react-native-fantom/tester/src/TesterAnimationChoreographer.cpp

@@ -7,6 +7,7 @@
 
 #include "TesterAnimationChoreographer.h"
 #include <react/renderer/core/ReactPrimitives.h>
+#include <react/renderer/core/ShadowNode.h>
 #include <react/runtime/ReactInstanceConfig.h>
 
 namespace facebook::react {
@@ -20,7 +21,9 @@ void TesterAnimationChoreographer::pause() {
 
 void TesterAnimationChoreographer::runUITick(AnimationTimestamp timestamp) {
   if (!isPaused_) {
+    ShadowNode::setUseRuntimeShadowNodeReferenceUpdateOnThread(false);
     onAnimationFrame(timestamp);
+    ShadowNode::setUseRuntimeShadowNodeReferenceUpdateOnThread(true);
   }
 }