Refactor FXIOS-7845 [v122] Remove shared TabManager, use dependency i…

…njection (#17607)

* [7845] Refactor shared global TabManager out of AppContainer, instead use individual instances passed via DI, one for each BVC. For currently iOS app this should have no change, but it helps pave the way for forthcoming multiwindow support for iPad.

* [7845] Minor cleanup, unused import

* Fix several SwiftLint warnings

* [7845] Update unit tests for changes to various initializers which now require passing in TabManager via DI

* [7845] Inject TabManager into HomepageSettingVC. This also should fix one of the failing UI tests.

Client/Application/AppDelegate.swift

@@ -25,13 +25,6 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
         sendTabDelegate: UIApplication.shared.sendTabDelegate,
         creditCardAutofillEnabled: creditCardAutofillStatus
     )
-    lazy var tabManager: TabManager = TabManagerImplementation(
-        profile: profile,
-        imageStore: DefaultDiskImageStore(
-            files: profile.files,
-            namespace: "TabManagerScreenshots",
-            quality: UIConstants.ScreenshotQuality)
-    )
 
     lazy var themeManager: ThemeManager = DefaultThemeManager(sharedContainerIdentifier: AppInfo.sharedContainerIdentifier)
     lazy var ratingPromptManager = RatingPromptManager(profile: profile)
@@ -232,7 +225,6 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
 
         handleBackgroundEvent()
         TelemetryWrapper.recordEvent(category: .action, method: .background, object: .app)
-        TabsQuantityTelemetry.trackTabsQuantity(tabManager: tabManager)
 
         profile.syncManager.applicationDidEnterBackground()
 
@@ -246,7 +238,6 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
         singleShotTimer.resume()
         shutdownWebServer = singleShotTimer
         backgroundWorkUtility?.scheduleOnAppBackground()
-        tabManager.preserveTabs()
 
         logger.log("applicationDidEnterBackground end",
                    level: .info,

Client/Application/DependencyHelper.swift

@@ -16,9 +16,6 @@ class DependencyHelper {
         let profile: Profile = appDelegate.profile
         AppContainer.shared.register(service: profile)
 
-        let tabManager: TabManager = appDelegate.tabManager
-        AppContainer.shared.register(service: tabManager)
-
         let appSessionProvider: AppSessionProvider = appDelegate.appSessionManager
         AppContainer.shared.register(service: appSessionProvider)
 

Client/Coordinators/Browser/BrowserCoordinator.swift

@@ -35,8 +35,8 @@ class BrowserCoordinator: BaseCoordinator,
 
     init(router: Router,
          screenshotService: ScreenshotService,
+         tabManager: TabManager,
          profile: Profile = AppContainer.shared.resolve(),
-         tabManager: TabManager = AppContainer.shared.resolve(),
          themeManager: ThemeManager = AppContainer.shared.resolve(),
          glean: GleanWrapper = DefaultGleanWrapper.shared,
          applicationHelper: ApplicationHelper = DefaultApplicationHelper()) {
@@ -70,6 +70,10 @@ class BrowserCoordinator: BaseCoordinator,
         guard ReduxFlagManager.isReduxEnabled else { return }
         let sceneUUID = WindowData.DefaultSingleWindowUUID
         store.dispatch(TabManagerAction.tabManagerDidConnectToScene(tabManager, sceneUUID))
+
+        // TODO [7856]: Additional telemetry updates forthcoming once iPad multi-window enabled.
+        // For now, we only have a single BVC and TabManager. Plug it into our TelemetryWrapper:
+        TelemetryWrapper.shared.defaultTabManager = tabManager
     }
 
     private func startLaunch(with launchType: LaunchType) {
@@ -150,6 +154,7 @@ class BrowserCoordinator: BaseCoordinator,
                 profile: profile,
                 isZeroSearch: inline,
                 toastContainer: toastContainer,
+                tabManager: tabManager,
                 overlayManager: overlayManager)
             homepageViewController.homePanelDelegate = homepanelDelegate
             homepageViewController.libraryPanelDelegate = libraryPanelDelegate
@@ -294,7 +299,7 @@ class BrowserCoordinator: BaseCoordinator,
         navigationController.modalPresentationStyle = modalPresentationStyle
         let settingsRouter = DefaultRouter(navigationController: navigationController)
 
-        let settingsCoordinator = SettingsCoordinator(router: settingsRouter)
+        let settingsCoordinator = SettingsCoordinator(router: settingsRouter, tabManager: tabManager)
         settingsCoordinator.parentCoordinator = self
         add(child: settingsCoordinator)
         settingsCoordinator.start(with: section)
@@ -313,7 +318,8 @@ class BrowserCoordinator: BaseCoordinator,
             navigationController.modalPresentationStyle = .formSheet
 
             let libraryCoordinator = LibraryCoordinator(
-                router: DefaultRouter(navigationController: navigationController)
+                router: DefaultRouter(navigationController: navigationController),
+                tabManager: browserViewController.tabManager
             )
             libraryCoordinator.parentCoordinator = self
             add(child: libraryCoordinator)
@@ -324,7 +330,8 @@ class BrowserCoordinator: BaseCoordinator,
     }
 
     private func showETPMenu(sourceView: UIView) {
-        let enhancedTrackingProtectionCoordinator = EnhancedTrackingProtectionCoordinator(router: router)
+        let enhancedTrackingProtectionCoordinator = EnhancedTrackingProtectionCoordinator(router: router,
+                                                                                          tabManager: tabManager)
         enhancedTrackingProtectionCoordinator.parentCoordinator = self
         add(child: enhancedTrackingProtectionCoordinator)
         enhancedTrackingProtectionCoordinator.start(sourceView: sourceView)
@@ -455,7 +462,7 @@ class BrowserCoordinator: BaseCoordinator,
             return nil // flow is already handled
         }
 
-        let coordinator = FakespotCoordinator(router: router)
+        let coordinator = FakespotCoordinator(router: router, tabManager: tabManager)
         coordinator.parentCoordinator = self
         add(child: coordinator)
         return coordinator
@@ -467,7 +474,7 @@ class BrowserCoordinator: BaseCoordinator,
             // If this case is hitted it means the share extension coordinator wasn't removed correctly in the previous session.
             return
         }
