Skip to content

Add FXIOS-14379 [Performance] Add back equatable for BVC state #31217

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.

Sign up for GitHub

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

Merged
Cramsden merged 3 commits into from
Dec 11, 2025
Merged

Add FXIOS-14379 [Performance] Add back equatable for BVC state #31217

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b3f3d36
Add back equatable for BVC state
Cramsden Dec 9, 2025
9debf5e
add ticket number
Cramsden Dec 10, 2025
c06570c
fix tests
Cramsden Dec 11, 2025
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
6 changes: 3 additions & 3 deletions firefox-ios/Client/Frontend/Browser/BrowserViewController/State/BrowserNavigationType.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import enum MozillaAppServices.VisitType
import SummarizeKit

/// View types that the browser coordinator can navigate to
enum BrowserNavigationDestination {
enum BrowserNavigationDestination: Equatable {
// Native views
case contextMenu
case settings(Route.SettingsSection)
Expand All @@ -29,7 +29,7 @@ enum BrowserNavigationDestination {
case shareSheet(ShareSheetConfiguration)
}

struct ShareSheetConfiguration {
struct ShareSheetConfiguration: Equatable {
let shareType: ShareType
let shareMessage: ShareMessage?
let sourceView: UIView
Expand All @@ -39,7 +39,7 @@ struct ShareSheetConfiguration {
}

/// This type exists as a field on the BrowserViewControllerState
struct NavigationDestination {
struct NavigationDestination: Equatable {
let destination: BrowserNavigationDestination
let url: URL?
let isPrivate: Bool?
Expand Down
2 changes: 1 addition & 1 deletion ...-ios/Client/Frontend/Browser/BrowserViewController/State/BrowserViewControllerState.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import WebKit
import SummarizeKit

struct BrowserViewControllerState: ScreenState {
enum NavigationType {
enum NavigationType: Equatable {
case home
case back
case forward
Expand Down
2 changes: 1 addition & 1 deletion firefox-ios/Client/Frontend/Browser/BrowserViewController/State/BrowserViewType.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import Foundation

enum BrowserViewType {
enum BrowserViewType: Equatable {
case normalHomepage
case privateHomepage
case webview
Expand Down
2 changes: 2 additions & 0 deletions firefox-ios/Client/Frontend/Share/ShareTab.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import Foundation

protocol ShareTab: Sendable {
nonisolated var tabUUID: TabUUID { get }
Copy link
Collaborator

@ih-codes ih-codes Dec 10, 2025

Choose a reason for hiding this comment

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

Nice, seems a reasonable solution for this weird edge case! 👌


@MainActor
var displayTitle: String { get }
@MainActor
Expand Down
15 changes: 14 additions & 1 deletion firefox-ios/Client/Frontend/Share/ShareType.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,24 @@ import Foundation
/// __SPECIAL NOTE__: If you download a PDF, you can view that in a tab. In that case, the URL may have a `file://`
/// scheme instead of `http(s)://`, so certain options, like Send to Device / Add to Home Screen, may not be
/// available.
enum ShareType {
enum ShareType: Equatable, Sendable {
case file(url: URL, remoteURL: URL?)
case site(url: URL)
case tab(url: URL, tab: any ShareTab)

static func == (lhs: Self, rhs: Self) -> Bool {
switch (lhs, rhs) {
case let (.file(lhsURL, rhsRemoteURL), .file(rhsURL, lhsRemoteURL)):
return lhsURL == rhsURL && rhsRemoteURL == lhsRemoteURL
case let (.site(lhsURL), .site(rhsURL)):
return lhsURL == rhsURL
case let (.tab(lhsURL, lhsTab), .tab(rhsURL, rhsTab)):
return lhsURL == rhsURL && lhsTab.tabUUID == rhsTab.tabUUID
default:
return false
}
}

/// The share URL wrapped by the given type.
var wrappedURL: URL {
switch self {
Expand Down
5 changes: 4 additions & 1 deletion firefox-ios/Client/TabManagement/Tab.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,10 @@ class Tab: NSObject, ThemeApplicable, FeatureFlaggable, ShareTab {

// Setting default page as topsites
var newTabPageType: NewTabPage = .topSites
var tabUUID: TabUUID = UUID().uuidString
// TODO: FXIOS-14381 This can't be nonisolated because it is a var but it is only set once.
// We should probably just do this as part of a Tab init from TabData but it will
// be a bigger refactor.
nonisolated(unsafe) var tabUUID: TabUUID = UUID().uuidString
private var screenshotUUIDString: String?

var screenshotUUID: UUID? {
Expand Down
3 changes: 3 additions & 0 deletions firefox-ios/firefox-ios-tests/Tests/ClientTests/Mocks/MockShareTab.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import Foundation

@MainActor
class MockShareTab: ShareTab {
nonisolated let tabUUID: Client.TabUUID
var canonicalURL: URL?
var displayTitle: String
var url: URL?
Expand All @@ -18,12 +19,14 @@ class MockShareTab: ShareTab {
title: String,
url: URL?,
canonicalURL: URL?,
tabUUID: TabUUID = UUID().uuidString,
withActiveWebView: Bool = true,
withTemporaryDocument: TemporaryDocument? = nil
) {
self.displayTitle = title
self.url = url
self.canonicalURL = canonicalURL
self.tabUUID = tabUUID
self.webView = TabWebView(frame: CGRect.zero, configuration: .init(), windowUUID: .XCTestDefaultUUID)
self.temporaryDocument = withTemporaryDocument
}
Expand Down