Pass SurfacePresenter directly instead of relying on bridge

Summary: `RCTPropsAnimatedNode` uses the bridge to access it's current surface presenter to perform animations. In bridgeless mode, the surface presenter is not owned by the bridge. Instead, pass the surface presenter through the ownership chain: `RCTNativeAnimated*Module` -> `RCTNativeAnimatedNodesManager` -> `RCTPropsAnimatedNode` `RCTSurfacePresenter` should not be strongly held by any of these animation internals. If it gets destroyed at a higher level, animations should not be completed. Changelog: [Internal] Differential Revision: D23272735 fbshipit-source-id: ce08ee3b59ac2ba70e31cebb7ba8e9f3a644c848
facebook · Aug 24, 2020 · 700960c · 700960c
1 parent e1d53ce
commit 700960c
Show file tree

Hide file tree

Showing 7 changed files with 35 additions and 19 deletions.
diff --git a/Libraries/NativeAnimation/Nodes/RCTPropsAnimatedNode.h b/Libraries/NativeAnimation/Nodes/RCTPropsAnimatedNode.h
@@ -7,14 +7,17 @@
 
 #import "RCTAnimatedNode.h"
 
+#import <React/RCTSurfacePresenterStub.h>
+
 @class RCTBridge;
 @class RCTViewPropertyMapper;
 
 @interface RCTPropsAnimatedNode : RCTAnimatedNode
 
 - (void)connectToView:(NSNumber *)viewTag
              viewName:(NSString *)viewName
-               bridge:(RCTBridge *)bridge;
+               bridge:(RCTBridge *)bridge
+     surfacePresenter:(id<RCTSurfacePresenterStub>)surfacePresenter;
 
 - (void)disconnectFromView:(NSNumber *)viewTag;
 

diff --git a/Libraries/NativeAnimation/Nodes/RCTPropsAnimatedNode.m b/Libraries/NativeAnimation/Nodes/RCTPropsAnimatedNode.m
@@ -5,24 +5,21 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-#import <React/RCTPropsAnimatedNode.h>
-
-#import <React/RCTLog.h>
-#import <React/RCTSurfacePresenterStub.h>
-#import <React/RCTUIManager.h>
+#import "RCTPropsAnimatedNode.h"
 
 #import <React/RCTAnimationUtils.h>
+#import <React/RCTLog.h>
 #import <React/RCTStyleAnimatedNode.h>
+#import <React/RCTUIManager.h>
 #import <React/RCTValueAnimatedNode.h>
 
-
-
 @implementation RCTPropsAnimatedNode
 {
   NSNumber *_connectedViewTag;
   NSNumber *_rootTag;
   NSString *_connectedViewName;
   __weak RCTBridge *_bridge;
+  __weak id<RCTSurfacePresenterStub> _surfacePresenter;
   NSMutableDictionary<NSString *, NSObject *> *_propsDictionary; // TODO: use RawProps or folly::dynamic directly
   BOOL _managedByFabric;
 }
@@ -44,8 +41,10 @@ - (BOOL)isManagedByFabric
 - (void)connectToView:(NSNumber *)viewTag
              viewName:(NSString *)viewName
                bridge:(RCTBridge *)bridge
