Merge branch 'master' into fixes/flaky-tests

src/Avalonia.Base/PropertyStore/BindingEntryBase.cs

@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using System.Reactive.Disposables;
 using Avalonia.Data;
+using Avalonia.Threading;
 
 namespace Avalonia.PropertyStore
 {
@@ -116,26 +117,42 @@ namespace Avalonia.PropertyStore
 
         private void SetValue(BindingValue<TValue> value)
         {
-            if (Frame.Owner is null)
-                return;
+            static void Execute(BindingEntryBase<TValue, TSource> instance, BindingValue<TValue> value)
+            {
+                if (instance.Frame.Owner is null)
+                    return;
 
-            LoggingUtils.LogIfNecessary(Frame.Owner.Owner, Property, value);
+                LoggingUtils.LogIfNecessary(instance.Frame.Owner.Owner, instance.Property, value);
 
-            if (value.HasValue)
-            {
-                if (!_hasValue || !EqualityComparer<TValue>.Default.Equals(_value, value.Value))
+                if (value.HasValue)
+                {
+                    if (!instance._hasValue || !EqualityComparer<TValue>.Default.Equals(instance._value, value.Value))
+                    {
+                        instance._value = value.Value;
+                        instance._hasValue = true;
+                        if (instance._subscription is not null && instance._subscription != s_creatingQuiet)
+                            instance.Frame.Owner?.OnBindingValueChanged(instance, instance.Frame.Priority);
+                    }
+                }
+                else if (value.Type != BindingValueType.DoNothing)
                 {
-                    _value = value.Value;
-                    _hasValue = true;
-                    if (_subscription is not null && _subscription != s_creatingQuiet)
-                        Frame.Owner?.OnBindingValueChanged(this, Frame.Priority);
+                    instance.ClearValue();
+                    if (instance._subscription is not null && instance._subscription != s_creatingQuiet)
+                        instance.Frame.Owner?.OnBindingValueCleared(instance.Property, instance.Frame.Priority);
                 }
             }
-            else if (value.Type != BindingValueType.DoNothing)
+
+            if (Dispatcher.UIThread.CheckAccess())
             {
-                ClearValue();
-                if (_subscription is not null && _subscription != s_creatingQuiet)
-                    Frame.Owner?.OnBindingValueCleared(Property, Frame.Priority);
+                Execute(this, value);
+            }
+            else
+            {
+                // To avoid allocating closure in the outer scope we need to capture variables
+                // locally. This allows us to skip most of the allocations when on UI thread.
+                var instance = this;
+                var newValue = value;
+                Dispatcher.UIThread.Post(() => Execute(instance, newValue));
             }
         }
 

src/Avalonia.Base/PropertyStore/LocalValueBindingObserver.cs

@@ -1,5 +1,6 @@
 using System;
 using Avalonia.Data;
+using Avalonia.Threading;
 
 namespace Avalonia.PropertyStore
 {
@@ -40,20 +41,56 @@ namespace Avalonia.PropertyStore
 
         public void OnNext(T value)
         {
-            if (Property.ValidateValue?.Invoke(value) != false)
-                _owner.SetValue(Property, value, BindingPriority.LocalValue);
+            static void Execute(ValueStore owner, StyledPropertyBase<T> property, T value)
+            {
+                if (property.ValidateValue?.Invoke(value) != false)
+                    owner.SetValue(property, value, BindingPriority.LocalValue);
+                else
+                    owner.ClearLocalValue(property);
+            }
+
+            if (Dispatcher.UIThread.CheckAccess())
+            {
+                Execute(_owner, Property, value);
+            }
             else
-                _owner.ClearLocalValue(Property);
+            {
+                // To avoid allocating closure in the outer scope we need to capture variables
+                // locally. This allows us to skip most of the allocations when on UI thread.
+                var instance = _owner;
+                var property = Property;
+                var newValue = value;
+                Dispatcher.UIThread.Post(() => Execute(instance, property, newValue));
+            }
         }
 
         public void OnNext(BindingValue<T> value)
         {
-            LoggingUtils.LogIfNecessary(_owner.Owner, Property, value);
+            static void Execute(LocalValueBindingObserver<T> instance, BindingValue<T> value)
+            {
+                var owner = instance._owner;
+                var property = instance.Property;
+
+                LoggingUtils.LogIfNecessary(owner.Owner, property, value);
 
-            if (value.HasValue)
-                _owner.SetValue(Property, value.Value, BindingPriority.LocalValue);
-            else if (value.Type != BindingValueType.DataValidationError)
-                _owner.ClearLocalValue(Property);
+                if (value.HasValue)
+                    owner.SetValue(property, value.Value, BindingPriority.LocalValue);
+                else if (value.Type != BindingValueType.DataValidationError)
+                    owner.ClearLocalValue(property);
+            }
+
+            if (Dispatcher.UIThread.CheckAccess())
+            {
+                Execute(this, value);
+            }
+            else
+            {
+                // To avoid allocating closure in the outer scope we need to capture variables
+                // locally. This allows us to skip most of the allocations when on UI thread.
+                var instance = this;
+                var newValue = value;
+                Dispatcher.UIThread.Post(() => Execute(instance, newValue));
+            }
         }
     }
 }

src/Avalonia.Base/PropertyStore/LocalValueUntypedBindingObserver.cs

@@ -1,5 +1,7 @@
 using System;
+using System.Security.Cryptography;
 using Avalonia.Data;
+using Avalonia.Threading;
 
 namespace Avalonia.PropertyStore
 {
@@ -34,28 +36,47 @@ namespace Avalonia.PropertyStore
 
         public void OnNext(object? value)
         {
-            if (value is BindingNotification n)
+            static void Execute(LocalValueUntypedBindingObserver<T> instance, object? value)
             {
-                value = n.Value;
-                LoggingUtils.LogIfNecessary(_owner.Owner, Property, n);
-            }
+                var owner = instance._owner;
+                var property = instance.Property;
 
-            if (value == AvaloniaProperty.UnsetValue)
-            {
-                _owner.ClearLocalValue(Property);
-            }
-            else if (value == BindingOperations.DoNothing)
-            {
-                // Do nothing!
+                if (value is BindingNotification n)
+                {
+                    value = n.Value;
+                    LoggingUtils.LogIfNecessary(owner.Owner, property, n);
+                }
+
+                if (value == AvaloniaProperty.UnsetValue)
+                {
+                    owner.ClearLocalValue(property);
+                }
+                else if (value == BindingOperations.DoNothing)
+                {
+                    // Do nothing!
+                }
+                else if (UntypedValueUtils.TryConvertAndValidate(property, value, out var typedValue))
+                {
+                    owner.SetValue(property, typedValue, BindingPriority.LocalValue);
+                }
+                else
+                {
+                    owner.ClearLocalValue(property);
+                    LoggingUtils.LogInvalidValue(owner.Owner, property, typeof(T), value);
+                }
             }
-            else if (UntypedValueUtils.TryConvertAndValidate(Property, value, out var typedValue))
+
+            if (Dispatcher.UIThread.CheckAccess())
             {
-                _owner.SetValue(Property, typedValue, BindingPriority.LocalValue);
+                Execute(this, value);
             }
-            else
+            else if (value != BindingOperations.DoNothing)
             {
-                _owner.ClearLocalValue(Property);
-                LoggingUtils.LogInvalidValue(_owner.Owner, Property, typeof(T), value);
+                // To avoid allocating closure in the outer scope we need to capture variables
+                // locally. This allows us to skip most of the allocations when on UI thread.
+                var instance = this;
+                var newValue = value;
+                Dispatcher.UIThread.Post(() => Execute(instance, newValue));
             }
         }
     }

src/Avalonia.Controls.DataGrid/DataGridColumn.cs

@@ -50,10 +50,13 @@ namespace Avalonia.Controls
             InheritsWidth = true;
         }
 
