Fix alert continuation leak when WebKit automatically cancels the req…

…uest. Handle manual cancellation as well.
brave · Dec 19, 2024 · e1c7ae8 · e1c7ae8
1 parent 7dbdad1
commit e1c7ae8
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 5 deletions.
diff --git a/...e-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+WKNavigationDelegate.swift b/...e-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+WKNavigationDelegate.swift
@@ -171,6 +171,15 @@ extension BrowserViewController: WKNavigationDelegate {
     guard var requestURL = navigationAction.request.url else {
       return (.cancel, preferences)
     }
+
+    let tab = tab(for: webView)
+    if tab?.isExternalAppAlertPresented == true {
+      tab?.externalAppPopup?.dismissWithType(dismissType: .noAnimation)
+      tab?.externalAppPopupContinuation?.resume(with: .success(false))
+      tab?.externalAppPopupContinuation = nil
+      tab?.externalAppPopup = nil
+    }
+
     if InternalURL.isValid(url: requestURL) {
       if navigationAction.navigationType != .backForward, navigationAction.isInternalUnprivileged,
         navigationAction.sourceFrame != nil || navigationAction.targetFrame?.isMainFrame == false
@@ -204,7 +213,6 @@ extension BrowserViewController: WKNavigationDelegate {
 
     // First special case are some schemes that are about Calling. We prompt the user to confirm this action. This
     // gives us the exact same behaviour as Safari.
-    let tab = tab(for: webView)
 
     if ["sms", "tel", "facetime", "facetime-audio"].contains(requestURL.scheme) {
       let shouldOpen = await handleExternalURL(
@@ -1093,6 +1101,15 @@ extension BrowserViewController: WKNavigationDelegate {
     recordFinishedPageLoadP3A()
   }
 
+  public func didFailNavigation(
+    _ webView: WKWebView,
+    withError error: Error
+  ) {
+    guard let tab = tab(for: webView) else { return }
+
+    print("HERE")
+  }
+
   public func webView(
     _ webView: WKWebView,
     didReceiveServerRedirectForProvisionalNavigation navigation: WKNavigation!
@@ -1262,7 +1279,12 @@ extension BrowserViewController {
         message: String(format: Strings.openExternalAppURLMessage, url.relativeString),
         titleWeight: .semibold,
         titleSize: 21
-      )
+      ) {
+        openedURLCompletionHandler(false)
+        return false
+      }
+
+      tab?.externalAppPopup = popup
 
       if isSuppressActive {
         popup.addButton(title: Strings.suppressAlertsActionTitle, type: .destructive) {
@@ -1322,10 +1344,17 @@ extension BrowserViewController {
 
     tab?.externalAppAlertCounter += 1
 
-    return await withCheckedContinuation { continuation in
-      showExternalSchemeAlert(isSuppressActive: tab?.externalAppAlertCounter ?? 0 > 2) {
-        continuation.resume(with: .success($0))
+    return await withTaskCancellationHandler {
+      return await withCheckedContinuation { [weak tab] continuation in
+        tab?.externalAppPopupContinuation = continuation
+        showExternalSchemeAlert(isSuppressActive: tab?.externalAppAlertCounter ?? 0 > 2) {
+          tab?.externalAppPopupContinuation = nil
+          continuation.resume(with: .success($0))
+        }
       }
+    } onCancel: { [weak tab] in
+      tab?.externalAppPopupContinuation?.resume(with: .success(false))
+      tab?.externalAppPopupContinuation = nil
     }
   }
 }

diff --git a/ios/brave-ios/Sources/Brave/Frontend/Browser/Tab.swift b/ios/brave-ios/Sources/Brave/Frontend/Browser/Tab.swift
@@ -4,6 +4,7 @@
 
 import BraveCore
 import BraveShields
+import BraveUI
 import BraveWallet
 import CertificateUtilities
 import Data
@@ -429,6 +430,8 @@ class Tab: NSObject {
 
   /// Boolean tracking custom url-scheme alert presented
   var isExternalAppAlertPresented = false
+  var externalAppPopup: AlertPopupView?
+  var externalAppPopupContinuation: CheckedContinuation<Bool, Never>?
   var externalAppAlertCounter = 0
   var isExternalAppAlertSuppressed = false
   var externalAppURLDomain: String?