Fabric: RCTWeakEventEmitterWrapper, NSAttributedString and co.

Summary: Previously, we stored a pointer to ShadowNode inside NSAttributedString's attributes to make possible retrieving an EventEmitter associated with some text fragment. That worked fine besides only one caveat: the internal implementation of NSAttributedString is quite strange and that causes a memory leak. Because of some reason, NSAttributedString does not release stored attributes after own destruction (maybe OS uses some kind of caching). So, now, instead of storing a strong pointer to ShadowNode inside NSAttributedString, we store a weak pointer to EventEmitter. Storing a weak pointer is okay because a desired lifetime of EventEmitter is guaranteed by LocalData stored inside a View. Storing a weak EventEmitter instead of weak ShadowNode will also help us with migration to ShadowView (we cannot store ShadowView weakly because it's a stack allocated object). Reviewed By: sahrens Differential Revision: D13196886 fbshipit-source-id: f8714e4b3709765629d6456edf0c635bf5f7c53b
bumplzz69 · Nov 26, 2018 · e61a14e · e61a14e
1 parent 0ec1d21
commit e61a14e
Show file tree

Hide file tree

Showing 5 changed files with 48 additions and 32 deletions.
diff --git a/React/Fabric/Mounting/ComponentViews/Text/RCTParagraphComponentView.mm b/React/Fabric/Mounting/ComponentViews/Text/RCTParagraphComponentView.mm
@@ -114,18 +114,18 @@ - (SharedTouchEventEmitter)touchEventEmitterAtPoint:(CGPoint)point
   RCTTextLayoutManager *nativeTextLayoutManager = (__bridge RCTTextLayoutManager *)textLayoutManager->getNativeTextLayoutManager();
   CGRect frame = RCTCGRectFromRect(_layoutMetrics.getContentFrame());
 
-  SharedShadowNode textShadowNode = [nativeTextLayoutManager getParentShadowNodeWithAttributeString:_paragraphLocalData->getAttributedString()
-                                                                                paragraphAttributes:_paragraphAttributes
-                                                                                              frame:frame
-                                                                                            atPoint:point];
+  SharedEventEmitter eventEmitter =
+    [nativeTextLayoutManager getEventEmitterWithAttributeString:_paragraphLocalData->getAttributedString()
+                                            paragraphAttributes:_paragraphAttributes
+                                                          frame:frame
+                                                        atPoint:point];
 
-  if (!textShadowNode) {
+  if (!eventEmitter) {
     return _eventEmitter;
   }
 
-  SharedEventEmitter eventEmitter = textShadowNode->getEventEmitter();
   assert(std::dynamic_pointer_cast<const TouchEventEmitter>(eventEmitter));
   return std::static_pointer_cast<const TouchEventEmitter>(eventEmitter);
 }
 
- @end
+@end
diff --git a/ReactCommon/fabric/textlayoutmanager/platform/ios/RCTAttributedTextUtils.h b/ReactCommon/fabric/textlayoutmanager/platform/ios/RCTAttributedTextUtils.h
@@ -14,16 +14,16 @@ NS_ASSUME_NONNULL_BEGIN
 
 NSString *const RCTAttributedStringIsHighlightedAttributeName =
     @"IsHighlighted";
-NSString *const RCTAttributedStringParentShadowNode = @"ParentShadowNode";
+NSString *const RCTAttributedStringEventEmitterKey = @"EventEmitter";
 
 /**
  * Constructs ready-to-render `NSAttributedString` by given `AttributedString`.
  */
 NSAttributedString *RCTNSAttributedStringFromAttributedString(
     const facebook::react::AttributedString &attributedString);
 
-@interface RCTSharedShadowNodeWrapper : NSObject
-@property(nonatomic, assign) facebook::react::SharedShadowNode node;
+@interface RCTWeakEventEmitterWrapper : NSObject
+@property(nonatomic, assign) facebook::react::SharedEventEmitter eventEmitter;
 @end
 
 NS_ASSUME_NONNULL_END
diff --git a/ReactCommon/fabric/textlayoutmanager/platform/ios/RCTAttributedTextUtils.mm b/ReactCommon/fabric/textlayoutmanager/platform/ios/RCTAttributedTextUtils.mm
@@ -12,7 +12,24 @@
 #include <react/textlayoutmanager/RCTFontUtils.h>
 #include <react/textlayoutmanager/RCTTextPrimitivesConversions.h>
 