-        internal DataGrid OwningGrid
+        /// <summary>
+        /// Gets the <see cref="T:Avalonia.Controls.DataGrid"/> control that contains this column.
+        /// </summary>
+        protected internal DataGrid OwningGrid
         {
             get;
-            set;
+            internal set;
         }
 
         internal int Index

src/Avalonia.Controls.DataGrid/DataGridValueConverter.cs

@@ -22,14 +22,18 @@ namespace Avalonia.Controls
             return DefaultValueConverter.Instance.Convert(value, targetType, parameter, culture);
         }
 
-        // This suppresses a warning saying that we should use String.IsNullOrEmpty instead of a string
-        // comparison, but in this case we want to explicitly check for Empty and not Null.
+
         public object ConvertBack(object value, Type targetType, object parameter, CultureInfo culture)
         {
             if (targetType != null && targetType.IsNullableType())
             {
                 var strValue = value as string;
-                if (string.IsNullOrEmpty(strValue))
+
+                // This suppresses a warning saying that we should use String.IsNullOrEmpty instead of a string
+                // comparison, but in this case we want to explicitly check for Empty and not Null.
+#pragma warning disable CA1820
+                if (strValue == string.Empty)
+#pragma warning restore CA1820
                 {
                     return null;
                 }

src/Avalonia.Controls/NativeMenuBar.cs

@@ -23,15 +23,6 @@ namespace Avalonia.Controls
             });
         }
 
