Merge pull request #19221 from unoplatform/mergify/bp/release/stable/…

…5.6/pr-19187

perf: Resources resolution and layout improvements (backport #19187)

src/Uno.UI.RuntimeTests/Tests/Windows_UI/Given_WeakEventHelper.cs

@@ -6,14 +6,15 @@
 using System.Runtime.CompilerServices;
 using System.Text;
 using System.Threading.Tasks;
+using Microsoft.UI.Xaml.Controls;
 using Uno.Buffers;
 using Windows.Graphics.Capture;
 using Windows.UI.Core;
 
 namespace Uno.UI.RuntimeTests.Tests.Windows_UI;
 
 [TestClass]
-public class Given_WeakEventHelper
+public partial class Given_WeakEventHelper
 {
 	[TestMethod]
 	public void When_Explicit_Dispose()
@@ -96,6 +97,61 @@ void Do()
 		SUT.Invoke(this, null);
 
 		Assert.AreEqual(2, invoked);
+
+		disposable.Dispose();
+		disposable = null;
+
+		GC.Collect(2);
+		GC.WaitForPendingFinalizers();
+
+		SUT.Invoke(this, null);
+
+		Assert.AreEqual(2, invoked);
+	}
+
+	[TestMethod]
+	[RunsOnUIThread]
+	public void When_UIElement_Target_Collected()
+	{
+		WeakEventHelper.WeakEventCollection SUT = new();
+
+		var invoked = 0;
+		IDisposable disposable = null;
+
+		void Do()
+		{
+			Action action = () => invoked++;
+
+			// Wrapping the action and registering the one on the target
+			// allows for the WeakEventHelper to check for collection native
+			// objects on android.
+			MyCollectibleObject target = new(action);
+
+			disposable = WeakEventHelper.RegisterEvent(SUT, target.MyAction, (s, e, a) => (s as Action).Invoke());
+
+			SUT.Invoke(this, null);
+
+			Assert.AreEqual(1, invoked);
+		}
+
+		Do();
+
+		GC.Collect(2);
+		GC.WaitForPendingFinalizers();
+
+		SUT.Invoke(this, null);
+
+		Assert.AreEqual(2, invoked);
+
+		disposable.Dispose();
+		disposable = null;
+
+		GC.Collect(2);
+		GC.WaitForPendingFinalizers();
+
+		SUT.Invoke(this, null);
+
+		Assert.AreEqual(2, invoked);
 	}
 
 	[TestMethod]
@@ -220,6 +276,18 @@ public void When_Empty_Trim_Stops()
 		Assert.AreEqual(1, invoked);
 	}
 
+	private partial class MyCollectibleObject : Grid
+	{
+		private Action _action;
+
+		public MyCollectibleObject(Action action)
+		{
+			_action = action;
+		}
+
+		public void MyAction() => _action.Invoke();
+	}
+
 	private class TestPlatformProvider : WeakEventHelper.ITrimProvider
 	{
 		private object _target;

src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_ResourceDictionary.cs

@@ -246,6 +246,65 @@ public void When_LinkedResDict_ThemeUpdated()
 				ResourceDictionary.SetActiveTheme(theme);
 			}
 		}
