Fix retain cycles preventing objects from being deallocated after log…

…ging out (wordpress-mobile#21047)

* Set weak reference for BlogDashboardViewController in DashboardBlazeCardCell

* Weakly capture action closure in DashbordPagesListCardCell

* Pass weak closure instead of strongly captured method in DashboardPromptCardCell

* Break circular dependency cycle for BlogDashboardViewController in quick action cell

* Break circular dependency cycle for BlogDashboardViewController in DashboardStatsCardCell

* Capture BlogDashboardViewController weekly in BlogDashboardViewModel

* Break retain cycle by capturing presented view controllers weakly in MySiteViewController

* Weak self in NotificationsViewController

* Capture ReaderTabViewModel weakly in a closure

* Use weak closures instead of passing methods to a closure to avoid retain cycle

* Resolve memory leaks in RootViewCoordinator

Problem:
- RootViewCoordinator is a singleton
- RootViewCoordinator holds a strong reference to RootViewPresenter
- WPTabBarController implements RootViewPresenter and is presenter as rootViewController
- When WPTabBarController is dismissed, RootViewCoordinator continues to hold a strong reference to a WPTabBarController creating a memory leak

Solution:
- Move presentation logic from WindowsManager to RootViewCoordinator
- Only initialize RootViewPresenter when needed
- Release RootViewPresenter after logging out. Not making RootViewPresenter weak or unowned to avoid further cascading changes through the codebase to handle RootViewPresenter being optional
- RootViewPresenter should not be accessed before the root view is presented. If it happens - print a warning. All the issues should be addressed with further improvements.

* Update RELEASE-NOTES.txt

* Update RELEASE-NOTES.txt

* Added unit tests confirming deallocation of rootViewController after logout

- Made WordPressAuthenticator injectable
- Checking if rootViewController is deinitialized after presenting signInUI

* Update RELEASE-NOTES.txt

RELEASE-NOTES.txt

@@ -1,7 +1,8 @@
 22.9
 -----
+* [*] [internal] Fix multiple memory leaks after logging in and logging out. [#21047]
 * [**] Block editor: Move undo/redo buttons to the navigation bar. [#20930]
-* [*] Fixed an issue that caused the UI to be briefly unresponsive in certian case when opening the app. [#21065]
+* [*] Fixed an issue that caused the UI to be briefly unresponsive in certain case when opening the app. [#21065]
 
 22.8
 -----

WordPress/Classes/System/RootViewCoordinator.swift

@@ -1,4 +1,5 @@
 import Foundation
+import WordPressAuthenticator
 
 extension NSNotification.Name {
     static let WPAppUITypeChanged = NSNotification.Name(rawValue: "WPAppUITypeChanged")
@@ -19,7 +20,17 @@ class RootViewCoordinator {
     static let shared = RootViewCoordinator(featureFlagStore: RemoteFeatureFlagStore(),
                                             windowManager: WordPressAppDelegate.shared?.windowManager)
     static var sharedPresenter: RootViewPresenter {
-        shared.rootViewPresenter
+        guard let rootViewPresenter = shared.rootViewPresenter else {
+            /// Accessing RootViewPresenter before root view is presented is incorrect behavior
+            /// It shows either inconsistent order of app dependency initialization
+            /// or that RootViewPresenter contains actions unrelated to presented views
+            DDLogWarn("RootViewPresenter is accessed before root view is presented")
+            let rootViewPresenter = shared.createPresenter(shared.currentAppUIType)
+            shared.rootViewPresenter = rootViewPresenter
+            return rootViewPresenter
+        }
+
+        return rootViewPresenter
     }
 
     // MARK: Public Variables
@@ -34,33 +45,58 @@ class RootViewCoordinator {
 
     // MARK: Private instance variables
 
-    private(set) var rootViewPresenter: RootViewPresenter
+    private var rootViewPresenter: RootViewPresenter?
     private var currentAppUIType: AppUIType {
         didSet {
             updateJetpackFeaturesRemovalCoordinatorState()
         }
     }
     private var featureFlagStore: RemoteFeatureFlagStore
     private var windowManager: WindowManager?
+    private let wordPressAuthenticator: WordPressAuthenticatorProtocol.Type
 
     // MARK: Initializer
 
     init(featureFlagStore: RemoteFeatureFlagStore,
-         windowManager: WindowManager?) {
+         windowManager: WindowManager?,
+         wordPressAuthenticator: WordPressAuthenticatorProtocol.Type = WordPressAuthenticator.self) {
         self.featureFlagStore = featureFlagStore
         self.windowManager = windowManager
         self.currentAppUIType = Self.appUIType(featureFlagStore: featureFlagStore)
-        switch self.currentAppUIType {
+        self.wordPressAuthenticator = wordPressAuthenticator
+        updateJetpackFeaturesRemovalCoordinatorState()
+    }
+
+    // MARK: - Root Coordination
+
+    func showAppUI(animated: Bool = true, completion: (() -> Void)? = nil) {
+        let rootViewPresenter = createPresenter(currentAppUIType)
+        windowManager?.show(rootViewPresenter.rootViewController, animated: animated, completion: completion)
+        self.rootViewPresenter = rootViewPresenter
+
+        updatePromptsIfNeeded()
+    }
+
+    func showSignInUI(completion: (() -> Void)? = nil) {
+        guard let loginViewController = wordPressAuthenticator.loginUI() else {
+            fatalError("No login UI to show to the user.  There's no way to gracefully handle this error.")
+        }
+
+        windowManager?.show(loginViewController, completion: completion)
+        wordPressAuthenticator.track(.openedLogin)
+        self.rootViewPresenter = nil
+    }
+
+    private func createPresenter(_ appType: AppUIType) -> RootViewPresenter {
+        switch appType {
         case .normal:
-            self.rootViewPresenter = WPTabBarController(staticScreens: false)
+            return WPTabBarController(staticScreens: false)
         case .simplified:
             let meScenePresenter = MeScenePresenter()
-            self.rootViewPresenter = MySitesCoordinator(meScenePresenter: meScenePresenter, onBecomeActiveTab: {})
+            return MySitesCoordinator(meScenePresenter: meScenePresenter, onBecomeActiveTab: {})
         case .staticScreens:
-            self.rootViewPresenter = StaticScreensTabBarWrapper()
+            return StaticScreensTabBarWrapper()
         }
-        updateJetpackFeaturesRemovalCoordinatorState()
-        updatePromptsIfNeeded()
     }
 
     // MARK: JP Features State
@@ -122,15 +158,6 @@ class RootViewCoordinator {
     }
 
     private func reloadUI(using windowManager: WindowManager) {
-        switch currentAppUIType {
-        case .normal:
-            self.rootViewPresenter = WPTabBarController(staticScreens: false)
-        case .simplified:
-            let meScenePresenter = MeScenePresenter()
-            self.rootViewPresenter = MySitesCoordinator(meScenePresenter: meScenePresenter, onBecomeActiveTab: {})
-        case .staticScreens:
-            self.rootViewPresenter = StaticScreensTabBarWrapper()
-        }
         windowManager.showUI(animated: false)
     }
 

WordPress/Classes/System/WindowManager.swift

@@ -67,7 +67,7 @@ class WindowManager: NSObject {
     ///
     @objc func showAppUI(for blog: Blog? = nil, animated: Bool = true, completion: Completion? = nil) {
         isShowingFullscreenSignIn = false
-        show(RootViewCoordinator.sharedPresenter.rootViewController, animated: animated, completion: completion)
+        RootViewCoordinator.shared.showAppUI(animated: animated, completion: completion)
 
         guard let blog = blog else {
             return
@@ -81,12 +81,7 @@ class WindowManager: NSObject {
     func showSignInUI(completion: Completion? = nil) {
         isShowingFullscreenSignIn = true
 
-        guard let loginViewController = WordPressAuthenticator.loginUI() else {
-            fatalError("No login UI to show to the user.  There's no way to gracefully handle this error.")
-        }
-
-        show(loginViewController, completion: completion)
-        WordPressAuthenticator.track(.openedLogin)
+        RootViewCoordinator.shared.showSignInUI(completion: completion)
     }
 
     /// Shows the specified VC as the root VC for the managed window.  Takes care of animating the transition whenever the existing

WordPress/Classes/System/WordPressAuthenticatorProtocol.swift

@@ -0,0 +1,13 @@
+import UIKit
+import WordPressAuthenticator
+
+protocol WordPressAuthenticatorProtocol {
+    static func loginUI() -> UIViewController?
+    static func track(_ event: WPAnalyticsStat)
+}
+
+extension WordPressAuthenticator: WordPressAuthenticatorProtocol {
+    static func loginUI() -> UIViewController? {
+        Self.loginUI(showCancel: false, restrictToWPCom: false, onLoginButtonTapped: nil)
+    }
+}

WordPress/Classes/ViewRelated/Blog/Blog Dashboard/Cards/Blaze/DashboardBlazeCardCell.swift

@@ -3,7 +3,7 @@ import WordPressKit
 
 final class DashboardBlazeCardCell: DashboardCollectionViewCell {
     private var blog: Blog?
-    private var viewController: BlogDashboardViewController?
+    private weak var viewController: BlogDashboardViewController?
     private var viewModel: DashboardBlazeCardCellViewModel?
 
     func configure(blog: Blog, viewController: BlogDashboardViewController?, apiResponse: BlogDashboardRemoteEntity?) {

WordPress/Classes/ViewRelated/Blog/Blog Dashboard/Cards/Pages/DashboardPagesListCardCell.swift

@@ -115,7 +115,9 @@ extension DashboardPagesListCardCell {
     private func makeAllPagesAction() -> UIMenuElement {
         let allPagesAction = UIAction(title: Strings.allPages,
                                       image: Style.allPagesImage,
-                                      handler: { _ in self.showPagesList(source: .contextMenu) })
+                                      handler: { [weak self] _ in
+            self?.showPagesList(source: .contextMenu)
+        })
 
         // Wrap the pages action in a menu to display a divider between the pages action and hide this action.
         // https://developer.apple.com/documentation/uikit/uimenu/options/3261455-displayinline

WordPress/Classes/ViewRelated/Blog/Blog Dashboard/Cards/Prompts/DashboardPromptsCardCell.swift

@@ -289,6 +289,26 @@ class DashboardPromptsCardCell: UICollectionViewCell, Reusable {
 
     // Defines the structure of the contextual menu items.
     private var contextMenuItems: [[MenuItem]] {
+        let viewMoreMenuTapped = { [weak self] in
+            guard let self else { return }
+            self.viewMoreMenuTapped()
+        }
+
+        let skipMenuTapped = { [weak self] in
+            guard let self else { return }
+            self.skipMenuTapped()
+        }
+
+        let learnMoreTapped = { [weak self] in
+            guard let self else { return }
+            self.learnMoreTapped()
+        }
+
+        let removeMenuTapped = { [weak self] in
+            guard let self else { return }
+            self.removeMenuTapped()
+        }
+
         let defaultItems: [MenuItem] = [
             .viewMore(viewMoreMenuTapped),
             .skip(skipMenuTapped)

WordPress/Classes/ViewRelated/Blog/Blog Dashboard/Cards/Quick Actions/DashboardQuickActionsCardCell.swift

@@ -74,20 +74,24 @@ extension DashboardQuickActionsCardCell {
     }
 
     private func configureTapEvents(for blog: Blog, with sourceController: UIViewController) {
-        statsButton.onTap = { [weak self] in
-            self?.showStats(for: blog, from: sourceController)
+        statsButton.onTap = { [weak self, weak sourceController] in
+            guard let self, let sourceController else { return }
+            self.showStats(for: blog, from: sourceController)
         }
 
-        postsButton.onTap = { [weak self] in
-            self?.showPostList(for: blog, from: sourceController)
+        postsButton.onTap = { [weak self, weak sourceController] in
+            guard let self, let sourceController else { return }
+            self.showPostList(for: blog, from: sourceController)
         }
 
-        mediaButton.onTap = { [weak self] in
-            self?.showMediaLibrary(for: blog, from: sourceController)
+        mediaButton.onTap = { [weak self, weak sourceController] in
+            guard let self, let sourceController else { return }
+            self.showMediaLibrary(for: blog, from: sourceController)
         }
 
-        pagesButton.onTap = { [weak self] in
-            self?.showPageList(for: blog, from: sourceController)
+        pagesButton.onTap = { [weak self, weak sourceController] in
+            guard let self, let sourceController else { return }
+            self.showPageList(for: blog, from: sourceController)
         }
     }
 

WordPress/Classes/ViewRelated/Blog/Blog Dashboard/Cards/Stats/DashboardStatsCardCell.swift

@@ -70,8 +70,10 @@ extension DashboardStatsCardCell: BlogDashboardCardConfigurable {
     }
 
     private func configureCard(for blog: Blog, in viewController: UIViewController) {
-        frameView.onViewTap = { [weak self] in
-            self?.showStats(for: blog, from: viewController)
+        frameView.onViewTap = { [weak self, weak viewController] in
+            guard let self, let viewController else { return }
+
+            self.showStats(for: blog, from: viewController)
         }
 
         if FeatureFlag.personalizeHomeTab.enabled {
@@ -89,8 +91,10 @@ extension DashboardStatsCardCell: BlogDashboardCardConfigurable {
         statsStackView?.visitors = viewModel?.todaysVisitors
         statsStackView?.likes = viewModel?.todaysLikes
 
-        nudgeView?.onTap = { [weak self] in
-            self?.showNudgeHint(for: blog, from: viewController)
+        nudgeView?.onTap = { [weak self, weak viewController] in
+            guard let self, let viewController else { return }
+
+            self.showNudgeHint(for: blog, from: viewController)
         }
 
         nudgeView?.isHidden = !(viewModel?.shouldDisplayNudge ?? false)
@@ -101,8 +105,10 @@ extension DashboardStatsCardCell: BlogDashboardCardConfigurable {
     }
 
     private func makeShowStatsMenuAction(for blog: Blog, in viewController: UIViewController) -> UIAction {
-        UIAction(title: Strings.viewStats, image: UIImage(systemName: "chart.bar.xaxis")) { [weak self] _ in
-            self?.showStats(for: blog, from: viewController)
+        UIAction(title: Strings.viewStats, image: UIImage(systemName: "chart.bar.xaxis")) { [weak self, weak viewController] _ in
+            guard let self, let viewController else { return }
+
+            self.showStats(for: blog, from: viewController)
         }
     }
 

WordPress/Classes/ViewRelated/Blog/Blog Dashboard/ViewModel/BlogDashboardViewModel.swift

@@ -49,7 +49,7 @@ final class BlogDashboardViewModel {
             return nil
         }
 
-        return DashboardDataSource(collectionView: viewController.collectionView) { [unowned self] collectionView, indexPath, item in
+        return DashboardDataSource(collectionView: viewController.collectionView) { [unowned self, unowned viewController] collectionView, indexPath, item in
 
             var cellType: DashboardCollectionViewCell.Type
             var cardType: DashboardCard

WordPress/Classes/ViewRelated/Blog/Blogging Prompts/RootViewCoordinator+BloggingPrompt.swift

@@ -8,7 +8,7 @@ extension RootViewCoordinator {
     }
 
     @objc func updatePromptsIfNeeded() {
-        guard let blog = rootViewPresenter.currentOrLastBlog() else {
+        guard let blog = Self.sharedPresenter.currentOrLastBlog() else {
             return
         }
 
@@ -22,7 +22,7 @@ extension RootViewCoordinator {
         guard Feature.enabled(.bloggingPrompts),
               let siteID = userInfo[BloggingPrompt.NotificationKeys.siteID] as? Int,
               let blog = accountSites?.first(where: { $0.dotComID == NSNumber(value: siteID) }),
-              let viewController = rootViewPresenter.currentViewController else {
+              let viewController = Self.sharedPresenter.currentViewController else {
             return
         }
 

WordPress/Classes/ViewRelated/Blog/My Site/MySiteViewController.swift

@@ -139,13 +139,13 @@ class MySiteViewController: UIViewController, NoResultsViewHost {
         }
     }
 
-    private(set) var sitePickerViewController: SitePickerViewController?
-    private(set) var blogDetailsViewController: BlogDetailsViewController? {
+    private(set) weak var sitePickerViewController: SitePickerViewController?
+    private(set) weak var blogDetailsViewController: BlogDetailsViewController? {
         didSet {
             blogDetailsViewController?.presentationDelegate = self
         }
     }
-    private var blogDashboardViewController: BlogDashboardViewController?
+    private weak var blogDashboardViewController: BlogDashboardViewController?
 
     /// When we display a no results view, we'll do so in a scrollview so that
     /// we can allow pull to refresh to sync the user's list of sites.

WordPress/Classes/ViewRelated/Notifications/Controllers/NotificationsViewController.swift

@@ -513,10 +513,14 @@ class NotificationsViewController: UIViewController, UIViewControllerRestoration
         detailsViewController.dataSource = self
         detailsViewController.notificationCommentDetailCoordinator = notificationCommentDetailCoordinator
         detailsViewController.note = note
-        detailsViewController.onDeletionRequestCallback = { request in
+        detailsViewController.onDeletionRequestCallback = { [weak self] request in
+            guard let self else { return }
+
             self.showUndeleteForNoteWithID(note.objectID, request: request)
         }
-        detailsViewController.onSelectedNoteChange = { note in
+        detailsViewController.onSelectedNoteChange = { [weak self] note in
+            guard let self else { return }
+
             self.selectRow(for: note)
         }
     }
@@ -752,7 +756,9 @@ extension NotificationsViewController {
             return
         }
 
-        syncNotification(with: noteId, timeout: Syncing.pushMaxWait) { note in
+        syncNotification(with: noteId, timeout: Syncing.pushMaxWait) { [weak self] note in
+            guard let self else { return }
+
             self.showDetails(for: note)
         }
     }
@@ -941,7 +947,9 @@ private extension NotificationsViewController {
         reloadResultsController()
 
         // Hit the Deletion Action
-        request.action { success in
+        request.action { [weak self] success in
+            guard let self else { return }
+
             self.notificationDeletionRequests.removeValue(forKey: noteObjectID)
             self.notificationIdsBeingDeleted.remove(noteObjectID)
 

WordPress/Classes/ViewRelated/Reader/Tab Navigation/ReaderTabViewController.swift

@@ -7,7 +7,7 @@ class ReaderTabViewController: UIViewController {
 
     private let makeReaderTabView: (ReaderTabViewModel) -> ReaderTabView
 
-    private lazy var readerTabView: ReaderTabView = {
+    private lazy var readerTabView: ReaderTabView = { [unowned viewModel] in
         return makeReaderTabView(viewModel)
     }()
 

WordPress/Classes/ViewRelated/Reader/Tab Navigation/WPTabBarController+ReaderTabNavigation.swift

@@ -27,14 +27,22 @@ extension WPTabBarController {
     }
 
     @objc func makeReaderTabViewController() -> ReaderTabViewController {
-        return ReaderTabViewController(viewModel: self.readerTabViewModel, readerTabViewFactory: makeReaderTabView(_:))
+        return ReaderTabViewController(viewModel: readerTabViewModel) { [unowned self] viewModel in
+            return self.makeReaderTabView(viewModel)
+        }
     }
 
     @objc func makeReaderTabViewModel() -> ReaderTabViewModel {
-        let viewModel = ReaderTabViewModel(readerContentFactory: makeReaderContentViewController(with:),
-                                           searchNavigationFactory: navigateToReaderSearch,
-                                           tabItemsStore: ReaderTabItemsStore(),
-                                           settingsPresenter: ReaderManageScenePresenter())
+        let viewModel = ReaderTabViewModel(
+            readerContentFactory: { [unowned self] in
+                self.makeReaderContentViewController(with: $0)
+            },
+            searchNavigationFactory: { [unowned self] in
+                self.navigateToReaderSearch()
+            },
+            tabItemsStore: ReaderTabItemsStore(),
+            settingsPresenter: ReaderManageScenePresenter()
+        )
         return viewModel
     }