-        [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(NativeMenu))]
-        [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(NativeMenuItem))]
-        [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(NativeMenuItemBase))]
-        [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(NativeMenuItemSeparator))]
-        public NativeMenuBar()
-        {
-
-        }
-
         public static void SetEnableMenuItemClickForwarding(MenuItem menuItem, bool enable)
         {
             menuItem.SetValue(EnableMenuItemClickForwardingProperty, enable);

src/Avalonia.Themes.Fluent/Controls/NativeMenuBar.xaml

@@ -9,17 +9,16 @@
           IsVisible="{Binding !$parent[TopLevel].(NativeMenu.IsNativeMenuExported)}"
           Items="{Binding $parent[TopLevel].(NativeMenu.Menu).Items}">
           <Menu.Styles>
-            <!-- Don't use x:DataType and compiled bindings here, as it might crash https://github.com/AvaloniaUI/Avalonia/pull/7954  -->
-            <Style Selector="MenuItem">
-              <Setter Property="Header" Value="{ReflectionBinding Header}"/>
-              <Setter Property="IsEnabled" Value="{ReflectionBinding IsEnabled}"/>
-              <Setter Property="InputGesture" Value="{ReflectionBinding Gesture}"/>
-              <Setter Property="Items" Value="{ReflectionBinding Menu.Items}"/>
-              <Setter Property="Command" Value="{ReflectionBinding Command}"/>
-              <Setter Property="CommandParameter" Value="{ReflectionBinding CommandParameter}"/>
+            <Style Selector="MenuItem" x:DataType="NativeMenuItem">
+              <Setter Property="Header" Value="{Binding Header}"/>
+              <Setter Property="IsEnabled" Value="{Binding IsEnabled}"/>
+              <Setter Property="InputGesture" Value="{Binding Gesture}"/>
+              <Setter Property="Items" Value="{Binding Menu.Items}"/>
+              <Setter Property="Command" Value="{Binding Command}"/>
+              <Setter Property="CommandParameter" Value="{Binding CommandParameter}"/>
               <Setter Property="(NativeMenuBar.EnableMenuItemClickForwarding)" Value="True"/>
               <!--NativeMenuItem is IBitmap and MenuItem is Image-->
-              <Setter Property="Icon" Value="{ReflectionBinding Icon , Converter={StaticResource AvaloniaThemesFluentNativeMenuBarIBitmapToImageConverter}}"/>
+              <Setter Property="Icon" Value="{Binding Icon , Converter={StaticResource AvaloniaThemesFluentNativeMenuBarIBitmapToImageConverter}}"/>
             </Style>
           </Menu.Styles>
         </Menu>

src/Avalonia.Themes.Simple/Controls/NativeMenuBar.xaml

@@ -9,17 +9,16 @@
         <Menu IsVisible="{Binding !$parent[TopLevel].(NativeMenu.IsNativeMenuExported)}"
               Items="{Binding $parent[TopLevel].(NativeMenu.Menu).Items}">
           <Menu.Styles>
-            <!-- Don't use x:DataType and compiled bindings here, as it might crash https://github.com/AvaloniaUI/Avalonia/pull/7954  -->
-            <Style Selector="MenuItem">
-              <Setter Property="Header" Value="{ReflectionBinding Header}" />
-              <Setter Property="IsEnabled" Value="{ReflectionBinding IsEnabled}" />
-              <Setter Property="InputGesture" Value="{ReflectionBinding Gesture}" />
-              <Setter Property="Items" Value="{ReflectionBinding Menu.Items}" />
-              <Setter Property="Command" Value="{ReflectionBinding Command}" />
-              <Setter Property="CommandParameter" Value="{ReflectionBinding CommandParameter}" />
-              <Setter Property="(NativeMenuBar.EnableMenuItemClickForwarding)" Value="True" />
-              <!--  NativeMenuItem is IBitmap and MenuItem is Image  -->
-              <Setter Property="Icon" Value="{ReflectionBinding Icon, Converter={StaticResource AvaloniaThemesSimpleNativeMenuBarIBitmapToImageConverter}}" />
+            <Style Selector="MenuItem" x:DataType="NativeMenuItem">
+              <Setter Property="Header" Value="{Binding Header}"/>
+              <Setter Property="IsEnabled" Value="{Binding IsEnabled}"/>
+              <Setter Property="InputGesture" Value="{Binding Gesture}"/>
+              <Setter Property="Items" Value="{Binding Menu.Items}"/>
+              <Setter Property="Command" Value="{Binding Command}"/>
+              <Setter Property="CommandParameter" Value="{Binding CommandParameter}"/>
+              <Setter Property="(NativeMenuBar.EnableMenuItemClickForwarding)" Value="True"/>
+              <!--NativeMenuItem is IBitmap and MenuItem is Image-->
+              <Setter Property="Icon" Value="{Binding Icon , Converter={StaticResource AvaloniaThemesSimpleNativeMenuBarIBitmapToImageConverter}}"/>
             </Style>
           </Menu.Styles>
         </Menu>

src/Windows/Avalonia.Win32/Interop/UnmanagedMethods.cs

@@ -1409,7 +1409,7 @@ namespace Avalonia.Win32.Interop
         [DllImport("user32.dll")]
         public static extern bool TranslateMessage(ref MSG lpMsg);
 
-        [DllImport("user32.dll")]
+        [DllImport("user32.dll", CharSet = CharSet.Unicode)]
         public static extern bool UnregisterClass(string lpClassName, IntPtr hInstance);
 
         [DllImport("user32.dll", SetLastError = true, CharSet = CharSet.Unicode, EntryPoint = "SetWindowTextW")]

src/Windows/Avalonia.Win32/WindowImpl.AppWndProc.cs

@@ -10,6 +10,7 @@ using Avalonia.Controls.Remote;
 using Avalonia.Input;
 using Avalonia.Input.Raw;
 using Avalonia.Platform;
+using Avalonia.Threading;
 using Avalonia.Win32.Automation;
 using Avalonia.Win32.Input;
 using Avalonia.Win32.Interop.Automation;
@@ -106,6 +107,9 @@ namespace Avalonia.Win32
                         _touchDevice?.Dispose();
                         //Free other resources
                         Dispose();
+
+                        // Schedule cleanup of anything that requires window to be destroyed
+                        Dispatcher.UIThread.Post(AfterCloseCleanup);
                         return IntPtr.Zero;
                     }
 

