Fix memory leaks through BrowserTab

Through the debugger, one could see that after they were closed, tabs
were remaining in memory: the `deinit` routine was never called.

Leak caused by passing delegate for link hover displaying as script
message handler. This led to a retain cycle: delegate retains WebView
retains Configuration retains UserContentController retains delegate (as
script listener)

To break it, adopt the same strategy as with the other script handlers:
- Custom listener object that only weakly retains the HoverUiDelegate
- The delegate is only weakly retained by the CustomWkWebView

Thus, the UserContentController transitively only holds a weak reference
to the delegate.

Also, put the link hover ui into BrowserTab+Interface.swift because it
purely deals with the UI.

Cherry-pick and squash of commit 8ee5d9c
and commit a884081

Issue #1667
Co-authored-by: Tassilo Karge <[email protected]>

Author: barijaona

Vienna.xcodeproj/project.pbxproj

@@ -71,6 +71,7 @@
 		2F88B2A62545BB450067EEA6 /* ArticleConverter.m in Sources */ = {isa = PBXBuildFile; fileRef = 2F88B2A52545BB450067EEA6 /* ArticleConverter.m */; };
 		2F88B2AD2545BCA90067EEA6 /* WebViewArticleConverter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2F88B2AC2545BCA90067EEA6 /* WebViewArticleConverter.swift */; };
 		2FA946C52517D8D8006134C5 /* CustomWKUIDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2FA946C42517D8D8006134C5 /* CustomWKUIDelegate.swift */; };
+		2FB5A9E929E4BCD800409BA5 /* CustomWKHoverUIDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2FB5A9E829E4BCD800409BA5 /* CustomWKHoverUIDelegate.swift */; };
 		2FC4A190257CF775005FF227 /* WebKitArticleConverter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2FC4A18F257CF775005FF227 /* WebKitArticleConverter.swift */; };
 		2FC4A1BF25852C0E005FF227 /* ArticleConverter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2FC4A1BE25852C0E005FF227 /* ArticleConverter.swift */; };
 		2FC91DDD21850ADB00D7F223 /* Browser+WebUIDelegate.m in Sources */ = {isa = PBXBuildFile; fileRef = 2FC91DDB21850ADB00D7F223 /* Browser+WebUIDelegate.m */; };
@@ -308,6 +309,7 @@
 		2F88B2AC2545BCA90067EEA6 /* WebViewArticleConverter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WebViewArticleConverter.swift; sourceTree = "<group>"; };
 		2F88B2C22545DAB40067EEA6 /* ArticleViewDelegate.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; lineEnding = 0; path = ArticleViewDelegate.h; sourceTree = "<group>"; };
 		2FA946C42517D8D8006134C5 /* CustomWKUIDelegate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CustomWKUIDelegate.swift; sourceTree = "<group>"; };
+		2FB5A9E829E4BCD800409BA5 /* CustomWKHoverUIDelegate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CustomWKHoverUIDelegate.swift; sourceTree = "<group>"; };
 		2FC4A17A25797A3D005FF227 /* WKPreferences+Private.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "WKPreferences+Private.h"; sourceTree = "<group>"; };
 		2FC4A18F257CF775005FF227 /* WebKitArticleConverter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WebKitArticleConverter.swift; sourceTree = "<group>"; };
 		2FC4A1BE25852C0E005FF227 /* ArticleConverter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ArticleConverter.swift; sourceTree = "<group>"; };
@@ -1097,6 +1099,7 @@
 				2FC4A17A25797A3D005FF227 /* WKPreferences+Private.h */,
 				2F82DCA925CED70500F00EE8 /* WKWebView+Private.h */,
 				F6879288270117AF009B7BB5 /* WKWebViewConfiguration+Private.h */,
+				2FB5A9E829E4BCD800409BA5 /* CustomWKHoverUIDelegate.swift */,
 			);
 			name = CustomWebKit;
 			sourceTree = "<group>";
@@ -2044,6 +2047,7 @@
 				2F125EE024D066ED00868E11 /* AsyncHelper.swift in Sources */,
 				AA71168F09BEB17300E3EA8A /* AddressBarCell.m in Sources */,
 				0379E2381A2082E600D58BFE /* AppearancePreferencesViewController.m in Sources */,
+				2FB5A9E929E4BCD800409BA5 /* CustomWKHoverUIDelegate.swift in Sources */,
 				F63833D81EF842EA00E07213 /* MainWindowController.swift in Sources */,
 				F41CA694218F56140072F734 /* NSWorkspace+OpenWithMenu.m in Sources */,
 				2FF61D3024F2ED1500A7E2B5 /* BrowserTab+Interface.swift in Sources */,

Vienna/Sources/Main window/BrowserTab+Interface.swift

