Merge pull request #18659 from ramezgerges/skia-runtime-tests-locally…

…-green

Make skia runtime tests green locally on Debug

src/SamplesApp/SamplesApp.Shared/App.xaml.cs

@@ -159,11 +159,8 @@ override void OnLaunched(LaunchActivatedEventArgs e)
 			}
 
 			var sw = Stopwatch.StartNew();
-#if DEBUG
-			if (System.Diagnostics.Debugger.IsAttached)
-			{
-				// this.DebugSettings.EnableFrameRateCounter = true;
-			}
+#if WINAPPSDK && DEBUG
+			// this.DebugSettings.EnableFrameRateCounter = true;
 #endif
 			AssertInitialWindowSize();
 
@@ -549,6 +546,7 @@ static void ConfigureFeatureFlags()
 #if HAS_UNO
 			Uno.UI.FeatureConfiguration.TextBox.UseOverlayOnSkia = false;
 			Uno.UI.FeatureConfiguration.ToolTip.UseToolTips = true;
+			Uno.UI.FeatureConfiguration.DependencyProperty.ValidatePropertyOwnerOnReadWrite = true;
 
 			Uno.UI.FeatureConfiguration.Font.DefaultTextFontFamily = "ms-appx:///Uno.Fonts.OpenSans/Fonts/OpenSans.ttf";
 #endif

src/SamplesApp/SamplesApp.UnitTests.Shared/Controls/UnitTest/UnitTestsControl.cs

