Node TintColor Tweaks (#1666)

* Node TintColor Tweaks

- If the tint color changes, be sure to call -setNeedsDisplay on the view.
- Explictly declare tintColor ivar and getter method.
- If the tintColor is the same as the existing one, early-exit.
- Use the ivar and not the property when setting the tintColor, just as is done for background color.
- Add tests (for non iOS13 and iOS13) that the tintColor will call -setNeedsDisplay
- Backfill tests to document/verify the tintColor behavior on a node.

Source/Private/_ASPendingState.mm

@@ -15,7 +15,8 @@
 
 #define __shouldSetNeedsDisplayForView(view) (flags.needsDisplay \
   || (flags.setOpaque && _flags.opaque != (view).opaque)\
-  || (flags.setBackgroundColor && ![backgroundColor isEqual:(view).backgroundColor]))
+  || (flags.setBackgroundColor && ![backgroundColor isEqual:(view).backgroundColor])\
+  || (flags.setTintColor && ![tintColor isEqual:(view).tintColor]))
 
 #define __shouldSetNeedsDisplayForLayer(layer) (flags.needsDisplay \
   || (flags.setOpaque && _flags.opaque != (layer).opaque)\
@@ -104,6 +105,7 @@ @implementation _ASPendingState
   CGRect frame;   // Frame is only to be used for synchronous views wrapped by nodes (see setFrame:)
   CGRect bounds;
   UIColor *backgroundColor;
+  UIColor *tintColor;
   CGFloat alpha;
   CGFloat cornerRadius;
   UIViewContentMode contentMode;
@@ -428,8 +430,16 @@ - (void)setBackgroundColor:(UIColor *)color
   _stateToApplyFlags.setBackgroundColor = YES;
 }
 
+- (UIColor *)tintColor
+{
+  return tintColor;
+}
+
 - (void)setTintColor:(UIColor *)newTintColor
 {
+  if ([newTintColor isEqual:tintColor]) {
+    return;
+  }
   tintColor = newTintColor;
   _stateToApplyFlags.setTintColor = YES;
 }
@@ -1097,7 +1107,7 @@ - (void)applyToView:(UIView *)view withSpecialPropertiesHandling:(BOOL)specialPr
   }
 
   if (flags.setTintColor)
-    view.tintColor = self.tintColor;
+    view.tintColor = tintColor;
 
   if (flags.setOpaque) {
     view.opaque = _flags.opaque;

Tests/ASBridgedPropertiesTests.mm

@@ -21,6 +21,7 @@ - (BOOL)test_isFlushScheduled;
 
 @interface ASBridgedPropertiesTestView : UIView
 @property (nonatomic, readonly) BOOL receivedSetNeedsLayout;
+@property (nonatomic, readonly) NSUInteger setNeedsDisplayCount;
 @end
 
 @implementation ASBridgedPropertiesTestView
@@ -31,6 +32,12 @@ - (void)setNeedsLayout
   [super setNeedsLayout];
 }
 
+- (void)setNeedsDisplay
+{
+  _setNeedsDisplayCount += 1;
+  [super setNeedsDisplay];
+}
+
 @end
 
 @interface ASBridgedPropertiesTestNode : ASDisplayNode
@@ -138,6 +145,32 @@ - (void)testThatReadingABridgedLayerPropertyInBackgroundThrowsAnException
   });
 }
 
+- (void)testThatSettingTintColorSetNeedsDisplayOnView
+{
+  ASPendingStateController *ctrl = [ASPendingStateController sharedInstance];
+
+  ASDisplayNode *node = [[ASDisplayNode alloc] initWithViewClass:ASBridgedPropertiesTestView.class];
+  ASBridgedPropertiesTestView *view = (ASBridgedPropertiesTestView *)node.view;
+  NSUInteger initialSetNeedsDisplayCount = view.setNeedsDisplayCount;
+#if AS_AT_LEAST_IOS13
+  // This is called an extra time on iOS13 for unknown reasons. Need to Investigate.
+  if (@available(iOS 13.0, *)) {
+    XCTAssertEqual(initialSetNeedsDisplayCount, 2);
+  } else {
+    XCTAssertEqual(initialSetNeedsDisplayCount, 1);
+  }
+#endif
+
+  ASDispatchSyncOnOtherThread(^{
+    node.tintColor = UIColor.orangeColor;
+  });
+  XCTAssertNotEqualObjects(view.tintColor, UIColor.orangeColor);
+  XCTAssertEqual(view.setNeedsDisplayCount, initialSetNeedsDisplayCount);
+  [ctrl flush];
+  XCTAssertEqualObjects(view.tintColor, UIColor.orangeColor);
+  XCTAssertEqual(view.setNeedsDisplayCount, initialSetNeedsDisplayCount + 1);
+}
+
 - (void)testThatManuallyFlushingTheSyncControllerImmediatelyAppliesChanges
 {
   ASPendingStateController *ctrl = [ASPendingStateController sharedInstance];

Tests/ASDisplayNodeTests.mm

@@ -515,9 +515,15 @@ - (void)checkValuesMatchDefaults:(ASDisplayNode *)node isLayerBacked:(BOOL)isLay
     XCTAssertEqual(NO, node.preservesSuperviewLayoutMargins, @"default preservesSuperviewLayoutMargins broken %@", hasLoadedView);
     XCTAssertTrue(UIEdgeInsetsEqualToEdgeInsets(UIEdgeInsetsZero, node.safeAreaInsets), @"default safeAreaInsets broken %@", hasLoadedView);
     XCTAssertEqual(YES, node.insetsLayoutMarginsFromSafeArea, @"default insetsLayoutMarginsFromSafeArea broken %@", hasLoadedView);
+    if (node.nodeLoaded) {
+      XCTAssertNotNil(node.tintColor, @"default tintColor broken %@", hasLoadedView); // It has been populated by the UIView.
+    } else {
+      XCTAssertNil(node.tintColor, @"default tintColor broken %@", hasLoadedView);
+    }
   } else {
     XCTAssertEqual(NO, node.userInteractionEnabled, @"layer-backed nodes do not support userInteractionEnabled %@", hasLoadedView);
     XCTAssertEqual(NO, node.exclusiveTouch, @"layer-backed nodes do not support exclusiveTouch %@", hasLoadedView);
+    XCTAssertNil(node.tintColor, @"default tintColor broken %@", hasLoadedView);
   }
 }
 
