fix: Invalidate cached theme dictionary when theme dictionaries are a…

…dded/removed
unoplatform · Sep 22, 2021 · 57068a3 · 57068a3
1 parent ecbbe90
commit 57068a3
Show file tree

Hide file tree

Showing 2 changed files with 149 additions and 7 deletions.
diff --git a/src/Uno.UI.Tests/Windows_UI_Xaml/Given_ResourceDictionary.cs b/src/Uno.UI.Tests/Windows_UI_Xaml/Given_ResourceDictionary.cs
@@ -795,5 +795,84 @@ public void When_Custom_Resource_Dictionary_With_Custom_Property_in_Custom_Contr
 			Assert.IsTrue(resources.ContainsKey("TestKey"));
 			Assert.AreEqual(resources["TestKey"], "Test123");
 		}
+
+		[TestMethod]
+		public void When_Theme_Dictionary_Is_Cached_Then_Add_And_Clear_Theme()
+		{
+			var dictionary = new ResourceDictionary();
+			dictionary.TryGetValue("TestKey", out _); // This causes _activeThemeDictionary to be cached.
+
+			var lightTheme = new ResourceDictionary();
+			lightTheme.Add("TestKey", "TestValue");
+			dictionary.ThemeDictionaries.Add("Light", lightTheme); // Cached value is no longer valid due to adding theme.
+
+			// Make sure the cache is updated.
+			Assert.IsTrue(dictionary.TryGetValue("TestKey", out var testValue));
+			Assert.AreEqual("TestValue", testValue);
+
+			dictionary.ThemeDictionaries.Clear(); // Cached value is no longer valid due to clearing themes.
+
+			Assert.IsFalse(dictionary.TryGetValue("TestKey", out _));
+		}
+
+		[TestMethod]
+		public void When_Theme_Dictionary_Is_Cached_Then_Add_And_Remove_Theme()
+		{
+			var dictionary = new ResourceDictionary();
+			dictionary.TryGetValue("TestKey", out _); // This causes _activeThemeDictionary to be cached.
+
+			var lightTheme = new ResourceDictionary();
+			lightTheme.Add("TestKey", "TestValue");
+			dictionary.ThemeDictionaries.Add("Light", lightTheme); // Cached value is no longer valid due to adding theme.
+
+			// Make sure the cache is updated.
+			Assert.IsTrue(dictionary.TryGetValue("TestKey", out var testValue));
+			Assert.AreEqual("TestValue", testValue);
+
+			Assert.IsTrue(dictionary.ThemeDictionaries.Remove("Light")); // Cached value is no longer valid due to removing theme.
+
+			Assert.IsFalse(dictionary.TryGetValue("TestKey", out _));
+		}
+
+
+		[TestMethod]
+		public void When_Default_Theme_Dictionary_Is_Cached()
+		{
+			var defaultDictionary = new ResourceDictionary();
+			defaultDictionary.Add("TestKey", "TestValueFromDefaultDictionary");
+
+			var dictionary = new ResourceDictionary();
+			dictionary.ThemeDictionaries.Add("Default", defaultDictionary);
+			Assert.IsTrue(dictionary.TryGetValue("TestKey", out var testValue));
+			Assert.AreEqual("TestValueFromDefaultDictionary", testValue);
+
+			var lightDictionary = new ResourceDictionary();
+			lightDictionary.Add("TestKey", "TestValueFromLightDictionary");
+			dictionary.ThemeDictionaries.Add("Light", lightDictionary);
+			Assert.IsTrue(dictionary.TryGetValue("TestKey", out testValue));
+			Assert.AreEqual("TestValueFromLightDictionary", testValue);
+		}
+
+		[TestMethod]
+		public void When_Default_Theme_Dictionary_Should_Be_Used_After_Removing_Active_Theme()
+		{
+			var lightDictionary = new ResourceDictionary();
+			lightDictionary.Add("TestKey", "TestValueFromLightDictionary");
+
+			var dictionary = new ResourceDictionary();
+			dictionary.ThemeDictionaries.Add("Light", lightDictionary);
+			Assert.IsTrue(dictionary.TryGetValue("TestKey", out var testValue));
+			Assert.AreEqual("TestValueFromLightDictionary", testValue);
+
+			var defaultDictionary = new ResourceDictionary();
+			defaultDictionary.Add("TestKey", "TestValueFromDefaultDictionary");
+			dictionary.ThemeDictionaries.Add("Default", defaultDictionary);
+			dictionary.ThemeDictionaries.Remove("Light");
+			Assert.IsTrue(dictionary.TryGetValue("TestKey", out testValue));
+			Assert.AreEqual("TestValueFromDefaultDictionary", testValue);
+
+			dictionary.ThemeDictionaries.Remove("Default");
+			Assert.IsFalse(dictionary.TryGetValue("TestKey", out _));
+		}
 	}
 }
diff --git a/src/Uno.UI/UI/Xaml/ResourceDictionary.cs b/src/Uno.UI/UI/Xaml/ResourceDictionary.cs
@@ -21,6 +21,30 @@ public partial class ResourceDictionary : DependencyObject, IDependencyObjectPar
 		private readonly List<ResourceDictionary> _mergedDictionaries = new List<ResourceDictionary>();
 		private ResourceDictionary _themeDictionaries;
 