@@ -166,3 +166,23 @@ extension BrowserTab: NSTextFieldDelegate {
     // TODO: things like address suggestion etc
     // TODO: restore url string when user presses escape in textfield, make webview first responder
 }
+
+// MARK: hover link ui
+
+extension BrowserTab: CustomWKHoverUIDelegate {
+
+    func viewDidLoadHoverLinkUI() {
+        registerNavigationStartHandler { [weak self] in
+            //TODO is it really necessary to do this on every navigation start instead of only once after webview initialization?
+            self?.webView.hoverUiDelegate = self
+            self?.statusBar?.label = ""
+        }
+    }
+
+    func hovered(link: String?) {
+        guard let statusBar = statusBar else {
+            return
+        }
+        statusBar.label = link
+    }
+}

Vienna/Sources/Main window/BrowserTab.swift

@@ -76,7 +76,7 @@ class BrowserTab: NSViewController {
     private var urlObservation: NSKeyValueObservation?
     private var statusBarObservation: NSKeyValueObservation?
 
-    private var statusBar: OverlayStatusBar?
+    private(set) var statusBar: OverlayStatusBar?
 
     // MARK: object lifecycle
 
@@ -171,6 +171,8 @@ class BrowserTab: NSViewController {
 
         self.viewDidLoadRss()
 
+        self.viewDidLoadHoverLinkUI()
+
         updateWebViewInsets()
 
         // set up address bar handling
@@ -301,13 +303,8 @@ extension BrowserTab: Tab {
 
     func closeTab() {
         stopLoadingTab()
-        // free webView by force stopping JavaScript and resetting delegates
-        self.webView.evaluateJavaScript("window.location.replace('about:blank');") { _, _ in
-            DispatchQueue.main.async {
-                self.webView.navigationDelegate = nil
-                self.webView.uiDelegate = nil
-            }
-        }
+        // free webView by force stopping JavaScript
+        self.webView.evaluateJavaScript("window.location.replace('about:blank');")
     }
 
     @objc
@@ -346,10 +343,6 @@ extension BrowserTab: WKNavigationDelegate {
 
     func webView(_ webView: WKWebView, didStartProvisionalNavigation navigation: WKNavigation?) {
         handleNavigationStart()
-        if let webView = webView as? CustomWKWebView {
-            webView.hoverListener = self
-            self.statusBar?.label = ""
-        }
     }
 
     func webView(_ webView: WKWebView, didFailProvisionalNavigation navigation: WKNavigation?, withError error: Error) {
@@ -384,20 +377,3 @@ extension BrowserTab: WKNavigationDelegate {
         self.navigationEndHandler.append(navigationEndHandler)
     }
 }
-
-// MARK: Javascript messages handler
-
-extension BrowserTab: WKScriptMessageHandler {
-
-    func userContentController(_ userContentController: WKUserContentController, didReceive message: WKScriptMessage) {
-        if self.statusBar != nil {
-            if message.name == CustomWKWebView.mouseDidEnterName {
-                if let link = message.body as? String {
-                    self.statusBar?.label = link
-                }
-            } else if message.name == CustomWKWebView.mouseDidExitName {
-                self.statusBar?.label = ""
-            }
-        }
-    }
-}

Vienna/Sources/Main window/CustomWKHoverUIDelegate.swift

@@ -0,0 +1,26 @@
+//
+//  CustomWKHoverUIDelegate.swift
+//  Vienna
+//
+//  Copyright 2023 Tassilo Karge
+//
+//  Licensed under the Apache License, Version 2.0 (the "License");
+//  you may not use this file except in compliance with the License.
+//  You may obtain a copy of the License at
+//
+//  https://www.apache.org/licenses/LICENSE-2.0
+//
+//  Unless required by applicable law or agreed to in writing, software
+//  distributed under the License is distributed on an "AS IS" BASIS,
+//  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+//  See the License for the specific language governing permissions and
+//  limitations under the License.
+//
+
+import Foundation
+
+//TODO: to me it looks like this protocol should be part of the CustomWKUIDelegate, but the unified display view does not declare conformance to that protocol. Probably that is a design flaw in how the unified display view integrates the browser.
+@objc protocol CustomWKHoverUIDelegate {
+    @objc
+    func hovered(link: String?)
+}

Vienna/Sources/Main window/CustomWKWebView.swift

@@ -24,15 +24,9 @@ class CustomWKWebView: WKWebView {
         }
     }
 
-    @objc weak var hoverListener: WKScriptMessageHandler? {
+    @objc weak var hoverUiDelegate: CustomWKHoverUIDelegate? {
         didSet {
-            let contentController = self.configuration.userContentController
-            contentController.removeScriptMessageHandler(forName: CustomWKWebView.mouseDidEnterName)
-            contentController.removeScriptMessageHandler(forName: CustomWKWebView.mouseDidExitName)
-            if let hoverListener = hoverListener {
-                contentController.add(hoverListener, name: CustomWKWebView.mouseDidEnterName)
-                contentController.add(hoverListener, name: CustomWKWebView.mouseDidExitName)
-            }
+            resetHoverUiListener()
         }
     }
 
@@ -118,6 +112,18 @@ class CustomWKWebView: WKWebView {
         self.contextMenuListener = contextMenuListener
         contentController.add(contextMenuListener, name: CustomWKWebView.clickListenerName)
         contentController.add(CustomWKWebViewErrorListener(), name: CustomWKWebView.jsErrorListenerName)
+        resetHoverUiListener()
+    }
+
+    func resetHoverUiListener() {
+        let contentController = self.configuration.userContentController
+        contentController.removeScriptMessageHandler(forName: CustomWKWebView.mouseDidEnterName)
+        contentController.removeScriptMessageHandler(forName: CustomWKWebView.mouseDidExitName)
+        if let hoverUiDelegate = hoverUiDelegate {
+            let hoverListener = CustomWKWebViewHoverListener(hoverDelegate: hoverUiDelegate)
+            contentController.add(hoverListener, name: CustomWKWebView.mouseDidEnterName)
+            contentController.add(hoverListener, name: CustomWKWebView.mouseDidExitName)
+        }
     }
 
     func search(_ text: String = "", upward: Bool = false) {
@@ -344,6 +350,28 @@ extension CustomWKWebView {
     }
 }
 
+class CustomWKWebViewHoverListener: NSObject, WKScriptMessageHandler {
+
+    weak var hoverDelegate: CustomWKHoverUIDelegate?
+
+    init(hoverDelegate: CustomWKHoverUIDelegate) {
+        self.hoverDelegate = hoverDelegate
+    }
+
+    func userContentController(_ userContentController: WKUserContentController, didReceive message: WKScriptMessage) {
+        guard let hoverDelegate = hoverDelegate else {
+            return
+        }
+        if message.name == CustomWKWebView.mouseDidEnterName {
+            if let link = message.body as? String {
+                hoverDelegate.hovered(link: link)
+            }
+        } else if message.name == CustomWKWebView.mouseDidExitName {
+            hoverDelegate.hovered(link: nil)
+        }
+    }
+}
+
 class CustomWKWebViewContextMenuListener: NSObject, WKScriptMessageHandler {
 
     var lastRightClickedLink: URL?

Vienna/Sources/Main window/UnifiedDisplayView.h

@@ -25,7 +25,7 @@
 @class AppController;
 @class ExtendedTableView;
 
-@interface UnifiedDisplayView : NSView <BaseView, ArticleBaseView, NSMenuItemValidation, MessageListViewDelegate, NSTableViewDataSource, WKScriptMessageHandler>
+@interface UnifiedDisplayView : NSView <BaseView, ArticleBaseView, NSMenuItemValidation, MessageListViewDelegate, NSTableViewDataSource>
 {
     IBOutlet ExtendedTableView *articleList;
 

Vienna/Sources/Main window/UnifiedDisplayView.m

@@ -42,7 +42,7 @@
 #define YPOS_IN_CELL	2.0
 #define PROGRESS_INDICATOR_DIMENSION 16
 
-@interface UnifiedDisplayView ()
+@interface UnifiedDisplayView () <CustomWKHoverUIDelegate>
 
 @property (nonatomic) OverlayStatusBar *statusBar;
 
@@ -766,7 +766,7 @@ - (NSView *)tableView:(NSTableView *)tableView viewForTableColumn:(NSTableColumn
     if ([articleContentView isKindOfClass:WebKitArticleView.class])
     {
         view = (WebKitArticleView *)articleContentView;
-        ((CustomWKWebView *)view).hoverListener = self;
+        ((CustomWKWebView *)view).hoverUiDelegate = self;
     } else {
         view = ((ArticleView *) articleContentView);
     }
@@ -1018,15 +1018,10 @@ - (void)observeValueForKeyPath:(NSString *)keyPath
     }
 }
 
-// MARK: WKScriptMessageHandler
+// MARK: CustomWKHoverUIDelegate
 
-- (void)userContentController:(WKUserContentController *)userContentController didReceiveScriptMessage:(WKScriptMessage *)message
-{
-    if ([message.name isEqualToString:CustomWKWebView.mouseDidEnterName]) {
-        NSString * link = (NSString *)message.body;
-        self.statusBar.label = link;
-    } else if ([message.name isEqualToString:CustomWKWebView.mouseDidExitName]) {
-        self.statusBar.label = nil;
-    }
+-(void)hoveredWithLink:(NSString *)link {
+    self.statusBar.label = link;
 }
+
 @end