From 24ffd8f6fb1ee95586276ef84ef8617b2ac9098c Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Wed, 26 Sep 2018 10:01:40 -0700 Subject: [PATCH] Fabric: Polishing RCTFabricSurface Summary: All integration with Bridge was removed from RCTFabricSurface, now it's Surface's responsibility to start and stop JS app and register the ShadowTree in the Scheduler. Reviewed By: mdvacca Differential Revision: D9931317 fbshipit-source-id: 55a682f0afb1c542a904e1a8570029e4690967cc --- React/Base/Surface/RCTSurfaceStage.h | 10 ++ React/Fabric/Surface/RCTFabricSurface.h | 18 +++- React/Fabric/Surface/RCTFabricSurface.mm | 122 +++++++---------------- 3 files changed, 57 insertions(+), 93 deletions(-) diff --git a/React/Base/Surface/RCTSurfaceStage.h b/React/Base/Surface/RCTSurfaceStage.h index 4b06daf0c..b79d6b3cb 100644 --- a/React/Base/Surface/RCTSurfaceStage.h +++ b/React/Base/Surface/RCTSurfaceStage.h @@ -21,6 +21,16 @@ typedef NS_OPTIONS(NSInteger, RCTSurfaceStage) { RCTSurfaceStageSurfaceDidInitialLayout = 1 << 5, // UIManager completed the first layout pass RCTSurfaceStageSurfaceDidInitialMounting = 1 << 6, // UIManager completed the first mounting pass RCTSurfaceStageSurfaceDidStop = 1 << 7, // Surface stopped + + // Most of the previously existed stages make no sense in the new architecture; + // now Surface exposes only three simple stages: + // + // Surface object was constructed and still valid. + RCTSurfaceStageInitialized = RCTSurfaceStageSurfaceDidInitialize, + // All off-main-thread work is done; we are ready to mount the UI. + RCTSurfaceStagePrepared = RCTSurfaceStageBridgeDidLoad | RCTSurfaceStageModuleDidLoad | RCTSurfaceStageSurfaceDidRun | RCTSurfaceStageSurfaceDidInitialRendering | RCTSurfaceStageSurfaceDidInitialLayout, + // All main-thread work is done, the UI was mounted. + RCTSurfaceStageMounted = RCTSurfaceStageSurfaceDidInitialMounting, }; /** diff --git a/React/Fabric/Surface/RCTFabricSurface.h b/React/Fabric/Surface/RCTFabricSurface.h index 5e2f245e8..2deeae736 100644 --- a/React/Fabric/Surface/RCTFabricSurface.h +++ b/React/Fabric/Surface/RCTFabricSurface.h @@ -43,10 +43,6 @@ NS_ASSUME_NONNULL_BEGIN @property (atomic, copy, readwrite) NSDictionary *properties; -- (instancetype)initWithBridge:(RCTBridge *)bridge - moduleName:(NSString *)moduleName - initialProperties:(NSDictionary *)initialProperties; - - (instancetype)initWithSurfacePresenter:(RCTSurfacePresenter *)surfacePresenter moduleName:(NSString *)moduleName initialProperties:(NSDictionary *)initialProperties; @@ -120,12 +116,24 @@ NS_ASSUME_NONNULL_BEGIN @interface RCTFabricSurface (Internal) -- (void)_setStage:(RCTSurfaceStage)stage; +/** + * Sets and clears given stage flags (bitmask). + * Returns `YES` if the actual state was changed. + */ +- (BOOL)_setStage:(RCTSurfaceStage)stage; +- (BOOL)_unsetStage:(RCTSurfaceStage)stage; @end @interface RCTFabricSurface (Deprecated) +/** + * Deprecated. Use `initWithSurfacePresenter:moduleName:initialProperties` instead. + */ +- (instancetype)initWithBridge:(RCTBridge *)bridge + moduleName:(NSString *)moduleName + initialProperties:(NSDictionary *)initialProperties; + /** * Deprecated. Use `rootTag` instead. */ diff --git a/React/Fabric/Surface/RCTFabricSurface.mm b/React/Fabric/Surface/RCTFabricSurface.mm index 0bfd7d633..597c2e790 100644 --- a/React/Fabric/Surface/RCTFabricSurface.mm +++ b/React/Fabric/Surface/RCTFabricSurface.mm @@ -7,22 +7,18 @@ #import "RCTFabricSurface.h" -#import - #import -#import #import -#import #import #import #import +#import #import #import #import #import "RCTSurfacePresenter.h" -#import "RCTMountingManager.h" @implementation RCTFabricSurface { // Immutable @@ -42,19 +38,6 @@ RCTSurfaceTouchHandler *_Nullable _touchHandler; } -- (instancetype)initWithBridge:(RCTBridge *)bridge - moduleName:(NSString *)moduleName - initialProperties:(NSDictionary *)initialProperties -{ - RCTAssert(bridge.valid, @"Valid bridge is required to instanciate `RCTSurface`."); - - self = [self initWithSurfacePresenter:bridge.surfacePresenter - moduleName:moduleName - initialProperties:initialProperties]; - - return self; -} - - (instancetype)initWithSurfacePresenter:(RCTSurfacePresenter *)surfacePresenter moduleName:(NSString *)moduleName initialProperties:(NSDictionary *)initialProperties @@ -72,19 +55,7 @@ _touchHandler = [RCTSurfaceTouchHandler new]; - [self _run]; - - // TODO: This will be moved to RCTSurfacePresenter. - RCTBridge *bridge = surfacePresenter.bridge_DO_NOT_USE; - - [[NSNotificationCenter defaultCenter] addObserver:self - selector:@selector(handleJavaScriptWillStartLoadingNotification:) - name:RCTJavaScriptWillStartLoadingNotification - object:bridge]; - [[NSNotificationCenter defaultCenter] addObserver:self - selector:@selector(handleJavaScriptDidLoadNotification:) - name:RCTJavaScriptDidLoadNotification - object:bridge]; + [_surfacePresenter registerSurface:self]; } return self; @@ -92,9 +63,7 @@ - (void)dealloc { - [[NSNotificationCenter defaultCenter] removeObserver:self]; - - [self _stop]; + [_surfacePresenter unregisterSurface:self]; } #pragma mark - Immutable Properties (no need to enforce synchonization) @@ -104,11 +73,6 @@ return _moduleName; } -- (NSNumber *)rootViewTag -{ - return @(_rootTag); -} - #pragma mark - Main-Threaded Routines - (RCTSurfaceView *)view @@ -131,21 +95,37 @@ return _stage; } -- (void)_setStage:(RCTSurfaceStage)stage +- (BOOL)_setStage:(RCTSurfaceStage)stage +{ + return [self _setStage:stage setOrUnset:YES]; +} + +- (BOOL)_unsetStage:(RCTSurfaceStage)stage +{ + return [self _setStage:stage setOrUnset:NO]; +} + +- (BOOL)_setStage:(RCTSurfaceStage)stage setOrUnset:(BOOL)setOrUnset { RCTSurfaceStage updatedStage; { std::lock_guard lock(_mutex); - if (_stage & stage) { - return; + if (setOrUnset) { + updatedStage = (RCTSurfaceStage)(_stage | stage); + } else { + updatedStage = (RCTSurfaceStage)(_stage & ~stage); + } + + if (updatedStage == _stage) { + return NO; } - updatedStage = (RCTSurfaceStage)(_stage | stage); _stage = updatedStage; } [self _propagateStageChange:updatedStage]; + return YES; } - (void)_propagateStageChange:(RCTSurfaceStage)stage @@ -182,21 +162,8 @@ _properties = [properties copy]; } - [self _run]; -} - -#pragma mark - Running - -- (void)_run -{ - [_surfacePresenter registerSurface:self]; - [self _setStage:RCTSurfaceStageSurfaceDidRun]; -} - -- (void)_stop -{ - [_surfacePresenter unregisterSurface:self]; - [self _setStage:RCTSurfaceStageSurfaceDidStop]; + // TODO: Implement this in RCTSurfacePresenter. + // [_surfacePresenter setProps:properties surface:self]; } #pragma mark - Layout @@ -281,41 +248,20 @@ return NO; } -#pragma mark - Bridge events +#pragma mark - Deprecated -- (void)handleJavaScriptWillStartLoadingNotification:(NSNotification *)notification +- (instancetype)initWithBridge:(RCTBridge *)bridge + moduleName:(NSString *)moduleName + initialProperties:(NSDictionary *)initialProperties { - // 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]; + return [self initWithSurfacePresenter:bridge.surfacePresenter + moduleName:moduleName + initialProperties:initialProperties]; } -- (void)handleJavaScriptDidLoadNotification:(NSNotification *)notification +- (NSNumber *)rootViewTag { - // TODO: Move the bridge lifecycle handling up to the RCTSurfacePresenter. - - // Note: this covers both JS reloads and initial load after the bridge starts. - // When it's not a reload, surface should already be running since we run it immediately in the initializer, so do - // nothing. - // When it's a reload, we rely on the `RCTJavaScriptWillStartLoadingNotification` notification to reset the stage, - // then we need to run the surface and update its size. - if (!RCTSurfaceStageIsRunning(_stage)) { - [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]; - } + return @(_rootTag); } @end