From a426c8dc7779eb7566bae5f3e55874d3a75ba64c Mon Sep 17 00:00:00 2001
From: Valentin Shergin <shergin@fb.com>
Date: Sun, 8 Mar 2020 21:28:37 -0700
Subject: [PATCH] Fabric: Using `(un)wrapManagedObject` in TextLayoutManager

Summary:
Investigating a crash, I spend half of an hour staring at `__bridge`, `__bridge_retained`, `CFRelease` and etc trying to understand is there a bug or not. Even if I think there was no bug there, it should not be this way. We have a nice abstraction around that madness we should use to make the code obvious.

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: sammy-SC

Differential Revision: D20260917

fbshipit-source-id: 2b511ebf46a78950c4720e749099134aed1dd890
---
 .../ComponentViews/Text/RCTParagraphComponentView.mm |  6 ++++--
 .../platform/ios/TextLayoutManager.h                 |  5 ++---
 .../platform/ios/TextLayoutManager.mm                | 12 +++---------
 3 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/React/Fabric/Mounting/ComponentViews/Text/RCTParagraphComponentView.mm b/React/Fabric/Mounting/ComponentViews/Text/RCTParagraphComponentView.mm
index b666f176eb6..2b3cc1f6cf5 100644
--- a/React/Fabric/Mounting/ComponentViews/Text/RCTParagraphComponentView.mm
+++ b/React/Fabric/Mounting/ComponentViews/Text/RCTParagraphComponentView.mm
@@ -15,6 +15,8 @@
 #import <react/graphics/Geometry.h>
 #import <react/textlayoutmanager/RCTTextLayoutManager.h>
 #import <react/textlayoutmanager/TextLayoutManager.h>
+#import <react/utils/ManagedObjectWrapper.h>
+
 #import "RCTConversions.h"
 
 using namespace facebook::react;
@@ -88,7 +90,7 @@ using namespace facebook::react;
   }
 
   RCTTextLayoutManager *nativeTextLayoutManager =
-      (__bridge RCTTextLayoutManager *)textLayoutManager->getNativeTextLayoutManager();
+      (RCTTextLayoutManager *)unwrapManagedObject(textLayoutManager->getNativeTextLayoutManager());
 
   CGRect frame = RCTCGRectFromRect(_layoutMetrics.getContentFrame());
 
@@ -128,7 +130,7 @@ using namespace facebook::react;
   }
 
   RCTTextLayoutManager *nativeTextLayoutManager =
-      (__bridge RCTTextLayoutManager *)textLayoutManager->getNativeTextLayoutManager();
+      (RCTTextLayoutManager *)unwrapManagedObject(textLayoutManager->getNativeTextLayoutManager());
   CGRect frame = RCTCGRectFromRect(_layoutMetrics.getContentFrame());
 
   auto eventEmitter = [nativeTextLayoutManager getEventEmitterWithAttributeString:_state->getData().attributedString
diff --git a/ReactCommon/fabric/textlayoutmanager/platform/ios/TextLayoutManager.h b/ReactCommon/fabric/textlayoutmanager/platform/ios/TextLayoutManager.h
index 6abf45b979b..57478586edb 100644
--- a/ReactCommon/fabric/textlayoutmanager/platform/ios/TextLayoutManager.h
+++ b/ReactCommon/fabric/textlayoutmanager/platform/ios/TextLayoutManager.h
@@ -30,7 +30,6 @@ class TextLayoutManager {
   using Shared = std::shared_ptr<TextLayoutManager const>;
 
   TextLayoutManager(ContextContainer::Shared const &contextContainer);
-  ~TextLayoutManager();
 
   /*
    * Measures `attributedString` using native text rendering infrastructure.
@@ -44,10 +43,10 @@ class TextLayoutManager {
    * Returns an opaque pointer to platform-specific TextLayoutManager.
    * Is used on a native views layer to delegate text rendering to the manager.
    */
-  void *getNativeTextLayoutManager() const;
+  std::shared_ptr<void> getNativeTextLayoutManager() const;
 
  private:
-  void *self_;
+  std::shared_ptr<void> self_;
   TextMeasureCache measureCache_{};
 };
 
diff --git a/ReactCommon/fabric/textlayoutmanager/platform/ios/TextLayoutManager.mm b/ReactCommon/fabric/textlayoutmanager/platform/ios/TextLayoutManager.mm
index 345100b224e..6c28e51ff46 100644
--- a/ReactCommon/fabric/textlayoutmanager/platform/ios/TextLayoutManager.mm
+++ b/ReactCommon/fabric/textlayoutmanager/platform/ios/TextLayoutManager.mm
@@ -16,16 +16,10 @@ namespace react {
 
 TextLayoutManager::TextLayoutManager(ContextContainer::Shared const &contextContainer)
 {
-  self_ = (__bridge_retained void *)[RCTTextLayoutManager new];
+  self_ = wrapManagedObject([RCTTextLayoutManager new]);
 }
 
-TextLayoutManager::~TextLayoutManager()
-{
-  CFRelease(self_);
-  self_ = nullptr;
-}
-
-void *TextLayoutManager::getNativeTextLayoutManager() const
+std::shared_ptr<void> TextLayoutManager::getNativeTextLayoutManager() const
 {
   assert(self_ && "Stored NativeTextLayoutManager must not be null.");
   return self_;
@@ -36,7 +30,7 @@ Size TextLayoutManager::measure(
     ParagraphAttributes paragraphAttributes,
     LayoutConstraints layoutConstraints) const
 {
-  RCTTextLayoutManager *textLayoutManager = (__bridge RCTTextLayoutManager *)self_;
+  RCTTextLayoutManager *textLayoutManager = (RCTTextLayoutManager *)unwrapManagedObject(self_);
 
   auto size = Size{};