+
+		[TestMethod]
+		public void When_Key_Added_Then_NotFound_Cleared()
+		{
+			var resourceDictionary = new ResourceDictionary();
+
+			Assert.IsFalse(resourceDictionary.TryGetValue("Key1", out var res1, shouldCheckSystem: false));
+			resourceDictionary["Key1"] = "Value1";
+			Assert.IsTrue(resourceDictionary.TryGetValue("Key1", out var res2, shouldCheckSystem: false));
+		}
+
+		[TestMethod]
+		public void When_Merged_Dictionary_Added_Then_NotFound_Cleared()
+		{
+			var resourceDictionary = new ResourceDictionary();
+
+			Assert.IsFalse(resourceDictionary.TryGetValue("Key1", out var res1, shouldCheckSystem: false));
+
+			var m1 = new ResourceDictionary();
+			m1["Key1"] = "Value1";
+
+			resourceDictionary.MergedDictionaries.Add(m1);
+
+			Assert.IsTrue(resourceDictionary.TryGetValue("Key1", out var res2, shouldCheckSystem: false));
+		}
+
+		[TestMethod]
+		public void When_Merged_Dictionary_Key_Added_Then_NotFound_Cleared()
+		{
+			var resourceDictionary = new ResourceDictionary();
+
+			Assert.IsFalse(resourceDictionary.TryGetValue("Key1", out var res1, shouldCheckSystem: false));
+
+			var m1 = new ResourceDictionary();
+			resourceDictionary.MergedDictionaries.Add(m1);
+
+			Assert.IsFalse(resourceDictionary.TryGetValue("Key1", out var res2, shouldCheckSystem: false));
+
+			m1["Key1"] = "Value1";
+
+			Assert.IsTrue(resourceDictionary.TryGetValue("Key1", out var res3, shouldCheckSystem: false));
+		}
+
+		[TestMethod]
+		public void When_Theme_Dictionary_Key_Added_Then_NotFound_Cleared()
+		{
+			var resourceDictionary = new ResourceDictionary();
+
+			Assert.IsFalse(resourceDictionary.TryGetValue("Key1", out var res1, shouldCheckSystem: false));
+
+			var m1 = new ResourceDictionary();
+			resourceDictionary.ThemeDictionaries["Light"] = m1;
+
+			Assert.IsFalse(resourceDictionary.TryGetValue("Key1", out var res2, shouldCheckSystem: false));
+
+			m1["Key1"] = "Value1";
+
+			Assert.IsTrue(resourceDictionary.TryGetValue("Key1", out var res3, shouldCheckSystem: false));
+		}
 #endif
 	}
 }

src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_Pivot.cs

@@ -77,9 +77,14 @@ public async Task Check_Binding()
 
 			tbs2.Should().NotBeNull();
 
-			// For some reason, the count is 0 in Windows. So this doesn't currently match Windows.
+#if !__IOS__ && !__ANDROID__
+			// Pivot items are materialized on demand, there should not be any text block in the second item.
+			tbs2.Should().HaveCount(0);
+#else
+			// iOS/Android still materializes the content of the second item, even if it's not visible.
 			tbs2.Should().HaveCount(1);
 			items[1].Content.Should().Be(tbs2.ElementAt(0).Text);
+#endif
 		}
 
 #if !WINAPPSDK // GetTemplateChild is protected in UWP while public in Uno.

src/Uno.UI/Microsoft/UI/Xaml/Controls/Repeater/ItemsRepeater.cs

@@ -624,13 +624,12 @@ void OnLoaded(object sender, RoutedEventArgs args)
 			// Uno specific: If the control was unloaded but is loaded again, reattach Layout and DataSource events
 			if (_layoutSubscriptionsRevoker.Disposable is null && Layout is { } layout)
 			{
-				layout.MeasureInvalidated += InvalidateMeasureForLayout;
-				layout.ArrangeInvalidated += InvalidateArrangeForLayout;
-				_layoutSubscriptionsRevoker.Disposable = Disposable.Create(() =>
-				{
-					layout.MeasureInvalidated -= InvalidateMeasureForLayout;
-					layout.ArrangeInvalidated -= InvalidateArrangeForLayout;
-				});
+				InvalidateMeasure();
+
+				var disposables = new CompositeDisposable();
+				layout.RegisterMeasureInvalidated(InvalidateMeasureForLayout).DisposeWith(disposables);
+				layout.RegisterArrangeInvalidated(InvalidateArrangeForLayout).DisposeWith(disposables);
+				_layoutSubscriptionsRevoker.Disposable = disposables;
 			}
 
 			if (_dataSourceSubscriptionsRevoker.Disposable is null && m_itemsSourceView is not null)
