Improved robustness of implicit animation support (#53)

* Checkpoint for templated animations.

* Arg naming.

Author: jverkoey

src/MDMMotionAnimator.h

@@ -160,6 +160,7 @@ NS_SWIFT_NAME(MotionAnimator)
  be able to implicitly animate its properties with MDMMotionAnimator. This is not necessary for
  layers that are backing a UIView.
  */
-+ (nonnull id<CALayerDelegate>)sharedLayerDelegate;
++ (nonnull id<CALayerDelegate>)sharedLayerDelegate
+    __deprecated_msg("No longer needed for implicit animations of headless layers.");
 
 @end

src/private/MDMBlockAnimations.m

@@ -25,8 +25,9 @@ @interface MDMActionContext: NSObject
 @property(nonatomic, readonly) NSArray<MDMImplicitAction *> *interceptedActions;
 @end
 
-// The original UIView method implementation of actionForLayer:forKey:.
-static IMP sOriginalActionForLayerImp = NULL;
+// The original CALayer method implementation of -actionForKey:
+static IMP sOriginalActionForKeyLayerImp = NULL;
+
 static NSMutableArray<MDMActionContext *> *sActionContext = nil;
 
 @implementation MDMImplicitAction
@@ -74,9 +75,9 @@ - (void)addActionForLayer:(CALayer *)layer
 @interface MDMLayerDelegate: NSObject <CALayerDelegate>
 @end
 
-static id<CAAction> ActionForLayer(id self, SEL _cmd, CALayer *layer, NSString *event) {
+static id<CAAction> ActionForKey(CALayer *layer, SEL _cmd, NSString *event) {
   NSCAssert([NSStringFromSelector(_cmd) isEqualToString:
-             NSStringFromSelector(@selector(actionForLayer:forKey:))],
+                NSStringFromSelector(@selector(actionForKey:))],
             @"Invalid method signature.");
 
   MDMActionContext *context = [sActionContext lastObject];
@@ -85,8 +86,8 @@ @interface MDMLayerDelegate: NSObject <CALayerDelegate>
   if (context == nil) {
     // Graceful handling of invalid state on non-debug builds for if our context is nil invokes our
     // original implementation:
-    return ((id<CAAction>(*)(id, SEL, CALayer *, NSString *))sOriginalActionForLayerImp)
-              (self, _cmd, layer, event);
+    return ((id<CAAction>(*)(id, SEL, NSString *))sOriginalActionForKeyLayerImp)
+              (layer, _cmd, event);
   }
 
   // We don't have access to the "to" value of our animation here, so we unfortunately can't
@@ -103,33 +104,33 @@ @interface MDMLayerDelegate: NSObject <CALayerDelegate>
     return nil;
   }
 
+  SEL actionForKeySelector = @selector(actionForKey:);
+  Method actionForKeyMethod = class_getInstanceMethod([CALayer class], actionForKeySelector);
+
   // This method can be called recursively, so we maintain a context stack in the scope of this
   // method. Note that this is absolutely not thread safe, but neither is Core Animation.
   if (!sActionContext) {
     sActionContext = [NSMutableArray array];
-  }
-  [sActionContext addObject:[[MDMActionContext alloc] init]];
 
-  SEL selector = @selector(actionForLayer:forKey:);
-  Method method = class_getInstanceMethod([UIView class], selector);
-
-  if (sOriginalActionForLayerImp == nil) {
-    // Swap the original UIView implementation with our own so that we can intercept all
-    // actionForLayer:forKey: events.
-    sOriginalActionForLayerImp = method_setImplementation(method, (IMP)ActionForLayer);
+    // Swap the original CALayer implementation with our own so that we can intercept all
+    // actionForKey: events.
+    sOriginalActionForKeyLayerImp = method_setImplementation(actionForKeyMethod,
+                                                             (IMP)ActionForKey);
   }
 
+  [sActionContext addObject:[[MDMActionContext alloc] init]];
+
   work();
 
   // Return any intercepted actions we received during the invocation of work.
   MDMActionContext *context = [sActionContext lastObject];
   [sActionContext removeLastObject];
 
   if ([sActionContext count] == 0) {
-    // Restore our original method if we've emptied the stack:
-    method_setImplementation(method, sOriginalActionForLayerImp);
+    // Restore our original method if we've emptied the stack.
+    method_setImplementation(actionForKeyMethod, sOriginalActionForKeyLayerImp);
+    sOriginalActionForKeyLayerImp = nil;
 
-    sOriginalActionForLayerImp = nil;
     sActionContext = nil;
   }
 
@@ -140,10 +141,10 @@ @implementation MDMLayerDelegate
 
 - (id<CAAction>)actionForLayer:(CALayer *)layer forKey:(NSString *)event {
   // Check whether we're inside of an MDMAnimateImplicitly block or not.
-  if (sOriginalActionForLayerImp == nil) {
+  if (sOriginalActionForKeyLayerImp == nil) {
     return nil; // Tell Core Animation to Keep searching for an action provider.
   }
-  return ActionForLayer(layer, _cmd, layer, event);
+  return ActionForKey(layer, _cmd, event);
 }
 
 @end

tests/unit/HeadlessLayerImplicitAnimationTests.swift

@@ -67,7 +67,9 @@ class HeadlessLayerImplicitAnimationTests: XCTestCase {
     layer.opacity = 0.5
     CATransaction.commit()
 
-    XCTAssertEqual(layer.animationKeys()!, ["opacity"])
+    let animation = layer.animation(forKey: "opacity") as! CABasicAnimation
+    XCTAssertEqual(animation.keyPath, "opacity")
+    XCTAssertEqual(animation.duration, 0.5)
   }
 
   func testDoesNotImplicitlyAnimateInCATransactionWithActionsDisabled() {
@@ -80,12 +82,43 @@ class HeadlessLayerImplicitAnimationTests: XCTestCase {
     XCTAssertNil(layer.animationKeys())
   }
 
+  func testCATransactionTimingTakesPrecedenceOverUIViewTimingInside() {
+    UIView.animate(withDuration: 0.5) {
+      CATransaction.begin()
+      CATransaction.setAnimationDuration(0.2)
+      self.layer.opacity = 0.5
+      CATransaction.commit()
+    }
+
+    let animation = layer.animation(forKey: "opacity") as! CABasicAnimation
+    XCTAssertEqual(animation.keyPath, "opacity")
+    XCTAssertEqual(animation.duration, 0.2)
+  }
+
+  // Verifies the somewhat counter-intuitive fact that CATransaction's animation duration always
+  // takes precedence over UIView's animation duration. This means that animating a headless layer
+  // using UIView animation APIs may not result in the expected timings.
+  func testCATransactionTimingTakesPrecedenceOverUIViewTimingOutside() {
+    CATransaction.begin()
+    CATransaction.setAnimationDuration(0.2)
+    UIView.animate(withDuration: 0.5) {
+      self.layer.opacity = 0.5
+    }
+    CATransaction.commit()
+
+    let animation = layer.animation(forKey: "opacity") as! CABasicAnimation
+    XCTAssertEqual(animation.keyPath, "opacity")
+    XCTAssertEqual(animation.duration, 0.2)
+  }
+
   func testDoesImplicitlyAnimateInUIViewAnimateBlock() {
     UIView.animate(withDuration: 0.5) {
       self.layer.opacity = 0.5
     }
 
-    XCTAssertEqual(layer.animationKeys()!, ["opacity"])
+    let animation = layer.animation(forKey: "opacity") as! CABasicAnimation
+    XCTAssertEqual(animation.keyPath, "opacity")
+    XCTAssertEqual(animation.duration, CATransaction.animationDuration())
   }
 
   func testDoesNotImplicitlyAnimateInUIViewAnimateBlockWithActionsDisabledInside() {
@@ -110,8 +143,27 @@ class HeadlessLayerImplicitAnimationTests: XCTestCase {
     XCTAssertNil(layer.animationKeys())
   }
 
+  func testAnimatorTimingTakesPrecedenceOverCATransactionTiming() {
+    let animator = MotionAnimator()
+    animator.additive = false
+    let timing = MotionTiming(delay: 0,
+                              duration: 1,
+                              curve: MotionCurveMakeBezier(p1x: 0, p1y: 0, p2x: 0, p2y: 0),
+                              repetition: .init(type: .none, amount: 0, autoreverses: false))
+
+    animator.animate(with: timing) {
+      self.layer.opacity = 0.5
+    }
+
+    let animation = layer.animation(forKey: "opacity") as! CABasicAnimation
+    XCTAssertEqual(animation.keyPath, "opacity")
+    XCTAssertEqual(animation.duration, timing.duration)
+  }
+
+  // MARK: Deprecated tests.
+
+  @available(*, deprecated)
   func testDoesImplicitlyAnimateInCATransactionWithLayerDelegateAlone() {
-    // Delegate will allow us to do implicit animations, but only via the motion animator.
     layer.delegate = MotionAnimator.sharedLayerDelegate()
 
     CATransaction.begin()
@@ -122,8 +174,8 @@ class HeadlessLayerImplicitAnimationTests: XCTestCase {
     XCTAssertEqual(layer.animationKeys()!, ["opacity"])
   }
 
+  @available(*, deprecated)
   func testDoesNotImplicitlyAnimateInCATransactionWithLayerDelegateAloneAndActionsAreDisabled() {
-    // Delegate will allow us to do implicit animations, but only via the motion animator.
     layer.delegate = MotionAnimator.sharedLayerDelegate()
 
     CATransaction.begin()
@@ -135,8 +187,8 @@ class HeadlessLayerImplicitAnimationTests: XCTestCase {
     XCTAssertNil(layer.animationKeys())
   }
 
+  @available(*, deprecated)
   func testDoesImplicitlyAnimateInUIViewAnimateBlockWithLayerDelegateAlone() {
-    // Delegate will allow us to do implicit animations, but only via the motion animator.
     layer.delegate = MotionAnimator.sharedLayerDelegate()
 
     UIView.animate(withDuration: 0.5) {
@@ -146,6 +198,7 @@ class HeadlessLayerImplicitAnimationTests: XCTestCase {
     XCTAssertEqual(layer.animationKeys()!, ["opacity"])
   }
 
+  @available(*, deprecated)
   func testDoesImplicitlyAnimateWithLayerDelegateAndAnimator() {
     layer.delegate = MotionAnimator.sharedLayerDelegate()
 

tests/unit/ImplicitAnimationTests.swift

@@ -53,12 +53,12 @@ class ImplicitAnimationTests: XCTestCase {
     CATransaction.flush()
 
     originalImplementation =
-      class_getMethodImplementation(UIView.self, #selector(UIView.action(for:forKey:)))
+      class_getMethodImplementation(CALayer.self, #selector(CALayer.action(forKey:)))
   }
 
   override func tearDown() {
     let implementation =
-        class_getMethodImplementation(UIView.self, #selector(UIView.action(for:forKey:)))
+        class_getMethodImplementation(CALayer.self, #selector(CALayer.action(forKey:)))
     XCTAssertEqual(originalImplementation, implementation)
 
     animator = nil