ResourceDictionary fixes (#536)

* [C] avoid leaking RDs, remove reflection call, validate arguments

* [C,Xaml] The only way to get merged values are internal

6 files changed, 110 insertions, 45 deletions

diff --git a/Xamarin.Forms.Core.UnitTests/ResourceDictionaryTests.cs b/Xamarin.Forms.Core.UnitTests/ResourceDictionaryTests.cs
index c4e6bbf3..e5b8b60d 100644
--- a/Xamarin.Forms.Core.UnitTests/ResourceDictionaryTests.cs
+++ b/Xamarin.Forms.Core.UnitTests/ResourceDictionaryTests.cs
@@ -253,13 +253,44 @@ namespace Xamarin.Forms.Core.UnitTests
 			Assert.Fail ();
 		}
 
-        [Test]
-        public void ShowKeyInExceptionIfNotFound()
-        {
-            var rd = new ResourceDictionary();
-            rd.Add("foo", "bar");
-            var ex = Assert.Throws<KeyNotFoundException>(() => { var foo = rd["test_invalid_key"]; });
-            Assert.That(ex.Message, Is.StringContaining("test_invalid_key"));
-        }
-    }
+		[Test]
+		public void ShowKeyInExceptionIfNotFound()
+		{
+			var rd = new ResourceDictionary();
+			rd.Add("foo", "bar");
+			var ex = Assert.Throws<KeyNotFoundException>(() => { var foo = rd ["test_invalid_key"]; });
+			Assert.That(ex.Message, Is.StringContaining("test_invalid_key"));
+		}
+
+		class MyRD : ResourceDictionary
+		{
+			public MyRD()
+			{
+				CreationCount = CreationCount + 1;
+				Add("foo", "Foo");
+				Add("bar", "Bar");
+			}
+
+			public static int CreationCount { get; set; }
+		}
+
+		[Test]
+		public void MergedWithFailsToMergeAnythingButRDs()
+		{
+			var rd = new ResourceDictionary();
+			Assert.DoesNotThrow(() => rd.MergedWith = typeof(MyRD));
+			Assert.Throws<ArgumentException>(() => rd.MergedWith = typeof(ContentPage));
+		}
+
+		[Test]
+		public void MergedResourcesAreFound()
+		{
+			var rd0 = new ResourceDictionary();
+			rd0.MergedWith = typeof(MyRD);
+
+			object _;
+			Assert.True(rd0.TryGetMergedValue("foo", out _));
+			Assert.AreEqual("Foo", _);
+		}
+	}
 }
\ No newline at end of file
diff --git a/Xamarin.Forms.Core/ResourceDictionary.cs b/Xamarin.Forms.Core/ResourceDictionary.cs
index 0747eaac..ed3ea7f5 100644
--- a/Xamarin.Forms.Core/ResourceDictionary.cs
+++ b/Xamarin.Forms.Core/ResourceDictionary.cs
@@ -2,13 +2,14 @@ using System;
 using System.Collections;
 using System.Collections.Generic;
 using System.Runtime.CompilerServices;
-using System.Reflection;
 using System.Linq;
