From 631286e2d12670e44ff9c74f9608ea740773228a Mon Sep 17 00:00:00 2001 From: Andrej Mihajlov Date: Tue, 22 Dec 2020 11:09:18 +0100 Subject: [PATCH] UI: use NotificationToken to properly clean up observers When the variable goes out of scope, the observer isn't removed unless an explicit call is made to the token. Signed-off-by: Andrej Mihajlov --- Sources/Shared/NotificationToken.swift | 33 +++++++++++++++ .../WireGuardApp/Tunnel/TunnelsManager.swift | 10 ++--- .../ViewController/LogViewController.swift | 7 +++- WireGuard.xcodeproj/project.pbxproj | 42 +++++++++++-------- 4 files changed, 67 insertions(+), 25 deletions(-) create mode 100644 Sources/Shared/NotificationToken.swift diff --git a/Sources/Shared/NotificationToken.swift b/Sources/Shared/NotificationToken.swift new file mode 100644 index 0000000..df5bc25 --- /dev/null +++ b/Sources/Shared/NotificationToken.swift @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: MIT +// Copyright © 2018-2020 WireGuard LLC. All Rights Reserved. + +import Foundation + +/// This source file contains bits of code from: +/// https://oleb.net/blog/2018/01/notificationcenter-removeobserver/ + +/// Wraps the observer token received from +/// `NotificationCenter.addObserver(forName:object:queue:using:)` +/// and unregisters it in deinit. +final class NotificationToken { + let notificationCenter: NotificationCenter + let token: Any + + init(notificationCenter: NotificationCenter = .default, token: Any) { + self.notificationCenter = notificationCenter + self.token = token + } + + deinit { + notificationCenter.removeObserver(token) + } +} + +extension NotificationCenter { + /// Convenience wrapper for addObserver(forName:object:queue:using:) + /// that returns our custom `NotificationToken`. + func observe(name: NSNotification.Name?, object obj: Any?, queue: OperationQueue?, using block: @escaping (Notification) -> Void) -> NotificationToken { + let token = addObserver(forName: name, object: obj, queue: queue, using: block) + return NotificationToken(notificationCenter: self, token: token) + } +} diff --git a/Sources/WireGuardApp/Tunnel/TunnelsManager.swift b/Sources/WireGuardApp/Tunnel/TunnelsManager.swift index 024c3fd..93aa384 100644 --- a/Sources/WireGuardApp/Tunnel/TunnelsManager.swift +++ b/Sources/WireGuardApp/Tunnel/TunnelsManager.swift @@ -23,9 +23,9 @@ class TunnelsManager { private var tunnels: [TunnelContainer] weak var tunnelsListDelegate: TunnelsManagerListDelegate? weak var activationDelegate: TunnelsManagerActivationDelegate? - private var statusObservationToken: AnyObject? - private var waiteeObservationToken: AnyObject? - private var configurationsObservationToken: AnyObject? + private var statusObservationToken: NotificationToken? + private var waiteeObservationToken: NSKeyValueObservation? + private var configurationsObservationToken: NotificationToken? init(tunnelProviders: [NETunnelProviderManager]) { tunnels = tunnelProviders.map { TunnelContainer(tunnel: $0) }.sorted { TunnelsManager.tunnelNameIsLessThan($0.name, $1.name) } @@ -406,7 +406,7 @@ class TunnelsManager { } private func startObservingTunnelStatuses() { - statusObservationToken = NotificationCenter.default.addObserver(forName: .NEVPNStatusDidChange, object: nil, queue: OperationQueue.main) { [weak self] statusChangeNotification in + statusObservationToken = NotificationCenter.default.observe(name: .NEVPNStatusDidChange, object: nil, queue: OperationQueue.main) { [weak self] statusChangeNotification in guard let self = self, let session = statusChangeNotification.object as? NETunnelProviderSession, let tunnelProvider = session.manager as? NETunnelProviderManager, @@ -438,7 +438,7 @@ class TunnelsManager { } func startObservingTunnelConfigurations() { - configurationsObservationToken = NotificationCenter.default.addObserver(forName: .NEVPNConfigurationChange, object: nil, queue: OperationQueue.main) { [weak self] _ in + configurationsObservationToken = NotificationCenter.default.observe(name: .NEVPNConfigurationChange, object: nil, queue: OperationQueue.main) { [weak self] _ in DispatchQueue.main.async { [weak self] in // We schedule reload() in a subsequent runloop to ensure that the completion handler of loadAllFromPreferences // (reload() calls loadAllFromPreferences) is called after the completion handler of the saveToPreferences or diff --git a/Sources/WireGuardApp/UI/macOS/ViewController/LogViewController.swift b/Sources/WireGuardApp/UI/macOS/ViewController/LogViewController.swift index c3c8d87..ca4e575 100644 --- a/Sources/WireGuardApp/UI/macOS/ViewController/LogViewController.swift +++ b/Sources/WireGuardApp/UI/macOS/ViewController/LogViewController.swift @@ -18,6 +18,9 @@ class LogViewController: NSViewController { } } + private var boundsChangedNotificationToken: NotificationToken? + private var frameChangedNotificationToken: NotificationToken? + let scrollView: NSScrollView = { let scrollView = NSScrollView() scrollView.hasVerticalScroller = true @@ -104,13 +107,13 @@ class LogViewController: NSViewController { clipView.documentView = tableView scrollView.contentView = clipView - _ = NotificationCenter.default.addObserver(forName: NSView.boundsDidChangeNotification, object: clipView, queue: OperationQueue.main) { [weak self] _ in + boundsChangedNotificationToken = NotificationCenter.default.observe(name: NSView.boundsDidChangeNotification, object: clipView, queue: OperationQueue.main) { [weak self] _ in guard let self = self else { return } let lastVisibleRowIndex = self.tableView.row(at: NSPoint(x: 0, y: self.scrollView.contentView.documentVisibleRect.maxY - 1)) self.isInScrolledToEndMode = lastVisibleRowIndex < 0 || lastVisibleRowIndex == self.logEntries.count - 1 } - _ = NotificationCenter.default.addObserver(forName: NSView.frameDidChangeNotification, object: tableView, queue: OperationQueue.main) { [weak self] _ in + frameChangedNotificationToken = NotificationCenter.default.observe(name: NSView.frameDidChangeNotification, object: tableView, queue: OperationQueue.main) { [weak self] _ in guard let self = self else { return } if self.isInScrolledToEndMode { DispatchQueue.main.async { diff --git a/WireGuard.xcodeproj/project.pbxproj b/WireGuard.xcodeproj/project.pbxproj index 1b282d2..d192148 100644 --- a/WireGuard.xcodeproj/project.pbxproj +++ b/WireGuard.xcodeproj/project.pbxproj @@ -7,6 +7,8 @@ objects = { /* Begin PBXBuildFile section */ + 58233BCF2591F842002060A8 /* NotificationToken.swift in Sources */ = {isa = PBXBuildFile; fileRef = 58233BCE2591F842002060A8 /* NotificationToken.swift */; }; + 58233BD02591F842002060A8 /* NotificationToken.swift in Sources */ = {isa = PBXBuildFile; fileRef = 58233BCE2591F842002060A8 /* NotificationToken.swift */; }; 585B105A2577E293004F691E /* InterfaceConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 585B10462577E293004F691E /* InterfaceConfiguration.swift */; }; 585B105B2577E293004F691E /* InterfaceConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 585B10462577E293004F691E /* InterfaceConfiguration.swift */; }; 585B105C2577E293004F691E /* InterfaceConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 585B10462577E293004F691E /* InterfaceConfiguration.swift */; }; @@ -280,6 +282,7 @@ /* End PBXCopyFilesBuildPhase section */ /* Begin PBXFileReference section */ + 58233BCE2591F842002060A8 /* NotificationToken.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NotificationToken.swift; sourceTree = ""; }; 585B10462577E293004F691E /* InterfaceConfiguration.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = InterfaceConfiguration.swift; sourceTree = ""; }; 585B10472577E293004F691E /* PeerConfiguration.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PeerConfiguration.swift; sourceTree = ""; }; 585B10482577E293004F691E /* DNSServer.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DNSServer.swift; sourceTree = ""; }; @@ -344,6 +347,24 @@ 6F6483E6229293300075BA15 /* LaunchedAtLoginDetector.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LaunchedAtLoginDetector.swift; sourceTree = ""; }; 6F689999218043390012E523 /* WireGuard-Bridging-Header.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "WireGuard-Bridging-Header.h"; sourceTree = ""; }; 6F70E20D221058DF008BDFB4 /* Base */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = Base; path = Sources/WireGuardApp/Base.lproj/InfoPlist.strings; sourceTree = ""; }; + 6F70E20D221058DF008BDFB5 /* fr */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = fr; path = Sources/WireGuardApp/fr.lproj/Localizable.strings; sourceTree = ""; }; + 6F70E20D221058DF008BDFB6 /* pl */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = pl; path = Sources/WireGuardApp/pl.lproj/Localizable.strings; sourceTree = ""; }; + 6F70E20D221058DF008BDFB7 /* es */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = es; path = Sources/WireGuardApp/es.lproj/Localizable.strings; sourceTree = ""; }; + 6F70E20D221058DF008BDFB8 /* ca */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = ca; path = Sources/WireGuardApp/ca.lproj/Localizable.strings; sourceTree = ""; }; + 6F70E20D221058DF008BDFB9 /* pa */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = pa; path = Sources/WireGuardApp/pa.lproj/Localizable.strings; sourceTree = ""; }; + 6F70E20D221058DF008BDFBA /* zh-Hans */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = "zh-Hans"; path = "Sources/WireGuardApp/zh-Hans.lproj/Localizable.strings"; sourceTree = ""; }; + 6F70E20D221058DF008BDFBB /* ja */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = ja; path = Sources/WireGuardApp/ja.lproj/Localizable.strings; sourceTree = ""; }; + 6F70E20D221058DF008BDFBC /* it */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = it; path = Sources/WireGuardApp/it.lproj/Localizable.strings; sourceTree = ""; }; + 6F70E20D221058DF008BDFBD /* ro */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = ro; path = Sources/WireGuardApp/ro.lproj/Localizable.strings; sourceTree = ""; }; + 6F70E20D221058DF008BDFBE /* fa */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = fa; path = Sources/WireGuardApp/fa.lproj/Localizable.strings; sourceTree = ""; }; + 6F70E20D221058DF008BDFBF /* fi */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = fi; path = Sources/WireGuardApp/fi.lproj/Localizable.strings; sourceTree = ""; }; + 6F70E20D221058DF008BDFC0 /* ru */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = ru; path = Sources/WireGuardApp/ru.lproj/Localizable.strings; sourceTree = ""; }; + 6F70E20D221058DF008BDFC1 /* ko */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = ko; path = Sources/WireGuardApp/ko.lproj/Localizable.strings; sourceTree = ""; }; + 6F70E20D221058DF008BDFC2 /* tr */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = tr; path = Sources/WireGuardApp/tr.lproj/Localizable.strings; sourceTree = ""; }; + 6F70E20D221058DF008BDFC3 /* de */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = de; path = Sources/WireGuardApp/de.lproj/Localizable.strings; sourceTree = ""; }; + 6F70E20D221058DF008BDFC4 /* sl */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = sl; path = Sources/WireGuardApp/sl.lproj/Localizable.strings; sourceTree = ""; }; + 6F70E20D221058DF008BDFC5 /* id */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = id; path = Sources/WireGuardApp/id.lproj/Localizable.strings; sourceTree = ""; }; + 6F70E20D221058DF008BDFC6 /* zh-Hant */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = "zh-Hant"; path = "Sources/WireGuardApp/zh-Hant.lproj/Localizable.strings"; sourceTree = ""; }; 6F70E22922106A2D008BDFB4 /* WireGuardLoginItemHelper.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = WireGuardLoginItemHelper.app; sourceTree = BUILT_PRODUCTS_DIR; }; 6F70E23222106A31008BDFB4 /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = ""; }; 6F70E23922109BEF008BDFB4 /* LoginItemHelper.entitlements */ = {isa = PBXFileReference; lastKnownFileType = text.plist.entitlements; path = LoginItemHelper.entitlements; sourceTree = ""; }; @@ -399,24 +420,6 @@ 6FDEF801218646B900D8FBF6 /* ZipArchive.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ZipArchive.swift; sourceTree = ""; }; 6FDEF805218725D200D8FBF6 /* SettingsTableViewController.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SettingsTableViewController.swift; sourceTree = ""; }; 6FE1765521C90BBE002690EA /* Base */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = Base; path = Sources/WireGuardApp/Base.lproj/Localizable.strings; sourceTree = ""; }; - 6F70E20D221058DF008BDFC6 /* zh-Hant */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = zh-Hant; path = Sources/WireGuardApp/zh-Hant.lproj/Localizable.strings; sourceTree = ""; }; - 6F70E20D221058DF008BDFBA /* zh-Hans */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = zh-Hans; path = Sources/WireGuardApp/zh-Hans.lproj/Localizable.strings; sourceTree = ""; }; - 6F70E20D221058DF008BDFC5 /* id */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = id; path = Sources/WireGuardApp/id.lproj/Localizable.strings; sourceTree = ""; }; - 6F70E20D221058DF008BDFBC /* it */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = it; path = Sources/WireGuardApp/it.lproj/Localizable.strings; sourceTree = ""; }; - 6F70E20D221058DF008BDFC3 /* de */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = de; path = Sources/WireGuardApp/de.lproj/Localizable.strings; sourceTree = ""; }; - 6F70E20D221058DF008BDFB5 /* fr */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = fr; path = Sources/WireGuardApp/fr.lproj/Localizable.strings; sourceTree = ""; }; - 6F70E20D221058DF008BDFBF /* fi */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = fi; path = Sources/WireGuardApp/fi.lproj/Localizable.strings; sourceTree = ""; }; - 6F70E20D221058DF008BDFBE /* fa */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = fa; path = Sources/WireGuardApp/fa.lproj/Localizable.strings; sourceTree = ""; }; - 6F70E20D221058DF008BDFC4 /* sl */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = sl; path = Sources/WireGuardApp/sl.lproj/Localizable.strings; sourceTree = ""; }; - 6F70E20D221058DF008BDFB6 /* pl */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = pl; path = Sources/WireGuardApp/pl.lproj/Localizable.strings; sourceTree = ""; }; - 6F70E20D221058DF008BDFB9 /* pa */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = pa; path = Sources/WireGuardApp/pa.lproj/Localizable.strings; sourceTree = ""; }; - 6F70E20D221058DF008BDFC1 /* ko */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = ko; path = Sources/WireGuardApp/ko.lproj/Localizable.strings; sourceTree = ""; }; - 6F70E20D221058DF008BDFB8 /* ca */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = ca; path = Sources/WireGuardApp/ca.lproj/Localizable.strings; sourceTree = ""; }; - 6F70E20D221058DF008BDFC0 /* ru */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = ru; path = Sources/WireGuardApp/ru.lproj/Localizable.strings; sourceTree = ""; }; - 6F70E20D221058DF008BDFBD /* ro */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = ro; path = Sources/WireGuardApp/ro.lproj/Localizable.strings; sourceTree = ""; }; - 6F70E20D221058DF008BDFC2 /* tr */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = tr; path = Sources/WireGuardApp/tr.lproj/Localizable.strings; sourceTree = ""; }; - 6F70E20D221058DF008BDFBB /* ja */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = ja; path = Sources/WireGuardApp/ja.lproj/Localizable.strings; sourceTree = ""; }; - 6F70E20D221058DF008BDFB7 /* es */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = es; path = Sources/WireGuardApp/es.lproj/Localizable.strings; sourceTree = ""; }; 6FE1765921C90E87002690EA /* LocalizationHelper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LocalizationHelper.swift; sourceTree = ""; }; 6FE254FA219C10800028284D /* ZipImporter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ZipImporter.swift; sourceTree = ""; }; 6FE254FE219C60290028284D /* ZipExporter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ZipExporter.swift; sourceTree = ""; }; @@ -580,6 +583,7 @@ 6F7774E6217201E0006A79B3 /* Model */, 6F5A2B4421AFDE020081EDD8 /* FileManager+Extension.swift */, 6B5C5E26220A48D30024272E /* Keychain.swift */, + 58233BCE2591F842002060A8 /* NotificationToken.swift */, ); name = Shared; path = Sources/Shared; @@ -1289,6 +1293,7 @@ isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; files = ( + 58233BD02591F842002060A8 /* NotificationToken.swift in Sources */, 6FBA101521D613F90051C35F /* Application.swift in Sources */, 6FB1BDCC21D50F5300A991BF /* TunnelsManager.swift in Sources */, 6F8F0D7222258153000E8335 /* ActivateOnDemandViewModel.swift in Sources */, @@ -1442,6 +1447,7 @@ 6F61F1EB21B937EF00483816 /* WireGuardResult.swift in Sources */, 6F7774F321774263006A79B3 /* TunnelEditTableViewController.swift in Sources */, 6FBA103B21D6B4290051C35F /* ErrorPresenterProtocol.swift in Sources */, + 58233BCF2591F842002060A8 /* NotificationToken.swift in Sources */, 585B10862577E294004F691E /* IPAddressRange.swift in Sources */, 6FDEF802218646BA00D8FBF6 /* ZipArchive.swift in Sources */, 585B10922577E294004F691E /* key.c in Sources */,