[MOB-3166] Tabs migration cleanup

[d4r1091]

MOB-3166

Context

We ran an investigation regarding the tabs migration between version upgrades.
If all good, then the next steps would require a migration cleanup.

Approach

• Cleanup the respective code, especially counting on this commit 5e5ab45
• Restore the codebase as referenced in our Firefox Vanilla version
• Execute smoke tests
• Execute Unit Tests

Other

Refer to this jira comment for more info on the tests performed and the overall versions installed against current users.

Before merging

Checklist

• I performed some relevant testing on a real device and/or simulator
• I checked Unit Tests that confirm the expected behaviour
• I made sure that any change to the Analytics events included in PR won't alter current analytics (e.g. new users, upgrading users)

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Compatibility The removal of NSCoding conformance and the updated class signature may affect serialization and legacy data handling. Confirm that these changes are fully compatible with downstream consumers and migration routines. class LegacyTabGroupData: Codable { var tabAssociatedSearchTerm: String = "" var tabAssociatedSearchUrl: String = "" var tabAssociatedNextUrl: String = "" var tabHistoryCurrentState = "" func tabHistoryMetadatakey() -> HistoryMetadataKey { return HistoryMetadataKey( url: tabAssociatedSearchUrl, searchTerm: tabAssociatedSearchTerm, referrerUrl: tabAssociatedNextUrl ) } enum CodingKeys: String, CodingKey { case tabAssociatedSearchTerm case tabAssociatedSearchUrl case tabAssociatedNextUrl case tabHistoryCurrentState } convenience init() { self.init(searchTerm: "", searchUrl: "", nextReferralUrl: "", tabHistoryCurrentState: LegacyTabGroupTimerState.none.rawValue) } init(searchTerm: String, searchUrl: String, nextReferralUrl: String, tabHistoryCurrentState: String = "") { self.tabAssociatedSearchTerm = searchTerm self.tabAssociatedSearchUrl = searchUrl self.tabAssociatedNextUrl = nextReferralUrl self.tabHistoryCurrentState = tabHistoryCurrentState } } Migration Behavior The simplified logic in the restoreOnly function now omits additional migration fallback for v10 tabs. Ensure that users with legacy tab data are correctly handled without unintended side effects. Task { // Only attempt a tab data store fetch if we know we should have tabs on disk (ignore new windows) let windowData: WindowData? = windowIsNew ? nil : await self.tabDataStore.fetchWindowData(uuid: windowUUID) await buildTabRestore(window: windowData) logger.log("Tabs restore ended after fetching window data", level: .debug, category: .tabs) logger.log("Normal tabs count; \(normalTabs.count), Inactive tabs count; \(inactiveTabs.count), Private tabs count; \(privateTabs.count)", level: .debug, category: .tabs) } Migration Cleanup Removal of deprecated migration methods and archived data cleanup logic must be validated to ensure that no necessary fallback behavior is lost during the migration process. return true } logger.log("Key exists - running migration? \(shouldRunMigration)", level: .debug, category: .tabs) return shouldRunMigration }

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Ensure nil window handling Verify that buildTabRestore can safely handle a nil value; if not, add a guard or conditional check before calling it. firefox-ios/Client/TabManagement/TabManagerImplementation.swift [112-113] let windowData: WindowData? = windowIsNew ? nil : await self.tabDataStore.fetchWindowData(uuid: windowUUID) -await buildTabRestore(window: windowData) +if let validWindowData = windowData { + await buildTabRestore(window: validWindowData) +} else { + // Handle the nil case appropriately or return early +} Suggestion importance[1-10]: 4 __ Why: The suggestion introduces a conditional check for a nil windowData before calling buildTabRestore, which is a conservative safety measure. However, if buildTabRestore is already designed to handle nil values, the change may be unnecessary, making the overall impact a minor improvement.	Low

[lucaschifino]

🧹 ✨