Skip to content

Commit

Permalink
[iOS][Layout Animations] Remove exiting views from UIManager's view…
Browse files Browse the repository at this point in the history
… registry (#3824)

<!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please follow the template so that the reviewers can easily understand what the code changes affect. -->

## 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.

<!-- Explain the motivation for this PR. Include "Fixes #<number>" if applicable. -->

## Test plan

<!-- Provide a minimal but complete code snippet that can be used to test out this change along with instructions how to run it and a description of the expected behavior. -->
- 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.
  • Loading branch information
jwajgelt authored and kmagiera committed Dec 13, 2022
1 parent bdd6c7b commit 2010e5b
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 81 deletions.
4 changes: 3 additions & 1 deletion ios/LayoutReanimation/REAAnimationsManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<UIView *> *)children fromContainer:(UIView *)container;
- (void)reattachAnimatedChildren:(NSArray<id<RCTComponent>> *)children
toContainer:(id<RCTComponent>)container
atIndices:(NSArray<NSNumber *> *)indices;
- (void)onViewCreate:(UIView *)view after:(REASnapshot *)after;
- (void)onViewUpdate:(UIView *)view before:(REASnapshot *)before after:(REASnapshot *)after;

Expand Down
52 changes: 37 additions & 15 deletions ios/LayoutReanimation/REAAnimationsManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}
}
Expand All @@ -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];
Expand All @@ -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<UIView *> *)children fromContainer:(UIView *)container
- (void)reattachAnimatedChildren:(NSArray<id<RCTComponent>> *)children
toContainer:(id<RCTComponent>)container
atIndices:(NSArray<NSNumber *> *)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<RCTComponent> 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++;
}
}
}
Expand Down
84 changes: 22 additions & 62 deletions ios/LayoutReanimation/REAUIManager.mm
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,13 @@ - (void)_manageChildren:(NSNumber *)containerTag
removeAtIndices:(NSArray<NSNumber *> *)removeAtIndices
registry:(NSMutableDictionary<NSNumber *, id<RCTComponent>> *)registry;

- (void)_removeChildren:(NSArray<UIView *> *)children fromContainer:(UIView *)container;

- (void)_removeChildren:(NSArray<UIView *> *)children
fromContainer:(UIView *)container
withAnimation:(RCTLayoutAnimationGroup *)animation;

- (RCTViewManagerUIBlock)uiBlockWithLayoutUpdateForRootView:(RCTRootShadowView *)rootShadowView;

- (NSArray<id<RCTComponent>> *)_childrenToRemoveFromContainer:(id<RCTComponent>)container
atIndices:(NSArray<NSNumber *> *)atIndices;
@end

@implementation REAUIManager {
RCTLayoutAnimationGroup *_reactLayoutAnimationGroup;
NSMutableDictionary<NSNumber *, NSMutableSet<id<RCTComponent>> *> *_toBeRemovedRegister;
NSMutableDictionary<NSNumber *, NSNumber *> *_parentMapper;
REAAnimationsManager *_animationsManager;
Expand All @@ -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;

Expand All @@ -79,21 +65,6 @@ - (void)setBridge:(RCTBridge *)bridge
}
}

- (void)_removeChildren:(NSArray<UIView *> *)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<NSNumber *> *)moveFromIndices
moveToIndices:(NSArray<NSNumber *> *)moveToIndices
Expand All @@ -102,48 +73,37 @@ - (void)_manageChildren:(NSNumber *)containerTag
removeAtIndices:(NSArray<NSNumber *> *)removeAtIndices
registry:(NSMutableDictionary<NSNumber *, id<RCTComponent>> *)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<RCTComponent> container;
NSArray<id<RCTComponent>> *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<RCTComponent> container = registry[containerTag];
NSArray<id<RCTComponent>> *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
addChildReactTags:addChildReactTags
addAtIndices:addAtIndices
removeAtIndices:removeAtIndices
registry:registry];

if (isLayoutAnimationEnabled) {
// we sort the (index, view) pairs to make sure we insert views back in order
NSMutableArray<NSArray<id> *> *removedViewsWithIndices = [NSMutableArray new];
for (int i = 0; i < removeAtIndices.count; i++) {
removedViewsWithIndices[i] = @[ removeAtIndices[i], permanentlyRemovedChildren[i] ];
}
[removedViewsWithIndices
sortUsingComparator:^NSComparisonResult(NSArray<id> *_Nonnull obj1, NSArray<id> *_Nonnull obj2) {
return [(NSNumber *)obj1[0] compare:(NSNumber *)obj2[0]];
}];

[_animationsManager reattachAnimatedChildren:permanentlyRemovedChildren
toContainer:container
atIndices:removeAtIndices];
}
}

- (void)callAnimationForTree:(UIView *)view parentTag:(NSNumber *)parentTag
Expand Down
3 changes: 0 additions & 3 deletions src/reanimated2/layoutReanimation/animationsManager.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
import { runOnUI } from '../core';
import { withStyleAnimation } from '../animation/styleAnimation';
import { LogBox } from 'react-native';
import { SharedValue } from '../commonTypes';
import { makeUIMutable } from '../mutables';
import {
LayoutAnimationFunction,
LayoutAnimationsValues,
} from './animationBuilder';

LogBox.ignoreLogs(['Overriding previous layout animation with']);

const TAG_OFFSET = 1e9;

function startObservingProgress(
Expand Down

0 comments on commit 2010e5b

Please sign in to comment.