-        let shareExtensionCoordinator = ShareExtensionCoordinator(alertContainer: toastContainer, router: router, profile: profile, parentCoordinator: self)
+        let shareExtensionCoordinator = ShareExtensionCoordinator(alertContainer: toastContainer, router: router, profile: profile, parentCoordinator: self, tabManager: tabManager)
         add(child: shareExtensionCoordinator)
         shareExtensionCoordinator.start(url: url, sourceView: sourceView, sourceRect: sourceRect, popoverArrowDirection: popoverArrowDirection)
     }
@@ -490,7 +497,7 @@ class BrowserCoordinator: BaseCoordinator,
         if let bottomSheetCoordinator = childCoordinators.first(where: { $0 is CredentialAutofillCoordinator }) as? CredentialAutofillCoordinator {
             return bottomSheetCoordinator
         }
-        let bottomSheetCoordinator = CredentialAutofillCoordinator(profile: profile, router: router, parentCoordinator: self)
+        let bottomSheetCoordinator = CredentialAutofillCoordinator(profile: profile, router: router, parentCoordinator: self, tabManager: tabManager)
         add(child: bottomSheetCoordinator)
         return bottomSheetCoordinator
     }

Client/Coordinators/CredentialAutofillCoordinator.swift

@@ -25,7 +25,7 @@ class CredentialAutofillCoordinator: BaseCoordinator {
         router: Router,
         parentCoordinator: BottomSheetCardParentCoordinator?,
         themeManager: ThemeManager = AppContainer.shared.resolve(),
-        tabManager: TabManager = AppContainer.shared.resolve()
+        tabManager: TabManager
     ) {
         self.profile = profile
         self.themeManager = themeManager

Client/Coordinators/EnhancedTrackingProtectionCoordinator.swift

@@ -21,7 +21,7 @@ class EnhancedTrackingProtectionCoordinator: BaseCoordinator,
 
     init(router: Router,
          profile: Profile = AppContainer.shared.resolve(),
-         tabManager: TabManager = AppContainer.shared.resolve()
+         tabManager: TabManager
     ) {
         let tab = tabManager.selectedTab
         let url = tab?.url ?? URL(fileURLWithPath: "")

Client/Coordinators/Library/DownloadsCoordinator.swift

@@ -18,16 +18,19 @@ class DownloadsCoordinator: BaseCoordinator, ParentCoordinatorDelegate, Download
 
     private weak var parentCoordinator: LibraryCoordinatorDelegate?
     private let profile: Profile
+    private let tabManager: TabManager
 
     // MARK: - Initializers
 
     init(
         router: Router,
         profile: Profile,
-        parentCoordinator: LibraryCoordinatorDelegate?
+        parentCoordinator: LibraryCoordinatorDelegate?,
+        tabManager: TabManager
     ) {
         self.parentCoordinator = parentCoordinator
         self.profile = profile
+        self.tabManager = tabManager
         super.init(router: router)
     }
 
@@ -53,7 +56,8 @@ class DownloadsCoordinator: BaseCoordinator, ParentCoordinatorDelegate, Download
             alertContainer: UIView(),
             router: router,
             profile: profile,
-            parentCoordinator: self
+            parentCoordinator: self,
+            tabManager: tabManager
         )
         add(child: coordinator)
         return coordinator

Client/Coordinators/Library/LibraryCoordinator.swift

@@ -25,7 +25,7 @@ class LibraryCoordinator: BaseCoordinator, LibraryPanelDelegate, LibraryNavigati
     init(
         router: Router,
         profile: Profile = AppContainer.shared.resolve(),
-        tabManager: TabManager = AppContainer.shared.resolve()
+        tabManager: TabManager
     ) {
         self.profile = profile
         self.tabManager = tabManager
@@ -103,7 +103,8 @@ class LibraryCoordinator: BaseCoordinator, LibraryPanelDelegate, LibraryNavigati
         let downloadsCoordinator = DownloadsCoordinator(
             router: router,
             profile: profile,
-            parentCoordinator: parentCoordinator
+            parentCoordinator: parentCoordinator,
+            tabManager: tabManager
         )
         add(child: downloadsCoordinator)
         (navigationController.topViewController as? DownloadsPanel)?.navigationHandler = downloadsCoordinator

Client/Coordinators/Scene/SceneCoordinator.swift

@@ -5,6 +5,7 @@
 import Common
 import UIKit
 import Shared
+import Storage
 
 typealias SceneUUID = UUID
 
@@ -105,8 +106,17 @@ class SceneCoordinator: BaseCoordinator, LaunchCoordinatorDelegate, LaunchFinish
                    level: .info,
                    category: .coordinator)
 
+        // Create the TabManager instance that will be associated with this browser
+        let profile: Profile = AppContainer.shared.resolve()
+        // TODO: [7885] Once iPad multi-window enabled each TabManager will likely share same default image store.
+        let imageStore = DefaultDiskImageStore(
+            files: profile.files,
+            namespace: "TabManagerScreenshots",
+            quality: UIConstants.ScreenshotQuality)
+        let tabManager = TabManagerImplementation(profile: profile, imageStore: imageStore)
         let browserCoordinator = BrowserCoordinator(router: router,
-                                                    screenshotService: screenshotService)
+                                                    screenshotService: screenshotService,
+                                                    tabManager: tabManager)
         add(child: browserCoordinator)
         browserCoordinator.start(with: launchType)
 

