Skip to content

Commit

Permalink
fix: Prevent reentrancy of UpdateLayout with native layouting on xamarin
Browse files Browse the repository at this point in the history
  • Loading branch information
@dr1rrb
dr1rrb committed Feb 3, 2021
1 parent ce2641d commit a3fd5d3
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 49 deletions.
80 changes: 42 additions & 38 deletions src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_UIElement.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,14 @@
namespace Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml
{
[TestClass]
public partial class Given_UIElement
public partial class Given_UIElement
{
#if HAS_UNO // Tests use IsArrangeDirty, which is an internal property
[TestMethod]
[RunsOnUIThread]
public async Task When_Visible_InvalidateArrange()
{
var sut = new Border()
{
Width = 100,
Height = 10
};
var sut = new Border() {Width = 100, Height = 10};

TestServices.WindowHelper.WindowContent = sut;
await TestServices.WindowHelper.WaitForIdle();
Expand Down Expand Up @@ -74,7 +70,7 @@ public async Task When_TextBlock_ActualSize()

border.UpdateLayout();

await TestServices.WindowHelper.WaitFor(()=>Math.Abs(text.ActualWidth - text.ActualSize.X) < 0.01);
await TestServices.WindowHelper.WaitFor(() => Math.Abs(text.ActualWidth - text.ActualSize.X) < 0.01);
await TestServices.WindowHelper.WaitFor(() => Math.Abs(text.ActualHeight - text.ActualSize.Y) < 0.01);

text.Text = "This is a longer text";
Expand Down Expand Up @@ -102,15 +98,19 @@ public async Task When_Rectangle_Set_ActualSize()

border.UpdateLayout();

await TestServices.WindowHelper.WaitFor(() => Math.Abs(rectangle.ActualWidth - rectangle.ActualSize.X) < 0.01);
await TestServices.WindowHelper.WaitFor(() => Math.Abs(rectangle.ActualHeight - rectangle.ActualSize.Y) < 0.01);
await TestServices.WindowHelper.WaitFor(() =>
Math.Abs(rectangle.ActualWidth - rectangle.ActualSize.X) < 0.01);
await TestServices.WindowHelper.WaitFor(() =>
Math.Abs(rectangle.ActualHeight - rectangle.ActualSize.Y) < 0.01);

rectangle.Width = 16;
rectangle.Height = 32;
border.UpdateLayout();

await TestServices.WindowHelper.WaitFor(() => Math.Abs(rectangle.ActualWidth - rectangle.ActualSize.X) < 0.01);
await TestServices.WindowHelper.WaitFor(() => Math.Abs(rectangle.ActualHeight - rectangle.ActualSize.Y) < 0.01);
await TestServices.WindowHelper.WaitFor(() =>
Math.Abs(rectangle.ActualWidth - rectangle.ActualSize.X) < 0.01);
await TestServices.WindowHelper.WaitFor(() =>
Math.Abs(rectangle.ActualHeight - rectangle.ActualSize.Y) < 0.01);
}
#endif

Expand All @@ -121,7 +121,10 @@ public void When_UpdateLayout_Then_TreeNotMeasuredUsingCachedValue()
{
if (Window.Current.RootElement is Panel root)
{
var sut = new Grid { HorizontalAlignment = HorizontalAlignment.Stretch, VerticalAlignment = VerticalAlignment.Stretch };
var sut = new Grid
{
HorizontalAlignment = HorizontalAlignment.Stretch, VerticalAlignment = VerticalAlignment.Stretch
};

var originalRootAvailableSize = LayoutInformation.GetAvailableSize(root);
var originalRootDesiredSize = LayoutInformation.GetDesiredSize(root);
Expand All @@ -148,7 +151,8 @@ public void When_UpdateLayout_Then_TreeNotMeasuredUsingCachedValue()
LayoutInformation.SetLayoutSlot(root, originalRootLayoutSlot);

root.Children.Remove(sut);
try { root.UpdateLayout(); } catch { } // Make sure to restore visual tree if test has failed!
try { root.UpdateLayout(); }
catch { } // Make sure to restore visual tree if test has failed!
}

Assert.AreNotEqual(default, availableSize);
Expand All @@ -174,40 +178,40 @@ public async Task When_UpdateLayout_Then_ReentrancyNotAllowed()

Assert.IsFalse(sut.Failed);
}
}

private class When_UpdateLayout_Then_ReentrancyNotAllowed_Element : FrameworkElement
{
private bool _isMeasuring, _isArranging;

public bool Failed { get; private set; }
internal partial class When_UpdateLayout_Then_ReentrancyNotAllowed_Element : FrameworkElement
{
private bool _isMeasuring, _isArranging;

protected override Size MeasureOverride(Size availableSize)
{
Failed |= _isMeasuring;
public bool Failed { get; private set; }

if (!Failed)
{
_isMeasuring = true;
UpdateLayout();
_isMeasuring = false;
}
protected override Size MeasureOverride(Size availableSize)
{
Failed |= _isMeasuring;

return base.MeasureOverride(availableSize);
if (!Failed)
{
_isMeasuring = true;
UpdateLayout();
_isMeasuring = false;
}

protected override Size ArrangeOverride(Size finalSize)
{
Failed |= _isArranging;
return base.MeasureOverride(availableSize);
}

if (!Failed)
{
_isArranging = true;
UpdateLayout();
_isArranging = false;
}
protected override Size ArrangeOverride(Size finalSize)
{
Failed |= _isArranging;

return base.ArrangeOverride(finalSize);
if (!Failed)
{
_isArranging = true;
UpdateLayout();
_isArranging = false;
}

return base.ArrangeOverride(finalSize);
}
}
}
47 changes: 37 additions & 10 deletions src/Uno.UI/UI/Xaml/Controls/Layouter/Layouter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,14 @@ internal abstract partial class Layouter : ILayouter

internal Size _unclippedDesiredSize;

private UIElement _elementAsUIElement;

public IFrameworkElement Panel { get; }

protected Layouter(IFrameworkElement element)
{
Panel = element;
_elementAsUIElement = element as UIElement;

var log = this.Log();
if (log.IsEnabled(LogLevel.Debug))
Expand Down Expand Up @@ -90,10 +93,14 @@ public Size Measure(Size availableSize)
return default;
}

using (traceActivity)
try
{
var (minSize, maxSize) = Panel.GetMinMax();
if (_elementAsUIElement?.IsVisualTreeRoot ?? false)
{
UIElement.IsLayoutingVisualTreeRoot = true;
}

var (minSize, maxSize) = Panel.GetMinMax();
var marginSize = Panel.GetMarginSize();

// NaN values are accepted as input here, particularly when coming from
Expand Down Expand Up @@ -141,6 +148,14 @@ public Size Measure(Size availableSize)
// We return "clipped" desiredSize to caller... the unclipped version stays internal
return clippedDesiredSize;
}
finally
{
if (_elementAsUIElement?.IsVisualTreeRoot ?? false)
{
UIElement.IsLayoutingVisualTreeRoot = false;
}
traceActivity?.Dispose();
}
}

[Pure]
Expand All @@ -163,8 +178,6 @@ public void Arrange(Rect finalRect)
{
LayoutInformation.SetLayoutSlot(Panel, finalRect);

var uiElement = Panel as UIElement;

IDisposable traceActivity = null;
if (_trace.IsEnabled)
{
Expand All @@ -174,14 +187,20 @@ public void Arrange(Rect finalRect)
new object[] { LoggingOwnerTypeName, Panel.GetDependencyObjectId() }
);
}
using (traceActivity)

try
{
if (_elementAsUIElement?.IsVisualTreeRoot ?? false)
{
UIElement.IsLayoutingVisualTreeRoot = true;
}

if (this.Log().IsEnabled(Microsoft.Extensions.Logging.LogLevel.Debug))
{
this.Log().DebugFormat("[{0}/{1}] Arrange({2}/{3}/{4}/{5})", LoggingOwnerTypeName, Name, GetType(), Panel.Name, finalRect, Panel.Margin);
}

var clippedArrangeSize = uiElement?.ClippedFrame is Rect clip && !uiElement.IsArrangeDirty
var clippedArrangeSize = _elementAsUIElement?.ClippedFrame is Rect clip && !_elementAsUIElement.IsArrangeDirty
? clip.Size
: finalRect.Size;

Expand Down Expand Up @@ -247,11 +266,11 @@ public void Arrange(Rect finalRect)

var renderSize = ArrangeOverride(arrangeSize);

if (uiElement != null)
if (_elementAsUIElement != null)
{
uiElement.RenderSize = renderSize.AtMost(maxSize);
uiElement.NeedsClipToSlot = needsClipToSlot;
uiElement.ApplyClip();
_elementAsUIElement.RenderSize = renderSize.AtMost(maxSize);
_elementAsUIElement.NeedsClipToSlot = needsClipToSlot;
_elementAsUIElement.ApplyClip();

if (Panel is FrameworkElement fe)
{
Expand All @@ -263,6 +282,14 @@ public void Arrange(Rect finalRect)
evp.OnLayoutUpdated();
}
}
finally
{
if (_elementAsUIElement?.IsVisualTreeRoot ?? false)
{
UIElement.IsLayoutingVisualTreeRoot = true;
}
traceActivity?.Dispose();
}
}

/// <summary>
Expand Down
12 changes: 11 additions & 1 deletion src/Uno.UI/UI/Xaml/UIElement.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -348,8 +348,18 @@ private void OpenContextFlyout(object sender, RightTappedRoutedEventArgs args)
[ThreadStatic]
private static bool _isInUpdateLayout; // Currently within the UpdateLayout() method (explicit managed layout request)

#pragma warning disable CS0649 // Field not used on Desktop/Tests
[ThreadStatic]
private static bool _isLayoutingVisualTreeRoot; // Currenlty in Measure or Arrange of the element flagged with IsVisualTreeRoot (layout requested by the system)
private static bool _isLayoutingVisualTreeRoot; // Currently in Measure or Arrange of the element flagged with IsVisualTreeRoot (layout requested by the system)
#pragma warning restore CS0649

#if !__NETSTD__ // We need an internal accessor for the Layouter
internal static bool IsLayoutingVisualTreeRoot
{
get => _isLayoutingVisualTreeRoot;
set => _isLayoutingVisualTreeRoot = value;
}
#endif

private const int MaxLayoutIterations = 250;

Expand Down

0 comments on commit a3fd5d3

Please sign in to comment.