ensure only one history database context (#3068)

Core/HistoryManager.swift

@@ -25,9 +25,11 @@ import Common
 import Persistence
 
 public protocol HistoryManaging {
-
+    
     var historyCoordinator: HistoryCoordinating { get }
-    func loadStore(onCleanFinished: @escaping () -> Void) throws
+    func isHistoryFeatureEnabled() -> Bool
+    var isEnabledByUser: Bool { get }
+    func removeAllHistory() async
 
 }
 
@@ -44,43 +46,38 @@ public class HistoryManager: HistoryManaging {
 
     let privacyConfigManager: PrivacyConfigurationManaging
     let variantManager: VariantManager
-    let database: CoreDataDatabase
     let internalUserDecider: InternalUserDecider
-    let isEnabledByUser: () -> Bool
-
-    private var currentHistoryCoordinator: HistoryCoordinating?
+    let dbCoordinator: HistoryCoordinator
 
     public var historyCoordinator: HistoryCoordinating {
         guard isHistoryFeatureEnabled(),
-                isEnabledByUser() else {
-            currentHistoryCoordinator = nil
+                isEnabledByUser else {
             return NullHistoryCoordinator()
         }
+        return dbCoordinator
+    }
 
-        if let currentHistoryCoordinator {
-            return currentHistoryCoordinator
-        }
+    public let isAutocompleteEnabledByUser: () -> Bool
+    public let isRecentlyVisitedSitesEnabledByUser: () -> Bool
 
-        let coordinator = makeDatabaseHistoryCoordinator()
-        coordinator.loadHistory {
-            // no-op - only done here in case it was flipped in settings
-        }
-
-        currentHistoryCoordinator = coordinator
-        return coordinator
+    public var isEnabledByUser: Bool {
+        return isAutocompleteEnabledByUser() && isRecentlyVisitedSitesEnabledByUser()
     }
 
-    public init(privacyConfigManager: PrivacyConfigurationManaging,
-                variantManager: VariantManager,
-                database: CoreDataDatabase,
-                internalUserDecider: InternalUserDecider,
-                isEnabledByUser: @autoclosure @escaping () -> Bool) {
+    /// Use `make()`
+    init(privacyConfigManager: PrivacyConfigurationManaging,
+         variantManager: VariantManager,
+         internalUserDecider: InternalUserDecider,
+         dbCoordinator: HistoryCoordinator,
+         isAutocompleteEnabledByUser: @autoclosure @escaping () -> Bool,
+         isRecentlyVisitedSitesEnabledByUser: @autoclosure @escaping () -> Bool) {
 
         self.privacyConfigManager = privacyConfigManager
         self.variantManager = variantManager
-        self.database = database
         self.internalUserDecider = internalUserDecider
-        self.isEnabledByUser = isEnabledByUser
+        self.dbCoordinator = dbCoordinator
+        self.isAutocompleteEnabledByUser = isAutocompleteEnabledByUser
+        self.isRecentlyVisitedSitesEnabledByUser = isRecentlyVisitedSitesEnabledByUser
     }
 
     /// Determines if the history feature is enabled.  This code will need to be cleaned up once the roll out is at 100%
@@ -106,25 +103,12 @@ public class HistoryManager: HistoryManaging {
 
     public func removeAllHistory() async {
         await withCheckedContinuation { continuation in
-            historyCoordinator.burnAll {
+            dbCoordinator.burnAll {
                 continuation.resume()
             }
         }
     }
 
-    public func loadStore(onCleanFinished: @escaping () -> Void) {
-        let coordinator = makeDatabaseHistoryCoordinator()
-        coordinator.loadHistory {
-            onCleanFinished()
-        }
-    }
-
-    private func makeDatabaseHistoryCoordinator() -> HistoryCoordinator {
-        let context = database.makeContext(concurrencyType: .privateQueueConcurrencyType)
-        let historyCoordinator = HistoryCoordinator(historyStoring: HistoryStore(context: context, eventMapper: HistoryStoreEventMapper()))
-        return historyCoordinator
-    }
-
 }
 
 class NullHistoryCoordinator: HistoryCoordinating {
@@ -244,3 +228,59 @@ class HistoryStoreEventMapper: EventMapping<HistoryStore.HistoryStoreEvents> {
         fatalError("Use init()")
     }
 }
+
+extension HistoryManager {
+
+    /// Should only be called once in the app
+    public static func make(isAutocompleteEnabledByUser: @autoclosure @escaping () -> Bool,
+                            isRecentlyVisitedSitesEnabledByUser: @autoclosure @escaping () -> Bool,
+                            internalUserDecider: InternalUserDecider,
+                            privacyConfigManager: PrivacyConfigurationManaging) -> Result<HistoryManager, Error> {
+
+        let database = HistoryDatabase.make()
+        var loadError: Error?
+        database.loadStore { _, error in
+            loadError = error
+        }
+
+        if let loadError {
+            return .failure(loadError)
+        }
+
+        let context = database.makeContext(concurrencyType: .privateQueueConcurrencyType)
+        let dbCoordinator = HistoryCoordinator(historyStoring: HistoryStore(context: context, eventMapper: HistoryStoreEventMapper()))
+
+        let historyManager = HistoryManager(privacyConfigManager: privacyConfigManager,
+                                            variantManager: DefaultVariantManager(),
+                                            internalUserDecider: internalUserDecider,
+                                            dbCoordinator: dbCoordinator,
+                                            isAutocompleteEnabledByUser: isAutocompleteEnabledByUser(),
+                                            isRecentlyVisitedSitesEnabledByUser: isRecentlyVisitedSitesEnabledByUser())
+
+        dbCoordinator.loadHistory(onCleanFinished: {
+            // Do future migrations after clean has finished.  See macOS for an example.
+        })
+
+        return .success(historyManager)
+    }
+
+}
+
+// Available in case `make` fails so that we don't have to pass optional around.
+public struct NullHistoryManager: HistoryManaging {
+
+    public var isEnabledByUser = false
+
+    public let historyCoordinator: HistoryCoordinating = NullHistoryCoordinator()
+
+    public func removeAllHistory() async {
+        // No-op
+    }
+
+    public func isHistoryFeatureEnabled() -> Bool {
+        return false
+    }
+
+    public init() { }
+
+}

DuckDuckGo/AppDelegate.swift

@@ -279,9 +279,7 @@ import WebKit
             }
 
         let previewsSource = TabPreviewsSource()
-        let historyManager = makeHistoryManager(AppDependencyProvider.shared.appSettings,
-                                                AppDependencyProvider.shared.internalUserDecider,
-                                                ContentBlocking.shared.privacyConfigurationManager)
+        let historyManager = makeHistoryManager()
         let tabsModel = prepareTabsModel(previewsSource: previewsSource)
 
         let main = MainViewController(bookmarksDatabase: bookmarksDatabase,
@@ -346,6 +344,29 @@ import WebKit
         return true
     }
 
+    private func makeHistoryManager() -> HistoryManaging {
+
+        let settings = AppDependencyProvider.shared.appSettings
+
+        switch HistoryManager.make(isAutocompleteEnabledByUser: settings.autocomplete,
+                                   isRecentlyVisitedSitesEnabledByUser: settings.recentlyVisitedSites,
+                                   internalUserDecider: AppDependencyProvider.shared.internalUserDecider,
+                                   privacyConfigManager: ContentBlocking.shared.privacyConfigurationManager) {
+
+        case .failure(let error):
+            Pixel.fire(pixel: .historyStoreLoadFailed, error: error)
+            if error.isDiskFull {
+                self.presentInsufficientDiskSpaceAlert()
+            } else {
+                self.presentPreemptiveCrashAlert()
+            }
+            return NullHistoryManager()
+
+        case .success(let historyManager):
+            return historyManager
+        }
+    }
+
     private func prepareTabsModel(previewsSource: TabPreviewsSource = TabPreviewsSource(),
                                   appSettings: AppSettings = AppDependencyProvider.shared.appSettings,
                                   isDesktop: Bool = UIDevice.current.userInterfaceIdiom == .pad) -> TabsModel {
@@ -367,39 +388,6 @@ import WebKit
         return tabsModel
     }
 
-    private func makeHistoryManager(_ appSettings: AppSettings,
-                                    _ internalUserDecider: InternalUserDecider,
-                                    _ privacyConfigManager: PrivacyConfigurationManaging) -> HistoryManager {
-
-        let db = HistoryDatabase.make()
-        var loadError: Error?
-        db.loadStore { _, error in
-            loadError = error
-        }
-
-        if let loadError {
-            Pixel.fire(pixel: .historyStoreLoadFailed, error: loadError)
-            if loadError.isDiskFull {
-                self.presentInsufficientDiskSpaceAlert()
-            } else {
-                self.presentPreemptiveCrashAlert()
-            }
-        }
-
-        let historyManager = HistoryManager(privacyConfigManager: privacyConfigManager,
-                                            variantManager: DefaultVariantManager(),
-                                            database: db,
-                                            internalUserDecider: internalUserDecider,
-                                            isEnabledByUser: appSettings.recentlyVisitedSites)
-
-        // Ensure we don't do this if the history is disabled in privacy confg
-        guard historyManager.isHistoryFeatureEnabled() else { return historyManager }
-        historyManager.loadStore(onCleanFinished: {
-            // Do future migrations after clean has finished.  See macOS for an example.
-        })
-        return historyManager
-    }
-
     private func presentPreemptiveCrashAlert() {
         Task { @MainActor in
             let alertController = CriticalAlerts.makePreemptiveCrashAlert()

DuckDuckGo/AutocompleteViewController.swift

@@ -40,7 +40,7 @@ class AutocompleteViewController: UIHostingController<AutocompleteView> {
     weak var delegate: AutocompleteViewControllerDelegate?
     weak var presentationDelegate: AutocompleteViewControllerPresentationDelegate?
 
-    private let historyManager: HistoryManager
+    private let historyManager: HistoryManaging
     var historyCoordinator: HistoryCoordinating {
         historyManager.historyCoordinator
     }
@@ -63,7 +63,7 @@ class AutocompleteViewController: UIHostingController<AutocompleteView> {
 
     private var historyMessageManager: HistoryMessageManager
 
-    init(historyManager: HistoryManager,
+    init(historyManager: HistoryManaging,
          bookmarksDatabase: CoreDataDatabase,
          appSettings: AppSettings,
          historyMessageManager: HistoryMessageManager = HistoryMessageManager()) {

DuckDuckGo/MainViewController.swift

@@ -170,13 +170,13 @@ class MainViewController: UIViewController {
         fatalError("Use init?(code:")
     }
 
-    var historyManager: HistoryManager
+    var historyManager: HistoryManaging
     var viewCoordinator: MainViewCoordinator!
 
     init(
         bookmarksDatabase: CoreDataDatabase,
         bookmarksDatabaseCleaner: BookmarkDatabaseCleaner,
-        historyManager: HistoryManager,
+        historyManager: HistoryManaging,
         syncService: DDGSyncing,
         syncDataProviders: SyncDataProviders,
         appSettings: AppSettings,

DuckDuckGo/SettingsViewModel.swift

@@ -44,7 +44,7 @@ final class SettingsViewModel: ObservableObject {
     private let voiceSearchHelper: VoiceSearchHelperProtocol
     private let syncPausedStateManager: any SyncPausedStateManaging
     var emailManager: EmailManager { EmailManager() }
-    private let historyManager: HistoryManager
+    private let historyManager: HistoryManaging
 
     // Subscription Dependencies
     private let subscriptionManager: SubscriptionManager
@@ -164,6 +164,7 @@ final class SettingsViewModel: ObservableObject {
             set: {
                 self.appSettings.autocomplete = $0
                 self.state.autocomplete = $0
+                self.clearHistoryIfNeeded()
                 self.updateRecentlyVisitedSitesVisibility()
 
                 if $0 {
@@ -181,6 +182,7 @@ final class SettingsViewModel: ObservableObject {
             set: {
                 self.appSettings.autocomplete = $0
                 self.state.autocomplete = $0
+                self.clearHistoryIfNeeded()
                 self.updateRecentlyVisitedSitesVisibility()
 
                 if $0 {
@@ -203,6 +205,7 @@ final class SettingsViewModel: ObservableObject {
                 } else {
                     Pixel.fire(pixel: .settingsRecentlyVisitedOff)
                 }
+                self.clearHistoryIfNeeded()
             }
         )
     }
@@ -331,7 +334,7 @@ final class SettingsViewModel: ObservableObject {
          voiceSearchHelper: VoiceSearchHelperProtocol = AppDependencyProvider.shared.voiceSearchHelper,
          variantManager: VariantManager = AppDependencyProvider.shared.variantManager,
          deepLink: SettingsDeepLinkSection? = nil,
-         historyManager: HistoryManager,
+         historyManager: HistoryManaging,
          syncPausedStateManager: any SyncPausedStateManaging) {
 
         self.state = SettingsState.defaults
@@ -401,6 +404,14 @@ extension SettingsViewModel {
         }
     }
 
+    private func clearHistoryIfNeeded() {
+        if !historyManager.isEnabledByUser {
+            Task {
+                await self.historyManager.removeAllHistory()
+            }
+        }
+    }
+
     private func getNetworkProtectionState() -> SettingsState.NetworkProtection {
         return SettingsState.NetworkProtection(enabled: false, status: "")
     }

DuckDuckGo/SuggestionTrayViewController.swift

@@ -49,7 +49,7 @@ class SuggestionTrayViewController: UIViewController {
     private var willRemoveAutocomplete = false
     private let bookmarksDatabase: CoreDataDatabase
     private let favoritesModel: FavoritesListInteracting
-    private let historyManager: HistoryManager
+    private let historyManager: HistoryManaging
 
     var selectedSuggestion: Suggestion? {
         autocompleteController?.selectedSuggestion
@@ -79,7 +79,7 @@ class SuggestionTrayViewController: UIViewController {
         }
     }
 
-    required init?(coder: NSCoder, favoritesViewModel: FavoritesListInteracting, bookmarksDatabase: CoreDataDatabase, historyManager: HistoryManager) {
+    required init?(coder: NSCoder, favoritesViewModel: FavoritesListInteracting, bookmarksDatabase: CoreDataDatabase, historyManager: HistoryManaging) {
         self.favoritesModel = favoritesViewModel
         self.bookmarksDatabase = bookmarksDatabase
         self.historyManager = historyManager

DuckDuckGo/TabManager.swift

@@ -32,7 +32,7 @@ class TabManager {
     private var tabControllerCache = [TabViewController]()
 
     private let bookmarksDatabase: CoreDataDatabase
-    private let historyManager: HistoryManager
+    private let historyManager: HistoryManaging
     private let syncService: DDGSyncing
     private var previewsSource: TabPreviewsSource
 
@@ -45,7 +45,7 @@ class TabManager {
     init(model: TabsModel,
          previewsSource: TabPreviewsSource,
          bookmarksDatabase: CoreDataDatabase,
-         historyManager: HistoryManager,
+         historyManager: HistoryManaging,
          syncService: DDGSyncing) {
         self.model = model
         self.previewsSource = previewsSource

DuckDuckGo/TabViewController.swift

@@ -298,7 +298,7 @@ class TabViewController: UIViewController {
     static func loadFromStoryboard(model: Tab,
                                    appSettings: AppSettings = AppDependencyProvider.shared.appSettings,
                                    bookmarksDatabase: CoreDataDatabase,
-                                   historyManager: HistoryManager,
+                                   historyManager: HistoryManaging,
                                    syncService: DDGSyncing) -> TabViewController {
         let storyboard = UIStoryboard(name: "Tab", bundle: nil)
         let controller = storyboard.instantiateViewController(identifier: "TabViewController", creator: { coder in
@@ -316,7 +316,7 @@ class TabViewController: UIViewController {
         (webView.configuration.userContentController as? UserContentController)!
     }
 
-    let historyManager: HistoryManager
+    let historyManager: HistoryManaging
     let historyCapture: HistoryCapture
 
     var duckPlayer: DuckPlayerProtocol = DuckPlayer()
@@ -326,7 +326,7 @@ class TabViewController: UIViewController {
                    tabModel: Tab,
                    appSettings: AppSettings,
                    bookmarksDatabase: CoreDataDatabase,
-                   historyManager: HistoryManager,
+                   historyManager: HistoryManaging,
                    syncService: DDGSyncing) {
         self.tabModel = tabModel
         self.appSettings = appSettings