From e576ec178c95ccbd8a5772754ad1c30d87465d26 Mon Sep 17 00:00:00 2001 From: Benedikt Schroeder Date: Wed, 6 Jun 2018 21:06:05 +0200 Subject: [PATCH 01/13] Initial --- src/Avalonia.Controls/AppBuilderBase.cs | 45 +++--- src/Avalonia.Controls/Application.cs | 118 ++++++++++++++- src/Avalonia.Controls/ExitMode.cs | 12 ++ src/Avalonia.Controls/Window.cs | 51 ++++--- src/Avalonia.Controls/WindowCollection.cs | 134 ++++++++++++++++++ .../ApplicationTests.cs | 107 ++++++++++++++ .../WindowTests.cs | 10 +- 7 files changed, 428 insertions(+), 49 deletions(-) create mode 100644 src/Avalonia.Controls/ExitMode.cs create mode 100644 src/Avalonia.Controls/WindowCollection.cs create mode 100644 tests/Avalonia.Controls.UnitTests/ApplicationTests.cs diff --git a/src/Avalonia.Controls/AppBuilderBase.cs b/src/Avalonia.Controls/AppBuilderBase.cs index 7af3deef34..875f5263c2 100644 --- a/src/Avalonia.Controls/AppBuilderBase.cs +++ b/src/Avalonia.Controls/AppBuilderBase.cs @@ -15,7 +15,7 @@ namespace Avalonia.Controls public abstract class AppBuilderBase where TAppBuilder : AppBuilderBase, new() { private static bool s_setupWasAlreadyCalled; - + /// /// Gets or sets the instance. /// @@ -92,7 +92,7 @@ namespace Avalonia.Controls }; } - protected TAppBuilder Self => (TAppBuilder) this; + protected TAppBuilder Self => (TAppBuilder)this; /// /// Registers a callback to call before Start is called on the . @@ -125,7 +125,6 @@ namespace Avalonia.Controls var window = new TMainWindow(); if (dataContextProvider != null) window.DataContext = dataContextProvider(); - window.Show(); Instance.Run(window); } @@ -143,7 +142,6 @@ namespace Avalonia.Controls if (dataContextProvider != null) mainWindow.DataContext = dataContextProvider(); - mainWindow.Show(); Instance.Run(mainWindow); } @@ -209,6 +207,17 @@ namespace Avalonia.Controls public TAppBuilder UseAvaloniaModules() => AfterSetup(builder => SetupAvaloniaModules()); + /// + /// Sets the shutdown mode of the application. + /// + /// The shutdown mode. + /// + public TAppBuilder SetExitMode(ExitMode exitMode) + { + Instance.ExitMode = exitMode; + return Self; + } + private bool CheckSetup { get; set; } = true; /// @@ -223,20 +232,20 @@ namespace Avalonia.Controls private void SetupAvaloniaModules() { var moduleInitializers = from assembly in AvaloniaLocator.Current.GetService().GetLoadedAssemblies() - from attribute in assembly.GetCustomAttributes() - where attribute.ForWindowingSubsystem == "" - || attribute.ForWindowingSubsystem == WindowingSubsystemName - where attribute.ForRenderingSubsystem == "" - || attribute.ForRenderingSubsystem == RenderingSubsystemName - group attribute by attribute.Name into exports - select (from export in exports - orderby export.ForWindowingSubsystem.Length descending - orderby export.ForRenderingSubsystem.Length descending - select export).First().ModuleType into moduleType - select (from constructor in moduleType.GetTypeInfo().DeclaredConstructors - where constructor.GetParameters().Length == 0 && !constructor.IsStatic - select constructor).Single() into constructor - select (Action)(() => constructor.Invoke(new object[0])); + from attribute in assembly.GetCustomAttributes() + where attribute.ForWindowingSubsystem == "" + || attribute.ForWindowingSubsystem == WindowingSubsystemName + where attribute.ForRenderingSubsystem == "" + || attribute.ForRenderingSubsystem == RenderingSubsystemName + group attribute by attribute.Name into exports + select (from export in exports + orderby export.ForWindowingSubsystem.Length descending + orderby export.ForRenderingSubsystem.Length descending + select export).First().ModuleType into moduleType + select (from constructor in moduleType.GetTypeInfo().DeclaredConstructors + where constructor.GetParameters().Length == 0 && !constructor.IsStatic + select constructor).Single() into constructor + select (Action)(() => constructor.Invoke(new object[0])); Delegate.Combine(moduleInitializers.ToArray()).DynamicInvoke(); } diff --git a/src/Avalonia.Controls/Application.cs b/src/Avalonia.Controls/Application.cs index 6fdca557eb..ffe4a9c513 100644 --- a/src/Avalonia.Controls/Application.cs +++ b/src/Avalonia.Controls/Application.cs @@ -43,11 +43,15 @@ namespace Avalonia private Styles _styles; private IResourceDictionary _resources; + private CancellationTokenSource _mainLoopCancellationTokenSource; + /// /// Initializes a new instance of the class. /// public Application() { + Windows = new WindowCollection(this); + OnExit += OnExiting; } @@ -158,6 +162,40 @@ namespace Avalonia /// IResourceNode IResourceNode.ResourceParent => null; + /// + /// Gets or sets the . This property indicates whether the application exits explicitly or implicitly. + /// If is set to OnExplicitExit the application is only closes if Exit is called. + /// The default is OnLastWindowClose + /// + /// + /// The shutdown mode. + /// + public ExitMode ExitMode { get; set; } + + /// + /// Gets or sets the main window of the application. + /// + /// + /// The main window. + /// + public Window MainWindow { get; set; } + + /// + /// Gets the open windows of the application. + /// + /// + /// The windows. + /// + public WindowCollection Windows { get; } + + /// + /// Gets or sets a value indicating whether this instance is existing. + /// + /// + /// true if this instance is existing; otherwise, false. + /// + internal bool IsExiting { get; set; } + /// /// Initializes the application by loading XAML etc. /// @@ -171,19 +209,81 @@ namespace Avalonia /// The closable to track public void Run(ICloseable closable) { - var source = new CancellationTokenSource(); - closable.Closed += OnExiting; - closable.Closed += (s, e) => source.Cancel(); - Dispatcher.UIThread.MainLoop(source.Token); + if (_mainLoopCancellationTokenSource != null) + { + throw new Exception("Run should only called once"); + } + + closable.Closed += (s, e) => Exit(); + + _mainLoopCancellationTokenSource = new CancellationTokenSource(); + + Dispatcher.UIThread.MainLoop(_mainLoopCancellationTokenSource.Token); + + // Make sure we call OnExit in case an error happened and Exit() wasn't called explicitly + if (!IsExiting) + { + OnExit?.Invoke(this, EventArgs.Empty); + } + } + + /// + /// Runs the application's main loop until some condition occurs that is specified by ExitMode. + /// + /// The main window + public void Run(Window mainWindow) + { + if (_mainLoopCancellationTokenSource != null) + { + throw new Exception("Run should only called once"); + } + + _mainLoopCancellationTokenSource = new CancellationTokenSource(); + + Dispatcher.UIThread.InvokeAsync( + () => + { + if (mainWindow == null) + { + return; + } + + if (MainWindow != null) + { + return; + } + + if (!mainWindow.IsVisible) + { + mainWindow.Show(); + } + + MainWindow = mainWindow; + }, + DispatcherPriority.Send); + + Dispatcher.UIThread.MainLoop(_mainLoopCancellationTokenSource.Token); + + // Make sure we call OnExit in case an error happened and Exit() wasn't called explicitly + if (!IsExiting) + { + OnExit?.Invoke(this, EventArgs.Empty); + } } - + /// - /// Runs the application's main loop until the is cancelled. + /// Runs the application's main loop until the is canceled. /// /// The token to track public void Run(CancellationToken token) { Dispatcher.UIThread.MainLoop(token); + + // Make sure we call OnExit in case an error happened and Exit() wasn't called explicitly + if (!IsExiting) + { + OnExit?.Invoke(this, EventArgs.Empty); + } } /// @@ -191,7 +291,13 @@ namespace Avalonia /// public void Exit() { + IsExiting = true; + + Windows.Clear(); + OnExit?.Invoke(this, EventArgs.Empty); + + _mainLoopCancellationTokenSource?.Cancel(); } /// diff --git a/src/Avalonia.Controls/ExitMode.cs b/src/Avalonia.Controls/ExitMode.cs new file mode 100644 index 0000000000..0c5ecd7171 --- /dev/null +++ b/src/Avalonia.Controls/ExitMode.cs @@ -0,0 +1,12 @@ +// Copyright (c) The Avalonia Project. All rights reserved. +// Licensed under the MIT license. See licence.md file in the project root for full license information. + +namespace Avalonia +{ + public enum ExitMode + { + OnLastWindowClose, + OnMainWindowClose, + OnExplicitExit + } +} \ No newline at end of file diff --git a/src/Avalonia.Controls/Window.cs b/src/Avalonia.Controls/Window.cs index 3cbfdbd657..c19c69ce73 100644 --- a/src/Avalonia.Controls/Window.cs +++ b/src/Avalonia.Controls/Window.cs @@ -49,14 +49,6 @@ namespace Avalonia.Controls /// public class Window : WindowBase, IStyleable, IFocusScope, ILayoutRoot, INameScope { - private static List s_windows = new List(); - - /// - /// Retrieves an enumeration of all Windows in the currently running application. - /// - public static IReadOnlyList OpenWindows => s_windows; - - /// /// Defines the property. /// public static readonly StyledProperty SizeToContentProperty = @@ -75,7 +67,7 @@ namespace Avalonia.Controls AvaloniaProperty.Register(nameof(ShowInTaskbar), true); /// - /// Enables or disables the taskbar icon + /// Represents the current window state (normal, minimized, maximized) /// public static readonly StyledProperty WindowStateProperty = AvaloniaProperty.Register(nameof(WindowState)); @@ -117,7 +109,7 @@ namespace Avalonia.Controls BackgroundProperty.OverrideDefaultValue(typeof(Window), Brushes.White); TitleProperty.Changed.AddClassHandler((s, e) => s.PlatformImpl?.SetTitle((string)e.NewValue)); HasSystemDecorationsProperty.Changed.AddClassHandler( - (s, e) => s.PlatformImpl?.SetSystemDecorations((bool) e.NewValue)); + (s, e) => s.PlatformImpl?.SetSystemDecorations((bool)e.NewValue)); ShowInTaskbarProperty.Changed.AddClassHandler((w, e) => w.PlatformImpl?.ShowTaskbarIcon((bool)e.NewValue)); @@ -149,7 +141,7 @@ namespace Avalonia.Controls _maxPlatformClientSize = PlatformImpl?.MaxClientSize ?? default(Size); Screens = new Screens(PlatformImpl?.Screen); } - + /// event EventHandler INameScope.Registered { @@ -199,7 +191,7 @@ namespace Avalonia.Controls get { return GetValue(HasSystemDecorationsProperty); } set { SetValue(HasSystemDecorationsProperty, value); } } - + /// /// Enables or disables the taskbar icon /// @@ -259,6 +251,26 @@ namespace Avalonia.Controls /// public event EventHandler Closing; + private static void AddWindow(Window window) + { + if (Application.Current == null) + { + return; + } + + Application.Current.Windows.Add(window); + } + + private static void RemoveWindow(Window window) + { + if (Application.Current == null) + { + return; + } + + Application.Current.Windows.Remove(window); + } + /// /// Closes the window. /// @@ -298,10 +310,9 @@ namespace Avalonia.Controls finally { if (ignoreCancel || !cancelClosing) - { - s_windows.Remove(this); + { PlatformImpl?.Dispose(); - IsVisible = false; + HandleClosed(); } } } @@ -359,7 +370,7 @@ namespace Avalonia.Controls return; } - s_windows.Add(this); + AddWindow(this); EnsureInitialized(); SetWindowStartupLocation(); @@ -400,7 +411,7 @@ namespace Avalonia.Controls throw new InvalidOperationException("The window is already being shown."); } - s_windows.Add(this); + AddWindow(this); EnsureInitialized(); SetWindowStartupLocation(); @@ -409,7 +420,7 @@ namespace Avalonia.Controls using (BeginAutoSizing()) { - var affectedWindows = s_windows.Where(w => w.IsEnabled && w != this).ToList(); + var affectedWindows = Application.Current.Windows.Where(w => w.IsEnabled && w != this).ToList(); var activated = affectedWindows.Where(w => w.IsActive).FirstOrDefault(); SetIsEnabled(affectedWindows, false); @@ -513,8 +524,8 @@ namespace Avalonia.Controls protected override void HandleClosed() { - IsVisible = false; - s_windows.Remove(this); + RemoveWindow(this); + base.HandleClosed(); } diff --git a/src/Avalonia.Controls/WindowCollection.cs b/src/Avalonia.Controls/WindowCollection.cs new file mode 100644 index 0000000000..c21a12f05b --- /dev/null +++ b/src/Avalonia.Controls/WindowCollection.cs @@ -0,0 +1,134 @@ +// Copyright (c) The Avalonia Project. All rights reserved. +// Licensed under the MIT license. See licence.md file in the project root for full license information. + +using System.Collections; +using System.Collections.Generic; + +using Avalonia.Controls; + +namespace Avalonia +{ + public class WindowCollection : IReadOnlyList + { + private readonly Application _application; + private readonly List _windows = new List(); + + public WindowCollection(Application application) + { + _application = application; + } + + /// + /// + /// Gets the number of elements in the collection. + /// + public int Count => _windows.Count; + + /// + /// + /// Gets the at the specified index. + /// + /// + /// The . + /// + /// The index. + /// + public Window this[int index] => _windows[index]; + + /// + /// + /// Returns an enumerator that iterates through the collection. + /// + /// + /// An enumerator that can be used to iterate through the collection. + /// + public IEnumerator GetEnumerator() + { + return _windows.GetEnumerator(); + } + + /// + /// + /// Returns an enumerator that iterates through a collection. + /// + /// + /// An object that can be used to iterate through the collection. + /// + IEnumerator IEnumerable.GetEnumerator() + { + return GetEnumerator(); + } + + /// + /// Adds the specified window. + /// + /// The window. + internal void Add(Window window) + { + if (window == null) + { + return; + } + + _windows.Add(window); + } + + /// + /// Removes the specified window. + /// + /// The window. + internal void Remove(Window window) + { + if (window == null) + { + return; + } + + _windows.Remove(window); + + OnRemoveWindow(window); + } + + /// + /// Closes all windows and removes them from the underlying collection. + /// + internal void Clear() + { + while (_windows.Count > 0) + { + _windows[0].Close(); + } + } + + private void OnRemoveWindow(Window window) + { + if (window == null) + { + return; + } + + if (_application.IsExiting) + { + return; + } + + switch (_application.ExitMode) + { + case ExitMode.OnLastWindowClose: + if (Count == 0) + { + _application.Exit(); + } + + break; + case ExitMode.OnMainWindowClose: + if (window == _application.MainWindow) + { + _application.Exit(); + } + + break; + } + } + } +} \ No newline at end of file diff --git a/tests/Avalonia.Controls.UnitTests/ApplicationTests.cs b/tests/Avalonia.Controls.UnitTests/ApplicationTests.cs new file mode 100644 index 0000000000..85f95b2b5c --- /dev/null +++ b/tests/Avalonia.Controls.UnitTests/ApplicationTests.cs @@ -0,0 +1,107 @@ +// Copyright (c) The Avalonia Project. All rights reserved. +// Licensed under the MIT license. See licence.md file in the project root for full license information. + +using System.Collections.Generic; +using Avalonia.UnitTests; +using Xunit; + +namespace Avalonia.Controls.UnitTests +{ + public class ApplicationTests + { + [Fact] + public void Should_Exit_After_MainWindow_Closed() + { + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + Application.Current.ExitMode = ExitMode.OnMainWindowClose; + + var mainWindow = new Window(); + + mainWindow.Show(); + + Application.Current.MainWindow = mainWindow; + + var window = new Window(); + + window.Show(); + + mainWindow.Close(); + + Assert.True(Application.Current.IsExiting); + } + } + + [Fact] + public void Should_Exit_After_Last_Window_Closed() + { + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + Application.Current.ExitMode = ExitMode.OnLastWindowClose; + + var windowA = new Window(); + + windowA.Show(); + + var windowB = new Window(); + + windowB.Show(); + + windowA.Close(); + + Assert.False(Application.Current.IsExiting); + + windowB.Close(); + + Assert.True(Application.Current.IsExiting); + } + } + + [Fact] + public void Should_Only_Exit_On_Explicit_Exit() + { + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + Application.Current.ExitMode = ExitMode.OnExplicitExit; + + var windowA = new Window(); + + windowA.Show(); + + var windowB = new Window(); + + windowB.Show(); + + windowA.Close(); + + Assert.False(Application.Current.IsExiting); + + windowB.Close(); + + Assert.False(Application.Current.IsExiting); + + Application.Current.Exit(); + + Assert.True(Application.Current.IsExiting); + } + } + + [Fact] + public void Should_Close_All_Remaining_Open_Windows_After_Explicit_Exit_Call() + { + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + var windows = new List { new Window(), new Window(), new Window(), new Window() }; + + foreach (var window in windows) + { + window.Show(); + } + + Application.Current.Exit(); + + Assert.Empty(Application.Current.Windows); + } + } + } +} diff --git a/tests/Avalonia.Controls.UnitTests/WindowTests.cs b/tests/Avalonia.Controls.UnitTests/WindowTests.cs index a85c4df8af..e80ffd97cd 100644 --- a/tests/Avalonia.Controls.UnitTests/WindowTests.cs +++ b/tests/Avalonia.Controls.UnitTests/WindowTests.cs @@ -129,7 +129,7 @@ namespace Avalonia.Controls.UnitTests window.Show(); - Assert.Equal(new[] { window }, Window.OpenWindows); + Assert.Equal(new[] { window }, Application.Current.Windows); } } @@ -145,7 +145,7 @@ namespace Avalonia.Controls.UnitTests window.Show(); window.IsVisible = true; - Assert.Equal(new[] { window }, Window.OpenWindows); + Assert.Equal(new[] { window }, Application.Current.Windows); window.Close(); } @@ -162,7 +162,7 @@ namespace Avalonia.Controls.UnitTests window.Show(); window.Close(); - Assert.Empty(Window.OpenWindows); + Assert.Empty(Application.Current.Windows); } } @@ -184,7 +184,7 @@ namespace Avalonia.Controls.UnitTests window.Show(); windowImpl.Object.Closed(); - Assert.Empty(Window.OpenWindows); + Assert.Empty(Application.Current.Windows); } } @@ -339,7 +339,7 @@ namespace Avalonia.Controls.UnitTests { // HACK: We really need a decent way to have "statics" that can be scoped to // AvaloniaLocator scopes. - ((IList)Window.OpenWindows).Clear(); + Application.Current.Windows.Clear(); } } } From fca102655ea12961e18e03e616adc3bdff6c0817 Mon Sep 17 00:00:00 2001 From: Benedikt Schroeder Date: Sun, 17 Jun 2018 19:34:46 +0200 Subject: [PATCH 02/13] Add ExitMode comments --- src/Avalonia.Controls/ExitMode.cs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/Avalonia.Controls/ExitMode.cs b/src/Avalonia.Controls/ExitMode.cs index 0c5ecd7171..b73fe4a963 100644 --- a/src/Avalonia.Controls/ExitMode.cs +++ b/src/Avalonia.Controls/ExitMode.cs @@ -3,10 +3,24 @@ namespace Avalonia { + /// + /// Enum for ExitMode + /// public enum ExitMode { + /// + /// Indicates an implicit call to Application.Exit when the last window closes. + /// OnLastWindowClose, + + /// + /// Indicates an implicit call to Application.Exit when the main window closes. + /// OnMainWindowClose, + + /// + /// Indicates that the application only exits on an explicit call to Application.Exit. + /// OnExplicitExit } } \ No newline at end of file From 42dadae3ecdfeedb6fdde2d5aea59bd76bd6d3c0 Mon Sep 17 00:00:00 2001 From: Benedikt Schroeder Date: Sun, 17 Jun 2018 22:46:40 +0200 Subject: [PATCH 03/13] Merge fix --- src/Avalonia.Controls/AppBuilderBase.cs | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/Avalonia.Controls/AppBuilderBase.cs b/src/Avalonia.Controls/AppBuilderBase.cs index 814c662341..83763c0836 100644 --- a/src/Avalonia.Controls/AppBuilderBase.cs +++ b/src/Avalonia.Controls/AppBuilderBase.cs @@ -218,17 +218,6 @@ namespace Avalonia.Controls return Self; } - private bool CheckSetup { get; set; } = true; - - /// - /// Set this AppBuilder to ignore the setup check. Used for testing purposes. - /// - internal TAppBuilder IgnoreSetupCheck() - { - CheckSetup = false; - return Self; - } - protected virtual bool CheckSetup => true; private void SetupAvaloniaModules() From 7e8f9fbf619313389065950244924c9e46cf0658 Mon Sep 17 00:00:00 2001 From: Benedikt Schroeder Date: Tue, 19 Jun 2018 21:15:22 +0200 Subject: [PATCH 04/13] Add NullArgumentException for Run with main window --- src/Avalonia.Controls/Application.cs | 8 ++++---- .../Avalonia.Controls.UnitTests/ApplicationTests.cs | 12 +++++++++++- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/Avalonia.Controls/Application.cs b/src/Avalonia.Controls/Application.cs index ffe4a9c513..99575771da 100644 --- a/src/Avalonia.Controls/Application.cs +++ b/src/Avalonia.Controls/Application.cs @@ -243,14 +243,14 @@ namespace Avalonia Dispatcher.UIThread.InvokeAsync( () => { - if (mainWindow == null) + if (MainWindow != null) { return; } - if (MainWindow != null) + if (mainWindow == null) { - return; + throw new ArgumentNullException(nameof(mainWindow)); } if (!mainWindow.IsVisible) @@ -283,7 +283,7 @@ namespace Avalonia if (!IsExiting) { OnExit?.Invoke(this, EventArgs.Empty); - } + } } /// diff --git a/tests/Avalonia.Controls.UnitTests/ApplicationTests.cs b/tests/Avalonia.Controls.UnitTests/ApplicationTests.cs index 85f95b2b5c..694a6d2278 100644 --- a/tests/Avalonia.Controls.UnitTests/ApplicationTests.cs +++ b/tests/Avalonia.Controls.UnitTests/ApplicationTests.cs @@ -1,6 +1,7 @@ // Copyright (c) The Avalonia Project. All rights reserved. // Licensed under the MIT license. See licence.md file in the project root for full license information. +using System; using System.Collections.Generic; using Avalonia.UnitTests; using Xunit; @@ -103,5 +104,14 @@ namespace Avalonia.Controls.UnitTests Assert.Empty(Application.Current.Windows); } } + + [Fact] + public void Throws_ArgumentNullException_On_Run_If_MainWindow_Is_Null() + { + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + Assert.Throws(() => { Application.Current.Run(null); }); + } + } } -} +} \ No newline at end of file From 0214939fd3bf865655b50ab9e0c5e592cfb8ef5c Mon Sep 17 00:00:00 2001 From: Benedikt Schroeder Date: Tue, 19 Jun 2018 21:56:13 +0200 Subject: [PATCH 05/13] Add unit test --- tests/Avalonia.Controls.UnitTests/ApplicationTests.cs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/Avalonia.Controls.UnitTests/ApplicationTests.cs b/tests/Avalonia.Controls.UnitTests/ApplicationTests.cs index 694a6d2278..9602bb0460 100644 --- a/tests/Avalonia.Controls.UnitTests/ApplicationTests.cs +++ b/tests/Avalonia.Controls.UnitTests/ApplicationTests.cs @@ -105,6 +105,17 @@ namespace Avalonia.Controls.UnitTests } } + [Fact] + public void Should_Show_MainWindow_After_Run() + { + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + var mainWindow = new Window(); + Application.Current.Run(mainWindow); + Assert.True(mainWindow.IsVisible); + } + } + [Fact] public void Throws_ArgumentNullException_On_Run_If_MainWindow_Is_Null() { From 90fdfae3bd93667abe9bdc92094688c1a8244029 Mon Sep 17 00:00:00 2001 From: Benedikt Schroeder Date: Wed, 20 Jun 2018 00:01:57 +0200 Subject: [PATCH 06/13] Remove invoke call on Application.Run --- src/Avalonia.Controls/Application.cs | 35 ++++++++----------- .../ApplicationTests.cs | 11 ------ 2 files changed, 14 insertions(+), 32 deletions(-) diff --git a/src/Avalonia.Controls/Application.cs b/src/Avalonia.Controls/Application.cs index 99575771da..de27aa94f8 100644 --- a/src/Avalonia.Controls/Application.cs +++ b/src/Avalonia.Controls/Application.cs @@ -240,27 +240,20 @@ namespace Avalonia _mainLoopCancellationTokenSource = new CancellationTokenSource(); - Dispatcher.UIThread.InvokeAsync( - () => - { - if (MainWindow != null) - { - return; - } - - if (mainWindow == null) - { - throw new ArgumentNullException(nameof(mainWindow)); - } - - if (!mainWindow.IsVisible) - { - mainWindow.Show(); - } - - MainWindow = mainWindow; - }, - DispatcherPriority.Send); + if (MainWindow == null) + { + if (mainWindow == null) + { + throw new ArgumentNullException(nameof(mainWindow)); + } + + if (!mainWindow.IsVisible) + { + mainWindow.Show(); + } + + MainWindow = mainWindow; + } Dispatcher.UIThread.MainLoop(_mainLoopCancellationTokenSource.Token); diff --git a/tests/Avalonia.Controls.UnitTests/ApplicationTests.cs b/tests/Avalonia.Controls.UnitTests/ApplicationTests.cs index 9602bb0460..694a6d2278 100644 --- a/tests/Avalonia.Controls.UnitTests/ApplicationTests.cs +++ b/tests/Avalonia.Controls.UnitTests/ApplicationTests.cs @@ -105,17 +105,6 @@ namespace Avalonia.Controls.UnitTests } } - [Fact] - public void Should_Show_MainWindow_After_Run() - { - using (UnitTestApplication.Start(TestServices.StyledWindow)) - { - var mainWindow = new Window(); - Application.Current.Run(mainWindow); - Assert.True(mainWindow.IsVisible); - } - } - [Fact] public void Throws_ArgumentNullException_On_Run_If_MainWindow_Is_Null() { From 0c078c9dec3086b262dcd1bde391106625e106eb Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 23 Jun 2018 18:23:08 +0200 Subject: [PATCH 07/13] Don't use rx for ExpressionNodes. `ExpressionNode`s were always single-subscriber and making them use `IObservable<>` meant that we had to have extra allocations in order to return `IDisposable`s. Instead of using `IObservable` use a simpler `Subscribe`/`Unsubscribe` pattern. This saves a bunch more memory. --- .../Data/Core/EmptyExpressionNode.cs | 5 - src/Avalonia.Base/Data/Core/ExpressionNode.cs | 120 +++++++++--------- .../Data/Core/ExpressionObserver.cs | 35 ++--- src/Avalonia.Base/Data/Core/IndexerNode.cs | 11 +- .../Plugins/AvaloniaPropertyAccessorPlugin.cs | 10 +- .../Data/Core/Plugins/DataValidatiorBase.cs | 10 +- .../Core/Plugins/ExceptionValidationPlugin.cs | 5 +- .../Data/Core/Plugins/IPropertyAccessor.cs | 14 +- .../Core/Plugins/IndeiValidationPlugin.cs | 19 ++- .../Plugins/InpcPropertyAccessorPlugin.cs | 16 +-- .../Data/Core/Plugins/MethodAccessorPlugin.cs | 8 +- .../Data/Core/Plugins/PropertyAccessorBase.cs | 68 +++++----- .../Data/Core/Plugins/PropertyError.cs | 11 +- .../Data/Core/PropertyAccessorNode.cs | 23 ++-- src/Avalonia.Base/Data/Core/StreamNode.cs | 17 ++- .../Plugins/IndeiValidationPluginTests.cs | 5 +- 16 files changed, 193 insertions(+), 184 deletions(-) diff --git a/src/Avalonia.Base/Data/Core/EmptyExpressionNode.cs b/src/Avalonia.Base/Data/Core/EmptyExpressionNode.cs index 93e0d5947a..c4166b44e5 100644 --- a/src/Avalonia.Base/Data/Core/EmptyExpressionNode.cs +++ b/src/Avalonia.Base/Data/Core/EmptyExpressionNode.cs @@ -9,10 +9,5 @@ namespace Avalonia.Data.Core internal class EmptyExpressionNode : ExpressionNode { public override string Description => "."; - - protected override IObservable StartListeningCore(WeakReference reference) - { - return Observable.Return(reference.Target); - } } } diff --git a/src/Avalonia.Base/Data/Core/ExpressionNode.cs b/src/Avalonia.Base/Data/Core/ExpressionNode.cs index ac7e97a4b1..600cd68d60 100644 --- a/src/Avalonia.Base/Data/Core/ExpressionNode.cs +++ b/src/Avalonia.Base/Data/Core/ExpressionNode.cs @@ -2,22 +2,18 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; -using System.Reactive.Disposables; -using System.Reactive.Linq; -using System.Reactive.Subjects; -using Avalonia.Data; namespace Avalonia.Data.Core { - internal abstract class ExpressionNode : ISubject + internal abstract class ExpressionNode { private static readonly object CacheInvalid = new object(); protected static readonly WeakReference UnsetReference = new WeakReference(AvaloniaProperty.UnsetValue); private WeakReference _target = UnsetReference; - private IDisposable _valueSubscription; - private IObserver _observer; + private Action _subscriber; + private bool _listening; protected WeakReference LastValue { get; private set; } @@ -33,92 +29,66 @@ namespace Avalonia.Data.Core var oldTarget = _target?.Target; var newTarget = value.Target; - var running = _valueSubscription != null; if (!ReferenceEquals(oldTarget, newTarget)) { - _valueSubscription?.Dispose(); - _valueSubscription = null; + if (_listening) + { + StopListening(); + } + _target = value; - if (running) + if (_subscriber != null) { - _valueSubscription = StartListening(); + StartListening(); } } } } - public IDisposable Subscribe(IObserver observer) + public void Subscribe(Action subscriber) { - if (_observer != null) + if (_subscriber != null) { throw new AvaloniaInternalException("ExpressionNode can only be subscribed once."); } - _observer = observer; - var nextSubscription = Next?.Subscribe(this); - _valueSubscription = StartListening(); - - return Disposable.Create(() => - { - _valueSubscription?.Dispose(); - _valueSubscription = null; - LastValue = null; - nextSubscription?.Dispose(); - _observer = null; - }); + _subscriber = subscriber; + Next?.Subscribe(NextValueChanged); + StartListening(); } - void IObserver.OnCompleted() + public void Unsubscribe() { - throw new AvaloniaInternalException("ExpressionNode.OnCompleted should not be called."); - } + Next?.Unsubscribe(); - void IObserver.OnError(Exception error) - { - throw new AvaloniaInternalException("ExpressionNode.OnError should not be called."); + if (_listening) + { + StopListening(); + } + + LastValue = null; + _subscriber = null; } - void IObserver.OnNext(object value) + protected virtual void StartListeningCore(WeakReference reference) { - NextValueChanged(value); + ValueChanged(reference.Target); } - protected virtual IObservable StartListeningCore(WeakReference reference) + protected virtual void StopListeningCore() { - return Observable.Return(reference.Target); } protected virtual void NextValueChanged(object value) { var bindingBroken = BindingNotification.ExtractError(value) as MarkupBindingChainException; bindingBroken?.AddNode(Description); - _observer.OnNext(value); - } - - private IDisposable StartListening() - { - var target = _target.Target; - IObservable source; - - if (target == null) - { - source = Observable.Return(TargetNullNotification()); - } - else if (target == AvaloniaProperty.UnsetValue) - { - source = Observable.Empty(); - } - else - { - source = StartListeningCore(_target); - } - - return source.Subscribe(ValueChanged); + _subscriber(value); } - private void ValueChanged(object value) + protected void ValueChanged(object value) { var notification = value as BindingNotification; @@ -131,24 +101,50 @@ namespace Avalonia.Data.Core } else { - _observer.OnNext(value); + _subscriber(value); } } else { LastValue = new WeakReference(notification.Value); + if (Next != null) { Next.Target = new WeakReference(notification.Value); } - + if (Next == null || notification.Error != null) { - _observer.OnNext(value); + _subscriber(value); } } } + private void StartListening() + { + var target = _target.Target; + + if (target == null) + { + ValueChanged(TargetNullNotification()); + _listening = false; + } + else if (target != AvaloniaProperty.UnsetValue) + { + StartListeningCore(_target); + _listening = true; + } + else + { + _listening = false; + } + } + + private void StopListening() + { + StopListeningCore(); + } + private BindingNotification TargetNullNotification() { return new BindingNotification( diff --git a/src/Avalonia.Base/Data/Core/ExpressionObserver.cs b/src/Avalonia.Base/Data/Core/ExpressionObserver.cs index 3a25407133..3a061206bf 100644 --- a/src/Avalonia.Base/Data/Core/ExpressionObserver.cs +++ b/src/Avalonia.Base/Data/Core/ExpressionObserver.cs @@ -14,9 +14,7 @@ namespace Avalonia.Data.Core /// /// Observes and sets the value of an expression on an object. /// - public class ExpressionObserver : LightweightObservableBase, - IDescription, - IObserver + public class ExpressionObserver : LightweightObservableBase, IDescription { /// /// An ordered collection of property accessor plugins that can be used to customize @@ -55,7 +53,6 @@ namespace Avalonia.Data.Core private static readonly object UninitializedValue = new object(); private readonly ExpressionNode _node; - private IDisposable _nodeSubscription; private object _root; private IDisposable _rootSubscription; private WeakReference _value; @@ -202,34 +199,18 @@ namespace Avalonia.Data.Core } } - void IObserver.OnNext(object value) - { - var broken = BindingNotification.ExtractError(value) as MarkupBindingChainException; - broken?.Commit(Description); - _value = new WeakReference(value); - PublishNext(value); - } - - void IObserver.OnCompleted() - { - } - - void IObserver.OnError(Exception error) - { - } - protected override void Initialize() { _value = null; - _nodeSubscription = _node.Subscribe(this); + _node.Subscribe(ValueChanged); StartRoot(); } protected override void Deinitialize() { _rootSubscription?.Dispose(); - _nodeSubscription?.Dispose(); - _rootSubscription = _nodeSubscription = null; + _rootSubscription = null; + _node.Unsubscribe(); } protected override void Subscribed(IObserver observer, bool first) @@ -266,5 +247,13 @@ namespace Avalonia.Data.Core _node.Target = (WeakReference)_root; } } + + private void ValueChanged(object value) + { + var broken = BindingNotification.ExtractError(value) as MarkupBindingChainException; + broken?.Commit(Description); + _value = new WeakReference(value); + PublishNext(value); + } } } diff --git a/src/Avalonia.Base/Data/Core/IndexerNode.cs b/src/Avalonia.Base/Data/Core/IndexerNode.cs index 633d3558ee..afdfe90c4c 100644 --- a/src/Avalonia.Base/Data/Core/IndexerNode.cs +++ b/src/Avalonia.Base/Data/Core/IndexerNode.cs @@ -17,6 +17,8 @@ namespace Avalonia.Data.Core { internal class IndexerNode : SettableNode { + private IDisposable _subscription; + public IndexerNode(IList arguments) { Arguments = arguments; @@ -24,7 +26,7 @@ namespace Avalonia.Data.Core public override string Description => "[" + string.Join(",", Arguments) + "]"; - protected override IObservable StartListeningCore(WeakReference reference) + protected override void StartListeningCore(WeakReference reference) { var target = reference.Target; var incc = target as INotifyCollectionChanged; @@ -49,7 +51,12 @@ namespace Avalonia.Data.Core .Select(_ => GetValue(target))); } - return Observable.Merge(inputs).StartWith(GetValue(target)); + _subscription = Observable.Merge(inputs).StartWith(GetValue(target)).Subscribe(ValueChanged); + } + + protected override void StopListeningCore() + { + _subscription.Dispose(); } protected override bool SetTargetValueCore(object value, BindingPriority priority) diff --git a/src/Avalonia.Base/Data/Core/Plugins/AvaloniaPropertyAccessorPlugin.cs b/src/Avalonia.Base/Data/Core/Plugins/AvaloniaPropertyAccessorPlugin.cs index 48edb218dc..a163c07f87 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/AvaloniaPropertyAccessorPlugin.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/AvaloniaPropertyAccessorPlugin.cs @@ -145,15 +145,15 @@ namespace Avalonia.Data.Core.Plugins return false; } - protected override void Dispose(bool disposing) + protected override void SubscribeCore() { - _subscription?.Dispose(); - _subscription = null; + _subscription = Instance?.GetObservable(_property).Subscribe(PublishValue); } - protected override void SubscribeCore(IObserver observer) + protected override void UnsubscribeCore() { - _subscription = Instance?.GetObservable(_property).Subscribe(observer); + _subscription?.Dispose(); + _subscription = null; } } } diff --git a/src/Avalonia.Base/Data/Core/Plugins/DataValidatiorBase.cs b/src/Avalonia.Base/Data/Core/Plugins/DataValidatiorBase.cs index bd429f04d6..03ab7712bd 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/DataValidatiorBase.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/DataValidatiorBase.cs @@ -55,13 +55,13 @@ namespace Avalonia.Data.Core.Plugins /// The value. void IObserver.OnNext(object value) => InnerValueChanged(value); - /// - protected override void Dispose(bool disposing) => _inner.Dispose(); - /// /// Begins listening to the inner . /// - protected override void SubscribeCore(IObserver observer) => _inner.Subscribe(this); + protected override void SubscribeCore() => _inner.Subscribe(InnerValueChanged); + + /// + protected override void UnsubscribeCore() => _inner.Dispose(); /// /// Called when the inner notifies with a new value. @@ -74,7 +74,7 @@ namespace Avalonia.Data.Core.Plugins protected virtual void InnerValueChanged(object value) { var notification = value as BindingNotification ?? new BindingNotification(value); - Observer.OnNext(notification); + PublishValue(notification); } } } \ No newline at end of file diff --git a/src/Avalonia.Base/Data/Core/Plugins/ExceptionValidationPlugin.cs b/src/Avalonia.Base/Data/Core/Plugins/ExceptionValidationPlugin.cs index 35f9f7e59a..4507b32e0c 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/ExceptionValidationPlugin.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/ExceptionValidationPlugin.cs @@ -1,7 +1,6 @@ // Copyright (c) The Avalonia Project. All rights reserved. // Licensed under the MIT license. See licence.md file in the project root for full license information. -using Avalonia.Data; using System; using System.Reflection; @@ -36,11 +35,11 @@ namespace Avalonia.Data.Core.Plugins } catch (TargetInvocationException ex) { - Observer.OnNext(new BindingNotification(ex.InnerException, BindingErrorType.DataValidationError)); + PublishValue(new BindingNotification(ex.InnerException, BindingErrorType.DataValidationError)); } catch (Exception ex) { - Observer.OnNext(new BindingNotification(ex, BindingErrorType.DataValidationError)); + PublishValue(new BindingNotification(ex, BindingErrorType.DataValidationError)); } return false; diff --git a/src/Avalonia.Base/Data/Core/Plugins/IPropertyAccessor.cs b/src/Avalonia.Base/Data/Core/Plugins/IPropertyAccessor.cs index d7dda57a72..33ea5bba08 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/IPropertyAccessor.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/IPropertyAccessor.cs @@ -2,7 +2,6 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; -using Avalonia.Data; namespace Avalonia.Data.Core.Plugins { @@ -10,7 +9,7 @@ namespace Avalonia.Data.Core.Plugins /// Defines an accessor to a property on an object returned by a /// /// - public interface IPropertyAccessor : IObservable, IDisposable + public interface IPropertyAccessor : IDisposable { /// /// Gets the type of the property. @@ -38,5 +37,16 @@ namespace Avalonia.Data.Core.Plugins /// True if the property was set; false if the property could not be set. /// bool SetValue(object value, BindingPriority priority); + + /// + /// Subscribes to the value of the member. + /// + /// A method that receives the values. + void Subscribe(Action listener); + + /// + /// Unsubscribes to the value of the member. + /// + void Unsubscribe(); } } diff --git a/src/Avalonia.Base/Data/Core/Plugins/IndeiValidationPlugin.cs b/src/Avalonia.Base/Data/Core/Plugins/IndeiValidationPlugin.cs index 436046f3fa..4d6fc01229 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/IndeiValidationPlugin.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/IndeiValidationPlugin.cs @@ -5,7 +5,6 @@ using System; using System.Collections.Generic; using System.ComponentModel; using System.Linq; -using Avalonia.Data; using Avalonia.Utilities; namespace Avalonia.Data.Core.Plugins @@ -40,43 +39,43 @@ namespace Avalonia.Data.Core.Plugins { if (e.PropertyName == _name || string.IsNullOrEmpty(e.PropertyName)) { - Observer.OnNext(CreateBindingNotification(Value)); + PublishValue(CreateBindingNotification(Value)); } } - protected override void Dispose(bool disposing) + protected override void SubscribeCore() { - base.Dispose(disposing); - var target = _reference.Target as INotifyDataErrorInfo; if (target != null) { - WeakSubscriptionManager.Unsubscribe( + WeakSubscriptionManager.Subscribe( target, nameof(target.ErrorsChanged), this); } + + base.SubscribeCore(); } - protected override void SubscribeCore(IObserver observer) + protected override void UnsubscribeCore() { var target = _reference.Target as INotifyDataErrorInfo; if (target != null) { - WeakSubscriptionManager.Subscribe( + WeakSubscriptionManager.Unsubscribe( target, nameof(target.ErrorsChanged), this); } - base.SubscribeCore(observer); + base.UnsubscribeCore(); } protected override void InnerValueChanged(object value) { - base.InnerValueChanged(CreateBindingNotification(value)); + PublishValue(CreateBindingNotification(value)); } private BindingNotification CreateBindingNotification(object value) diff --git a/src/Avalonia.Base/Data/Core/Plugins/InpcPropertyAccessorPlugin.cs b/src/Avalonia.Base/Data/Core/Plugins/InpcPropertyAccessorPlugin.cs index ba4e60eb74..dab32b639a 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/InpcPropertyAccessorPlugin.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/InpcPropertyAccessorPlugin.cs @@ -103,7 +103,13 @@ namespace Avalonia.Data.Core.Plugins } } - protected override void Dispose(bool disposing) + protected override void SubscribeCore() + { + SendCurrentValue(); + SubscribeToChanges(); + } + + protected override void UnsubscribeCore() { var inpc = _reference.Target as INotifyPropertyChanged; @@ -116,18 +122,12 @@ namespace Avalonia.Data.Core.Plugins } } - protected override void SubscribeCore(IObserver observer) - { - SendCurrentValue(); - SubscribeToChanges(); - } - private void SendCurrentValue() { try { var value = Value; - Observer.OnNext(value); + PublishValue(value); } catch { } } diff --git a/src/Avalonia.Base/Data/Core/Plugins/MethodAccessorPlugin.cs b/src/Avalonia.Base/Data/Core/Plugins/MethodAccessorPlugin.cs index b2b3a107fa..cf0abc6f35 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/MethodAccessorPlugin.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/MethodAccessorPlugin.cs @@ -74,14 +74,18 @@ namespace Avalonia.Data.Core.Plugins public override bool SetValue(object value, BindingPriority priority) => false; - protected override void SubscribeCore(IObserver observer) + protected override void SubscribeCore() { try { - Observer.OnNext(Value); + PublishValue(Value); } catch { } } + + protected override void UnsubscribeCore() + { + } } } } diff --git a/src/Avalonia.Base/Data/Core/Plugins/PropertyAccessorBase.cs b/src/Avalonia.Base/Data/Core/Plugins/PropertyAccessorBase.cs index 9cc78369a7..e840b2c5c9 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/PropertyAccessorBase.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/PropertyAccessorBase.cs @@ -2,67 +2,75 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; -using Avalonia.Data; namespace Avalonia.Data.Core.Plugins { /// /// Defines a default base implementation for a . /// - /// - /// is an observable that will only be subscribed to one time. - /// In addition, the subscription can be disposed by calling on the - /// property accessor itself - this prevents needing to hold two references for a subscription. - /// public abstract class PropertyAccessorBase : IPropertyAccessor { + private Action _listener; + /// public abstract Type PropertyType { get; } /// public abstract object Value { get; } - /// - /// Stops the subscription. - /// - public void Dispose() => Dispose(true); + /// + public void Dispose() + { + if (_listener != null) + { + Unsubscribe(); + } + } /// public abstract bool SetValue(object value, BindingPriority priority); - /// - /// The currently subscribed observer. - /// - protected IObserver Observer { get; private set; } - /// - public IDisposable Subscribe(IObserver observer) + public void Subscribe(Action listener) { - Contract.Requires(observer != null); + Contract.Requires(listener != null); - if (Observer != null) + if (_listener != null) { throw new InvalidOperationException( - "A property accessor can be subscribed to only once."); + "A member accessor can be subscribed to only once."); } - Observer = observer; - SubscribeCore(observer); - return this; + _listener = listener; + SubscribeCore(); } + public void Unsubscribe() + { + if (_listener == null) + { + throw new InvalidOperationException( + "The member accessor was not subscribed."); + } + + UnsubscribeCore(); + _listener = null; + } + + /// + /// Publishes a value to the listener. + /// + /// The value. + protected void PublishValue(object value) => _listener?.Invoke(value); + /// - /// Stops listening to the property. + /// When overridden in a derived class, begins listening to the member. /// - /// - /// True if the method was called, false if the object is being - /// finalized. - /// - protected virtual void Dispose(bool disposing) => Observer = null; + protected abstract void SubscribeCore(); /// - /// When overridden in a derived class, begins listening to the property. + /// When overridden in a derived class, stops listening to the member. /// - protected abstract void SubscribeCore(IObserver observer); + protected abstract void UnsubscribeCore(); } } diff --git a/src/Avalonia.Base/Data/Core/Plugins/PropertyError.cs b/src/Avalonia.Base/Data/Core/Plugins/PropertyError.cs index 647adc36cb..eb2400807a 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/PropertyError.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/PropertyError.cs @@ -1,6 +1,4 @@ using System; -using System.Reactive.Disposables; -using Avalonia.Data; namespace Avalonia.Data.Core.Plugins { @@ -37,10 +35,13 @@ namespace Avalonia.Data.Core.Plugins return false; } - public IDisposable Subscribe(IObserver observer) + public void Subscribe(Action listener) + { + listener(_error); + } + + public void Unsubscribe() { - observer.OnNext(_error); - return Disposable.Empty; } } } diff --git a/src/Avalonia.Base/Data/Core/PropertyAccessorNode.cs b/src/Avalonia.Base/Data/Core/PropertyAccessorNode.cs index 9d657b3144..2565a34322 100644 --- a/src/Avalonia.Base/Data/Core/PropertyAccessorNode.cs +++ b/src/Avalonia.Base/Data/Core/PropertyAccessorNode.cs @@ -3,9 +3,7 @@ using System; using System.Linq; -using System.Reactive.Disposables; using System.Reactive.Linq; -using Avalonia.Data; using Avalonia.Data.Core.Plugins; namespace Avalonia.Data.Core @@ -39,7 +37,7 @@ namespace Avalonia.Data.Core return false; } - protected override IObservable StartListeningCore(WeakReference reference) + protected override void StartListeningCore(WeakReference reference) { var plugin = ExpressionObserver.PropertyAccessors.FirstOrDefault(x => x.Match(reference.Target, PropertyName)); var accessor = plugin?.Start(reference, PropertyName); @@ -55,17 +53,14 @@ namespace Avalonia.Data.Core } } - // Ensure that _accessor is set for the duration of the subscription. - return Observable.Using( - () => - { - _accessor = accessor; - return Disposable.Create(() => - { - _accessor = null; - }); - }, - _ => accessor); + accessor.Subscribe(ValueChanged); + _accessor = accessor; + } + + protected override void StopListeningCore() + { + _accessor.Dispose(); + _accessor = null; } } } diff --git a/src/Avalonia.Base/Data/Core/StreamNode.cs b/src/Avalonia.Base/Data/Core/StreamNode.cs index 187c79af49..415def4d30 100644 --- a/src/Avalonia.Base/Data/Core/StreamNode.cs +++ b/src/Avalonia.Base/Data/Core/StreamNode.cs @@ -2,30 +2,37 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; -using System.Globalization; -using Avalonia.Data; using System.Reactive.Linq; namespace Avalonia.Data.Core { internal class StreamNode : ExpressionNode { + private IDisposable _subscription; + public override string Description => "^"; - protected override IObservable StartListeningCore(WeakReference reference) + protected override void StartListeningCore(WeakReference reference) { foreach (var plugin in ExpressionObserver.StreamHandlers) { if (plugin.Match(reference)) { - return plugin.Start(reference); + _subscription = plugin.Start(reference).Subscribe(ValueChanged); + return; } } // TODO: Improve error. - return Observable.Return(new BindingNotification( + ValueChanged(new BindingNotification( new MarkupBindingChainException("Stream operator applied to unsupported type", Description), BindingErrorType.Error)); } + + protected override void StopListeningCore() + { + _subscription?.Dispose(); + _subscription = null; + } } } diff --git a/tests/Avalonia.Base.UnitTests/Data/Core/Plugins/IndeiValidationPluginTests.cs b/tests/Avalonia.Base.UnitTests/Data/Core/Plugins/IndeiValidationPluginTests.cs index 45c084014b..383030cb6c 100644 --- a/tests/Avalonia.Base.UnitTests/Data/Core/Plugins/IndeiValidationPluginTests.cs +++ b/tests/Avalonia.Base.UnitTests/Data/Core/Plugins/IndeiValidationPluginTests.cs @@ -4,7 +4,6 @@ using System; using System.Collections; using System.Collections.Generic; -using System.Reactive.Linq; using Avalonia.Data; using Avalonia.Data.Core.Plugins; using Xunit; @@ -58,9 +57,9 @@ namespace Avalonia.Base.UnitTests.Data.Core.Plugins var validator = validatorPlugin.Start(new WeakReference(data), nameof(data.Value), accessor); Assert.Equal(0, data.ErrorsChangedSubscriptionCount); - var sub = validator.Subscribe(_ => { }); + validator.Subscribe(_ => { }); Assert.Equal(1, data.ErrorsChangedSubscriptionCount); - sub.Dispose(); + validator.Unsubscribe(); Assert.Equal(0, data.ErrorsChangedSubscriptionCount); } From 6d0e46134919956ba20a115218b5ba7f43fa933c Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Mon, 25 Jun 2018 10:46:50 +0200 Subject: [PATCH 08/13] Throw if no matching property accessor found. This shouldn't happen normally as `InpcPropertyAcessorPlugin` matches everything. --- src/Avalonia.Base/Data/Core/PropertyAccessorNode.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Avalonia.Base/Data/Core/PropertyAccessorNode.cs b/src/Avalonia.Base/Data/Core/PropertyAccessorNode.cs index 2565a34322..e9831eb047 100644 --- a/src/Avalonia.Base/Data/Core/PropertyAccessorNode.cs +++ b/src/Avalonia.Base/Data/Core/PropertyAccessorNode.cs @@ -53,6 +53,12 @@ namespace Avalonia.Data.Core } } + if (accessor == null) + { + throw new NotSupportedException( + $"Could not find a matching property accessor for {PropertyName}."); + } + accessor.Subscribe(ValueChanged); _accessor = accessor; } From 9ccf63d51bd4fe548d920219f256a0df2620766d Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 28 Jun 2018 23:45:10 +0200 Subject: [PATCH 09/13] Add failing test for clearing templated child's parent. --- .../Primitives/TemplatedControlTests.cs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/TemplatedControlTests.cs b/tests/Avalonia.Controls.UnitTests/Primitives/TemplatedControlTests.cs index cd71717619..166586ace1 100644 --- a/tests/Avalonia.Controls.UnitTests/Primitives/TemplatedControlTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Primitives/TemplatedControlTests.cs @@ -160,6 +160,24 @@ namespace Avalonia.Controls.UnitTests.Primitives Assert.Equal(target, child.GetLogicalParent()); } + [Fact] + public void Changing_Template_Should_Clear_Old_Templated_Childs_Parent() + { + var target = new TemplatedControl + { + Template = new FuncControlTemplate(_ => new Decorator()) + }; + + target.ApplyTemplate(); + + var child = (Decorator)target.GetVisualChildren().Single(); + + target.Template = new FuncControlTemplate(_ => new Canvas()); + target.ApplyTemplate(); + + Assert.Null(child.Parent); + } + [Fact] public void Nested_Templated_Control_Should_Not_Have_Template_Applied() { From 95fe6f4cdfa9ca67be64037f956ad0a70c3c567c Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 28 Jun 2018 23:46:03 +0200 Subject: [PATCH 10/13] Clear templated child's parent when template detached. --- src/Avalonia.Controls/Primitives/TemplatedControl.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Avalonia.Controls/Primitives/TemplatedControl.cs b/src/Avalonia.Controls/Primitives/TemplatedControl.cs index 8514104c91..296134ca48 100644 --- a/src/Avalonia.Controls/Primitives/TemplatedControl.cs +++ b/src/Avalonia.Controls/Primitives/TemplatedControl.cs @@ -247,6 +247,7 @@ namespace Avalonia.Controls.Primitives foreach (var child in this.GetTemplateChildren()) { child.SetValue(TemplatedParentProperty, null); + ((ISetLogicalParent)child).SetParent(null); } VisualChildren.Clear(); From 9c1c8749ddd3e11a717901726f7e61119a8c9e64 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 29 Jun 2018 00:50:15 +0200 Subject: [PATCH 11/13] Added failing test for #1709. --- .../ItemsControlTests.cs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/Avalonia.Controls.UnitTests/ItemsControlTests.cs b/tests/Avalonia.Controls.UnitTests/ItemsControlTests.cs index 4da803353e..9ef1e9f0d2 100644 --- a/tests/Avalonia.Controls.UnitTests/ItemsControlTests.cs +++ b/tests/Avalonia.Controls.UnitTests/ItemsControlTests.cs @@ -315,6 +315,26 @@ namespace Avalonia.Controls.UnitTests Assert.Same(before, after); } + [Fact] + public void Should_Clear_Containers_When_ItemsPresenter_Changes() + { + var target = new ItemsControl + { + Items = new[] { "foo", "bar" }, + Template = GetTemplate(), + }; + + target.ApplyTemplate(); + target.Presenter.ApplyTemplate(); + + Assert.Equal(2, target.ItemContainerGenerator.Containers.Count()); + + target.Template = GetTemplate(); + target.ApplyTemplate(); + + Assert.Empty(target.ItemContainerGenerator.Containers); + } + [Fact] public void Empty_Class_Should_Initially_Be_Applied() { From cf14976dcce4ce011a664a83d9ea57184706f183 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 29 Jun 2018 00:50:28 +0200 Subject: [PATCH 12/13] Fix #1709. --- src/Avalonia.Controls/ItemsControl.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Avalonia.Controls/ItemsControl.cs b/src/Avalonia.Controls/ItemsControl.cs index 5119096965..3cb997f615 100644 --- a/src/Avalonia.Controls/ItemsControl.cs +++ b/src/Avalonia.Controls/ItemsControl.cs @@ -155,6 +155,7 @@ namespace Avalonia.Controls void IItemsPresenterHost.RegisterItemsPresenter(IItemsPresenter presenter) { Presenter = presenter; + ItemContainerGenerator.Clear(); } /// From 2f7a578c38eddc108028ea2aff34e7e4e9d68932 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 29 Jun 2018 00:51:03 +0200 Subject: [PATCH 13/13] Added null check for panel. Problem reared its head when #1709 was fixed. --- .../Primitives/SelectingItemsControl.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs index 2e668fda95..a7b8981583 100644 --- a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs +++ b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs @@ -408,12 +408,15 @@ namespace Avalonia.Controls.Primitives var panel = (InputElement)Presenter.Panel; - foreach (var container in e.Containers) + if (panel != null) { - if (KeyboardNavigation.GetTabOnceActiveElement(panel) == container.ContainerControl) + foreach (var container in e.Containers) { - KeyboardNavigation.SetTabOnceActiveElement(panel, null); - break; + if (KeyboardNavigation.GetTabOnceActiveElement(panel) == container.ContainerControl) + { + KeyboardNavigation.SetTabOnceActiveElement(panel, null); + break; + } } } }