Skip to content

Commit

Permalink
Fix RCTModalHostViewController crash while presenting react-native mo…
Browse files Browse the repository at this point in the history
…dals (#7423)

In some specific (and hard to reproduce) cases, RN modal presentation block tries to present a modal that is already presented in the hierarchy which caused that crash.

Addresses facebook/react-native#32366
  • Loading branch information
yogevbd committed Jan 18, 2022
1 parent 345344a commit b335451
Show file tree
Hide file tree
Showing 9 changed files with 115 additions and 31 deletions.
8 changes: 5 additions & 3 deletions lib/ios/RNNBridgeManager.mm
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@
#import "RNNComponentViewCreator.h"
#import "RNNEventEmitter.h"
#import "RNNLayoutManager.h"
#import "RNNModalHostViewManagerHandler.h"
#import "RNNReactComponentRegistry.h"
#import "RNNReactRootViewCreator.h"
#import "RNNSplashScreen.h"
#import <React/RCTBridge.h>
#import <React/RCTModalHostViewManager.h>
#import <React/RCTUIManager.h>

@interface RNNBridgeManager ()
Expand All @@ -19,6 +19,7 @@ @interface RNNBridgeManager ()
@property(nonatomic, strong, readonly) RNNLayoutManager *layoutManager;
@property(nonatomic, strong, readonly) RNNOverlayManager *overlayManager;
@property(nonatomic, strong, readonly) RNNModalManager *modalManager;
@property(nonatomic, strong, readonly) RNNModalHostViewManagerHandler *modalHostViewHandler;

@end

Expand Down Expand Up @@ -66,7 +67,8 @@ - (void)registerExternalComponent:(NSString *)name callback:(RNNExternalViewCrea
[[RNNModalManagerEventHandler alloc] initWithEventEmitter:eventEmitter];
_modalManager = [[RNNModalManager alloc] initWithBridge:bridge
eventHandler:modalManagerEventHandler];

_modalHostViewHandler =
[[RNNModalHostViewManagerHandler alloc] initWithModalManager:_modalManager];
_layoutManager = [[RNNLayoutManager alloc] init];

id<RNNComponentViewCreator> rootViewCreator =
Expand Down Expand Up @@ -105,7 +107,7 @@ - (void)onJavaScriptWillLoad {

- (void)onJavaScriptLoaded {
[_commandsHandler setReadyToReceiveCommands:true];
[_modalManager
[_modalHostViewHandler
connectModalHostViewManager:[self.bridge moduleForClass:RCTModalHostViewManager.class]];
[[_bridge moduleForClass:[RNNEventEmitter class]] sendOnAppLaunched];
}
Expand Down
11 changes: 11 additions & 0 deletions lib/ios/RNNModalHostViewManagerHandler.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#import "RNNModalManager.h"
#import <Foundation/Foundation.h>
#import <React/RCTModalHostViewManager.h>

@interface RNNModalHostViewManagerHandler : NSObject

- (instancetype)initWithModalManager:(RNNModalManager *)modalManager;

- (void)connectModalHostViewManager:(RCTModalHostViewManager *)modalHostViewManager;

@end
40 changes: 40 additions & 0 deletions lib/ios/RNNModalHostViewManagerHandler.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
#import "RNNModalHostViewManagerHandler.h"

@implementation RNNModalHostViewManagerHandler {
RNNModalManager *_modalManager;
}

- (instancetype)initWithModalManager:(RNNModalManager *)modalManager {
self = [super init];
_modalManager = modalManager;
return self;
}

- (void)connectModalHostViewManager:(RCTModalHostViewManager *)modalHostViewManager {
modalHostViewManager.presentationBlock =
^(UIViewController *reactViewController, UIViewController *viewController, BOOL animated,
dispatch_block_t completionBlock) {
if (reactViewController.presentedViewController != viewController &&
[self->_modalManager topPresentedVC] != viewController) {
[self->_modalManager showModal:viewController
animated:animated
completion:^(NSString *_Nonnull componentId) {
if (completionBlock)
completionBlock();
}];
}
};

modalHostViewManager.dismissalBlock =
^(UIViewController *reactViewController, UIViewController *viewController, BOOL animated,
dispatch_block_t completionBlock) {
[self->_modalManager dismissModal:viewController
animated:animated
completion:^{
if (completionBlock)
completionBlock();
}];
};
}

@end
4 changes: 2 additions & 2 deletions lib/ios/RNNModalManager.h
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#import "RNNModalManagerEventHandler.h"
#import <Foundation/Foundation.h>
#import <React/RCTBridge.h>
#import <React/RCTModalHostViewManager.h>
#import <UIKit/UIKit.h>

typedef void (^RNNTransitionCompletionBlock)(void);
Expand All @@ -13,7 +12,6 @@ typedef void (^RNNTransitionRejectionBlock)(NSString *_Nonnull code, NSString *_

- (instancetype _Nonnull)initWithBridge:(RCTBridge *_Nonnull)bridge
eventHandler:(RNNModalManagerEventHandler *_Nonnull)eventHandler;
- (void)connectModalHostViewManager:(RCTModalHostViewManager *_Nonnull)modalHostViewManager;

- (void)showModal:(UIViewController *_Nonnull)viewController
animated:(BOOL)animated
Expand All @@ -25,4 +23,6 @@ typedef void (^RNNTransitionRejectionBlock)(NSString *_Nonnull code, NSString *_

- (void)reset;

- (UIViewController *)topPresentedVC;

@end
26 changes: 0 additions & 26 deletions lib/ios/RNNModalManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -32,32 +32,6 @@ - (instancetype)initWithBridge:(RCTBridge *)bridge
return self;
}

- (void)connectModalHostViewManager:(RCTModalHostViewManager *)modalHostViewManager {
modalHostViewManager.presentationBlock =
^(UIViewController *reactViewController, UIViewController *viewController, BOOL animated,
dispatch_block_t completionBlock) {
if (reactViewController.presentedViewController != viewController) {
[self showModal:viewController
animated:animated
completion:^(NSString *_Nonnull componentId) {
if (completionBlock)
completionBlock();
}];
}
};

modalHostViewManager.dismissalBlock =
^(UIViewController *reactViewController, UIViewController *viewController, BOOL animated,
dispatch_block_t completionBlock) {
[self dismissModal:viewController
animated:animated
completion:^{
if (completionBlock)
completionBlock();
}];
};
}

- (void)showModal:(UIViewController<RNNLayoutProtocol> *)viewController
animated:(BOOL)animated
completion:(RNNTransitionWithComponentIdCompletionBlock)completion {
Expand Down
8 changes: 8 additions & 0 deletions lib/ios/ReactNativeNavigation.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@
4534E72620CB6724009F8185 /* RNNLargeTitleOptions.m in Sources */ = {isa = PBXBuildFile; fileRef = 4534E72420CB6724009F8185 /* RNNLargeTitleOptions.m */; };
500623A525B7003A0086AB39 /* RNNShadowOptions.h in Headers */ = {isa = PBXBuildFile; fileRef = 500623A325B7003A0086AB39 /* RNNShadowOptions.h */; };
500623A625B7003A0086AB39 /* RNNShadowOptions.m in Sources */ = {isa = PBXBuildFile; fileRef = 500623A425B7003A0086AB39 /* RNNShadowOptions.m */; };
5006E12C27974B8900D106A6 /* RNNModalHostViewManagerHandler.h in Headers */ = {isa = PBXBuildFile; fileRef = 5006E12A27974B8900D106A6 /* RNNModalHostViewManagerHandler.h */; };
5006E12D27974B8900D106A6 /* RNNModalHostViewManagerHandler.m in Sources */ = {isa = PBXBuildFile; fileRef = 5006E12B27974B8900D106A6 /* RNNModalHostViewManagerHandler.m */; };
5008641223856A2D00A55BE9 /* UITabBar+utils.m in Sources */ = {isa = PBXBuildFile; fileRef = 5008641023856A2C00A55BE9 /* UITabBar+utils.m */; };
501214C9217741A000435148 /* libOCMock.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 501214C8217741A000435148 /* libOCMock.a */; };
501223D72173590F000F5F98 /* RNNStackPresenter.h in Headers */ = {isa = PBXBuildFile; fileRef = 501223D52173590F000F5F98 /* RNNStackPresenter.h */; };
Expand Down Expand Up @@ -580,6 +582,8 @@
4534E72420CB6724009F8185 /* RNNLargeTitleOptions.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = RNNLargeTitleOptions.m; sourceTree = "<group>"; };
500623A325B7003A0086AB39 /* RNNShadowOptions.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = RNNShadowOptions.h; sourceTree = "<group>"; };
500623A425B7003A0086AB39 /* RNNShadowOptions.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = RNNShadowOptions.m; sourceTree = "<group>"; };
5006E12A27974B8900D106A6 /* RNNModalHostViewManagerHandler.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = RNNModalHostViewManagerHandler.h; sourceTree = "<group>"; };
5006E12B27974B8900D106A6 /* RNNModalHostViewManagerHandler.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = RNNModalHostViewManagerHandler.m; sourceTree = "<group>"; };
5008641023856A2C00A55BE9 /* UITabBar+utils.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = "UITabBar+utils.m"; sourceTree = "<group>"; };
5008641123856A2D00A55BE9 /* UITabBar+utils.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "UITabBar+utils.h"; sourceTree = "<group>"; };
501214C8217741A000435148 /* libOCMock.a */ = {isa = PBXFileReference; lastKnownFileType = archive.ar; path = libOCMock.a; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1092,6 +1096,8 @@
390AD476200F499D00A8250D /* RNNSwizzles.m */,
506A2B1220973DFD00F43A95 /* RNNErrorHandler.h */,
506A2B1320973DFD00F43A95 /* RNNErrorHandler.m */,
5006E12A27974B8900D106A6 /* RNNModalHostViewManagerHandler.h */,
5006E12B27974B8900D106A6 /* RNNModalHostViewManagerHandler.m */,
50644A1E20E11A720026709C /* Constants.h */,
50644A1F20E11A720026709C /* Constants.m */,
50706E6B20CE7CA5003345C3 /* UIImage+utils.h */,
Expand Down Expand Up @@ -1818,6 +1824,7 @@
files = (
506BF6622600AE7600A22755 /* BoundsTransition.h in Headers */,
91CB34C9250ED50C000C132B /* RNNSearchBarOptions.h in Headers */,
5006E12C27974B8900D106A6 /* RNNModalHostViewManagerHandler.h in Headers */,
5060DE73219DAD7E00D0C052 /* ReactNativeNavigation.h in Headers */,
506BF7CE26067B0500A22755 /* AnimatedUIImageView.h in Headers */,
5022EDBD2405237100852BA6 /* BottomTabPresenterCreator.h in Headers */,
Expand Down Expand Up @@ -2337,6 +2344,7 @@
5017D9EF239D2FAF00B74047 /* BottomTabsAfterInitialTabAttacher.m in Sources */,
5008641223856A2D00A55BE9 /* UITabBar+utils.m in Sources */,
9FDA2ABE24F2A42C005678CC /* RCTConvert+UIFontWeight.m in Sources */,
5006E12D27974B8900D106A6 /* RNNModalHostViewManagerHandler.m in Sources */,
9FDA2AC024F2A43B005678CC /* RCTConvert+SideMenuOpenGestureMode.m in Sources */,
50BCB27223F1650800D6C8E5 /* SharedElementTransition.m in Sources */,
E5F6C3A822DB4D0F0093C2CE /* UIView+Utils.m in Sources */,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
#import "RNNModalHostViewManagerHandler.h"
#import <OCMock/OCMock.h>
#import <XCTest/XCTest.h>

@interface RNNModalHostViewManagerHandlerTest : XCTestCase
@end

@implementation RNNModalHostViewManagerHandlerTest {
RNNModalHostViewManagerHandler *_uut;
RNNModalManager *_modalManager;
RCTModalHostViewManager *_modalHostViewManager;
}

- (void)setUp {
_modalManager = [OCMockObject partialMockForObject:[[RNNModalManager alloc] init]];
_uut = [[RNNModalHostViewManagerHandler alloc] initWithModalManager:_modalManager];
_modalHostViewManager = [RCTModalHostViewManager new];
[_uut connectModalHostViewManager:_modalHostViewManager];
}

- (void)testPresentationBlock_shouldNotPresentModalTwice {
[[(id)_modalManager reject] showModal:OCMArg.any animated:NO completion:OCMArg.any];
_modalHostViewManager.presentationBlock(nil, _modalManager.topPresentedVC, NO, nil);
}

- (void)testPresentationBlock_shouldShowModal {
UIViewController *vc = UIViewController.new;
_modalHostViewManager.presentationBlock(nil, vc, NO, nil);
XCTAssertEqual(vc, _modalManager.topPresentedVC);
}

- (void)testPresentationBlock_shouldShowAndDismissModal {
XCTestExpectation *expectation = [self expectationWithDescription:@"Testing Async Method"];
UIViewController *vc = UIViewController.new;
_modalHostViewManager.presentationBlock(nil, vc, NO, nil);
XCTAssertEqual(vc, _modalManager.topPresentedVC);
_modalHostViewManager.dismissalBlock(nil, vc, NO, ^{
XCTAssertNotEqual(vc, self->_modalManager.topPresentedVC);
[expectation fulfill];
});
[self waitForExpectationsWithTimeout:5 handler:nil];
}

@end
1 change: 1 addition & 0 deletions playground/ios/NavigationTests/RNNModalManagerTest.m
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#import "RNNComponentViewController.h"
#import "RNNStackController.h"
#import <OCMock/OCMock.h>
#import <React/RCTModalHostViewManager.h>
#import <XCTest/XCTest.h>

@interface MockViewController : UIViewController
Expand Down
4 changes: 4 additions & 0 deletions playground/ios/playground.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
13B07FBC1A68108700A75B9A /* AppDelegate.m in Sources */ = {isa = PBXBuildFile; fileRef = 13B07FB01A68108700A75B9A /* AppDelegate.m */; };
13B07FBF1A68108700A75B9A /* Images.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 13B07FB51A68108700A75B9A /* Images.xcassets */; };
13B07FC11A68108700A75B9A /* main.m in Sources */ = {isa = PBXBuildFile; fileRef = 13B07FB71A68108700A75B9A /* main.m */; };
5006E16627974DEA00D106A6 /* RNNModalHostViewManagerHandlerTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 5006E16527974DEA00D106A6 /* RNNModalHostViewManagerHandlerTest.m */; };
5007B4312472CA390002AA4E /* RNNNativeViewController.m in Sources */ = {isa = PBXBuildFile; fileRef = 5007B4302472CA390002AA4E /* RNNNativeViewController.m */; };
5007B4342472CBD40002AA4E /* RNNCustomViewController.m in Sources */ = {isa = PBXBuildFile; fileRef = 5007B4322472CBD30002AA4E /* RNNCustomViewController.m */; };
5007B4352472D3C90002AA4E /* RNNNativeViewController.m in Sources */ = {isa = PBXBuildFile; fileRef = 5007B4302472CA390002AA4E /* RNNNativeViewController.m */; };
Expand Down Expand Up @@ -117,6 +118,7 @@
4259AF43A23D928FE78B4A3A /* Pods-NavigationTests.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-NavigationTests.debug.xcconfig"; path = "Target Support Files/Pods-NavigationTests/Pods-NavigationTests.debug.xcconfig"; sourceTree = "<group>"; };
4AE37ACF6BFBAB211EE8E7E9 /* Pods-NavigationIOS12Tests.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-NavigationIOS12Tests.release.xcconfig"; path = "Target Support Files/Pods-NavigationIOS12Tests/Pods-NavigationIOS12Tests.release.xcconfig"; sourceTree = "<group>"; };
4C14E49C47AA48BEDE90A218 /* Pods-SnapshotTests.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-SnapshotTests.debug.xcconfig"; path = "Target Support Files/Pods-SnapshotTests/Pods-SnapshotTests.debug.xcconfig"; sourceTree = "<group>"; };
5006E16527974DEA00D106A6 /* RNNModalHostViewManagerHandlerTest.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = RNNModalHostViewManagerHandlerTest.m; sourceTree = "<group>"; };
5007B42F2472CA390002AA4E /* RNNNativeViewController.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = RNNNativeViewController.h; sourceTree = "<group>"; };
5007B4302472CA390002AA4E /* RNNNativeViewController.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = RNNNativeViewController.m; sourceTree = "<group>"; };
5007B4322472CBD30002AA4E /* RNNCustomViewController.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = RNNCustomViewController.m; sourceTree = "<group>"; };
Expand Down Expand Up @@ -403,6 +405,7 @@
E58D26282385888B003F36BA /* RNNFontAttributesCreatorTest.m */,
E58D262A2385888B003F36BA /* RNNLayoutManagerTest.m */,
E58D26432385888C003F36BA /* RNNModalManagerTest.m */,
5006E16527974DEA00D106A6 /* RNNModalHostViewManagerHandlerTest.m */,
503F775B24D19D96005E596D /* RNNModalManagerEventHandlerTest.m */,
E58D263F2385888C003F36BA /* RNNStackControllerTest.m */,
E58D26322385888B003F36BA /* RNNNavigationOptionsTest.m */,
Expand Down Expand Up @@ -865,6 +868,7 @@
E58D265F2385888C003F36BA /* RNNBasePresenterTest.m in Sources */,
E58D26542385888C003F36BA /* RNNSideMenuControllerTest.m in Sources */,
E58D264A2385888C003F36BA /* BottomTabsControllerTest.m in Sources */,
5006E16627974DEA00D106A6 /* RNNModalHostViewManagerHandlerTest.m in Sources */,
E58D26472385888C003F36BA /* RNNRootViewControllerTest.m in Sources */,
50C085E8259141E200B0502C /* RNNButtonsPresenterTest.m in Sources */,
E58D26582385888C003F36BA /* UITabBarController+RNNOptionsTest.m in Sources */,
Expand Down

0 comments on commit b335451

Please sign in to comment.