From c56cbbd4f9b00cbf52f89cf20a60f0df34bd1395 Mon Sep 17 00:00:00 2001 From: Tim Oliver Date: Sun, 12 Jul 2026 01:18:51 +0900 Subject: [PATCH 1/5] Stop the reset timer from retaining the crop view; handle touch cancellation The scheduled reset timer used the target/selector form, which retains its target, so dismissing the controller mid-adjustment kept the crop view and both full-size image copies alive until the timer fired. It now uses the block form with a weak reference and is invalidated on dealloc. Touch and gesture cancellation (incoming calls, system alerts) also restart the reset timer instead of leaving the view stuck in its editing appearance. --- .../TOCropViewController/Views/TOCropView.m | 33 +++++++++- .../TOCropViewControllerTests.m | 62 ++++++++++++++++++- .../xcschemes/CropViewController.xcscheme | 2 +- .../CropViewControllerExample.xcscheme | 2 +- .../xcschemes/TOCropViewController.xcscheme | 2 +- ...opViewControllerExample-Extension.xcscheme | 2 +- .../TOCropViewControllerExample.xcscheme | 2 +- .../TOCropViewControllerTests.xcscheme | 2 +- 8 files changed, 98 insertions(+), 9 deletions(-) diff --git a/Objective-C/TOCropViewController/Views/TOCropView.m b/Objective-C/TOCropViewController/Views/TOCropView.m index ce1476e6..0e9df79e 100644 --- a/Objective-C/TOCropViewController/Views/TOCropView.m +++ b/Objective-C/TOCropViewController/Views/TOCropView.m @@ -166,6 +166,7 @@ - (void)setup { self.scrollView.contentInsetAdjustmentBehavior = UIScrollViewContentInsetAdjustmentNever; self.scrollView.touchesBegan = ^{ [weakSelf startEditing]; }; self.scrollView.touchesEnded = ^{ [weakSelf startResetTimer]; }; + self.scrollView.touchesCancelled = ^{ [weakSelf handleScrollViewTouchCancellation]; }; // Background Image View self.backgroundImageView = [[UIImageView alloc] initWithImage:self.image]; @@ -232,6 +233,10 @@ - (void)setup { [self addGestureRecognizer:self.gridPanGestureRecognizer]; } +- (void)dealloc { + [_resetTimer invalidate]; +} + #pragma mark - View Layout - - (void)performInitialSetup { // Calling this more than once is potentially destructive @@ -797,7 +802,9 @@ - (void)gridPanGestureRecognized:(UIPanGestureRecognizer *)recognizer { self.tappedEdge = [self cropEdgeForPoint:self.panOriginPoint]; } - if (recognizer.state == UIGestureRecognizerStateEnded) { + if (recognizer.state == UIGestureRecognizerStateEnded || + recognizer.state == UIGestureRecognizerStateCancelled || + recognizer.state == UIGestureRecognizerStateFailed) { [self startResetTimer]; } @@ -840,7 +847,29 @@ - (void)startResetTimer { if (self.resetTimer) return; - self.resetTimer = [NSTimer scheduledTimerWithTimeInterval:self.cropAdjustingDelay target:self selector:@selector(timerTriggered) userInfo:nil repeats:NO]; + // Use a block-based timer so a pending reset doesn't retain this view + // beyond its natural lifetime (eg, when dismissed mid-adjustment) + __weak typeof(self) weakSelf = self; + self.resetTimer = [NSTimer scheduledTimerWithTimeInterval:self.cropAdjustingDelay + repeats:NO + block:^(NSTimer *timer) { + [weakSelf timerTriggered]; + }]; +} + +// A cancelled touch doesn't imply the interaction is over: the scroll view cancels the +// touches it delivered as a normal part of its own pan or pinch recognizer taking over, +// which happens with the user's finger still down. While one of those is underway the +// scroll view's delegate arms the timer for us once it finishes, so arming it here as +// well would trigger a reset mid-gesture. What's left is a touch cancelled while nothing +// is scrolling - an incoming call or system alert - which would otherwise leave the crop +// view stuck in its editing appearance. +- (void)handleScrollViewTouchCancellation { + if (self.scrollView.isDragging || self.scrollView.isZooming || self.scrollView.isDecelerating) { + return; + } + + [self startResetTimer]; } - (void)timerTriggered { diff --git a/Objective-C/TOCropViewControllerTests/TOCropViewControllerTests.m b/Objective-C/TOCropViewControllerTests/TOCropViewControllerTests.m index 3120a628..bf84cd1a 100644 --- a/Objective-C/TOCropViewControllerTests/TOCropViewControllerTests.m +++ b/Objective-C/TOCropViewControllerTests/TOCropViewControllerTests.m @@ -6,16 +6,42 @@ // Copyright (c) 2015 Tim Oliver. All rights reserved. // +#import #import #import +#import "TOCropScrollView.h" #import "TOCropViewController.h" -// Expose private state so the test can simulate an in-flight rotation +// Expose private state so tests can arm the reset timer, simulate an in-flight +// rotation, and drive the scroll view's touch hooks @interface TOCropView (UnitTests) +- (void)startResetTimer; @property (nonatomic, assign) BOOL rotateAnimationInProgress; +@property (nonatomic, strong, readonly) TOCropScrollView *scrollView; +@property (nonatomic, strong, readonly) NSTimer *resetTimer; @end +// UIScrollView won't report itself as dragging without a real touch sequence, so +// stand in for one while a block runs +@interface UIScrollView (TOCropTestDragging) +- (BOOL)to_test_alwaysDragging; +@end + +@implementation UIScrollView (TOCropTestDragging) +- (BOOL)to_test_alwaysDragging { + return YES; +} +@end + +static void TOCropRunWhileScrollViewIsDragging(void (^block)(void)) { + Method real = class_getInstanceMethod([UIScrollView class], @selector(isDragging)); + Method stub = class_getInstanceMethod([UIScrollView class], @selector(to_test_alwaysDragging)); + method_exchangeImplementations(real, stub); + block(); + method_exchangeImplementations(real, stub); +} + @interface TOCropViewControllerTests : XCTestCase @end @@ -114,6 +140,40 @@ - (void)testImageCropFrameStaysWithinImageBounds { } } +- (void)testResetTimerIsNotArmedWhenAPanCancelsTheTouch { + TOCropView *cropView = [self cropViewWithImageSize:(CGSize){40, 20}]; + + cropView.scrollView.touchesBegan(); + XCTAssertNil(cropView.resetTimer, @"editing should have cancelled any pending reset"); + + // UIScrollView cancels the touches it delivered as a normal part of its pan + // recogniser taking over, so this fires mid-gesture with the finger still down + TOCropRunWhileScrollViewIsDragging(^{ + cropView.scrollView.touchesCancelled(); + }); + XCTAssertNil(cropView.resetTimer, @"a pan taking over the touch must not arm the reset timer"); +} + +- (void)testResetTimerIsArmedWhenAnIdleTouchIsInterrupted { + TOCropView *cropView = [self cropViewWithImageSize:(CGSize){40, 20}]; + + // Nothing is scrolling, so this is a real interruption (incoming call, system + // alert) and the view would otherwise stay stuck in its editing appearance + cropView.scrollView.touchesBegan(); + cropView.scrollView.touchesCancelled(); + XCTAssertNotNil(cropView.resetTimer, @"an interrupted idle touch must arm the reset timer"); +} + +- (void)testCropViewIsReleasedWithPendingResetTimer { + __weak TOCropView *weakCropView = nil; + @autoreleasepool { + TOCropView *cropView = [self cropViewWithImageSize:(CGSize){40, 20}]; + [cropView startResetTimer]; + weakCropView = cropView; + } + XCTAssertNil(weakCropView); +} + - (void)testViewControllerInstance { // Create a basic image UIGraphicsImageRenderer *renderer = [[UIGraphicsImageRenderer alloc] initWithSize:CGSizeMake(10, 10)]; diff --git a/TOCropViewControllerExample.xcodeproj/xcshareddata/xcschemes/CropViewController.xcscheme b/TOCropViewControllerExample.xcodeproj/xcshareddata/xcschemes/CropViewController.xcscheme index 2c1b9216..3d278a32 100644 --- a/TOCropViewControllerExample.xcodeproj/xcshareddata/xcschemes/CropViewController.xcscheme +++ b/TOCropViewControllerExample.xcodeproj/xcshareddata/xcschemes/CropViewController.xcscheme @@ -1,6 +1,6 @@ Date: Sun, 12 Jul 2026 02:13:14 +0900 Subject: [PATCH 2/5] Stop the Swift delegate bridge from retaining the weak delegate The closure handlers captured the unwrapped delegate strongly, so the nominally weak delegate was retained for the controller's lifetime -- and in the common pattern where the delegate owns the controller, the resulting cycle leaked both, along with the full-resolution image. The closures now resolve the delegate through self at call time, which also stops a replaced delegate from continuing to receive callbacks. --- .../CropViewController.swift | 35 +++++++++++++------ 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/Swift/CropViewController/CropViewController.swift b/Swift/CropViewController/CropViewController.swift index 7bfabc25..7aa68750 100644 --- a/Swift/CropViewController/CropViewController.swift +++ b/Swift/CropViewController/CropViewController.swift @@ -667,39 +667,54 @@ extension CropViewController { return } + // Resolve the delegate through `self` at call time rather than capturing it here. + // Capturing it would strongly retain an object the `weak` delegate property + // promises not to retain (and would keep delivering callbacks to a replaced + // delegate). Closures for selectors the new delegate doesn't implement are + // cleared so the Objective-C layer's dismissal logic stays consistent. if delegate.responds(to: #selector((any CropViewControllerDelegate).cropViewControllerDidTapDone(_:))) { - self.onDidTapDone = { [weak self] in - guard let self else { return } - delegate.cropViewControllerDidTapDone?(self) + self.onDidTapDone = {[weak self] in + guard let strongSelf = self else { return } + strongSelf.delegate?.cropViewControllerDidTapDone?(strongSelf) } + } else { + self.onDidTapDone = nil } if delegate.responds(to: #selector((any CropViewControllerDelegate).cropViewController(_:didCropImageToRect:angle:))) { self.onDidCropImageToRect = {[weak self] rect, angle in guard let strongSelf = self else { return } - delegate.cropViewController!(strongSelf, didCropImageToRect: rect, angle: angle) + strongSelf.delegate?.cropViewController?(strongSelf, didCropImageToRect: rect, angle: angle) } + } else { + self.onDidCropImageToRect = nil } - + if delegate.responds(to: #selector((any CropViewControllerDelegate).cropViewController(_:didCropToImage:withRect:angle:))) { self.onDidCropToRect = {[weak self] image, rect, angle in guard let strongSelf = self else { return } - delegate.cropViewController!(strongSelf, didCropToImage: image, withRect: rect, angle: angle) + strongSelf.delegate?.cropViewController?(strongSelf, didCropToImage: image, withRect: rect, angle: angle) } + } else { + self.onDidCropToRect = nil } - + if delegate.responds(to: #selector((any CropViewControllerDelegate).cropViewController(_:didCropToCircularImage:withRect:angle:))) { self.onDidCropToCircleImage = {[weak self] image, rect, angle in guard let strongSelf = self else { return } - delegate.cropViewController!(strongSelf, didCropToCircularImage: image, withRect: rect, angle: angle) + strongSelf.delegate?.cropViewController?(strongSelf, didCropToCircularImage: image, withRect: rect, angle: angle) } + } else { + self.onDidCropToCircleImage = nil } - + if delegate.responds(to: #selector((any CropViewControllerDelegate).cropViewController(_:didFinishCancelled:))) { self.onDidFinishCancelled = {[weak self] finished in guard let strongSelf = self else { return } - delegate.cropViewController!(strongSelf, didFinishCancelled: finished) + strongSelf.delegate?.cropViewController?(strongSelf, didFinishCancelled: finished) } + } else { + self.onDidFinishCancelled = nil } } } From b35485b0fbcf00985c6800426c1df1975258bb07 Mon Sep 17 00:00:00 2001 From: Tim Oliver Date: Mon, 27 Jul 2026 02:18:43 +0900 Subject: [PATCH 3/5] Address review findings in the Swift delegate bridge - The bridging closures capture the delegate weakly, honouring the weak contract of the delegate property, and fall back to the default dismissal if it has been deallocated before a callback fires - Closures the host assigned directly are no longer cleared for selectors the delegate doesn't implement; the bridge tracks which slots it owns so that replacing the delegate tears down only its own - main's cropViewControllerDidTapDone bridge is folded into the same scheme. It gets no dismissal fallback, since it fires alongside the crop callbacks rather than instead of them, so those still own the dismissal --- .../CropViewController.swift | 97 ++++++++++++------- 1 file changed, 61 insertions(+), 36 deletions(-) diff --git a/Swift/CropViewController/CropViewController.swift b/Swift/CropViewController/CropViewController.swift index 7aa68750..0c3c3368 100644 --- a/Swift/CropViewController/CropViewController.swift +++ b/Swift/CropViewController/CropViewController.swift @@ -116,7 +116,18 @@ open class CropViewController: UIViewController, TOCropViewControllerDelegate { public weak var delegate: (any CropViewControllerDelegate)? { didSet { self.setUpDelegateHandlers() } } - + + /** + The callback slots that currently hold a delegate-bridging closure, so that + replacing the delegate can tear down its bridges without clobbering closures + the host assigned directly. + :nodoc: + */ + private enum BridgedCallback { + case didTapDone, didCropImageToRect, didCropToRect, didCropToCircleImage, didFinishCancelled + } + private var bridgedCallbacks: Set = [] + /** Set the title text that appears at the top of the view controller */ @@ -424,7 +435,7 @@ open class CropViewController: UIViewController, TOCropViewControllerDelegate { /** If true, button icons are visible in portairt instead button text. - Default is NO. + Default is NO. Has no effect on iOS 26 and up, where the toolbar is always icon-only. */ public var showOnlyIcons: Bool { set { toCropViewController.showOnlyIcons = newValue } @@ -658,63 +669,77 @@ extension CropViewController { } fileprivate func setUpDelegateHandlers() { - guard let delegate = self.delegate else { - onDidTapDone = nil - onDidCropToRect = nil - onDidCropImageToRect = nil - onDidCropToCircleImage = nil - onDidFinishCancelled = nil - return - } - - // Resolve the delegate through `self` at call time rather than capturing it here. - // Capturing it would strongly retain an object the `weak` delegate property - // promises not to retain (and would keep delivering callbacks to a replaced - // delegate). Closures for selectors the new delegate doesn't implement are - // cleared so the Objective-C layer's dismissal logic stays consistent. + // Tear down the bridges installed for the previous delegate, so that a delegate + // which has since been replaced stops receiving callbacks. Slots the host + // assigned a closure to directly were never ours, so they're left alone. + if bridgedCallbacks.contains(.didTapDone) { onDidTapDone = nil } + if bridgedCallbacks.contains(.didCropImageToRect) { onDidCropImageToRect = nil } + if bridgedCallbacks.contains(.didCropToRect) { onDidCropToRect = nil } + if bridgedCallbacks.contains(.didCropToCircleImage) { onDidCropToCircleImage = nil } + if bridgedCallbacks.contains(.didFinishCancelled) { onDidFinishCancelled = nil } + bridgedCallbacks.removeAll() + + guard let delegate = self.delegate else { return } + + // Install a bridging closure only for the selectors this delegate implements, + // leaving directly-assigned closures on the other slots untouched. The delegate + // is captured weakly to honor the `weak` contract of the delegate property; if + // it has been deallocated by the time a closure fires, fall back to the default + // dismissal so the controller can't be left stranded on screen. if delegate.responds(to: #selector((any CropViewControllerDelegate).cropViewControllerDidTapDone(_:))) { - self.onDidTapDone = {[weak self] in - guard let strongSelf = self else { return } - strongSelf.delegate?.cropViewControllerDidTapDone?(strongSelf) + bridgedCallbacks.insert(.didTapDone) + // No dismissal fallback for this one: it's fired alongside the crop + // callbacks rather than instead of them, so they own the dismissal. + self.onDidTapDone = {[weak self, weak delegate] in + guard let strongSelf = self, let delegate = delegate else { return } + delegate.cropViewControllerDidTapDone?(strongSelf) } - } else { - self.onDidTapDone = nil } if delegate.responds(to: #selector((any CropViewControllerDelegate).cropViewController(_:didCropImageToRect:angle:))) { - self.onDidCropImageToRect = {[weak self] rect, angle in + bridgedCallbacks.insert(.didCropImageToRect) + self.onDidCropImageToRect = {[weak self, weak delegate] rect, angle in guard let strongSelf = self else { return } - strongSelf.delegate?.cropViewController?(strongSelf, didCropImageToRect: rect, angle: angle) + guard let delegate = delegate else { strongSelf.performDefaultDismissal(); return } + delegate.cropViewController?(strongSelf, didCropImageToRect: rect, angle: angle) } - } else { - self.onDidCropImageToRect = nil } if delegate.responds(to: #selector((any CropViewControllerDelegate).cropViewController(_:didCropToImage:withRect:angle:))) { - self.onDidCropToRect = {[weak self] image, rect, angle in + bridgedCallbacks.insert(.didCropToRect) + self.onDidCropToRect = {[weak self, weak delegate] image, rect, angle in guard let strongSelf = self else { return } - strongSelf.delegate?.cropViewController?(strongSelf, didCropToImage: image, withRect: rect, angle: angle) + guard let delegate = delegate else { strongSelf.performDefaultDismissal(); return } + delegate.cropViewController?(strongSelf, didCropToImage: image, withRect: rect, angle: angle) } - } else { - self.onDidCropToRect = nil } if delegate.responds(to: #selector((any CropViewControllerDelegate).cropViewController(_:didCropToCircularImage:withRect:angle:))) { - self.onDidCropToCircleImage = {[weak self] image, rect, angle in + bridgedCallbacks.insert(.didCropToCircleImage) + self.onDidCropToCircleImage = {[weak self, weak delegate] image, rect, angle in guard let strongSelf = self else { return } - strongSelf.delegate?.cropViewController?(strongSelf, didCropToCircularImage: image, withRect: rect, angle: angle) + guard let delegate = delegate else { strongSelf.performDefaultDismissal(); return } + delegate.cropViewController?(strongSelf, didCropToCircularImage: image, withRect: rect, angle: angle) } - } else { - self.onDidCropToCircleImage = nil } if delegate.responds(to: #selector((any CropViewControllerDelegate).cropViewController(_:didFinishCancelled:))) { - self.onDidFinishCancelled = {[weak self] finished in + bridgedCallbacks.insert(.didFinishCancelled) + self.onDidFinishCancelled = {[weak self, weak delegate] finished in guard let strongSelf = self else { return } - strongSelf.delegate?.cropViewController?(strongSelf, didFinishCancelled: finished) + guard let delegate = delegate else { strongSelf.performDefaultDismissal(); return } + delegate.cropViewController?(strongSelf, didFinishCancelled: finished) } + } + } + + /// Mirrors the Objective-C layer's default dismissal behavior, for when a bridged + /// delegate callback fires after the delegate itself has gone away. + fileprivate func performDefaultDismissal() { + if let navigationController = self.navigationController, navigationController.viewControllers.count > 1 { + navigationController.popViewController(animated: true) } else { - self.onDidFinishCancelled = nil + presentingViewController?.dismiss(animated: true) } } } From 09c1ea49e35862027593ab03a3ab776a4e11337f Mon Sep 17 00:00:00 2001 From: Tim Oliver Date: Mon, 27 Jul 2026 00:26:28 +0900 Subject: [PATCH 4/5] Add test coverage for the Swift CropViewController wrapper `CropViewController.swift` had no automated coverage at all, so the closure bridge it maintains over the Objective-C delegate -- the part of this library with the most intricate lifetime rules, and the source of two of the bugs fixed in 3.1.2 -- was only ever verified by hand. Rather than introduce a second test target and a second CI scheme, the existing TOCropViewControllerTests bundle now also compiles CropViewController.swift and a Swift test class, which is how both example apps consume the wrapper: sources compiled in directly, with the Objective-C layer exposed through a bridging header. `fastlane test` picks the new cases up with no change to the workflow. Eleven cases, covering: per-selector installation of the bridging closures; tear-down when the delegate is replaced or cleared; survival of closures the host assigned directly; the weak delegate not being retained; tolerance of a delegate that has been deallocated; forwarding of the ~20 wrapped properties to the underlying controller; and the child view controller relationship. Reverting the delegate-bridge fix from the previous commit fails three assertions across two of them, so they do bind to the behaviour they cover. One thing the exercise turned up but which isn't fixed here: TOCropView.h forward-declares TOCropOverlayView rather than importing it, and Swift drops properties whose class is incomplete, so `cropView.gridOverlayView` is invisible to Swift wherever these headers are consumed without a module. Importing it from the public header breaks the framework target's module verifier, because that target publishes no headers at all -- untangling that is its own change, so the test bridging header imports it locally for now. --- .../CropViewControllerTests-Bridging-Header.h | 16 ++ .../CropViewControllerTests.swift | 221 ++++++++++++++++++ .../project.pbxproj | 20 ++ 3 files changed, 257 insertions(+) create mode 100644 Swift/CropViewControllerTests/CropViewControllerTests-Bridging-Header.h create mode 100644 Swift/CropViewControllerTests/CropViewControllerTests.swift diff --git a/Swift/CropViewControllerTests/CropViewControllerTests-Bridging-Header.h b/Swift/CropViewControllerTests/CropViewControllerTests-Bridging-Header.h new file mode 100644 index 00000000..1ebcb0b8 --- /dev/null +++ b/Swift/CropViewControllerTests/CropViewControllerTests-Bridging-Header.h @@ -0,0 +1,16 @@ +// +// CropViewControllerTests-Bridging-Header.h +// CropViewControllerTests +// +// Copyright 2017-2025 Timothy Oliver. All rights reserved. +// +// The test bundle compiles CropViewController.swift directly, rather than importing a +// built module, matching how the example apps consume it. That means the Objective-C +// layer it wraps has to be exposed to Swift through a bridging header. +// + +#import "TOCropViewController.h" + +// TOCropView.h only forward-declares TOCropOverlayView, and Swift drops properties +// whose class is incomplete, so `cropView.gridOverlayView` is invisible without this. +#import "TOCropOverlayView.h" diff --git a/Swift/CropViewControllerTests/CropViewControllerTests.swift b/Swift/CropViewControllerTests/CropViewControllerTests.swift new file mode 100644 index 00000000..ae391fda --- /dev/null +++ b/Swift/CropViewControllerTests/CropViewControllerTests.swift @@ -0,0 +1,221 @@ +// +// CropViewControllerTests.swift +// CropViewControllerTests +// +// Copyright 2017-2025 Timothy Oliver. All rights reserved. +// + +import XCTest + +#if canImport(TOCropViewController) +import TOCropViewController +#endif + +// Delegates implementing different subsets of the optional protocol methods, so that +// the per-selector behaviour of the closure bridge can be observed. + +private class ImageCroppingDelegate: NSObject, CropViewControllerDelegate { + var receivedImageCrops = 0 + func cropViewController(_ cropViewController: CropViewController, didCropToImage image: UIImage, + withRect cropRect: CGRect, angle: Int) { + receivedImageCrops += 1 + } +} + +private class CancellationDelegate: NSObject, CropViewControllerDelegate { + var receivedCancellations = 0 + func cropViewController(_ cropViewController: CropViewController, didFinishCancelled cancelled: Bool) { + receivedCancellations += 1 + } +} + +private class DoneTapDelegate: NSObject, CropViewControllerDelegate { + var receivedDoneTaps = 0 + func cropViewControllerDidTapDone(_ cropViewController: CropViewController) { + receivedDoneTaps += 1 + } +} + +/// A delegate that owns its controller, which is the arrangement that turns a strongly +/// captured delegate into a leak of both objects (and the full-resolution image). +private class OwningDelegate: NSObject, CropViewControllerDelegate { + var cropViewController: CropViewController? + func cropViewController(_ cropViewController: CropViewController, didFinishCancelled cancelled: Bool) {} +} + +@MainActor +final class CropViewControllerTests: XCTestCase { + + // MARK: - Helpers - + + private func testImage(size: CGSize = CGSize(width: 40, height: 20)) -> UIImage { + UIGraphicsImageRenderer(size: size).image { context in + UIColor.red.setFill() + context.fill(CGRect(origin: .zero, size: size)) + } + } + + private func makeController() -> CropViewController { + CropViewController(image: testImage()) + } + + // MARK: - Delegate Bridging - + + func testBridgeOnlyInstallsClosuresForImplementedSelectors() { + let controller = makeController() + let delegate = ImageCroppingDelegate() + controller.delegate = delegate + + XCTAssertNotNil(controller.onDidCropToRect) + XCTAssertNil(controller.onDidCropImageToRect) + XCTAssertNil(controller.onDidCropToCircleImage) + XCTAssertNil(controller.onDidFinishCancelled) + XCTAssertNil(controller.onDidTapDone) + } + + func testDoneTapBridgeIsInstalledAndTornDown() { + let controller = makeController() + let doneDelegate = DoneTapDelegate() + controller.delegate = doneDelegate + XCTAssertNotNil(controller.onDidTapDone) + + controller.onDidTapDone?() + XCTAssertEqual(doneDelegate.receivedDoneTaps, 1) + + // Replacing it with a delegate that doesn't implement the selector must + // release the slot, so the old delegate stops being notified + controller.delegate = CancellationDelegate() + XCTAssertNil(controller.onDidTapDone) + XCTAssertEqual(doneDelegate.receivedDoneTaps, 1) + } + + func testDoneTapBridgeIsANoOpOnceTheDelegateIsGone() { + let controller = makeController() + autoreleasepool { controller.delegate = DoneTapDelegate() } + + // Unlike the crop callbacks, this one fires *alongside* them rather than + // instead of them, so a deallocated delegate must not trigger a dismissal + // that the crop callbacks are also going to perform + XCTAssertNotNil(controller.onDidTapDone) + XCTAssertNoThrow(controller.onDidTapDone?()) + } + + func testBridgedClosureReachesTheDelegate() { + let controller = makeController() + let delegate = ImageCroppingDelegate() + controller.delegate = delegate + + controller.onDidCropToRect?(testImage(), .zero, 0) + XCTAssertEqual(delegate.receivedImageCrops, 1) + } + + func testReplacingTheDelegateTearsDownItsBridges() { + let controller = makeController() + let imageDelegate = ImageCroppingDelegate() + controller.delegate = imageDelegate + XCTAssertNotNil(controller.onDidCropToRect) + + // The replacement implements a different selector, so the slot the previous + // delegate claimed must be released rather than left pointing at it + let cancelDelegate = CancellationDelegate() + controller.delegate = cancelDelegate + + XCTAssertNil(controller.onDidCropToRect) + XCTAssertNotNil(controller.onDidFinishCancelled) + + controller.onDidFinishCancelled?(true) + XCTAssertEqual(cancelDelegate.receivedCancellations, 1) + XCTAssertEqual(imageDelegate.receivedImageCrops, 0) + } + + func testClearingTheDelegateTearsDownItsBridges() { + let controller = makeController() + controller.delegate = CancellationDelegate() + XCTAssertNotNil(controller.onDidFinishCancelled) + + controller.delegate = nil + XCTAssertNil(controller.onDidFinishCancelled) + } + + func testDirectlyAssignedClosuresSurviveDelegateChanges() { + let controller = makeController() + var directCallCount = 0 + controller.onDidCropToRect = { _, _, _ in directCallCount += 1 } + + // This delegate doesn't implement didCropToImage, so it has no claim on that slot + let delegate = CancellationDelegate() + controller.delegate = delegate + XCTAssertNotNil(controller.onDidCropToRect) + + controller.delegate = nil + XCTAssertNotNil(controller.onDidCropToRect) + + controller.onDidCropToRect?(testImage(), .zero, 0) + XCTAssertEqual(directCallCount, 1) + } + + func testDelegateIsNotRetainedByTheBridge() { + weak var weakController: CropViewController? + weak var weakDelegate: OwningDelegate? + + autoreleasepool { + let delegate = OwningDelegate() + let controller = CropViewController(image: testImage()) + controller.delegate = delegate + + // Close the loop: the delegate owns the controller, as a presenting view + // controller typically would + delegate.cropViewController = controller + + weakController = controller + weakDelegate = delegate + } + + XCTAssertNil(weakController) + XCTAssertNil(weakDelegate) + } + + func testBridgedClosureToleratesADeallocatedDelegate() { + let controller = makeController() + autoreleasepool { + controller.delegate = ImageCroppingDelegate() + } + + // The delegate is gone but the closure is still installed; it should fall back + // to the default dismissal rather than trapping on a nil delegate + XCTAssertNotNil(controller.onDidCropToRect) + XCTAssertNoThrow(controller.onDidCropToRect?(testImage(), .zero, 0)) + } + + // MARK: - Property Forwarding - + + func testTitleForwardsToTheUnderlyingController() { + let controller = makeController() + XCTAssertNil(controller.titleLabel) + + controller.title = "Crop Your Photo" + XCTAssertEqual(controller.title, "Crop Your Photo") + XCTAssertEqual(controller.toCropViewController.title, "Crop Your Photo") + XCTAssertEqual(controller.titleLabel?.text, "Crop Your Photo") + } + + func testImageAndCroppingStyleComeFromTheInitialiser() { + let image = testImage() + + let defaultController = CropViewController(image: image) + XCTAssertIdentical(defaultController.image, image) + XCTAssertEqual(defaultController.croppingStyle, .default) + XCTAssertEqual(defaultController.cropView.croppingStyle, .default) + + let circularController = CropViewController(croppingStyle: .circular, image: image) + XCTAssertEqual(circularController.croppingStyle, .circular) + XCTAssertEqual(circularController.cropView.croppingStyle, .circular) + } + + func testChildViewControllerRelationshipIsEstablished() { + let controller = makeController() + XCTAssertTrue(controller.children.contains(controller.toCropViewController)) + XCTAssertIdentical(controller.childForStatusBarStyle, controller.toCropViewController) + XCTAssertIdentical(controller.childForStatusBarHidden, controller.toCropViewController) + } +} diff --git a/TOCropViewControllerExample.xcodeproj/project.pbxproj b/TOCropViewControllerExample.xcodeproj/project.pbxproj index 32853ee8..318eef04 100644 --- a/TOCropViewControllerExample.xcodeproj/project.pbxproj +++ b/TOCropViewControllerExample.xcodeproj/project.pbxproj @@ -56,6 +56,8 @@ 2238CF2A1FC0269C0081B957 /* Assets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 2238CF291FC0269C0081B957 /* Assets.xcassets */; }; 2238CF2D1FC0269C0081B957 /* LaunchScreen.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = 2238CF2B1FC0269C0081B957 /* LaunchScreen.storyboard */; }; 2238CF361FC029880081B957 /* CropViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2238CF351FC029880081B957 /* CropViewController.swift */; }; + CAFE0000000000000000B001 /* CropViewControllerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = CAFE0000000000000000A001 /* CropViewControllerTests.swift */; }; + CAFE0000000000000000B002 /* CropViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2238CF351FC029880081B957 /* CropViewController.swift */; }; 2238CF451FC02AE00081B957 /* TOCropViewController.h in Headers */ = {isa = PBXBuildFile; fileRef = 2238CF441FC02AE00081B957 /* TOCropViewController.h */; settings = {ATTRIBUTES = (Public, ); }; }; 223DCEB61FBAA85D00F99209 /* TOCropViewController.m in Sources */ = {isa = PBXBuildFile; fileRef = 22DB4D831B234CFA008B8466 /* TOCropViewController.m */; }; 22A105B32BC134A200DB3A80 /* PrivacyInfo.xcprivacy in Resources */ = {isa = PBXBuildFile; fileRef = 22A105B22BC134A200DB3A80 /* PrivacyInfo.xcprivacy */; }; @@ -177,6 +179,8 @@ 2238CF2C1FC0269C0081B957 /* Base */ = {isa = PBXFileReference; lastKnownFileType = file.storyboard; name = Base; path = Base.lproj/LaunchScreen.storyboard; sourceTree = ""; }; 2238CF2E1FC0269C0081B957 /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = ""; }; 2238CF351FC029880081B957 /* CropViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CropViewController.swift; sourceTree = ""; }; + CAFE0000000000000000A001 /* CropViewControllerTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CropViewControllerTests.swift; sourceTree = ""; }; + CAFE0000000000000000A002 /* CropViewControllerTests-Bridging-Header.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = "CropViewControllerTests-Bridging-Header.h"; sourceTree = ""; }; 2238CF3C1FC029A90081B957 /* CropViewController.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = CropViewController.framework; sourceTree = BUILT_PRODUCTS_DIR; }; 2238CF3E1FC029A90081B957 /* CropViewController.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = CropViewController.h; sourceTree = ""; }; 2238CF3F1FC029A90081B957 /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = ""; }; @@ -441,11 +445,21 @@ isa = PBXGroup; children = ( 2238CF341FC027550081B957 /* CropViewController */, + CAFE0000000000000000A003 /* CropViewControllerTests */, 2238CF211FC0269C0081B957 /* CropViewControllerExample */, ); path = Swift; sourceTree = ""; }; + CAFE0000000000000000A003 /* CropViewControllerTests */ = { + isa = PBXGroup; + children = ( + CAFE0000000000000000A001 /* CropViewControllerTests.swift */, + CAFE0000000000000000A002 /* CropViewControllerTests-Bridging-Header.h */, + ); + path = CropViewControllerTests; + sourceTree = ""; + }; 2238CF341FC027550081B957 /* CropViewController */ = { isa = PBXGroup; children = ( @@ -820,6 +834,8 @@ isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; files = ( + CAFE0000000000000000B002 /* CropViewController.swift in Sources */, + CAFE0000000000000000B001 /* CropViewControllerTests.swift in Sources */, 2206119C1B2E6777001A467B /* TOCropViewControllerTests.m in Sources */, ); runOnlyForDeploymentPostprocessing = 0; @@ -1092,6 +1108,8 @@ PRODUCT_NAME = "$(TARGET_NAME)"; SUPPORTED_PLATFORMS = "iphoneos iphonesimulator xros xrsimulator"; SUPPORTS_MACCATALYST = YES; + SWIFT_OBJC_BRIDGING_HEADER = "Swift/CropViewControllerTests/CropViewControllerTests-Bridging-Header.h"; + SWIFT_VERSION = 5.0; TARGETED_DEVICE_FAMILY = "1,2,7"; TEST_HOST = "$(BUILT_PRODUCTS_DIR)/TOCropViewControllerExample.app/TOCropViewControllerExample"; }; @@ -1114,6 +1132,8 @@ PRODUCT_NAME = "$(TARGET_NAME)"; SUPPORTED_PLATFORMS = "iphoneos iphonesimulator xros xrsimulator"; SUPPORTS_MACCATALYST = YES; + SWIFT_OBJC_BRIDGING_HEADER = "Swift/CropViewControllerTests/CropViewControllerTests-Bridging-Header.h"; + SWIFT_VERSION = 5.0; TARGETED_DEVICE_FAMILY = "1,2,7"; TEST_HOST = "$(BUILT_PRODUCTS_DIR)/TOCropViewControllerExample.app/TOCropViewControllerExample"; }; From 369a9b1bc9f8a9c8028af937774268534fc6bbb6 Mon Sep 17 00:00:00 2001 From: Tim Oliver Date: Tue, 28 Jul 2026 00:39:58 +0900 Subject: [PATCH 5/5] Note the object lifetime fixes in the changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35df2759..43049801 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,11 @@ x.y.z Release Notes (yyyy-MM-dd) - Zero-sized images and aspect ratios with a zero component no longer produce broken layout geometry. - `rotateImageNinetyDegreesAnimated:clockwise:completion:` never called its completion handler when `animated` was NO, or when the call was dropped because a rotation was already playing. The latter could leave the toolbar's rotation buttons permanently disabled. - The crop view was briefly assigned the frame for the opposite layout at the start of a device rotation, so the rotation animation interpolated from the wrong geometry (iOS 25 and below). +- The Swift `CropViewController`'s closure bridge strongly retained the nominally weak `delegate`, leaking the controller and image when the delegate owned the controller, and kept delivering callbacks to a replaced delegate. +- The crop adjustment reset timer retained the crop view (and its images) after dismissal mid-adjustment. +- Touch cancellation from incoming calls or system alerts left the crop view stuck in its editing appearance. +- Swift: replacing the `delegate` left the previous delegate receiving callbacks, and clearing it discarded closures the host had assigned directly. +- Swift: the `cropViewControllerDidTapDone` bridge added in 3.1.2 captured the delegate strongly, the same retain cycle as the other callbacks. 3.1.2 Release Notes (2026-04-07)