From a3b348eacbcc4d679cad44b8fd72873ef8689c8e Mon Sep 17 00:00:00 2001 From: Spencer Ahrens Date: Fri, 21 Dec 2018 17:57:32 -0800 Subject: [PATCH] clean up surface register / start Summary: `RCTSurfaceHostingProxyRootView` surfaces are still automatically started right after the initialization to match `RCTRootView` interface, but `RCTSurfaceHostingView` must be started explicitly now. Also fixed some internal stuff so start and register are clear and distinct. Background / initial motivation: One tricky bit - we render the template as part of init`ing the rootView, so we don't know what the surfaceId will be before hand to register the UITemplate. Two possible solutions: 1) Require start be called explicitly after initializing the rootView, and setup the context in between. 2) Do something like "setUITemplateConfigForNextSurface" before creating the rootView, and have some hook when the surfaceId is assigned that associates the surfaceId with that "next" UITemplate stuff before. (1) seems a lot cleaner, but it requires ever user of rootView to explicitly call start on it - how do you feel about that? Seems like we could also use that start call to decide if the initial render should be synchronous or not? start vs. startSync? Reviewed By: mdvacca Differential Revision: D13372914 fbshipit-source-id: 6db297870610e6c231f8a78c0dd74d584cb64910 --- .../SurfaceHostingView/RCTSurfaceHostingProxyRootView.mm | 1 + .../Surface/SurfaceHostingView/RCTSurfaceHostingView.mm | 1 - React/Fabric/RCTSurfacePresenter.h | 8 ++++++-- React/Fabric/RCTSurfacePresenter.mm | 4 ++++ React/Fabric/Surface/RCTFabricSurface.mm | 5 ++++- 5 files changed, 15 insertions(+), 4 deletions(-) diff --git a/React/Base/Surface/SurfaceHostingView/RCTSurfaceHostingProxyRootView.mm b/React/Base/Surface/SurfaceHostingView/RCTSurfaceHostingProxyRootView.mm index 3f72b057d7d..539e236bbc2 100644 --- a/React/Base/Surface/SurfaceHostingView/RCTSurfaceHostingProxyRootView.mm +++ b/React/Base/Surface/SurfaceHostingView/RCTSurfaceHostingProxyRootView.mm @@ -69,6 +69,7 @@ static RCTRootViewSizeFlexibility convertToRootViewSizeFlexibility(RCTSurfaceSiz if (self = [super initWithBridge:bridge moduleName:moduleName initialProperties:initialProperties sizeMeasureMode:sizeMeasureMode]) { self.backgroundColor = [UIColor whiteColor]; + [super.surface start]; } RCT_PROFILE_END_EVENT(RCTProfileTagAlways, @""); diff --git a/React/Base/Surface/SurfaceHostingView/RCTSurfaceHostingView.mm b/React/Base/Surface/SurfaceHostingView/RCTSurfaceHostingView.mm index 9f55e635101..7e4284abe38 100644 --- a/React/Base/Surface/SurfaceHostingView/RCTSurfaceHostingView.mm +++ b/React/Base/Surface/SurfaceHostingView/RCTSurfaceHostingView.mm @@ -51,7 +51,6 @@ RCT_NOT_IMPLEMENTED(- (nullable instancetype)initWithCoder:(NSCoder *)coder) { if (self = [super initWithFrame:CGRectZero]) { _surface = surface; - [_surface start]; _sizeMeasureMode = sizeMeasureMode; _surface.delegate = self; diff --git a/React/Fabric/RCTSurfacePresenter.h b/React/Fabric/RCTSurfacePresenter.h index 0e01623b752..c4f1f98fbfa 100644 --- a/React/Fabric/RCTSurfacePresenter.h +++ b/React/Fabric/RCTSurfacePresenter.h @@ -38,10 +38,14 @@ NS_ASSUME_NONNULL_BEGIN @interface RCTSurfacePresenter (Surface) /** - * Surface uses those methods to register itself in the Presenter. - * Registering initiates running, rendering and mounting processes. + * Surface uses these methods to register itself in the Presenter. */ - (void)registerSurface:(RCTFabricSurface *)surface; +/** + * Starting initiates running, rendering and mounting processes. + * Should be called after registerSurface and any other surface-specific setup is done + */ +- (void)startSurface:(RCTFabricSurface *)surface; - (void)unregisterSurface:(RCTFabricSurface *)surface; - (void)setProps:(NSDictionary *)props surface:(RCTFabricSurface *)surface; diff --git a/React/Fabric/RCTSurfacePresenter.mm b/React/Fabric/RCTSurfacePresenter.mm index d7f56e3fb6a..9685f6c7821 100644 --- a/React/Fabric/RCTSurfacePresenter.mm +++ b/React/Fabric/RCTSurfacePresenter.mm @@ -99,6 +99,10 @@ using namespace facebook::react; - (void)registerSurface:(RCTFabricSurface *)surface { [_surfaceRegistry registerSurface:surface]; +} + +- (void)startSurface:(RCTFabricSurface *)surface +{ [self _startSurface:surface]; } diff --git a/React/Fabric/Surface/RCTFabricSurface.mm b/React/Fabric/Surface/RCTFabricSurface.mm index ddc18756828..f5f55cf8ed6 100644 --- a/React/Fabric/Surface/RCTFabricSurface.mm +++ b/React/Fabric/Surface/RCTFabricSurface.mm @@ -55,6 +55,8 @@ _touchHandler = [RCTSurfaceTouchHandler new]; _stage = RCTSurfaceStageSurfaceDidInitialize; + + [_surfacePresenter registerSurface:self]; } return self; @@ -66,7 +68,8 @@ return NO; } - [_surfacePresenter registerSurface:self]; + [_surfacePresenter startSurface:self]; + return YES; }