Skip to content

Commit

Permalink
fix(xLoad): [Android] Fix invalid x:Load reentrancy issue
Browse files Browse the repository at this point in the history
This scenario may happen when `x:Load` is used with a visibility binding causing a invalid invocation of the ViewGroup.RemoveViewAt Android API.
  • Loading branch information
jeromelaban committed Sep 17, 2021
1 parent 1a1b091 commit 9784464
Show file tree
Hide file tree
Showing 4 changed files with 133 additions and 11 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<UserControl
x:Class="Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml.Controls.When_xLoad_Visibility_While_Materializing"
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
xmlns:local="using:Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml.Controls"
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
mc:Ignorable="d">

<Grid>
<local:When_xLoad_Visibility_While_Materializing_Content
x:Name="item1"
x:FieldModifier="public"
x:Load="false"
Visibility="{x:Bind Model.IsVisible, Mode=OneWay, FallbackValue=false}" />
</Grid>
</UserControl>
Original file line number Diff line number Diff line change
@@ -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; }
}
}
19 changes: 19 additions & 0 deletions src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_xLoad.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
namespace Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml
{
[TestClass]
[RunsOnUIThread]
public class Given_xLoad
{
[TestMethod]
Expand Down Expand Up @@ -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
44 changes: 33 additions & 11 deletions src/Uno.UI/UI/Xaml/ElementStub.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,14 @@ private View _content
private View _content;
#endif

/// <summary>
/// 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.
/// </summary>
private bool _isMaterializing;

/// <summary>
/// A delegate used to raise materialization changes in <see cref="ElementStub.MaterializationChanged"/>
/// </summary>
Expand Down Expand Up @@ -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
}
}

Expand Down

0 comments on commit 9784464

Please sign in to comment.