Fix NativeAnimation invalidation & races on iOS

Summary:
This diff attempts to fix a number of iOS native animation bugs related to improper node invalidation and a race with view creation. The major issues were presented in #9120 as problems 3 and 3b, but I'll recap here:

The invalidation model we use is overly complicated and incomplete. The proper combination of `_needsUpdate` and `_hasUpdated` will result in nodes values being recomputed. However, we do not invalidate nodes in all the places we should, e.g. if we create a new view and attach it to an existing value node (see example in #9120). This diff chooses to remove the `_hasUpdated` flag, and simply relies on the `_needsUpdate` flag to mark a node as dirty.

We mark nodes as dirty when they are:
- created
- updated
- attached to new parents
- detached from old parents
- attached to a view

Calling `updateNodeIfNecessary` will, if necessary, compute all invalidated parent values before recomputing the node value. It will then apply the update, and mark the no
Closes https://github.com/facebook/react-native/pull/10663

Differential Revision: D4120301

Pulled By: mkonicek

fbshipit-source-id: e247afcb5d8c15999b8328c664b9f7e764d76a75

Libraries/NativeAnimation/Nodes/RCTAnimatedNode.h

@@ -21,7 +21,6 @@
 @property (nonatomic, copy, readonly) NSDictionary<NSNumber *, RCTAnimatedNode *> *parentNodes;
 
 @property (nonatomic, readonly) BOOL needsUpdate;
-@property (nonatomic, readonly) BOOL hasUpdated;
 
 /**
  * Marks a node and its children as needing update.
@@ -38,11 +37,6 @@
  */
 - (void)performUpdate NS_REQUIRES_SUPER;
 
-/**
- * Cleans up after a round of updates.
- */
-- (void)cleanupAnimationUpdate NS_REQUIRES_SUPER;
-
 - (void)addChild:(RCTAnimatedNode *)child NS_REQUIRES_SUPER;
 - (void)removeChild:(RCTAnimatedNode *)child NS_REQUIRES_SUPER;
 

Libraries/NativeAnimation/Nodes/RCTAnimatedNode.m

@@ -93,30 +93,15 @@ RCT_NOT_IMPLEMENTED(- (instancetype)init)
 
 - (void)setNeedsUpdate
 {
-  if (_needsUpdate) {
-    // Has already been marked. Stop branch.
-    return;
-  }
   _needsUpdate = YES;
   for (RCTAnimatedNode *child in _childNodes.allValues) {
     [child setNeedsUpdate];
   }
 }
 
-- (void)cleanupAnimationUpdate
-{
-  if (_hasUpdated) {
-    _needsUpdate = NO;
-    _hasUpdated = NO;
-    for (RCTAnimatedNode *child in _childNodes.allValues) {
-      [child cleanupAnimationUpdate];
-    }
-  }
-}
-
 - (void)updateNodeIfNecessary
 {
-  if (_needsUpdate && !_hasUpdated) {
+  if (_needsUpdate) {
     for (RCTAnimatedNode *parent in _parentNodes.allValues) {
       [parent updateNodeIfNecessary];
     }
@@ -126,7 +111,7 @@ RCT_NOT_IMPLEMENTED(- (instancetype)init)
 
 - (void)performUpdate
 {
-  _hasUpdated = YES;
+  _needsUpdate = NO;
   // To be overidden by subclasses
   // This method is called on a node only if it has been marked for update
   // during the current update loop

Libraries/NativeAnimation/Nodes/RCTPropsAnimatedNode.h

@@ -9,14 +9,14 @@
 
 #import "RCTAnimatedNode.h"
 
-@class RCTNativeAnimatedModule;
+@class RCTUIManager;
 @class RCTViewPropertyMapper;
 
 @interface RCTPropsAnimatedNode : RCTAnimatedNode
 
 @property (nonatomic, readonly) RCTViewPropertyMapper *propertyMapper;
 
-- (void)connectToView:(NSNumber *)viewTag animatedModule:(RCTNativeAnimatedModule *)animationModule;
+- (void)connectToView:(NSNumber *)viewTag uiManager:(RCTUIManager *)uiManager;
 - (void)disconnectFromView:(NSNumber *)viewTag;
 
 - (void)performViewUpdatesIfNecessary;

Libraries/NativeAnimation/Nodes/RCTPropsAnimatedNode.m

@@ -8,35 +8,17 @@
  */
 
 #import "RCTPropsAnimatedNode.h"
+
 #import "RCTAnimationUtils.h"
-#import "RCTNativeAnimatedModule.h"
 #import "RCTStyleAnimatedNode.h"
+#import "RCTValueAnimatedNode.h"
 #import "RCTViewPropertyMapper.h"
 
 @implementation RCTPropsAnimatedNode
-{
-  RCTStyleAnimatedNode *_parentNode;
-}
 
-- (void)onAttachedToNode:(RCTAnimatedNode *)parent
+- (void)connectToView:(NSNumber *)viewTag uiManager:(RCTUIManager *)uiManager
 {
-  [super onAttachedToNode:parent];
-  if ([parent isKindOfClass:[RCTStyleAnimatedNode class]]) {
-    _parentNode = (RCTStyleAnimatedNode *)parent;
-  }
-}
-
-- (void)onDetachedFromNode:(RCTAnimatedNode *)parent
-{
-  [super onDetachedFromNode:parent];
-  if (_parentNode == parent) {
-    _parentNode = nil;
-  }
-}
-
-- (void)connectToView:(NSNumber *)viewTag animatedModule:(RCTNativeAnimatedModule *)animationModule
-{
-  _propertyMapper = [[RCTViewPropertyMapper alloc] initWithViewTag:viewTag animationModule:animationModule];
+  _propertyMapper = [[RCTViewPropertyMapper alloc] initWithViewTag:viewTag uiManager:uiManager];
 }
 
 - (void)disconnectFromView:(NSNumber *)viewTag
@@ -50,11 +32,36 @@
   [self performViewUpdatesIfNecessary];
 }
 
+- (NSString *)propertyNameForParentTag:(NSNumber *)parentTag
+{
+  __block NSString *propertyName;
+  [self.config[@"props"] enumerateKeysAndObjectsUsingBlock:^(NSString * _Nonnull property, NSNumber * _Nonnull tag, BOOL * _Nonnull stop) {
+    if ([tag isEqualToNumber:parentTag]) {
+      propertyName = property;
+      *stop = YES;
+    }
+  }];
+  return propertyName;
+}
+
 - (void)performViewUpdatesIfNecessary
 {
-  NSDictionary *updates = [_parentNode updatedPropsDictionary];
-  if (updates.count) {
-    [_propertyMapper updateViewWithDictionary:updates];
+  NSMutableDictionary *props = [NSMutableDictionary dictionary];
+  [self.parentNodes enumerateKeysAndObjectsUsingBlock:^(NSNumber * _Nonnull parentTag, RCTAnimatedNode * _Nonnull parentNode, BOOL * _Nonnull stop) {
+
+    if ([parentNode isKindOfClass:[RCTStyleAnimatedNode class]]) {
+      [props addEntriesFromDictionary:[(RCTStyleAnimatedNode *)parentNode propsDictionary]];
+
+    } else if ([parentNode isKindOfClass:[RCTValueAnimatedNode class]]) {
+      NSString *property = [self propertyNameForParentTag:parentTag];
+      CGFloat value = [(RCTValueAnimatedNode *)parentNode value];
+      [props setObject:@(value) forKey:property];
+    }
+
+  }];
+
+  if (props.count) {
+    [_propertyMapper updateViewWithDictionary:props];
   }
 }
 

Libraries/NativeAnimation/Nodes/RCTStyleAnimatedNode.h

@@ -11,6 +11,6 @@
 
 @interface RCTStyleAnimatedNode : RCTAnimatedNode
 
-- (NSDictionary<NSString *, NSObject *> *)updatedPropsDictionary;
+- (NSDictionary<NSString *, NSObject *> *)propsDictionary;
 
 @end

Libraries/NativeAnimation/Nodes/RCTStyleAnimatedNode.m

@@ -14,21 +14,21 @@
 
 @implementation RCTStyleAnimatedNode
 {
-  NSMutableDictionary<NSString *, NSObject *> *_updatedPropsDictionary;
+  NSMutableDictionary<NSString *, NSObject *> *_propsDictionary;
 }
 
 - (instancetype)initWithTag:(NSNumber *)tag
                      config:(NSDictionary<NSString *, id> *)config;
 {
   if ((self = [super initWithTag:tag config:config])) {
-    _updatedPropsDictionary = [NSMutableDictionary new];
+    _propsDictionary = [NSMutableDictionary new];
   }
   return self;
 }
 
-- (NSDictionary *)updatedPropsDictionary
+- (NSDictionary *)propsDictionary
 {
-  return _updatedPropsDictionary;
+  return _propsDictionary;
 }
 
 - (void)performUpdate
@@ -38,22 +38,16 @@
   NSDictionary<NSString *, NSNumber *> *style = self.config[@"style"];
   [style enumerateKeysAndObjectsUsingBlock:^(NSString *property, NSNumber *nodeTag, __unused BOOL *stop) {
     RCTAnimatedNode *node = self.parentNodes[nodeTag];
-    if (node && node.hasUpdated) {
+    if (node) {
       if ([node isKindOfClass:[RCTValueAnimatedNode class]]) {
         RCTValueAnimatedNode *parentNode = (RCTValueAnimatedNode *)node;
-        [self->_updatedPropsDictionary setObject:@(parentNode.value) forKey:property];
+        [self->_propsDictionary setObject:@(parentNode.value) forKey:property];
       } else if ([node isKindOfClass:[RCTTransformAnimatedNode class]]) {
         RCTTransformAnimatedNode *parentNode = (RCTTransformAnimatedNode *)node;
-        [self->_updatedPropsDictionary addEntriesFromDictionary:parentNode.updatedPropsDictionary];
+        [self->_propsDictionary addEntriesFromDictionary:parentNode.propsDictionary];
       }
     }
   }];
 }
 
-- (void)cleanupAnimationUpdate
-{
-  [super cleanupAnimationUpdate];
-  [_updatedPropsDictionary removeAllObjects];
-}
-
 @end

Libraries/NativeAnimation/Nodes/RCTTransformAnimatedNode.h

@@ -11,6 +11,6 @@
 
 @interface RCTTransformAnimatedNode : RCTAnimatedNode
 
-- (NSDictionary<NSString *, NSObject *> *)updatedPropsDictionary;
+- (NSDictionary<NSString *, NSObject *> *)propsDictionary;
 
 @end

Libraries/NativeAnimation/Nodes/RCTTransformAnimatedNode.m

@@ -12,21 +12,21 @@
 
 @implementation RCTTransformAnimatedNode
 {
-  NSMutableDictionary<NSString *, NSObject *> *_updatedPropsDictionary;
+  NSMutableDictionary<NSString *, NSObject *> *_propsDictionary;
 }
 
 - (instancetype)initWithTag:(NSNumber *)tag
                      config:(NSDictionary<NSString *, id> *)config;
 {
   if ((self = [super initWithTag:tag config:config])) {
-    _updatedPropsDictionary = [NSMutableDictionary new];
+    _propsDictionary = [NSMutableDictionary new];
   }
   return self;
 }
 
-- (NSDictionary *)updatedPropsDictionary
+- (NSDictionary *)propsDictionary
 {
-  return _updatedPropsDictionary;
+  return _propsDictionary;
 }
 
 - (void)performUpdate
@@ -44,7 +44,7 @@
     if ([type isEqualToString: @"animated"]) {
       NSNumber *nodeTag = transformConfig[@"nodeTag"];
       RCTAnimatedNode *node = self.parentNodes[nodeTag];
-      if (!node.hasUpdated || ![node isKindOfClass:[RCTValueAnimatedNode class]]) {
+      if (![node isKindOfClass:[RCTValueAnimatedNode class]]) {
         continue;
       }
       RCTValueAnimatedNode *parentNode = (RCTValueAnimatedNode *)node;
@@ -82,13 +82,7 @@
     }
   }
 
-  _updatedPropsDictionary[@"transform"] = [NSValue valueWithCATransform3D:transform];
-}
-
-- (void)cleanupAnimationUpdate
-{
-  [super cleanupAnimationUpdate];
-  [_updatedPropsDictionary removeAllObjects];
+  _propsDictionary[@"transform"] = [NSValue valueWithCATransform3D:transform];
 }
 
 @end