chore: adopt UIScene lifecycle - WPB-25097#4826
Conversation
Launch options are nil when app is configured for scenes
This is already being set in AppDelegate.application(_:didReceiveRemoteNotification:fetchCompletionHandler:) and apples docs state on the deprecation notice: > "Continue using UIApplicationDelegate's application(_:didReceiveRemoteNotification:fetchCompletionHandler:) to process silent remote notifications after scene connection."
…rely on UIApplication.applicationState
This `launchType == .push` code path is never reached.
This notification is no longer observed
| <key>UIApplicationSceneManifest</key> | ||
| <dict> | ||
| <key>UIApplicationSupportsMultipleScenes</key> | ||
| <false/> |
There was a problem hiding this comment.
We don't support multiple scenes. Changing this would be a big task
| static let pushChannel = WireLogger(tag: "push-channel") | ||
| static let webSocket = WireLogger(tag: "websocket") | ||
| static let proteus = WireLogger(tag: "proteus") | ||
| static let sceneDelegate = WireLogger(tag: "SceneDelegate") |
There was a problem hiding this comment.
This is the only content change in this file. Other than that I sorted the options.
| _ application: ZMApplication, | ||
| didFinishLaunching launchOptions: [UIApplication.LaunchOptionsKey: Any?] | ||
| ) { | ||
| startEphemeralTimers() |
There was a problem hiding this comment.
This was moved to the caller of this method as this method no longer makes sense.
| .appendingPathComponent(remoteIdentifier.uuidString, isDirectory: true) | ||
| } | ||
|
|
||
| NotificationCenter.default.post(name: NSNotification.Name.ZMUserSessionDidBecomeAvailable, object: nil) |
There was a problem hiding this comment.
This is no longer observed.
| // Singletons | ||
| var unauthenticatedSession: UnauthenticatedSession? { | ||
| SessionManager.shared?.unauthenticatedSession | ||
| var mainWindow: UIWindow? { |
There was a problem hiding this comment.
This is now optional as its existence is a little later in the lifecycle.
| private let cookieStorage = CookieStorage(cookieEncryptionKey: UserDefaults.cookiesKey()) | ||
| // MARK: - Private properties | ||
|
|
||
| private lazy var voIPPushManager: VoIPPushManager = .init( | ||
| application: UIApplication.shared, | ||
| pushTokenService: pushTokenService | ||
| ) | ||
|
|
||
| private let pushTokenService = PushTokenService() |
There was a problem hiding this comment.
These have moved to AppDependencies so they are accessible here and from the SceneDelegate.
Test Results3 010 tests 2 983 ✅ 3m 54s ⏱️ Results for commit e12dbf3. ♻️ This comment has been updated with latest results. Summary: workflow run #27207229732 |
|
netbe
left a comment
There was a problem hiding this comment.
left some questions before approving
| public func start(connectionOptions: UIScene.ConnectionOptions) async { | ||
| if | ||
| let url = launchOptions[UIApplication.LaunchOptionsKey.url] as? URL, | ||
| let url = connectionOptions.urlContexts.first?.url, // Currently we only support one URL |
There was a problem hiding this comment.
question: what's the url? same as launch url so deeplink?
There was a problem hiding this comment.
Yes exactly. I was not able to find the reason that this is now a collection of URLs. The best I could work out with the help of a LLM (e.g. maybe nonsense) is that:
- An app could quickly try and launch our app with multiple calls in succession. The OS might batch these into a single instantiation.
- Future proofing for future Apple plans.
I couldn't find a better explanation.
There was a problem hiding this comment.
question: why are snapshots recorded here?
There was a problem hiding this comment.
Oh right, I forgot to mention this in the description. I've updated the PR description. Please see there.
|
|
||
| // MARK: - UISceneDelegate | ||
|
|
||
| var window: UIWindow? |
There was a problem hiding this comment.
I can't unfortunately because this is the UISceneDelegate API:
optional var window: UIWindow? { get set }| sceneConnectionOptions: connectionOptions | ||
| ) | ||
|
|
||
| (UIApplication.shared.delegate as? AppDelegate)?.queueInitializationOperations() |
There was a problem hiding this comment.
question: should this be called inside the scenedelegate? maybe the launchOperations need some review. I wonder if some should be done even if a scene is not connected (background)
There was a problem hiding this comment.
Conceptually launch options should really be handled by the AppDelegate as they shouldn't really be specific to a scene. I've taken some time to understand the launch operations better. As far as I understand these should really be done before we setup the appRouteRouter but there is some complexity here as access to things like group user defaults can be blocked by file protection. Making worthwhile changes here is somewhat risky and I don't want to really add additional risk to this current PR. Therefore I've created a Jira ticket to look into to this further: https://wearezeta.atlassian.net/browse/WPB-26555 and don't plan to address your comment in this PR.
| func applicationWillEnterForeground(_ application: UIApplication) { | ||
| WireLogger.appDelegate.info( | ||
| "applicationWillEnterForeground: (applicationState = \(application.applicationState)", | ||
| attributes: .safePublic | ||
| ) | ||
| } | ||
|
|
||
| func applicationDidBecomeActive(_ application: UIApplication) { | ||
| WireLogger.appDelegate.info( | ||
| "applicationDidBecomeActive (applicationState = \(application.applicationState))", | ||
| attributes: .safePublic | ||
| ) | ||
|
|
||
| switch launchType { | ||
| case .url, | ||
| .push: | ||
| break | ||
| default: | ||
| launchType = .direct | ||
| } | ||
| } |
There was a problem hiding this comment.
question: what replaces the app lifecycle events since these are removed ?
There was a problem hiding this comment.
These app lifecycle are no longer called on the AppDelegate. However, the equivalent notifications still exist and can be observed.



Issue
This PR migrates the app for the UIScene lifecycle. This is necessary because apple will stope supporting the legacy lifecycle with iOS 27 SDK and the app will crash on launch:
Considering we have big plans for our apps architecture, this PR attempts to do a kind of minimum to solve the immediate problem without resorting to big hacks. It only supports a single app instance so we don't gain anything from these changes accept compatibility with the future SDK. To achieve this the following changes were necessary:
ApplicationLaunchType. This abstraction is hard to make work with the UIScene lifecycle and it was only being used in two places:CallQualityControllerwhere a simple alternative was foundSoundEventListenerfrom testing this code never hit theappDelegate.launchType == .pushpath (the behavior doesn't work) so I just removed the related code.Snapshot updates
There were 4 screens whose snapshots needed to be update. I looked into this and I don't believe there was any changes in the app - rather they relate to how snapshots are rendered in the library - specifically the view.safeAreaInsets that are applied to the view controllers being tested by the library.
I decided just to update the snapshots as the safe area insets applied in the snapshots are not related to those applied in the actual app.
Testing
General testing of the app.
Checklist
[WPB-XXX].