@@ -853,14 +852,14 @@ void OnLayoutChanged(Layout oldValue, Layout newValue)
 
 			if (newValue != null)
 			{
+				_layoutSubscriptionsRevoker.Disposable = null;
+
 				newValue.InitializeForContext(GetLayoutContext());
-				newValue.MeasureInvalidated += InvalidateMeasureForLayout;
-				newValue.ArrangeInvalidated += InvalidateArrangeForLayout;
-				_layoutSubscriptionsRevoker.Disposable = Disposable.Create(() =>
-				{
-					newValue.MeasureInvalidated -= InvalidateMeasureForLayout;
-					newValue.ArrangeInvalidated -= InvalidateArrangeForLayout;
-				});
+
+				var disposables = new CompositeDisposable();
+				newValue.RegisterMeasureInvalidated(InvalidateMeasureForLayout).DisposeWith(disposables);
+				newValue.RegisterArrangeInvalidated(InvalidateArrangeForLayout).DisposeWith(disposables);
+				_layoutSubscriptionsRevoker.Disposable = disposables;
 			}
 
 			bool isVirtualizingLayout = newValue is VirtualizingLayout;

src/Uno.UI/Microsoft/UI/Xaml/Controls/Repeater/Layout.cs

@@ -4,11 +4,40 @@
 using System;
 using Windows.Foundation;
 using Microsoft.UI.Xaml;
+using Windows.UI.Core;
 
 namespace Microsoft/* UWP don't rename */.UI.Xaml.Controls
 {
 	public partial class Layout : DependencyObject
 	{
+		// Begin Uno specific:
+		//
+		// We rely on the GC to manage registrations
+		// but in the case of layouts, for ItemView for instance, actual instances
+		// may be placed directly in dictionaries, such as:
+		// https://github.com/unoplatform/uno/blob/c992ed058d1479cce8e6bca58acbf82cc54ce938/src/Uno.UI/Microsoft/UI/Xaml/Controls/ItemsView/ItemsView.xaml#L12-L16
+		// To avoid memory leaks, it's best to use the two register methods.
+
+		private WeakEventHelper.WeakEventCollection _measureInvalidatedHandlers;
+		private WeakEventHelper.WeakEventCollection _arrangeInvalidatedHandlers;
+
+		internal IDisposable RegisterMeasureInvalidated(TypedEventHandler<Layout, object> handler)
+			=> WeakEventHelper.RegisterEvent(
+				_measureInvalidatedHandlers ??= new(),
+				handler,
+				(h, s, e) =>
+					(h as TypedEventHandler<Layout, object>)?.Invoke((Layout)s, e)
+			);
+		internal IDisposable RegisterArrangeInvalidated(TypedEventHandler<Layout, object> handler)
+			=> WeakEventHelper.RegisterEvent(
+				_arrangeInvalidatedHandlers ??= new(),
+				handler,
+				(h, s, e) =>
+					(h as TypedEventHandler<Layout, object>)?.Invoke((Layout)s, e)
+			);
+
+		// End Uno specific: 
+
 		public event TypedEventHandler<Layout, object> MeasureInvalidated;
 		public event TypedEventHandler<Layout, object> ArrangeInvalidated;
 
@@ -103,9 +132,15 @@ public Size Arrange(LayoutContext context, Size finalSize)
 		}
 
 		protected void InvalidateMeasure()
-			=> MeasureInvalidated?.Invoke(this, null);
+		{
+			_measureInvalidatedHandlers?.Invoke(this, null);
+			MeasureInvalidated?.Invoke(this, null);
+		}
 
 		protected void InvalidateArrange()
-			=> ArrangeInvalidated?.Invoke(this, null);
+		{
+			_arrangeInvalidatedHandlers?.Invoke(this, null);
+			ArrangeInvalidated?.Invoke(this, null);
+		}
 	}
 }

src/Uno.UI/UI/Xaml/Application.cs

@@ -61,6 +61,7 @@ public partial class Application
 		private ApplicationTheme _requestedTheme = ApplicationTheme.Dark;
 		private SpecializedResourceDictionary.ResourceKey _requestedThemeForResources;
 		private bool _isInBackground;
+		private ResourceDictionary _resources = new ResourceDictionary();
 
 		static Application()
 		{
@@ -208,7 +209,15 @@ internal void SetExplicitRequestedTheme(ApplicationTheme? explicitTheme)
 			SetRequestedTheme(theme);
 		}
 
-		public ResourceDictionary Resources { get; set; } = new ResourceDictionary();
+		public ResourceDictionary Resources
+		{
+			get => _resources;
+			set
+			{
+				_resources = value;
+				_resources.InvalidateNotFoundCache(true);
+			}
+		}
 
 #pragma warning disable CS0067 // The event is never used
 		/// <summary>