src/Windows/Avalonia.Win32/WindowImpl.cs

@@ -643,12 +643,6 @@ namespace Avalonia.Win32
                 _hwnd = IntPtr.Zero;
             }
 
-            if (_className != null)
-            {
-                UnregisterClass(_className, GetModuleHandle(null));
-                _className = null;
-            }
-
             _framebuffer.Dispose();
         }
 
@@ -1144,6 +1138,15 @@ namespace Avalonia.Win32
             }
         }
 
+        private void AfterCloseCleanup()
+        {
+            if (_className != null)
+            {
+                UnregisterClass(_className, GetModuleHandle(null));
+                _className = null;
+            }
+        }
+
         private void MaximizeWithoutCoveringTaskbar()
         {
             IntPtr monitor = MonitorFromWindow(_hwnd, MONITOR.MONITOR_DEFAULTTONEAREST);

tests/Avalonia.Base.UnitTests/Avalonia.Base.UnitTests.csproj

@@ -17,6 +17,9 @@
     <EmbeddedResource Remove="..\Avalonia.RenderTests\Assets\NotoColorEmoji.ttf" />
     <EmbeddedResource Include="Media\TextFormatting\BreakPairTable.txt" />
   </ItemGroup>
+  <ItemGroup>
+    <PackageReference Include="Nito.AsyncEx.Context" Version="5.1.2" />
+  </ItemGroup>
   <ItemGroup>
     <ProjectReference Include="..\..\src\Avalonia.Base\Avalonia.Base.csproj" />
     <ProjectReference Include="..\..\src\Markup\Avalonia.Markup.Xaml.Loader\Avalonia.Markup.Xaml.Loader.csproj" />

tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Binding.cs

@@ -12,6 +12,7 @@ using Avalonia.Platform;
 using Avalonia.Threading;
 using Avalonia.UnitTests;
 using Moq;
+using Nito.AsyncEx;
 using Xunit;
 
 #nullable enable
@@ -920,6 +921,120 @@ namespace Avalonia.Base.UnitTests
             }
         }
 
