Skip to content

Commit

Permalink
fix: Load and Unload framework elements from resources [Skia,Wasm,And…
Browse files Browse the repository at this point in the history
…roid only]
  • Loading branch information
Youssef1313 committed Mar 4, 2024
1 parent a1a6402 commit 9d2cab8
Show file tree
Hide file tree
Showing 4 changed files with 212 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
using Microsoft.UI.Xaml.Markup;
using Microsoft.UI.Xaml.Media;
using Uno.UI.RuntimeTests.Helpers;
using Microsoft.UI.Xaml.Controls;
using Private.Infrastructure;



#if HAS_UNO
Expand All @@ -18,6 +21,129 @@ namespace Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml
public class Given_ResourceDictionary
{
[TestMethod]
#if __IOS__ || __MACOS__
[Ignore("iOS and macOS don't yet load/unload from resources - https://github.com/unoplatform/uno/issues/5208")]
#endif
public async Task When_FrameworkElement_In_Resources_Should_Receive_Loaded_Unloaded()
{
var textBlock = new TextBlock();
var resourceTextBlock = new TextBlock();
var resourceGrid = new Grid()
{
Children =
{
resourceTextBlock,
},
};
var grid = new Grid()
{
Children =
{
textBlock,
},
Width = 100,
Height = 100,
};

grid.Resources.Add("MyResourceGridKey", resourceGrid);

string result = "";

resourceTextBlock.Loaded += (_, _) => result += "ResourceTextBlockLoaded,";
resourceGrid.Loaded += (_, _) => result += "ResourceGridLoaded,";
textBlock.Loaded += (_, _) => result += "TextBlockLoaded,";

resourceTextBlock.Unloaded += (_, _) => result += "ResourceTextBlockUnloaded,";
resourceGrid.Unloaded += (_, _) => result += "ResourceGridUnloaded,";
textBlock.Unloaded += (_, _) => result += "TextBlockUnloaded,";

await UITestHelper.Load(grid);

#if WINAPPSDK
Assert.AreEqual("ResourceTextBlockLoaded,ResourceGridLoaded,TextBlockLoaded,", result);
#else
Assert.AreEqual("ResourceGridLoaded,ResourceTextBlockLoaded,TextBlockLoaded,", result);
#endif

await UITestHelper.Load(new Border() { Width = 1, Height = 1 });

#if WINAPPSDK
Assert.AreEqual("ResourceTextBlockLoaded,ResourceGridLoaded,TextBlockLoaded,ResourceGridUnloaded,ResourceTextBlockUnloaded,TextBlockUnloaded,", result);
#elif __ANDROID__
Assert.AreEqual("ResourceGridLoaded,ResourceTextBlockLoaded,TextBlockLoaded,ResourceGridUnloaded,ResourceTextBlockUnloaded,TextBlockUnloaded,", result);
#else
Assert.AreEqual("ResourceGridLoaded,ResourceTextBlockLoaded,TextBlockLoaded,ResourceTextBlockUnloaded,ResourceGridUnloaded,TextBlockUnloaded,", result);
#endif
}

[TestMethod]
#if __IOS__ || __MACOS__
[Ignore("iOS and macOS don't yet load/unload from resources - https://github.com/unoplatform/uno/issues/5208")]
#endif
public async Task When_FrameworkElement_In_Resources_Then_Removed_Should_Receive_Loaded_Unloaded()
{
var textBlock = new TextBlock();
var resourceTextBlock = new TextBlock();
var resourceGrid = new Grid()
{
Children =
{
resourceTextBlock,
},
};
var grid = new Grid()
{
Children =
{
textBlock,
},
Width = 100,
Height = 100,
};

grid.Resources.Add("MyResourceGridKey", resourceGrid);

string result = "";

resourceTextBlock.Loaded += (_, _) => result += "ResourceTextBlockLoaded,";
resourceGrid.Loaded += (_, _) => result += "ResourceGridLoaded,";
textBlock.Loaded += (_, _) => result += "TextBlockLoaded,";

resourceTextBlock.Unloaded += (_, _) => result += "ResourceTextBlockUnloaded,";
resourceGrid.Unloaded += (_, _) => result += "ResourceGridUnloaded,";
textBlock.Unloaded += (_, _) => result += "TextBlockUnloaded,";

await UITestHelper.Load(grid);

#if WINAPPSDK
Assert.AreEqual("ResourceTextBlockLoaded,ResourceGridLoaded,TextBlockLoaded,", result);
#else
Assert.AreEqual("ResourceGridLoaded,ResourceTextBlockLoaded,TextBlockLoaded,", result);
#endif

grid.Resources.Remove("MyResourceGridKey");
await TestServices.WindowHelper.WaitForIdle();

#if WINAPPSDK
Assert.AreEqual("ResourceTextBlockLoaded,ResourceGridLoaded,TextBlockLoaded,ResourceGridUnloaded,ResourceTextBlockUnloaded,", result);
#elif __ANDROID__
Assert.AreEqual("ResourceGridLoaded,ResourceTextBlockLoaded,TextBlockLoaded,ResourceGridUnloaded,ResourceTextBlockUnloaded,", result);
#else
Assert.AreEqual("ResourceGridLoaded,ResourceTextBlockLoaded,TextBlockLoaded,ResourceTextBlockUnloaded,ResourceGridUnloaded,", result);
#endif

await UITestHelper.Load(new Border() { Width = 1, Height = 1 });

#if WINAPPSDK
Assert.AreEqual("ResourceTextBlockLoaded,ResourceGridLoaded,TextBlockLoaded,ResourceGridUnloaded,ResourceTextBlockUnloaded,TextBlockUnloaded,", result);
#elif __ANDROID__
Assert.AreEqual("ResourceGridLoaded,ResourceTextBlockLoaded,TextBlockLoaded,ResourceGridUnloaded,ResourceTextBlockUnloaded,TextBlockUnloaded,", result);
#else
Assert.AreEqual("ResourceGridLoaded,ResourceTextBlockLoaded,TextBlockLoaded,ResourceTextBlockUnloaded,ResourceGridUnloaded,TextBlockUnloaded,", result);
#endif
}

[TestMethod]
public void When_Key_Overwritten()
{
const string key = "TestKey";
Expand Down
50 changes: 41 additions & 9 deletions src/Uno.UI/UI/Xaml/FrameworkElement.Android.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,13 @@ protected FrameworkElement()
partial void Initialize();

protected override void OnNativeLoaded()
=> OnNativeLoaded(isFromResources: false);

private void OnNativeLoaded(bool isFromResources)
{
try
{
PerformOnLoaded();
PerformOnLoaded(isFromResources);

base.OnNativeLoaded();
}
Expand All @@ -48,10 +51,25 @@ protected override void OnNativeLoaded()
}
}

private void PerformOnLoaded()
private void PerformOnLoaded(bool isFromResources = false)
{
((IDependencyObjectStoreProvider)this).Store.Parent = base.Parent;
OnLoading();
if (!isFromResources)
{
((IDependencyObjectStoreProvider)this).Store.Parent = base.Parent;
OnLoading();
}

if (this.Resources is not null)
{
foreach (var resource in Resources.Values)
{
if (resource is FrameworkElement resourceAsFrameworkElement)
{
resourceAsFrameworkElement.PerformOnLoaded(isFromResources: true);
}
}
}

OnLoaded();

if (FeatureConfiguration.FrameworkElement.AndroidUseManagedLoadedUnloaded)
Expand All @@ -67,17 +85,20 @@ private void PerformOnLoaded()
// Calling this method is acceptable as it is an abstract method that
// will never do interop with the java class. It is required to invoke
// Loaded/Unloaded actions.
e.OnNativeLoaded();
e.OnNativeLoaded(isFromResources);
}
}
}
}