src/Uno.UI/UI/Xaml/Controls/CommandBar/CommandBar.Partial.cs

@@ -1411,6 +1411,8 @@ private void OnPrimaryCommandsChanged(IObservableVector<ICommandBarElement> send
 				var element = m_tpDynamicPrimaryCommands?[(int)changeIndex];
 				if (element is { })
 				{
+					element.SetParent(this);
+
 					element.IsCompact = shouldBeCompact;
 					PropagateDefaultLabelPositionToElement(element);
 				}
@@ -1425,6 +1427,8 @@ private void OnPrimaryCommandsChanged(IObservableVector<ICommandBarElement> send
 					var element = m_tpDynamicPrimaryCommands?[i];
 					if (element is { })
 					{
+						element.SetParent(null);
+
 						PropagateDefaultLabelPositionToElement(element);
 					}
 				}
@@ -1453,6 +1457,8 @@ private void OnSecondaryCommandsChanged(IObservableVector<ICommandBarElement> se
 
 				if (element is { })
 				{
+					element.SetParent(this);
+
 					PropagateDefaultLabelPositionToElement(element);
 					SetOverflowStyleAndInputModeOnSecondaryCommand((int)changeIndex, true);
 					PropagateDefaultLabelPositionToElement(element);
@@ -1468,6 +1474,8 @@ private void OnSecondaryCommandsChanged(IObservableVector<ICommandBarElement> se
 
 					if (element is { })
 					{
+						element.SetParent(null);
+
 						SetOverflowStyleAndInputModeOnSecondaryCommand(i, true);
 						PropagateDefaultLabelPositionToElement(element);
 					}

src/Uno.UI/UI/Xaml/DependencyObjectStore.cs

@@ -1632,7 +1632,7 @@ internal IEnumerable<ResourceDictionary> GetResourceDictionaries(
 			{
 				if (candidate is FrameworkElement fe)
 				{
-					if (fe.Resources is { IsEmpty: false }) // It's legal (if pointless) on UWP to set Resources to null from user code, so check
+					if (fe.TryGetResources() is { IsEmpty: false }) // It's legal (if pointless) on UWP to set Resources to null from user code, so check
 					{
 						yield return fe.Resources;
 					}

src/Uno.UI/UI/Xaml/FrameworkElement.Layout.crossruntime.cs

@@ -25,6 +25,7 @@ public partial class FrameworkElement
 		private readonly static IEventProvider _trace = Tracing.Get(FrameworkElement.TraceProvider.Id);
 
 		private bool m_firedLoadingEvent;
+		private bool m_requiresResourcesUpdate = true;
 
 		private const double SIZE_EPSILON = 0.05d;
 		private readonly Size MaxSize = new Size(double.PositiveInfinity, double.PositiveInfinity);
@@ -285,8 +286,9 @@ private void InnerMeasureCore(Size availableSize)
 				InvokeApplyTemplate(out _);
 
 				// TODO: BEGIN Uno specific
-				if (this is Control thisAsControl)
+				if (m_requiresResourcesUpdate && this is Control thisAsControl)
 				{
+					m_requiresResourcesUpdate = false;
 					// Update bindings to ensure resources defined
 					// in visual parents get applied.
 					this.UpdateResourceBindings();
@@ -991,6 +993,10 @@ internal override void EnterImpl(EnterParams @params, int depth)
 		{
 			var core = this.GetContext();
 
+			// ---------- Uno-specific BEGIN ----------
+			m_requiresResourcesUpdate = true;
+			// ---------- Uno-specific END ----------
+
 			//if (@params.IsLive && @params.CheckForResourceOverrides == false)
 			//{
 			//    var resources = GetResourcesNoCreate();
@@ -1066,7 +1072,7 @@ internal override void LeaveImpl(LeaveParams @params)
 			// of properties that are marked with MetaDataPropertyInfoFlags::IsSparse and MetaDataPropertyInfoFlags::IsVisualTreeProperty
 			// are entered as well.
 			// The property we currently know it has an effect is Resources
-			if (Resources is not null)
+			if (TryGetResources() is not null)
 			{
 				// Using ValuesInternal to avoid Enumerator boxing
 				foreach (var resource in Resources.ValuesInternal)