Fix main thread assertion for tint color on text nodes, re-render tin…

…t-able nodes on hierarchy changes (#1670)

- Address various issues with tint color on text nodes
- Avoid accessing `self.tintColor` off-main since this will trigger an
assertion and might be undefined behavior
- Trigger setNeedsDisplay in cases of hierarchy change and
tintColorDidChange in ASButtonNode, ASTextNode{1,2}
- Fix bug in _ASTableViewCell where we were overwriting the tint color
instead of inheriting the correct value
- Add tests for new logic in changing hierarchy

Source/ASButtonNode.mm

@@ -140,9 +140,12 @@ -(void)tintColorDidChange
   // UIButton documentation states that it tints the image and title of buttons when tintColor is set.
   // | "The tint color to apply to the button title and image."
   // | From: https://developer.apple.com/documentation/uikit/uibutton/1624025-tintcolor
+  [self lock];
   UIColor *tintColor = self.tintColor;
   self.imageNode.tintColor = tintColor;
   self.titleNode.tintColor = tintColor;
+  [self unlock];
+  [self setNeedsDisplay];
 }
 
 - (void)updateImage

Source/ASTableView.mm

@@ -20,6 +20,7 @@
 #import <AsyncDisplayKit/ASDelegateProxy.h>
 #import <AsyncDisplayKit/ASDisplayNodeExtras.h>
 #import <AsyncDisplayKit/ASDisplayNode+FrameworkPrivate.h>
+#import <AsyncDisplayKit/ASDisplayNodeInternal.h>
 #import <AsyncDisplayKit/ASElementMap.h>
 #import <AsyncDisplayKit/ASInternalHelpers.h>
 #import <AsyncDisplayKit/ASLayout.h>
@@ -28,6 +29,7 @@
 #import <AsyncDisplayKit/ASTableLayoutController.h>
 #import <AsyncDisplayKit/ASBatchContext.h>
 
+
 static NSString * const kCellReuseIdentifier = @"_ASTableViewCell";
 
 //#define LOG(...) NSLog(__VA_ARGS__)
@@ -113,12 +115,14 @@ - (void)setElement:(ASCollectionElement *)element
     self.selectionStyle = node.selectionStyle; 
     self.focusStyle = node.focusStyle;
     self.accessoryType = node.accessoryType;
-    self.tintColor = node.tintColor;
 
     // the following ensures that we clip the entire cell to it's bounds if node.clipsToBounds is set (the default)
     // This is actually a workaround for a bug we are seeing in some rare cases (selected background view
     // overlaps other cells if size of ASCellNode has changed.)
     self.clipsToBounds = node.clipsToBounds;
+
+    // If the cell node has been explicitly configured with a tint color, we can apply that directly to the cell view to preserve the previous behavior
+    self.tintColor = node->_tintColor;
   }
 
   [node __setSelectedFromUIKit:self.selected];

Source/ASTextNode.mm

@@ -183,6 +183,7 @@ @implementation ASTextNode {
   CGSize _shadowOffset;
   CGColorRef _shadowColor;
   UIColor *_cachedShadowUIColor;
+  UIColor *_cachedTintColor;
   UIColor *_placeholderColor;
   CGFloat _shadowOpacity;
   CGFloat _shadowRadius;
@@ -382,7 +383,7 @@ - (ASTextKitAttributes)_locked_rendererAttributes
     .shadowColor = _cachedShadowUIColor,
     .shadowOpacity = _shadowOpacity,
     .shadowRadius = _shadowRadius,
-    .tintColor = self.textColorFollowsTintColor ? self.tintColor : nil
+    .tintColor = _cachedTintColor
   };
 }
 
