Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor FXIOS-10467 - Remove force_cast violations from Utilities and applications support #23149

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,13 @@ class BackgroundNotificationSurfaceUtility: BackgroundUtilityProtocol {
// MARK: Private
private func setUp() {
BGTaskScheduler.shared.register(forTaskWithIdentifier: taskIdentifier, using: nil) { task in
self.handleAppRefresh(task: task as! BGAppRefreshTask)
guard let backgroundRefreshTask = task as? BGAppRefreshTask else {
self.logger.log("Failed to cast task to BGAppRefreshTask",
level: .fatal,
category: .lifecycle)
Comment on lines +60 to +62
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR adds several new .fatal logs; I think that's fine, since these would be crashes anyway. But @cyndichin @adudenamedruby please LMK if we have an official policy (or team plan) on how we're updating these force-cast violations.

return
}
self.handleAppRefresh(task: backgroundRefreshTask)

// Schedule a new refresh task.
self.scheduleTaskOnAppBackground()
Expand Down
4 changes: 3 additions & 1 deletion firefox-ios/Client/Application/UITestAppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,9 @@ class UITestAppDelegate: AppDelegate, FeatureFlaggable {
let output = URL(fileURLWithPath: "\(dirForTestProfile)/places.db")

let enumerator = FileManager.default.enumerator(atPath: dirForTestProfile)
let filePaths = enumerator?.allObjects as! [String]
guard let filePaths = enumerator?.allObjects as? [String] else {
fatalError("Failed to cast enumerator.allObjects to [String] during filePaths extraction")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bmihai23 Can we please double-check if there is any other way to handle this besides fatalError? This will crash just like as!, so this change really doesn't provide much benefit.

}
filePaths.filter { $0.contains(".db") }.forEach { item in
try? FileManager.default.removeItem(
at: URL(fileURLWithPath: "\(dirForTestProfile)/\(item)")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at http://mozilla.org/MPL/2.0/

import Common
import Foundation

/// This Activity Item Provider subclass does two things that are non-standard behaviour:
Expand All @@ -15,12 +16,15 @@ import Foundation
/// Note that not all applications use the Subject. For example OmniFocus ignores it, so we need to do both.

class TitleActivityItemProvider: UIActivityItemProvider, @unchecked Sendable {
private let logger: Logger

static let activityTypesToIgnore = [
UIActivity.ActivityType.copyToPasteboard,
UIActivity.ActivityType.message,
UIActivity.ActivityType.mail]

init(title: String) {
init(title: String, logger: Logger = DefaultLogger.shared) {
self.logger = logger
super.init(placeholderItem: title)
}

Expand All @@ -30,13 +34,26 @@ class TitleActivityItemProvider: UIActivityItemProvider, @unchecked Sendable {
return NSNull()
}
}
return placeholderItem! as AnyObject
if let item = placeholderItem as? AnyObject {
return item
} else {
logger.log("placeholderItem is not of expected type AnyObject",
level: .fatal,
category: .library)
return NSNull()
}
}

override func activityViewController(
_ activityViewController: UIActivityViewController,
subjectForActivityType activityType: UIActivity.ActivityType?
) -> String {
return placeholderItem as! String
guard let placeholderString = placeholderItem as? String else {
logger.log("Failed to cast placeholderItem to String",
level: .fatal,
category: .library)
return ""
}
return placeholderString
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at http://mozilla.org/MPL/2.0/

import Common
import Storage
import UIKit

Expand All @@ -15,6 +16,7 @@ class PhotonActionSheetViewModel: FeatureFlaggable {
// MARK: - Properties
var actions: [[PhotonRowActions]]
var modalStyle: UIModalPresentationStyle
private let logger: Logger

var closeButtonTitle: String?
var site: Site?
Expand Down Expand Up @@ -42,25 +44,29 @@ class PhotonActionSheetViewModel: FeatureFlaggable {
// MARK: - Initializers
init(actions: [[PhotonRowActions]],
site: Site? = nil,
modalStyle: UIModalPresentationStyle) {
modalStyle: UIModalPresentationStyle,
logger: Logger = DefaultLogger.shared) {
self.actions = actions
self.site = site
self.modalStyle = modalStyle
self.logger = logger
}

init(actions: [[PhotonRowActions]],
closeButtonTitle: String? = nil,
title: String? = nil,
modalStyle: UIModalPresentationStyle,
isMainMenu: Bool = false,
isMainMenuInverted: Bool = false) {
isMainMenuInverted: Bool = false,
logger: Logger = DefaultLogger.shared) {
self.actions = actions
self.closeButtonTitle = closeButtonTitle
self.title = title
self.modalStyle = modalStyle

self.isMainMenu = isMainMenu
self.isMainMenuInverted = isMainMenuInverted
self.logger = logger
setMainMenuStyle()
}

Expand Down Expand Up @@ -103,8 +109,14 @@ class PhotonActionSheetViewModel: FeatureFlaggable {
switch sheetStyle {
case .site:
guard let site = site else { break }
let header = tableView.dequeueReusableHeaderFooterView(
withIdentifier: PhotonActionSheetSiteHeaderView.cellIdentifier) as! PhotonActionSheetSiteHeaderView
guard let header = tableView.dequeueReusableHeaderFooterView(
withIdentifier: PhotonActionSheetSiteHeaderView.cellIdentifier
) as? PhotonActionSheetSiteHeaderView else {
logger.log("Failed to dequeue PhotonActionSheetSiteHeaderView",
level: .fatal,
category: .library)
return UIView()
}
header.configure(with: site)
return header

Expand All @@ -114,9 +126,14 @@ class PhotonActionSheetViewModel: FeatureFlaggable {
return tableView.dequeueReusableHeaderFooterView(
withIdentifier: PhotonActionSheetLineSeparator.cellIdentifier)
} else {
let header = tableView.dequeueReusableHeaderFooterView(
guard let header = tableView.dequeueReusableHeaderFooterView(
withIdentifier: PhotonActionSheetTitleHeaderView.cellIdentifier
) as! PhotonActionSheetTitleHeaderView
) as? PhotonActionSheetTitleHeaderView else {
logger.log("Failed to dequeue PhotonActionSheetTitleHeaderView",
level: .fatal,
category: .library)
return UIView()
}
header.configure(with: title)
return header
}
Expand Down
14 changes: 12 additions & 2 deletions firefox-ios/Client/Utils/ConfigurableGradientView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,15 @@
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at http://mozilla.org/MPL/2.0/

import Common
import Foundation

/// A view whose primary modifiable layer is a gradient layer
public class ConfigurableGradientView: UIView {
init() {
private let logger: Logger

init(logger: Logger = DefaultLogger.shared) {
self.logger = logger
super.init(frame: .zero)
}

Expand All @@ -16,7 +20,13 @@ public class ConfigurableGradientView: UIView {
}

private var gradientLayer: CAGradientLayer {
return layer as! CAGradientLayer
guard let gradientLayer = layer as? CAGradientLayer else {
logger.log("Failed to cast layer to CAGradientLayer in ConfigurableGradientView class",
level: .fatal,
category: .library)
return CAGradientLayer()
}
return gradientLayer
}

override public class var layerClass: AnyClass {
Expand Down