@@ -940,7 +940,7 @@ await TestServices.WindowHelper.RootElementDispatcher.RunAsync(() =>
 							}
 							else if (test.ExpectedException is null || !test.ExpectedException.IsInstanceOfType(e))
 							{
-								if (_currentRun.CurrentRepeatCount < config.Attempts - 1 && !Debugger.IsAttached)
+								if (_currentRun.CurrentRepeatCount < config.Attempts - 1)
 								{
 									_currentRun.CurrentRepeatCount++;
 									canRetry = true;

src/Uno.UI.RuntimeTests/IntegrationTests/common/CalendarHelper.h.cs

@@ -236,7 +236,7 @@ internal void VerifyNoSelectedDatesChanged()
 			{
 				// we expect no event here, so below statement will timeout and throw WEX.Common.Exception.
 				TestServices.VERIFY_THROWS_WINRT<Exception>(
-					async () => await m_selectedDatesChangedEvent.WaitFor(TimeSpan.FromMilliseconds(5000), true /* enforceUnderDebugger */),
+					async () => await m_selectedDatesChangedEvent.WaitFor(TimeSpan.FromMilliseconds(5000)),
 					"SelectedDatesChanged event should not raise!");
 				TestServices.VERIFY_IS_FALSE(m_selectedDatesChangedEvent.HasFired());
 				m_selectedDatesChangedRegistration.Detach();

src/Uno.UI.RuntimeTests/IntegrationTests/dxaml/Event.cs

@@ -74,15 +74,5 @@ public Task WaitFor(TimeSpan timeout, CancellationToken ct = default)
 		{
 			return WaitForDefault((int)timeout.TotalMilliseconds, ct);
 		}
-
-		internal Task WaitFor(TimeSpan timeout, bool enforceUnderDebugger, CancellationToken ct = default)
-		{
-			if (!enforceUnderDebugger && Debugger.IsAttached)
-			{
-				return Task.CompletedTask;
-			}
-
-			return WaitForDefault((int)timeout.TotalMilliseconds, ct);
-		}
 	}
 }

src/Uno.UI.RuntimeTests/MUX/Helpers/EventTester.cs

@@ -15,6 +15,7 @@
 using Microsoft.UI.Xaml;
 using UIExecutor = MUXControlsTestApp.Utilities.RunOnUIThread;
 using MUXControlsTestApp.Utilities;
+using Uno.UI;
 
 namespace Microsoft/* UWP don't rename */.UI.Xaml.Tests.Common
 {
@@ -92,11 +93,11 @@ protected EventTester(TSender sender, Type senderType, string eventName, Action<
 #if !__WASM__
 			if (this.options.HasFlag(EventTesterOptions.CaptureWindowBefore))
 			{
-				this.CaptureWindowAsync("Before").Wait(this.Timeout);
+				this.CaptureWindowAsync("Before").Wait(this.DefaultTimeout);
 			}
 			if (this.options.HasFlag(EventTesterOptions.CaptureScreenBefore))
 			{
-				this.CaptureScreenAsync("Before").Wait(this.Timeout);
+				this.CaptureScreenAsync("Before").Wait(this.DefaultTimeout);
 			}
 #endif
 		}
@@ -153,22 +154,14 @@ public static EventTester<UIElement, TEventArgs> FromRoutedEvent(UIElement sende
 			return new RoutedEventTester<TEventArgs>(sender, eventName, action);
 		}
 
-		public TimeSpan DefaultTimeout = EventTesterConfig.Timeout;
-
-		private TimeSpan Timeout
-		{
-			get
-			{
-				if (Debugger.IsAttached)
-				{
-					return TimeSpan.FromMilliseconds(-1); // Wait indefinitely if debugger is attached.
-				}
-				else
-				{
-					return this.DefaultTimeout;
-				}
-			}
-		}
+		public TimeSpan DefaultTimeout =
+#if HAS_UNO
+			FeatureConfiguration.DebugOptions.WaitIndefinitelyInEventTester
+#else
+			Debugger.IsAttached
+#endif
+				? TimeSpan.FromMilliseconds(-1)
+				: EventTesterConfig.Timeout;
 
 		private TSender Sender
 		{
@@ -258,7 +251,7 @@ public TEventArgs LastArgs
 
 		public async Task<bool> Wait()
 		{
-			return await this.Wait(this.Timeout);
+			return await this.Wait(this.DefaultTimeout);
 		}
 
 		public async Task<bool> Wait(TimeSpan timeout)
@@ -332,7 +325,7 @@ private async Task CaptureWindowAsync(string prefix = "")
 
 		public async Task<bool> WaitAsync()
 		{
-			return await this.WaitAsync(this.Timeout);
+			return await this.WaitAsync(this.DefaultTimeout);
 		}
 
 		public async Task<bool> WaitAsync(TimeSpan timeout)
@@ -441,11 +434,11 @@ private void Dispose(bool disposing)
 #if !__WASM__
 					if (this.options.HasFlag(EventTesterOptions.CaptureWindowAfter))
 					{
-						this.CaptureWindowAsync("After").Wait(this.Timeout);
+						this.CaptureWindowAsync("After").Wait(this.DefaultTimeout);
 					}
 					if (this.options.HasFlag(EventTesterOptions.CaptureScreenAfter))
 					{
-						this.CaptureScreenAsync("After").Wait(this.Timeout);
+						this.CaptureScreenAsync("After").Wait(this.DefaultTimeout);
 					}
 #endif
 

src/Uno.UI.RuntimeTests/MUX/Utilities/TestHelpers.cs

@@ -139,7 +139,7 @@ namespace MUXControlsTestApp.Utilities
 {
 	public static class TestUtilities
 	{
-		public static int DefaultWaitMs = Debugger.IsAttached ? 120000 : 5000;
+		public static int DefaultWaitMs = 5000;
 
 		public static async Task SetAsVisualTreeRoot(FrameworkElement element)
 		{

src/Uno.UI.RuntimeTests/Tests/UnitTestsTests/Given_UnitTest.cs

@@ -10,6 +10,7 @@ public class Given_UnitTest
 	{
 		static int When_UnhandledException_Count;
 
+		// This tests that automatic retry is working.
 		[TestMethod]
 		public void When_UnhandledException()
 		{
@@ -25,6 +26,7 @@ public class Given_UnitTest_Initialize
 	{
 		static int Initialize_Count;
 
+		// This tests that automatic retry is working.
 		[TestInitialize]
 		public void Initialize()
 		{

src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_Image.cs

@@ -663,8 +663,8 @@ public async Task When_Exif_Rotated_MsAppx_Unequal_Dimensions()
 			await WindowHelper.WaitFor(() => imageOpened);
 			var screenshot = await TakeScreenshot(image);
 			ImageAssert.HasColorAt(screenshot, 5, screenshot.Height / 2, Color.FromArgb(0xFF, 0xED, 0x1B, 0x24), tolerance: 5);
-			ImageAssert.HasColorAt(screenshot, screenshot.Width / 2 - 10, screenshot.Height / 2, Color.FromArgb(0xFF, 0xED, 0x1B, 0x24), tolerance: 5);
-			ImageAssert.HasColorAt(screenshot, screenshot.Width / 2 + 10, screenshot.Height / 2, Color.FromArgb(0xFF, 0x23, 0xB1, 0x4D), tolerance: 5);
+			ImageAssert.HasColorAt(screenshot, screenshot.Width / 2.2f, screenshot.Height / 2, Color.FromArgb(0xFF, 0xED, 0x1B, 0x24), tolerance: 5);
+			ImageAssert.HasColorAt(screenshot, screenshot.Width / 1.8f, screenshot.Height / 2, Color.FromArgb(0xFF, 0x23, 0xB1, 0x4D), tolerance: 5);
 			ImageAssert.HasColorAt(screenshot, screenshot.Width - 5, screenshot.Height / 2, Color.FromArgb(0xFF, 0x23, 0xB1, 0x4D), tolerance: 5);
 
 		}

src/Uno.UI/FeatureConfiguration.cs

@@ -841,6 +841,29 @@ public static class DependencyProperty
 			/// of having an undefined behavior and/or race conditions.
 			/// </summary>
 			public static bool DisableThreadingCheck { get; set; }
+
+			/// <summary>
+			/// Enables checks that make sure that <see cref="DependencyObjectStore.GetValue" /> and
+			/// <see cref="DependencyObjectStore.SetValue" /> are only called on the owner of the property being
+			/// set/got.
+			/// </summary>
+			public static bool ValidatePropertyOwnerOnReadWrite { get; set; } =
+#if DEBUG
+				true;
+#else
+				global::System.Diagnostics.Debugger.IsAttached;
+#endif
+		}
+
+		/// <summary>
+		/// This is for internal use to facilitate turning on/off certain logic that makes it easier/harder
+		/// to debug.
+		/// </summary>
+		internal static class DebugOptions
+		{
+			public static bool PreventKeyboardStateTrackerFromResettingOnWindowActivationChange { get; set; }
+
+			public static bool WaitIndefinitelyInEventTester { get; set; }
 		}
 	}
 }

src/Uno.UI/UI/Xaml/Controls/AppBar/AppBar.Partial.cs

@@ -769,7 +769,8 @@ private void OnIsOpenChanged(bool isOpen)
 		{
 			// If the AppBar is not live, then wait until it's loaded before
 			// responding to changes to opened state and firing our Opening/Opened events.
-			if (!IsInLiveTree)
+			// Uno Specific: using IsLoaded instead of IsInLiveTree, which makes more sense because OnOpening (called below) -> SetupOverlayState expects OnApplyTemplate to have already been called
+			if (!IsLoaded)
 			{
 				return;
 			}

src/Uno.UI/UI/Xaml/DependencyObjectStore.cs

@@ -107,8 +107,6 @@ public static class TraceProvider
 		/// </summary>
 		private SpecializedResourceDictionary.ResourceKey? _themeLastUsed;
 
-		private static readonly bool _validatePropertyOwner = Debugger.IsAttached;
-
 #if UNO_HAS_ENHANCED_LIFECYCLE
 		internal bool IsDisposed => _isDisposed;
 #endif
@@ -802,7 +800,7 @@ private void WritePropertyEventTrace(int eventId, DependencyProperty property, D
 		[MethodImpl(MethodImplOptions.AggressiveInlining)]
 		private void ValidatePropertyOwner(DependencyProperty property)
 		{
-			if (_validatePropertyOwner)
+			if (FeatureConfiguration.DependencyProperty.ValidatePropertyOwnerOnReadWrite)
 			{
 				var isFrameworkElement = _originalObjectType.Is(typeof(FrameworkElement));
 				var isMixinFrameworkElement = _originalObjectRef.Target is IFrameworkElement && !isFrameworkElement;

src/Uno.UI/UI/Xaml/UIElementCollection.cs

@@ -57,7 +57,9 @@ public UIElement this[int index]
 
 		public void Add(UIElement item)
 		{
+#if !__CROSSRUNTIME__ // SetParent is already called in AddCore and calling it here messes up the check inside AddCore for a preexisting parent. VerifyNavigationViewItemToolTipPaneDisplayMode (in DEBUG) fails otherwise because Enter is called multiple times on the same element
 			item.SetParent(_owner);
+#endif
 
 			AddCore(item);
 			OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item));

src/Uno.UI/UI/Xaml/Window/Implementations/BaseWindowImplementation.cs

@@ -269,7 +269,10 @@ private void OnActivationStateChanged(CoreWindowActivationState state)
 		CoreWindow?.OnActivated(coreWindowActivatedEventArgs);
 		Activated?.Invoke(Window, activatedEventArgs);
 		SystemThemeHelper.RefreshSystemTheme();
-		KeyboardStateTracker.Reset();
+		if (!FeatureConfiguration.DebugOptions.PreventKeyboardStateTrackerFromResettingOnWindowActivationChange)
+		{
+			KeyboardStateTracker.Reset();
+		}
 	}
 
 	public bool Close()

src/Uno.UWP/Buffers/DefaultArrayPoolBucket.cs

@@ -40,7 +40,13 @@ private sealed class Bucket
 			/// </summary>
 			internal Bucket(int bufferLength, int numberOfBuffers, int poolId)
 			{
-				_lock = new SpinLock(Debugger.IsAttached); // only enable thread tracking if debugger is attached; it adds non-trivial overheads to Enter/Exit
+				_lock = new SpinLock(
+#if DEBUG // thread tracking adds non-trivial overheads to Enter/Exit
+					true
+#else
+					global::System.Diagnostics.Debugger.IsAttached
+#endif
+					);
 				_buffers = new T[numberOfBuffers][];
 				_bufferLength = bufferLength;
 				_poolId = poolId;

src/Uno.UWP/UI/Core/Internal/KeyboardStateTracker.cs

@@ -98,15 +98,7 @@ private static void SetStateOnNonSideKeys(VirtualKey key)
 		}
 	}
 
-	internal static void Reset()
-	{
-		// Clearing state when debugger is attached would
-		// make it hard to debug key events.
-		if (!System.Diagnostics.Debugger.IsAttached)
-		{
-			_keyStates.Clear();
-		}
-	}
+	internal static void Reset() => _keyStates.Clear();
 
 #if __WASM__
 #pragma warning disable IDE0051 // Remove unused private members