From 2010e5b74a68d99bbb43d489b5e9b8cbd7be8e5e Mon Sep 17 00:00:00 2001 From: Juliusz Wajgelt <49338439+jwajgelt@users.noreply.github.com> Date: Fri, 9 Dec 2022 13:30:42 +0100 Subject: [PATCH] [iOS][Layout Animations] Remove `exiting` views from UIManager's view registry (#3824) ## Summary Currently, when views with an `exiting` animation are unmounted, any deleted in that transaction aren't removed from the UIManager's view registry (the mapping of react tags to native views). This can cause memory leaks of removed views. This PR restores the original behaviour of UIManager when removing views, allowing it to clean them up properly, and reattaches the views with `exiting` animations after the transaction removing the views has completed. ## Test plan - In example app, go to "Entering and Exiting with Layout" - Set a breakpoint somewhere in `RCTUIManager`, for example `_manageChildren`. - Press "Toggle" to create the animated views, and note the number of entries in `_viewRegistry`. - Disable the breakpoint, resume the app and start spamming the toggle button. - Enable the breakpoint again, and press toggle one more time. - The number of entries in `_viewRegistry` should be the same as before spamming toggle. --- ios/LayoutReanimation/REAAnimationsManager.h | 4 +- ios/LayoutReanimation/REAAnimationsManager.m | 52 ++++++++---- ios/LayoutReanimation/REAUIManager.mm | 84 +++++-------------- .../layoutReanimation/animationsManager.ts | 3 - 4 files changed, 62 insertions(+), 81 deletions(-) diff --git a/ios/LayoutReanimation/REAAnimationsManager.h b/ios/LayoutReanimation/REAAnimationsManager.h index e1a0a7a26f4..5ef6b3a310d 100644 --- a/ios/LayoutReanimation/REAAnimationsManager.h +++ b/ios/LayoutReanimation/REAAnimationsManager.h @@ -33,7 +33,9 @@ typedef void (^REAAnimationRemovingBlock)(NSNumber *_Nonnull tag); - (REASnapshot *)prepareSnapshotBeforeMountForView:(UIView *)view; - (BOOL)wantsHandleRemovalOfView:(UIView *)view; - (void)removeAnimationsFromSubtree:(UIView *)view; -- (void)removeChildren:(NSArray *)children fromContainer:(UIView *)container; +- (void)reattachAnimatedChildren:(NSArray> *)children + toContainer:(id)container + atIndices:(NSArray *)indices; - (void)onViewCreate:(UIView *)view after:(REASnapshot *)after; - (void)onViewUpdate:(UIView *)view before:(REASnapshot *)before after:(REASnapshot *)after; diff --git a/ios/LayoutReanimation/REAAnimationsManager.m b/ios/LayoutReanimation/REAAnimationsManager.m index 920ecc194cc..e30b0ea5846 100644 --- a/ios/LayoutReanimation/REAAnimationsManager.m +++ b/ios/LayoutReanimation/REAAnimationsManager.m @@ -279,20 +279,22 @@ - (void)maybeDropAncestors:(UIView *)child } } -- (BOOL)removeRecursive:(UIView *)view fromContainer:(UIView *)container withoutAnimation:(BOOL)removeImmediately; +- (BOOL)startAnimationsRecursive:(UIView *)view + shouldRemoveSubviewsWithoutAnimations:(BOOL)shouldRemoveSubviewsWithoutAnimations; { if (!view.reactTag) { return NO; } BOOL hasExitAnimation = _hasAnimationForTag(view.reactTag, @"exiting") || [_exitingViews objectForKey:view.reactTag]; BOOL hasAnimatedChildren = NO; - removeImmediately = removeImmediately && !hasExitAnimation; + shouldRemoveSubviewsWithoutAnimations = shouldRemoveSubviewsWithoutAnimations && !hasExitAnimation; NSMutableArray *toBeRemoved = [[NSMutableArray alloc] init]; for (UIView *subview in [view.reactSubviews copy]) { - if ([self removeRecursive:subview fromContainer:view withoutAnimation:removeImmediately]) { + if ([self startAnimationsRecursive:subview + shouldRemoveSubviewsWithoutAnimations:shouldRemoveSubviewsWithoutAnimations]) { hasAnimatedChildren = YES; - } else if (removeImmediately) { + } else if (shouldRemoveSubviewsWithoutAnimations) { [toBeRemoved addObject:subview]; } } @@ -307,14 +309,8 @@ - (BOOL)removeRecursive:(UIView *)view fromContainer:(UIView *)container without if (hasExitAnimation) { before = [[REASnapshot alloc] init:view]; } - // start exit animation - UIView *originalSuperview = view.superview; - NSUInteger originalIndex = [originalSuperview.subviews indexOfObjectIdenticalTo:view]; - [container removeReactSubview:view]; - // we don't want user interaction on exiting views - view.userInteractionEnabled = NO; - [originalSuperview insertSubview:view atIndex:originalIndex]; + // start exit animation if (hasExitAnimation && ![_exitingViews objectForKey:view.reactTag]) { NSDictionary *preparedValues = [self prepareDataForAnimatingWorklet:before.values frameConfig:ExitingFrame]; [_exitingViews setObject:view forKey:view.reactTag]; @@ -335,14 +331,40 @@ - (BOOL)removeRecursive:(UIView *)view fromContainer:(UIView *)container without // start new animations for it, and might as well remove // the layout animation config now _clearAnimationConfigForTag(view.reactTag); + + // we don't want user interaction on exiting views + view.userInteractionEnabled = NO; + return YES; } -- (void)removeChildren:(NSArray *)children fromContainer:(UIView *)container +- (void)reattachAnimatedChildren:(NSArray> *)children + toContainer:(id)container + atIndices:(NSArray *)indices { - for (UIView *removedChild in children) { - if (![self removeRecursive:removedChild fromContainer:container withoutAnimation:true]) { - [removedChild removeFromSuperview]; + if (![container isKindOfClass:[UIView class]]) { + return; + } + + // since we reattach only some of the views, + // we count the views we DIDN'T reattach + // and shift later views' indices by that number + // to make sure they appear at correct relative posisitons + // in the `subviews` array + int skippedViewsCount = 0; + + for (int i = 0; i < children.count; i++) { + id child = children[i]; + if (![child isKindOfClass:[UIView class]]) { + skippedViewsCount++; + continue; + } + UIView *childView = (UIView *)child; + NSNumber *originalIndex = indices[i]; + if ([self startAnimationsRecursive:childView shouldRemoveSubviewsWithoutAnimations:YES]) { + [(UIView *)container insertSubview:childView atIndex:[originalIndex intValue] - skippedViewsCount]; + } else { + skippedViewsCount++; } } } diff --git a/ios/LayoutReanimation/REAUIManager.mm b/ios/LayoutReanimation/REAUIManager.mm index eca019a99d6..47dfda2aa4c 100644 --- a/ios/LayoutReanimation/REAUIManager.mm +++ b/ios/LayoutReanimation/REAUIManager.mm @@ -24,12 +24,6 @@ - (void)_manageChildren:(NSNumber *)containerTag removeAtIndices:(NSArray *)removeAtIndices registry:(NSMutableDictionary> *)registry; -- (void)_removeChildren:(NSArray *)children fromContainer:(UIView *)container; - -- (void)_removeChildren:(NSArray *)children - fromContainer:(UIView *)container - withAnimation:(RCTLayoutAnimationGroup *)animation; - - (RCTViewManagerUIBlock)uiBlockWithLayoutUpdateForRootView:(RCTRootShadowView *)rootShadowView; - (NSArray> *)_childrenToRemoveFromContainer:(id)container @@ -37,7 +31,6 @@ - (RCTViewManagerUIBlock)uiBlockWithLayoutUpdateForRootView:(RCTRootShadowView * @end @implementation REAUIManager { - RCTLayoutAnimationGroup *_reactLayoutAnimationGroup; NSMutableDictionary> *> *_toBeRemovedRegister; NSMutableDictionary *_parentMapper; REAAnimationsManager *_animationsManager; @@ -51,13 +44,6 @@ + (NSString *)moduleName - (void)setBridge:(RCTBridge *)bridge { - // setting a layout animation group with a deleting animation in order to - // allows us to call a different method in RCTUIManager for cleaning up exiting views - RCTLayoutAnimation *deletingAnimation = [[RCTLayoutAnimation alloc] initWithDuration:0 config:@{}]; - _reactLayoutAnimationGroup = [[RCTLayoutAnimationGroup alloc] initWithCreatingLayoutAnimation:nil - updatingLayoutAnimation:nil - deletingLayoutAnimation:deletingAnimation - callback:nil]; if (!_blockSetter) { _blockSetter = true; @@ -79,21 +65,6 @@ - (void)setBridge:(RCTBridge *)bridge } } -- (void)_removeChildren:(NSArray *)children - fromContainer:(UIView *)container - withAnimation:(RCTLayoutAnimationGroup *)animation -{ - if (animation == _reactLayoutAnimationGroup) { - // if a removed view in this batch has an `exiting` animation, - // let REAAnimationsManager handle the removal - [_animationsManager removeChildren:children fromContainer:container]; - } else { - // otherwise, if there's a layout animation group set, - // delegate to the React Native implementation for layout animations - [super _removeChildren:children fromContainer:container withAnimation:animation]; - } -} - - (void)_manageChildren:(NSNumber *)containerTag moveFromIndices:(NSArray *)moveFromIndices moveToIndices:(NSArray *)moveToIndices @@ -102,41 +73,14 @@ - (void)_manageChildren:(NSNumber *)containerTag removeAtIndices:(NSArray *)removeAtIndices registry:(NSMutableDictionary> *)registry { - if (!reanimated::FeaturesConfig::isLayoutAnimationEnabled()) { - [super _manageChildren:containerTag - moveFromIndices:moveFromIndices - moveToIndices:moveToIndices - addChildReactTags:addChildReactTags - addAtIndices:addAtIndices - removeAtIndices:removeAtIndices - registry:registry]; - return; + bool isLayoutAnimationEnabled = reanimated::FeaturesConfig::isLayoutAnimationEnabled(); + id container; + NSArray> *permanentlyRemovedChildren; + if (isLayoutAnimationEnabled) { + container = registry[containerTag]; + permanentlyRemovedChildren = [self _childrenToRemoveFromContainer:container atIndices:removeAtIndices]; } - // Reanimated changes /start - BOOL isUIViewRegistry = ((id)registry == (id)[self valueForKey:@"_viewRegistry"]); - if (isUIViewRegistry) { - BOOL wasProxyRemovalSet = [self valueForKey:@"_layoutAnimationGroup"] == _reactLayoutAnimationGroup; - BOOL wantProxyRemoval = NO; - if (!wasProxyRemovalSet) { - id container = registry[containerTag]; - NSArray> *permanentlyRemovedChildren = [self _childrenToRemoveFromContainer:container - atIndices:removeAtIndices]; - for (UIView *removedChild in permanentlyRemovedChildren) { - if ([_animationsManager wantsHandleRemovalOfView:removedChild]) { - wantProxyRemoval = YES; - break; - } - [_animationsManager removeAnimationsFromSubtree:removedChild]; - } - if (wantProxyRemoval) { - // set layout animation group - [super setNextLayoutAnimationGroup:_reactLayoutAnimationGroup]; - } - } - } - // Reanimated changes /end - [super _manageChildren:containerTag moveFromIndices:moveFromIndices moveToIndices:moveToIndices @@ -144,6 +88,22 @@ - (void)_manageChildren:(NSNumber *)containerTag addAtIndices:addAtIndices removeAtIndices:removeAtIndices registry:registry]; + + if (isLayoutAnimationEnabled) { + // we sort the (index, view) pairs to make sure we insert views back in order + NSMutableArray *> *removedViewsWithIndices = [NSMutableArray new]; + for (int i = 0; i < removeAtIndices.count; i++) { + removedViewsWithIndices[i] = @[ removeAtIndices[i], permanentlyRemovedChildren[i] ]; + } + [removedViewsWithIndices + sortUsingComparator:^NSComparisonResult(NSArray *_Nonnull obj1, NSArray *_Nonnull obj2) { + return [(NSNumber *)obj1[0] compare:(NSNumber *)obj2[0]]; + }]; + + [_animationsManager reattachAnimatedChildren:permanentlyRemovedChildren + toContainer:container + atIndices:removeAtIndices]; + } } - (void)callAnimationForTree:(UIView *)view parentTag:(NSNumber *)parentTag diff --git a/src/reanimated2/layoutReanimation/animationsManager.ts b/src/reanimated2/layoutReanimation/animationsManager.ts index 33de28d2fcb..bd173fae583 100644 --- a/src/reanimated2/layoutReanimation/animationsManager.ts +++ b/src/reanimated2/layoutReanimation/animationsManager.ts @@ -1,6 +1,5 @@ import { runOnUI } from '../core'; import { withStyleAnimation } from '../animation/styleAnimation'; -import { LogBox } from 'react-native'; import { SharedValue } from '../commonTypes'; import { makeUIMutable } from '../mutables'; import { @@ -8,8 +7,6 @@ import { LayoutAnimationsValues, } from './animationBuilder'; -LogBox.ignoreLogs(['Overriding previous layout animation with']); - const TAG_OFFSET = 1e9; function startObservingProgress(