From 4057670f43e2f31ebcb8d3301c325d6f31908bf5 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sun, 6 Mar 2016 16:20:32 +0100 Subject: [PATCH] Ensure that PerspexObject bindings are unsubbed. --- .../Plugins/PerspexPropertyAccessorPlugin.cs | 2 -- src/Perspex.Base/Collections/InccDebug.cs | 12 --------- src/Perspex.Base/Collections/PerspexList.cs | 5 ++-- .../INotifyCollectionChangedDebug.cs | 25 +++++++++++++++++++ .../Diagnostics/IPerspexObjectDebug.cs | 22 ++++++++++++++++ src/Perspex.Base/Perspex.Base.csproj | 5 +++- src/Perspex.Base/PerspexObject.cs | 21 +++++++++++++--- .../Reactive/WeakPropertyChangedObservable.cs | 2 +- src/Perspex.Controls/Control.cs | 3 ++- tests/Perspex.LeakTests/ControlTests.cs | 5 ++-- ...ExpressionObserverTests_PerspexProperty.cs | 5 ++++ .../SelectorTests_Template.cs | 3 ++- 12 files changed, 85 insertions(+), 25 deletions(-) delete mode 100644 src/Perspex.Base/Collections/InccDebug.cs create mode 100644 src/Perspex.Base/Diagnostics/INotifyCollectionChangedDebug.cs create mode 100644 src/Perspex.Base/Diagnostics/IPerspexObjectDebug.cs diff --git a/src/Markup/Perspex.Markup/Data/Plugins/PerspexPropertyAccessorPlugin.cs b/src/Markup/Perspex.Markup/Data/Plugins/PerspexPropertyAccessorPlugin.cs index 61f05ce4f2..9f50359570 100644 --- a/src/Markup/Perspex.Markup/Data/Plugins/PerspexPropertyAccessorPlugin.cs +++ b/src/Markup/Perspex.Markup/Data/Plugins/PerspexPropertyAccessorPlugin.cs @@ -2,9 +2,7 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; -using System.ComponentModel; using System.Reactive.Linq; -using System.Reflection; namespace Perspex.Markup.Data.Plugins { diff --git a/src/Perspex.Base/Collections/InccDebug.cs b/src/Perspex.Base/Collections/InccDebug.cs deleted file mode 100644 index a5cee01a74..0000000000 --- a/src/Perspex.Base/Collections/InccDebug.cs +++ /dev/null @@ -1,12 +0,0 @@ -// Copyright (c) The Perspex Project. All rights reserved. -// Licensed under the MIT license. See licence.md file in the project root for full license information. - -using System; - -namespace Perspex.Collections -{ - public interface InccDebug - { - Delegate[] GetCollectionChangedSubscribers(); - } -} diff --git a/src/Perspex.Base/Collections/PerspexList.cs b/src/Perspex.Base/Collections/PerspexList.cs index 8a5c209c72..82f654a4e0 100644 --- a/src/Perspex.Base/Collections/PerspexList.cs +++ b/src/Perspex.Base/Collections/PerspexList.cs @@ -7,6 +7,7 @@ using System.Collections.Generic; using System.Collections.Specialized; using System.ComponentModel; using System.Linq; +using Perspex.Diagnostics; using Perspex.Platform; namespace Perspex.Collections @@ -53,7 +54,7 @@ namespace Perspex.Collections /// /// /// - public class PerspexList : IPerspexList, IList, InccDebug + public class PerspexList : IPerspexList, IList, INotifyCollectionChangedDebug { private List _inner; private NotifyCollectionChangedEventHandler _collectionChanged; @@ -435,7 +436,7 @@ namespace Perspex.Collections } /// - Delegate[] InccDebug.GetCollectionChangedSubscribers() => _collectionChanged?.GetInvocationList(); + Delegate[] INotifyCollectionChangedDebug.GetCollectionChangedSubscribers() => _collectionChanged?.GetInvocationList(); /// /// Raises the event with an add action. diff --git a/src/Perspex.Base/Diagnostics/INotifyCollectionChangedDebug.cs b/src/Perspex.Base/Diagnostics/INotifyCollectionChangedDebug.cs new file mode 100644 index 0000000000..468c2b3ff7 --- /dev/null +++ b/src/Perspex.Base/Diagnostics/INotifyCollectionChangedDebug.cs @@ -0,0 +1,25 @@ +// Copyright (c) The Perspex 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.Specialized; +using Perspex.Collections; + +namespace Perspex.Diagnostics +{ + /// + /// Provides a debug interface into subscribers on + /// + /// + public interface INotifyCollectionChangedDebug + { + /// + /// Gets the subscriber list for the + /// event. + /// + /// + /// The subscribers or null if no subscribers. + /// + Delegate[] GetCollectionChangedSubscribers(); + } +} diff --git a/src/Perspex.Base/Diagnostics/IPerspexObjectDebug.cs b/src/Perspex.Base/Diagnostics/IPerspexObjectDebug.cs new file mode 100644 index 0000000000..1d2778dc30 --- /dev/null +++ b/src/Perspex.Base/Diagnostics/IPerspexObjectDebug.cs @@ -0,0 +1,22 @@ +// Copyright (c) The Perspex Project. All rights reserved. +// Licensed under the MIT license. See licence.md file in the project root for full license information. + +using System; + +namespace Perspex.Diagnostics +{ + /// + /// Provides a debug interface into . + /// + public interface IPerspexObjectDebug + { + /// + /// Gets the subscriber list for the + /// event. + /// + /// + /// The subscribers or null if no subscribers. + /// + Delegate[] GetPropertyChangedSubscribers(); + } +} diff --git a/src/Perspex.Base/Perspex.Base.csproj b/src/Perspex.Base/Perspex.Base.csproj index 9a6acd4048..7420fef3af 100644 --- a/src/Perspex.Base/Perspex.Base.csproj +++ b/src/Perspex.Base/Perspex.Base.csproj @@ -43,7 +43,7 @@ Properties\SharedAssemblyInfo.cs - + @@ -51,6 +51,7 @@ + @@ -68,6 +69,7 @@ + @@ -104,6 +106,7 @@ + diff --git a/src/Perspex.Base/PerspexObject.cs b/src/Perspex.Base/PerspexObject.cs index 16819428ca..f35bb21b3f 100644 --- a/src/Perspex.Base/PerspexObject.cs +++ b/src/Perspex.Base/PerspexObject.cs @@ -9,6 +9,7 @@ using System.Reactive.Disposables; using System.Reactive.Linq; using System.Reactive.Subjects; using Perspex.Data; +using Perspex.Diagnostics; using Perspex.Threading; using Perspex.Utilities; using Serilog; @@ -22,7 +23,7 @@ namespace Perspex /// /// This class is analogous to DependencyObject in WPF. /// - public class PerspexObject : IPerspexObject, INotifyPropertyChanged + public class PerspexObject : IPerspexObject, IPerspexObjectDebug, INotifyPropertyChanged { /// /// The parent object that inherited values are inherited from. @@ -40,6 +41,11 @@ namespace Perspex /// private PropertyChangedEventHandler _inpcChanged; + /// + /// Event handler for implementation. + /// + private EventHandler _propertyChanged; + /// /// A serilog logger for logging property events. /// @@ -77,7 +83,11 @@ namespace Perspex /// /// Raised when a value changes on this object. /// - public event EventHandler PropertyChanged; + public event EventHandler PropertyChanged + { + add { _propertyChanged += value; } + remove { _propertyChanged -= value; } + } /// /// Raised when a value changes on this object. @@ -456,6 +466,11 @@ namespace Perspex } /// + Delegate[] IPerspexObjectDebug.GetPropertyChangedSubscribers() + { + return _propertyChanged?.GetInvocationList(); + } + /// /// Gets all priority values set on the object. /// @@ -519,7 +534,7 @@ namespace Perspex OnPropertyChanged(e); property.NotifyChanged(e); - PropertyChanged?.Invoke(this, e); + _propertyChanged?.Invoke(this, e); if (_inpcChanged != null) { diff --git a/src/Perspex.Base/Reactive/WeakPropertyChangedObservable.cs b/src/Perspex.Base/Reactive/WeakPropertyChangedObservable.cs index 117fcc65ad..63b1df5fb2 100644 --- a/src/Perspex.Base/Reactive/WeakPropertyChangedObservable.cs +++ b/src/Perspex.Base/Reactive/WeakPropertyChangedObservable.cs @@ -45,7 +45,7 @@ namespace Perspex.Reactive if (_sourceReference.TryGetTarget(out instance)) { - if (_count == 0) + if (_count++ == 0) { WeakSubscriptionManager.Subscribe( instance, diff --git a/src/Perspex.Controls/Control.cs b/src/Perspex.Controls/Control.cs index eb096e2f0f..460b0a5c8a 100644 --- a/src/Perspex.Controls/Control.cs +++ b/src/Perspex.Controls/Control.cs @@ -12,6 +12,7 @@ using Perspex.Collections; using Perspex.Controls.Primitives; using Perspex.Controls.Templates; using Perspex.Data; +using Perspex.Diagnostics; using Perspex.Input; using Perspex.Interactivity; using Perspex.LogicalTree; @@ -542,7 +543,7 @@ namespace Perspex.Controls child.OnDetachedFromLogicalTree(e); } - if (((InccDebug)_classes).GetCollectionChangedSubscribers()?.Length > 0) + if (((INotifyCollectionChangedDebug)_classes).GetCollectionChangedSubscribers()?.Length > 0) { // TODO: This should be output using a standard logging mechanism. System.Diagnostics.Debug.WriteLine( diff --git a/tests/Perspex.LeakTests/ControlTests.cs b/tests/Perspex.LeakTests/ControlTests.cs index ffc8bc128e..5b6e0d1617 100644 --- a/tests/Perspex.LeakTests/ControlTests.cs +++ b/tests/Perspex.LeakTests/ControlTests.cs @@ -9,6 +9,7 @@ using Perspex.Collections; using Perspex.Controls; using Perspex.Controls.Primitives; using Perspex.Controls.Templates; +using Perspex.Diagnostics; using Perspex.Layout; using Perspex.Styling; using Perspex.UnitTests; @@ -230,7 +231,7 @@ namespace Perspex.LeakTests // The TextBox should have subscriptions to its Classes collection from the // default theme. - Assert.NotEmpty(((InccDebug)textBox.Classes).GetCollectionChangedSubscribers()); + Assert.NotEmpty(((INotifyCollectionChangedDebug)textBox.Classes).GetCollectionChangedSubscribers()); // Clear the content and ensure the TextBox is removed. window.Content = null; @@ -238,7 +239,7 @@ namespace Perspex.LeakTests Assert.Null(window.Presenter.Child); // Check that the TextBox has no subscriptions to its Classes collection. - Assert.Null(((InccDebug)textBox.Classes).GetCollectionChangedSubscribers()); + Assert.Null(((INotifyCollectionChangedDebug)textBox.Classes).GetCollectionChangedSubscribers()); } } diff --git a/tests/Perspex.Markup.UnitTests/Data/ExpressionObserverTests_PerspexProperty.cs b/tests/Perspex.Markup.UnitTests/Data/ExpressionObserverTests_PerspexProperty.cs index 05093d81be..d12f5c4493 100644 --- a/tests/Perspex.Markup.UnitTests/Data/ExpressionObserverTests_PerspexProperty.cs +++ b/tests/Perspex.Markup.UnitTests/Data/ExpressionObserverTests_PerspexProperty.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Reactive.Linq; +using Perspex.Diagnostics; using Perspex.Markup.Data; using Xunit; @@ -24,6 +25,8 @@ namespace Perspex.Markup.UnitTests.Data var result = await target.Take(1); Assert.Equal("foo", result); + + Assert.Null(((IPerspexObjectDebug)data).GetPropertyChangedSubscribers()); } [Fact] @@ -39,6 +42,8 @@ namespace Perspex.Markup.UnitTests.Data Assert.Equal(new[] { "foo", "bar" }, result); sub.Dispose(); + + Assert.Null(((IPerspexObjectDebug)data).GetPropertyChangedSubscribers()); } [Fact] diff --git a/tests/Perspex.Styling.UnitTests/SelectorTests_Template.cs b/tests/Perspex.Styling.UnitTests/SelectorTests_Template.cs index f4a26a8e90..4b47f53a38 100644 --- a/tests/Perspex.Styling.UnitTests/SelectorTests_Template.cs +++ b/tests/Perspex.Styling.UnitTests/SelectorTests_Template.cs @@ -15,6 +15,7 @@ namespace Perspex.Styling.UnitTests using System.Reactive; using System.Reactive.Subjects; using Collections; + using Diagnostics; using Controls = Controls.Controls; public class SelectorTests_Template @@ -124,7 +125,7 @@ namespace Perspex.Styling.UnitTests var border = (Border)target.Object.VisualChildren.Single(); var selector = new Selector().OfType(templatedControl.Object.GetType()).Class("foo").Template().OfType(); var activator = selector.Match(border).ObservableResult; - var inccDebug = (InccDebug)styleable.Object.Classes; + var inccDebug = (INotifyCollectionChangedDebug)styleable.Object.Classes; using (activator.Subscribe(_ => { })) {