Revert "Refactor FXIOS-9084 update select tab to be syncronous (#20708)…

… (#20787)" (#20851)
mozilla-mobile · Jun 28, 2024 · 6070aa9 · 6070aa9
1 parent 16a69cd
commit 6070aa9
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 25 deletions.
diff --git a/firefox-ios/Client/Frontend/Browser/Event Queue/AppEvent.swift b/firefox-ios/Client/Frontend/Browser/Event Queue/AppEvent.swift
@@ -32,4 +32,5 @@ public enum AppEvent: AppEventType {
 
     // Activites: Tabs
     case tabRestoration(WindowUUID)
+    case selectTab(URL?, WindowUUID)
 }
diff --git a/firefox-ios/Client/Frontend/Library/RecentlyClosedTabsPanel.swift b/firefox-ios/Client/Frontend/Library/RecentlyClosedTabsPanel.swift
@@ -131,8 +131,19 @@ class RecentlyClosedTabsPanelSiteTableViewController: SiteTableViewController {
         let url = recentlyClosedTabs[indexPath.row].url
         recentlyClosedTabsDelegate?.openRecentlyClosedSiteInNewTab(url, isPrivate: false)
 
-        let visitType = VisitType.typed    // Means History, too.
-        self.libraryPanelDelegate?.libraryPanel(didSelectURL: url, visitType: visitType)
+        // The code above creates new tab and selects it, but TabManagerImplementation.selectTab()
+        // currently performs the actual selection update asynchronously via Swift Async + Task/Await.
+        // This means that selectTab() returns before the tab is actually selected. As a result, the
+        // delegate callback below will incorrectly cause a duplicate tab (since it will treat the
+        // url as having been applied to the current tab, not our newly-added tab from above). This
+        // is avoided by making sure we wait for our expected tab above to be selected before
+        // notifying our library panel delegate. [FXIOS-7741]
+
+        let tabWindowUUID = windowUUID
+        AppEventQueue.wait(for: .selectTab(url, tabWindowUUID)) {
+            let visitType = VisitType.typed    // Means History, too.
+            self.libraryPanelDelegate?.libraryPanel(didSelectURL: url, visitType: visitType)
+        }
     }
 
     override func tableView(_ tableView: UITableView, heightForHeaderInSection section: Int) -> CGFloat {

diff --git a/firefox-ios/Client/TabManagement/TabManagerImplementation.swift b/firefox-ios/Client/TabManagement/TabManagerImplementation.swift
@@ -322,16 +322,6 @@ class TabManagerImplementation: LegacyTabManager, Notifiable, WindowSimpleTabsPr
         saveCurrentTabSessionData()
 
         willSelectTab(url)
-
-        let previous = previous ?? selectedTab
-
-        previous?.metadataManager?.updateTimerAndObserving(state: .tabSwitched, isPrivate: previous?.isPrivate ?? false)
-        tab.metadataManager?.updateTimerAndObserving(state: .tabSelected, isPrivate: tab.isPrivate)
-
-        _selectedIndex = tabs.firstIndex(of: tab) ?? -1
-
-        preserveTabs()
-
         Task(priority: .high) {
             var sessionData: Data?
             if !tab.isFxHomeTab {
@@ -340,29 +330,32 @@ class TabManagerImplementation: LegacyTabManager, Notifiable, WindowSimpleTabsPr
             await selectTabWithSession(tab: tab,
                                        previous: previous,
                                        sessionData: sessionData)
-        }
 
-        // Default to false if the feature flag is not enabled
-        var isPrivate = false
-        if featureFlags.isFeatureEnabled(.feltPrivacySimplifiedUI, checking: .buildOnly) {
-            isPrivate = tab.isPrivate
-        }
+            // Default to false if the feature flag is not enabled
+            var isPrivate = false
+            if featureFlags.isFeatureEnabled(.feltPrivacySimplifiedUI, checking: .buildOnly) {
+                isPrivate = tab.isPrivate
+            }
 
-        let action = PrivateModeAction(isPrivate: isPrivate,
-                                       windowUUID: windowUUID,
-                                       actionType: PrivateModeActionType.setPrivateModeTo)
-        store.dispatch(action)
+            let action = PrivateModeAction(isPrivate: isPrivate,
+                                           windowUUID: windowUUID,
+                                           actionType: PrivateModeActionType.setPrivateModeTo)
+            store.dispatch(action)
 
-        didSelectTab(url)
-        updateMenuItemsForSelectedTab()
+            didSelectTab(url)
+            updateMenuItemsForSelectedTab()
+        }
     }
 
     private func willSelectTab(_ url: URL?) {
         tabsTelemetry.startTabSwitchMeasurement()
+        guard let url else { return }
+        AppEventQueue.started(.selectTab(url, windowUUID))
     }
 
     private func didSelectTab(_ url: URL?) {
         tabsTelemetry.stopTabSwitchMeasurement()
+        AppEventQueue.completed(.selectTab(url, windowUUID))
         let action = GeneralBrowserAction(selectedTabURL: url,
                                           isPrivateBrowsing: selectedTab?.isPrivate ?? false,
                                           windowUUID: windowUUID,
@@ -372,7 +365,14 @@ class TabManagerImplementation: LegacyTabManager, Notifiable, WindowSimpleTabsPr
 
     @MainActor
     private func selectTabWithSession(tab: Tab, previous: Tab?, sessionData: Data?) {
-        guard tab == selectedTab else { return }
+        let previous = previous ?? selectedTab
+
+        previous?.metadataManager?.updateTimerAndObserving(state: .tabSwitched, isPrivate: previous?.isPrivate ?? false)
+        tab.metadataManager?.updateTimerAndObserving(state: .tabSelected, isPrivate: tab.isPrivate)
+
+        _selectedIndex = tabs.firstIndex(of: tab) ?? -1
+
+        preserveTabs()
 
         selectedTab?.createWebview(with: sessionData)
         selectedTab?.lastExecutedTime = Date.now()