Browse files

Only retain the previousViews that need to be validated

Fixes #4740, where views would unnecessarily be retained after performing `navigator.pop()` - this was particularly problematic for big lists and memory-intensive custom views.

This fix causes no functional change: `_previousViews` are only used in the loop starting at line 564 to ensure that the JavaScript and Native navigation stacks are equivalent at all times. As we do in this fix, that loop limits itself to only the views expected to be on the React navigation stack. So overall this change makes the code logically 'more correct'.

Tested by checking that `_previousViews.count` is always equivalent to `previousReactCount` in the loop (which means we could remove the complex `MIN(... MIN(previousReactCount, _previousViews.count)` in the loop too, but I wanted to keep the diff as small as possible for now).
Closes #10789

Differential Revision: D4140502

Pulled By: ericvicenti

fbshipit-source-id: 4491ad3c16642914c3081295cf95c4cf36be9f94
  • Loading branch information...
ephemer authored and Facebook Github Bot committed Nov 7, 2016
1 parent 0089cd7 commit a4bb4d25f5ed1414e40de92df7d7c20e148804e6
Showing with 4 additions and 1 deletion.
  1. +4 −1 React/Views/RCTNavigator.m
@@ -590,7 +590,10 @@ - (void)reactBridgeDidFinishTransaction
- _previousViews = [self.reactSubviews copy];
+ // Only make a copy of the subviews whose validity we expect to be able to check (in the loop, above),
+ // otherwise we would unnecessarily retain a reference to view(s) no longer on the React navigation stack:
+ NSUInteger expectedCount = MIN(currentReactCount, self.reactSubviews.count);
+ _previousViews = [[self.reactSubviews subarrayWithRange: NSMakeRange(0, expectedCount)] copy];
_previousRequestedTopOfStack = _requestedTopOfStack;

0 comments on commit a4bb4d2

Please sign in to comment.