Client/Coordinators/SettingsCoordinator.swift

@@ -33,7 +33,7 @@ class SettingsCoordinator: BaseCoordinator,
     init(router: Router,
          wallpaperManager: WallpaperManagerInterface = WallpaperManager(),
          profile: Profile = AppContainer.shared.resolve(),
-         tabManager: TabManager = AppContainer.shared.resolve(),
+         tabManager: TabManager,
          themeManager: ThemeManager = AppContainer.shared.resolve()) {
         self.wallpaperManager = wallpaperManager
         self.profile = profile
@@ -84,7 +84,9 @@ class SettingsCoordinator: BaseCoordinator,
             return viewController
 
         case .homePage:
-            let viewController = HomePageSettingViewController(prefs: profile.prefs, settingsDelegate: self)
+            let viewController = HomePageSettingViewController(prefs: profile.prefs,
+                                                               settingsDelegate: self,
+                                                               tabManager: tabManager)
             viewController.profile = profile
             return viewController
 
@@ -259,7 +261,8 @@ class SettingsCoordinator: BaseCoordinator,
 
     func pressedHome() {
         let viewController = HomePageSettingViewController(prefs: profile.prefs,
-                                                           settingsDelegate: self)
+                                                           settingsDelegate: self,
+                                                           tabManager: tabManager)
         viewController.profile = profile
         router.push(viewController)
     }

