From aca0f418baf4890b8be3c1214be70819e4b88a95 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Mon, 22 Mar 2021 18:45:03 -0700 Subject: [PATCH] Change State references to TextLayoutManager to a weak_ptr (ParagraphState) Summary: TextLayoutManager indirectly holds ShadowNodes, which could hold onto TextLayoutManager via a shared_ptr; so we probably have some reference cycles here. Changelog: [Internal] Reviewed By: sammy-SC Differential Revision: D27238439 fbshipit-source-id: b0b65cc451891e75bafddb7a08aa34ddf86d6a35 --- .../Text/RCTParagraphComponentView.mm | 28 ++++++++++--------- .../components/text/ParagraphShadowNode.cpp | 8 +----- .../renderer/components/text/ParagraphState.h | 6 ++-- 3 files changed, 20 insertions(+), 22 deletions(-) diff --git a/React/Fabric/Mounting/ComponentViews/Text/RCTParagraphComponentView.mm b/React/Fabric/Mounting/ComponentViews/Text/RCTParagraphComponentView.mm index 3425d844e51..bc0fa7b8417 100644 --- a/React/Fabric/Mounting/ComponentViews/Text/RCTParagraphComponentView.mm +++ b/React/Fabric/Mounting/ComponentViews/Text/RCTParagraphComponentView.mm @@ -106,8 +106,8 @@ using namespace facebook::react; return; } - auto textLayoutManager = _state->getData().layoutManager; - assert(textLayoutManager && "TextLayoutManager must not be `nullptr`."); + auto textLayoutManager = _state->getData().layoutManager.lock(); + // assert(textLayoutManager && "TextLayoutManager must not be `nullptr`."); if (!textLayoutManager) { return; @@ -152,14 +152,18 @@ using namespace facebook::react; auto &data = _state->getData(); if (![_accessibilityProvider isUpToDate:data.attributedString]) { - RCTTextLayoutManager *textLayoutManager = - (RCTTextLayoutManager *)unwrapManagedObject(data.layoutManager->getNativeTextLayoutManager()); - CGRect frame = RCTCGRectFromRect(_layoutMetrics.getContentFrame()); - _accessibilityProvider = [[RCTParagraphComponentAccessibilityProvider alloc] initWithString:data.attributedString - layoutManager:textLayoutManager - paragraphAttributes:data.paragraphAttributes - frame:frame - view:self]; + auto textLayoutManager = data.layoutManager.lock(); + if (textLayoutManager) { + RCTTextLayoutManager *nativeTextLayoutManager = + (RCTTextLayoutManager *)unwrapManagedObject(textLayoutManager->getNativeTextLayoutManager()); + CGRect frame = RCTCGRectFromRect(_layoutMetrics.getContentFrame()); + _accessibilityProvider = + [[RCTParagraphComponentAccessibilityProvider alloc] initWithString:data.attributedString + layoutManager:nativeTextLayoutManager + paragraphAttributes:data.paragraphAttributes + frame:frame + view:self]; + } } return _accessibilityProvider.accessibilityElements; @@ -178,9 +182,7 @@ using namespace facebook::react; return _eventEmitter; } - auto textLayoutManager = _state->getData().layoutManager; - - assert(textLayoutManager && "TextLayoutManager must not be `nullptr`."); + auto textLayoutManager = _state->getData().layoutManager.lock(); if (!textLayoutManager) { return _eventEmitter; diff --git a/ReactCommon/react/renderer/components/text/ParagraphShadowNode.cpp b/ReactCommon/react/renderer/components/text/ParagraphShadowNode.cpp index 0c485964273..4d78d91b412 100644 --- a/ReactCommon/react/renderer/components/text/ParagraphShadowNode.cpp +++ b/ReactCommon/react/renderer/components/text/ParagraphShadowNode.cpp @@ -102,13 +102,7 @@ void ParagraphShadowNode::updateStateIfNeeded(Content const &content) { auto &state = getStateData(); - react_native_assert(textLayoutManager_); - react_native_assert( - (!state.layoutManager || state.layoutManager == textLayoutManager_) && - "`StateData` refers to a different `TextLayoutManager`"); - - if (state.attributedString == content.attributedString && - state.layoutManager == textLayoutManager_) { + if (state.attributedString == content.attributedString) { return; } diff --git a/ReactCommon/react/renderer/components/text/ParagraphState.h b/ReactCommon/react/renderer/components/text/ParagraphState.h index bc72ac28c55..b31b8798d84 100644 --- a/ReactCommon/react/renderer/components/text/ParagraphState.h +++ b/ReactCommon/react/renderer/components/text/ParagraphState.h @@ -40,14 +40,16 @@ class ParagraphState final { * `TextLayoutManager` provides a connection to platform-specific * text rendering infrastructure which is capable to render the * `AttributedString`. + * This is not on every platform. This is not used on Android, but is + * used on the iOS mounting layer. */ - SharedTextLayoutManager layoutManager; + std::weak_ptr layoutManager; #ifdef ANDROID ParagraphState( AttributedString const &attributedString, ParagraphAttributes const ¶graphAttributes, - SharedTextLayoutManager const &layoutManager) + std::weak_ptr const &layoutManager) : attributedString(attributedString), paragraphAttributes(paragraphAttributes), layoutManager(layoutManager) {}