@@ -589,6 +595,7 @@ - (void)checkValuesMatchSetValues:(ASDisplayNode *)node isLayerBacked:(BOOL)isLa
   XCTAssertTrue(CATransform3DEqualToTransform(CATransform3DMakeScale(0.5, 0.5, 1.0), node.transform), @"transform broken %@", hasLoadedView);
   XCTAssertTrue(CATransform3DEqualToTransform(CATransform3DMakeTranslation(1337, 7357, 7007), node.subnodeTransform), @"subnodeTransform broken %@", hasLoadedView);
   XCTAssertEqualObjects([UIColor clearColor], node.backgroundColor, @"backgroundColor broken %@", hasLoadedView);
+  XCTAssertEqualObjects([UIColor orangeColor], node.tintColor, @"tintColor broken %@", hasLoadedView);
   XCTAssertEqual(UIViewContentModeBottom, node.contentMode, @"contentMode broken %@", hasLoadedView);
   XCTAssertEqual([[UIColor cyanColor] CGColor], node.shadowColor, @"shadowColor broken %@", hasLoadedView);
   XCTAssertEqual(.5f, node.shadowOpacity, @"shadowOpacity broken %@", hasLoadedView);
@@ -663,6 +670,7 @@ - (void)checkSimpleBridgePropertiesSetPropagate:(BOOL)isLayerBacked
     node.transform = CATransform3DMakeScale(0.5, 0.5, 1.0);
     node.subnodeTransform = CATransform3DMakeTranslation(1337, 7357, 7007);
     node.backgroundColor = [UIColor clearColor];
+    node.tintColor = [UIColor orangeColor];
     node.contentMode = UIViewContentModeBottom;
     node.shadowColor = [[UIColor cyanColor] CGColor];
     node.shadowOpacity = .5f;
@@ -753,6 +761,7 @@ - (void)testPropertiesSetOffThreadBeforeLoadingExternalView
       return view;
     }];
     node.backgroundColor = [UIColor blueColor];
+    node.tintColor = [UIColor orangeColor];
     node.frame = CGRectMake(10, 20, 30, 40);
     node.autoresizingMask = UIViewAutoresizingFlexibleWidth;
     node.userInteractionEnabled = YES;
@@ -772,6 +781,7 @@ - (void)testPropertiesSetOnThreadAfterLoadingExternalView
   [node view];
 
   node.backgroundColor = [UIColor blueColor];
+  node.tintColor = [UIColor orangeColor];
   node.frame = CGRectMake(10, 20, 30, 40);
   node.autoresizingMask = UIViewAutoresizingFlexibleWidth;
   node.userInteractionEnabled = YES;
@@ -784,6 +794,7 @@ - (void)checkExternalViewAppliedPropertiesMatch:(ASDisplayNode *)node
   UIView *view = node.view;
 
   XCTAssertEqualObjects([UIColor blueColor], view.backgroundColor, @"backgroundColor not propagated to view");
+  XCTAssertEqualObjects([UIColor orangeColor], view.tintColor, @"tintColor not propagated to view");
   XCTAssertTrue(CGRectEqualToRect(CGRectMake(10, 20, 30, 40), view.frame), @"frame not propagated to view");
   XCTAssertEqual(UIViewAutoresizingFlexibleWidth, view.autoresizingMask, @"autoresizingMask not propagated to view");
   XCTAssertEqual(YES, view.userInteractionEnabled, @"userInteractionEnabled not propagated to view");