protected override void OnNativeUnloaded()
=> OnNativeUnloaded();

private void OnNativeUnloaded(bool isFromResources = false)

Check failure on line 97 in src/Uno.UI/UI/Xaml/FrameworkElement.Android.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/Uno.UI/UI/Xaml/FrameworkElement.Android.cs#L97

This method signature overlaps the one defined on line 94, the default parameter value can't be used.
{
try
{
PerformOnUnloaded();
PerformOnUnloaded(isFromResources);

base.OnNativeUnloaded();
}
Expand All @@ -88,11 +109,22 @@ protected override void OnNativeUnloaded()
}
}

internal void PerformOnUnloaded()
internal void PerformOnUnloaded(bool isFromResources = false)
{
if (this.Resources is not null)
{
foreach (var resource in this.Resources.Values)
{
if (resource is FrameworkElement fe)
{
fe.PerformOnUnloaded(isFromResources: true);
}
}
}

if (FeatureConfiguration.FrameworkElement.AndroidUseManagedLoadedUnloaded)
{
if (IsNativeLoaded)
if (isFromResources || IsNativeLoaded)
{
OnUnloaded();

Expand All @@ -107,7 +139,7 @@ void ProcessView(View view)
// Calling this method is acceptable as it is an abstract method that
// will never do interop with the java class. It is required to invoke
// Loaded/Unloaded actions.
e.OnNativeUnloaded();
e.OnNativeUnloaded(isFromResources);
}
else if (view is ViewGroup childViewGroup)
{
Expand Down
18 changes: 18 additions & 0 deletions src/Uno.UI/UI/Xaml/ResourceDictionary.cs
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,29 @@ public bool Insert(object key, object value)
public bool Remove(object key)
{
var keyToRemove = new ResourceKey(key);
#if __SKIA__ || __WASM__ || __ANDROID__
if (_values.TryGetValue(keyToRemove, out var value))
{
_values.Remove(keyToRemove);
if (value is FrameworkElement fe)
{
#if __SKIA__ || __WASM__
fe.OnElementUnloaded();
#else
fe.PerformOnUnloaded(isFromResources: true);
#endif
}

ResourceDictionaryValueChange?.Invoke(this, EventArgs.Empty);
return true;
}
#else
if (_values.Remove(keyToRemove))
{
ResourceDictionaryValueChange?.Invoke(this, EventArgs.Empty);
return true;
}
#endif

return false;
}
Expand Down
28 changes: 27 additions & 1 deletion src/Uno.UI/UI/Xaml/UIElement.crossruntime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,21 @@ private void OnElementLoaded()
OnFwEltLoaded();
UpdateHitTest();

// The way this works on WinUI is that when an element enters the visual tree, all values
// 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 (this is FrameworkElement { Resources: { } resources })

Check failure on line 128 in src/Uno.UI/UI/Xaml/UIElement.crossruntime.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/Uno.UI/UI/Xaml/UIElement.crossruntime.cs#L128

Offload the code that's conditional on this type test to the appropriate subclass and remove the condition.
{
foreach (var resource in resources.Values)
{
if (resource is FrameworkElement resourceAsUIElement)
{
resourceAsUIElement.OnElementLoaded();
}
}
}

// Get a materialized copy for Wasm to avoid the use of iterators
// where try/finally has a high cost.
var children = _children.Materialized;
Expand All @@ -130,7 +145,7 @@ private void OnElementLoaded()
}
}

private void OnElementUnloaded()
internal void OnElementUnloaded()
{
if (!IsLoaded)
{
Expand All @@ -140,6 +155,17 @@ private void OnElementUnloaded()
IsLoaded = false;
Depth = int.MinValue;

if (this is FrameworkElement { Resources: { } resources })
{
foreach (var resource in resources.Values)
{
if (resource is FrameworkElement resourceAsUIElement)
{
resourceAsUIElement.OnElementUnloaded();
}
}
}

// Get a materialized copy for Wasm to avoid the use of iterators
// where try/finally has a high cost.
var children = _children.Materialized;
Expand Down

0 comments on commit 9d2cab8

Please sign in to comment.