From e532f861ecb0764a12e38f50836e708e2db31a3a Mon Sep 17 00:00:00 2001 From: Vincent Riemer Date: Wed, 10 Aug 2022 16:44:59 -0700 Subject: [PATCH] Fix ghost leave/out events firing due to view recycling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Changelog: [iOS][Internal] - Fix ghost pointer leave/out events firing due to view recycling on iOS While implementing the properties on the PointerEvent object on iOS I noticed that in certain specific scenarios I was seeing pointerLeave events being fired seemingly without corresponding pointerEvent events and even more strangely, when the pointer wasn't even close to the view in question. After a lot of research I discovered that this was caused by an incompatibility between my strategy of keeping track/identifying views which are being hovered and RN's handling of creating/deleting views on iOS. See on iOS RN has the `RCTComponentViewRegistry` class which manages the creation & deletion of UIViews and adds an optimization of recycling views instead of outright deleting them. This is causing issues with my tracking of which views are hovered because I compare the view's object references which, while accurate towards confirming equality of an underlying UIView — isn't accurate in confirming the equality of views from react's perspective. This diff addresses this issue by adding a simple wrapper class that can be used around the UIViews which stores the view's react ID at initialization time ensuring it doesn't get updated even if the underlying view's react id is. As an additional precaution the wrapper class will also not return the view it's wrapping if their react tags do not match. Reviewed By: lunaleaps Differential Revision: D38546628 fbshipit-source-id: 8b732d52da0e61a5447001e8940e4439f49c6baf --- React/Fabric/RCTSurfaceTouchHandler.mm | 53 +++++++++++++---------- React/Fabric/Utils/RCTReactTaggedView.h | 34 +++++++++++++++ React/Fabric/Utils/RCTReactTaggedView.mm | 55 ++++++++++++++++++++++++ packages/rn-tester/Podfile.lock | 2 +- 4 files changed, 121 insertions(+), 23 deletions(-) create mode 100644 React/Fabric/Utils/RCTReactTaggedView.h create mode 100644 React/Fabric/Utils/RCTReactTaggedView.mm diff --git a/React/Fabric/RCTSurfaceTouchHandler.mm b/React/Fabric/RCTSurfaceTouchHandler.mm index 67c6aa9b60d..65371954e60 100644 --- a/React/Fabric/RCTSurfaceTouchHandler.mm +++ b/React/Fabric/RCTSurfaceTouchHandler.mm @@ -7,6 +7,7 @@ #import "RCTSurfaceTouchHandler.h" +#import #import #import #import @@ -237,12 +238,12 @@ static UIView *FindClosestFabricManagedTouchableView(UIView *componentView) return nil; } -static NSOrderedSet *GetTouchableViewsInPathToRoot(UIView *componentView) +static NSOrderedSet *GetTouchableViewsInPathToRoot(UIView *componentView) { NSMutableOrderedSet *results = [NSMutableOrderedSet orderedSet]; do { if ([componentView respondsToSelector:@selector(touchEventEmitterAtPoint:)]) { - [results addObject:componentView]; + [results addObject:[RCTReactTaggedView wrap:componentView]]; } componentView = componentView.superview; } while (componentView); @@ -378,9 +379,10 @@ static BOOL AnyTouchesChanged(NSSet *touches) return NO; } -static BOOL IsViewListeningToEvent(UIView *view, ViewEvents::Offset eventType) +static BOOL IsViewListeningToEvent(RCTReactTaggedView *taggedView, ViewEvents::Offset eventType) { - if ([view.class conformsToProtocol:@protocol(RCTComponentViewProtocol)]) { + UIView *view = taggedView.view; + if (view && [view.class conformsToProtocol:@protocol(RCTComponentViewProtocol)]) { auto props = ((id)view).props; if (SharedViewProps viewProps = std::dynamic_pointer_cast(props)) { return viewProps->events[eventType]; @@ -389,10 +391,10 @@ static BOOL IsViewListeningToEvent(UIView *view, ViewEvents::Offset eventType) return NO; } -static BOOL IsAnyViewInPathListeningToEvent(NSOrderedSet *viewPath, ViewEvents::Offset eventType) +static BOOL IsAnyViewInPathListeningToEvent(NSOrderedSet *viewPath, ViewEvents::Offset eventType) { - for (UIView *view in viewPath) { - if (IsViewListeningToEvent(view, eventType)) { + for (RCTReactTaggedView *taggedView in viewPath) { + if (IsViewListeningToEvent(taggedView, eventType)) { return YES; } } @@ -427,7 +429,7 @@ struct PointerHasher { IdentifierPool<11> _identifierPool; UIHoverGestureRecognizer *_hoverRecognizer API_AVAILABLE(ios(13.0)); - NSOrderedSet *_currentlyHoveredViews; + NSOrderedSet *_currentlyHoveredViews; int _primaryTouchPointerId; } @@ -780,7 +782,10 @@ RCT_NOT_IMPLEMENTED(-(instancetype)initWithTarget : (id)target action : (SEL)act UIView *targetView = [listenerView hitTest:clientLocation withEvent:nil]; targetView = FindClosestFabricManagedTouchableView(targetView); - UIView *prevTargetView = [_currentlyHoveredViews firstObject]; + + RCTReactTaggedView *targetTaggedView = [RCTReactTaggedView wrap:targetView]; + RCTReactTaggedView *prevTargetTaggedView = [_currentlyHoveredViews firstObject]; + UIView *prevTargetView = prevTargetTaggedView.view; CGPoint offsetLocation = [recognizer locationInView:targetView]; @@ -791,12 +796,12 @@ RCT_NOT_IMPLEMENTED(-(instancetype)initWithTarget : (id)target action : (SEL)act modifierFlags = 0; } - NSOrderedSet *eventPathViews = GetTouchableViewsInPathToRoot(targetView); + NSOrderedSet *eventPathViews = GetTouchableViewsInPathToRoot(targetView); BOOL hasMoveListenerInEventPath = NO; // Over - if (prevTargetView != targetView) { + if (prevTargetTaggedView.tag != targetTaggedView.tag) { BOOL shouldEmitOverEvent = IsAnyViewInPathListeningToEvent(eventPathViews, ViewEvents::Offset::PointerOver); SharedTouchEventEmitter eventEmitter = GetTouchEmitterFromView(targetView, [recognizer locationInView:targetView]); if (shouldEmitOverEvent && eventEmitter != nil) { @@ -815,11 +820,13 @@ RCT_NOT_IMPLEMENTED(-(instancetype)initWithTarget : (id)target action : (SEL)act // of events we send to JS BOOL hasParentEnterListener = NO; - for (UIView *componentView in [eventPathViews reverseObjectEnumerator]) { - BOOL shouldEmitEvent = - hasParentEnterListener || IsViewListeningToEvent(componentView, ViewEvents::Offset::PointerEnter); + for (RCTReactTaggedView *taggedView in [eventPathViews reverseObjectEnumerator]) { + UIView *componentView = taggedView.view; - if (shouldEmitEvent && ![_currentlyHoveredViews containsObject:componentView]) { + BOOL shouldEmitEvent = componentView != nil && + (hasParentEnterListener || IsViewListeningToEvent(taggedView, ViewEvents::Offset::PointerEnter)); + + if (shouldEmitEvent && ![_currentlyHoveredViews containsObject:taggedView]) { SharedTouchEventEmitter eventEmitter = GetTouchEmitterFromView(componentView, [recognizer locationInView:componentView]); if (eventEmitter != nil) { @@ -833,7 +840,7 @@ RCT_NOT_IMPLEMENTED(-(instancetype)initWithTarget : (id)target action : (SEL)act hasParentEnterListener = YES; } - if (!hasMoveListenerInEventPath && IsViewListeningToEvent(componentView, ViewEvents::Offset::PointerMove)) { + if (!hasMoveListenerInEventPath && IsViewListeningToEvent(taggedView, ViewEvents::Offset::PointerMove)) { hasMoveListenerInEventPath = YES; } } @@ -849,7 +856,7 @@ RCT_NOT_IMPLEMENTED(-(instancetype)initWithTarget : (id)target action : (SEL)act } // Out - if (prevTargetView != targetView) { + if (prevTargetView != nil && prevTargetTaggedView.tag != targetTaggedView.tag) { BOOL shouldEmitOutEvent = IsAnyViewInPathListeningToEvent(_currentlyHoveredViews, ViewEvents::Offset::PointerOut); SharedTouchEventEmitter eventEmitter = GetTouchEmitterFromView(prevTargetView, [recognizer locationInView:prevTargetView]); @@ -866,14 +873,16 @@ RCT_NOT_IMPLEMENTED(-(instancetype)initWithTarget : (id)target action : (SEL)act // we also need to efficiently keep track of if a view has a parent which is listening to the leave events, // so we first iterate from the root to the target, collecting the views which need events fired for, of which // we reverse iterate (now from target to root), actually emitting the events. - NSMutableOrderedSet *viewsToEmitLeaveEventsTo = [NSMutableOrderedSet orderedSet]; + NSMutableOrderedSet *viewsToEmitLeaveEventsTo = [NSMutableOrderedSet orderedSet]; BOOL hasParentLeaveListener = NO; - for (UIView *componentView in [_currentlyHoveredViews reverseObjectEnumerator]) { - BOOL shouldEmitEvent = - hasParentLeaveListener || IsViewListeningToEvent(componentView, ViewEvents::Offset::PointerLeave); + for (RCTReactTaggedView *taggedView in [_currentlyHoveredViews reverseObjectEnumerator]) { + UIView *componentView = taggedView.view; - if (shouldEmitEvent && ![eventPathViews containsObject:componentView]) { + BOOL shouldEmitEvent = componentView != nil && + (hasParentLeaveListener || IsViewListeningToEvent(taggedView, ViewEvents::Offset::PointerLeave)); + + if (shouldEmitEvent && ![eventPathViews containsObject:taggedView]) { [viewsToEmitLeaveEventsTo addObject:componentView]; } diff --git a/React/Fabric/Utils/RCTReactTaggedView.h b/React/Fabric/Utils/RCTReactTaggedView.h new file mode 100644 index 00000000000..6fa54392020 --- /dev/null +++ b/React/Fabric/Utils/RCTReactTaggedView.h @@ -0,0 +1,34 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#import + +NS_ASSUME_NONNULL_BEGIN + +/** + * Lightweight wrapper class around a UIView with a react tag which registers a + * constant react tag at initialization time for a stable hash and provides the + * udnerlying view to a caller if that underlying view's react tag has not + * changed from the one provided at initalization time (i.e. recycled). + */ +@interface RCTReactTaggedView : NSObject { + UIView *_view; + NSInteger _tag; +} + ++ (RCTReactTaggedView *)wrap:(UIView *)view; + +- (instancetype)initWithView:(UIView *)view; +- (nullable UIView *)view; +- (NSInteger)tag; + +- (BOOL)isEqual:(id)other; +- (NSUInteger)hash; + +@end + +NS_ASSUME_NONNULL_END diff --git a/React/Fabric/Utils/RCTReactTaggedView.mm b/React/Fabric/Utils/RCTReactTaggedView.mm new file mode 100644 index 00000000000..4a4a60ff830 --- /dev/null +++ b/React/Fabric/Utils/RCTReactTaggedView.mm @@ -0,0 +1,55 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#import "RCTReactTaggedView.h" + +@implementation RCTReactTaggedView + ++ (RCTReactTaggedView *)wrap:(UIView *)view +{ + return [[RCTReactTaggedView alloc] initWithView:view]; +} + +- (instancetype)initWithView:(UIView *)view +{ + if (self = [super init]) { + _view = view; + _tag = view.tag; + } + return self; +} + +- (nullable UIView *)view +{ + if (_view.tag == _tag) { + return _view; + } + return nil; +} + +- (NSInteger)tag +{ + return _tag; +} + +- (BOOL)isEqual:(id)other +{ + if (other == self) { + return YES; + } + if (!other || ![other isKindOfClass:[self class]]) { + return NO; + } + return _tag == [other tag]; +} + +- (NSUInteger)hash +{ + return _tag; +} + +@end diff --git a/packages/rn-tester/Podfile.lock b/packages/rn-tester/Podfile.lock index 3c6574fec14..f7061a0a78a 100644 --- a/packages/rn-tester/Podfile.lock +++ b/packages/rn-tester/Podfile.lock @@ -912,7 +912,7 @@ SPEC CHECKSUMS: React-bridging: cc10a051eff1f03306a1d7659593d8aac3242bc3 React-callinvoker: 5f16202ad4e45f0607b1fae0f6955a8f7c87eef1 React-Codegen: 5adf19af97eb37a7d441c040521191e446255086 - React-Core: 0cfb25c65d4dcb856b1807fe44a1ebe5e7ec9749 + React-Core: ce4282fb714ffbe444b84d296d1728eaee4d0e9f React-CoreModules: 675170bccf156da3a3348e04e2036ce401b2010d React-cxxreact: 7276467c246302fedf598cc40d7003896ddb20ba React-Fabric: abfd61dc5498ce165634af85d65fcc42b82b5bf4