+		/// <summary>
+		/// This event is fired when a key that has value of type <see cref="ResourceDictionary"/> is added or changed in the current <see cref="ResourceDictionary" />
+		/// </summary>
+		private event EventHandler<ResourceDictionaryChangedEventArgs> ResourceDictionaryValueChange;
+
+		private class ResourceDictionaryChangedEventArgs
+		{
+			public ResourceDictionaryChangedEventArgs(object changedKey, ResourceDictionary newValue)
+			{
+				ChangedKey = changedKey;
+				NewValue = newValue;
+			}
+
+			/// <summary>
+			/// The key that was added or removed. This can be null if the dictionary is cleared, meaning all keys are removed.
+			/// </summary>
+			public object ChangedKey { get; }
+
+			/// <summary>
+			/// The <see cref="ResourceDictionary" /> that was added. This can be null if the theme was removed.
+			/// </summary>
+			public ResourceDictionary NewValue { get; }
+		}
+
 		/// <summary>
 		/// If true, there may be lazily-set values in the dictionary that need to be initialized.
 		/// </summary>
@@ -48,7 +72,25 @@ public Uri Source
 		}
 
 		public IList<ResourceDictionary> MergedDictionaries => _mergedDictionaries;
-		public IDictionary<object, object> ThemeDictionaries => _themeDictionaries ??= new ResourceDictionary();
+		public IDictionary<object, object> ThemeDictionaries => GetOrCreateThemeDictionaries();
+
+		private ResourceDictionary GetOrCreateThemeDictionaries()
+		{
+			if (_themeDictionaries is null)
+			{
+				_themeDictionaries = new ResourceDictionary();
+				_themeDictionaries.ResourceDictionaryValueChange += (sender, e) =>
+				{
+					// Invalidate the cache whenever a theme dictionary is added/removed.
+					// This is safest and avoids handling edge cases.
+					// Note that adding or removing theme dictionary isn't very common,
+					// so invalidating the cache shouldn't be a performance issue.
+					_activeTheme = ResourceKey.Empty;
+				};
+			}
+
+			return _themeDictionaries;
+		}
 
 		/// <summary>
 		/// Is this a ResourceDictionary created from system resources, ie within the Uno.UI assembly?
@@ -96,11 +138,25 @@ public bool Insert(object key, object value)
 			}
 		}
 
-		public bool Remove(object key) => _values.Remove(new ResourceKey(key));
+		public bool Remove(object key)
+		{
+			var keyToRemove = new ResourceKey(key);
+			if (_values.Remove(keyToRemove))
+			{
+				ResourceDictionaryValueChange?.Invoke(this, new ResourceDictionaryChangedEventArgs(keyToRemove, null));
+				return true;
+			}
+
+			return false;
+		}
 
-		public bool Remove(KeyValuePair<object, object> key) => _values.Remove(new ResourceKey(key.Key));
+		public bool Remove(KeyValuePair<object, object> key) => Remove(key.Key);
 
-		public void Clear() => _values.Clear();
+		public void Clear()
+		{
+			_values.Clear();
+			ResourceDictionaryValueChange?.Invoke(this, new ResourceDictionaryChangedEventArgs(null, null));
+		}
 
 		public void Add(object key, object value) => Set(new ResourceKey(key), value, throwIfPresent: true);
 
@@ -189,7 +245,7 @@ public object this[object key]
 			}
 			set
 			{
-				if(!(key is null))
+				if (!(key is null))
 				{
 					Set(new ResourceKey(key), value, throwIfPresent: false);
 				}
@@ -216,6 +272,10 @@ private void Set(in ResourceKey resourceKey, object value, bool throwIfPresent)
 			else
 			{
 				_values[resourceKey] = value;
+				if (value is ResourceDictionary addedOrChangedResourceDictionary)
+				{
+					ResourceDictionaryValueChange?.Invoke(this, new ResourceDictionaryChangedEventArgs(resourceKey, addedOrChangedResourceDictionary));
+				}
 			}
 		}
 
@@ -241,6 +301,10 @@ private void TryMaterializeLazy(in ResourceKey key, ref object value)
 				{
 					value = newValue;
 					_values[key] = newValue; // If Initializer threw an exception this will push null, to avoid running buggy initialization again and again (and avoid surfacing initializer to consumer code)
+					if (newValue is ResourceDictionary addedOrChangedResourceDictionary)
+					{
+						ResourceDictionaryValueChange?.Invoke(this, new ResourceDictionaryChangedEventArgs(key, addedOrChangedResourceDictionary));
+					}
 					ResourceResolver.PopScope();
 				}
 			}
@@ -381,8 +445,7 @@ private void CopyFrom(ResourceDictionary source)
 			_mergedDictionaries.AddRange(source._mergedDictionaries);
 			if (source._themeDictionaries != null)
 			{
-				_themeDictionaries ??= new ResourceDictionary();
-				_themeDictionaries.CopyFrom(source._themeDictionaries);
+				GetOrCreateThemeDictionaries().CopyFrom(source._themeDictionaries);
 			}
 		}