From 66fdc780eef56da85597bc27a47e79f08f84bded Mon Sep 17 00:00:00 2001 From: Juan Manuel Pereira Date: Fri, 22 Mar 2024 18:32:04 -0300 Subject: [PATCH] DBP: Fix memory leak on WebViewHandler (#2483) --- .../CCF/WebViewHandler.swift | 28 ++++++++----------- .../DebugUI/DebugScanOperation.swift | 6 ++++ .../Operations/DataBrokerOperation.swift | 4 ++- .../DataBrokerProtectionTests/Mocks.swift | 2 +- 4 files changed, 21 insertions(+), 19 deletions(-) diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/CCF/WebViewHandler.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/CCF/WebViewHandler.swift index f25a39ff74..cd92efc9a1 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/CCF/WebViewHandler.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/CCF/WebViewHandler.swift @@ -27,7 +27,7 @@ protocol WebViewHandler: NSObject { func load(url: URL) async throws func takeSnaphost(path: String, fileName: String) async throws func saveHTML(path: String, fileName: String) async throws - func waitForWebViewLoad(timeoutInSeconds: Int) async throws + func waitForWebViewLoad() async throws func finish() async func execute(action: Action, data: CCFRequestData) async func evaluateJavaScript(_ javaScript: String) async throws @@ -38,7 +38,7 @@ final class DataBrokerProtectionWebViewHandler: NSObject, WebViewHandler { private var activeContinuation: CheckedContinuation? private let isFakeBroker: Bool - private let webViewConfiguration: WKWebViewConfiguration + private var webViewConfiguration: WKWebViewConfiguration? private var userContentController: DataBrokerUserContentController? private var webView: WebView? @@ -59,7 +59,11 @@ final class DataBrokerProtectionWebViewHandler: NSObject, WebViewHandler { } func initializeWebView(showWebView: Bool) async { - webView = WebView(frame: CGRect(origin: .zero, size: CGSize(width: 1024, height: 1024)), configuration: webViewConfiguration) + guard let configuration = self.webViewConfiguration else { + return + } + + webView = WebView(frame: CGRect(origin: .zero, size: CGSize(width: 1024, height: 1024)), configuration: configuration) webView?.navigationDelegate = self if showWebView { @@ -78,40 +82,30 @@ final class DataBrokerProtectionWebViewHandler: NSObject, WebViewHandler { func load(url: URL) async throws { webView?.load(url) os_log("Loading URL: %@", log: .action, String(describing: url.absoluteString)) - try await waitForWebViewLoad(timeoutInSeconds: 120) + try await waitForWebViewLoad() } func finish() { os_log("WebViewHandler finished", log: .action) - webView?.stopLoading() userContentController?.cleanUpBeforeClosing() WKWebsiteDataStore.default().removeData(ofTypes: [WKWebsiteDataTypeDiskCache, WKWebsiteDataTypeMemoryCache], modifiedSince: Date(timeIntervalSince1970: 0)) { os_log("WKWebView data store deleted correctly", log: .action) } + webViewConfiguration = nil userContentController = nil webView?.navigationDelegate = nil webView = nil } deinit { - print("WebViewHandler Deinit") + os_log("WebViewHandler Deinit", log: .action) } - func waitForWebViewLoad(timeoutInSeconds: Int = 0) async throws { + func waitForWebViewLoad() async throws { try await withCheckedThrowingContinuation { continuation in self.activeContinuation = continuation - - if timeoutInSeconds > 0 { - Task { - try await Task.sleep(nanoseconds: UInt64(timeoutInSeconds) * NSEC_PER_SEC) - if self.activeContinuation != nil { - self.activeContinuation?.resume() - self.activeContinuation = nil - } - } - } } } diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/DebugUI/DebugScanOperation.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/DebugUI/DebugScanOperation.swift index 94ee1b481c..cd8f7adf78 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/DebugUI/DebugScanOperation.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/DebugUI/DebugScanOperation.swift @@ -166,6 +166,8 @@ final class DebugScanOperation: DataBrokerOperation { } else { os_log("Releasing the web view", log: .action) await webViewHandler?.finish() // If we executed all steps we release the web view + continuation = nil + webViewHandler = nil } } @@ -178,4 +180,8 @@ final class DebugScanOperation: DataBrokerOperation { await completeWith(error: error) } } + + deinit { + os_log("DebugScanOperation Deinit", log: .action) + } } diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Operations/DataBrokerOperation.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Operations/DataBrokerOperation.swift index dd34f6c0c6..de66d1e85d 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Operations/DataBrokerOperation.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Operations/DataBrokerOperation.swift @@ -186,7 +186,9 @@ extension DataBrokerOperation { switch actionType { case .click: stageCalculator?.fireOptOutFillForm() - try? await webViewHandler?.waitForWebViewLoad(timeoutInSeconds: 30) + try? await webViewHandler?.waitForWebViewLoad() + // We wait 10 seconds before tapping + try? await Task.sleep(nanoseconds: UInt64(10) * 1_000_000_000) await executeNextStep() case .fillForm: stageCalculator?.fireOptOutFillForm() diff --git a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/Mocks.swift b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/Mocks.swift index cc36c1e9de..b237c96669 100644 --- a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/Mocks.swift +++ b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/Mocks.swift @@ -191,7 +191,7 @@ final class WebViewHandlerMock: NSObject, WebViewHandler { wasLoadCalledWithURL = url } - func waitForWebViewLoad(timeoutInSeconds: Int) async throws { + func waitForWebViewLoad() async throws { wasWaitForWebViewLoadCalled = true }