Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ASTextKitComponents] Make sure Main Thread Checker isn't triggered during background calculations #trivial #612

Merged
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
66 changes: 52 additions & 14 deletions Source/TextKit/ASTextKitComponents.mm
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,65 @@

#import <AsyncDisplayKit/ASTextKitComponents.h>
#import <AsyncDisplayKit/ASAssert.h>
#import <AsyncDisplayKit/ASThread.h>

#import <tgmath.h>

@interface ASTextKitComponentsTextView ()
@property (nonatomic, assign) CGRect threadSafeBounds;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be atomic?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point!

@end

@implementation ASTextKitComponentsTextView
{
ASDN::Mutex __instanceLock__;
}

@synthesize threadSafeBounds = _threadSafeBounds;

- (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;
}

- (void)setThreadSafeBounds:(CGRect)bounds
{
ASDN::MutexLocker l(__instanceLock__);
_threadSafeBounds = bounds;
}

- (CGRect)threadSafeBounds
{
ASDN::MutexLocker l(__instanceLock__);
return _threadSafeBounds;
}

@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 +107,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 +130,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 +152,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