Merge pull request facebookarchive#1624 from maicki/FixDelegateProxyC…

…rash

[Accessibility / iOS 8] Fix 8.0 and 8.1-only, VoiceOver crash when clearing delegate / dataSource for Table or Collection

AsyncDisplayKit/ASCollectionView.mm

@@ -343,10 +343,10 @@ - (void)setAsyncDataSource:(id<ASCollectionViewDataSource>)asyncDataSource
 {
  // Note: It's common to check if the value hasn't changed and short-circuit but we aren't doing that here to handle
  // the (common) case of nilling the asyncDataSource in the ViewController's dealloc. In this case our _asyncDataSource
- // will return as nil (ARC magic) even though the _proxyDataSource still exists. It's really important to nil out
- // super.dataSource in this case because calls to ASCollectionViewProxy will start failing and cause crashes.
-
- super.dataSource = nil;
+ // will return as nil (ARC magic) even though the _proxyDataSource still exists. It's really important to hold a strong
+ // reference to the old dataSource in this case because calls to ASCollectionViewProxy will start failing and cause crashes.
+ NS_VALID_UNTIL_END_OF_SCOPE id oldDataSource = super.dataSource;
+
  if (asyncDataSource == nil) {
  _asyncDataSource = nil;
  _proxyDataSource = _isDeallocating ? nil : [[ASCollectionViewProxy alloc] initWithTarget:nil interceptor:self];
@@ -375,13 +375,9 @@ - (void)setAsyncDelegate:(id<ASCollectionViewDelegate>)asyncDelegate
 {
  // Note: It's common to check if the value hasn't changed and short-circuit but we aren't doing that here to handle
  // the (common) case of nilling the asyncDelegate in the ViewController's dealloc. In this case our _asyncDelegate
- // will return as nil (ARC magic) even though the _proxyDelegate still exists. It's really important to nil out
- // super.delegate in this case because calls to ASCollectionViewProxy will start failing and cause crashes.
-
- // Order is important here, the asyncDelegate must be callable while nilling super.delegate to avoid random crashes
- // in UIScrollViewAccessibility.
-
- super.delegate = nil;
+ // will return as nil (ARC magic) even though the _proxyDataSource still exists. It's really important to hold a strong
+ // reference to the old delegate in this case because calls to ASCollectionViewProxy will start failing and cause crashes.
+ NS_VALID_UNTIL_END_OF_SCOPE id oldDelegate = super.delegate;
 
  if (asyncDelegate == nil) {
  _asyncDelegate = nil;

AsyncDisplayKit/ASTableView.mm

@@ -268,10 +268,9 @@ - (void)setAsyncDataSource:(id<ASTableViewDataSource>)asyncDataSource
 {
  // Note: It's common to check if the value hasn't changed and short-circuit but we aren't doing that here to handle
  // the (common) case of nilling the asyncDataSource in the ViewController's dealloc. In this case our _asyncDataSource
- // will return as nil (ARC magic) even though the _proxyDataSource still exists. It's really important to nil out
- // super.dataSource in this case because calls to ASTableViewProxy will start failing and cause crashes.
-
- super.dataSource = nil;
+ // will return as nil (ARC magic) even though the _proxyDataSource still exists. It's really important to hold a strong
+ // reference to the old dataSource in this case because calls to ASTableViewProxy will start failing and cause crashes.
+ NS_VALID_UNTIL_END_OF_SCOPE id oldDataSource = self.dataSource;
 
  if (asyncDataSource == nil) {
  _asyncDataSource = nil;
@@ -299,13 +298,9 @@ - (void)setAsyncDelegate:(id<ASTableViewDelegate>)asyncDelegate
 {
  // Note: It's common to check if the value hasn't changed and short-circuit but we aren't doing that here to handle
  // the (common) case of nilling the asyncDelegate in the ViewController's dealloc. In this case our _asyncDelegate
- // will return as nil (ARC magic) even though the _proxyDelegate still exists. It's really important to nil out
- // super.delegate in this case because calls to ASTableViewProxy will start failing and cause crashes.
-
- // Order is important here, the asyncDelegate must be callable while nilling super.delegate to avoid random crashes
- // in UIScrollViewAccessibility.
-
- super.delegate = nil;
+ // will return as nil (ARC magic) even though the _proxyDataSource still exists. It's really important to hold a strong
+ // reference to the old delegate in this case because calls to ASTableViewProxy will start failing and cause crashes.
+ NS_VALID_UNTIL_END_OF_SCOPE id oldDelegate = super.delegate;
 
  if (asyncDelegate == nil) {
  _asyncDelegate = nil;

AsyncDisplayKit/Details/ASDelegateProxy.m

@@ -125,7 +125,15 @@ - (id)forwardingTargetForSelector:(SEL)aSelector
  if (_target) {
  return [_target respondsToSelector:aSelector] ? _target : nil;
  } else {
- [_interceptor proxyTargetHasDeallocated:self];
+ // The _interceptor needs to be nilled out in this scenario. For that a strong reference needs to be created
+ // to be able to nil out the _interceptor but still let it know that the proxy target has deallocated
+ // We have to hold a strong reference to the interceptor as we have to nil it out and call the proxyTargetHasDeallocated
+ // The reason that the interceptor needs to be nilled out is that there maybe a change of a infinite loop, for example
+ // if a method will be called in the proxyTargetHasDeallocated: that again would trigger a whole new forwarding cycle
+ id <ASDelegateProxyInterceptor> interceptor = _interceptor;
+ _interceptor = nil;
+ [interceptor proxyTargetHasDeallocated:self];
+
  return nil;
  }
  }