From 4791b6fa3cb83a604948b69ddb69a10fd4dd5e75 Mon Sep 17 00:00:00 2001 From: Povilas Staskus Date: Mon, 10 Jul 2023 23:14:46 +0300 Subject: [PATCH 01/15] Set weak reference for BlogDashboardViewController in DashboardBlazeCardCell --- .../Blog Dashboard/Cards/Blaze/DashboardBlazeCardCell.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPress/Classes/ViewRelated/Blog/Blog Dashboard/Cards/Blaze/DashboardBlazeCardCell.swift b/WordPress/Classes/ViewRelated/Blog/Blog Dashboard/Cards/Blaze/DashboardBlazeCardCell.swift index 35a47c94fb16..e5c28396933c 100644 --- a/WordPress/Classes/ViewRelated/Blog/Blog Dashboard/Cards/Blaze/DashboardBlazeCardCell.swift +++ b/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?) { From 692c523f70e972aa6278a0d8f061a9d5a117b903 Mon Sep 17 00:00:00 2001 From: Povilas Staskus Date: Mon, 10 Jul 2023 23:15:14 +0300 Subject: [PATCH 02/15] Weakly capture action closure in DashbordPagesListCardCell --- .../Cards/Pages/DashboardPagesListCardCell.swift | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/WordPress/Classes/ViewRelated/Blog/Blog Dashboard/Cards/Pages/DashboardPagesListCardCell.swift b/WordPress/Classes/ViewRelated/Blog/Blog Dashboard/Cards/Pages/DashboardPagesListCardCell.swift index 67dc8d6ab8c4..744d0963db03 100644 --- a/WordPress/Classes/ViewRelated/Blog/Blog Dashboard/Cards/Pages/DashboardPagesListCardCell.swift +++ b/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 From 9ec904c25d25b15da297e06d1ced183b3324b360 Mon Sep 17 00:00:00 2001 From: Povilas Staskus Date: Mon, 10 Jul 2023 23:16:02 +0300 Subject: [PATCH 03/15] Pass weak closure instead of strongly captured method in DashboardPromptCardCell --- .../Prompts/DashboardPromptsCardCell.swift | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/WordPress/Classes/ViewRelated/Blog/Blog Dashboard/Cards/Prompts/DashboardPromptsCardCell.swift b/WordPress/Classes/ViewRelated/Blog/Blog Dashboard/Cards/Prompts/DashboardPromptsCardCell.swift index dec2db18d784..c208ed2ca384 100644 --- a/WordPress/Classes/ViewRelated/Blog/Blog Dashboard/Cards/Prompts/DashboardPromptsCardCell.swift +++ b/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) From a7f12dd77cd60a7a18bb6334d99231e23ca26796 Mon Sep 17 00:00:00 2001 From: Povilas Staskus Date: Mon, 10 Jul 2023 23:16:36 +0300 Subject: [PATCH 04/15] Break circular dependency cycle for BlogDashboardViewController in quick action cell --- .../DashboardQuickActionsCardCell.swift | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/WordPress/Classes/ViewRelated/Blog/Blog Dashboard/Cards/Quick Actions/DashboardQuickActionsCardCell.swift b/WordPress/Classes/ViewRelated/Blog/Blog Dashboard/Cards/Quick Actions/DashboardQuickActionsCardCell.swift index 6fef2411bba3..25b1c319e90c 100644 --- a/WordPress/Classes/ViewRelated/Blog/Blog Dashboard/Cards/Quick Actions/DashboardQuickActionsCardCell.swift +++ b/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) } } From db45915836cb73fcec44fcee1957b7731ef45698 Mon Sep 17 00:00:00 2001 From: Povilas Staskus Date: Mon, 10 Jul 2023 23:17:22 +0300 Subject: [PATCH 05/15] Break circular dependency cycle for BlogDashboardViewController in DashboardStatsCardCell --- .../Cards/Stats/DashboardStatsCardCell.swift | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/WordPress/Classes/ViewRelated/Blog/Blog Dashboard/Cards/Stats/DashboardStatsCardCell.swift b/WordPress/Classes/ViewRelated/Blog/Blog Dashboard/Cards/Stats/DashboardStatsCardCell.swift index 31e10f8870c7..577c9c27db9b 100644 --- a/WordPress/Classes/ViewRelated/Blog/Blog Dashboard/Cards/Stats/DashboardStatsCardCell.swift +++ b/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) } } From 857430c173910923eb47bee39486163c33adb4ac Mon Sep 17 00:00:00 2001 From: Povilas Staskus Date: Mon, 10 Jul 2023 23:17:59 +0300 Subject: [PATCH 06/15] Capture BlogDashboardViewController weekly in BlogDashboardViewModel --- .../Blog/Blog Dashboard/ViewModel/BlogDashboardViewModel.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPress/Classes/ViewRelated/Blog/Blog Dashboard/ViewModel/BlogDashboardViewModel.swift b/WordPress/Classes/ViewRelated/Blog/Blog Dashboard/ViewModel/BlogDashboardViewModel.swift index 9f7335f6ce2b..43223a720465 100644 --- a/WordPress/Classes/ViewRelated/Blog/Blog Dashboard/ViewModel/BlogDashboardViewModel.swift +++ b/WordPress/Classes/ViewRelated/Blog/Blog Dashboard/ViewModel/BlogDashboardViewModel.swift @@ -49,7 +49,7 @@ 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 From c90bfdf7ae8a476a3717b138a6ced9bd80f861ea Mon Sep 17 00:00:00 2001 From: Povilas Staskus Date: Mon, 10 Jul 2023 23:19:44 +0300 Subject: [PATCH 07/15] Break retain cycle by capturing presented view controllers weakly in MySiteViewController --- .../ViewRelated/Blog/My Site/MySiteViewController.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/WordPress/Classes/ViewRelated/Blog/My Site/MySiteViewController.swift b/WordPress/Classes/ViewRelated/Blog/My Site/MySiteViewController.swift index 4812d2cf0152..f5940b748070 100644 --- a/WordPress/Classes/ViewRelated/Blog/My Site/MySiteViewController.swift +++ b/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(set) var blogDashboardViewController: BlogDashboardViewController? + private(set) 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. From a30d50cedc518360b81f076c1f42ad77a71c5cf6 Mon Sep 17 00:00:00 2001 From: Povilas Staskus Date: Mon, 10 Jul 2023 23:20:13 +0300 Subject: [PATCH 08/15] Weak self in NotificationsViewController --- .../NotificationsViewController.swift | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/WordPress/Classes/ViewRelated/Notifications/Controllers/NotificationsViewController.swift b/WordPress/Classes/ViewRelated/Notifications/Controllers/NotificationsViewController.swift index 5b769ed57fc7..6c2bdd008872 100644 --- a/WordPress/Classes/ViewRelated/Notifications/Controllers/NotificationsViewController.swift +++ b/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) From 7cff19a65e4b586b715fd9083c6d0a239755a463 Mon Sep 17 00:00:00 2001 From: Povilas Staskus Date: Mon, 10 Jul 2023 23:21:24 +0300 Subject: [PATCH 09/15] Capture ReaderTabViewModel weakly in a closure --- .../Reader/Tab Navigation/ReaderTabViewController.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPress/Classes/ViewRelated/Reader/Tab Navigation/ReaderTabViewController.swift b/WordPress/Classes/ViewRelated/Reader/Tab Navigation/ReaderTabViewController.swift index 92b63fbc272b..7607c266e362 100644 --- a/WordPress/Classes/ViewRelated/Reader/Tab Navigation/ReaderTabViewController.swift +++ b/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) }() From 03f3598bd9a8ecfa4081a618abd3a3a4863f0c51 Mon Sep 17 00:00:00 2001 From: Povilas Staskus Date: Mon, 10 Jul 2023 23:22:34 +0300 Subject: [PATCH 10/15] Use weak closures instead of passing methods to a closure to avoid retain cycle --- ...PTabBarController+ReaderTabNavigation.swift | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/WordPress/Classes/ViewRelated/Reader/Tab Navigation/WPTabBarController+ReaderTabNavigation.swift b/WordPress/Classes/ViewRelated/Reader/Tab Navigation/WPTabBarController+ReaderTabNavigation.swift index f502fde27a23..5fb6377da5de 100644 --- a/WordPress/Classes/ViewRelated/Reader/Tab Navigation/WPTabBarController+ReaderTabNavigation.swift +++ b/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 } From 12f7efd00a81773420f52ca06b2642e5ff353f77 Mon Sep 17 00:00:00 2001 From: Povilas Staskus Date: Tue, 11 Jul 2023 13:24:57 +0300 Subject: [PATCH 11/15] 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. --- .../Classes/System/RootViewCoordinator.swift | 58 +++++++++++++------ WordPress/Classes/System/WindowManager.swift | 9 +-- .../RootViewCoordinator+BloggingPrompt.swift | 4 +- 3 files changed, 45 insertions(+), 26 deletions(-) diff --git a/WordPress/Classes/System/RootViewCoordinator.swift b/WordPress/Classes/System/RootViewCoordinator.swift index a95d52b78264..66b36e23075e 100644 --- a/WordPress/Classes/System/RootViewCoordinator.swift +++ b/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,7 +45,7 @@ class RootViewCoordinator { // MARK: Private instance variables - private(set) var rootViewPresenter: RootViewPresenter + private var rootViewPresenter: RootViewPresenter? private var currentAppUIType: AppUIType { didSet { updateJetpackFeaturesRemovalCoordinatorState() @@ -50,17 +61,39 @@ class RootViewCoordinator { self.featureFlagStore = featureFlagStore self.windowManager = windowManager self.currentAppUIType = Self.appUIType(featureFlagStore: featureFlagStore) - switch self.currentAppUIType { + 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 +155,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) } diff --git a/WordPress/Classes/System/WindowManager.swift b/WordPress/Classes/System/WindowManager.swift index 2e90579cfeb3..42a83d8f2811 100644 --- a/WordPress/Classes/System/WindowManager.swift +++ b/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 diff --git a/WordPress/Classes/ViewRelated/Blog/Blogging Prompts/RootViewCoordinator+BloggingPrompt.swift b/WordPress/Classes/ViewRelated/Blog/Blogging Prompts/RootViewCoordinator+BloggingPrompt.swift index fdee3998d56e..3f00f442f1e9 100644 --- a/WordPress/Classes/ViewRelated/Blog/Blogging Prompts/RootViewCoordinator+BloggingPrompt.swift +++ b/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 } From daf8e0811f281304fbe3f098737133fefd8c5e3f Mon Sep 17 00:00:00 2001 From: Povilas Staskus Date: Tue, 11 Jul 2023 13:25:23 +0300 Subject: [PATCH 12/15] Update RELEASE-NOTES.txt --- RELEASE-NOTES.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index f4db083c0afa..815d0523274a 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -1,3 +1,7 @@ +22.9 +----- +* [**] [internal] Fix multiple memory leaks after logging in and logging out. [#21047] + 22.8 ----- * [*] Blogging Reminders: Disabled prompt for self-hosted sites not connected to Jetpack. [#20970] From 8d048feb77ab8ee7aae5481f5dbf102f2755ad50 Mon Sep 17 00:00:00 2001 From: Povilas Staskus Date: Wed, 12 Jul 2023 11:54:37 +0300 Subject: [PATCH 13/15] Update RELEASE-NOTES.txt --- RELEASE-NOTES.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index a8655e47f9ff..2da93d87637d 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -1,8 +1,9 @@ 22.9 ----- -* [**] [internal] Fix multiple memory leaks after logging in and logging out. [#21047] +* [*] [internal] Fix multiple memory leaks after logging in and logging out. [#21047] +* [*] Performance improvements in login and logouts flows. [#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 ----- From 3ce507690c4068de02736f867746780ad2715e1e Mon Sep 17 00:00:00 2001 From: Povilas Staskus Date: Wed, 12 Jul 2023 14:08:52 +0300 Subject: [PATCH 14/15] Added unit tests confirming deallocation of rootViewController after logout - Made WordPressAuthenticator injectable - Checking if rootViewController is deinitialized after presenting signInUI --- .../Classes/System/RootViewCoordinator.swift | 9 ++-- .../WordPressAuthenticatorProtocol.swift | 13 +++++ WordPress/WordPress.xcodeproj/project.pbxproj | 38 +++++++++---- .../RootViewCoordinatorTests.swift | 54 +++++++++++++++++++ 4 files changed, 101 insertions(+), 13 deletions(-) create mode 100644 WordPress/Classes/System/WordPressAuthenticatorProtocol.swift create mode 100644 WordPress/WordPressTest/RootViewCoordinatorTests.swift diff --git a/WordPress/Classes/System/RootViewCoordinator.swift b/WordPress/Classes/System/RootViewCoordinator.swift index 66b36e23075e..a6e44da107bd 100644 --- a/WordPress/Classes/System/RootViewCoordinator.swift +++ b/WordPress/Classes/System/RootViewCoordinator.swift @@ -53,14 +53,17 @@ class RootViewCoordinator { } 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) + self.wordPressAuthenticator = wordPressAuthenticator updateJetpackFeaturesRemovalCoordinatorState() } @@ -75,12 +78,12 @@ class RootViewCoordinator { } func showSignInUI(completion: (() -> Void)? = nil) { - guard let loginViewController = WordPressAuthenticator.loginUI() else { + 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) + wordPressAuthenticator.track(.openedLogin) self.rootViewPresenter = nil } diff --git a/WordPress/Classes/System/WordPressAuthenticatorProtocol.swift b/WordPress/Classes/System/WordPressAuthenticatorProtocol.swift new file mode 100644 index 000000000000..1d6d49d5a7ed --- /dev/null +++ b/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) + } +} diff --git a/WordPress/WordPress.xcodeproj/project.pbxproj b/WordPress/WordPress.xcodeproj/project.pbxproj index 33f8a57e75fd..120bb5e70fa0 100644 --- a/WordPress/WordPress.xcodeproj/project.pbxproj +++ b/WordPress/WordPress.xcodeproj/project.pbxproj @@ -175,6 +175,9 @@ 014ACD142A1E5034008A706C /* WebKitViewController+SandboxStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = 014ACD132A1E5033008A706C /* WebKitViewController+SandboxStore.swift */; }; 014ACD152A1E5034008A706C /* WebKitViewController+SandboxStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = 014ACD132A1E5033008A706C /* WebKitViewController+SandboxStore.swift */; }; 015BA4EB29A788A300920F4B /* StatsTotalInsightsCellTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 015BA4EA29A788A300920F4B /* StatsTotalInsightsCellTests.swift */; }; + 019D699E2A5EA963003B676D /* RootViewCoordinatorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 019D699D2A5EA963003B676D /* RootViewCoordinatorTests.swift */; }; + 019D69A02A5EBF47003B676D /* WordPressAuthenticatorProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = 019D699F2A5EBF47003B676D /* WordPressAuthenticatorProtocol.swift */; }; + 019D69A12A5EBF47003B676D /* WordPressAuthenticatorProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = 019D699F2A5EBF47003B676D /* WordPressAuthenticatorProtocol.swift */; }; 01CE5007290A889F00A9C2E0 /* TracksConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 01CE5006290A889F00A9C2E0 /* TracksConfiguration.swift */; }; 01CE5008290A88BD00A9C2E0 /* TracksConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 01CE5006290A889F00A9C2E0 /* TracksConfiguration.swift */; }; 01CE500C290A88BF00A9C2E0 /* TracksConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 01CE5006290A889F00A9C2E0 /* TracksConfiguration.swift */; }; @@ -5881,6 +5884,8 @@ 0148CC2A2859C87000CF5D96 /* BlogServiceMock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BlogServiceMock.swift; sourceTree = ""; }; 014ACD132A1E5033008A706C /* WebKitViewController+SandboxStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "WebKitViewController+SandboxStore.swift"; sourceTree = ""; }; 015BA4EA29A788A300920F4B /* StatsTotalInsightsCellTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StatsTotalInsightsCellTests.swift; sourceTree = ""; }; + 019D699D2A5EA963003B676D /* RootViewCoordinatorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RootViewCoordinatorTests.swift; sourceTree = ""; }; + 019D699F2A5EBF47003B676D /* WordPressAuthenticatorProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WordPressAuthenticatorProtocol.swift; sourceTree = ""; }; 01CE5006290A889F00A9C2E0 /* TracksConfiguration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TracksConfiguration.swift; sourceTree = ""; }; 01CE5010290A890300A9C2E0 /* TracksConfiguration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TracksConfiguration.swift; sourceTree = ""; }; 01DBFD8629BDCBF200F3720F /* JetpackNativeConnectionService.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = JetpackNativeConnectionService.swift; sourceTree = ""; }; @@ -9817,6 +9822,14 @@ path = Support; sourceTree = ""; }; + 019D699C2A5EA950003B676D /* Coordinators */ = { + isa = PBXGroup; + children = ( + 019D699D2A5EA963003B676D /* RootViewCoordinatorTests.swift */, + ); + name = Coordinators; + sourceTree = ""; + }; 027AC51F2278982D0033E56E /* DomainCredit */ = { isa = PBXGroup; children = ( @@ -10537,7 +10550,7 @@ path = Classes; sourceTree = ""; }; - 29B97314FDCFA39411CA2CEA = { + 29B97314FDCFA39411CA2CEA /* CustomTemplate */ = { isa = PBXGroup; children = ( 3F20FDF3276BF21000DA3CAD /* Packages */, @@ -13218,6 +13231,7 @@ 803BB982295957F600B3F6D6 /* MySitesCoordinator+RootViewPresenter.swift */, 80A2153C29C35197002FE8EB /* StaticScreensTabBarWrapper.swift */, F484D4E92A32B51C0050BE15 /* RootViewPresenter+AppSettingsNavigation.swift */, + 019D699F2A5EBF47003B676D /* WordPressAuthenticatorProtocol.swift */, ); name = "Root View"; sourceTree = ""; @@ -16451,6 +16465,7 @@ 7EC9FE0822C6275900C5A888 /* Analytics */, FF7691661EE06CF500713F4B /* Aztec */, B5AEEC7F1ACAD099008BF2A4 /* Categories */, + 019D699C2A5EA950003B676D /* Coordinators */, 572FB3FE223A800500933C76 /* Classes */, B5AEEC731ACACF3B008BF2A4 /* Core Data */, 8B6214E427B1B420001DF7B6 /* Dashboard */, @@ -18894,14 +18909,14 @@ bg, sk, ); - mainGroup = 29B97314FDCFA39411CA2CEA; + mainGroup = 29B97314FDCFA39411CA2CEA /* CustomTemplate */; packageReferences = ( 3FF1442E266F3C2400138163 /* XCRemoteSwiftPackageReference "ScreenObject" */, 3FC2C33B26C4CF0A00C6D98F /* XCRemoteSwiftPackageReference "XCUITestHelpers" */, 17A8858B2757B97F0071FCA3 /* XCRemoteSwiftPackageReference "AutomatticAbout-swift" */, 3F2B62DA284F4E0B0008CD59 /* XCRemoteSwiftPackageReference "Charts" */, 3F3B23C02858A1B300CACE60 /* XCRemoteSwiftPackageReference "test-collector-swift" */, - 3F411B6D28987E3F002513AE /* XCRemoteSwiftPackageReference "lottie-ios.git" */, + 3F411B6D28987E3F002513AE /* XCRemoteSwiftPackageReference "lottie-ios" */, 3F338B6F289BD3040014ADC5 /* XCRemoteSwiftPackageReference "Nimble" */, ); productRefGroup = 19C28FACFE9D520D11CA2CBB /* Products */; @@ -20657,11 +20672,11 @@ files = ( ); inputPaths = ( - "$SRCROOT/../Scripts/BuildPhases/CopyGutenbergJS.inputs.xcfilelist", + $SRCROOT/../Scripts/BuildPhases/CopyGutenbergJS.inputs.xcfilelist, ); name = "Copy Gutenberg JS"; outputFileListPaths = ( - "$SRCROOT/../Scripts/BuildPhases/CopyGutenbergJS.outputs.xcfilelist", + $SRCROOT/../Scripts/BuildPhases/CopyGutenbergJS.outputs.xcfilelist, ); outputPaths = ( "", @@ -20876,13 +20891,13 @@ files = ( ); inputFileListPaths = ( - "$SRCROOT/../Scripts/BuildPhases/CopyGutenbergJS.inputs.xcfilelist", + $SRCROOT/../Scripts/BuildPhases/CopyGutenbergJS.inputs.xcfilelist, ); inputPaths = ( ); name = "Copy Gutenberg JS"; outputFileListPaths = ( - "$SRCROOT/../Scripts/BuildPhases/CopyGutenbergJS.outputs.xcfilelist", + $SRCROOT/../Scripts/BuildPhases/CopyGutenbergJS.outputs.xcfilelist, ); outputPaths = ( ); @@ -22547,6 +22562,7 @@ 80C740FB2989FC4600199027 /* PostStatsTableViewController+JetpackBannerViewController.swift in Sources */, B5EEDB971C91F10400676B2B /* Blog+Interface.swift in Sources */, 80D9D04A29FC0D9000FE3400 /* NSMutableArray+NullableObjects.m in Sources */, + 019D69A02A5EBF47003B676D /* WordPressAuthenticatorProtocol.swift in Sources */, 982DDF96263238A6002B3904 /* LikeUserPreferredBlog+CoreDataProperties.swift in Sources */, 5D839AA8187F0D6B00811F4A /* PostFeaturedImageCell.m in Sources */, C7BB601F2863B9E800748FD9 /* QRLoginCameraSession.swift in Sources */, @@ -23767,6 +23783,7 @@ 85B125411B028E34008A3D95 /* PushAuthenticationManagerTests.swift in Sources */, 4A76A4BB29D4381100AABF4B /* CommentService+LikesTests.swift in Sources */, 8BB185CE24B62CE100A4CCE8 /* ReaderCardServiceTests.swift in Sources */, + 019D699E2A5EA963003B676D /* RootViewCoordinatorTests.swift in Sources */, 57DF04C1231489A200CC93D6 /* PostCardStatusViewModelTests.swift in Sources */, D821C81B21003AE9002ED995 /* FormattableContentGroupTests.swift in Sources */, 93D86B981C691E71003D8E3E /* LocalCoreDataServiceTests.m in Sources */, @@ -24232,6 +24249,7 @@ FABB21D02602FC2C00C8785C /* SiteStatsPeriodTableViewController.swift in Sources */, FABB21D12602FC2C00C8785C /* TextBundleWrapper.m in Sources */, FABB21D22602FC2C00C8785C /* WPCategoryTree.swift in Sources */, + 019D69A12A5EBF47003B676D /* WordPressAuthenticatorProtocol.swift in Sources */, FABB21D32602FC2C00C8785C /* PageListTableViewCell.m in Sources */, FABB21D42602FC2C00C8785C /* PageSettingsViewController.m in Sources */, FA73D7E62798765B00DF24B3 /* SitePickerViewController.swift in Sources */, @@ -30715,7 +30733,7 @@ minimumVersion = 0.3.0; }; }; - 3F411B6D28987E3F002513AE /* XCRemoteSwiftPackageReference "lottie-ios.git" */ = { + 3F411B6D28987E3F002513AE /* XCRemoteSwiftPackageReference "lottie-ios" */ = { isa = XCRemoteSwiftPackageReference; repositoryURL = "https://github.com/airbnb/lottie-ios.git"; requirement = { @@ -30796,12 +30814,12 @@ }; 3F411B6E28987E3F002513AE /* Lottie */ = { isa = XCSwiftPackageProductDependency; - package = 3F411B6D28987E3F002513AE /* XCRemoteSwiftPackageReference "lottie-ios.git" */; + package = 3F411B6D28987E3F002513AE /* XCRemoteSwiftPackageReference "lottie-ios" */; productName = Lottie; }; 3F44DD57289C379C006334CD /* Lottie */ = { isa = XCSwiftPackageProductDependency; - package = 3F411B6D28987E3F002513AE /* XCRemoteSwiftPackageReference "lottie-ios.git" */; + package = 3F411B6D28987E3F002513AE /* XCRemoteSwiftPackageReference "lottie-ios" */; productName = Lottie; }; 3FC2C33C26C4CF0A00C6D98F /* XCUITestHelpers */ = { diff --git a/WordPress/WordPressTest/RootViewCoordinatorTests.swift b/WordPress/WordPressTest/RootViewCoordinatorTests.swift new file mode 100644 index 000000000000..510e256305be --- /dev/null +++ b/WordPress/WordPressTest/RootViewCoordinatorTests.swift @@ -0,0 +1,54 @@ +import Foundation +import XCTest + +@testable import WordPress + +final class RootViewCoordinatorTests: XCTestCase { + private var sut: RootViewCoordinator! + private var windowManager: WindowManagerMock! + + override func setUp() { + super.setUp() + windowManager = WindowManagerMock(window: .init()) + sut = RootViewCoordinator( + featureFlagStore: RemoteFeatureFlagStoreMock(), + windowManager: windowManager, + wordPressAuthenticator: WordPressAuthenticatorMock.self + ) + } + + override func tearDown() { + super.tearDown() + windowManager = nil + sut = nil + } + + func testAppUIDeallocatedAfterLogout() { + sut.showAppUI() + let mainRootViewController = windowManager.presentedViewController + XCTAssertNotNil(mainRootViewController) + + sut.showSignInUI() + + addTeardownBlock { [weak mainRootViewController] in + XCTAssertNil(mainRootViewController) + } + } +} + +private class WindowManagerMock: WindowManager { + var presentedViewController: UIViewController? + + override func show(_ viewController: UIViewController, animated: Bool = true, completion: WindowManager.Completion? = nil) { + self.presentedViewController = viewController + completion?() + } +} + +private class WordPressAuthenticatorMock: WordPressAuthenticatorProtocol { + static func loginUI() -> UIViewController? { + return UIViewController(nibName: nil, bundle: nil) + } + + static func track(_ event: WPAnalyticsStat) {} +} From de729ae1c83a7e162f431715136fa3e33fbf31f2 Mon Sep 17 00:00:00 2001 From: Povilas Staskus Date: Thu, 13 Jul 2023 10:30:53 +0300 Subject: [PATCH 15/15] Update RELEASE-NOTES.txt --- RELEASE-NOTES.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 2da93d87637d..887d42fb4c00 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -1,7 +1,6 @@ 22.9 ----- * [*] [internal] Fix multiple memory leaks after logging in and logging out. [#21047] -* [*] Performance improvements in login and logouts flows. [#21047] * [**] Block editor: Move undo/redo buttons to the navigation bar. [#20930] * [*] Fixed an issue that caused the UI to be briefly unresponsive in certain case when opening the app. [#21065]