From e4a268669589b80708cfb69e4095357b03164ee4 Mon Sep 17 00:00:00 2001 From: Huy Nguyen Date: Tue, 17 Oct 2017 14:20:20 +0100 Subject: [PATCH] [ASTextKitComponents] Make sure Main Thread Checker isn't triggered during background calculations #trivial (#612) * Add unit test * Make sure TextKit components can calculate size in background without upsetting Main Thread Checker - Add a new thread-safe text view bounds. - Temporary components stack doesn't have a text view so it can be safely deallocated off main. * Add ASTextKitComponentsTextView * Remove unnecessary change * Fix minor mistake * ASTextKitComponentsTextView has only 1 initializer * Minor change * Switch to atomic property * Remove manual synthesization --- Source/ASEditableTextNode.mm | 4 +-- Source/TextKit/ASTextKitComponents.h | 9 +++-- Source/TextKit/ASTextKitComponents.mm | 48 +++++++++++++++++++-------- Tests/ASTextKitTests.mm | 29 +++++++++++++++- 4 files changed, 71 insertions(+), 19 deletions(-) diff --git a/Source/ASEditableTextNode.mm b/Source/ASEditableTextNode.mm index 98351f076..404049fcf 100644 --- a/Source/ASEditableTextNode.mm +++ b/Source/ASEditableTextNode.mm @@ -75,7 +75,7 @@ other content (setting scrollEnabled = NO on the UITextView itself, See issue: https://github.com/facebook/AsyncDisplayKit/issues/1063 */ -@interface ASPanningOverriddenUITextView : UITextView +@interface ASPanningOverriddenUITextView : ASTextKitComponentsTextView { BOOL _shouldBlockPanGesture; } @@ -215,7 +215,7 @@ - (void)didLoad ASDN::MutexLocker l(_textKitLock); // Create and configure the placeholder text view. - _placeholderTextKitComponents.textView = [[UITextView alloc] initWithFrame:CGRectZero textContainer:_placeholderTextKitComponents.textContainer]; + _placeholderTextKitComponents.textView = [[ASTextKitComponentsTextView alloc] initWithFrame:CGRectZero textContainer:_placeholderTextKitComponents.textContainer]; _placeholderTextKitComponents.textView.userInteractionEnabled = NO; _placeholderTextKitComponents.textView.accessibilityElementsHidden = YES; configureTextView(_placeholderTextKitComponents.textView); diff --git a/Source/TextKit/ASTextKitComponents.h b/Source/TextKit/ASTextKitComponents.h index 223abf5c1..20acc2c47 100644 --- a/Source/TextKit/ASTextKitComponents.h +++ b/Source/TextKit/ASTextKitComponents.h @@ -20,6 +20,12 @@ NS_ASSUME_NONNULL_BEGIN +@interface ASTextKitComponentsTextView : UITextView +- (instancetype)initWithFrame:(CGRect)frame textContainer:(nullable NSTextContainer *)textContainer NS_DESIGNATED_INITIALIZER; +- (nullable instancetype)initWithCoder:(NSCoder *)aDecoder __unavailable; +- (instancetype)init __unavailable; +@end + AS_SUBCLASSING_RESTRICTED @interface ASTextKitComponents : NSObject @@ -52,14 +58,13 @@ AS_SUBCLASSING_RESTRICTED */ - (CGSize)sizeForConstrainedWidth:(CGFloat)constrainedWidth; - - (CGSize)sizeForConstrainedWidth:(CGFloat)constrainedWidth forMaxNumberOfLines:(NSInteger)numberOfLines; @property (nonatomic, strong, readonly) NSTextStorage *textStorage; @property (nonatomic, strong, readonly) NSTextContainer *textContainer; @property (nonatomic, strong, readonly) NSLayoutManager *layoutManager; -@property (nonatomic, strong, nullable) UITextView *textView; +@property (nonatomic, strong, nullable) ASTextKitComponentsTextView *textView; @end diff --git a/Source/TextKit/ASTextKitComponents.mm b/Source/TextKit/ASTextKitComponents.mm index 6b3955eb7..7e4a7ae68 100644 --- a/Source/TextKit/ASTextKitComponents.mm +++ b/Source/TextKit/ASTextKitComponents.mm @@ -20,6 +20,37 @@ #import +@interface ASTextKitComponentsTextView () +@property (atomic, assign) CGRect threadSafeBounds; +@end + +@implementation ASTextKitComponentsTextView + +- (instancetype)initWithFrame:(CGRect)frame textContainer:(NSTextContainer *)textContainer +{ + self = [super initWithFrame:frame textContainer:textContainer]; + if (self) { + _threadSafeBounds = self.bounds; + } + return self; +} + +- (void)setFrame:(CGRect)frame +{ + ASDisplayNodeAssertMainThread(); + [super setFrame:frame]; + self.threadSafeBounds = self.bounds; +} + +- (void)setBounds:(CGRect)bounds +{ + ASDisplayNodeAssertMainThread(); + [super setBounds:bounds]; + self.threadSafeBounds = bounds; +} + +@end + @interface ASTextKitComponents () // read-write redeclarations @@ -27,9 +58,6 @@ @interface ASTextKitComponents () @property (nonatomic, strong, readwrite) NSTextContainer *textContainer; @property (nonatomic, strong, readwrite) NSLayoutManager *layoutManager; -// Indicates whether or not this object must be deallocated on main thread. Defaults to YES. -@property (nonatomic, assign) BOOL requiresMainThreadDeallocation; - @end @implementation ASTextKitComponents @@ -61,8 +89,6 @@ + (instancetype)componentsWithTextStorage:(NSTextStorage *)textStorage components.textContainer.lineFragmentPadding = 0.0; // We want the text laid out up to the very edges of the text-view. [components.layoutManager addTextContainer:components.textContainer]; - components.requiresMainThreadDeallocation = YES; - return components; } @@ -70,11 +96,9 @@ + (instancetype)componentsWithTextStorage:(NSTextStorage *)textStorage - (void)dealloc { - if (_requiresMainThreadDeallocation) { - ASDisplayNodeAssertMainThread(); - } // Nil out all delegates to prevent crash if (_textView) { + ASDisplayNodeAssertMainThread(); _textView.delegate = nil; } _layoutManager.delegate = nil; @@ -88,10 +112,8 @@ - (CGSize)sizeForConstrainedWidth:(CGFloat)constrainedWidth // If our text-view's width is already the constrained width, we can use our existing TextKit stack for this sizing calculation. // Otherwise, we create a temporary stack to size for `constrainedWidth`. - if (CGRectGetWidth(components.textView.bounds) != constrainedWidth) { + if (CGRectGetWidth(components.textView.threadSafeBounds) != constrainedWidth) { components = [ASTextKitComponents componentsWithAttributedSeedString:components.textStorage textContainerSize:CGSizeMake(constrainedWidth, CGFLOAT_MAX)]; - // The temporary stack can be deallocated off main - components.requiresMainThreadDeallocation = NO; } // Force glyph generation and layout, which may not have happened yet (and isn't triggered by -usedRectForTextContainer:). @@ -112,9 +134,7 @@ - (CGSize)sizeForConstrainedWidth:(CGFloat)constrainedWidth // Always use temporary stack in case of threading issues components = [ASTextKitComponents componentsWithAttributedSeedString:components.textStorage textContainerSize:CGSizeMake(constrainedWidth, CGFLOAT_MAX)]; - // The temporary stack can be deallocated off main - components.requiresMainThreadDeallocation = NO; - + // Force glyph generation and layout, which may not have happened yet (and isn't triggered by - usedRectForTextContainer:). [components.layoutManager ensureLayoutForTextContainer:components.textContainer]; diff --git a/Tests/ASTextKitTests.mm b/Tests/ASTextKitTests.mm index a39a333b1..687206178 100644 --- a/Tests/ASTextKitTests.mm +++ b/Tests/ASTextKitTests.mm @@ -20,11 +20,14 @@ #import -#import #import +#import +#import #import #import +#import + @interface ASTextKitTests : XCTestCase @end @@ -201,4 +204,28 @@ - (void)testRectsForRangeBeyondTruncationSizeReturnsNonZeroNumberOfRects XCTAssert([renderer rectsForTextRange:NSMakeRange(0, attributedString.length) measureOption:ASTextKitRendererMeasureOptionBlock].count > 0); } +- (void)testTextKitComponentsCanCalculateSizeInBackground +{ + NSAttributedString *attributedString = + [[NSAttributedString alloc] + initWithString:@"90's cray photo booth tote bag bespoke Carles. Plaid wayfarers Odd Future master cleanse tattooed four dollar toast small batch kale chips leggings meh photo booth occupy irony. " attributes:@{ASTextKitEntityAttributeName : [[ASTextKitEntityAttribute alloc] initWithEntity:@"entity"]}]; + ASTextKitComponents *components = [ASTextKitComponents componentsWithAttributedSeedString:attributedString textContainerSize:CGSizeZero]; + components.textView = [[ASTextKitComponentsTextView alloc] initWithFrame:CGRectZero textContainer:components.textContainer]; + components.textView.frame = CGRectMake(0, 0, 20, 1000); + + XCTestExpectation *expectation = [self expectationWithDescription:@"Components deallocated in background"]; + + ASPerformBlockOnBackgroundThread(^{ + // Use an autorelease pool here to ensure temporary components are (and can be) released in background + @autoreleasepool { + [components sizeForConstrainedWidth:100]; + [components sizeForConstrainedWidth:50 forMaxNumberOfLines:5]; + } + + [expectation fulfill]; + }); + + [self waitForExpectationsWithTimeout:1 handler:nil]; +} + @end