+     surfacePresenter:(id<RCTSurfacePresenterStub> )surfacePresenter
 {
   _bridge = bridge;
+  _surfacePresenter = surfacePresenter;
   _connectedViewTag = viewTag;
   _connectedViewName = viewName;
   _managedByFabric = RCTUIManagerTypeForTagIsFabric(viewTag);
@@ -55,6 +54,7 @@ - (void)connectToView:(NSNumber *)viewTag
 - (void)disconnectFromView:(NSNumber *)viewTag
 {
   _bridge = nil;
+  _surfacePresenter = nil;
   _connectedViewTag = nil;
   _connectedViewName = nil;
   _managedByFabric = NO;
@@ -64,8 +64,13 @@ - (void)disconnectFromView:(NSNumber *)viewTag
 - (void)updateView
 {
   if (_managedByFabric) {
-    [_bridge.surfacePresenter synchronouslyUpdateViewOnUIThread:_connectedViewTag
-                                                          props:_propsDictionary];
+    if (_bridge.surfacePresenter) {
+      [_bridge.surfacePresenter synchronouslyUpdateViewOnUIThread:_connectedViewTag
+      props:_propsDictionary];
+    } else {
+      [_surfacePresenter synchronouslyUpdateViewOnUIThread:_connectedViewTag
+      props:_propsDictionary];
+    }
   } else {
     [_bridge.uiManager synchronouslyUpdateViewOnUIThread:_connectedViewTag
                                                 viewName:_connectedViewName

diff --git a/Libraries/NativeAnimation/RCTNativeAnimatedModule.mm b/Libraries/NativeAnimation/RCTNativeAnimatedModule.mm
@@ -62,7 +62,7 @@ - (dispatch_queue_t)methodQueue
 - (void)setBridge:(RCTBridge *)bridge
 {
   [super setBridge:bridge];
-  _nodesManager = [[RCTNativeAnimatedNodesManager alloc] initWithBridge:self.bridge];
+  _nodesManager = [[RCTNativeAnimatedNodesManager alloc] initWithBridge:self.bridge surfacePresenter:bridge.surfacePresenter];
   [bridge.eventDispatcher addDispatchObserver:self];
   [bridge.uiManager.observerCoordinator addObserver:self];
   [bridge.surfacePresenter addObserver:self];

diff --git a/Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.h b/Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.h
@@ -8,12 +8,14 @@
 #import <Foundation/Foundation.h>
 #import <React/RCTBridgeModule.h>
 #import <React/RCTUIManager.h>
+#import <React/RCTSurfacePresenterStub.h>
 
 @protocol RCTValueAnimatedNodeObserver;
 
 @interface RCTNativeAnimatedNodesManager : NSObject
 
-- (nonnull instancetype)initWithBridge:(nonnull RCTBridge *)bridge;
+- (nonnull instancetype)initWithBridge:(nonnull RCTBridge *)bridge
+                      surfacePresenter:(id<RCTSurfacePresenterStub>)surfacePresenter;
 
 - (void)updateAnimations;
 

diff --git a/Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.m b/Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.m
@@ -46,6 +46,7 @@
 @implementation RCTNativeAnimatedNodesManager
 {
   __weak RCTBridge *_bridge;
+  __weak id<RCTSurfacePresenterStub> _surfacePresenter;
   NSMutableDictionary<NSNumber *, RCTAnimatedNode *> *_animationNodes;
   // Mapping of a view tag and an event name to a list of event animation drivers. 99% of the time
   // there will be only one driver per mapping so all code code should be optimized around that.
@@ -54,10 +55,11 @@ @implementation RCTNativeAnimatedNodesManager
   CADisplayLink *_displayLink;
 }
 
-- (instancetype)initWithBridge:(nonnull RCTBridge *)bridge
+- (instancetype)initWithBridge:(nonnull RCTBridge *)bridge surfacePresenter:(id<RCTSurfacePresenterStub>)surfacePresenter;
 {
   if ((self = [super init])) {
     _bridge = bridge;
+    _surfacePresenter = surfacePresenter;
     _animationNodes = [NSMutableDictionary new];
     _eventDrivers = [NSMutableDictionary new];
     _activeAnimations = [NSMutableSet new];
@@ -148,7 +150,10 @@ - (void)connectAnimatedNodeToView:(nonnull NSNumber *)nodeTag
 {
   RCTAnimatedNode *node = _animationNodes[nodeTag];
   if ([node isKindOfClass:[RCTPropsAnimatedNode class]]) {
-    [(RCTPropsAnimatedNode *)node connectToView:viewTag viewName:viewName bridge:_bridge];
+    [(RCTPropsAnimatedNode *)node connectToView:viewTag
+                                       viewName:viewName
+                                         bridge:_bridge
+                               surfacePresenter:_surfacePresenter];
   }
   [node setNeedsUpdate];
 }

diff --git a/Libraries/NativeAnimation/RCTNativeAnimatedTurboModule.mm b/Libraries/NativeAnimation/RCTNativeAnimatedTurboModule.mm
@@ -20,7 +20,7 @@ @interface RCTNativeAnimatedTurboModule() <NativeAnimatedModuleSpec>
 @implementation RCTNativeAnimatedTurboModule
 {
   RCTNativeAnimatedNodesManager *_nodesManager;
-
+  __weak id<RCTSurfacePresenterStub> _surfacePresenter;
   // Operations called after views have been updated.
   NSMutableArray<AnimatedOperation> *_operations;
   // Operations called before views have been updated.
@@ -50,7 +50,7 @@ - (void)invalidate
   [_nodesManager stopAnimationLoop];
   [self.bridge.eventDispatcher removeDispatchObserver:self];
   [self.bridge.uiManager.observerCoordinator removeObserver:self];
-  [self.bridge.surfacePresenter removeObserver:self];
+  [_surfacePresenter removeObserver:self];
 }
 
 - (dispatch_queue_t)methodQueue
@@ -64,10 +64,11 @@ - (dispatch_queue_t)methodQueue
 - (void)setBridge:(RCTBridge *)bridge
 {
   [super setBridge:bridge];
-  _nodesManager = [[RCTNativeAnimatedNodesManager alloc] initWithBridge:self.bridge];
+  _surfacePresenter = bridge.surfacePresenter;
+  _nodesManager = [[RCTNativeAnimatedNodesManager alloc] initWithBridge:self.bridge surfacePresenter:_surfacePresenter];
   [bridge.eventDispatcher addDispatchObserver:self];
   [bridge.uiManager.observerCoordinator addObserver:self];
-  [bridge.surfacePresenter addObserver:self];
+  [_surfacePresenter addObserver:self];
 }
 
 #pragma mark -- API

diff --git a/packages/rn-tester/RNTesterUnitTests/RCTNativeAnimatedNodesManagerTests.m b/packages/rn-tester/RNTesterUnitTests/RCTNativeAnimatedNodesManagerTests.m
@@ -129,7 +129,7 @@ - (void)setUp
   RCTBridge *bridge = [OCMockObject niceMockForClass:[RCTBridge class]];
   _uiManager = [OCMockObject niceMockForClass:[RCTUIManager class]];
   OCMStub([bridge uiManager]).andReturn(_uiManager);
-  _nodesManager = [[RCTNativeAnimatedNodesManager alloc] initWithBridge:bridge];
+  _nodesManager = [[RCTNativeAnimatedNodesManager alloc] initWithBridge:bridge surfacePresenter:bridge.surfacePresenter];
   _displayLink = [RCTFakeDisplayLink new];
 }