Bugfix FXIOS-8309 [v122.1] System theme resetting bug (#18429)

* fix issue

* improvements because they're nice

* fix mock

BrowserKit/Sources/Common/Theming/DefaultThemeManager.swift

@@ -47,10 +47,14 @@ public final class DefaultThemeManager: ThemeManager, Notifiable {
         return userDefaults.bool(forKey: ThemeKeys.PrivateMode.isOn)
     }
 
-    public var isSystemThemeOn: Bool {
+    public var systemThemeIsOn: Bool {
         return userDefaults.bool(forKey: ThemeKeys.systemThemeIsOn)
     }
 
+    public var automaticBrightnessIsOn: Bool {
+        return userDefaults.bool(forKey: ThemeKeys.AutomaticBrightness.isOn)
+    }
+
     // MARK: - Initializers
 
     public init(
@@ -64,8 +68,6 @@ public final class DefaultThemeManager: ThemeManager, Notifiable {
         self.mainQueue = mainQueue
         self.sharedContainerIdentifier = sharedContainerIdentifier
 
-        migrateDefaultsToUseStandard()
-
         self.userDefaults.register(defaults: [
             ThemeKeys.systemThemeIsOn: true,
             ThemeKeys.NightMode.isOn: NSNumber(value: false),
@@ -93,7 +95,7 @@ public final class DefaultThemeManager: ThemeManager, Notifiable {
         // the system theme is off
         // OR night mode is on
         // OR private mode is on
-        guard isSystemThemeOn,
+        guard systemThemeIsOn,
               !nightModeIsOn,
               !privateModeIsOn
         else { return }
@@ -104,9 +106,9 @@ public final class DefaultThemeManager: ThemeManager, Notifiable {
     public func setSystemTheme(isOn: Bool) {
         userDefaults.set(isOn, forKey: ThemeKeys.systemThemeIsOn)
 
-        if isOn {
+        if systemThemeIsOn {
             systemThemeChanged()
-        } else if userDefaults.bool(forKey: ThemeKeys.AutomaticBrightness.isOn) {
+        } else if automaticBrightnessIsOn {
             updateThemeBasedOnBrightness()
         }
     }
@@ -118,8 +120,7 @@ public final class DefaultThemeManager: ThemeManager, Notifiable {
     }
 
     public func setAutomaticBrightness(isOn: Bool) {
-        let currentState = userDefaults.bool(forKey: ThemeKeys.AutomaticBrightness.isOn)
-        guard currentState != isOn else { return }
+        guard automaticBrightnessIsOn != isOn else { return }
 
         userDefaults.set(isOn, forKey: ThemeKeys.AutomaticBrightness.isOn)
         brightnessChanged()
@@ -181,9 +182,7 @@ public final class DefaultThemeManager: ThemeManager, Notifiable {
     }
 
     private func brightnessChanged() {
-        let brightnessIsOn = userDefaults.bool(forKey: ThemeKeys.AutomaticBrightness.isOn)
-
-        if brightnessIsOn {
+        if automaticBrightnessIsOn {
             updateThemeBasedOnBrightness()
         } else {
             systemThemeChanged()
@@ -201,23 +200,6 @@ public final class DefaultThemeManager: ThemeManager, Notifiable {
         }
     }
 
-    private func migrateDefaultsToUseStandard() {
-        guard let oldDefaultsStore = UserDefaults(suiteName: sharedContainerIdentifier) else { return }
-
-        if let systemThemeIsOn = oldDefaultsStore.value(forKey: ThemeKeys.systemThemeIsOn) {
-            userDefaults.set(systemThemeIsOn, forKey: ThemeKeys.systemThemeIsOn)
-        }
-        if let nightModeIsOn = oldDefaultsStore.value(forKey: ThemeKeys.NightMode.isOn) {
-            userDefaults.set(nightModeIsOn, forKey: ThemeKeys.NightMode.isOn)
-        }
-        if let automaticBrightnessIsOn = oldDefaultsStore.value(forKey: ThemeKeys.AutomaticBrightness.isOn) {
-            userDefaults.set(automaticBrightnessIsOn, forKey: ThemeKeys.AutomaticBrightness.isOn)
-        }
-        if let automaticBrighnessValue = oldDefaultsStore.value(forKey: ThemeKeys.AutomaticBrightness.thresholdValue) {
-            userDefaults.set(automaticBrighnessValue, forKey: ThemeKeys.AutomaticBrightness.thresholdValue)
-        }
-    }
-
     // MARK: - Notifiable
 
     public func handleNotifications(_ notification: Notification) {

BrowserKit/Sources/Common/Theming/ThemeManager.swift

@@ -8,6 +8,9 @@ public protocol ThemeManager {
     var currentTheme: Theme { get }
     var window: UIWindow? { get set }
 
+    var systemThemeIsOn: Bool { get }
+    var automaticBrightnessIsOn: Bool { get }
+
     func changeCurrentTheme(_ newTheme: ThemeType)
     func systemThemeChanged()
     func setSystemTheme(isOn: Bool)

firefox-ios/Client/Frontend/Settings/Main/General/ThemeSetting.swift

@@ -3,10 +3,12 @@
 // file, You can obtain one at http://mozilla.org/MPL/2.0/
 
 import Foundation
+import Common
 
 class ThemeSetting: Setting {
     private weak var settingsDelegate: GeneralSettingsDelegate?
     private let profile: Profile
+    private let themeManager: ThemeManager
 
     override var accessoryView: UIImageView? {
         return SettingDisclosureUtility.buildDisclosureIndicator(theme: theme)
@@ -18,20 +20,25 @@ class ThemeSetting: Setting {
     }
 
     override var status: NSAttributedString {
-        if LegacyThemeManager.instance.systemThemeIsOn {
+        if themeManager.systemThemeIsOn {
             return NSAttributedString(string: .SystemThemeSectionHeader)
-        } else if !LegacyThemeManager.instance.automaticBrightnessIsOn {
+        } else if !themeManager.automaticBrightnessIsOn {
             return NSAttributedString(string: .DisplayThemeManualStatusLabel)
-        } else if LegacyThemeManager.instance.automaticBrightnessIsOn {
+        } else if themeManager.automaticBrightnessIsOn {
             return NSAttributedString(string: .DisplayThemeAutomaticStatusLabel)
         }
+
         return NSAttributedString(string: "")
     }
 
     init(settings: SettingsTableViewController,
-         settingsDelegate: GeneralSettingsDelegate?) {
+         settingsDelegate: GeneralSettingsDelegate?,
+         themeManager: ThemeManager = AppContainer.shared.resolve()
+    ) {
         self.profile = settings.profile
         self.settingsDelegate = settingsDelegate
+        self.themeManager = themeManager
+
         super.init(
             title: NSAttributedString(
                 string: .SettingsDisplayThemeTitle,

firefox-ios/Client/Frontend/Settings/ThemeSettings/ThemeMiddleware.swift

@@ -31,7 +31,7 @@ class ThemeManagerMiddleware: ThemeManagerProvider {
             store.dispatch(ThemeSettingsAction.receivedThemeManagerValues(currentThemeState))
         case ThemeSettingsAction.toggleUseSystemAppearance(let enabled):
             self.toggleUseSystemAppearance(enabled)
-            store.dispatch(ThemeSettingsAction.systemThemeChanged(self.legacyThemeManager.systemThemeIsOn))
+            store.dispatch(ThemeSettingsAction.systemThemeChanged(self.themeManager.systemThemeIsOn))
         case ThemeSettingsAction.enableAutomaticBrightness(let enabled):
             self.toggleAutomaticBrightness(enabled)
             store.dispatch(
@@ -58,15 +58,14 @@ class ThemeManagerMiddleware: ThemeManagerProvider {
     func getCurrentThemeManagerState(windowUUID: WindowUUID?) -> ThemeSettingsState {
         // TODO: [8188] Revisit UUID handling, needs additional investigation.
         ThemeSettingsState(windowUUID: windowUUID ?? WindowUUID.unavailable,
-                           useSystemAppearance: legacyThemeManager.systemThemeIsOn,
+                           useSystemAppearance: themeManager.systemThemeIsOn,
                            isAutomaticBrightnessEnable: legacyThemeManager.automaticBrightnessIsOn,
                            manualThemeSelected: themeManager.currentTheme.type,
                            userBrightnessThreshold: legacyThemeManager.automaticBrightnessValue,
                            systemBrightness: getScreenBrightness())
     }
 
     func toggleUseSystemAppearance(_ enabled: Bool) {
-        legacyThemeManager.systemThemeIsOn = enabled
         themeManager.setSystemTheme(isOn: enabled)
     }
 
@@ -79,10 +78,10 @@ class ThemeManagerMiddleware: ThemeManagerProvider {
         themeManager.setAutomaticBrightness(isOn: enabled)
     }
 
-    func updateManualTheme(_ theme: ThemeType) {
-        let isLightTheme = theme == .light
+    func updateManualTheme(_ newTheme: ThemeType) {
+        let isLightTheme = newTheme == .light
         legacyThemeManager.current = isLightTheme ? LegacyNormalTheme() : LegacyDarkTheme()
-        themeManager.changeCurrentTheme(isLightTheme ? .light : .dark)
+        themeManager.changeCurrentTheme(newTheme)
     }
 
     func updateUserBrightness(_ value: Float) {

firefox-ios/Client/Frontend/Theme/LegacyThemeManager/LegacyThemeManager.swift

@@ -45,18 +45,12 @@ class LegacyThemeManager {
         }
     }
 
-    var systemThemeIsOn: Bool {
-        didSet {
-            UserDefaults.standard.set(
-                systemThemeIsOn,
-                forKey: LegacyThemeManagerPrefs.systemThemeIsOn.rawValue
-            )
-        }
+    private var systemThemeIsOn: Bool {
+        return UserDefaults.standard.bool(forKey: LegacyThemeManagerPrefs.systemThemeIsOn.rawValue)
     }
 
     private init() {
         UserDefaults.standard.register(defaults: [LegacyThemeManagerPrefs.systemThemeIsOn.rawValue: true])
-        systemThemeIsOn = UserDefaults.standard.bool(forKey: LegacyThemeManagerPrefs.systemThemeIsOn.rawValue)
 
         NotificationCenter.default.addObserver(self,
                                                selector: #selector(brightnessChanged),

firefox-ios/Client/Telemetry/TelemetryWrapper.swift

@@ -161,14 +161,17 @@ class TelemetryWrapper: TelemetryWrapperProtocol, FeatureFlaggable {
         // If the setting exists at the key location, use that value. Otherwise record the default
         // value for that preference to ensure it makes it into the metrics ping.
         let prefs = profile.prefs
+
         // FxA Account Login status
         GleanMetrics.Preferences.fxaLoggedIn.set(profile.hasSyncableAccount())
+
         // Record New Tab setting
         if let newTabChoice = prefs.stringForKey(NewTabAccessors.HomePrefKey) {
             GleanMetrics.Preferences.newTabExperience.set(newTabChoice)
         } else {
             GleanMetrics.Preferences.newTabExperience.set(NewTabAccessors.Default.rawValue)
         }
+
         // Record `Home` setting, where Firefox Home is "Home", a custom URL is "other" and blank is "Blank".
         let homePageSetting = NewTabAccessors.getHomePage(prefs)
         switch homePageSetting {
@@ -181,45 +184,54 @@ class TelemetryWrapper: TelemetryWrapperProtocol, FeatureFlaggable {
         default:
             GleanMetrics.Preferences.homePageSetting.set(homePageSetting.rawValue)
         }
+
         // Notifications
         GleanMetrics.Preferences.tipsAndFeaturesNotifs.set(UserDefaults.standard.bool(forKey: PrefsKeys.Notifications.TipsAndFeaturesNotifications))
         GleanMetrics.Preferences.syncNotifs.set(UserDefaults.standard.bool(forKey: PrefsKeys.Notifications.SyncNotifications))
+
         // Save logins
         if let saveLogins = prefs.boolForKey(PrefsKeys.LoginsSaveEnabled) {
             GleanMetrics.Preferences.saveLogins.set(saveLogins)
         } else {
             GleanMetrics.Preferences.saveLogins.set(true)
         }
+
         // Show clipboard bar
         if let showClipboardBar = prefs.boolForKey("showClipboardBar") {
             GleanMetrics.Preferences.showClipboardBar.set(showClipboardBar)
         } else {
             GleanMetrics.Preferences.showClipboardBar.set(false)
         }
+
         // Close private tabs
         if let closePrivateTabs = prefs.boolForKey("settings.closePrivateTabs") {
             GleanMetrics.Preferences.closePrivateTabs.set(closePrivateTabs)
         } else {
             GleanMetrics.Preferences.closePrivateTabs.set(false)
         }
+
         // Tracking protection - enabled
         if let tpEnabled = prefs.boolForKey(ContentBlockingConfig.Prefs.EnabledKey) {
             GleanMetrics.TrackingProtection.enabled.set(tpEnabled)
         } else {
             GleanMetrics.TrackingProtection.enabled.set(true)
         }
+
         // Tracking protection - strength
         if let tpStrength = prefs.stringForKey(ContentBlockingConfig.Prefs.StrengthKey) {
             GleanMetrics.TrackingProtection.strength.set(tpStrength)
         } else {
             GleanMetrics.TrackingProtection.strength.set("basic")
         }
+
         // System theme enabled
         let themeManager = DefaultThemeManager(sharedContainerIdentifier: AppInfo.sharedContainerIdentifier)
-        GleanMetrics.Theme.useSystemTheme.set(themeManager.isSystemThemeOn)
+        GleanMetrics.Theme.useSystemTheme.set(themeManager.systemThemeIsOn)
+
         // Installed Mozilla applications
         GleanMetrics.InstalledMozillaProducts.focus.set(UIApplication.shared.canOpenURL(URL(string: "firefox-focus://")!))
         GleanMetrics.InstalledMozillaProducts.klar.set(UIApplication.shared.canOpenURL(URL(string: "firefox-klar://")!))
+
         // Device Authentication
         GleanMetrics.Device.authentication.set(AppAuthenticator().canAuthenticateDeviceOwner)
 

firefox-ios/firefox-ios-tests/Tests/ClientTests/Mocks/MockThemeManager.swift

@@ -9,6 +9,10 @@ class MockThemeManager: ThemeManager {
     var currentTheme: Theme = LightTheme()
     var window: UIWindow?
 
+    var systemThemeIsOn: Bool { return true}
+
+    var automaticBrightnessIsOn: Bool { return false}
+
     func getInterfaceStyle() -> UIUserInterfaceStyle {
         return .light
     }