Client/Coordinators/ShareExtensionCoordinator.swift

@@ -24,7 +24,7 @@ class ShareExtensionCoordinator: BaseCoordinator, DevicePickerViewControllerDele
         profile: Profile,
         parentCoordinator: ParentCoordinatorDelegate? = nil,
         themeManager: ThemeManager = AppContainer.shared.resolve(),
-        tabManager: TabManager = AppContainer.shared.resolve()
+        tabManager: TabManager
     ) {
         self.alertContainer = alertContainer
         self.profile = profile

Client/Frontend/Browser/BrowserViewController.swift

@@ -377,6 +377,11 @@ class BrowserViewController: UIViewController,
         if self.presentedViewController as? PhotonActionSheet != nil {
             self.presentedViewController?.dismiss(animated: true, completion: nil)
         }
+        // Formerly these calls were run during AppDelegate.didEnterBackground(), but we have
+        // individual TabManager instances for each BVC, so we perform these here instead.
+        tabManager.preserveTabs()
+        // TODO: [7856] Some additional updates for telemetry forthcoming, once iPad multi-window is enabled.
+        TabsQuantityTelemetry.trackTabsQuantity(tabManager: tabManager)
     }
 
     @objc

Client/Frontend/Fakespot/FakespotCoordinator.swift

