Skip to content

Commit

Permalink
Bugfix FXIOS-9815 cookie issue (#21624)
Browse files Browse the repository at this point in the history
* Delay webview config setup to the point it is needed

* Fix tests

* Remove commented code

* Fix tests

* Re-add create webview call
  • Loading branch information
OrlaM authored Aug 26, 2024
1 parent f8f4974 commit 1a53ed9
Show file tree
Hide file tree
Showing 20 changed files with 57 additions and 77 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,8 @@
"kind" : "remoteSourceControl",
"location" : "https://github.com/apple/swift-asn1.git",
"state" : {
"revision" : "c7e239b5c1492ffc3ebd7fbcc7a92548ce4e78f0",
"version" : "1.1.0"
"revision" : "df5d2fcd22e3f480e3ef85bf23e277a4a0ef524d",
"version" : "1.2.0"
}
},
{
Expand All @@ -140,8 +140,8 @@
"kind" : "remoteSourceControl",
"location" : "https://github.com/apple/swift-crypto.git",
"state" : {
"revision" : "f0525da24dc3c6cbb2b6b338b65042bc91cbc4bb",
"version" : "3.3.0"
"revision" : "a53a7e8f858902659d4784322bede34f4e49097e",
"version" : "3.6.1"
}
},
{
Expand Down
6 changes: 2 additions & 4 deletions firefox-ios/Client/Frontend/Reader/ReadabilityService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,11 @@ class ReadabilityOperation: Operation {

DispatchQueue.main.async(execute: { () in
let configuration = WKWebViewConfiguration()
// TODO: To resolve profile from DI container

let windowManager: WindowManager = AppContainer.shared.resolve()
let defaultUUID = windowManager.windows.first?.key ?? .unavailable
let tab = Tab(profile: self.profile, configuration: configuration, windowUUID: defaultUUID)
let tab = Tab(profile: self.profile, windowUUID: defaultUUID)
self.tab = tab
tab.createWebview()
tab.createWebview(configuration: configuration)
tab.navigationDelegate = self

let readerMode = ReaderMode(tab: tab)
Expand Down
15 changes: 4 additions & 11 deletions firefox-ios/Client/TabManagement/Legacy/LegacyTabManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ class LegacyTabManager: NSObject, FeatureFlaggable, TabManager, TabEventHandler

// MARK: - Webview configuration
// A WKWebViewConfiguration used for normal tabs
private lazy var configuration: WKWebViewConfiguration = {
lazy var configuration: WKWebViewConfiguration = {
return LegacyTabManager.makeWebViewConfig(isPrivate: false, prefs: profile.prefs)
}()

Expand Down Expand Up @@ -344,7 +344,6 @@ class LegacyTabManager: NSObject, FeatureFlaggable, TabManager, TabEventHandler
// MARK: - Add tabs
func addTab(_ request: URLRequest?, afterTab: Tab?, isPrivate: Bool) -> Tab {
return addTab(request,
configuration: nil,
afterTab: afterTab,
flushToDisk: true,
zombie: false,
Expand All @@ -353,13 +352,11 @@ class LegacyTabManager: NSObject, FeatureFlaggable, TabManager, TabEventHandler

@discardableResult
func addTab(_ request: URLRequest! = nil,
configuration: WKWebViewConfiguration! = nil,
afterTab: Tab? = nil,
zombie: Bool = false,
isPrivate: Bool = false
) -> Tab {
return addTab(request,
configuration: configuration,
afterTab: afterTab,
flushToDisk: true,
zombie: zombie,
Expand Down Expand Up @@ -389,23 +386,18 @@ class LegacyTabManager: NSObject, FeatureFlaggable, TabManager, TabEventHandler
}

func addTab(_ request: URLRequest? = nil,
configuration: WKWebViewConfiguration? = nil,
afterTab: Tab? = nil,
flushToDisk: Bool,
zombie: Bool,
isPrivate: Bool = false
) -> Tab {
// Take the given configuration. Or if it was nil, take our default configuration for the current browsing mode.
let configuration: WKWebViewConfiguration = configuration ?? (isPrivate ? privateConfiguration : self.configuration)

let tab = Tab(profile: profile, configuration: configuration, isPrivate: isPrivate, windowUUID: windowUUID)
let tab = Tab(profile: profile, isPrivate: isPrivate, windowUUID: windowUUID)
configureTab(tab, request: request, afterTab: afterTab, flushToDisk: flushToDisk, zombie: zombie)
return tab
}

func addPopupForParentTab(profile: Profile, parentTab: Tab, configuration: WKWebViewConfiguration) -> Tab {
let popup = Tab(profile: profile,
configuration: configuration,
isPrivate: parentTab.isPrivate,
windowUUID: windowUUID)
configureTab(popup, request: nil, afterTab: parentTab, flushToDisk: true, zombie: false, isPopup: true)
Expand Down Expand Up @@ -455,7 +447,8 @@ class LegacyTabManager: NSObject, FeatureFlaggable, TabManager, TabEventHandler
}

if !zombie {
tab.createWebview()
let configuration = tab.isPrivate ? self.privateConfiguration : self.configuration
tab.createWebview(configuration: configuration)
}
tab.navigationDelegate = self.navDelegate

Expand Down
11 changes: 5 additions & 6 deletions firefox-ios/Client/TabManagement/Tab.swift
Original file line number Diff line number Diff line change
Expand Up @@ -391,23 +391,21 @@ class Tab: NSObject, ThemeApplicable {
// If this tab has been opened from another, its parent will point to the tab from which it was opened
weak var parent: Tab?

fileprivate var contentScriptManager = TabContentScriptManager()
private var contentScriptManager = TabContentScriptManager()

fileprivate let configuration: WKWebViewConfiguration
private var configuration: WKWebViewConfiguration?

/// Any time a tab tries to make requests to display a Javascript Alert and we are not the active
/// tab instance, queue it for later until we become foregrounded.
fileprivate var alertQueue = [JSAlertInfo]()
private var alertQueue = [JSAlertInfo]()

var profile: Profile

init(profile: Profile,
configuration: WKWebViewConfiguration,
isPrivate: Bool = false,
windowUUID: WindowUUID,
faviconHelper: SiteImageHandler = DefaultSiteImageHandler.factory(),
logger: Logger = DefaultLogger.shared) {
self.configuration = configuration
self.nightMode = false
self.windowUUID = windowUUID
self.noImageMode = false
Expand Down Expand Up @@ -460,7 +458,8 @@ class Tab: NSObject, ThemeApplicable {
}
}

func createWebview(with restoreSessionData: Data? = nil) {
func createWebview(with restoreSessionData: Data? = nil, configuration: WKWebViewConfiguration) {
self.configuration = configuration
if webView == nil {
configuration.userContentController = WKUserContentController()
configuration.allowsInlineMediaPlayback = true
Expand Down
3 changes: 0 additions & 3 deletions firefox-ios/Client/TabManagement/TabManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ protocol TabManager: AnyObject {

@discardableResult
func addTab(_ request: URLRequest!,
configuration: WKWebViewConfiguration!,
afterTab: Tab?,
zombie: Bool,
isPrivate: Bool) -> Tab
Expand Down Expand Up @@ -131,13 +130,11 @@ extension TabManager {

@discardableResult
func addTab(_ request: URLRequest! = nil,
configuration: WKWebViewConfiguration! = nil,
afterTab: Tab? = nil,
zombie: Bool = false,
isPrivate: Bool = false
) -> Tab {
addTab(request,
configuration: configuration,
afterTab: afterTab,
zombie: zombie,
isPrivate: isPrivate)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import TabDataStore
import Storage
import Common
import Shared
import WebKit

// This class subclasses the legacy tab manager temporarily so we can
// gradually migrate to the new system
Expand Down Expand Up @@ -432,7 +433,9 @@ class TabManagerImplementation: LegacyTabManager, Notifiable, WindowSimpleTabsPr

private func selectTabWithSession(tab: Tab, previous: Tab?, sessionData: Data?) {
assert(Thread.isMainThread, "Currently expected to be called only on main thread.")
selectedTab?.createWebview(with: sessionData)
let configuration: WKWebViewConfiguration = tab.isPrivate ? self.privateConfiguration : self.configuration

selectedTab?.createWebview(with: sessionData, configuration: configuration)
selectedTab?.lastExecutedTime = Date.now()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class FormAutofillHelperTests: XCTestCase {
profile = MockProfile()
DependencyHelperMock().bootstrapDependencies()
LegacyFeatureFlagsManager.shared.initializeDeveloperFeatures(with: profile)
tab = Tab(profile: profile, configuration: WKWebViewConfiguration(), windowUUID: windowUUID)
tab = Tab(profile: profile, windowUUID: windowUUID)
formAutofillHelper = FormAutofillHelper(tab: tab)
secureWebviewMock = WKWebViewMock(URL(string: "https://foo.com")!)
secureFrameMock = WKFrameInfoMock(webView: secureWebviewMock, frameURL: URL(string: "https://foo.com")!)
Expand Down Expand Up @@ -281,10 +281,10 @@ class FormAutofillHelperTests: XCTestCase {
}

func test_formAutofillHelper_foundFieldValuesClosure_doesntLeak() {
let tab = Tab(profile: profile, configuration: WKWebViewConfiguration(), windowUUID: windowUUID)
let tab = Tab(profile: profile, windowUUID: windowUUID)
let subject = FormAutofillHelper(tab: tab)
trackForMemoryLeaks(subject)
tab.createWebview()
tab.createWebview(configuration: .init())
tab.addContentScript(subject, name: FormAutofillHelper.name())

subject.foundFieldValues = { fieldValues, type, frame in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,9 +286,9 @@ final class BrowserCoordinatorTests: XCTestCase, FeatureFlaggable {
}

func testShowBackForwardList_presentsBackForwardListViewController() {
let mockTab = Tab(profile: profile, configuration: .init(), windowUUID: windowUUID)
let mockTab = Tab(profile: profile, windowUUID: windowUUID)
mockTab.url = URL(string: "https://www.google.com")
mockTab.createWebview()
mockTab.createWebview(configuration: .init())
tabManager.selectedTab = mockTab

let subject = createSubject()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ final class TabScrollControllerTests: XCTestCase {

self.mockProfile = MockProfile()
self.subject = TabScrollingController(windowUUID: windowUUID)
self.tab = Tab(profile: mockProfile, configuration: WKWebViewConfiguration(), windowUUID: windowUUID)
self.tab = Tab(profile: mockProfile, windowUUID: windowUUID)
LegacyFeatureFlagsManager.shared.initializeDeveloperFeatures(with: mockProfile)
mockGesture = UIPanGestureRecognizerMock()
DependencyHelperMock().bootstrapDependencies()
Expand Down Expand Up @@ -92,7 +92,7 @@ final class TabScrollControllerTests: XCTestCase {
}

private func setupTabScroll() {
tab.createWebview()
tab.createWebview(configuration: .init())
tab.webView?.scrollView.contentSize = CGSize(width: 200, height: 2000)
tab.webView?.scrollView.delegate = subject
subject.tab = tab
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ final class JumpBackInDataAdaptorTests: XCTestCase {
var mockTabManager: MockTabManager!
var mockProfile: MockProfile!
let sleepTime: UInt64 = 100_000_000
let webViewConfig = WKWebViewConfiguration()
let windowUUID: WindowUUID = .XCTestDefaultUUID

override func setUp() {
Expand Down Expand Up @@ -149,7 +148,7 @@ extension JumpBackInDataAdaptorTests {

func createTab(profile: MockProfile,
urlString: String? = "www.website.com") -> Tab {
let tab = Tab(profile: profile, configuration: webViewConfig, windowUUID: windowUUID)
let tab = Tab(profile: profile, windowUUID: windowUUID)

if let urlString = urlString {
tab.url = URL(string: urlString)!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -586,9 +586,8 @@ extension JumpBackInViewModelTests {
}

func createTab(profile: MockProfile,
configuration: WKWebViewConfiguration = WKWebViewConfiguration(),
urlString: String? = "www.website.com") -> Tab {
let tab = Tab(profile: profile, configuration: configuration, windowUUID: windowUUID)
let tab = Tab(profile: profile, windowUUID: windowUUID)

if let urlString = urlString {
tab.url = URL(string: urlString)!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,8 @@ class AccountSyncHandlerTests: XCTestCase {
// MARK: - Helper methods
private extension AccountSyncHandlerTests {
func createTab(profile: MockProfile,
configuration: WKWebViewConfiguration = WKWebViewConfiguration(),
urlString: String? = "www.website.com") -> Tab {
let tab = Tab(profile: profile, configuration: configuration, windowUUID: windowUUID)
let tab = Tab(profile: profile, windowUUID: windowUUID)

if let urlString = urlString {
tab.url = URL(string: urlString)!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ class MockBrowserViewController: BrowserViewController {
openURLInNewTabURL = url
openURLInNewTabIsPrivate = isPrivate
openURLInNewTabCount += 1
return Tab(profile: MockProfile(), configuration: .init(), windowUUID: windowUUID)
return Tab(profile: MockProfile(), windowUUID: windowUUID)
}

override func handleQRCode() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,8 @@ class MockTabManager: TabManager {
}

func addTab(_ request: URLRequest?, afterTab: Tab?, isPrivate: Bool) -> Tab {
let configuration = WKWebViewConfiguration()
let profile = MockProfile()
let tab = Tab(profile: profile, configuration: configuration, isPrivate: isPrivate, windowUUID: windowUUID)
let tab = Tab(profile: profile, isPrivate: isPrivate, windowUUID: windowUUID)
tabs.append(tab)
return tab
}
Expand Down Expand Up @@ -137,7 +136,7 @@ class MockTabManager: TabManager {
}

func addPopupForParentTab(profile: Profile, parentTab: Tab, configuration: WKWebViewConfiguration) -> Tab {
return Tab(profile: MockProfile(), configuration: WKWebViewConfiguration(), windowUUID: windowUUID)
return Tab(profile: MockProfile(), windowUUID: windowUUID)
}

func makeToastFromRecentlyClosedUrls(_ recentlyClosedTabs: [Tab],
Expand All @@ -148,12 +147,11 @@ class MockTabManager: TabManager {

@discardableResult
func addTab(_ request: URLRequest!,
configuration: WKWebViewConfiguration!,
afterTab: Tab?,
zombie: Bool,
isPrivate: Bool
) -> Tab {
return Tab(profile: MockProfile(), configuration: WKWebViewConfiguration(), windowUUID: windowUUID)
return Tab(profile: MockProfile(), windowUUID: windowUUID)
}

func backgroundRemoveAllTabs(isPrivate: Bool,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ class TabEventHandlerTests: XCTestCase {
let windowUUID: WindowUUID = .XCTestDefaultUUID
func testEventDelivery() {
let tab = Tab(profile: MockProfile(),
configuration: WKWebViewConfiguration(),
windowUUID: windowUUID)
let handler = DummyHandler()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ class TabManagerTests: XCTestCase {
var mockSessionStore: MockTabSessionStore!
var mockProfile: MockProfile!
var mockDiskImageStore: MockDiskImageStore!
let webViewConfig = WKWebViewConfiguration()
let sleepTime: UInt64 = 1 * NSEC_PER_SEC
let windowUUID: WindowUUID = .XCTestDefaultUUID

Expand Down Expand Up @@ -219,7 +218,7 @@ class TabManagerTests: XCTestCase {

private func addTabs(to subject: TabManagerImplementation, count: Int) {
for _ in 0..<count {
let tab = Tab(profile: mockProfile, configuration: webViewConfig, windowUUID: windowUUID)
let tab = Tab(profile: mockProfile, windowUUID: windowUUID)
tab.url = URL(string: "https://mozilla.com")!
subject.tabs.append(tab)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@ class TabTests: XCTestCase {

func testDisplayTitle_ForHomepageURL() {
let url = URL(string: "internal://local/about/home")!
let tab = Tab(profile: MockProfile(), configuration: WKWebViewConfiguration(), windowUUID: windowUUID)
let tab = Tab(profile: MockProfile(), windowUUID: windowUUID)
tab.url = url
let expectedDisplayTitle = String.AppMenu.AppMenuOpenHomePageTitleString
XCTAssertEqual(tab.displayTitle, expectedDisplayTitle)
}

func testTabDoesntLeak() {
let tab = Tab(profile: MockProfile(), configuration: WKWebViewConfiguration(), windowUUID: windowUUID)
let tab = Tab(profile: MockProfile(), windowUUID: windowUUID)
tab.tabDelegate = tabDelegate
trackForMemoryLeaks(tab)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,17 +72,17 @@ final class InactiveTabsManagerTests: XCTestCase {
amountOfInactiveTabs: Int = 0) -> [Tab] {
var tabs = [Tab]()
for _ in 0..<amountOfRegularTabs {
let tab = Tab(profile: profile, configuration: WKWebViewConfiguration(), windowUUID: windowUUID)
let tab = Tab(profile: profile, windowUUID: windowUUID)
tabs.append(tab)
}

for _ in 0..<amountOfPrivateTabs {
let tab = Tab(profile: profile, configuration: WKWebViewConfiguration(), isPrivate: true, windowUUID: windowUUID)
let tab = Tab(profile: profile, isPrivate: true, windowUUID: windowUUID)
tabs.append(tab)
}

for _ in 0..<amountOfInactiveTabs {
let tab = Tab(profile: profile, configuration: WKWebViewConfiguration(), windowUUID: windowUUID)
let tab = Tab(profile: profile, windowUUID: windowUUID)
let lastExecutedDate = Calendar.current.add(numberOfDays: -15, to: Date())
tab.lastExecutedTime = lastExecutedDate?.toTimestamp()
tabs.append(tab)
Expand Down
Loading

0 comments on commit 1a53ed9

Please sign in to comment.