From a69c18cfbc19eeee38e80ea8963343ee4eb7fe83 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Sat, 30 Nov 2019 12:29:34 -0800 Subject: [PATCH] Fabric: Moving ScrollView's delegate splitter to RCTEnhancedScrollView Summary: This diff moves the delegate splitter from RCTScrollViewComponentView class to RCTEnhancedScrollView. Now, it's a key feature of RCTEnhancedScrollView. We need this to make `delegate` property of our UIScrollView subclass as resilient to any abuse as possible. E.g., if some other part of the app, unrelated to RN (e.g. BottomSheet component), nils the property, all dependent RN specific infra should continue to work. To make it possible, we make an exposed property to use the internal delegate splitter instead of providing direct access to the property of a superclass. Changelog: [Internal] Fabric-specific internal change. Reviewed By: sammy-SC Differential Revision: D18752886 fbshipit-source-id: 04ec4758afefb8e17481d69471d53c61ab396698 --- .../ScrollView/RCTEnhancedScrollView.h | 18 +++++- .../ScrollView/RCTEnhancedScrollView.mm | 55 ++++++++++++++++++- .../ScrollView/RCTScrollViewComponentView.mm | 12 +--- 3 files changed, 73 insertions(+), 12 deletions(-) diff --git a/React/Fabric/Mounting/ComponentViews/ScrollView/RCTEnhancedScrollView.h b/React/Fabric/Mounting/ComponentViews/ScrollView/RCTEnhancedScrollView.h index 091b0d951d2..9132adb5624 100644 --- a/React/Fabric/Mounting/ComponentViews/ScrollView/RCTEnhancedScrollView.h +++ b/React/Fabric/Mounting/ComponentViews/ScrollView/RCTEnhancedScrollView.h @@ -7,16 +7,30 @@ #import +#import #import NS_ASSUME_NONNULL_BEGIN -/** +/* * `UIScrollView` subclass which has some improvements and tweaks - * which are not directly related to React. + * which are not directly related to React Native. */ @interface RCTEnhancedScrollView : UIScrollView +/* + * Returns a delegate splitter that can be used to create as many `UIScrollView` delegates as needed. + * Use that instead of accessing `delegate` property directly. + * + * This class overrides the `delegate` property and wires that to the delegate splitter. + * + * We never know which another part of the app might introspect the view hierarchy and mess with `UIScrollView`'s + * delegate, so we expose a fake delegate connected to the original one via the splitter to make the component as + * resilient to other code as possible: even if something else nil the delegate, other delegates that were subscribed + * via the splitter will continue working. + */ +@property (nonatomic, strong, readonly) RCTGenericDelegateSplitter> *delegateSplitter; + @property (nonatomic, assign) BOOL pinchGestureEnabled; @property (nonatomic, assign) BOOL centerContent; diff --git a/React/Fabric/Mounting/ComponentViews/ScrollView/RCTEnhancedScrollView.mm b/React/Fabric/Mounting/ComponentViews/ScrollView/RCTEnhancedScrollView.mm index e2e41057f60..97867b3d9dd 100644 --- a/React/Fabric/Mounting/ComponentViews/ScrollView/RCTEnhancedScrollView.mm +++ b/React/Fabric/Mounting/ComponentViews/ScrollView/RCTEnhancedScrollView.mm @@ -7,7 +7,20 @@ #import "RCTEnhancedScrollView.h" -@implementation RCTEnhancedScrollView +@implementation RCTEnhancedScrollView { + __weak id _publicDelegate; +} + ++ (BOOL)automaticallyNotifiesObserversForKey:(NSString *)key +{ + if ([key isEqualToString:@"delegate"]) { + // For `delegate` property, we issue KVO notifications manually. + // We need that to block notifications caused by setting the original `UIScrollView`s property. + return NO; + } + + return [super automaticallyNotifiesObserversForKey:key]; +} - (instancetype)initWithFrame:(CGRect)frame { @@ -18,9 +31,49 @@ // and keeps it as an opt-in behavior. self.contentInsetAdjustmentBehavior = UIScrollViewContentInsetAdjustmentNever; } + + __weak __typeof(self) weakSelf = self; + _delegateSplitter = [[RCTGenericDelegateSplitter alloc] initWithDelegateUpdateBlock:^(id delegate) { + [weakSelf setPrivateDelegate:delegate]; + }]; } return self; } +- (void)dealloc +{ + // This is not strictly necessary but that prevents a crash caused by a bug in UIKit. + [self setPrivateDelegate:nil]; +} + +- (void)setPrivateDelegate:(id)delegate +{ + [super setDelegate:delegate]; +} + +- (id)delegate +{ + return _publicDelegate; +} + +- (void)setDelegate:(id)delegate +{ + if (_publicDelegate == delegate) { + return; + } + + if (_publicDelegate) { + [_delegateSplitter removeDelegate:_publicDelegate]; + } + + [self willChangeValueForKey:@"delegate"]; + _publicDelegate = delegate; + [self didChangeValueForKey:@"delegate"]; + + if (_publicDelegate) { + [_delegateSplitter addDelegate:_publicDelegate]; + } +} + @end diff --git a/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm b/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm index 709e9c03e6a..ca45882d7d1 100644 --- a/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm +++ b/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm @@ -71,12 +71,7 @@ static void RCTSendPaperScrollEvent_DEPRECATED(UIScrollView *scrollView, NSInteg _containerView = [[UIView alloc] initWithFrame:CGRectZero]; [_scrollView addSubview:_containerView]; - __weak __typeof(self) weakSelf = self; - _scrollViewDelegateSplitter = [[RCTGenericDelegateSplitter alloc] initWithDelegateUpdateBlock:^(id delegate) { - weakSelf.scrollView.delegate = delegate; - }]; - - [_scrollViewDelegateSplitter addDelegate:self]; + [self.scrollViewDelegateSplitter addDelegate:self]; _scrollEventThrottle = INFINITY; } @@ -84,10 +79,9 @@ static void RCTSendPaperScrollEvent_DEPRECATED(UIScrollView *scrollView, NSInteg return self; } -- (void)dealloc +- (RCTGenericDelegateSplitter> *)scrollViewDelegateSplitter { - // This is not strictly necessary but that prevents a crash caused by a bug in UIKit. - _scrollView.delegate = nil; + return ((RCTEnhancedScrollView *)_scrollView).delegateSplitter; } #pragma mark - RCTComponentViewProtocol