-@implementation RCTSharedShadowNodeWrapper
+using namespace facebook::react;
+
+@implementation RCTWeakEventEmitterWrapper {
+  std::weak_ptr<const EventEmitter> _weakEventEmitter;
+}
+
+- (void)setEventEmitter:(SharedEventEmitter)eventEmitter {
+  _weakEventEmitter = eventEmitter;
+}
+
+- (SharedEventEmitter)eventEmitter {
+  return _weakEventEmitter.lock();
+}
+
+- (void)dealloc {
+  _weakEventEmitter.reset();
+}
+
 @end
 
 inline static UIFont *RCTEffectiveFontFromTextAttributes(
@@ -243,12 +260,13 @@ inline static CGFloat RCTEffectiveFontSizeMultiplierFromTextAttributes(
             initWithAttributedString:nsAttributedStringFragment];
 
     if (fragment.parentShadowNode) {
-      RCTSharedShadowNodeWrapper *parentShadowNode =
-          [RCTSharedShadowNodeWrapper new];
-      parentShadowNode.node = fragment.parentShadowNode;
+      RCTWeakEventEmitterWrapper *eventEmitterWrapper =
+          [RCTWeakEventEmitterWrapper new];
+      eventEmitterWrapper.eventEmitter =
+          fragment.parentShadowNode->getEventEmitter();
 
       NSDictionary<NSAttributedStringKey, id> *additionalTextAttributes =
-          @{RCTAttributedStringParentShadowNode : parentShadowNode};
+          @{RCTAttributedStringEventEmitterKey : eventEmitterWrapper};
 
       [nsMutableAttributedStringFragment
           addAttributes:additionalTextAttributes

diff --git a/ReactCommon/fabric/textlayoutmanager/platform/ios/RCTTextLayoutManager.h b/ReactCommon/fabric/textlayoutmanager/platform/ios/RCTTextLayoutManager.h
@@ -32,14 +32,13 @@ NS_ASSUME_NONNULL_BEGIN
              (facebook::react::ParagraphAttributes)paragraphAttributes
                        frame:(CGRect)frame;
 
-- (facebook::react::SharedShadowNode)
-    getParentShadowNodeWithAttributeString:
+- (facebook::react::SharedEventEmitter)
+    getEventEmitterWithAttributeString:
         (facebook::react::AttributedString)attributedString
-                       paragraphAttributes:
-                           (facebook::react::ParagraphAttributes)
-                               paragraphAttributes
-                                     frame:(CGRect)frame
-                                   atPoint:(CGPoint)point;
+                   paragraphAttributes:
+                       (facebook::react::ParagraphAttributes)paragraphAttributes
+                                 frame:(CGRect)frame
+                               atPoint:(CGPoint)point;
 
 @end
 

diff --git a/ReactCommon/fabric/textlayoutmanager/platform/ios/RCTTextLayoutManager.mm b/ReactCommon/fabric/textlayoutmanager/platform/ios/RCTTextLayoutManager.mm
@@ -106,12 +106,11 @@ - (void)drawAttributedString:(AttributedString)attributedString
   return textStorage;
 }
 
-- (SharedShadowNode)
-    getParentShadowNodeWithAttributeString:(AttributedString)attributedString
-                       paragraphAttributes:
-                           (ParagraphAttributes)paragraphAttributes
-                                     frame:(CGRect)frame
-                                   atPoint:(CGPoint)point {
+- (SharedEventEmitter)
+    getEventEmitterWithAttributeString:(AttributedString)attributedString
+                   paragraphAttributes:(ParagraphAttributes)paragraphAttributes
+                                 frame:(CGRect)frame
+                               atPoint:(CGPoint)point {
   NSTextStorage *textStorage =
       [self _textStorageAndLayoutManagerWithAttributesString:
                 RCTNSAttributedStringFromAttributedString(attributedString)
@@ -130,12 +129,12 @@ - (void)drawAttributedString:(AttributedString)attributedString
   // after (fraction == 1.0) the last character, then the attribute is valid.
   if (textStorage.length > 0 && (fraction > 0 || characterIndex > 0) &&
       (fraction < 1 || characterIndex < textStorage.length - 1)) {
-    RCTSharedShadowNodeWrapper *parentShadowNode =
-        (RCTSharedShadowNodeWrapper *)[textStorage
-                 attribute:RCTAttributedStringParentShadowNode
+    RCTWeakEventEmitterWrapper *eventEmitterWrapper =
+        (RCTWeakEventEmitterWrapper *)[textStorage
+                 attribute:RCTAttributedStringEventEmitterKey
                    atIndex:characterIndex
             effectiveRange:NULL];
-    return parentShadowNode.node;
+    return eventEmitterWrapper.eventEmitter;
   }
 
   return nil;