From 1a1b0914c117392391253cecd0a324cf82729ae1 Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Thu, 16 Sep 2021 11:17:48 -0400 Subject: [PATCH 1/2] fix(2022): Fix remote control build under VS2022 --- src/Uno.UI.RemoteControl.VS/Uno.UI.RemoteControl.VS.csproj | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Uno.UI.RemoteControl.VS/Uno.UI.RemoteControl.VS.csproj b/src/Uno.UI.RemoteControl.VS/Uno.UI.RemoteControl.VS.csproj index 1f152ce04fa6..59d825b5cf5e 100644 --- a/src/Uno.UI.RemoteControl.VS/Uno.UI.RemoteControl.VS.csproj +++ b/src/Uno.UI.RemoteControl.VS/Uno.UI.RemoteControl.VS.csproj @@ -68,4 +68,7 @@ + + + From 9784464c49c4941ad7d9a11c18ef85ab2010cc8d Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Thu, 16 Sep 2021 11:20:49 -0400 Subject: [PATCH 2/2] fix(xLoad): [Android] Fix invalid x:Load reentrancy issue This scenario may happen when `x:Load` is used with a visibility binding causing a invalid invocation of the ViewGroup.RemoveViewAt Android API. --- ..._xLoad_Visibility_While_Materializing.xaml | 17 +++++ ...oad_Visibility_While_Materializing.xaml.cs | 64 +++++++++++++++++++ .../Tests/Windows_UI_Xaml/Given_xLoad.cs | 19 ++++++ src/Uno.UI/UI/Xaml/ElementStub.cs | 44 +++++++++---- 4 files changed, 133 insertions(+), 11 deletions(-) create mode 100644 src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/When_xLoad_Visibility_While_Materializing.xaml create mode 100644 src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/When_xLoad_Visibility_While_Materializing.xaml.cs diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/When_xLoad_Visibility_While_Materializing.xaml b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/When_xLoad_Visibility_While_Materializing.xaml new file mode 100644 index 000000000000..ac084e9118a2 --- /dev/null +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/When_xLoad_Visibility_While_Materializing.xaml @@ -0,0 +1,17 @@ + + + + + + diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/When_xLoad_Visibility_While_Materializing.xaml.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/When_xLoad_Visibility_While_Materializing.xaml.cs new file mode 100644 index 000000000000..583f2bc086ca --- /dev/null +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/When_xLoad_Visibility_While_Materializing.xaml.cs @@ -0,0 +1,64 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Runtime.InteropServices.WindowsRuntime; +using Windows.Foundation; +using Windows.Foundation.Collections; +using Windows.UI.Xaml; +using Windows.UI.Xaml.Controls; +using Windows.UI.Xaml.Controls.Primitives; +using Windows.UI.Xaml.Data; +using Windows.UI.Xaml.Input; +using Windows.UI.Xaml.Media; +using Windows.UI.Xaml.Navigation; + +// The User Control item template is documented at https://go.microsoft.com/fwlink/?LinkId=234236 + +namespace Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml.Controls +{ + public sealed partial class When_xLoad_Visibility_While_Materializing : UserControl + { + public When_xLoad_Visibility_While_Materializing() + { + Model = new Model(); + + Model.PropertyChanged += delegate { + FindName("item1"); + }; + + this.InitializeComponent(); + } + + public Model Model { get; } + } + + public class Model : System.ComponentModel.INotifyPropertyChanged + { + public event System.ComponentModel.PropertyChangedEventHandler PropertyChanged; + + private bool _isVisible; + + public bool IsVisible + { + get { return _isVisible; } + set + { + _isVisible = value; + + PropertyChanged.Invoke(this, new System.ComponentModel.PropertyChangedEventArgs(nameof(IsVisible))); + } + } + + } + + public partial class When_xLoad_Visibility_While_Materializing_Content : UserControl + { + public When_xLoad_Visibility_While_Materializing_Content() + { + Instances++; + } + + public static int Instances { get; set; } + } +} diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_xLoad.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_xLoad.cs index 5e2cec738733..06b053380ea8 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_xLoad.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_xLoad.cs @@ -10,6 +10,7 @@ namespace Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml { [TestClass] + [RunsOnUIThread] public class Given_xLoad { [TestMethod] @@ -47,6 +48,24 @@ public async Task When_xLoad_xBind() Assert.IsFalse((parent.Child as ElementStub).Load); } + + [TestMethod] + [RunsOnUIThread] + public void When_xLoad_Visibility_While_Materializing() + { + var SUT = new When_xLoad_Visibility_While_Materializing(); + + Assert.AreEqual(0, When_xLoad_Visibility_While_Materializing_Content.Instances); + + TestServices.WindowHelper.WindowContent = SUT; + + Assert.AreEqual(0, When_xLoad_Visibility_While_Materializing_Content.Instances); + + SUT.Model.IsVisible = true; + + Assert.AreEqual(1, When_xLoad_Visibility_While_Materializing_Content.Instances); + } + } } #endif diff --git a/src/Uno.UI/UI/Xaml/ElementStub.cs b/src/Uno.UI/UI/Xaml/ElementStub.cs index 0d0ac21ffa3b..db41a4b93a9a 100644 --- a/src/Uno.UI/UI/Xaml/ElementStub.cs +++ b/src/Uno.UI/UI/Xaml/ElementStub.cs @@ -45,6 +45,14 @@ private View _content private View _content; #endif + /// + /// Ensures that materialization handles reentrancy properly. + /// This scenario can happen on android specifically because the Parent + /// property is not immediately set to null once a view is removed from + /// the tree. + /// + private bool _isMaterializing; + /// /// A delegate used to raise materialization changes in /// @@ -144,22 +152,36 @@ public void Materialize() private void Materialize(bool isVisibilityChanged) { - if (_content == null) + if (_content == null && !_isMaterializing) { - _content = SwapViews(oldView: (FrameworkElement)this, newViewProvider: ContentBuilder); - var targetDependencyObject = _content as DependencyObject; - - if (isVisibilityChanged && targetDependencyObject != null) +#if !HAS_EXPENSIVE_TRYFINALLY // Try/finally incurs a very large performance hit in mono-wasm - https://github.com/dotnet/runtime/issues/50783 + try { - var visibilityProperty = GetVisibilityProperty(_content); +#endif + _isMaterializing = true; - // Set the visibility at the same precedence it was currently set with on the stub. - var precedence = this.GetCurrentHighestValuePrecedence(visibilityProperty); + _content = SwapViews(oldView: (FrameworkElement)this, newViewProvider: ContentBuilder); + var targetDependencyObject = _content as DependencyObject; - targetDependencyObject.SetValue(visibilityProperty, Visibility.Visible, precedence); - } + if (isVisibilityChanged && targetDependencyObject != null) + { + var visibilityProperty = GetVisibilityProperty(_content); - MaterializationChanged?.Invoke(this); + // Set the visibility at the same precedence it was currently set with on the stub. + var precedence = this.GetCurrentHighestValuePrecedence(visibilityProperty); + + targetDependencyObject.SetValue(visibilityProperty, Visibility.Visible, precedence); + } + + MaterializationChanged?.Invoke(this); + +#if !HAS_EXPENSIVE_TRYFINALLY // Try/finally incurs a very large performance hit in mono-wasm - https://github.com/dotnet/runtime/issues/50783 + } + finally + { + _isMaterializing = false; + } +#endif } }