+        [Theory]
+        [InlineData(BindingPriority.LocalValue)]
+        [InlineData(BindingPriority.Style)]
+        public void Typed_Bind_Executes_On_UIThread(BindingPriority priority)
+        {
+            AsyncContext.Run(async () =>
+            {
+                var target = new Class1();
+                var source = new Subject<string>();
+                var currentThreadId = Thread.CurrentThread.ManagedThreadId;
+                var raised = 0;
+
+                var threadingInterfaceMock = new Mock<IPlatformThreadingInterface>();
+                threadingInterfaceMock.SetupGet(mock => mock.CurrentThreadIsLoopThread)
+                    .Returns(() => Thread.CurrentThread.ManagedThreadId == currentThreadId);
+
+                var services = new TestServices(
+                    threadingInterface: threadingInterfaceMock.Object);
+
+                target.PropertyChanged += (s, e) =>
+                {
+                    Assert.Equal(currentThreadId, Thread.CurrentThread.ManagedThreadId);
+                    ++raised;
+                };
+
+                using (UnitTestApplication.Start(services))
+                {
+                    target.Bind(Class1.FooProperty, source, priority);
+
+                    await Task.Run(() => source.OnNext("foobar"));
+                    Dispatcher.UIThread.RunJobs();
+
+                    Assert.Equal("foobar", target.GetValue(Class1.FooProperty));
+                    Assert.Equal(1, raised);
+                }
+            });
+        }
+
+        [Theory]
+        [InlineData(BindingPriority.LocalValue)]
+        [InlineData(BindingPriority.Style)]
+        public void Untyped_Bind_Executes_On_UIThread(BindingPriority priority)
+        {
+            AsyncContext.Run(async () =>
+            {
+                var target = new Class1();
+                var source = new Subject<object>();
+                var currentThreadId = Thread.CurrentThread.ManagedThreadId;
+                var raised = 0;
+
+                var threadingInterfaceMock = new Mock<IPlatformThreadingInterface>();
+                threadingInterfaceMock.SetupGet(mock => mock.CurrentThreadIsLoopThread)
+                    .Returns(() => Thread.CurrentThread.ManagedThreadId == currentThreadId);
+
+                var services = new TestServices(
+                    threadingInterface: threadingInterfaceMock.Object);
+
+                target.PropertyChanged += (s, e) =>
+                {
+                    Assert.Equal(currentThreadId, Thread.CurrentThread.ManagedThreadId);
+                    ++raised;
+                };
+
+                using (UnitTestApplication.Start(services))
+                {
+                    target.Bind(Class1.FooProperty, source, priority);
+
+                    await Task.Run(() => source.OnNext("foobar"));
+                    Dispatcher.UIThread.RunJobs();
+
+                    Assert.Equal("foobar", target.GetValue(Class1.FooProperty));
+                    Assert.Equal(1, raised);
+                }
+            });
+        }
+
+        [Theory]
+        [InlineData(BindingPriority.LocalValue)]
+        [InlineData(BindingPriority.Style)]
+        public void BindingValue_Bind_Executes_On_UIThread(BindingPriority priority)
+        {
+            AsyncContext.Run(async () =>
+            {
+                var target = new Class1();
+                var source = new Subject<BindingValue<string>>();
+                var currentThreadId = Thread.CurrentThread.ManagedThreadId;
+                var raised = 0;
+
+                var threadingInterfaceMock = new Mock<IPlatformThreadingInterface>();
+                threadingInterfaceMock.SetupGet(mock => mock.CurrentThreadIsLoopThread)
+                    .Returns(() => Thread.CurrentThread.ManagedThreadId == currentThreadId);
+
+                var services = new TestServices(
+                    threadingInterface: threadingInterfaceMock.Object);
+
+                target.PropertyChanged += (s, e) =>
+                {
+                    Assert.Equal(currentThreadId, Thread.CurrentThread.ManagedThreadId);
+                    ++raised;
+                };
+
+                using (UnitTestApplication.Start(services))
+                {
+                    target.Bind(Class1.FooProperty, source, priority);
+
+                    await Task.Run(() => source.OnNext("foobar"));
+                    Dispatcher.UIThread.RunJobs();
+
+                    Assert.Equal("foobar", target.GetValue(Class1.FooProperty));
+                    Assert.Equal(1, raised);
+                }
+            });
+        }
+
         [Fact]
         public async Task Bind_With_Scheduler_Executes_On_Scheduler()
         {

tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Direct.cs

@@ -7,8 +7,10 @@ using System.Threading.Tasks;
 using Avalonia.Data;
 using Avalonia.Logging;
 using Avalonia.Platform;
+using Avalonia.Threading;
 using Avalonia.UnitTests;
 using Moq;
+using Nito.AsyncEx;
 using Xunit;
 
 namespace Avalonia.Base.UnitTests
@@ -519,25 +521,39 @@ namespace Avalonia.Base.UnitTests
         }
 
         [Fact]