@@ -543,7 +544,11 @@ - (NSArray *)exclusionPaths
 - (NSObject *)drawParametersForAsyncLayer:(_ASDisplayLayer *)layer
 {
   ASLockScopeSelf();
-
+  if (_textColorFollowsTintColor) {
+    _cachedTintColor = self.tintColor;
+  } else {
+    _cachedTintColor = nil;
+  }
   return [[ASTextNodeDrawParameter alloc] initWithRendererAttributes:[self _locked_rendererAttributes]
                                                      backgroundColor:self.backgroundColor
                                                  textContainerInsets:_textContainerInset
@@ -945,6 +950,39 @@ - (CGRect)frameForTextRange:(NSRange)textRange
   return ASTextNodeAdjustRenderRectForShadowPadding(frame, self.shadowPadding);
 }
 
+#pragma mark - Tint Colors
+
+
+- (void)tintColorDidChange
+{
+  [super tintColorDidChange];
+
+  [self _setNeedsDisplayOnTintedTextColor];
+}
+
+- (void)_setNeedsDisplayOnTintedTextColor
+{
+  BOOL textColorFollowsTintColor = NO;
+  {
+    AS::MutexLocker l(__instanceLock__);
+    textColorFollowsTintColor = _textColorFollowsTintColor;
+  }
+
+  if (textColorFollowsTintColor) {
+    [self setNeedsDisplay];
+  }
+}
+
+
+#pragma mark Interface State
+
+- (void)didEnterHierarchy
+{
+  [super didEnterHierarchy];
+
+  [self _setNeedsDisplayOnTintedTextColor];
+}
+
 #pragma mark - Placeholders
 
 - (UIColor *)placeholderColor

Source/ASTextNode2.mm

@@ -511,19 +511,6 @@ - (void)prepareAttributedString:(NSMutableAttributedString *)attributedString is
     shadow.shadowBlurRadius = _shadowRadius;
     [attributedString addAttribute:NSShadowAttributeName value:shadow range:NSMakeRange(0, attributedString.length)];
   }
-
-  // Apply tint color if needed and foreground color is not already specified
-  if (self.textColorFollowsTintColor && attributedString.length > 0) {
-    // Apply tint color if specified and if foreground color is undefined for attributedString
-    NSRange limit = NSMakeRange(0, attributedString.length);
-    // Look for previous attributes that define foreground color
-    UIColor *attributeValue = (UIColor *)[attributedString attribute:NSForegroundColorAttributeName atIndex:limit.location effectiveRange:NULL];
-    UIColor *tintColor = self.tintColor;
-    if (attributeValue == nil && tintColor) {
-      // None are found, apply tint color if available. Fallback to "black" text color
-      [attributedString addAttributes:@{ NSForegroundColorAttributeName : tintColor } range:limit];
-    }
-  }
 }
 
 #pragma mark - Drawing
@@ -540,7 +527,20 @@ - (NSObject *)drawParametersForAsyncLayer:(_ASDisplayLayer *)layer
   NSMutableAttributedString *mutableText = [_attributedText mutableCopy] ?: [[NSMutableAttributedString alloc] init];
 
   [self prepareAttributedString:mutableText isForIntrinsicSize:NO];
-
+
+  // After all other attributes are set, apply tint color if needed and foreground color is not already specified
+  if (self.textColorFollowsTintColor && mutableText.length > 0) {
+    // Apply tint color if specified and if foreground color is undefined for attributedString
+    NSRange limit = NSMakeRange(0, mutableText.length);
+    // Look for previous attributes that define foreground color
+    UIColor *attributeValue = (UIColor *)[mutableText attribute:NSForegroundColorAttributeName atIndex:limit.location effectiveRange:NULL];
+    UIColor *tintColor = self.tintColor;
+    if (attributeValue == nil && tintColor) {
+      // None are found, apply tint color if available. Fallback to "black" text color
+      [mutableText addAttributes:@{ NSForegroundColorAttributeName : tintColor } range:limit];
+    }
+  }
+
   return @{
     @"container": copiedContainer,
     @"text": mutableText,
@@ -577,6 +577,38 @@ + (void)drawRect:(CGRect)bounds withParameters:(NSDictionary *)layoutDict isCanc
   [layout drawInContext:context size:bounds.size point:bounds.origin view:nil layer:nil debug:[ASTextDebugOption sharedDebugOption] cancel:isCancelledBlock];
 }
 
