Merge pull request #192 from RamblinWreck77/master

Thread Safety, closure captures, debug-only FatalErrors, self-self aliasing, misc tweaks

Sources/Pageboy/PageboyViewController+Management.swift

@@ -38,13 +38,17 @@ public extension PageboyViewController {
                 return
         }
 
-        updateViewControllers(to: [viewController], animated: false, async: false, force: false) { [weak self] _ in
-            self?.currentIndex = defaultIndex
-            if let self = self {
-                self.delegate?.pageboyViewController(self,
-                                                     didReloadWith: viewController,
-                                                     currentPageIndex: defaultIndex)
+        updateViewControllers(to: [viewController], animated: false, async: false, force: false) { [weak self, defaultIndex, viewController] _ in
+
+            guard let hasSelf = self else {
+                /// Self DNE
+                return
             }
+
+            hasSelf.currentIndex = defaultIndex
+            hasSelf.delegate?.pageboyViewController(hasSelf,
+                                                    didReloadWith: viewController,
+                                                    currentPageIndex: defaultIndex)
         }
     }
 
@@ -67,7 +71,7 @@ public extension PageboyViewController {
 
 // MARK: - VC Updating
 internal extension PageboyViewController {
-    
+
     func updateViewControllers(to viewControllers: [UIViewController],
                                from fromIndex: PageIndex = 0,
                                to toIndex: PageIndex = 0,
@@ -76,6 +80,41 @@ internal extension PageboyViewController {
                                async: Bool,
                                force: Bool,
                                completion: TransitionOperation.Completion?) {
+
+        if Thread.isMainThread {
+            _updateViewControllers(to: viewControllers,
+                                   from: fromIndex,
+                                   to: toIndex,
+                                   direction: direction,
+                                   animated: animated,
+                                   async: async, force:
+                                   force,
+                                   completion: completion)
+        } else {
+            DispatchQueue.main.sync {
+                _updateViewControllers(to: viewControllers,
+                                       from: fromIndex,
+                                       to: toIndex,
+                                       direction: direction,
+                                       animated: animated,
+                                       async: async,
+                                       force: force,
+                                       completion: completion)
+            }
+        }
+    }
+
+    private func _updateViewControllers(to viewControllers: [UIViewController],
+                               from fromIndex: PageIndex = 0,
+                               to toIndex: PageIndex = 0,
+                               direction: NavigationDirection = .forward,
+                               animated: Bool,
+                               async: Bool,
+                               force: Bool,
+                               completion: TransitionOperation.Completion?) {
+
+        assert(Thread.isMainThread)
+
         guard let pageViewController = pageViewController else {
             return
         }
@@ -98,21 +137,25 @@ internal extension PageboyViewController {
 
         // if not using a custom transition then animate using UIPageViewController mechanism
         let animateUpdate = animated ? !isUsingCustomTransition : false
-        let updateBlock = { [weak self] in
-            guard let self = self else {
+
+        let updateBlock = { [weak self, direction, animateUpdate, viewControllers, animated, isUsingCustomTransition] in
+            guard let hasSelf = self else {
                 return
             }
             pageViewController.setViewControllers(viewControllers,
-                                                  direction: direction.layoutNormalized(isRtL: self.view.layoutIsRightToLeft).rawValue,
+                                                  direction: direction.layoutNormalized(isRtL: hasSelf.view.layoutIsRightToLeft).rawValue,
                                                   animated: animateUpdate,
-                                                  completion:
-                { (finished) in
-                    self.isUpdatingViewControllers = false
-
-                    if !animated || !isUsingCustomTransition {
-                        completion?(finished)
-                    }
-            })
+                                                  completion: { [weak self, animated, isUsingCustomTransition, completion] (finished) in
+
+                                                        guard let hasSelf = self else {
+                                                            return
+                                                        }
+                                                        hasSelf.isUpdatingViewControllers = false
+
+                                                        if !animated || !isUsingCustomTransition {
+                                                            completion?(finished)
+                                                        }
+                                                  })
         }
 
         // Attempt to fix issue where fast scrolling causes crash.

Sources/Pageboy/PageboyViewController+Updating.swift

@@ -34,7 +34,7 @@ internal extension PageboyViewController {
         }
 
         if newIndex == currentIndex {
-            pageViewController?.view.crossDissolve(during: { [weak self] in
+            pageViewController?.view.crossDissolve(during: { [weak self, viewController] in
                 self?.updateViewControllers(to: [viewController],
                                             animated: false,
                                             async: true,
@@ -50,7 +50,7 @@ internal extension PageboyViewController {
                     return
                 }
 
-                updateViewControllers(to: [currentViewController], animated: false, async: true, force: false, completion: { [weak self] _ in
+                updateViewControllers(to: [currentViewController], animated: false, async: true, force: false, completion: { [weak self, newIndex, updateBehavior] _ in
                     self?.performScrollUpdate(to: newIndex, behavior: updateBehavior)
                 })
             } else { // Otherwise just perform scroll update
@@ -80,7 +80,7 @@ extension PageboyViewController {
         case .scrollTo(let index):
             scrollToPage(.at(index: index), animated: true)
 
-        default:()
+        default: break
         }
     }
 

Sources/Pageboy/PageboyViewController.swift

@@ -191,9 +191,11 @@ open class PageboyViewController: UIViewController {
 
         // If we are not at expected index when we appear - force a move to the expected index.
         if let expectedTransitionIndex = expectedTransitionIndex, expectedTransitionIndex != currentIndex {
-            scrollToPage(.at(index: expectedTransitionIndex),
-                         animated: false,
-                         force: true)
+
+            // Note: viewWillAppear is always called on main thread
+            _scrollToPage(.at(index: expectedTransitionIndex),
+                          animated: false,
+                          force: true)
         }
     }
 
@@ -219,7 +221,7 @@ open class PageboyViewController: UIViewController {
 
         // ignore scroll updates during orientation change
         pageViewController?.scrollView?.delegate = nil
-        coordinator.animate(alongsideTransition: nil) { [weak self] (_) in
+        coordinator.animate(alongsideTransition: nil) { [weak self] _ in
             self?.pageViewController?.scrollView?.delegate = self
         }
     }
@@ -304,10 +306,21 @@ public extension PageboyViewController {
     public func scrollToPage(_ page: Page,
                              animated: Bool,
                              completion: PageScrollCompletion? = nil) -> Bool {
-        return scrollToPage(page,
-                            animated: animated,
-                            force: false,
-                            completion: completion)
+        if Thread.isMainThread {
+            return _scrollToPage(page,
+                                 animated: animated,
+                                 force: false,
+                                 completion: completion)
+        } else {
+            var result: Bool = false
+            DispatchQueue.main.sync {
+                result = _scrollToPage(page,
+                                       animated: animated,
+                                       force: false,
+                                       completion: completion)
+            }
+            return result
+        }
     }
 
     /// Scroll the page view controller to a new page.
@@ -318,10 +331,13 @@ public extension PageboyViewController {
     /// - parameter completion: The completion closure.
     /// - Returns: Whether the scroll was executed.
     @discardableResult
-    internal func scrollToPage(_ page: Page,
+    private func _scrollToPage(_ page: Page,
                                animated: Bool,
                                force: Bool,
                                completion: PageScrollCompletion? = nil) -> Bool {
+
+        assert(Thread.isMainThread)
+
         guard let pageViewController = pageViewController, isSafeToScrollToANewPage(ignoringPosition: force) else {
             return false
         }
@@ -341,29 +357,33 @@ public extension PageboyViewController {
             isScrollingAnimated = animated
             let direction = NavigationDirection.forPageScroll(to: page, at: rawIndex, in: self)
 
-            let transitionCompletion: TransitionOperation.Completion = { [weak self] (finished) in
+            let transitionCompletion: TransitionOperation.Completion = { [weak self, rawIndex, direction, animated] (finished) in
+
+                guard let hasSelf = self else {
+                    /// Self DNE
+                    return
+                }
+
                 if finished {
-                    let isVertical = self?.navigationOrientation == .vertical
+                    let isVertical = hasSelf.navigationOrientation == .vertical
                     let currentPosition = CGPoint(x: isVertical ? 0.0 : CGFloat(rawIndex),
                                                   y: isVertical ? CGFloat(rawIndex) : 0.0)
-                    self?.currentPosition = currentPosition
-                    self?.currentIndex = rawIndex
+                    hasSelf.currentPosition = currentPosition
+                    hasSelf.currentIndex = rawIndex
 
                     // if not animated call position delegate update manually
                     if !animated {
-                        if let self = self {
-                            self.delegate?.pageboyViewController(self,
-                                                                 didScrollTo: currentPosition,
-                                                                 direction: direction,
-                                                                 animated: animated)
-                        }
-                        self?.expectedTransitionIndex = nil
+                        hasSelf.delegate?.pageboyViewController(hasSelf,
+                                                                didScrollTo: currentPosition,
+                                                                direction: direction,
+                                                                animated: animated)
+                        hasSelf.expectedTransitionIndex = nil
                     }
                 }
 
-                self?.autoScroller.didFinishScrollIfEnabled()
+                hasSelf.autoScroller.didFinishScrollIfEnabled()
                 completion?(viewController, animated, finished)
-                self?.isScrollingAnimated = false
+                hasSelf.isScrollingAnimated = false
             }
 
             updateViewControllers(to: [viewController],

Sources/Pageboy/Transitioning/PageboyViewController+Transitioning.swift

@@ -90,12 +90,20 @@ internal extension PageboyViewController {
                                     with direction: NavigationDirection,
                                     animated: Bool,
                                     completion: @escaping TransitionOperation.Completion) {
+
+        /// Note: This is (currently) only called from _updateViewControllers which is already on the main thread
+        assert(Thread.isMainThread)
+
         guard let transition = transition, animated == true, activeTransitionOperation == nil else {
                 completion(false)
                 return
         }
         guard let scrollView = pageViewController?.scrollView else {
+            #if DEBUG
             fatalError("Can't find UIPageViewController scroll view")
+            #else
+            return
+            #endif
         }
 
         prepareForTransition()
@@ -156,14 +164,14 @@ extension PageboyViewController: TransitionOperationDelegate {
     }
 }
 
-internal extension PageboyViewController.Transition {
+internal extension CATransition {
 
-    func configure(transition: inout CATransition) {
-        transition.duration = duration
+    func configure(from: PageboyViewController.Transition) {
+        duration = from.duration
         #if swift(>=4.2)
-        transition.type = CATransitionType(rawValue: style.rawValue)
+        type = CATransitionType(rawValue: from.style.rawValue)
         #else
-        transition.type = style.rawValue
+        type = from.style.rawValue
         #endif
     }
 }

Sources/Pageboy/Transitioning/TransitionOperation.swift

@@ -77,14 +77,16 @@ internal class TransitionOperation: NSObject, CAAnimationDelegate {
     init(for transition: PageboyViewController.Transition,
          action: Action,
          delegate: TransitionOperationDelegate) {
-        self.transition = transition
+
         self.action = action
         self.delegate = delegate
+        self.transition = transition
 
-        var animation = CATransition()
+        let animation = CATransition()
         animation.startProgress = 0.0
         animation.endProgress = 1.0
-        transition.configure(transition: &animation)
+        animation.configure(from: transition)
+
         animation.subtype = action.transitionSubType
         #if swift(>=4.2)
         animation.fillMode = .backwards

Sources/Pageboy/Utilities/Extensions/UIView+AutoLayout.swift

@@ -12,7 +12,15 @@ internal extension UIView {
 
     @discardableResult
     func pinToSuperviewEdges(priority: UILayoutPriority = .required) -> [NSLayoutConstraint] {
-        let superview = guardForSuperview()
+        guard let superview = guardForSuperview() else {
+            #if DEBUG
+            fatalError("Could not fetch superview in pinToSuperviewEdges")
+            #else
+            return [NSLayoutConstraint]()
+            #endif
+        }
+
+
 
         return addConstraints(priority: priority, { () -> [NSLayoutConstraint] in
             return [
@@ -38,14 +46,18 @@ internal extension UIView {
         })
 
         guard let constraint = constraints.first else {
+            #if DEBUG
             fatalError("Could not add matchWidth constraint")
+            #else
+            return nil
+            #endif
         }
         return constraint
     }
 
     @discardableResult
     func matchHeight(to view: UIView,
-                     priority: UILayoutPriority = .required) -> NSLayoutConstraint {
+                     priority: UILayoutPriority = .required) -> NSLayoutConstraint? {
         let constraints = addConstraints(priority: priority, { () -> [NSLayoutConstraint] in
             return [NSLayoutConstraint(item: self,
                                        attribute: .height,
@@ -57,7 +69,11 @@ internal extension UIView {
         })
 
         guard let constraint = constraints.first else {
+            #if DEBUG
             fatalError("Could not add matchHeight constraint")
+            #else
+            return nil
+            #endif
         }
         return constraint
     }
@@ -79,9 +95,13 @@ internal extension UIView {
         return constraints
     }
 
-    private func guardForSuperview() -> UIView {
+    private func guardForSuperview() -> UIView? {
         guard let superview = superview else {
+            #if DEBUG
             fatalError("No superview for view \(self)")
+            #else
+            return nil
+            #endif
         }
         return superview
     }

Sources/Pageboy/Utilities/Extensions/UIView+Localization.swift

@@ -27,5 +27,5 @@ extension UIView {
         assert(Thread.isMainThread)
         return UIView.userInterfaceLayoutDirection(for: semanticContentAttribute)
     }
-    
+
 }