+using System.Reflection;
 
 namespace Xamarin.Forms
 {
 	public class ResourceDictionary : IResourceDictionary, IDictionary<string, object>
 	{
+		static ConditionalWeakTable<Type, ResourceDictionary> s_instances = new ConditionalWeakTable<Type, ResourceDictionary>();
 		readonly Dictionary<string, object> _innerDictionary = new Dictionary<string, object>();
 
 		Type _mergedWith;
@@ -18,28 +19,19 @@ namespace Xamarin.Forms
 			set { 
 				if (_mergedWith == value)
 					return;
+
+				if (!typeof(ResourceDictionary).GetTypeInfo().IsAssignableFrom(value.GetTypeInfo()))
+					throw new ArgumentException("MergedWith should inherit from ResourceDictionary");
+
 				_mergedWith = value;
 				if (_mergedWith == null)
 					return;
 
-				_mergedInstance = _mergedWith.GetTypeInfo().BaseType.GetTypeInfo().DeclaredMethods.First(mi => mi.Name == "GetInstance").Invoke(null, new object[] {_mergedWith}) as ResourceDictionary;
+				_mergedInstance = s_instances.GetValue(_mergedWith,(key) => (ResourceDictionary)Activator.CreateInstance(key));
 				OnValuesChanged (_mergedInstance.ToArray());
 			}
 		}
 
-		static Dictionary<Type, ResourceDictionary> _instances;
-		static ResourceDictionary GetInstance(Type type)
-		{
-			_instances = _instances ?? new Dictionary<Type, ResourceDictionary>();
-			ResourceDictionary rd;
-			if (!_instances.TryGetValue(type, out rd))
-			{
-				rd = ((ResourceDictionary)Activator.CreateInstance(type));
-				_instances [type] = rd;
-			}
-			return rd;
-		}
-
 		ResourceDictionary _mergedInstance;
 
 		void ICollection<KeyValuePair<string, object>>.Add(KeyValuePair<string, object> item)
@@ -65,7 +57,7 @@ namespace Xamarin.Forms
 
 		public int Count
 		{
-			get { return _innerDictionary.Count + (_mergedInstance != null ? _mergedInstance.Count: 0); }
+			get { return _innerDictionary.Count; }
 		}
 
 		bool ICollection<KeyValuePair<string, object>>.IsReadOnly
@@ -96,8 +88,6 @@ namespace Xamarin.Forms
 			{
 				if (_innerDictionary.ContainsKey(index))
 					return _innerDictionary[index];
-				if (_mergedInstance != null && _mergedInstance.ContainsKey(index))
-					return _mergedInstance[index];
 				throw new KeyNotFoundException($"The resource '{index}' is not present in the dictionary.");
 			}
 			set
@@ -129,14 +119,26 @@ namespace Xamarin.Forms
 
 		public IEnumerator<KeyValuePair<string, object>> GetEnumerator()
 		{
-			var rd = (IEnumerable<KeyValuePair<string,object>>)_innerDictionary;
-			if (_mergedInstance != null)
-				rd = rd.Concat(_mergedInstance._innerDictionary);
-			return rd.GetEnumerator();
+			return _innerDictionary.GetEnumerator();
+		}
+
+		internal IEnumerable<KeyValuePair<string, object>> MergedResources {
+			get {
+				if (_mergedInstance != null)
+					foreach (var r in _mergedInstance.MergedResources)
+						yield return r;
+				foreach (var r in _innerDictionary)
+					yield return r;
+			}
 		}
 
 		public bool TryGetValue(string key, out object value)
 		{
+			return _innerDictionary.TryGetValue(key, out value);
+		}
+
+		internal bool TryGetMergedValue(string key, out object value)
+		{
 			return _innerDictionary.TryGetValue(key, out value) || (_mergedInstance != null && _mergedInstance.TryGetValue(key, out value));
 		}
 
diff --git a/Xamarin.Forms.Core/ResourcesExtensions.cs b/Xamarin.Forms.Core/ResourcesExtensions.cs
index a75e264a..8930abf2 100644
--- a/Xamarin.Forms.Core/ResourcesExtensions.cs
+++ b/Xamarin.Forms.Core/ResourcesExtensions.cs
@@ -3,7 +3,7 @@ using System.Collections.Generic;
 
 namespace Xamarin.Forms
 {
-	internal static class ResourcesExtensions
+	static class ResourcesExtensions
 	{
 		public static IEnumerable<KeyValuePair<string, object>> GetMergedResources(this IElement element)
 		{
@@ -11,10 +11,10 @@ namespace Xamarin.Forms
 			while (element != null)
 			{
 				var ve = element as IResourcesProvider;
-				if (ve != null && ve.Resources != null && ve.Resources.Count != 0)
+				if (ve != null && ve.Resources != null)
 				{
-					resources = resources ?? new Dictionary<string, object>(ve.Resources.Count);
-					foreach (KeyValuePair<string, object> res in ve.Resources)
+					resources = resources ?? new Dictionary<string, object>();
+					foreach (KeyValuePair<string, object> res in ve.Resources.MergedResources)
 						if (!resources.ContainsKey(res.Key))
 							resources.Add(res.Key, res.Value);
 						else if (res.Key.StartsWith(Style.StyleClassPrefix, StringComparison.Ordinal))
diff --git a/Xamarin.Forms.Xaml.UnitTests/TestSharedResourceDictionary.xaml b/Xamarin.Forms.Xaml.UnitTests/TestSharedResourceDictionary.xaml
index 62c1abd2..3e0f7f5b 100644
--- a/Xamarin.Forms.Xaml.UnitTests/TestSharedResourceDictionary.xaml
+++ b/Xamarin.Forms.Xaml.UnitTests/TestSharedResourceDictionary.xaml
@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="UTF-8"?>
+<?xml version="1.0" encoding="UTF-8"?>
 <ContentPage xmlns="http://xamarin.com/schemas/2014/forms"
 		xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
 		xmlns:local="clr-namespace:Xamarin.Forms.Xaml.UnitTests"
@@ -16,6 +16,7 @@
 				<ResourceDictionary MergedWith="local:SharedResourceDictionary2"/>
 			</ContentView.Resources>
 			<Label x:Name="label2" Style="{StaticResource sharedStyle2}"/>
+			<Label x:Name="label3" Text="{StaticResource foo}"/>
 		</ContentView>
 	</StackLayout>
 </ContentPage>
\ No newline at end of file
diff --git a/Xamarin.Forms.Xaml.UnitTests/TestSharedResourceDictionary.xaml.cs b/Xamarin.Forms.Xaml.UnitTests/TestSharedResourceDictionary.xaml.cs
index a272decc..b9329e4a 100644
--- a/Xamarin.Forms.Xaml.UnitTests/TestSharedResourceDictionary.xaml.cs
+++ b/Xamarin.Forms.Xaml.UnitTests/TestSharedResourceDictionary.xaml.cs
@@ -1,6 +1,5 @@
 using NUnit.Framework;
-
-using Xamarin.Forms;
+using Xamarin.Forms.Core.UnitTests;
 
 namespace Xamarin.Forms.Xaml.UnitTests
 {
@@ -19,6 +18,23 @@ namespace Xamarin.Forms.Xaml.UnitTests
 		[TestFixture]
 		public class Tests
 		{
+			[SetUp]
+			public void Setup()
+			{
+				Device.PlatformServices = new MockPlatformServices();
+				Application.Current = new MockApplication {
+					Resources = new ResourceDictionary {
+						MergedWith = typeof(MyRD)
+					}
+				};
+			}
+
+			[TearDown]
+			public void TearDown()
+			{
+				Device.PlatformServices = null;
+			}
+
 			[TestCase (false)]
 			[TestCase (true)]
 			public void MergedResourcesAreFound (bool useCompiledXaml)
@@ -43,6 +59,24 @@ namespace Xamarin.Forms.Xaml.UnitTests
 				var layout = new TestSharedResourceDictionary(useCompiledXaml);
 				Assert.AreEqual(Color.Red, layout.implicitLabel.TextColor);
 			}
+
+			class MyRD : ResourceDictionary
+			{
+				public MyRD()
+				{
+					Add("foo", "Foo");
+					Add("bar", "Bar");
+				}
+			}
+
+			[TestCase(false)]
+			[TestCase(true)]
+			public void MergedRDAtAppLevel(bool useCompiledXaml)
+			{
+				var layout = new TestSharedResourceDictionary(useCompiledXaml);
+				Assert.AreEqual("Foo", layout.label3.Text);
+			}
+
 		}
 	}
 }
\ No newline at end of file
diff --git a/Xamarin.Forms.Xaml/MarkupExtensions/StaticResourceExtension.cs b/Xamarin.Forms.Xaml/MarkupExtensions/StaticResourceExtension.cs
index dc80ade2..703fc206 100644
--- a/Xamarin.Forms.Xaml/MarkupExtensions/StaticResourceExtension.cs
+++ b/Xamarin.Forms.Xaml/MarkupExtensions/StaticResourceExtension.cs
@@ -29,14 +29,11 @@ namespace Xamarin.Forms.Xaml
 				var resDict = ve?.Resources ?? p as ResourceDictionary;
 				if (resDict == null)
 					continue;
-				if (resDict.TryGetValue(Key, out resource))
+				if (resDict.TryGetMergedValue(Key, out resource))
 					break;
 			}
-			if (resource == null && Application.Current != null && Application.Current.Resources != null &&
-				Application.Current.Resources.ContainsKey(Key))
-				resource = Application.Current.Resources[Key];
-
-			if (resource == null)
+			if (resource == null && (Application.Current == null || Application.Current.Resources == null ||
+									 !Application.Current.Resources.TryGetMergedValue(Key, out resource)))
 				throw new XamlParseException($"StaticResource not found for key {Key}", xmlLineInfo);
 
 			var bp = valueProvider.TargetProperty as BindableProperty;
@@ -59,4 +56,4 @@ namespace Xamarin.Forms.Xaml
 			return resource;
 		}
 	}
-}
\ No newline at end of file
+}