+#pragma mark - Tint Color
+
+- (void)tintColorDidChange
+{
+  [super tintColorDidChange];
+
+  [self _setNeedsDisplayOnTintedTextColor];
+}
+
+- (void)_setNeedsDisplayOnTintedTextColor
+{
+  BOOL textColorFollowsTintColor = NO;
+  {
+    AS::MutexLocker l(__instanceLock__);
+    textColorFollowsTintColor = _textColorFollowsTintColor;
+  }
+
+  if (textColorFollowsTintColor) {
+    [self setNeedsDisplay];
+  }
+}
+
+
+#pragma mark Interface State
+
+- (void)didEnterHierarchy
+{
+  [super didEnterHierarchy];
+
+  [self _setNeedsDisplayOnTintedTextColor];
+}
+
 #pragma mark - Attributes
 
 - (id)linkAttributeValueAtPoint:(CGPoint)point

Source/Private/ASDisplayNode+UIViewBridge.mm

@@ -823,6 +823,7 @@ - (void)setTintColor:(UIColor *)color
       }
     }
   } else {
+    _tintColor = color;
     _setToViewOnly(tintColor, color);
   }
   __instanceLock__.unlock();

Tests/ASButtonNodeSnapshotTests.mm

@@ -80,4 +80,36 @@ - (void)testTintColorWithForegroundColorSet
   ASSnapshotVerifyNode(node, nil);
 }
 
+- (void)testTintColorWithInheritedTintColor
+{
+  ASDisplayNode *container = [[ASDisplayNode alloc] init];
+  container.tintColor = UIColor.redColor;
+
+  // Add to hierarchy, assert new tint is picked up
+  ASButtonNode *node = [[ASButtonNode alloc] init];
+  [container addSubnode:node];
+  [node setImage:[[self testImage] imageWithRenderingMode:UIImageRenderingModeAlwaysTemplate] forState:UIControlStateNormal];
+  [node setTitle:@"Press Me"
+        withFont:[UIFont systemFontOfSize:48]
+       withColor:nil
+        forState:UIControlStateNormal];
+  node.imageNode.style.width = ASDimensionMake(200);
+  node.imageNode.style.height = ASDimensionMake(200);
+  container.style.preferredSize = CGSizeMake(1000,1000);
+  ASDisplayNodeSizeToFitSize(node, CGSizeMake(1000, 1000));
+  // Force load so the superview is set for the button _ASDisplayView
+  __unused UIView *v = container.view;
+  ASSnapshotVerifyNode(node, @"red_inherited_tint");
+
+  // Change hierarchy, assert new tint is picked up
+  ASDisplayNode *container2 = [[ASDisplayNode alloc] init];
+  container2.tintColor = UIColor.greenColor;
+  container2.style.preferredSize = CGSizeMake(1000,1000);
+  [container2 addSubnode:node];
+  ASDisplayNodeSizeToFitSize(node, CGSizeMake(1000, 1000));
+  // Force load so the superview is set for the button _ASDisplayView
+  __unused UIView *v2 = container2.view; // Force load
+  ASSnapshotVerifyNode(node, @"green_inherited_tint");
+}
+
 @end

Tests/ASTableViewTests.mm

@@ -190,6 +190,7 @@ - (ASCellNodeBlock)tableView:(ASTableView *)tableView nodeBlockForRowAtIndexPath
     ASTestTextCellNode *textCellNode = [ASTestTextCellNode new];
     textCellNode.text = [NSString stringWithFormat:@"{%d, %d}", (int)indexPath.section, (int)indexPath.row];
     textCellNode.backgroundColor = [UIColor whiteColor];
+    textCellNode.tintColor = [UIColor yellowColor];
     return textCellNode;
   };
 }
@@ -886,6 +887,26 @@ - (void)testTableViewReloadDoesReloadIfEditableTextNodeIsFirstResponder
   XCTAssertEqual([node.view numberOfRowsInSection:0], 2);
 }
 
