Skip to content

Commit

Permalink
Fix operation scheduling on iOS (#2483)
Browse files Browse the repository at this point in the history
## Description


#2467
replaces calls to `setImmediate` and `requestAnimationFrame` with
`queueMicrotask`. This introduces a bug, where on iOS the
`flushOperations` method would get called too early.

This PR changes `flushOperations` to only be used on the new
architecture, where it's actually required, and adds a retry mechanism
for attaching handlers to native views. It also updates parts of the new
arch-related code to function properly - finding the recognizer
responsible for the touch responder and attaching the root view handler.

Fixes
#2482

## Test plan

Test on the Example and FabricExample apps.

---------

Co-authored-by: Tomek Zawadzki <[email protected]>
  • Loading branch information
j-piasecki and tomekzaw authored May 17, 2023
1 parent 5c25dc2 commit 9926b7c
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 7 deletions.
63 changes: 56 additions & 7 deletions ios/RNGestureHandlerManager.mm
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#ifdef RCT_NEW_ARCH_ENABLED
#import <React/RCTSurfaceTouchHandler.h>
#import <React/RCTSurfaceView.h>
#import <React/RCTViewComponentView.h>
#else
#import <React/RCTTouchHandler.h>
Expand All @@ -37,6 +38,8 @@
RCTDefaultLogFunction( \
RCTLogLevelInfo, RCTLogSourceNative, @(__FILE__), @(__LINE__), [NSString stringWithFormat:__VA_ARGS__])

constexpr int NEW_ARCH_NUMBER_OF_ATTACH_RETRIES = 25;

@interface RNGestureHandlerManager () <RNGestureHandlerEventEmitter, RNRootViewGestureRecognizerDelegate>

@end
Expand All @@ -45,6 +48,7 @@ @implementation RNGestureHandlerManager {
RNGestureHandlerRegistry *_registry;
RCTUIManager *_uiManager;
NSHashTable<RNRootViewGestureRecognizer *> *_rootViewGestureRecognizers;
NSMutableDictionary<NSNumber *, NSNumber *> *_attachRetryCounter;
RCTEventDispatcher *_eventDispatcher;
id _reanimatedModule;
}
Expand All @@ -56,6 +60,7 @@ - (instancetype)initWithUIManager:(RCTUIManager *)uiManager eventDispatcher:(RCT
_eventDispatcher = eventDispatcher;
_registry = [RNGestureHandlerRegistry new];
_rootViewGestureRecognizers = [NSHashTable hashTableWithOptions:NSPointerFunctionsWeakMemory];
_attachRetryCounter = [[NSMutableDictionary alloc] init];
_reanimatedModule = nil;
}
return self;
Expand Down Expand Up @@ -100,12 +105,39 @@ - (void)attachGestureHandler:(nonnull NSNumber *)handlerTag
UIView *view = [_uiManager viewForReactTag:viewTag];

#ifdef RCT_NEW_ARCH_ENABLED
if (view == nil) {
// Happens when the view with given tag has been flattened.
// We cannot attach gesture handler to a non-existent view.
if (view == nil || view.superview == nil) {
// There are a few reasons we could end up here:
// - the native view corresponding to the viewtag hasn't yet been created
// - the native view has been created, but it's not attached to window
// - the native view will not exist because it got flattened
// In the first two cases we just want to wait until the view gets created or gets attached to its superview
// In the third case we don't want to do anything but we cannot easily distinguish it here, hece the abomination
// below
// TODO: would be great to have a better solution, although it might require migration to the shadow nodes from
// viewTags

NSNumber *counter = [_attachRetryCounter objectForKey:viewTag];
if (counter == nil) {
counter = @1;
} else {
counter = [NSNumber numberWithInt:counter.intValue + 1];
}

if (counter.intValue > NEW_ARCH_NUMBER_OF_ATTACH_RETRIES) {
[_attachRetryCounter removeObjectForKey:viewTag];
} else {
[_attachRetryCounter setObject:counter forKey:viewTag];

dispatch_after(dispatch_time(DISPATCH_TIME_NOW, 0.1 * NSEC_PER_SEC), dispatch_get_main_queue(), ^{
[self attachGestureHandler:handlerTag toViewWithTag:viewTag withActionType:actionType];
});
}

return;
}

[_attachRetryCounter removeObjectForKey:viewTag];

// I think it should be moved to RNNativeViewHandler, but that would require
// additional logic for setting contentView.reactTag, this works for now
if ([view isKindOfClass:[RCTViewComponentView class]]) {
Expand Down Expand Up @@ -164,18 +196,26 @@ - (id)handlerWithTag:(NSNumber *)handlerTag

- (void)registerViewWithGestureRecognizerAttachedIfNeeded:(UIView *)childView
{
#ifdef RCT_NEW_ARCH_ENABLED
UIView *touchHandlerView = childView;

while (touchHandlerView != nil && ![touchHandlerView isKindOfClass:[RCTSurfaceView class]]) {
touchHandlerView = touchHandlerView.superview;
}
#else
UIView *parent = childView;
while (parent != nil && ![parent respondsToSelector:@selector(touchHandler)])
parent = parent.superview;

// Many views can return the same touchHandler so we check if the one we want to register
// is not already present in the set.
UIView *touchHandlerView = [[parent performSelector:@selector(touchHandler)] view];
#endif // RCT_NEW_ARCH_ENABLED

if (touchHandlerView == nil) {
return;
}

// Many views can return the same touchHandler so we check if the one we want to register
// is not already present in the set.
for (UIGestureRecognizer *recognizer in touchHandlerView.gestureRecognizers) {
if ([recognizer isKindOfClass:[RNRootViewGestureRecognizer class]]) {
return;
Expand Down Expand Up @@ -208,10 +248,19 @@ - (void)gestureRecognizer:(UIGestureRecognizer *)gestureRecognizer
return;

#ifdef RCT_NEW_ARCH_ENABLED
RCTSurfaceTouchHandler *touchHandler = [viewWithTouchHandler performSelector:@selector(touchHandler)];
UIGestureRecognizer *touchHandler = nil;

// touchHandler (RCTSurfaceTouchHandler) is private in RCTFabricSurface so we have to do
// this little trick to get access to it
for (UIGestureRecognizer *recognizer in [viewWithTouchHandler gestureRecognizers]) {
if ([recognizer isKindOfClass:[RCTSurfaceTouchHandler class]]) {
touchHandler = recognizer;
break;
}
}
#else
RCTTouchHandler *touchHandler = [viewWithTouchHandler performSelector:@selector(touchHandler)];
#endif
#endif // RCT_NEW_ARCH_ENABLED
[touchHandler setEnabled:NO];
[touchHandler setEnabled:YES];
}
Expand Down
4 changes: 4 additions & 0 deletions ios/RNGestureHandlerModule.mm
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,9 @@ - (void)setBridge:(RCTBridge *)bridge

RCT_EXPORT_METHOD(flushOperations)
{
// On the new arch we rely on `flushOperations` for scheduling the operations on the UI thread.
// On the old arch we rely on `uiManagerWillPerformMounting`
#ifdef RCT_NEW_ARCH_ENABLED
if (_operations.count == 0) {
return;
}
Expand All @@ -197,6 +200,7 @@ - (void)setBridge:(RCTBridge *)bridge
operation(self->_manager);
}
}];
#endif // RCT_NEW_ARCH_ENABLED
}

- (void)setGestureState:(int)state forHandler:(int)handlerTag
Expand Down

0 comments on commit 9926b7c

Please sign in to comment.