From a3fd5d3d1f3cdc34e97bd2dab253faeef1e077b2 Mon Sep 17 00:00:00 2001 From: David Date: Wed, 3 Feb 2021 08:34:38 -0500 Subject: [PATCH] fix: Prevent reentrancy of UpdateLayout with native layouting on xamarin --- .../Tests/Windows_UI_Xaml/Given_UIElement.cs | 80 ++++++++++--------- .../UI/Xaml/Controls/Layouter/Layouter.cs | 47 ++++++++--- src/Uno.UI/UI/Xaml/UIElement.cs | 12 ++- 3 files changed, 90 insertions(+), 49 deletions(-) diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_UIElement.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_UIElement.cs index 3c504d8c96ea..3dc9282b1a05 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_UIElement.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_UIElement.cs @@ -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(); @@ -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"; @@ -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 @@ -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); @@ -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); @@ -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); } } } diff --git a/src/Uno.UI/UI/Xaml/Controls/Layouter/Layouter.cs b/src/Uno.UI/UI/Xaml/Controls/Layouter/Layouter.cs index 6dca22063e5d..6749869ca6f6 100644 --- a/src/Uno.UI/UI/Xaml/Controls/Layouter/Layouter.cs +++ b/src/Uno.UI/UI/Xaml/Controls/Layouter/Layouter.cs @@ -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)) @@ -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 @@ -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] @@ -163,8 +178,6 @@ public void Arrange(Rect finalRect) { LayoutInformation.SetLayoutSlot(Panel, finalRect); - var uiElement = Panel as UIElement; - IDisposable traceActivity = null; if (_trace.IsEnabled) { @@ -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; @@ -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) { @@ -263,6 +282,14 @@ public void Arrange(Rect finalRect) evp.OnLayoutUpdated(); } } + finally + { + if (_elementAsUIElement?.IsVisualTreeRoot ?? false) + { + UIElement.IsLayoutingVisualTreeRoot = true; + } + traceActivity?.Dispose(); + } } /// diff --git a/src/Uno.UI/UI/Xaml/UIElement.cs b/src/Uno.UI/UI/Xaml/UIElement.cs index 1fed53a3caa9..b2aed80a78dc 100644 --- a/src/Uno.UI/UI/Xaml/UIElement.cs +++ b/src/Uno.UI/UI/Xaml/UIElement.cs @@ -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;