@@ -13,12 +13,16 @@ protocol FakespotCoordinatorDelegate: AnyObject {
 class FakespotCoordinator: BaseCoordinator, FeatureFlaggable {
     weak var parentCoordinator: ParentCoordinatorDelegate?
     private var profile: Profile
+    private let tabManager: TabManager
 
     private var isOptedIn: Bool {
         return profile.prefs.boolForKey(PrefsKeys.Shopping2023OptIn) ?? false
     }
 
-    init(router: Router, profile: Profile = AppContainer.shared.resolve()) {
+    init(router: Router,
+         profile: Profile = AppContainer.shared.resolve(),
+         tabManager: TabManager) {
+        self.tabManager = tabManager
         self.profile = profile
         super.init(router: router)
     }
@@ -64,15 +68,15 @@ class FakespotCoordinator: BaseCoordinator, FeatureFlaggable {
 
     private func createFakespotViewController(productURL: URL) -> FakespotViewController {
         let viewModel = createFakespotViewModel(productURL: productURL)
-        let fakespotViewController = FakespotViewController(viewModel: viewModel)
+        let fakespotViewController = FakespotViewController(viewModel: viewModel, tabManager: tabManager)
         return fakespotViewController
     }
 
     private func createFakespotViewModel(productURL: URL) -> FakespotViewModel {
         let environment = featureFlags.isCoreFeatureEnabled(.useStagingFakespotAPI) ? FakespotEnvironment.staging : .prod
         let viewModel = FakespotViewModel(shoppingProduct: ShoppingProduct(
             url: productURL,
-            client: FakespotClient(environment: environment)))
+            client: FakespotClient(environment: environment)), tabManager: tabManager)
         return viewModel
     }
 }

Client/Frontend/Fakespot/FakespotOptInCardViewModel.swift

@@ -56,7 +56,7 @@ struct FakespotOptInCardViewModel {
 
     // MARK: Init
     init(profile: Profile = AppContainer.shared.resolve(),
-         tabManager: TabManager = AppContainer.shared.resolve()) {
+         tabManager: TabManager) {
         self.tabManager = tabManager
         prefs = profile.prefs
 

Client/Frontend/Fakespot/FakespotViewController.swift

@@ -100,15 +100,19 @@ class FakespotViewController:
         button.accessibilityIdentifier = AccessibilityIdentifiers.Shopping.sheetCloseButton
     }
 
+    private let tabManager: TabManager
+
     // MARK: - Initializers
     init(
         viewModel: FakespotViewModel,
         notificationCenter: NotificationProtocol = NotificationCenter.default,
-        themeManager: ThemeManager = AppContainer.shared.resolve()
+        themeManager: ThemeManager = AppContainer.shared.resolve(),
+        tabManager: TabManager
     ) {
         self.viewModel = viewModel
         self.notificationCenter = notificationCenter
         self.themeManager = themeManager
+        self.tabManager = tabManager
         super.init(nibName: nil, bundle: nil)
 
         listenToStateChange()
@@ -427,7 +431,7 @@ class FakespotViewController:
         case .productAdCard(let adData):
             guard viewModel.areAdsEnabled else { return nil }
             let view: FakespotAdView = .build()
-            var viewModel = FakespotAdViewModel(productAdsData: adData)
+            var viewModel = FakespotAdViewModel(tabManager: tabManager, productAdsData: adData)
             viewModel.dismissViewController = {
                 store.dispatch(FakespotAction.setAppearanceTo(false))
             }

Client/Frontend/Fakespot/FakespotViewModel.swift

@@ -300,16 +300,20 @@ class FakespotViewModel {
         a11yDescriptionIdentifier: AccessibilityIdentifiers.Shopping.InfoComingSoonCard.description
     )
 
-    let settingsCardViewModel = FakespotSettingsCardViewModel()
+    let settingsCardViewModel: FakespotSettingsCardViewModel
     var noAnalysisCardViewModel = FakespotNoAnalysisCardViewModel()
-    let reviewQualityCardViewModel = FakespotReviewQualityCardViewModel()
-    var optInCardViewModel = FakespotOptInCardViewModel()
+    let reviewQualityCardViewModel: FakespotReviewQualityCardViewModel
+    var optInCardViewModel: FakespotOptInCardViewModel
 
     private var analyzeCount = 0
 
     init(shoppingProduct: ShoppingProduct,
-         profile: Profile = AppContainer.shared.resolve()) {
+         profile: Profile = AppContainer.shared.resolve(),
+         tabManager: TabManager) {
         self.shoppingProduct = shoppingProduct
+        self.settingsCardViewModel = FakespotSettingsCardViewModel(tabManager: tabManager)
+        self.reviewQualityCardViewModel = FakespotReviewQualityCardViewModel(tabManager: tabManager)
+        optInCardViewModel = FakespotOptInCardViewModel(tabManager: tabManager)
         optInCardViewModel.productSitename = shoppingProduct.product?.sitename
         optInCardViewModel.supportedTLDWebsites = shoppingProduct.supportedTLDWebsites
         reviewQualityCardViewModel.productSitename = shoppingProduct.product?.sitename

Client/Frontend/Fakespot/Views/FakespotAdView.swift

@@ -24,7 +24,7 @@ struct FakespotAdViewModel {
     let productAdsData: ProductAdsResponse
 
     // MARK: Init
-    init(tabManager: TabManager = AppContainer.shared.resolve(),
+    init(tabManager: TabManager,
          productAdsData: ProductAdsResponse) {
         self.tabManager = tabManager
         self.productAdsData = productAdsData

Client/Frontend/Fakespot/Views/FakespotReviewQualityCardView.swift

@@ -33,7 +33,7 @@ final class FakespotReviewQualityCardViewModel {
     var learnMoreButtonA11yId = AccessibilityIdentifiers.Shopping.ReviewQualityCard.learnMoreButtonTitle
 
     // MARK: Init
-    init(tabManager: TabManager = AppContainer.shared.resolve()) {
+    init(tabManager: TabManager) {
         self.tabManager = tabManager
     }
 

Client/Frontend/Fakespot/Views/FakespotSettingsCardView.swift

@@ -53,7 +53,7 @@ class FakespotSettingsCardViewModel {
     }
 
     init(profile: Profile = AppContainer.shared.resolve(),
-         tabManager: TabManager = AppContainer.shared.resolve()) {
+         tabManager: TabManager) {
         prefs = profile.prefs
         self.tabManager = tabManager
     }