Unit tests for the PCL WeakEventManager (#280)

3 files changed, 222 insertions, 15 deletions

diff --git a/Xamarin.Forms.Core.UnitTests/WeakEventManagerTests.cs b/Xamarin.Forms.Core.UnitTests/WeakEventManagerTests.cs
new file mode 100644
index 00000000..8300613a
--- /dev/null
+++ b/Xamarin.Forms.Core.UnitTests/WeakEventManagerTests.cs
@@ -0,0 +1,189 @@
+using System;
+using NUnit.Framework;
+
+namespace Xamarin.Forms.Core.UnitTests
+{
+	[TestFixture]
+	public class WeakEventManagerTests
+	{
+		static int s_count;
+
+		static void Handler(object sender, EventArgs eventArgs)
+		{
+			s_count++;
+		}
+
+		internal class TestEventSource
+		{
+			readonly WeakEventManager _weakEventManager;
+
+			public TestEventSource()
+			{
+				_weakEventManager = new WeakEventManager();
+			}
+
+			public void FireTestEvent()
+			{
+				OnTestEvent();
+			}
+
+			internal event EventHandler TestEvent
+			{
+				add { _weakEventManager.AddEventHandler(nameof(TestEvent), value); }
+				remove { _weakEventManager.RemoveEventHandler(nameof(TestEvent), value); }
+			}
+
+			void OnTestEvent()
+			{
+				_weakEventManager.HandleEvent(this, EventArgs.Empty, nameof(TestEvent));
+			}
+		}
+
+		internal class TestSubscriber
+		{
+			public void Subscribe(TestEventSource source)
+			{
+				source.TestEvent += SourceOnTestEvent;
+			}
+
+			void SourceOnTestEvent(object sender, EventArgs eventArgs)
+			{
+				Assert.Fail();
+			}
+		}
+
+		[Test]
+		public void AddHandlerWithEmptyEventNameThrowsException()
+		{
+			var wem = new WeakEventManager();
+			Assert.Throws<ArgumentNullException>(() => wem.AddEventHandler("", (sender, args) => { }));
+		}
+
+		[Test]
+		public void AddHandlerWithNullEventHandlerThrowsException()
+		{
+			var wem = new WeakEventManager();
+			Assert.Throws<ArgumentNullException>(() => wem.AddEventHandler("test", null));
+		}
+
+		[Test]
+		public void AddHandlerWithNullEventNameThrowsException()
+		{
+			var wem = new WeakEventManager();
+			Assert.Throws<ArgumentNullException>(() => wem.AddEventHandler(null, (sender, args) => { }));
+		}
+
+		[Test]
+		public void CanRemoveStaticEventHandler()
+		{
+			int beforeRun = s_count;
+
+			var source = new TestEventSource();
+			source.TestEvent += Handler;
+			source.TestEvent -= Handler;
+
+			source.FireTestEvent();
+
+			Assert.IsTrue(s_count == beforeRun);
+		}
+
+		[Test]
+		public void EventHandlerCalled()
+		{
+			var called = false;
+
+			var source = new TestEventSource();
+			source.TestEvent += (sender, args) => { called = true; };
+
+			source.FireTestEvent();
+
+			Assert.IsTrue(called);
+		}
+
+		[Test]
+		public void FiringEventWithoutHandlerShouldNotThrow()
+		{
+			var source = new TestEventSource();
+			source.FireTestEvent();
+		}
+
+		[Test]
+		public void MultipleHandlersCalled()
+		{
+			var called1 = false;
+			var called2 = false;
+
+			var source = new TestEventSource();
+			source.TestEvent += (sender, args) => { called1 = true; };
+			source.TestEvent += (sender, args) => { called2 = true; };
+			source.FireTestEvent();
+
+			Assert.IsTrue(called1 && called2);
+		}
+
+		[Test]
+		public void RemoveHandlerWithEmptyEventNameThrowsException()
+		{
+			var wem = new WeakEventManager();
+			Assert.Throws<ArgumentNullException>(() => wem.RemoveEventHandler("", (sender, args) => { }));
+		}
+
+		[Test]
+		public void RemoveHandlerWithNullEventHandlerThrowsException()
+		{
+			var wem = new WeakEventManager();
+			Assert.Throws<ArgumentNullException>(() => wem.RemoveEventHandler("test", null));
+		}
+
+		[Test]
+		public void RemoveHandlerWithNullEventNameThrowsException()
+		{
+			var wem = new WeakEventManager();
+			Assert.Throws<ArgumentNullException>(() => wem.RemoveEventHandler(null, (sender, args) => { }));
+		}
+
+		[Test]
+		public void RemovingNonExistentHandlersShouldNotThrow()
+		{
+			var wem = new WeakEventManager();
+			wem.RemoveEventHandler("fake", (sender, args) => { });
+			wem.RemoveEventHandler("alsofake", Handler);
+		}
+
+		[Test]
+		public void StaticHandlerShouldRun()
+		{
+			int beforeRun = s_count;
+
+			var source = new TestEventSource();
+			source.TestEvent += Handler;
+
+			source.FireTestEvent();
+
+			Assert.IsTrue(s_count > beforeRun);
+		}
+
+		[Test]
+		public void VerifySubscriberCanBeCollected()
+		{
+			WeakReference wr = null;
+			var source = new TestEventSource();
+			new Action(() =>
+			{
+				var ts = new TestSubscriber();
+				wr = new WeakReference(ts);
+				ts.Subscribe(source);
+			})();
+
+			GC.Collect();
+			GC.WaitForPendingFinalizers();
+
+			Assert.IsNotNull(wr);
+			Assert.IsFalse(wr.IsAlive);
+
+			// The handler for this calls Assert.Fail, so if the subscriber has not been collected
+			// the handler will be called and the test will fail
+			source.FireTestEvent();
+		}
+	}
+}
\ No newline at end of file
diff --git a/Xamarin.Forms.Core.UnitTests/Xamarin.Forms.Core.UnitTests.csproj b/Xamarin.Forms.Core.UnitTests/Xamarin.Forms.Core.UnitTests.csproj
index 27fc07ee..4eae5d53 100644
--- a/Xamarin.Forms.Core.UnitTests/Xamarin.Forms.Core.UnitTests.csproj
+++ b/Xamarin.Forms.Core.UnitTests/Xamarin.Forms.Core.UnitTests.csproj
@@ -152,6 +152,7 @@
     <Compile Include="TypeUnitTests.cs" />
     <Compile Include="ViewCellTests.cs" />
     <Compile Include="ViewUnitTests.cs" />
+    <Compile Include="WeakEventManagerTests.cs" />
     <Compile Include="WebViewUnitTests.cs" />
     <Compile Include="ImageSourceTests.cs" />
     <Compile Include="DataTemplateTests.cs" />
diff --git a/Xamarin.Forms.Core/WeakEventManager.cs b/Xamarin.Forms.Core/WeakEventManager.cs
index 226f329a..11dabfb8 100644
--- a/Xamarin.Forms.Core/WeakEventManager.cs
+++ b/Xamarin.Forms.Core/WeakEventManager.cs
@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.Reflection;
+using static System.String;
 
 namespace Xamarin.Forms
 {
@@ -12,7 +13,7 @@ namespace Xamarin.Forms
 		public void AddEventHandler<TEventArgs>(string eventName, EventHandler<TEventArgs> handler)
 			where TEventArgs : EventArgs
 		{
-			if (eventName == null)
+			if (IsNullOrEmpty(eventName))
 			{
 				throw new ArgumentNullException(nameof(eventName));
 			}
@@ -27,7 +28,7 @@ namespace Xamarin.Forms
 
 		public void AddEventHandler(string eventName, EventHandler handler)
 		{
-			if (eventName == null)
+			if (IsNullOrEmpty(eventName))
 			{
 				throw new ArgumentNullException(nameof(eventName));
 			}
@@ -43,24 +44,38 @@ namespace Xamarin.Forms
 		public void HandleEvent(object sender, object args, string eventName)
 		{
 			var toRaise = new List<Tuple<object, MethodInfo>>();
+			var toRemove = new List<Subscription>();
 
 			List<Subscription> target;
 			if (_eventHandlers.TryGetValue(eventName, out target))
 			{
 				foreach (Subscription subscription in target)
 				{
+					bool isStatic = subscription.Subscriber == null;
+					if (isStatic)
+					{
+						// For a static method, we'll just pass null as the first parameter of MethodInfo.Invoke
+						toRaise.Add(Tuple.Create<object, MethodInfo>(null, subscription.Handler));
+						continue;
+					}
+
 					object subscriber = subscription.Subscriber.Target;
 
 					if (subscriber == null)
 					{
 						// The subscriber was collected, so there's no need to keep this subscription around
-						target.Remove(subscription);
+						toRemove.Add(subscription);
 					}
 					else
 					{
 						toRaise.Add(Tuple.Create(subscriber, subscription.Handler));
 					}
 				}
+
+				foreach (Subscription subscription in toRemove)
+				{
+					target.Remove(subscription);
+				}
 			}
 
 			foreach (Tuple<object, MethodInfo> tuple in toRaise)
@@ -72,7 +87,7 @@ namespace Xamarin.Forms
 		public void RemoveEventHandler<TEventArgs>(string eventName, EventHandler<TEventArgs> handler)
 			where TEventArgs : EventArgs
 		{
-			if (eventName == null)
+			if (IsNullOrEmpty(eventName))
 			{
 				throw new ArgumentNullException(nameof(eventName));
 			}
@@ -87,7 +102,7 @@ namespace Xamarin.Forms
 
 		public void RemoveEventHandler(string eventName, EventHandler handler)
 		{
-			if (eventName == null)
+			if (IsNullOrEmpty(eventName))
 			{
 				throw new ArgumentNullException(nameof(eventName));
 			}
@@ -102,14 +117,21 @@ namespace Xamarin.Forms
 
 		void AddEventHandler(string eventName, object handlerTarget, MethodInfo methodInfo)
 		{
-			List<Subscription> target;
-			if (!_eventHandlers.TryGetValue(eventName, out target))
+			List<Subscription> targets;
+			if (!_eventHandlers.TryGetValue(eventName, out targets))
 			{
-				target = new List<Subscription>();
-				_eventHandlers.Add(eventName, target);
+				targets = new List<Subscription>();
+				_eventHandlers.Add(eventName, targets);
 			}
 
-			target.Add(new Subscription(new WeakReference(handlerTarget), methodInfo));
+			if (handlerTarget == null)
+			{
+				// This event handler is a static method
+				targets.Add(new Subscription(null, methodInfo));
+				return;
+			}
+
+			targets.Add(new Subscription(new WeakReference(handlerTarget), methodInfo));
 		}
 
 		void RemoveEventHandler(string eventName, object handlerTarget, MemberInfo methodInfo)
@@ -138,11 +160,6 @@ namespace Xamarin.Forms
 		{
 			public Subscription(WeakReference subscriber, MethodInfo handler)
 			{
-				if (subscriber == null)
-				{
-					throw new ArgumentNullException(nameof(subscriber));
-				}
-
 				if (handler == null)
 				{
 					throw new ArgumentNullException(nameof(handler));