Skip to content

Commit

Permalink
[ASTextKitComponents] Make sure Main Thread Checker isn't triggered d…
Browse files Browse the repository at this point in the history
…uring background calculations #trivial (TextureGroup#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
  • Loading branch information
nguyenhuy authored and bernieperez committed Apr 25, 2018
1 parent c5b5fa6 commit e4a2686
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 19 deletions.
4 changes: 2 additions & 2 deletions Source/ASEditableTextNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
Expand Down
9 changes: 7 additions & 2 deletions Source/TextKit/ASTextKitComponents.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down
48 changes: 34 additions & 14 deletions Source/TextKit/ASTextKitComponents.mm
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,44 @@

#import <tgmath.h>

@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
@property (nonatomic, strong, readwrite) NSTextStorage *textStorage;
@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
Expand Down Expand Up @@ -61,20 +89,16 @@ + (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;
}

#pragma mark - Lifecycle

- (void)dealloc
{
if (_requiresMainThreadDeallocation) {
ASDisplayNodeAssertMainThread();
}
// Nil out all delegates to prevent crash
if (_textView) {
ASDisplayNodeAssertMainThread();
_textView.delegate = nil;
}
_layoutManager.delegate = nil;
Expand All @@ -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:).
Expand All @@ -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];

Expand Down
29 changes: 28 additions & 1 deletion Tests/ASTextKitTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,14 @@

#import <FBSnapshotTestCase/FBSnapshotTestController.h>

#import <AsyncDisplayKit/ASTextKitEntityAttribute.h>
#import <AsyncDisplayKit/ASTextKitAttributes.h>
#import <AsyncDisplayKit/ASTextKitComponents.h>
#import <AsyncDisplayKit/ASTextKitEntityAttribute.h>
#import <AsyncDisplayKit/ASTextKitRenderer.h>
#import <AsyncDisplayKit/ASTextKitRenderer+Positioning.h>

#import <AsyncDisplayKit/ASInternalHelpers.h>

@interface ASTextKitTests : XCTestCase

@end
Expand Down Expand Up @@ -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

0 comments on commit e4a2686

Please sign in to comment.