From 446ff77ea3c9932fa530cc8afbbb8b86c44370a8 Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Thu, 9 Jan 2025 21:35:34 -0500 Subject: [PATCH 01/17] fix(resources): Don't reevaluate all resources on all measure (cherry picked from commit 27b8530e2da5d8d4b91b8df88c43384718bc4eda) --- .../UI/Xaml/FrameworkElement.Layout.crossruntime.cs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/Uno.UI/UI/Xaml/FrameworkElement.Layout.crossruntime.cs b/src/Uno.UI/UI/Xaml/FrameworkElement.Layout.crossruntime.cs index 14fb83ac82e8..cfa7f832ecbd 100644 --- a/src/Uno.UI/UI/Xaml/FrameworkElement.Layout.crossruntime.cs +++ b/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); @@ -282,11 +283,12 @@ private void InnerMeasureCore(Size availableSize) //if (!bInLayoutTransition) { // Templates should be applied here. - InvokeApplyTemplate(out _); + InvokeApplyTemplate(out var addedVisual); // 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) From 31bc36bb9c657cf82313919cc0df9ec9d912fe34 Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Thu, 9 Jan 2025 21:36:03 -0500 Subject: [PATCH 02/17] fix(resources): Make the `FrameworkElement.Resources` property lazy initialized (cherry picked from commit 3f998c05847d8cfad5da318f5c0b2d1658c20cc0) --- src/Uno.UI/UI/Xaml/DependencyObjectStore.cs | 2 +- src/Uno.UI/UI/Xaml/FrameworkElement.cs | 15 ++++++++++++--- src/Uno.UI/UI/Xaml/ResourceResolver.cs | 14 +++++++++++--- src/Uno.UI/UI/Xaml/UIElement.mux.cs | 2 +- 4 files changed, 25 insertions(+), 8 deletions(-) diff --git a/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs b/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs index 2b32ae298291..52b6191309f5 100644 --- a/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs +++ b/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs @@ -1618,7 +1618,7 @@ internal IEnumerable 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; } diff --git a/src/Uno.UI/UI/Xaml/FrameworkElement.cs b/src/Uno.UI/UI/Xaml/FrameworkElement.cs index 1f6ebb6a0918..fc9a019a4024 100644 --- a/src/Uno.UI/UI/Xaml/FrameworkElement.cs +++ b/src/Uno.UI/UI/Xaml/FrameworkElement.cs @@ -73,6 +73,8 @@ public static class TraceProvider private bool _defaultStyleApplied; + private ResourceDictionary _resources; + private static readonly Uri DefaultBaseUri = new Uri("ms-appx://local"); private string _baseUriFromParser; @@ -249,7 +251,6 @@ partial void Initialize() #if !UNO_REFERENCE_API _layouter = new FrameworkElementLayouter(this, MeasureOverride, ArrangeOverride); #endif - Resources = new Microsoft.UI.Xaml.ResourceDictionary(); IFrameworkElementHelper.Initialize(this); } @@ -260,9 +261,17 @@ partial void Initialize() #endif Microsoft.UI.Xaml.ResourceDictionary Resources { - get; set; + get => _resources ??= new ResourceDictionary(); + set => _resources = value; } + /// + /// Tries getting the ResourceDictionary without initializing it. + /// + /// A ResourceDictionary instance or null + internal Microsoft.UI.Xaml.ResourceDictionary TryGetResources() + => _resources; + /// /// Gets the parent of this FrameworkElement in the object tree. /// @@ -956,7 +965,7 @@ async void ApplyPhase() /// internal virtual void UpdateThemeBindings(ResourceUpdateReason updateReason) { - Resources?.UpdateThemeBindings(updateReason); + TryGetResources()?.UpdateThemeBindings(updateReason); (this as IDependencyObjectStoreProvider).Store.UpdateResourceBindings(updateReason); if (updateReason == ResourceUpdateReason.ThemeResource) diff --git a/src/Uno.UI/UI/Xaml/ResourceResolver.cs b/src/Uno.UI/UI/Xaml/ResourceResolver.cs index 93ee027235ba..c7bce4501bd0 100644 --- a/src/Uno.UI/UI/Xaml/ResourceResolver.cs +++ b/src/Uno.UI/UI/Xaml/ResourceResolver.cs @@ -390,11 +390,19 @@ internal static bool TryStaticRetrieval(in SpecializedResourceDictionary.Resourc var source = sourcesEnumerator.Current; - var dictionary = (source.Target as FrameworkElement)?.Resources + var dictionary = (source.Target as FrameworkElement)?.TryGetResources() ?? source.Target as ResourceDictionary; - if (dictionary != null && dictionary.TryGetValue(resourceKey, out value, shouldCheckSystem: false)) + + if (dictionary != null) { - return true; + if (dictionary.TryGetValue(resourceKey, out value, shouldCheckSystem: false)) + { + return true; + } + } + else + { + } } diff --git a/src/Uno.UI/UI/Xaml/UIElement.mux.cs b/src/Uno.UI/UI/Xaml/UIElement.mux.cs index 70347faa3721..0f96185d171a 100644 --- a/src/Uno.UI/UI/Xaml/UIElement.mux.cs +++ b/src/Uno.UI/UI/Xaml/UIElement.mux.cs @@ -1072,7 +1072,7 @@ private void DependencyObject_EnterImpl(EnterParams @params) // are entered as well. // The property we currently know it has an effect is Resources // In WinUI, it happens in CDependencyObject::EnterImpl (the call to EnterSparseProperties) - if (this is FrameworkElement { Resources: { } resources }) + if (this is FrameworkElement fe && fe.TryGetResources() is { } resources) { // Using ValuesInternal to avoid Enumerator boxing foreach (var resource in resources.ValuesInternal) From 71182e2eaebe3065bf88788f34faecdfcd15143c Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Thu, 9 Jan 2025 22:30:22 -0500 Subject: [PATCH 03/17] Revert "perf: Misc perf improvement to ResourceDictionary" This reverts commit c3a2a3dea1a95b270dc4ea9b100d9a89a86dd51b. (cherry picked from commit 42c840c91664887ea56688f1f1fa46af3e94d1a3) --- src/Uno.UI/UI/Xaml/ResourceDictionary.cs | 25 +++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/Uno.UI/UI/Xaml/ResourceDictionary.cs b/src/Uno.UI/UI/Xaml/ResourceDictionary.cs index 7ad64a0e912a..81951b7fdba5 100644 --- a/src/Uno.UI/UI/Xaml/ResourceDictionary.cs +++ b/src/Uno.UI/UI/Xaml/ResourceDictionary.cs @@ -201,7 +201,11 @@ internal bool TryGetValue(Type resourceKey, out object value, bool shouldCheckSy => TryGetValue(new ResourceKey(resourceKey), out value, shouldCheckSystem); [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal bool TryGetValue(in ResourceKey resourceKey, out object value, bool shouldCheckSystem) + internal bool TryGetValue(in ResourceKey resourceKey, out object value, bool shouldCheckSystem) => + TryGetValue(resourceKey, ResourceKey.Empty, out value, shouldCheckSystem); + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private bool TryGetValue(in ResourceKey resourceKey, ResourceKey activeTheme, out object value, bool shouldCheckSystem) { if (_values.TryGetValue(resourceKey, out value)) { @@ -222,7 +226,12 @@ internal bool TryGetValue(in ResourceKey resourceKey, out object value, bool sho return true; } - if (GetFromTheme(resourceKey, GetActiveThemeDictionary(Themes.Active), out value)) + if (activeTheme.IsEmpty) + { + activeTheme = Themes.Active; + } + + if (GetFromTheme(resourceKey, activeTheme, out value)) { return true; } @@ -425,23 +434,25 @@ private ResourceDictionary GetThemeDictionary(in ResourceKey theme) return null; } - private bool GetFromTheme(in ResourceKey resourceKey, ResourceDictionary activeThemeDictionary, out object value) + private bool GetFromTheme(in ResourceKey resourceKey, in ResourceKey activeTheme, out object value) { - if (activeThemeDictionary != null && activeThemeDictionary.TryGetValue(resourceKey, out value, shouldCheckSystem: false)) + var dict = GetActiveThemeDictionary(activeTheme); + + if (dict != null && dict.TryGetValue(resourceKey, out value, shouldCheckSystem: false)) { return true; } - return GetFromThemeMerged(resourceKey, activeThemeDictionary, out value); + return GetFromThemeMerged(resourceKey, activeTheme, out value); } - private bool GetFromThemeMerged(in ResourceKey resourceKey, ResourceDictionary activeThemeDictionary, out object value) + private bool GetFromThemeMerged(in ResourceKey resourceKey, in ResourceKey activeTheme, out object value) { var count = _mergedDictionaries.Count; for (int i = count - 1; i >= 0; i--) { - if (_mergedDictionaries[i].GetFromTheme(resourceKey, activeThemeDictionary, out value)) + if (_mergedDictionaries[i].GetFromTheme(resourceKey, activeTheme, out value)) { return true; } From 914b026d8716e5101f105dd20dece6fd8ae3d8d5 Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Thu, 9 Jan 2025 23:24:56 -0500 Subject: [PATCH 04/17] perf: Enable ResourceDictionary "not found" caching (cherry picked from commit 795cc0a27ab4c71524b0abd62df61ec25c56977a) --- src/Uno.UI/UI/Xaml/Application.cs | 11 +- src/Uno.UI/UI/Xaml/FrameworkElement.cs | 6 +- src/Uno.UI/UI/Xaml/ResourceDictionary.cs | 116 ++++++++++++++---- .../UI/Xaml/SpecializedResourceDictionary.cs | 16 +++ 4 files changed, 125 insertions(+), 24 deletions(-) diff --git a/src/Uno.UI/UI/Xaml/Application.cs b/src/Uno.UI/UI/Xaml/Application.cs index 4e23b7144cb4..87c5a3bd6b31 100644 --- a/src/Uno.UI/UI/Xaml/Application.cs +++ b/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 /// diff --git a/src/Uno.UI/UI/Xaml/FrameworkElement.cs b/src/Uno.UI/UI/Xaml/FrameworkElement.cs index fc9a019a4024..cf3c367a3835 100644 --- a/src/Uno.UI/UI/Xaml/FrameworkElement.cs +++ b/src/Uno.UI/UI/Xaml/FrameworkElement.cs @@ -262,7 +262,11 @@ partial void Initialize() Microsoft.UI.Xaml.ResourceDictionary Resources { get => _resources ??= new ResourceDictionary(); - set => _resources = value; + set + { + _resources = value; + _resources.InvalidateNotFoundCache(true); + } } /// diff --git a/src/Uno.UI/UI/Xaml/ResourceDictionary.cs b/src/Uno.UI/UI/Xaml/ResourceDictionary.cs index 81951b7fdba5..ffa4cbc84333 100644 --- a/src/Uno.UI/UI/Xaml/ResourceDictionary.cs +++ b/src/Uno.UI/UI/Xaml/ResourceDictionary.cs @@ -14,15 +14,18 @@ using System.Runtime.CompilerServices; using Microsoft.UI.Xaml.Data; using Uno.UI.DataBinding; +using System.Collections.ObjectModel; namespace Microsoft.UI.Xaml { public partial class ResourceDictionary : DependencyObject, IDependencyObjectParse, IDictionary { private readonly SpecializedResourceDictionary _values = new SpecializedResourceDictionary(); - private readonly List _mergedDictionaries = new List(); + private readonly ObservableCollection _mergedDictionaries = new(); private ResourceDictionary _themeDictionaries; + private ResourceDictionary _parent; private ManagedWeakReference _sourceDictionary; + private HashSet _keyNotFoundCache; /// /// This event is fired when a key that has value of type is added or changed in the current @@ -36,6 +39,25 @@ public partial class ResourceDictionary : DependencyObject, IDependencyObjectPar public ResourceDictionary() { + _mergedDictionaries.CollectionChanged += (s, e) => + { + if (e.OldItems != null) + { + foreach (ResourceDictionary oldDict in e.OldItems) + { + oldDict._parent = null; + } + } + if (e.NewItems != null) + { + foreach (ResourceDictionary newDict in e.NewItems) + { + newDict._parent = this; + } + + InvalidateNotFoundCache(true); + } + }; } private Uri _source; @@ -70,7 +92,7 @@ private ResourceDictionary GetOrCreateThemeDictionaries() { if (_themeDictionaries is null) { - _themeDictionaries = new ResourceDictionary(); + _themeDictionaries = new ResourceDictionary() { _parent = this }; _themeDictionaries.ResourceDictionaryValueChange += (sender, e) => { // Invalidate the cache whenever a theme dictionary is added/removed. @@ -89,6 +111,10 @@ private ResourceDictionary GetOrCreateThemeDictionaries() /// internal bool IsSystemDictionary { get; set; } + + private HashSet KeyNotFoundCache + => _keyNotFoundCache ??= new(SpecializedResourceDictionary.ResourceKeyComparer.Default); + internal object Lookup(object key) { if (!TryGetValue(key, out var value)) @@ -207,7 +233,21 @@ internal bool TryGetValue(in ResourceKey resourceKey, out object value, bool sho [MethodImpl(MethodImplOptions.AggressiveInlining)] private bool TryGetValue(in ResourceKey resourceKey, ResourceKey activeTheme, out object value, bool shouldCheckSystem) { - if (_values.TryGetValue(resourceKey, out value)) + bool useKeysNotFoundCache = resourceKey.ShouldFilter; + var modifiedKey = resourceKey; + + if (useKeysNotFoundCache) + { + if (!shouldCheckSystem && KeyNotFoundCache.Contains(resourceKey)) + { + value = null; + return false; + } + + modifiedKey = modifiedKey with { ShouldFilter = false }; + } + + if (_values.TryGetValue(modifiedKey, out value)) { if (value is SpecialValue) { @@ -221,7 +261,7 @@ private bool TryGetValue(in ResourceKey resourceKey, ResourceKey activeTheme, ou return true; } - if (GetFromMerged(resourceKey, out value)) + if (GetFromMerged(modifiedKey, out value)) { return true; } @@ -231,14 +271,19 @@ private bool TryGetValue(in ResourceKey resourceKey, ResourceKey activeTheme, ou activeTheme = Themes.Active; } - if (GetFromTheme(resourceKey, activeTheme, out value)) + if (GetFromTheme(modifiedKey, activeTheme, out value)) { return true; } if (shouldCheckSystem && !IsSystemDictionary) // We don't fall back on system resources from within a system-defined dictionary, to avoid an infinite recurse { - return ResourceResolver.TrySystemResourceRetrieval(resourceKey, out value); + return ResourceResolver.TrySystemResourceRetrieval(modifiedKey, out value); + } + + if (useKeysNotFoundCache && !shouldCheckSystem) + { + KeyNotFoundCache.Add(resourceKey); } return false; @@ -288,6 +333,8 @@ private void Set(in ResourceKey resourceKey, object value, bool throwIfPresent) ResourceDictionaryValueChange?.Invoke(this, EventArgs.Empty); } } + + InvalidateNotFoundCache(true, resourceKey); } /// @@ -416,6 +463,7 @@ private ResourceDictionary GetActiveThemeDictionary(in ResourceKey activeTheme) { if (!activeTheme.Equals(_activeTheme)) { + InvalidateNotFoundCache(false); _activeTheme = activeTheme; _activeThemeDictionary = GetThemeDictionary(activeTheme) ?? GetThemeDictionary(Themes.Default); } @@ -443,23 +491,7 @@ private bool GetFromTheme(in ResourceKey resourceKey, in ResourceKey activeTheme return true; } - return GetFromThemeMerged(resourceKey, activeTheme, out value); - } - - private bool GetFromThemeMerged(in ResourceKey resourceKey, in ResourceKey activeTheme, out object value) - { - var count = _mergedDictionaries.Count; - - for (int i = count - 1; i >= 0; i--) - { - if (_mergedDictionaries[i].GetFromTheme(resourceKey, activeTheme, out value)) - { - return true; - } - } - value = null; - return false; } @@ -700,6 +732,46 @@ public StaticResourceAliasRedirect(string resourceKey, XamlParseContext parseCon internal static void SetActiveTheme(SpecializedResourceDictionary.ResourceKey key) => Themes.Active = key; + internal void InvalidateNotFoundCache(bool propagate) + { + if (propagate) + { + // Traverse dictionary sub-tree iteratively as it has less overhead. + var current = this; + + while (current is not null) + { + current._keyNotFoundCache?.Clear(); + + current = current._parent; + } + } + else + { + _keyNotFoundCache?.Clear(); + } + } + + internal void InvalidateNotFoundCache(bool propagate, in ResourceKey key) + { + if (propagate) + { + // Traverse dictionary sub-tree iteratively as it has less overhead. + var current = this; + + while (current is not null) + { + current._keyNotFoundCache?.Remove(key); + current = current._parent; + } + } + else + { + _keyNotFoundCache?.Remove(key); + } + } + + private static class Themes { public static SpecializedResourceDictionary.ResourceKey Light { get; } = "Light"; diff --git a/src/Uno.UI/UI/Xaml/SpecializedResourceDictionary.cs b/src/Uno.UI/UI/Xaml/SpecializedResourceDictionary.cs index a2358cf5d56f..286769201769 100644 --- a/src/Uno.UI/UI/Xaml/SpecializedResourceDictionary.cs +++ b/src/Uno.UI/UI/Xaml/SpecializedResourceDictionary.cs @@ -31,6 +31,8 @@ public readonly struct ResourceKey public readonly Type TypeKey; public readonly uint HashCode; + public bool ShouldFilter { get; init; } = true; + public static ResourceKey Empty { get; } = new ResourceKey(false); public bool IsEmpty => Key == null; @@ -110,6 +112,20 @@ public static implicit operator ResourceKey(Type key) => new ResourceKey(key); } + internal class ResourceKeyComparer : IEqualityComparer + { + public bool Equals(ResourceKey x, ResourceKey y) + { + return x.Equals(y); + } + public int GetHashCode(ResourceKey obj) + { + return (int)obj.HashCode; + } + + public static ResourceKeyComparer Default { get; } = new(); + } + private int[] _buckets; private Entry[] _entries; #if TARGET_64BIT From 652ad21c455db0c8df8992ac270b0c8f66845900 Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Fri, 10 Jan 2025 09:07:03 -0500 Subject: [PATCH 05/17] fix: Don't invoke ApplyTemplate for collapsed elements (cherry picked from commit 2fa51b2c746a519fd9904e8e5c776953b057f08a) --- .../UI/Xaml/UIElement.Layout.crossruntime.cs | 24 ------------------- 1 file changed, 24 deletions(-) diff --git a/src/Uno.UI/UI/Xaml/UIElement.Layout.crossruntime.cs b/src/Uno.UI/UI/Xaml/UIElement.Layout.crossruntime.cs index 99d520a90841..9205dbdf4455 100644 --- a/src/Uno.UI/UI/Xaml/UIElement.Layout.crossruntime.cs +++ b/src/Uno.UI/UI/Xaml/UIElement.Layout.crossruntime.cs @@ -245,7 +245,6 @@ private void DoMeasure(Size availableSize) if (this.Visibility == Visibility.Collapsed) { m_desiredSize = default; - RecursivelyApplyTemplateWorkaround(); return; } @@ -330,29 +329,6 @@ internal bool ShouldApplyLayoutClipAsAncestorClip() //&& !GetIsScrollViewerHeader(); // Special-case: ScrollViewer Headers, which can zoom, must scale the LayoutClip too } - private void RecursivelyApplyTemplateWorkaround() - { - // Uno workaround. The template should NOT be applied here. - // But, without this workaround, VerifyVisibilityChangeUpdatesCommandBarVisualState test will fail. - // The real root cause for the test failure is that FindParentCommandBarForElement will - // return null, that is because Uno doesn't yet properly have a "logical parent" concept. - // We eagerly apply the template so that FindParentCommandBarForElement will - // find the command bar through TemplatedParent - if (this is Control thisAsControl) - { - thisAsControl.ApplyTemplate(); - - // Update bindings to ensure resources defined - // in visual parents get applied. - this.UpdateResourceBindings(); - } - - foreach (var child in _children) - { - child.RecursivelyApplyTemplateWorkaround(); - } - } - public void Arrange(Rect finalRect) { From 0a7199daf74662c4e853edf796bcc33c550f8201 Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Fri, 10 Jan 2025 16:32:09 -0500 Subject: [PATCH 06/17] fix: Track the parent command bar for appbar buttons (cherry picked from commit 821c2372fa2ced529de6d81a9ca47ca4bdfadaec) --- .../Tests/Windows_UI_Xaml_Controls/Given_Pivot.cs | 7 ++++++- .../UI/Xaml/Controls/CommandBar/CommandBar.Partial.cs | 8 ++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_Pivot.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_Pivot.cs index 32fc7b0a5aed..dc1df89c351c 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_Pivot.cs +++ b/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 no 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. diff --git a/src/Uno.UI/UI/Xaml/Controls/CommandBar/CommandBar.Partial.cs b/src/Uno.UI/UI/Xaml/Controls/CommandBar/CommandBar.Partial.cs index 0a4de192653e..229b67ca0e61 100644 --- a/src/Uno.UI/UI/Xaml/Controls/CommandBar/CommandBar.Partial.cs +++ b/src/Uno.UI/UI/Xaml/Controls/CommandBar/CommandBar.Partial.cs @@ -1411,6 +1411,8 @@ private void OnPrimaryCommandsChanged(IObservableVector 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 send var element = m_tpDynamicPrimaryCommands?[i]; if (element is { }) { + element.SetParent(null); + PropagateDefaultLabelPositionToElement(element); } } @@ -1453,6 +1457,8 @@ private void OnSecondaryCommandsChanged(IObservableVector se if (element is { }) { + element.SetParent(this); + PropagateDefaultLabelPositionToElement(element); SetOverflowStyleAndInputModeOnSecondaryCommand((int)changeIndex, true); PropagateDefaultLabelPositionToElement(element); @@ -1468,6 +1474,8 @@ private void OnSecondaryCommandsChanged(IObservableVector se if (element is { }) { + element.SetParent(null); + SetOverflowStyleAndInputModeOnSecondaryCommand(i, true); PropagateDefaultLabelPositionToElement(element); } From 81a5069f5f2cd72544e560e20225fe97bd338cdf Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Fri, 10 Jan 2025 19:25:20 -0500 Subject: [PATCH 07/17] fix: Avoid handler leak on layout instance (cherry picked from commit 475079104c3e862735dcc88b1eace1e83344d338) --- .../Xaml/Controls/Repeater/ItemsRepeater.cs | 31 ++++++++------- .../UI/Xaml/Controls/Repeater/Layout.cs | 39 ++++++++++++++++++- 2 files changed, 54 insertions(+), 16 deletions(-) diff --git a/src/Uno.UI/Microsoft/UI/Xaml/Controls/Repeater/ItemsRepeater.cs b/src/Uno.UI/Microsoft/UI/Xaml/Controls/Repeater/ItemsRepeater.cs index f5bbb6c91400..5706262ac84a 100644 --- a/src/Uno.UI/Microsoft/UI/Xaml/Controls/Repeater/ItemsRepeater.cs +++ b/src/Uno.UI/Microsoft/UI/Xaml/Controls/Repeater/ItemsRepeater.cs @@ -624,17 +624,20 @@ 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; - }); + _layoutSubscriptionsRevoker.Disposable = null; + + 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) { + _dataSourceSubscriptionsRevoker.Disposable = null; + m_itemsSourceView.CollectionChanged += OnItemsSourceViewChanged; _dataSourceSubscriptionsRevoker.Disposable = Disposable.Create(() => { @@ -853,14 +856,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; diff --git a/src/Uno.UI/Microsoft/UI/Xaml/Controls/Repeater/Layout.cs b/src/Uno.UI/Microsoft/UI/Xaml/Controls/Repeater/Layout.cs index 3921d5ba2259..ec5469694621 100644 --- a/src/Uno.UI/Microsoft/UI/Xaml/Controls/Repeater/Layout.cs +++ b/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 handler) + => WeakEventHelper.RegisterEvent( + _measureInvalidatedHandlers ??= new(), + handler, + (h, s, e) => + (h as TypedEventHandler)?.Invoke((Layout)s, e) + ); + internal IDisposable RegisterArrangeInvalidated(TypedEventHandler handler) + => WeakEventHelper.RegisterEvent( + _arrangeInvalidatedHandlers ??= new(), + handler, + (h, s, e) => + (h as TypedEventHandler)?.Invoke((Layout)s, e) + ); + + // End Uno specific: + public event TypedEventHandler MeasureInvalidated; public event TypedEventHandler 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); + } } } From b9d4b860328c6bc910a3e90cbd1b06ba6fd8a69a Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Sat, 11 Jan 2025 10:43:12 -0500 Subject: [PATCH 08/17] fix: Don't invoke weak event target if the target is a collected peer reference (cherry picked from commit b117bff3e2f8bd5af288eb594c6b8dbb98eab531) --- .../Tests/Windows_UI/Given_WeakEventHelper.cs | 49 ++++++++++++++++++- src/Uno.UWP/UI/Core/WeakEventHelper.cs | 14 +++++- 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI/Given_WeakEventHelper.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI/Given_WeakEventHelper.cs index 2d33b095c8f7..a703da154bf4 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI/Given_WeakEventHelper.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI/Given_WeakEventHelper.cs @@ -6,6 +6,7 @@ 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; @@ -13,7 +14,7 @@ namespace Uno.UI.RuntimeTests.Tests.Windows_UI; [TestClass] -public class Given_WeakEventHelper +public partial class Given_WeakEventHelper { [TestMethod] public void When_Explicit_Dispose() @@ -98,6 +99,40 @@ void Do() Assert.AreEqual(2, invoked); } + [TestMethod] + 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); + } + [TestMethod] public void When_Many_Targets_Collected() { @@ -220,6 +255,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; diff --git a/src/Uno.UWP/UI/Core/WeakEventHelper.cs b/src/Uno.UWP/UI/Core/WeakEventHelper.cs index 9803c412b2dc..1d3bce5313a0 100644 --- a/src/Uno.UWP/UI/Core/WeakEventHelper.cs +++ b/src/Uno.UWP/UI/Core/WeakEventHelper.cs @@ -203,7 +203,19 @@ internal static IDisposable RegisterEvent(WeakEventCollection list, Delegate han if (weakHandler != null) { - raise(weakHandler, s, e); +#if __ANDROID__ + // If the target is a IJavaPeerable that does not have a valid peer reference, there + // is no need to call the handler. If it's not a IJavaPeerable, call the target. + // This scenario may happen when the object is about to be collected and has already + // collected its native counterpart. We know that the target will likely try to use + // native methods, which will fail if the peer reference is not valid. + var javaPeerable = weakHandler.Target as Java.Interop.IJavaPeerable; + if (javaPeerable?.PeerReference.IsValid ?? true) +#endif + { + + raise(weakHandler, s, e); + } } }; From 574c95188caede3b8cc119cc200199fa8a226311 Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Sat, 11 Jan 2025 20:34:41 -0500 Subject: [PATCH 09/17] chore: Adjust weakevent helper test for ui thread (cherry picked from commit 7e195c2526d9de5e7b4930e370bb9d6577626fa6) --- .../Tests/Windows_UI/Given_WeakEventHelper.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI/Given_WeakEventHelper.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI/Given_WeakEventHelper.cs index a703da154bf4..fc8e991113c6 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI/Given_WeakEventHelper.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI/Given_WeakEventHelper.cs @@ -100,6 +100,7 @@ void Do() } [TestMethod] + [RunsOnUIThread] public void When_UIElement_Target_Collected() { WeakEventHelper.WeakEventCollection SUT = new(); From 1262527cfa66cc0df2f5d88cbcad6b38418bfeb9 Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Sat, 11 Jan 2025 22:03:01 -0500 Subject: [PATCH 10/17] chore: Add tests, ensure modified theme dictionaries invalidate the cache (cherry picked from commit cf7173bf6cb3094c0991e737525ffd39f539d367) --- .../Given_ResourceDictionary.cs | 59 +++++++++++++++++++ src/Uno.UI/UI/Xaml/ResourceDictionary.cs | 12 +++- .../UI/Xaml/SpecializedResourceDictionary.cs | 18 ++++-- 3 files changed, 82 insertions(+), 7 deletions(-) diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_ResourceDictionary.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_ResourceDictionary.cs index 8bc400d9dfeb..e22d1a92403b 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_ResourceDictionary.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_ResourceDictionary.cs @@ -170,6 +170,65 @@ public void When_Key_Overwritten() Assert.AreEqual(newValue, resourceDictionary[key]); } + [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)); + } + [TestMethod] public async Task When_ResourceDictionary_DP() { diff --git a/src/Uno.UI/UI/Xaml/ResourceDictionary.cs b/src/Uno.UI/UI/Xaml/ResourceDictionary.cs index ffa4cbc84333..2014d2d340c9 100644 --- a/src/Uno.UI/UI/Xaml/ResourceDictionary.cs +++ b/src/Uno.UI/UI/Xaml/ResourceDictionary.cs @@ -15,6 +15,7 @@ using Microsoft.UI.Xaml.Data; using Uno.UI.DataBinding; using System.Collections.ObjectModel; +using System.Runtime.InteropServices; namespace Microsoft.UI.Xaml { @@ -327,9 +328,16 @@ private void Set(in ResourceKey resourceKey, object value, bool throwIfPresent) } else { - _values[resourceKey] = value; - if (value is ResourceDictionary) + _values.AddOrUpdate(resourceKey, value, out var previousValue); + + if (previousValue is ResourceDictionary previousDictionary) + { + previousDictionary._parent = null; + } + + if (value is ResourceDictionary newDictionary) { + newDictionary._parent = this; ResourceDictionaryValueChange?.Invoke(this, EventArgs.Empty); } } diff --git a/src/Uno.UI/UI/Xaml/SpecializedResourceDictionary.cs b/src/Uno.UI/UI/Xaml/SpecializedResourceDictionary.cs index 286769201769..9ff54140e6be 100644 --- a/src/Uno.UI/UI/Xaml/SpecializedResourceDictionary.cs +++ b/src/Uno.UI/UI/Xaml/SpecializedResourceDictionary.cs @@ -190,17 +190,22 @@ public object this[in ResourceKey key] } set { - bool modified = TryInsert(key, value, InsertionBehavior.OverwriteExisting); + bool modified = TryInsert(key, value, InsertionBehavior.OverwriteExisting, out _); Debug.Assert(modified); } } public void Add(in ResourceKey key, object value) { - bool modified = TryInsert(key, value, InsertionBehavior.ThrowOnExisting); + bool modified = TryInsert(key, value, InsertionBehavior.ThrowOnExisting, out _); Debug.Assert(modified); // If there was an existing key and the Add failed, an exception will already have been thrown. } + public void AddOrUpdate(in ResourceKey key, object value, out object previousValue) + { + TryInsert(key, value, InsertionBehavior.OverwriteExisting, out previousValue); + } + public void Clear() { int count = _count; @@ -264,7 +269,7 @@ public bool ContainsValue(object value) public Enumerator GetEnumerator() => new Enumerator(this, Enumerator.KeyValuePair); - private ref object FindValue(in ResourceKey key) + internal ref object FindValue(in ResourceKey key) { ref Entry entry = ref Unsafe.NullRef(); @@ -336,7 +341,7 @@ private int Initialize(int capacity) return size; } - private bool TryInsert(in ResourceKey key, object value, InsertionBehavior behavior) + private bool TryInsert(in ResourceKey key, object value, InsertionBehavior behavior, out object previousValue) { if (_buckets == null) { @@ -367,6 +372,7 @@ private bool TryInsert(in ResourceKey key, object value, InsertionBehavior behav { if (behavior == InsertionBehavior.OverwriteExisting) { + previousValue = entries[i].value; entries[i].value = value; return true; } @@ -376,6 +382,7 @@ private bool TryInsert(in ResourceKey key, object value, InsertionBehavior behav throw new InvalidOperationException("AddingDuplicateWithKeyArgumentException(key)"); } + previousValue = null; return false; } @@ -419,6 +426,7 @@ private bool TryInsert(in ResourceKey key, object value, InsertionBehavior behav bucket = index + 1; // Value in _buckets is 1-based _version++; + previousValue = null; return true; } @@ -601,7 +609,7 @@ public bool TryGetValue(in ResourceKey key, out object value) } public bool TryAdd(in ResourceKey key, object value) => - TryInsert(key, value, InsertionBehavior.None); + TryInsert(key, value, InsertionBehavior.None, out _); /// /// Ensures that the dictionary can hold up to 'capacity' entries without any further expansion of its backing storage From c7cf758edae992cc6ea779ac08f464989d947739 Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Sat, 11 Jan 2025 22:03:16 -0500 Subject: [PATCH 11/17] chore: Reduce TryGetValue indirection (cherry picked from commit d50f46c8e0b7a1ee2042306365379eabc3d052d0) --- src/Uno.UI/UI/Xaml/ResourceDictionary.cs | 28 +++--------------------- 1 file changed, 3 insertions(+), 25 deletions(-) diff --git a/src/Uno.UI/UI/Xaml/ResourceDictionary.cs b/src/Uno.UI/UI/Xaml/ResourceDictionary.cs index 2014d2d340c9..18a76c35352e 100644 --- a/src/Uno.UI/UI/Xaml/ResourceDictionary.cs +++ b/src/Uno.UI/UI/Xaml/ResourceDictionary.cs @@ -228,11 +228,7 @@ internal bool TryGetValue(Type resourceKey, out object value, bool shouldCheckSy => TryGetValue(new ResourceKey(resourceKey), out value, shouldCheckSystem); [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal bool TryGetValue(in ResourceKey resourceKey, out object value, bool shouldCheckSystem) => - TryGetValue(resourceKey, ResourceKey.Empty, out value, shouldCheckSystem); - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private bool TryGetValue(in ResourceKey resourceKey, ResourceKey activeTheme, out object value, bool shouldCheckSystem) + internal bool TryGetValue(in ResourceKey resourceKey, out object value, bool shouldCheckSystem) { bool useKeysNotFoundCache = resourceKey.ShouldFilter; var modifiedKey = resourceKey; @@ -267,12 +263,8 @@ private bool TryGetValue(in ResourceKey resourceKey, ResourceKey activeTheme, ou return true; } - if (activeTheme.IsEmpty) - { - activeTheme = Themes.Active; - } - - if (GetFromTheme(modifiedKey, activeTheme, out value)) + if (GetActiveThemeDictionary(Themes.Active) is { } activeThemeDictionary + && activeThemeDictionary.TryGetValue(resourceKey, out value, shouldCheckSystem: false)) { return true; } @@ -490,20 +482,6 @@ private ResourceDictionary GetThemeDictionary(in ResourceKey theme) return null; } - private bool GetFromTheme(in ResourceKey resourceKey, in ResourceKey activeTheme, out object value) - { - var dict = GetActiveThemeDictionary(activeTheme); - - if (dict != null && dict.TryGetValue(resourceKey, out value, shouldCheckSystem: false)) - { - return true; - } - - value = null; - return false; - } - - private bool ContainsKeyTheme(in ResourceKey resourceKey, in ResourceKey activeTheme) { return GetActiveThemeDictionary(activeTheme)?.ContainsKey(resourceKey, shouldCheckSystem: false) ?? ContainsKeyThemeMerged(resourceKey, activeTheme); From ba80b80e718e83fca8c758fb6daa1ea01875dd43 Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Sat, 11 Jan 2025 22:19:09 -0500 Subject: [PATCH 12/17] chore: Move to uno-only tests (cherry picked from commit 7f2589b8c5a9c92580b1aa3fa334d9bd72c5a8ae) --- .../Given_ResourceDictionary.cs | 118 +++++++++--------- 1 file changed, 59 insertions(+), 59 deletions(-) diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_ResourceDictionary.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_ResourceDictionary.cs index e22d1a92403b..5a38cbd3ca6f 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_ResourceDictionary.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_ResourceDictionary.cs @@ -170,65 +170,6 @@ public void When_Key_Overwritten() Assert.AreEqual(newValue, resourceDictionary[key]); } - [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)); - } - [TestMethod] public async Task When_ResourceDictionary_DP() { @@ -305,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 } } From 57798d1c3af6d1a401177e5f3bbeae88c898acd5 Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Mon, 13 Jan 2025 13:07:28 -0500 Subject: [PATCH 13/17] chore: Adjust leak testing (cherry picked from commit 3def740ba8301ca49a04c67ca753e2defa23baaa) --- .../Tests/Windows_UI/Given_WeakEventHelper.cs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI/Given_WeakEventHelper.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI/Given_WeakEventHelper.cs index fc8e991113c6..cbba8d594fec 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI/Given_WeakEventHelper.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI/Given_WeakEventHelper.cs @@ -97,6 +97,16 @@ 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] @@ -132,6 +142,16 @@ 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] From e91f017c626a09b1cb97dadda872d608d232c6d2 Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Mon, 13 Jan 2025 13:09:17 -0500 Subject: [PATCH 14/17] chore: Remove unused statements (cherry picked from commit 4c81f8e775822e2095f643fa94e0cf6270c5dc63) --- .../Microsoft/UI/Xaml/Controls/Repeater/ItemsRepeater.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Uno.UI/Microsoft/UI/Xaml/Controls/Repeater/ItemsRepeater.cs b/src/Uno.UI/Microsoft/UI/Xaml/Controls/Repeater/ItemsRepeater.cs index 5706262ac84a..42c3c2002907 100644 --- a/src/Uno.UI/Microsoft/UI/Xaml/Controls/Repeater/ItemsRepeater.cs +++ b/src/Uno.UI/Microsoft/UI/Xaml/Controls/Repeater/ItemsRepeater.cs @@ -624,8 +624,6 @@ 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) { - _layoutSubscriptionsRevoker.Disposable = null; - InvalidateMeasure(); var disposables = new CompositeDisposable(); @@ -636,8 +634,6 @@ void OnLoaded(object sender, RoutedEventArgs args) if (_dataSourceSubscriptionsRevoker.Disposable is null && m_itemsSourceView is not null) { - _dataSourceSubscriptionsRevoker.Disposable = null; - m_itemsSourceView.CollectionChanged += OnItemsSourceViewChanged; _dataSourceSubscriptionsRevoker.Disposable = Disposable.Create(() => { From 3f8326ef1e881536f48796e094f3198196cb0344 Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Mon, 13 Jan 2025 13:09:26 -0500 Subject: [PATCH 15/17] chore: Fix typo (cherry picked from commit 1715ff5b0c9f3f3c6cc60844d137cbf0e71b2843) --- .../Tests/Windows_UI_Xaml_Controls/Given_Pivot.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_Pivot.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_Pivot.cs index dc1df89c351c..414e3ce20cc8 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_Pivot.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_Pivot.cs @@ -78,7 +78,7 @@ public async Task Check_Binding() tbs2.Should().NotBeNull(); #if !__IOS__ && !__ANDROID__ - // Pivot items are materialized on demand, there should no be any text block in the second item. + // 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. From 88ebe359c6ad454b948788a43b28dc990bb722d2 Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Mon, 13 Jan 2025 14:08:02 -0500 Subject: [PATCH 16/17] chore: Remove unused variable (cherry picked from commit 9a47bb9b23fdd63c7c36e072b041864497af017b) --- src/Uno.UI/UI/Xaml/FrameworkElement.Layout.crossruntime.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Uno.UI/UI/Xaml/FrameworkElement.Layout.crossruntime.cs b/src/Uno.UI/UI/Xaml/FrameworkElement.Layout.crossruntime.cs index cfa7f832ecbd..7887e61aa32c 100644 --- a/src/Uno.UI/UI/Xaml/FrameworkElement.Layout.crossruntime.cs +++ b/src/Uno.UI/UI/Xaml/FrameworkElement.Layout.crossruntime.cs @@ -283,7 +283,7 @@ private void InnerMeasureCore(Size availableSize) //if (!bInLayoutTransition) { // Templates should be applied here. - InvokeApplyTemplate(out var addedVisual); + InvokeApplyTemplate(out _); // TODO: BEGIN Uno specific if (m_requiresResourcesUpdate && this is Control thisAsControl) From f5abecdae3e75a06906686fbecc3618e1773c47a Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Mon, 13 Jan 2025 14:09:16 -0500 Subject: [PATCH 17/17] chore: Cleanup unused branches (cherry picked from commit 7ecbfe6ab82019d8e0f14f844cf022b4081b2690) --- src/Uno.UI/UI/Xaml/ResourceResolver.cs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/Uno.UI/UI/Xaml/ResourceResolver.cs b/src/Uno.UI/UI/Xaml/ResourceResolver.cs index c7bce4501bd0..f8c16d11fe34 100644 --- a/src/Uno.UI/UI/Xaml/ResourceResolver.cs +++ b/src/Uno.UI/UI/Xaml/ResourceResolver.cs @@ -393,16 +393,10 @@ internal static bool TryStaticRetrieval(in SpecializedResourceDictionary.Resourc var dictionary = (source.Target as FrameworkElement)?.TryGetResources() ?? source.Target as ResourceDictionary; - if (dictionary != null) + if (dictionary != null + && dictionary.TryGetValue(resourceKey, out value, shouldCheckSystem: false)) { - if (dictionary.TryGetValue(resourceKey, out value, shouldCheckSystem: false)) - { - return true; - } - } - else - { - + return true; } }