Implementation of JS reload without crashing

Summary:
On JS reload the FabricUIManager and EventDispatcher didn't get release due to a retain cycle. This breaks the cycle.

In addition, force release the Scheduler on reload so that the stale classes get cleaned up properly, avoiding crashes. Also the surface now remounts the content correctly

Reviewed By: shergin

Differential Revision: D8414916

fbshipit-source-id: 4b14031f29b3bc9987d7aa765dc0d930a7de2b1e

React/Fabric/RCTSurfacePresenter.mm

@@ -47,6 +47,15 @@ using namespace facebook::react;
     _surfaceRegistry = [[RCTSurfaceRegistry alloc] init];
     _mountingManager = [[RCTMountingManager alloc] init];
     _mountingManager.delegate = self;
+
+    [[NSNotificationCenter defaultCenter] addObserver:self
+                                             selector:@selector(handleBridgeWillReloadNotification:)
+                                                 name:RCTBridgeWillReloadNotification
+                                               object:_bridge];
+    [[NSNotificationCenter defaultCenter] addObserver:self
+                                             selector:@selector(handleJavaScriptDidLoadNotification:)
+                                                 name:RCTJavaScriptDidLoadNotification
+                                               object:_bridge];
   }
 
   return self;
@@ -153,6 +162,25 @@ using namespace facebook::react;
   surface.view.rootView = (RCTSurfaceRootView *)rootComponentView;
 }
 
+#pragma mark - Bridge events
+
+- (void)handleBridgeWillReloadNotification:(NSNotification *)notification
+{
+  // TODO: Define a lifecycle contract for the pieces involved here including the scheduler, mounting manager, and
+  // the surface registry. For now simply recreate the scheduler on reload.
+  // The goal is to deallocate the Scheduler and its underlying references before the JS runtime is destroyed.
+  _scheduler = [[RCTScheduler alloc] init];
+  _scheduler.delegate = self;
+}
+
+- (void)handleJavaScriptDidLoadNotification:(NSNotification *)notification
+{
+  RCTBridge *bridge = notification.userInfo[@"bridge"];
+  if (bridge != _batchedBridge) {
+    _batchedBridge = bridge;
+  }
+}
+
 @end
 
 @implementation RCTSurfacePresenter (Deprecated)

React/Fabric/Surface/RCTFabricSurface.mm

@@ -74,6 +74,15 @@
     _touchHandler = [RCTSurfaceTouchHandler new];
 
     [self _run];
+
+    [[NSNotificationCenter defaultCenter] addObserver:self
+                                             selector:@selector(handleJavaScriptWillStartLoadingNotification:)
+                                                 name:RCTJavaScriptWillStartLoadingNotification
+                                               object:_bridge];
+    [[NSNotificationCenter defaultCenter] addObserver:self
+                                             selector:@selector(handleJavaScriptDidLoadNotification:)
+                                                 name:RCTJavaScriptDidLoadNotification
+                                               object:_bridge];
   }
 
   return self;
@@ -268,4 +277,35 @@
   return NO;
 }
 
+#pragma mark - Bridge events
+
+- (void)handleJavaScriptWillStartLoadingNotification:(NSNotification *)notification
+{
+  // TODO: Move the bridge lifecycle handling up to the RCTSurfacePresenter.
+
+  RCTAssertMainQueue();
+
+  // Reset states because the bridge is reloading. This is similar to initialization phase.
+  _stage = RCTSurfaceStageSurfaceDidInitialize;
+  _view = nil;
+  _touchHandler = [RCTSurfaceTouchHandler new];
+  [self _setStage:RCTSurfaceStageBridgeDidLoad];
+}
+
+- (void)handleJavaScriptDidLoadNotification:(NSNotification *)notification
+{
+  // TODO: Move the bridge lifecycle handling up to the RCTSurfacePresenter.
+
+  // Note: this covers only JS reloads.
+  [self _setStage:RCTSurfaceStageModuleDidLoad];
+  [self _run];
+
+  // After a reload surfacePresenter needs to know the last min/max size for this surface, because the surface hosting
+  // view was already attached to the ViewController's view.
+  // TODO: Find a better automatic way.
+  [_surfacePresenter setMinimumSize:_minimumSize
+                        maximumSize:_maximumSize
+                            surface:self];
+}
+
 @end

ReactCommon/fabric/uimanager/Scheduler.cpp

@@ -35,6 +35,7 @@ Scheduler::Scheduler() {
 
 Scheduler::~Scheduler() {
   uiManager_->setDelegate(nullptr);
+  eventDispatcher_->setUIManager(nullptr);
 }
 
 void Scheduler::registerRootTag(Tag rootTag) {

ReactCommon/fabric/uimanager/SchedulerEventDispatcher.cpp

@@ -22,7 +22,7 @@ static std::string normalizeEventType(const std::string &type) {
   return prefixedType;
 }
 
-void SchedulerEventDispatcher::setUIManager(std::shared_ptr<const FabricUIManager> uiManager) {
+void SchedulerEventDispatcher::setUIManager(std::shared_ptr<const FabricUIManager> uiManager) const {
   uiManager_ = uiManager;
 }
 

ReactCommon/fabric/uimanager/SchedulerEventDispatcher.h

@@ -27,7 +27,7 @@ class SchedulerEventDispatcher final:
 
 public:
 
-  void setUIManager(std::shared_ptr<const FabricUIManager> uiManager);
+  void setUIManager(std::shared_ptr<const FabricUIManager> uiManager) const;
 
 #pragma mark - EventDispatcher
 
@@ -42,7 +42,8 @@ public:
 
 private:
 
-  std::shared_ptr<const FabricUIManager> uiManager_;
+  // TODO: consider using std::weak_ptr<> instead for better memory management.
+  mutable std::shared_ptr<const FabricUIManager> uiManager_;
 };
 
 } // namespace react