-        public async Task Bind_Executes_On_UIThread()
+        public void Bind_Executes_On_UIThread()
         {
-            var target = new Class1();
-            var source = new Subject<object>();
-            var currentThreadId = Thread.CurrentThread.ManagedThreadId;
+            AsyncContext.Run(async () =>
+            {
+                var target = new Class1();
+                var source = new Subject<object>();
+                var currentThreadId = Thread.CurrentThread.ManagedThreadId;
+                var raised = 0;
 
-            var threadingInterfaceMock = new Mock<IPlatformThreadingInterface>();
-            threadingInterfaceMock.SetupGet(mock => mock.CurrentThreadIsLoopThread)
-                .Returns(() => Thread.CurrentThread.ManagedThreadId == currentThreadId);
+                var threadingInterfaceMock = new Mock<IPlatformThreadingInterface>();
+                threadingInterfaceMock.SetupGet(mock => mock.CurrentThreadIsLoopThread)
+                    .Returns(() => Thread.CurrentThread.ManagedThreadId == currentThreadId);
 
-            var services = new TestServices(
-                threadingInterface: threadingInterfaceMock.Object);
+                var services = new TestServices(
+                    threadingInterface: threadingInterfaceMock.Object);
 
-            using (UnitTestApplication.Start(services))
-            {
-                target.Bind(Class1.FooProperty, source);
+                target.PropertyChanged += (s, e) =>
+                {
+                    Assert.Equal(currentThreadId, Thread.CurrentThread.ManagedThreadId);
+                    ++raised;
+                };
 
-                await Task.Run(() => source.OnNext("foobar"));
-            }
+                using (UnitTestApplication.Start(services))
+                {
+                    target.Bind(Class1.FooProperty, source);
+
+                    await Task.Run(() => source.OnNext("foobar"));
+                    Dispatcher.UIThread.RunJobs();
+
+                    Assert.Equal("foobar", target.Foo);
+                    Assert.Equal(1, raised);
+                }
+            });
         }
 
         [Fact]