Started implementing weak bindings.

Tests currently failing as WeakSubscriptionManager doesn't support
unsubscribing yet.

src/Markup/Perspex.Markup/Data/ExpressionNode.cs

@@ -8,46 +8,55 @@ namespace Perspex.Markup.Data
 {
     internal abstract class ExpressionNode : IObservable<object>
     {
-        private object _target;
+        protected static readonly WeakReference UnsetReference = 
+            new WeakReference(PerspexProperty.UnsetValue);
+
+        private WeakReference _target;
 
         private Subject<object> _subject;
 
-        private object _value = PerspexProperty.UnsetValue;
+        private WeakReference _value = UnsetReference;
 
         public ExpressionNode Next { get; set; }
 
-        public object Target
+        public WeakReference Target
         {
-            get { return _target; }
+            get
+            {
+                return _target;
+            }
             set
             {
-                if (!object.Equals(value, _target))
+                var newInstance = value?.Target;
+                var oldInstance = _target?.Target;
+
+                if (!object.Equals(oldInstance, newInstance))
                 {
-                    if (_target != null)
+                    if (oldInstance != null)
                     {
-                        Unsubscribe(_target);
+                        Unsubscribe(oldInstance);
                     }
 
                     _target = value;
 
-                    if (_target != null)
+                    if (newInstance != null)
                     {
                         SubscribeAndUpdate(_target);
                     }
                     else
                     {
-                        CurrentValue = PerspexProperty.UnsetValue;
+                        CurrentValue = UnsetReference;
                     }
 
                     if (Next != null)
                     {
-                        Next.Target = CurrentValue;
+                        Next.Target = _value;
                     }
                 }
             }
         }
 
-        public object CurrentValue
+        public WeakReference CurrentValue
         {
             get
             {
@@ -63,7 +72,7 @@ namespace Perspex.Markup.Data
                     Next.Target = value;
                 }
 
-                _subject?.OnNext(value);
+                _subject?.OnNext(value.Target);
             }
         }
 
@@ -85,14 +94,14 @@ namespace Perspex.Markup.Data
                     _subject = new Subject<object>();
                 }
 
-                observer.OnNext(CurrentValue);
+                observer.OnNext(CurrentValue.Target);
                 return _subject.Subscribe(observer);
             }
         }
 
-        protected virtual void SubscribeAndUpdate(object target)
+        protected virtual void SubscribeAndUpdate(WeakReference reference)
         {
-            CurrentValue = target;
+            CurrentValue = reference;
         }
 
         protected virtual void Unsubscribe(object target)

src/Markup/Perspex.Markup/Data/ExpressionObserver.cs

@@ -27,7 +27,7 @@ namespace Perspex.Markup.Data
                 new InpcPropertyAccessorPlugin(),
             };
 
-        private readonly object _root;
+        private readonly WeakReference _root;
         private readonly Func<object> _rootGetter;
         private readonly IObservable<object> _rootObservable;
         private readonly IObservable<Unit> _update;
@@ -45,7 +45,7 @@ namespace Perspex.Markup.Data
         {
             Contract.Requires<ArgumentNullException>(expression != null);
 
-            _root = root;
+            _root = new WeakReference(root);
 
             if (!string.IsNullOrWhiteSpace(expression))
             {
@@ -115,7 +115,7 @@ namespace Perspex.Markup.Data
 
             if (_rootGetter != null && _node != null)
             {
-                _node.Target = _rootGetter();
+                _node.Target = new WeakReference(_rootGetter());
             }
 
             try
@@ -155,7 +155,7 @@ namespace Perspex.Markup.Data
                     }
                     else
                     {
-                        return _root?.GetType();
+                        return _root.Target?.GetType();
                     }
                 }
                 finally
