Skip to content

Commit

Permalink
Add FXIOS-10869 [ToS] Firefox: Handle Crash Reporting logic for onboa…
Browse files Browse the repository at this point in the history
…rding and settings (#23738)

FXIOS-10869 Firefox: Handle Crash Reporting logic for onboarding and settings
  • Loading branch information
dicarobinho authored Dec 16, 2024
1 parent db4641b commit e64c2e3
Show file tree
Hide file tree
Showing 16 changed files with 96 additions and 63 deletions.
6 changes: 3 additions & 3 deletions BrowserKit/Sources/Common/Logger/CrashManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import Sentry
public protocol CrashManager {
var crashedLastLaunch: Bool { get }
func captureError(error: Error)
func setup(sendUsageData: Bool)
func setup(sendCrashReports: Bool)
func send(message: String,
category: LoggerCategory,
level: LoggerLevel,
Expand Down Expand Up @@ -102,8 +102,8 @@ public class DefaultCrashManager: CrashManager {
return sentryWrapper.crashedInLastRun
}

public func setup(sendUsageData: Bool) {
guard shouldSetup, sendUsageData, let dsn = sentryWrapper.dsn else { return }
public func setup(sendCrashReports: Bool) {
guard shouldSetup, sendCrashReports, let dsn = sentryWrapper.dsn else { return }

sentryWrapper.startWithConfigureOptions(configure: { options in
options.dsn = dsn
Expand Down
4 changes: 2 additions & 2 deletions BrowserKit/Sources/Common/Logger/DefaultLogger.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ public class DefaultLogger: Logger {
self.crashManager = crashManager
}

public func setup(sendUsageData: Bool) {
crashManager?.setup(sendUsageData: sendUsageData)
public func setup(sendCrashReports: Bool) {
crashManager?.setup(sendCrashReports: sendCrashReports)
}

// TODO: FXIOS-7819 need to rethink if this should go to Sentry
Expand Down
2 changes: 1 addition & 1 deletion BrowserKit/Sources/Common/Logger/Logger.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import Foundation
public protocol Logger {
var crashedLastLaunch: Bool { get }

func setup(sendUsageData: Bool)
func setup(sendCrashReports: Bool)
func configure(crashManager: CrashManager)
func logCustomError(error: Error)

Expand Down
22 changes: 11 additions & 11 deletions BrowserKit/Tests/CommonTests/LoggerTests/CrashManagerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ final class CrashManagerTests: XCTestCase {
let subject = DefaultCrashManager(sentryWrapper: sentryWrapper,
isSimulator: true,
skipReleaseNameCheck: true)
subject.setup(sendUsageData: true)
subject.setup(sendCrashReports: true)

XCTAssertEqual(sentryWrapper.startWithConfigureOptionsCalled, 0)
XCTAssertEqual(sentryWrapper.configureScopeCalled, 0)
Expand All @@ -38,7 +38,7 @@ final class CrashManagerTests: XCTestCase {
let subject = DefaultCrashManager(sentryWrapper: sentryWrapper,
isSimulator: false,
skipReleaseNameCheck: true)
subject.setup(sendUsageData: false)
subject.setup(sendCrashReports: false)

XCTAssertEqual(sentryWrapper.startWithConfigureOptionsCalled, 0)
XCTAssertEqual(sentryWrapper.configureScopeCalled, 0)
Expand All @@ -48,7 +48,7 @@ final class CrashManagerTests: XCTestCase {
let subject = DefaultCrashManager(sentryWrapper: sentryWrapper,
isSimulator: false,
skipReleaseNameCheck: true)
subject.setup(sendUsageData: true)
subject.setup(sendCrashReports: true)

XCTAssertEqual(sentryWrapper.startWithConfigureOptionsCalled, 0)
XCTAssertEqual(sentryWrapper.configureScopeCalled, 0)
Expand All @@ -59,7 +59,7 @@ final class CrashManagerTests: XCTestCase {
let subject = DefaultCrashManager(sentryWrapper: sentryWrapper,
isSimulator: false,
skipReleaseNameCheck: true)
subject.setup(sendUsageData: true)
subject.setup(sendCrashReports: true)

XCTAssertEqual(sentryWrapper.startWithConfigureOptionsCalled, 1)
XCTAssertEqual(sentryWrapper.configureScopeCalled, 1)
Expand All @@ -70,8 +70,8 @@ final class CrashManagerTests: XCTestCase {
let subject = DefaultCrashManager(sentryWrapper: sentryWrapper,
isSimulator: false,
skipReleaseNameCheck: true)
subject.setup(sendUsageData: true)
subject.setup(sendUsageData: true)
subject.setup(sendCrashReports: true)
subject.setup(sendCrashReports: true)

XCTAssertEqual(sentryWrapper.startWithConfigureOptionsCalled, 1)
XCTAssertEqual(sentryWrapper.configureScopeCalled, 1)
Expand Down Expand Up @@ -114,7 +114,7 @@ final class CrashManagerTests: XCTestCase {
let subject = DefaultCrashManager(sentryWrapper: sentryWrapper,
isSimulator: false,
skipReleaseNameCheck: true)
subject.setup(sendUsageData: true)
subject.setup(sendCrashReports: true)

subject.send(message: "A message",
category: .setup,
Expand All @@ -132,7 +132,7 @@ final class CrashManagerTests: XCTestCase {
let subject = DefaultCrashManager(sentryWrapper: sentryWrapper,
isSimulator: false,
skipReleaseNameCheck: true)
subject.setup(sendUsageData: true)
subject.setup(sendCrashReports: true)

subject.send(message: "A message",
category: .setup,
Expand All @@ -150,7 +150,7 @@ final class CrashManagerTests: XCTestCase {
let subject = DefaultCrashManager(sentryWrapper: sentryWrapper,
isSimulator: false,
skipReleaseNameCheck: true)
subject.setup(sendUsageData: true)
subject.setup(sendCrashReports: true)

subject.send(message: "A message",
category: .setup,
Expand All @@ -168,7 +168,7 @@ final class CrashManagerTests: XCTestCase {
let subject = DefaultCrashManager(sentryWrapper: sentryWrapper,
isSimulator: false,
skipReleaseNameCheck: true)
subject.setup(sendUsageData: true)
subject.setup(sendCrashReports: true)

subject.send(message: "A message",
category: .setup,
Expand All @@ -186,7 +186,7 @@ final class CrashManagerTests: XCTestCase {
let subject = DefaultCrashManager(sentryWrapper: sentryWrapper,
isSimulator: false,
skipReleaseNameCheck: true)
subject.setup(sendUsageData: true)
subject.setup(sendCrashReports: true)

subject.send(message: "A message",
category: .setup,
Expand Down
18 changes: 9 additions & 9 deletions BrowserKit/Tests/CommonTests/LoggerTests/LoggerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -129,19 +129,19 @@ final class LoggerTests: XCTestCase {
XCTAssertEqual(extra, ["example": "test", "errorDescription": "A description"])
}

func testCrashManagerLog_sendUsageDataNotCalled() {
func testCrashManagerLog_sendCrashReportsNotCalled() {
let subject = DefaultLogger(swiftyBeaverBuilder: beaverBuilder)
subject.configure(crashManager: crashManager)
XCTAssertNil(crashManager.savedSendUsageData)
XCTAssertNil(crashManager.savedSendCrashReports)
}

func testCrashManagerLog_sendUsageDataCalled() throws {
func testCrashManagerLog_sendCrashReportsCalled() throws {
let subject = DefaultLogger(swiftyBeaverBuilder: beaverBuilder)
subject.configure(crashManager: crashManager)
subject.setup(sendUsageData: true)
subject.setup(sendCrashReports: true)

let savedSendUsageData = try XCTUnwrap(crashManager.savedSendUsageData)
XCTAssertTrue(savedSendUsageData)
let savedSendCrashReports = try XCTUnwrap(crashManager.savedSendCrashReports)
XCTAssertTrue(savedSendCrashReports)
}
}

Expand Down Expand Up @@ -201,9 +201,9 @@ class MockSwiftyBeaver: SwiftyBeaverWrapper {
class MockCrashManager: CrashManager {
var crashedLastLaunch = false

var savedSendUsageData: Bool?
func setup(sendUsageData: Bool) {
savedSendUsageData = sendUsageData
var savedSendCrashReports: Bool?
func setup(sendCrashReports: Bool) {
savedSendCrashReports = sendCrashReports
}

var message: String?
Expand Down
13 changes: 10 additions & 3 deletions firefox-ios/Client/Application/AppLaunchUtil.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@ class AppLaunchUtil {
// private var adjustHelper: AdjustHelper
private var profile: Profile
private let introScreenManager: IntroScreenManager
private let termsOfServiceManager: TermsOfServiceManager

init(logger: Logger = DefaultLogger.shared,
profile: Profile) {
self.logger = logger
self.profile = profile
// self.adjustHelper = AdjustHelper(profile: profile)
self.introScreenManager = IntroScreenManager(prefs: profile.prefs)
self.termsOfServiceManager = TermsOfServiceManager(prefs: profile.prefs)
}

func setUpPreLaunchDependencies() {
Expand All @@ -35,9 +37,12 @@ class AppLaunchUtil {
TelemetryWrapper.shared.setup(profile: profile)
recordStartUpTelemetry()

// Need to get "settings.sendUsageData" this way so that Sentry can be initialized before getting the Profile.
let sendUsageData = NSUserDefaultsPrefs(prefix: "profile").boolForKey(AppConstants.prefSendUsageData) ?? true
logger.setup(sendUsageData: sendUsageData)
// Need to get "settings.sendCrashReports" this way so that Sentry can be initialized before getting the Profile.
let sendCrashReports = NSUserDefaultsPrefs(prefix: "profile").boolForKey(AppConstants.prefSendCrashReports) ?? true

// For this phase, we should handle terms of service as accepted, in case of app updates.
logger.setup(sendCrashReports: sendCrashReports && (termsOfServiceManager.isAccepted ||
!introScreenManager.shouldShowIntroScreen))

setUserAgent()

Expand All @@ -56,6 +61,8 @@ class AppLaunchUtil {
// i.e. this must be run before initializing those systems.
LegacyFeatureFlagsManager.shared.initializeDeveloperFeatures(with: profile)

logger.setup(sendCrashReports: sendCrashReports && !termsOfServiceManager.isFeatureEnabled)

// Start initializing the Nimbus SDK. This should be done after Glean
// has been started.
initializeExperiments()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import Common
import Foundation
import Shared

protocol LaunchCoordinatorDelegate: AnyObject {
func didFinishTermsOfService(from coordinator: LaunchCoordinator)
Expand Down Expand Up @@ -49,10 +50,13 @@ class LaunchCoordinator: BaseCoordinator,
// MARK: - Terms of Service
private func presentTermsOfService(with manager: TermsOfServiceManager,
isFullScreen: Bool) {
let viewController = TermsOfServiceViewController(windowUUID: windowUUID)
let viewController = TermsOfServiceViewController(profile: profile, windowUUID: windowUUID)
viewController.didFinishFlow = { [weak self] in
manager.setAccepted()
guard let self = self else { return }
let sendCrashReports = profile.prefs.boolForKey(AppConstants.prefSendCrashReports) ?? true
self.profile.prefs.setBool(sendCrashReports, forKey: AppConstants.prefSendCrashReports)
self.logger.setup(sendCrashReports: sendCrashReports)
self.parentCoordinator?.didFinishTermsOfService(from: self)
}
viewController.modalPresentationStyle = .fullScreen
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class MockLogger: Logger {
var savedLevel: LoggerLevel?
var savedCategory: LoggerCategory?

func setup(sendUsageData: Bool) {}
func setup(sendCrashReports: Bool) {}
func configure(crashManager: Common.CrashManager) {}
func copyLogsToDocuments() {}
func logCustomError(error: Error) {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ final class PrivacyPreferencesViewController: UIViewController,
}

// MARK: - Properties
private var profile: Profile
var windowUUID: WindowUUID
var themeManager: ThemeManager
var themeObserver: (any NSObjectProtocol)?
Expand Down Expand Up @@ -46,16 +47,20 @@ final class PrivacyPreferencesViewController: UIViewController,

private lazy var contentView: UIView = .build()

private lazy var crashReportsSwitch: SwitchDetailedView = .build()
private lazy var crashReportsSwitch: SwitchDetailedView = .build { [weak self] view in
view.setSwitchValue(isOn: self?.profile.prefs.boolForKey(AppConstants.prefSendCrashReports) ?? true)
}

private lazy var technicalDataSwitch: SwitchDetailedView = .build()

// MARK: - Initializers
init(
profile: Profile,
windowUUID: WindowUUID,
themeManager: ThemeManager = AppContainer.shared.resolve(),
notificationCenter: NotificationProtocol = NotificationCenter.default
) {
self.profile = profile
self.windowUUID = windowUUID
self.themeManager = themeManager
self.notificationCenter = notificationCenter
Expand Down Expand Up @@ -154,8 +159,11 @@ final class PrivacyPreferencesViewController: UIViewController,
}

private func setupCallbacks() {
// TODO: FXIOS-10675 Firefox iOS: Manage Privacy Preferences during Onboarding - Logic
crashReportsSwitch.switchCallback = { _ in }
crashReportsSwitch.switchCallback = { [weak self] value in
self?.profile.prefs.setBool(value, forKey: AppConstants.prefSendCrashReports)
}

// TODO: FXIOS-10675 Firefox iOS: Manage Technical Data during Onboarding and Settings
technicalDataSwitch.switchCallback = { _ in }

// TODO: FXIOS-10739 Firefox iOS: Use the correct links for Learn more buttons, in Manage Privacy Preferences screen
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,10 @@ final class SwitchDetailedView: UIView, ThemeApplicable {
actionDescriptionLabel.attributedText = linkedDescription
}

func setSwitchValue(isOn: Bool) {
actionSwitch.isOn = isOn
}

// MARK: - Button actions
@objc
private func switchValueChanged(_ sender: UISwitch) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class TermsOfServiceViewController: UIViewController, Themeable {
}

// MARK: - Properties
private let profile: Profile
var windowUUID: WindowUUID
var themeManager: ThemeManager
var themeObserver: NSObjectProtocol?
Expand Down Expand Up @@ -69,10 +70,12 @@ class TermsOfServiceViewController: UIViewController, Themeable {

// MARK: - Initializers
init(
profile: Profile,
windowUUID: WindowUUID,
themeManager: ThemeManager = AppContainer.shared.resolve(),
notificationCenter: NotificationProtocol = NotificationCenter.default
) {
self.profile = profile
self.windowUUID = windowUUID
self.themeManager = themeManager
self.notificationCenter = notificationCenter
Expand Down Expand Up @@ -245,7 +248,7 @@ class TermsOfServiceViewController: UIViewController, Themeable {

@objc
private func presentManagePreferences(_ gesture: UIGestureRecognizer) {
let managePreferencesVC = PrivacyPreferencesViewController(windowUUID: windowUUID)
let managePreferencesVC = PrivacyPreferencesViewController(profile: profile, windowUUID: windowUUID)
if UIDevice.current.userInterfaceIdiom != .phone {
managePreferencesVC.modalPresentationStyle = .formSheet
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ class AppSettingsTableViewController: SettingsTableViewController,
// MARK: Data settings setup

private func setupDataSettings() {
let isSentCrashReportsEnabled = featureFlags.isFeatureEnabled(.tosFeature, checking: .buildOnly)
let isTermsOfServiceFeatureEnabled = featureFlags.isFeatureEnabled(.tosFeature, checking: .buildOnly)

let anonymousUsageDataSetting = SendDataSetting(
prefs: profile.prefs,
Expand All @@ -198,7 +198,7 @@ class AppSettingsTableViewController: SettingsTableViewController,
}

// Only add these toggles to the Settings if Terms Of Service feature flag is enabled
if isSentCrashReportsEnabled {
if isTermsOfServiceFeatureEnabled {
let sendTechnicalDataSettings = SendDataSetting(
prefs: profile.prefs,
delegate: settingsDelegate,
Expand All @@ -211,18 +211,6 @@ class AppSettingsTableViewController: SettingsTableViewController,
}
sendTechnicalDataSetting = sendTechnicalDataSettings

let sendCrashReportsSettings = SendDataSetting(
prefs: profile.prefs,
delegate: settingsDelegate,
theme: themeManager.getCurrentTheme(for: windowUUID),
settingsDelegate: parentCoordinator,
sendDataType: .crashReports
)
sendCrashReportsSettings.shouldSendData = { value in
// TODO: FXIOS-10754 Firefox iOS: Manage Privacy Preferences in Settings - Logic
}
self.sendCrashReportsSetting = sendCrashReportsSettings

let sendDailyUsagePingSettings = SendDataSetting(
prefs: profile.prefs,
delegate: settingsDelegate,
Expand All @@ -236,6 +224,15 @@ class AppSettingsTableViewController: SettingsTableViewController,
sendDailyUsagePingSetting = sendDailyUsagePingSettings
}

let sendCrashReportsSettings = SendDataSetting(
prefs: profile.prefs,
delegate: settingsDelegate,
theme: themeManager.getCurrentTheme(for: windowUUID),
settingsDelegate: parentCoordinator,
sendDataType: .crashReports
)
self.sendCrashReportsSetting = sendCrashReportsSettings

sendAnonymousUsageDataSetting = anonymousUsageDataSetting
studiesToggleSetting = studiesSetting
}
Expand Down
Loading

0 comments on commit e64c2e3

Please sign in to comment.