+
+- (void)testTintColorIsPropagatedToTableViewCell
+{
+  // If a tint color is explicitly defined on an ASCellNode, we should
+  CGSize tableViewSize = CGSizeMake(100, 500);
+  ASTestTableView *tableView = [[ASTestTableView alloc] initWithFrame:CGRectMake(0, 0, tableViewSize.width, tableViewSize.height)
+                                                                style:UITableViewStylePlain];
+  ASTableViewFilledDataSource *dataSource = [ASTableViewFilledDataSource new];
+
+  tableView.asyncDelegate = dataSource;
+  tableView.asyncDataSource = dataSource;
+
+  [tableView reloadData];
+  [tableView waitUntilAllUpdatesAreCommitted];
+  NSIndexPath *indexPath = [NSIndexPath indexPathForRow:0 inSection:0];
+  UITableViewCell *uikitCell = [tableView cellForRowAtIndexPath:indexPath];
+  BOOL areColorsEqual = CGColorEqualToColor(uikitCell.tintColor.CGColor, UIColor.yellowColor.CGColor);
+  XCTAssertTrue(areColorsEqual);
+}
+
 @end
 
 @implementation UITableView (Testing)

Tests/ASTextNode2SnapshotTests.mm

@@ -331,4 +331,27 @@ - (void)testTextTintColor_ASTextNode2
   ASSnapshotVerifyNode(textNode, nil);
 }
 
+
+- (void)testTintColorHierarchyChange
+{
+  ASDisplayNode *containerNode = [[ASDisplayNode alloc] init];
+  containerNode.tintColor = [UIColor greenColor];
+
+  ASTextNode *node = [[ASTextNode alloc] init];
+  [containerNode addSubnode:node];
+  [node setLayerBacked:YES];
+  node.textColorFollowsTintColor = YES;
+  node.attributedText = [[NSAttributedString alloc] initWithString:@"Hello" attributes:@{}];
+  ASDisplayNodeSizeToFitSizeRange(node, ASSizeRangeMake(CGSizeZero, CGSizeMake(INFINITY, INFINITY)));
+  containerNode.style.preferredSize = node.bounds.size;
+  ASSnapshotVerifyNode(node, @"green_tint_from_parent");
+
+  ASDisplayNode *containerNode2 = [[ASDisplayNode alloc] init];
+  containerNode2.tintColor = [UIColor redColor];
+  [containerNode2 addSubnode:node];
+  containerNode2.style.preferredSize = node.bounds.size;
+  ASSnapshotVerifyNode(node, @"red_tint_from_parent");
+}
+
+
 @end

Tests/ASTextNodeSnapshotTests.mm

@@ -162,6 +162,28 @@ - (void)testUIGraphicsRendererDrawingExperiment
   ASSnapshotVerifyNode(textNode, nil);
 }
 
+- (void)testTintColorHierarchyChange
+{
+  ASDisplayNode *containerNode = [[ASDisplayNode alloc] init];
+  containerNode.tintColor = [UIColor greenColor];
+
+  ASTextNode *node = [[ASTextNode alloc] init];
+  [containerNode addSubnode:node];
+  [node setLayerBacked:YES];
+  node.textColorFollowsTintColor = YES;
+  node.attributedText = [[NSAttributedString alloc] initWithString:@"Hello" attributes:@{}];
+  ASDisplayNodeSizeToFitSizeRange(node, ASSizeRangeMake(CGSizeZero, CGSizeMake(INFINITY, INFINITY)));
+  containerNode.style.preferredSize = node.bounds.size;
+  ASSnapshotVerifyNode(node, @"green_tint_from_parent");
+
+
+  ASDisplayNode *containerNode2 = [[ASDisplayNode alloc] init];
+  containerNode2.tintColor = [UIColor redColor];
+  [containerNode2 addSubnode:node];
+  containerNode2.style.preferredSize = node.bounds.size;
+  ASSnapshotVerifyNode(node, @"red_tint_from_parent");
+}
+
 #if AS_AT_LEAST_IOS13
 
 - (void)testUserInterfaceStyleSnapshotTesting
@@ -248,6 +270,9 @@ - (void)testUserInterfaceStyleSnapshotTestingTintColor
   }
 }
 
+
+
+
 #endif // #if AS_AT_LEAST_IOS13
 
 @end