@@ -220,7 +220,9 @@ namespace Perspex.Markup.Data
             {
                 if (_update == null)
                 {
-                    return Observable.Never<object>().StartWith(_root).Subscribe(observer);
+                    return Observable.Never<object>()
+                        .StartWith(_root.Target)
+                        .Subscribe(observer);
                 }
                 else
                 {
@@ -238,16 +240,18 @@ namespace Perspex.Markup.Data
             {
                 if (_rootGetter != null)
                 {
-                    _node.Target = _rootGetter();
+                    _node.Target = new WeakReference(_rootGetter());
 
                     if (_update != null)
                     {
-                        _updateSubscription = _update.Subscribe(x => _node.Target = _rootGetter());
+                        _updateSubscription = _update.Subscribe(x => 
+                            _node.Target = new WeakReference(_rootGetter()));
                     }
                 }
                 else if (_rootObservable != null)
                 {
-                    _rootObserverSubscription = _rootObservable.Subscribe(x => _node.Target = x);
+                    _rootObserverSubscription = _rootObservable.Subscribe(x => 
+                        _node.Target = new WeakReference(x));
                 }
                 else
                 {

src/Markup/Perspex.Markup/Data/IndexerNode.cs

@@ -22,9 +22,11 @@ namespace Perspex.Markup.Data
 
         public IList<string> Arguments { get; }
 
-        protected override void SubscribeAndUpdate(object target)
+        protected override void SubscribeAndUpdate(WeakReference reference)
         {
-            CurrentValue = GetValue(target);
+            object target = reference.Target;
+
+            CurrentValue = new WeakReference(GetValue(target));
 
             var incc = target as INotifyCollectionChanged;
 
@@ -50,7 +52,7 @@ namespace Perspex.Markup.Data
             }
             if (typeInfo.GetDeclaredProperty(e.PropertyName).GetIndexParameters().Any())
             {
-                CurrentValue = GetValue(sender);
+                CurrentValue = new WeakReference(GetValue(sender));
             }
         }
 
@@ -105,7 +107,7 @@ namespace Perspex.Markup.Data
 
             if (update)
             {
-                CurrentValue = GetValue(sender);
+                CurrentValue = new WeakReference(GetValue(sender));
             }
         }
 

src/Markup/Perspex.Markup/Data/Plugins/IPropertyAccessorPlugin.cs

@@ -6,7 +6,7 @@ using System;
 namespace Perspex.Markup.Data.Plugins
 {
     /// <summary>
-    /// Defines how a member is read, written and observed by a 
+    /// Defines how a member is read, written and observed by an
     /// <see cref="ExpressionObserver"/>.
     /// </summary>
     public interface IPropertyAccessorPlugin
@@ -14,20 +14,23 @@ namespace Perspex.Markup.Data.Plugins
         /// <summary>
         /// Checks whether this plugin can handle accessing the properties of the specified object.
         /// </summary>
-        /// <param name="instance">The object.</param>
+        /// <param name="reference">A weak reference to the object.</param>
         /// <returns>True if the plugin can handle the object; otherwise false.</returns>
-        bool Match(object instance);
+        bool Match(WeakReference reference);
 
         /// <summary>
         /// Starts monitoring the value of a property on an object.
         /// </summary>
-        /// <param name="instance">The object.</param>
+        /// <param name="reference">A weak reference to the object.</param>
         /// <param name="propertyName">The property name.</param>
         /// <param name="changed">A function to call when the property changes.</param>
         /// <returns>
         /// An <see cref="IPropertyAccessor"/> interface through which future interactions with the 
         /// property will be made, or null if the property was not found.
         /// </returns>
-        IPropertyAccessor Start(object instance, string propertyName, Action<object> changed);
+        IPropertyAccessor Start(
+            WeakReference reference, 
+            string propertyName, 
+            Action<object> changed);
     }
 }

src/Markup/Perspex.Markup/Data/Plugins/InpcPropertyAccessorPlugin.cs

@@ -6,6 +6,7 @@ using System.ComponentModel;
 using System.Linq;
 using System.Reactive.Linq;
 using System.Reflection;
+using Perspex.Utilities;
 
 namespace Perspex.Markup.Data.Plugins
 {
@@ -18,11 +19,11 @@ namespace Perspex.Markup.Data.Plugins
         /// <summary>
         /// Checks whether this plugin can handle accessing the properties of the specified object.
         /// </summary>
-        /// <param name="instance">The object.</param>
+        /// <param name="reference">The object.</param>
         /// <returns>True if the plugin can handle the object; otherwise false.</returns>
-        public bool Match(object instance)
+        public bool Match(WeakReference reference)
         {
-            Contract.Requires<ArgumentNullException>(instance != null);
+            Contract.Requires<ArgumentNullException>(reference != null);
 
             return true;
         }
@@ -30,24 +31,28 @@ namespace Perspex.Markup.Data.Plugins
         /// <summary>
         /// Starts monitoring the value of a property on an object.
         /// </summary>
-        /// <param name="instance">The object.</param>
+        /// <param name="reference">The object.</param>
         /// <param name="propertyName">The property name.</param>
         /// <param name="changed">A function to call when the property changes.</param>
         /// <returns>
         /// An <see cref="IPropertyAccessor"/> interface through which future interactions with the 
         /// property will be made, or null if the property was not found.
         /// </returns>
-        public IPropertyAccessor Start(object instance, string propertyName, Action<object> changed)
+        public IPropertyAccessor Start(
+            WeakReference reference, 
+            string propertyName, 
+            Action<object> changed)
         {
-            Contract.Requires<ArgumentNullException>(instance != null);
+            Contract.Requires<ArgumentNullException>(reference != null);
             Contract.Requires<ArgumentNullException>(propertyName != null);
             Contract.Requires<ArgumentNullException>(changed != null);
 
+            var instance = reference.Target;
             var p = instance.GetType().GetRuntimeProperties().FirstOrDefault(_ => _.Name == propertyName);
 
             if (p != null)
             {
-                return new Accessor(instance, p, changed);
+                return new Accessor(reference, p, changed);
             }
             else
             {
@@ -55,40 +60,49 @@ namespace Perspex.Markup.Data.Plugins
             }
         }
 
-        private class Accessor : IPropertyAccessor
+        private class Accessor : IPropertyAccessor, IWeakSubscriber<PropertyChangedEventArgs>
         {
-            private readonly object _instance;
+            private readonly WeakReference _reference;
             private readonly PropertyInfo _property;
             private readonly Action<object> _changed;
 
-            public Accessor(object instance, PropertyInfo property, Action<object> changed)
+            public Accessor(
+                WeakReference reference, 
+                PropertyInfo property, 
+                Action<object> changed)
             {
-                Contract.Requires<ArgumentNullException>(instance != null);
+                Contract.Requires<ArgumentNullException>(reference != null);
                 Contract.Requires<ArgumentNullException>(property != null);
 
-                _instance = instance;
+                _reference = reference;
                 _property = property;
                 _changed = changed;
 
-                var inpc = instance as INotifyPropertyChanged;
+                var inpc = reference.Target as INotifyPropertyChanged;
 
                 if (inpc != null)
                 {
-                    inpc.PropertyChanged += PropertyChanged;
+                    WeakSubscriptionManager.Subscribe(
+                        inpc,
+                        nameof(inpc.PropertyChanged),
+                        this);
                 }
             }
 
             public Type PropertyType => _property.PropertyType;
 
-            public object Value => _property.GetValue(_instance);
+            public object Value => _property.GetValue(_reference.Target);
 
             public void Dispose()
             {
-                var inpc = _instance as INotifyPropertyChanged;
+                var inpc = _reference.Target as INotifyPropertyChanged;
 
                 if (inpc != null)
                 {
-                    inpc.PropertyChanged -= PropertyChanged;
+                    //WeakSubscriptionManager.Unsubscribe(
+                    //    inpc,
+                    //    nameof(inpc.PropertyChanged),
+                    //    this);
                 }
             }
 
@@ -96,14 +110,14 @@ namespace Perspex.Markup.Data.Plugins
             {
                 if (_property.CanWrite)
                 {
-                    _property.SetValue(_instance, value);
+                    _property.SetValue(_reference.Target, value);
                     return true;
                 }
 
                 return false;
             }
 
-            private void PropertyChanged(object sender, PropertyChangedEventArgs e)
+            public void OnEvent(object sender, PropertyChangedEventArgs e)
             {
                 if (e.PropertyName == _property.Name || string.IsNullOrEmpty(e.PropertyName))
                 {

src/Markup/Perspex.Markup/Data/Plugins/PerspexPropertyAccessorPlugin.cs

@@ -16,37 +16,41 @@ namespace Perspex.Markup.Data.Plugins
         /// <summary>
         /// Checks whether this plugin can handle accessing the properties of the specified object.
         /// </summary>
-        /// <param name="instance">The object.</param>
+        /// <param name="reference">A weak reference to the object.</param>
         /// <returns>True if the plugin can handle the object; otherwise false.</returns>
-        public bool Match(object instance)
+        public bool Match(WeakReference reference)
         {
-            Contract.Requires<ArgumentNullException>(instance != null);
+            Contract.Requires<ArgumentNullException>(reference != null);
 
-            return instance is PerspexObject;
+            return reference.Target is PerspexObject;
         }
 
         /// <summary>
         /// Starts monitoring the value of a property on an object.
         /// </summary>
-        /// <param name="instance">The object.</param>
+        /// <param name="reference">A weak reference to the object.</param>
         /// <param name="propertyName">The property name.</param>
         /// <param name="changed">A function to call when the property changes.</param>
         /// <returns>
         /// An <see cref="IPropertyAccessor"/> interface through which future interactions with the 
         /// property will be made, or null if the property was not found.
         /// </returns>
-        public IPropertyAccessor Start(object instance, string propertyName, Action<object> changed)
+        public IPropertyAccessor Start(
+            WeakReference reference, 
+            string propertyName, 
+            Action<object> changed)
         {
-            Contract.Requires<ArgumentNullException>(instance != null);
+            Contract.Requires<ArgumentNullException>(reference != null);
             Contract.Requires<ArgumentNullException>(propertyName != null);
             Contract.Requires<ArgumentNullException>(changed != null);
 
+            var instance = reference.Target;
             var o = (PerspexObject)instance;
             var p = PerspexPropertyRegistry.Instance.FindRegistered(o, propertyName);
 
             if (p != null)
             {
-                return new Accessor(o, p, changed);
+                return new Accessor(new WeakReference<PerspexObject>(o), p, changed);
             }
             else
             {
@@ -56,23 +60,36 @@ namespace Perspex.Markup.Data.Plugins
 
         private class Accessor : IPropertyAccessor
         {
-            private readonly PerspexObject _instance;
+            private readonly WeakReference<PerspexObject> _reference;
             private readonly PerspexProperty _property;
             private IDisposable _subscription;
 
-            public Accessor(PerspexObject instance, PerspexProperty property, Action<object> changed)
+            public Accessor(
+                WeakReference<PerspexObject> reference, 
+                PerspexProperty property, 
+                Action<object> changed)
             {
-                Contract.Requires<ArgumentNullException>(instance != null);
+                Contract.Requires<ArgumentNullException>(reference != null);
                 Contract.Requires<ArgumentNullException>(property != null);
 
-                _instance = instance;
+                _reference = reference;
                 _property = property;
-                _subscription = instance.GetObservable(property).Skip(1).Subscribe(changed);
+                _subscription = Instance.GetObservable(property).Skip(1).Subscribe(changed);
+            }
+
+            public PerspexObject Instance
+            {
+                get
+                {
+                    PerspexObject result;
+                    _reference.TryGetTarget(out result);
+                    return result;
+                }
             }
 
             public Type PropertyType => _property.PropertyType;
 
-            public object Value => _instance.GetValue(_property);
+            public object Value => Instance.GetValue(_property);
 
             public void Dispose()
             {
@@ -84,7 +101,7 @@ namespace Perspex.Markup.Data.Plugins
             {
                 if (!_property.IsReadOnly)
                 {
-                    _instance.SetValue(_property, value);
+                    Instance.SetValue(_property, value);
                     return true;
                 }
 

src/Markup/Perspex.Markup/Data/PropertyAccessorNode.cs

@@ -43,15 +43,17 @@ namespace Perspex.Markup.Data
             }
         }
 
-        protected override void SubscribeAndUpdate(object target)
+        protected override void SubscribeAndUpdate(WeakReference reference)
         {
-            if (target != null)
+            var instance = reference.Target;
+
+            if (instance != null)
             {
-                var plugin = ExpressionObserver.PropertyAccessors.FirstOrDefault(x => x.Match(target));
+                var plugin = ExpressionObserver.PropertyAccessors.FirstOrDefault(x => x.Match(reference));
 
                 if (plugin != null)
                 {
-                    _accessor = plugin.Start(target, PropertyName, SetCurrentValue);
+                    _accessor = plugin.Start(reference, PropertyName, SetCurrentValue);
 
                     if (_accessor != null)
                     {
@@ -61,7 +63,7 @@ namespace Perspex.Markup.Data
                 }
             }
 
-            CurrentValue = PerspexProperty.UnsetValue;
+            CurrentValue = UnsetReference;
         }
 
         protected override void Unsubscribe(object target)
@@ -81,11 +83,11 @@ namespace Perspex.Markup.Data
             // We may need to make this a more general solution.
             if (observable != null && command == null)
             {
-                CurrentValue = PerspexProperty.UnsetValue;
+                CurrentValue = UnsetReference;
                 set = true;
                 _subscription = observable
                     .ObserveOn(SynchronizationContext.Current)
-                    .Subscribe(x => CurrentValue = x);
+                    .Subscribe(x => CurrentValue = new WeakReference(x));
             }
             else if (task != null)
             {
@@ -95,13 +97,13 @@ namespace Perspex.Markup.Data
                 {
                     if (task.Status == TaskStatus.RanToCompletion)
                     {
-                        CurrentValue = resultProperty.GetValue(task);
+                        CurrentValue = new WeakReference(resultProperty.GetValue(task));
                         set = true;
                     }
                     else
                     {
                         task.ContinueWith(
-                                x => CurrentValue = resultProperty.GetValue(task),
+                                x => CurrentValue = new WeakReference(resultProperty.GetValue(task)),
                                 TaskScheduler.FromCurrentSynchronizationContext())
                             .ConfigureAwait(false);
                     }
@@ -109,13 +111,13 @@ namespace Perspex.Markup.Data
             }
             else
             {
-                CurrentValue = value;
+                CurrentValue = new WeakReference(value);
                 set = true;
             }
 
             if (!set)
             {
-                CurrentValue = PerspexProperty.UnsetValue;
+                CurrentValue = UnsetReference;
             }
         }
     }

tests/Perspex.Markup.UnitTests/Data/ExpressionObserverTests_Indexer.cs

@@ -174,6 +174,42 @@ namespace Perspex.Markup.UnitTests.Data
             Assert.Equal(expected, result);
         }
 
+        [Fact]
+        public void Should_Not_Keep_Source_Alive_ObservableCollection()
+        {
+            Func<Tuple<ExpressionObserver, WeakReference>> run = () =>
+            {
+                var source = new { Foo = new ObservableCollection<string> { "foo", "bar" } };
+                var target = new ExpressionObserver(source, "Foo");
+                return Tuple.Create(target, new WeakReference(source.Foo));
+            };
+
+            var result = run();
+            result.Item1.Subscribe(x => { });
+
+            GC.Collect();
+
+            Assert.Null(result.Item2.Target);
+        }
+
+        [Fact]
+        public void Should_Not_Keep_Source_Alive_NonIntegerIndexer()
+        {
+            Func<Tuple<ExpressionObserver, WeakReference>> run = () =>
+            {
+                var source = new NonIntegerIndexer();
+                var target = new ExpressionObserver(source, "Foo");
+                return Tuple.Create(target, new WeakReference(source));
+            };
+
+            var result = run();
+            result.Item1.Subscribe(x => { });
+
+            GC.Collect();
+
+            Assert.Null(result.Item2.Target);
+        }
+
         private class NonIntegerIndexer : NotifyingBase
         {
             private Dictionary<string, string> storage = new Dictionary<string, string>();

tests/Perspex.Markup.UnitTests/Data/ExpressionObserverTests_Lifetime.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.Reactive;
+using System.Reactive.Linq;
 using System.Reactive.Subjects;
 using Microsoft.Reactive.Testing;
 using Perspex.Markup.Data;

tests/Perspex.Markup.UnitTests/Data/ExpressionObserverTests_PerspexProperty.cs

@@ -41,6 +41,24 @@ namespace Perspex.Markup.UnitTests.Data
             sub.Dispose();
         }
 
+        [Fact]
+        public void Should_Not_Keep_Source_Alive()
+        {
+            Func<Tuple<ExpressionObserver, WeakReference>> run = () =>
+            {
+                var source = new Class1();
+                var target = new ExpressionObserver(source, "Foo");
+                return Tuple.Create(target, new WeakReference(source));
+            };
+
+            var result = run();
+            result.Item1.Subscribe(x => { });
+
+            GC.Collect();
+
+            Assert.Null(result.Item2.Target);
+        }
+
         private class Class1 : PerspexObject
         {
             public static readonly StyledProperty<string> FooProperty =

tests/Perspex.Markup.UnitTests/Data/ExpressionObserverTests_Property.cs

@@ -331,6 +331,24 @@ namespace Perspex.Markup.UnitTests.Data
             Assert.Equal(0, second.SubscriptionCount);
         }
 
+        [Fact]
+        public void Should_Not_Keep_Source_Alive()
+        {
+            Func<Tuple<ExpressionObserver, WeakReference>> run = () =>
+            {
+                var source = new Class1 { Foo = "foo" };
+                var target = new ExpressionObserver(source, "Foo");
+                return Tuple.Create(target, new WeakReference(source));
+            };
+
+            var result = run();
+            result.Item1.Subscribe(x => { });
+
+            GC.Collect();
+
+            Assert.Null(result.Item2.Target);
+        }
+
         private interface INext
         {
             int SubscriptionCount { get; }