From 0689c0790edcc9cb8494e3f15300e2743556f442 Mon Sep 17 00:00:00 2001 From: Nick Lockwood Date: Tue, 26 May 2015 18:39:37 -0700 Subject: [PATCH] Fixed crash in RCTText due to NSTextContainer/NSLayoutManager being accessed concurrently from main and shadow queues --- Libraries/Text/RCTShadowText.h | 15 ++-- Libraries/Text/RCTShadowText.m | 124 +++++++++++++------------------- Libraries/Text/RCTText.h | 4 +- Libraries/Text/RCTText.m | 95 +++++------------------- Libraries/Text/RCTTextManager.m | 25 +++---- React/Modules/RCTUIManager.m | 19 ++--- 6 files changed, 88 insertions(+), 194 deletions(-) diff --git a/Libraries/Text/RCTShadowText.h b/Libraries/Text/RCTShadowText.h index 7a7b44cf82..c93f131842 100644 --- a/Libraries/Text/RCTShadowText.h +++ b/Libraries/Text/RCTShadowText.h @@ -14,8 +14,6 @@ extern NSString *const RCTReactTagAttributeName; @interface RCTShadowText : RCTShadowView -@property (nonatomic, assign) NSWritingDirection writingDirection; -@property (nonatomic, strong) UIColor *textBackgroundColor; @property (nonatomic, strong) UIColor *color; @property (nonatomic, copy) NSString *fontFamily; @property (nonatomic, assign) CGFloat fontSize; @@ -24,17 +22,12 @@ extern NSString *const RCTReactTagAttributeName; @property (nonatomic, assign) BOOL isHighlighted; @property (nonatomic, assign) CGFloat letterSpacing; @property (nonatomic, assign) CGFloat lineHeight; -@property (nonatomic, assign) NSUInteger maximumNumberOfLines; +@property (nonatomic, assign) NSUInteger numberOfLines; @property (nonatomic, assign) CGSize shadowOffset; @property (nonatomic, assign) NSTextAlignment textAlign; +@property (nonatomic, strong) UIColor *textBackgroundColor; +@property (nonatomic, assign) NSWritingDirection writingDirection; -// Not exposed to JS -@property (nonatomic, strong) UIFont *font; -@property (nonatomic, assign) NSLineBreakMode truncationMode; -@property (nonatomic, assign) CGFloat effectiveLetterSpacing; - -@property (nonatomic, copy, readonly) NSAttributedString *attributedString; -@property (nonatomic, strong, readonly) NSLayoutManager *layoutManager; -@property (nonatomic, strong, readonly) NSTextContainer *textContainer; +- (NSTextStorage *)buildTextStorageForWidth:(CGFloat)width; @end diff --git a/Libraries/Text/RCTShadowText.m b/Libraries/Text/RCTShadowText.m index b7e6f997ed..dd880340da 100644 --- a/Libraries/Text/RCTShadowText.m +++ b/Libraries/Text/RCTShadowText.m @@ -17,62 +17,61 @@ NSString *const RCTIsHighlightedAttributeName = @"IsHighlightedAttributeName"; NSString *const RCTReactTagAttributeName = @"ReactTagAttributeName"; +@implementation RCTShadowText +{ + NSAttributedString *_cachedAttributedString; + CGFloat _effectiveLetterSpacing; +} + static css_dim_t RCTMeasure(void *context, float width) { RCTShadowText *shadowText = (__bridge RCTShadowText *)context; - - NSTextStorage *textStorage = [[NSTextStorage alloc] initWithAttributedString:[shadowText attributedString]]; - NSTextStorage *previousTextStorage = shadowText.layoutManager.textStorage; - if (previousTextStorage) { - [previousTextStorage removeLayoutManager:shadowText.layoutManager]; - } - [textStorage addLayoutManager:shadowText.layoutManager]; - - shadowText.textContainer.size = CGSizeMake(isnan(width) ? CGFLOAT_MAX : width, CGFLOAT_MAX); - [shadowText.layoutManager ensureLayoutForTextContainer:shadowText.textContainer]; - - CGSize computedSize = [shadowText.layoutManager usedRectForTextContainer:shadowText.textContainer].size; - - [textStorage removeLayoutManager:shadowText.layoutManager]; - if (previousTextStorage) { - [previousTextStorage addLayoutManager:shadowText.layoutManager]; - } + NSTextStorage *textStorage = [shadowText buildTextStorageForWidth:width]; + NSLayoutManager *layoutManager = [textStorage.layoutManagers firstObject]; + NSTextContainer *textContainer = [layoutManager.textContainers firstObject]; + CGSize computedSize = [layoutManager usedRectForTextContainer:textContainer].size; css_dim_t result; result.dimensions[CSS_WIDTH] = RCTCeilPixelValue(computedSize.width); - if (shadowText.effectiveLetterSpacing < 0) { - result.dimensions[CSS_WIDTH] -= shadowText.effectiveLetterSpacing; + if (shadowText->_effectiveLetterSpacing < 0) { + result.dimensions[CSS_WIDTH] -= shadowText->_effectiveLetterSpacing; } result.dimensions[CSS_HEIGHT] = RCTCeilPixelValue(computedSize.height); return result; } -@implementation RCTShadowText -{ - NSLayoutManager *_layoutManager; - NSTextContainer *_textContainer; - NSAttributedString *_cachedAttributedString; - UIFont *_font; -} - - (instancetype)init { if ((self = [super init])) { _fontSize = NAN; _letterSpacing = NAN; _isHighlighted = NO; - - _textContainer = [[NSTextContainer alloc] init]; - _textContainer.lineBreakMode = NSLineBreakByTruncatingTail; - _textContainer.lineFragmentPadding = 0.0; - - _layoutManager = [[NSLayoutManager alloc] init]; - [_layoutManager addTextContainer:_textContainer]; } - return self; } +- (NSTextStorage *)buildTextStorageForWidth:(CGFloat)width +{ + NSLayoutManager *layoutManager = [[NSLayoutManager alloc] init]; + + NSTextStorage *textStorage = [[NSTextStorage alloc] initWithAttributedString:self.attributedString]; + [textStorage addLayoutManager:layoutManager]; + + NSTextContainer *textContainer = [[NSTextContainer alloc] init]; + textContainer.lineFragmentPadding = 0.0; + textContainer.lineBreakMode = _numberOfLines > 0 ? NSLineBreakByTruncatingTail : NSLineBreakByClipping; + textContainer.maximumNumberOfLines = _numberOfLines; + + UIEdgeInsets padding = self.paddingAsInsets; + width -= (padding.left + padding.right); + textContainer.size = (CGSize){isnan(width) ? CGFLOAT_MAX : width, CGFLOAT_MAX}; + + [layoutManager addTextContainer:textContainer]; + [layoutManager ensureLayoutForTextContainer:textContainer]; + + return textStorage; +} + - (NSAttributedString *)attributedString { return [self _attributedStringWithFontFamily:nil @@ -135,8 +134,8 @@ static css_dim_t RCTMeasure(void *context, float width) [self _addAttribute:NSBackgroundColorAttributeName withValue:self.textBackgroundColor toAttributedString:attributedString]; } - _font = [RCTConvert UIFont:nil withFamily:fontFamily size:fontSize weight:fontWeight style:fontStyle]; - [self _addAttribute:NSFontAttributeName withValue:_font toAttributedString:attributedString]; + UIFont *font = [RCTConvert UIFont:nil withFamily:fontFamily size:fontSize weight:fontWeight style:fontStyle]; + [self _addAttribute:NSFontAttributeName withValue:font toAttributedString:attributedString]; [self _addAttribute:NSKernAttributeName withValue:letterSpacing toAttributedString:attributedString]; [self _addAttribute:RCTReactTagAttributeName withValue:self.reactTag toAttributedString:attributedString]; [self _setParagraphStyleOnAttributedString:attributedString]; @@ -148,11 +147,6 @@ static css_dim_t RCTMeasure(void *context, float width) return _cachedAttributedString; } -- (UIFont *)font -{ - return _font ?: [RCTConvert UIFont:nil withFamily:_fontFamily size:@(_fontSize) weight:_fontWeight style:_fontStyle]; -} - - (void)_addAttribute:(NSString *)attribute withValue:(id)attributeValue toAttributedString:(NSMutableAttributedString *)attributedString { [attributedString enumerateAttribute:attribute inRange:NSMakeRange(0, [attributedString length]) options:0 usingBlock:^(id value, NSRange range, BOOL *stop) { @@ -231,38 +225,18 @@ static css_dim_t RCTMeasure(void *context, float width) [self dirtyText]; \ } -RCT_TEXT_PROPERTY(TextBackgroundColor, _textBackgroundColor, UIColor *); -RCT_TEXT_PROPERTY(Color, _color, UIColor *); -RCT_TEXT_PROPERTY(FontFamily, _fontFamily, NSString *); -RCT_TEXT_PROPERTY(FontSize, _fontSize, CGFloat); -RCT_TEXT_PROPERTY(FontWeight, _fontWeight, NSString *); -RCT_TEXT_PROPERTY(LetterSpacing, _letterSpacing, CGFloat); -RCT_TEXT_PROPERTY(LineHeight, _lineHeight, CGFloat); -RCT_TEXT_PROPERTY(ShadowOffset, _shadowOffset, CGSize); -RCT_TEXT_PROPERTY(TextAlign, _textAlign, NSTextAlignment); -RCT_TEXT_PROPERTY(IsHighlighted, _isHighlighted, BOOL); -RCT_TEXT_PROPERTY(Font, _font, UIFont *); - -- (NSLineBreakMode)truncationMode -{ - return _textContainer.lineBreakMode; -} - -- (void)setTruncationMode:(NSLineBreakMode)truncationMode -{ - _textContainer.lineBreakMode = truncationMode; - [self dirtyText]; -} - -- (NSUInteger)maximumNumberOfLines -{ - return _textContainer.maximumNumberOfLines; -} - -- (void)setMaximumNumberOfLines:(NSUInteger)maximumNumberOfLines -{ - _textContainer.maximumNumberOfLines = maximumNumberOfLines; - [self dirtyText]; -} +RCT_TEXT_PROPERTY(Color, _color, UIColor *) +RCT_TEXT_PROPERTY(FontFamily, _fontFamily, NSString *) +RCT_TEXT_PROPERTY(FontSize, _fontSize, CGFloat) +RCT_TEXT_PROPERTY(FontWeight, _fontWeight, NSString *) +RCT_TEXT_PROPERTY(FontStyle, _fontStyle, NSString *) +RCT_TEXT_PROPERTY(IsHighlighted, _isHighlighted, BOOL) +RCT_TEXT_PROPERTY(LetterSpacing, _letterSpacing, CGFloat) +RCT_TEXT_PROPERTY(LineHeight, _lineHeight, CGFloat) +RCT_TEXT_PROPERTY(NumberOfLines, _numberOfLines, NSUInteger) +RCT_TEXT_PROPERTY(ShadowOffset, _shadowOffset, CGSize) +RCT_TEXT_PROPERTY(TextAlign, _textAlign, NSTextAlignment) +RCT_TEXT_PROPERTY(TextBackgroundColor, _textBackgroundColor, UIColor *) +RCT_TEXT_PROPERTY(WritingDirection, _writingDirection, NSWritingDirection) @end diff --git a/Libraries/Text/RCTText.h b/Libraries/Text/RCTText.h index fe7803eb5d..487954f8a8 100644 --- a/Libraries/Text/RCTText.h +++ b/Libraries/Text/RCTText.h @@ -11,9 +11,7 @@ @interface RCTText : UIView -@property (nonatomic, strong) NSLayoutManager *layoutManager; -@property (nonatomic, strong) NSTextContainer *textContainer; -@property (nonatomic, copy) NSAttributedString *attributedText; @property (nonatomic, assign) UIEdgeInsets contentInset; +@property (nonatomic, strong) NSTextStorage *textStorage; @end diff --git a/Libraries/Text/RCTText.m b/Libraries/Text/RCTText.m index b4fcc69518..cdb09d4ff2 100644 --- a/Libraries/Text/RCTText.m +++ b/Libraries/Text/RCTText.m @@ -15,9 +15,7 @@ @implementation RCTText { - NSLayoutManager *_layoutManager; NSTextStorage *_textStorage; - NSTextContainer *_textContainer; } - (instancetype)initWithFrame:(CGRect)frame @@ -31,103 +29,42 @@ self.opaque = NO; self.contentMode = UIViewContentModeRedraw; } - return self; } -- (NSAttributedString *)attributedText +- (void)setTextStorage:(NSTextStorage *)textStorage { - return [_textStorage copy]; -} - -- (void)setAttributedText:(NSAttributedString *)attributedText -{ - for (NSLayoutManager *existingLayoutManager in _textStorage.layoutManagers) { - [_textStorage removeLayoutManager:existingLayoutManager]; - } - - _textStorage = [[NSTextStorage alloc] initWithAttributedString:attributedText]; - - if (_layoutManager) { - [_textStorage addLayoutManager:_layoutManager]; - } - + _textStorage = textStorage; [self setNeedsDisplay]; } -- (void)setTextContainer:(NSTextContainer *)textContainer -{ - if ([_textContainer isEqual:textContainer]) { - return; - } - - _textContainer = textContainer; - - for (NSInteger i = _layoutManager.textContainers.count - 1; i >= 0; i--) { - [_layoutManager removeTextContainerAtIndex:i]; - } - - if (_textContainer) { - [_layoutManager addTextContainer:_textContainer]; - } - - [self setNeedsDisplay]; -} - -- (void)setLayoutManager:(NSLayoutManager *)layoutManager -{ - if ([_layoutManager isEqual:layoutManager]) { - return; - } - - _layoutManager = layoutManager; - - for (NSLayoutManager *existingLayoutManager in _textStorage.layoutManagers) { - [_textStorage removeLayoutManager:existingLayoutManager]; - } - - if (_layoutManager) { - [_textStorage addLayoutManager:_layoutManager]; - } - - [self setNeedsDisplay]; -} - -- (CGRect)textFrame -{ - return UIEdgeInsetsInsetRect(self.bounds, _contentInset); -} - - (void)drawRect:(CGRect)rect { - CGRect textFrame = [self textFrame]; - - // We reset the text container size every time because RCTShadowText's - // RCTMeasure overrides it. The header comment for `size` says that a height - // of 0.0 should be enough, but it isn't. - _textContainer.size = CGSizeMake(textFrame.size.width, CGFLOAT_MAX); - - NSRange glyphRange = [_layoutManager glyphRangeForTextContainer:_textContainer]; - [_layoutManager drawBackgroundForGlyphRange:glyphRange atPoint:textFrame.origin]; - [_layoutManager drawGlyphsForGlyphRange:glyphRange atPoint:textFrame.origin]; + NSLayoutManager *layoutManager = [_textStorage.layoutManagers firstObject]; + NSTextContainer *textContainer = [layoutManager.textContainers firstObject]; + CGRect textFrame = UIEdgeInsetsInsetRect(self.bounds, _contentInset); + NSRange glyphRange = [layoutManager glyphRangeForTextContainer:textContainer]; + [layoutManager drawBackgroundForGlyphRange:glyphRange atPoint:textFrame.origin]; + [layoutManager drawGlyphsForGlyphRange:glyphRange atPoint:textFrame.origin]; } - (NSNumber *)reactTagAtPoint:(CGPoint)point { - CGFloat fraction; - NSUInteger characterIndex = [_layoutManager characterIndexForPoint:point - inTextContainer:_textContainer - fractionOfDistanceBetweenInsertionPoints:&fraction]; + NSNumber *reactTag = self.reactTag; - NSNumber *reactTag = nil; + CGFloat fraction; + NSLayoutManager *layoutManager = [_textStorage.layoutManagers firstObject]; + NSTextContainer *textContainer = [layoutManager.textContainers firstObject]; + NSUInteger characterIndex = [layoutManager characterIndexForPoint:point + inTextContainer:textContainer + fractionOfDistanceBetweenInsertionPoints:&fraction]; // If the point is not before (fraction == 0.0) the first character and not // 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)) { reactTag = [_textStorage attribute:RCTReactTagAttributeName atIndex:characterIndex effectiveRange:NULL]; } - - return reactTag ?: self.reactTag; + return reactTag; } #pragma mark - Accessibility diff --git a/Libraries/Text/RCTTextManager.m b/Libraries/Text/RCTTextManager.m index 31ae67c039..3dbb2ed537 100644 --- a/Libraries/Text/RCTTextManager.m +++ b/Libraries/Text/RCTTextManager.m @@ -47,20 +47,11 @@ RCT_EXPORT_SHADOW_PROPERTY(fontStyle, NSString) RCT_EXPORT_SHADOW_PROPERTY(isHighlighted, BOOL) RCT_EXPORT_SHADOW_PROPERTY(letterSpacing, CGFloat) RCT_EXPORT_SHADOW_PROPERTY(lineHeight, CGFloat) -RCT_EXPORT_SHADOW_PROPERTY(maximumNumberOfLines, NSInteger) RCT_EXPORT_SHADOW_PROPERTY(shadowOffset, CGSize) RCT_EXPORT_SHADOW_PROPERTY(textAlign, NSTextAlignment) RCT_REMAP_SHADOW_PROPERTY(backgroundColor, textBackgroundColor, UIColor) RCT_REMAP_SHADOW_PROPERTY(containerBackgroundColor, backgroundColor, UIColor) -RCT_CUSTOM_SHADOW_PROPERTY(numberOfLines, NSInteger, RCTShadowText) -{ - NSLineBreakMode truncationMode = NSLineBreakByClipping; - view.maximumNumberOfLines = json ? [RCTConvert NSInteger:json] : defaultView.maximumNumberOfLines; - if (view.maximumNumberOfLines > 0) { - truncationMode = NSLineBreakByTruncatingTail; - } - view.truncationMode = truncationMode; -} +RCT_EXPORT_SHADOW_PROPERTY(numberOfLines, NSUInteger) - (RCTViewManagerUIBlock)uiBlockToAmendWithShadowViewRegistry:(RCTSparseArray *)shadowViewRegistry { @@ -77,7 +68,7 @@ RCT_CUSTOM_SHADOW_PROPERTY(numberOfLines, NSInteger, RCTShadowText) continue; } - RCTSparseArray *reactTaggedAttributedStrings = [[RCTSparseArray alloc] init]; + RCTSparseArray *reactTaggedTextStorage = [[RCTSparseArray alloc] init]; NSMutableArray *queue = [NSMutableArray arrayWithObject:rootView]; for (NSInteger i = 0; i < [queue count]; i++) { RCTShadowView *shadowView = queue[i]; @@ -85,9 +76,11 @@ RCT_CUSTOM_SHADOW_PROPERTY(numberOfLines, NSInteger, RCTShadowText) if ([shadowView isKindOfClass:[RCTShadowText class]]) { RCTShadowText *shadowText = (RCTShadowText *)shadowView; - reactTaggedAttributedStrings[shadowText.reactTag] = [shadowText attributedString]; + NSTextStorage *textStorage = [shadowText buildTextStorageForWidth:shadowView.frame.size.width]; + reactTaggedTextStorage[shadowText.reactTag] = textStorage; } else if ([shadowView isKindOfClass:[RCTShadowRawText class]]) { - RCTLogError(@"Raw text cannot be used outside of a tag. Not rendering string: '%@'", [(RCTShadowRawText *)shadowView text]); + RCTLogError(@"Raw text cannot be used outside of a tag. Not rendering string: '%@'", + [(RCTShadowRawText *)shadowView text]); } else { for (RCTShadowView *child in [shadowView reactSubviews]) { if ([child isTextDirty]) { @@ -100,9 +93,9 @@ RCT_CUSTOM_SHADOW_PROPERTY(numberOfLines, NSInteger, RCTShadowText) } [uiBlocks addObject:^(RCTUIManager *uiManager, RCTSparseArray *viewRegistry) { - [reactTaggedAttributedStrings enumerateObjectsUsingBlock:^(NSAttributedString *attributedString, NSNumber *reactTag, BOOL *stop) { + [reactTaggedTextStorage enumerateObjectsUsingBlock:^(NSTextStorage *textStorage, NSNumber *reactTag, BOOL *stop) { RCTText *text = viewRegistry[reactTag]; - text.attributedText = attributedString; + text.textStorage = textStorage; }]; }]; } @@ -122,8 +115,6 @@ RCT_CUSTOM_SHADOW_PROPERTY(numberOfLines, NSInteger, RCTShadowText) return ^(RCTUIManager *uiManager, RCTSparseArray *viewRegistry) { RCTText *text = viewRegistry[reactTag]; text.contentInset = padding; - text.layoutManager = shadowView.layoutManager; - text.textContainer = shadowView.textContainer; }; } diff --git a/React/Modules/RCTUIManager.m b/React/Modules/RCTUIManager.m index e0698fef1f..998229111d 100644 --- a/React/Modules/RCTUIManager.m +++ b/React/Modules/RCTUIManager.m @@ -407,6 +407,10 @@ static NSDictionary *RCTViewConfigForModule(Class managerClass, NSString *viewNa @"-[RCTUIManager addUIBlock:] should only be called from the " "UIManager's _shadowQueue (it may be accessed via `bridge.uiManager.methodQueue`)"); + if (!block) { + return; + } + if (!self.isValid) { return; } @@ -909,15 +913,6 @@ RCT_EXPORT_METHOD(findSubviewIn:(NSNumber *)reactTag atPoint:(CGPoint)point call - (void)batchDidComplete { - // Gather blocks to be executed now that all view hierarchy manipulations have - // been completed (note that these may still take place before layout has finished) - for (RCTViewManager *manager in _viewManagers.allValues) { - RCTViewManagerUIBlock uiBlock = [manager uiBlockToAmendWithShadowViewRegistry:_shadowViewRegistry]; - if (uiBlock) { - [self addUIBlock:uiBlock]; - } - } - // Set up next layout animation if (_nextLayoutAnimation) { RCTLayoutAnimation *layoutAnimation = _nextLayoutAnimation; @@ -941,6 +936,12 @@ RCT_EXPORT_METHOD(findSubviewIn:(NSNumber *)reactTag atPoint:(CGPoint)point call _nextLayoutAnimation = nil; } + // Gather blocks to be executed now that layout is completed + for (RCTViewManager *manager in _viewManagers.allValues) { + RCTViewManagerUIBlock uiBlock = [manager uiBlockToAmendWithShadowViewRegistry:_shadowViewRegistry]; + [self addUIBlock:uiBlock]; + } + [self flushUIBlocks]; }