GH-179: Refactored the iOS Reachability (#184)

* Refactored the iOS Reachability #179
 - split the implementation into:
    - a static "get current state"
    - instance "listener"

* Just use the existing handler and clean up properly #179

Xamarin.Essentials/Connectivity/Connectivity.ios.cs

@@ -1,36 +1,40 @@
-using System;
-using System.Collections.Generic;
-using System.Threading.Tasks;
+using System.Collections.Generic;
 
 namespace Xamarin.Essentials
 {
     public static partial class Connectivity
     {
-        static void StartListeners() =>
-            Reachability.ReachabilityChanged += ReachabilityChanged;
+        static ReachabilityListener listener;
 
-        static async void ReachabilityChanged(object sender, EventArgs e)
+        static void StartListeners()
         {
-            await Task.Delay(100);
-            OnConnectivityChanged();
+            listener = new ReachabilityListener();
+            listener.ReachabilityChanged += OnConnectivityChanged;
         }
 
-        static void StopListeners() =>
-            Reachability.ReachabilityChanged -= ReachabilityChanged;
+        static void StopListeners()
+        {
+            if (listener == null)
+                return;
+
+            listener.ReachabilityChanged -= OnConnectivityChanged;
+            listener.Dispose();
+            listener = null;
+        }
 
         public static NetworkAccess NetworkAccess
         {
             get
             {
-                var remoteHostStatus = Reachability.RemoteHostStatus();
                 var internetStatus = Reachability.InternetConnectionStatus();
+                if (internetStatus == NetworkStatus.ReachableViaCarrierDataNetwork || internetStatus == NetworkStatus.ReachableViaWiFiNetwork)
+                    return NetworkAccess.Internet;
 
-                var isConnected = (internetStatus == NetworkStatus.ReachableViaCarrierDataNetwork ||
-                                internetStatus == NetworkStatus.ReachableViaWiFiNetwork) ||
-                              (remoteHostStatus == NetworkStatus.ReachableViaCarrierDataNetwork ||
-                                remoteHostStatus == NetworkStatus.ReachableViaWiFiNetwork);
+                var remoteHostStatus = Reachability.RemoteHostStatus();
+                if (remoteHostStatus == NetworkStatus.ReachableViaCarrierDataNetwork || remoteHostStatus == NetworkStatus.ReachableViaWiFiNetwork)
+                    return NetworkAccess.Internet;
 
-                return isConnected ? NetworkAccess.Internet : NetworkAccess.None;
+                return NetworkAccess.None;
             }
         }
 

Xamarin.Essentials/Connectivity/Connectivity.ios.reachability.cs

@@ -1,6 +1,5 @@
 using System;
 using System.Collections.Generic;
-using System.Diagnostics;
 using System.Net;
 using System.Threading.Tasks;
 using CoreFoundation;
@@ -17,174 +16,27 @@ namespace Xamarin.Essentials
 
     static class Reachability
     {
-        static string hostName = "www.microsoft.com";
-
-        /// <summary>
-        /// Checks if reachable without requiring a connection
-        /// </summary>
-        /// <param name="flags"></param>
-        /// <returns></returns>
-        internal static bool IsReachableWithoutRequiringConnection(NetworkReachabilityFlags flags)
-        {
-            // Is it reachable with the current network configuration?
-            var isReachable = (flags & NetworkReachabilityFlags.Reachable) != 0;
-
-            // Do we need a connection to reach it?
-            var noConnectionRequired = (flags & NetworkReachabilityFlags.ConnectionRequired) == 0;
-
-            // Since the network stack will automatically try to get the WAN up,
-            // probe that
-            if ((flags & NetworkReachabilityFlags.IsWWAN) != 0)
-                noConnectionRequired = true;
-
-            return isReachable && noConnectionRequired;
-        }
-
-        /// <summary>
-        /// Checks if host is reachable
-        /// </summary>
-        /// <param name="host"></param>
-        /// <param name="port"></param>
-        /// <returns></returns>
-        internal static bool IsHostReachable(string host, int port)
-        {
-            if (string.IsNullOrWhiteSpace(host))
-                return false;
-
-            if (!IPAddress.TryParse(host + ":" + port, out var address))
-            {
-                Debug.WriteLine(host + ":" + port + " is not valid");
-                return false;
-            }
-            using (var r = new NetworkReachability(host))
-            {
-                if (r.TryGetFlags(out var flags))
-                {
-                    return IsReachableWithoutRequiringConnection(flags);
-                }
-            }
-            return false;
-        }
-
-        /// <summary>
-        ///  Is the host reachable with the current network configuration
-        /// </summary>
-        /// <param name="host"></param>
-        /// <returns></returns>
-        internal static bool IsHostReachable(string host)
-        {
-            if (string.IsNullOrWhiteSpace(host))
-                return false;
-
-            using (var r = new NetworkReachability(host))
-            {
-                if (r.TryGetFlags(out var flags))
-                {
-                    return IsReachableWithoutRequiringConnection(flags);
-                }
-            }
-            return false;
-        }
-
-        /// <summary>
-        /// Raised every time there is an interesting reachable event,
-        /// we do not even pass the info as to what changed, and
-        /// we lump all three status we probe into one
-        /// </summary>
-        internal static event EventHandler ReachabilityChanged;
-
-        static async void OnChange(NetworkReachabilityFlags flags)
-        {
-            await Task.Delay(100);
-            ReachabilityChanged?.Invoke(null, EventArgs.Empty);
-        }
-
-        static NetworkReachability defaultRouteReachability;
-
-        static bool IsNetworkAvailable(out NetworkReachabilityFlags flags)
-        {
-            if (defaultRouteReachability == null)
-            {
-                var ip = new IPAddress(0);
-                defaultRouteReachability = new NetworkReachability(ip);
-                defaultRouteReachability.SetNotification(OnChange);
-                defaultRouteReachability.Schedule(CFRunLoop.Main, CFRunLoop.ModeDefault);
-            }
-            if (!defaultRouteReachability.TryGetFlags(out flags))
-                return false;
-            return IsReachableWithoutRequiringConnection(flags);
-        }
-
-        static NetworkReachability remoteHostReachability;
+        internal const string HostName = "www.microsoft.com";
 
         internal static NetworkStatus RemoteHostStatus()
         {
-            NetworkReachabilityFlags flags;
-            bool reachable;
-
-            if (remoteHostReachability == null)
+            using (var remoteHostReachability = new NetworkReachability(HostName))
             {
-                remoteHostReachability = new NetworkReachability(hostName);
+                var reachable = remoteHostReachability.TryGetFlags(out var flags);
 
-                // Need to probe before we queue, or we wont get any meaningful values
-                // this only happens when you create NetworkReachability from a hostname
-                reachable = remoteHostReachability.TryGetFlags(out flags);
+                if (!reachable)
+                    return NetworkStatus.NotReachable;
 
-                remoteHostReachability.SetNotification(OnChange);
-                remoteHostReachability.Schedule(CFRunLoop.Main, CFRunLoop.ModeDefault);
+                if (!IsReachableWithoutRequiringConnection(flags))
+                    return NetworkStatus.NotReachable;
+
+                if ((flags & NetworkReachabilityFlags.IsWWAN) != 0)
+                    return NetworkStatus.ReachableViaCarrierDataNetwork;
+
+                return NetworkStatus.ReachableViaWiFiNetwork;
             }
-            else
-            {
-                reachable = remoteHostReachability.TryGetFlags(out flags);
-            }
-
-            if (!reachable)
-                return NetworkStatus.NotReachable;
-
-            if (!IsReachableWithoutRequiringConnection(flags))
-                return NetworkStatus.NotReachable;
-
-            if ((flags & NetworkReachabilityFlags.IsWWAN) != 0)
-                return NetworkStatus.ReachableViaCarrierDataNetwork;
-
-            return NetworkStatus.ReachableViaWiFiNetwork;
         }
 
-        /// <summary>
-        /// Checks internet connection status
-        /// </summary>
-        /// <returns></returns>
-        internal static IEnumerable<NetworkStatus> GetActiveConnectionType()
-        {
-            var status = new List<NetworkStatus>();
-
-            var defaultNetworkAvailable = IsNetworkAvailable(out var flags);
-
-            // If it's a WWAN connection..
-            if ((flags & NetworkReachabilityFlags.IsWWAN) != 0)
-            {
-                status.Add(NetworkStatus.ReachableViaCarrierDataNetwork);
-            }
-            else if (defaultNetworkAvailable)
-            {
-                status.Add(NetworkStatus.ReachableViaWiFiNetwork);
-            }
-            else if (((flags & NetworkReachabilityFlags.ConnectionOnDemand) != 0
-                || (flags & NetworkReachabilityFlags.ConnectionOnTraffic) != 0)
-                && (flags & NetworkReachabilityFlags.InterventionRequired) == 0)
-            {
-                // If the connection is on-demand or on-traffic and no user intervention
-                // is required, then assume WiFi.
-                status.Add(NetworkStatus.ReachableViaWiFiNetwork);
-            }
-
-            return status;
-        }
-
-        /// <summary>
-        /// Checks internet connection status
-        /// </summary>
-        /// <returns></returns>
         public static NetworkStatus InternetConnectionStatus()
         {
             var status = NetworkStatus.NotReachable;
@@ -203,9 +55,8 @@ namespace Xamarin.Essentials
 
             // If the connection is on-demand or on-traffic and no user intervention
             // is required, then assume WiFi.
-            if (((flags & NetworkReachabilityFlags.ConnectionOnDemand) != 0
-                || (flags & NetworkReachabilityFlags.ConnectionOnTraffic) != 0)
-                && (flags & NetworkReachabilityFlags.InterventionRequired) == 0)
+            if (((flags & NetworkReachabilityFlags.ConnectionOnDemand) != 0 || (flags & NetworkReachabilityFlags.ConnectionOnTraffic) != 0) &&
+                 (flags & NetworkReachabilityFlags.InterventionRequired) == 0)
             {
                 status = NetworkStatus.ReachableViaWiFiNetwork;
             }
@@ -213,22 +64,100 @@ namespace Xamarin.Essentials
             return status;
         }
 
-        /// <summary>
-        /// Dispose
-        /// </summary>
-        internal static void Dispose()
+        internal static IEnumerable<NetworkStatus> GetActiveConnectionType()
         {
-            if (remoteHostReachability != null)
+            var status = new List<NetworkStatus>();
+
+            var defaultNetworkAvailable = IsNetworkAvailable(out var flags);
+
+            // If it's a WWAN connection..
+            if ((flags & NetworkReachabilityFlags.IsWWAN) != 0)
             {
-                remoteHostReachability.Dispose();
-                remoteHostReachability = null;
+                status.Add(NetworkStatus.ReachableViaCarrierDataNetwork);
+            }
+            else if (defaultNetworkAvailable)
+            {
+                status.Add(NetworkStatus.ReachableViaWiFiNetwork);
+            }
+            else if (((flags & NetworkReachabilityFlags.ConnectionOnDemand) != 0 || (flags & NetworkReachabilityFlags.ConnectionOnTraffic) != 0) &&
+                     (flags & NetworkReachabilityFlags.InterventionRequired) == 0)
+            {
+                // If the connection is on-demand or on-traffic and no user intervention
+                // is required, then assume WiFi.
+                status.Add(NetworkStatus.ReachableViaWiFiNetwork);
             }
 
-            if (defaultRouteReachability != null)
+            return status;
+        }
+
+        internal static bool IsNetworkAvailable(out NetworkReachabilityFlags flags)
+        {
+            var ip = new IPAddress(0);
+            using (var defaultRouteReachability = new NetworkReachability(ip))
             {
-                defaultRouteReachability.Dispose();
-                defaultRouteReachability = null;
+                if (!defaultRouteReachability.TryGetFlags(out flags))
+                    return false;
+
+                return IsReachableWithoutRequiringConnection(flags);
             }
         }
+
+        internal static bool IsReachableWithoutRequiringConnection(NetworkReachabilityFlags flags)
+        {
+            // Is it reachable with the current network configuration?
+            var isReachable = (flags & NetworkReachabilityFlags.Reachable) != 0;
+
+            // Do we need a connection to reach it?
+            var noConnectionRequired = (flags & NetworkReachabilityFlags.ConnectionRequired) == 0;
+
+            // Since the network stack will automatically try to get the WAN up,
+            // probe that
+            if ((flags & NetworkReachabilityFlags.IsWWAN) != 0)
+                noConnectionRequired = true;
+
+            return isReachable && noConnectionRequired;
+        }
+    }
+
+    class ReachabilityListener : IDisposable
+    {
+        NetworkReachability defaultRouteReachability;
+        NetworkReachability remoteHostReachability;
+
+        public ReachabilityListener()
+        {
+            var ip = new IPAddress(0);
+            defaultRouteReachability = new NetworkReachability(ip);
+            defaultRouteReachability.SetNotification(OnChange);
+            defaultRouteReachability.Schedule(CFRunLoop.Main, CFRunLoop.ModeDefault);
+
+            remoteHostReachability = new NetworkReachability(Reachability.HostName);
+
+            // Need to probe before we queue, or we wont get any meaningful values
+            // this only happens when you create NetworkReachability from a hostname
+            remoteHostReachability.TryGetFlags(out var flags);
+
+            remoteHostReachability.SetNotification(OnChange);
+            remoteHostReachability.Schedule(CFRunLoop.Main, CFRunLoop.ModeDefault);
+        }
+
+        public event Action ReachabilityChanged;
+
+        public void Dispose()
+        {
+            defaultRouteReachability?.Dispose();
+            defaultRouteReachability = null;
+            remoteHostReachability?.Dispose();
+            remoteHostReachability = null;
+        }
+
+        async void OnChange(NetworkReachabilityFlags flags)
+        {
+            // Add in artifical delay so the connection status has time to change
+            // else it will return true no matter what.
+            await Task.Delay(100);
+
+            ReachabilityChanged?.Invoke();
+        }
     }
 }