TunnelsManager: Observe status for all tunnels in one block

WireGuard/WireGuard/VPN/TunnelsManager.swift

@@ -60,9 +60,11 @@ class TunnelsManager {
     private var tunnels: [TunnelContainer]
     weak var tunnelsListDelegate: TunnelsManagerListDelegate?
     weak var activationDelegate: TunnelsManagerActivationDelegate?
+    private var statusObservationToken: AnyObject?
 
     init(tunnelProviders: [NETunnelProviderManager]) {
         self.tunnels = tunnelProviders.map { TunnelContainer(tunnel: $0) }.sorted { $0.name < $1.name }
+        self.startObservingTunnelStatuses()
     }
 
     static func create(completionHandler: @escaping (WireGuardResult<TunnelsManager>) -> Void) {
@@ -268,26 +270,37 @@ class TunnelsManager {
             t.refreshStatus()
         }
     }
+
+    private func startObservingTunnelStatuses() {
+        if (statusObservationToken != nil) { return }
+        statusObservationToken = NotificationCenter.default.addObserver(
+            forName: .NEVPNStatusDidChange,
+            object: nil,
+            queue: nil) { [weak self] (statusChangeNotification) in
+                guard let session = statusChangeNotification.object as? NETunnelProviderSession else { return }
+                guard let tunnelProvider = session.manager as? NETunnelProviderManager else { return }
+                if let tunnel = self?.tunnels.first(where: { $0.tunnelProvider == tunnelProvider }) {
+                    tunnel.tunnelConnectionStatusDidChange()
+                } else if let tunnelName = tunnelProvider.localizedDescription, let tunnel = self?.tunnel(named: tunnelName) {
+                    tunnel.tunnelConnectionStatusDidChange()
+                }
+        }
+    }
+
 }
 
 class TunnelContainer: NSObject {
     @objc dynamic var name: String
     @objc dynamic var status: TunnelStatus
 
-    @objc dynamic var isActivateOnDemandEnabled: Bool {
+    @objc dynamic var isActivateOnDemandEnabled: Bool
-        didSet {
-            if (isActivateOnDemandEnabled) {
-                startObservingTunnelStatus()
-            }
-        }
-    }
 
     var isAttemptingActivation: Bool = false
     var onActivationCommitted: ((Bool) -> Void)?
     var onDeactivationComplete: (() -> Void)?
 
     fileprivate let tunnelProvider: NETunnelProviderManager
-    private var statusObservationToken: AnyObject?
+    private var lastTunnelConnectionStatus: NEVPNStatus?
 
     init(tunnel: NETunnelProviderManager) {
         self.name = tunnel.localizedDescription ?? "Unnamed"
@@ -296,9 +309,6 @@ class TunnelContainer: NSObject {
         self.isActivateOnDemandEnabled = tunnel.isOnDemandEnabled
         self.tunnelProvider = tunnel
         super.init()
-        if (status != .inactive || isActivateOnDemandEnabled) {
-            startObservingTunnelStatus()
-        }
     }
 
     func tunnelConfiguration() -> TunnelConfiguration? {
@@ -313,9 +323,6 @@ class TunnelContainer: NSObject {
         let status = TunnelStatus(from: self.tunnelProvider.connection.status)
         self.status = status
         self.isActivateOnDemandEnabled = self.tunnelProvider.isOnDemandEnabled
-        if (status != .inactive || isActivateOnDemandEnabled) {
-            startObservingTunnelStatus()
-        }
     }
 
     fileprivate func startActivation(completionHandler: @escaping (TunnelsManagerError?) -> Void) {
@@ -359,7 +366,6 @@ class TunnelContainer: NSObject {
         }
 
         // Start the tunnel
-        startObservingTunnelStatus()
         let session = (tunnelProvider.connection as! NETunnelProviderSession)
         do {
             os_log("startActivation: Starting tunnel", log: OSLog.default, type: .debug)
@@ -396,55 +402,51 @@ class TunnelContainer: NSObject {
     }
 
     fileprivate func startDeactivation() {
-        assert(status == .active)
+        assert(status == .active || status == .waiting)
-        assert(statusObservationToken != nil)
         let session = (tunnelProvider.connection as! NETunnelProviderSession)
         session.stopTunnel()
     }
 
     fileprivate func beginRestart() {
         assert(status == .active || status == .activating || status == .reasserting)
-        assert(statusObservationToken != nil)
         status = .restarting
         let session = (tunnelProvider.connection as! NETunnelProviderSession)
         session.stopTunnel()
     }
 
-    private func startObservingTunnelStatus() {
+    fileprivate func tunnelConnectionStatusDidChange() {
-        if (statusObservationToken != nil) { return }
         let connection = tunnelProvider.connection
-        statusObservationToken = NotificationCenter.default.addObserver(
+        // Avoid acting on duplicate notifications of status change
-            forName: .NEVPNStatusDidChange,
+        if let lastTunnelConnectionStatus = self.lastTunnelConnectionStatus {
-            object: connection,
+            if (lastTunnelConnectionStatus == connection.status) {
-            queue: nil) { [weak self] (_) in
+                return
-                guard let s = self else { return }
+            }
-                if (s.isAttemptingActivation) {
+        }
-                    if (connection.status == .connecting || connection.status == .connected) {
+        self.lastTunnelConnectionStatus = connection.status
-                        // We tried to start the tunnel, and that attempt is on track to become succeessful
+        // Act on the status change
-                        s.onActivationCommitted?(true)
+        if (self.isAttemptingActivation) {
-                        s.onActivationCommitted = nil
+            if (connection.status == .connecting || connection.status == .connected) {
-                    } else if (connection.status == .disconnecting || connection.status == .disconnected) {
+                // We tried to start the tunnel, and that attempt is on track to become succeessful
-                        // We tried to start the tunnel, but that attempt didn't succeed
+                self.onActivationCommitted?(true)
-                        s.onActivationCommitted?(false)
+                self.onActivationCommitted = nil
-                        s.onActivationCommitted = nil
+            } else if (connection.status == .disconnecting || connection.status == .disconnected) {
-                    }
+                // We tried to start the tunnel, but that attempt didn't succeed
-                    s.isAttemptingActivation = false
+                self.onActivationCommitted?(false)
-                }
+                self.onActivationCommitted = nil
-                if ((s.status == .restarting) && (connection.status == .disconnected || connection.status == .disconnecting)) {
+            }
-                    // Don't change s.status when disconnecting for a restart
+            self.isAttemptingActivation = false
-                    if (connection.status == .disconnected) {
+        }
-                        self?.startActivation(completionHandler: { _ in })
+        if ((self.status == .restarting) && (connection.status == .disconnected || connection.status == .disconnecting)) {
-                    }
+            // Don't change self.status when disconnecting for a restart
-                    return
+            if (connection.status == .disconnected) {
-                }
+                self.startActivation(completionHandler: { _ in })
-                s.status = TunnelStatus(from: connection.status)
+            }
-                if (s.status == .inactive) {
+            return
-                    s.onDeactivationComplete?()
+        }
-                    s.onDeactivationComplete = nil
+        self.status = TunnelStatus(from: connection.status)
-                    if (!s.isActivateOnDemandEnabled) {
+        if (self.status == .inactive) {
-                        s.statusObservationToken = nil
+            self.onDeactivationComplete?()
-                    }
+            self.onDeactivationComplete = nil
-                }
         }
     }
 }