Addressed feedback (Moved TabHelper)

brave · Dec 18, 2024 · 6bc61a9 · 6bc61a9
1 parent f8e4610
commit 6bc61a9
Show file tree

Hide file tree

Showing 7 changed files with 27 additions and 75 deletions.
diff --git a/ios/brave-ios/Sources/Brave/Frontend/Brave Translate/BraveTranslateTabHelper.swift b/ios/brave-ios/Sources/Brave/Frontend/Brave Translate/BraveTranslateTabHelper.swift
@@ -20,7 +20,7 @@ enum BraveTranslateError: String, Error {
   case otherError
 }
 
-class BraveTranslateTabHelper: NSObject, TabHelper {
+class BraveTranslateTabHelper: NSObject {
   private weak var tab: Tab?
   private weak var delegate: BraveTranslateScriptHandlerDelegate?
   private let recognizer = NLLanguageRecognizer()
@@ -35,8 +35,6 @@ class BraveTranslateTabHelper: NSObject, TabHelper {
 
   var currentLanguageInfo = BraveTranslateLanguageInfo()
 
-  static let tabHelperName = String(describing: BraveTranslateTabHelper.self)
-
   // All TabHelpers in Chromium have a `WebState* web_state` parameter in their constructor
   // WebState in Brave, is the same as `Tab`.
   init(tab: Tab, delegate: BraveTranslateScriptHandlerDelegate) {

diff --git a/ios/brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+ShareActivity.swift b/ios/brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+ShareActivity.swift
@@ -106,17 +106,13 @@ extension BrowserViewController {
           callback: { [weak self] in
             guard let self = self, let tab = tab else { return }
 
-            let tabHelper =
-              tab.getTabHelper(named: BraveTranslateTabHelper.tabHelperName)
-              as? BraveTranslateTabHelper
-
-            if let tabHelper {
-              tabHelper.presentUI(on: self)
+            if let translateHelper = tab.translateHelper {
+              translateHelper.presentUI(on: self)
 
               if tab.translationState == .active {
-                tabHelper.revertTranslation()
+                translateHelper.revertTranslation()
               } else if tab.translationState != .active {
-                tabHelper.startTranslation(canShowToast: true)
+                translateHelper.startTranslation(canShowToast: true)
               }
             }
           }

diff --git a/ios/brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+ToolbarDelegate.swift b/ios/brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+ToolbarDelegate.swift
@@ -263,16 +263,14 @@ extension BrowserViewController: TopToolbarDelegate {
 
   func topToolbarDidPressTranslateButton(_ urlBar: TopToolbarView) {
     guard let tab = tabManager.selectedTab else { return }
-    let tabHelper =
-      tab.getTabHelper(named: BraveTranslateTabHelper.tabHelperName) as? BraveTranslateTabHelper
 
-    if let tabHelper {
-      tabHelper.presentUI(on: self)
+    if let translateHelper = tab.translateHelper {
+      translateHelper.presentUI(on: self)
 
       if tab.translationState == .active {
-        tabHelper.revertTranslation()
+        translateHelper.revertTranslation()
       } else if tab.translationState != .active {
-        tabHelper.startTranslation(canShowToast: true)
+        translateHelper.startTranslation(canShowToast: true)
       }
     }
   }

diff --git a/ios/brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+Translate.swift b/ios/brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+Translate.swift
@@ -21,11 +21,8 @@ extension BrowserViewController: BraveTranslateScriptHandlerDelegate {
       topToolbar.updateTranslateButtonState(state)
 
       showTranslateOnboarding(tab: tab) { [weak tab] translateEnabled in
-        let tabHelper =
-          tab?.getTabHelper(named: BraveTranslateTabHelper.tabHelperName)
-          as? BraveTranslateTabHelper
-        if let tabHelper {
-          tabHelper.startTranslation(canShowToast: true)
+        if let translateHelper = tab?.translateHelper {
+          translateHelper.startTranslation(canShowToast: true)
         }
       }
     }
@@ -63,11 +60,7 @@ extension BrowserViewController: BraveTranslateScriptHandlerDelegate {
               self.topToolbar.locationView.translateButton.setOnboardingState(enabled: false)
               Preferences.Translate.translateEnabled.value = true
 
-              let tabHelper =
-                tab.getTabHelper(named: BraveTranslateTabHelper.tabHelperName)
-                as? BraveTranslateTabHelper
-
-              tabHelper?.presentUI(on: self)
+              tab.translateHelper?.presentUI(on: self)
             },
             onDisableFeature: { [weak self, weak tab] in
               guard let tab = tab, let self = self else { return }
@@ -101,12 +94,7 @@ extension BrowserViewController: BraveTranslateScriptHandlerDelegate {
           self.topToolbar.locationView.translateButton.setOnboardingState(enabled: false)
 
           if Preferences.Translate.translateEnabled.value {
-            let tabHelper =
-              tab.getTabHelper(named: BraveTranslateTabHelper.tabHelperName)
-              as? BraveTranslateTabHelper
-
-            tabHelper?.presentUI(on: self)
-
+            tab.translateHelper?.presentUI(on: self)
             completion(true)
             return
           }
@@ -122,13 +110,8 @@ extension BrowserViewController: BraveTranslateScriptHandlerDelegate {
 
   func presentToast(tab: Tab?, languageInfo: BraveTranslateLanguageInfo) {
     let popover = PopoverController(
-      content: TranslateToast(languageInfo: languageInfo) { [weak tab] languageInfo in
-
-        let tabHelper =
-          tab?.getTabHelper(named: BraveTranslateTabHelper.tabHelperName)
-          as? BraveTranslateTabHelper
-
-        tabHelper?.startTranslation(canShowToast: false)
+      content: TranslateToast(languageInfo: languageInfo) { [weak tab] _ in
+        tab?.translateHelper?.startTranslation(canShowToast: false)
       },
       autoLayoutConfiguration: nil
     )

diff --git a/...rave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BrowserViewController.swift b/...rave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BrowserViewController.swift
@@ -2699,8 +2699,8 @@ extension BrowserViewController: TabDelegate {
     (tab.getContentScript(name: Web3NameServiceScriptHandler.scriptName)
       as? Web3NameServiceScriptHandler)?.delegate = self
 
-    // Tab Helpers
-    tab.addTabHelper(BraveTranslateTabHelper(tab: tab, delegate: self))
+    // Translate Helper
+    tab.translateHelper = BraveTranslateTabHelper(tab: tab, delegate: self)
   }
 
   func tab(_ tab: Tab, willDeleteWebView webView: WKWebView) {

diff --git a/ios/brave-ios/Sources/Brave/Frontend/Browser/Tab.swift b/ios/brave-ios/Sources/Brave/Frontend/Browser/Tab.swift
@@ -44,10 +44,6 @@ protocol TabContentScript: TabContentScriptLoader {
   )
 }
 
-protocol TabHelper {
-  static var tabHelperName: String { get }
-}
-
 protocol TabDelegate {
   func tab(_ tab: Tab, didAddSnackbar bar: SnackBar)
   func tab(_ tab: Tab, didRemoveSnackbar bar: SnackBar)
@@ -175,8 +171,6 @@ class Tab: NSObject {
 
   var sslPinningError: Error?
 
-  private var tabHelpers = [String: TabHelper]()
-
   private let _syncTab: BraveSyncTab?
   private let _faviconDriver: FaviconDriver?
   private var _walletEthProvider: BraveWalletEthereumProvider?
@@ -434,6 +428,8 @@ class Tab: NSObject {
     }
   }
 
+  var translateHelper: BraveTranslateTabHelper?
+
   /// Boolean tracking custom url-scheme alert presented
   var isExternalAppAlertPresented = false
   var externalAppAlertCounter = 0
@@ -631,7 +627,7 @@ class Tab: NSObject {
 
   func deleteWebView() {
     contentScriptManager.uninstall(from: self)
-    tabHelpers.removeAll()
+    translateHelper = nil
 
     if let webView = webView {
       webView.removeObserver(self, forKeyPath: KVOConstants.url.keyPath)
@@ -883,23 +879,6 @@ class Tab: NSObject {
     webView.customUserAgent = desktopMode ? UserAgent.desktop : UserAgent.mobile
   }
 
-  func addTabHelper(_ helper: TabHelper) {
-    let helperName = Swift.type(of: helper).tabHelperName
-    if tabHelpers[helperName] != nil {
-      assertionFailure("Tab Helper: \(helperName) already attached to this tab.")
-    }
-
-    self.tabHelpers[helperName] = helper
-  }
-
-  func getTabHelper(named name: String) -> TabHelper? {
-    self.tabHelpers[name]
-  }
-
-  func removeTabHelper(named name: String) {
-    self.tabHelpers.removeValue(forKey: name)
-  }
-
   func addContentScript(_ helper: TabContentScript, name: String, contentWorld: WKContentWorld) {
     contentScriptManager.addContentScript(
       helper,

diff --git a/...nt/UserScripts/Scripts_Dynamic/ScriptHandlers/Sandboxed/BraveTranslateScriptHandler.swift b/...nt/UserScripts/Scripts_Dynamic/ScriptHandlers/Sandboxed/BraveTranslateScriptHandler.swift
@@ -95,7 +95,7 @@ class BraveTranslateScriptHandler: NSObject, TabContentScript {
     if command == "ready" {
       Task { @MainActor [weak tab] in
         try await
-          (tab?.getTabHelper(named: BraveTranslateTabHelper.tabHelperName)
+          (tab?.translateHelper
           as? BraveTranslateTabHelper)?.setupTranslate()
         replyHandler(nil, nil)
       }
@@ -111,15 +111,13 @@ class BraveTranslateScriptHandler: NSObject, TabContentScript {
             from: JSONSerialization.data(withJSONObject: body, options: .fragmentsAllowed)
           )
 
-          guard let tab = tab,
-            let tabHelper = tab.getTabHelper(named: BraveTranslateTabHelper.tabHelperName)
-              as? BraveTranslateTabHelper
+          guard let tab = tab, let translateHelper = tab.translateHelper
           else {
             replyHandler(nil, BraveTranslateError.otherError.rawValue)
             return
           }
 
-          let (data, response) = try await tabHelper.processTranslationRequest(message)
+          let (data, response) = try await translateHelper.processTranslationRequest(message)
 
           replyHandler(
             [
@@ -201,8 +199,7 @@ class BraveTranslateScriptLanguageDetectionHandler: NSObject, TabContentScript {
     }
 
     guard
-      let tabHelper = tab?.getTabHelper(named: BraveTranslateTabHelper.tabHelperName)
-        as? BraveTranslateTabHelper
+      let translateHelper = tab?.translateHelper
     else {
       return
     }
@@ -214,9 +211,10 @@ class BraveTranslateScriptLanguageDetectionHandler: NSObject, TabContentScript {
       )
 
       if message.hasNoTranslate {
-        tabHelper.currentLanguageInfo.pageLanguage = tabHelper.currentLanguageInfo.currentLanguage
+        translateHelper.currentLanguageInfo.pageLanguage =
+          translateHelper.currentLanguageInfo.currentLanguage
       } else {
-        tabHelper.currentLanguageInfo.pageLanguage =
+        translateHelper.currentLanguageInfo.pageLanguage =
           !message.htmlLang.isEmpty ? Locale.Language(identifier: message.htmlLang) : nil
       }