diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/PresentationSource.cs b/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/PresentationSource.cs index ddd8abb434c..2c75428b282 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/PresentationSource.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/PresentationSource.cs @@ -449,7 +449,7 @@ protected void RootChanged(Visual oldRoot, Visual newRoot) // To fire PresentationSourceChanged when the RootVisual changes; // rather than simulate a "parent" pointer change, we just walk the // collection of all nodes that need the event. - foreach (DependencyObject element in _watchers) + foreach (DependencyObject element in s_watchers) { // We only need to update those elements that are in the // same context as this presentation source. @@ -475,7 +475,7 @@ protected void RootChanged(Visual oldRoot, Visual newRoot) /// protected void AddSource() { - _sources.Add(this); + s_sources.Add(this); } /// @@ -483,7 +483,7 @@ protected void AddSource() /// protected void RemoveSource() { - _sources.Remove(this); + s_sources.Remove(this); } /// @@ -613,17 +613,14 @@ internal static bool IsUnderSamePresentationSource(params ReadOnlySpan - internal static WeakReferenceList CriticalCurrentSources + internal static WeakReferenceList CriticalCurrentSources { - get - { - return _sources; - } + get => s_sources; } private static void AddElementToWatchList(DependencyObject element) { - if(_watchers.Add(element)) + if (s_watchers.Add(element)) { element.SetValue(CachedSourceProperty, PresentationSource.FindSource(element)); element.SetValue(GetsSourceChangedEventProperty, true); @@ -633,7 +630,7 @@ private static void AddElementToWatchList(DependencyObject element) private static void RemoveElementFromWatchList(DependencyObject element) { - if(_watchers.Remove(element)) + if (s_watchers.Remove(element)) { element.ClearValue(CachedSourceProperty); element.ClearValue(GetsSourceChangedEventProperty); @@ -741,11 +738,11 @@ private static readonly DependencyProperty CachedSourceProperty private static readonly object _globalLock = new object(); // An array of weak-references to sources that we know about. - private static WeakReferenceList _sources = new WeakReferenceList(_globalLock); + private static readonly WeakReferenceList s_sources = new(_globalLock); // An array of weak-references to elements that need to know // about source changes. - private static WeakReferenceList _watchers = new WeakReferenceList(_globalLock); + private static readonly WeakReferenceList s_watchers = new(_globalLock); #endregion } diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Diagnostics/ResourceDictionaryDiagnostics.cs b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Diagnostics/ResourceDictionaryDiagnostics.cs index 6ea9c31af7d..66ab9491522 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Diagnostics/ResourceDictionaryDiagnostics.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Diagnostics/ResourceDictionaryDiagnostics.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. // @@ -51,7 +51,7 @@ public static IEnumerable ThemedResourceDictionaries { if (!IsEnabled) { - return ResourceDictionaryDiagnostics.EmptyResourceDictionaryInfos; + return ReadOnlyCollection.Empty; } return SystemResources.ThemedResourceDictionaries; @@ -69,7 +69,7 @@ public static IEnumerable GenericResourceDictionaries { if (!IsEnabled) { - return ResourceDictionaryDiagnostics.EmptyResourceDictionaryInfos; + return ReadOnlyCollection.Empty; } return SystemResources.GenericResourceDictionaries; @@ -276,47 +276,38 @@ private static IReadOnlyCollection EmptyResourceDictionaries public static IEnumerable GetFrameworkElementOwners(ResourceDictionary dictionary) { - return GetOwners(dictionary.FrameworkElementOwners, EmptyFrameworkElementList); + return GetOwners(dictionary.FrameworkElementOwners); } public static IEnumerable GetFrameworkContentElementOwners(ResourceDictionary dictionary) { - return GetOwners(dictionary.FrameworkContentElementOwners, EmptyFrameworkContentElementList); + return GetOwners(dictionary.FrameworkContentElementOwners); } public static IEnumerable GetApplicationOwners(ResourceDictionary dictionary) { - return GetOwners(dictionary.ApplicationOwners, EmptyApplicationList); + return GetOwners(dictionary.ApplicationOwners); } - private static IEnumerable GetOwners(WeakReferenceList list, IEnumerable emptyList) + private static IEnumerable GetOwners(WeakReferenceList list) where T : DispatcherObject { if (!IsEnabled || list == null || list.Count == 0) { - return emptyList; + return Array.Empty(); } - List result = new List(list.Count); - foreach (Object o in list) + // Copy list manually as it doesn't implement ICollection + List owners = new List(list.Count); + foreach (T item in list) { - T owner = o as T; - if (owner != null) - { - result.Add(owner); - } + owners.Add(item); } - return result.AsReadOnly(); + // Create a read-only copy of the list + return owners.AsReadOnly(); } - private static IReadOnlyCollection EmptyFrameworkElementList - => Array.Empty(); - private static IReadOnlyCollection EmptyFrameworkContentElementList - => Array.Empty(); - private static IReadOnlyCollection EmptyApplicationList - => Array.Empty(); - #endregion #region Notify when static resource references are resolved @@ -548,7 +539,5 @@ internal class LookupResult internal static bool IsEnabled { get; private set; } - private static readonly ReadOnlyCollection EmptyResourceDictionaryInfos - = new List().AsReadOnly(); } } diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs index 84e2c746bdf..7093caf50c5 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs @@ -1495,7 +1495,7 @@ internal void AddOwner(DispatcherObject owner) { if (_ownerFEs == null) { - _ownerFEs = new WeakReferenceList(1); + _ownerFEs = new WeakReferenceList(1); } else if (_ownerFEs.Contains(fe) && ContainsCycle(this)) { @@ -1517,7 +1517,7 @@ internal void AddOwner(DispatcherObject owner) { if (_ownerFCEs == null) { - _ownerFCEs = new WeakReferenceList(1); + _ownerFCEs = new WeakReferenceList(1); } else if (_ownerFCEs.Contains(fce) && ContainsCycle(this)) { @@ -1539,7 +1539,7 @@ internal void AddOwner(DispatcherObject owner) { if (_ownerApps == null) { - _ownerApps = new WeakReferenceList(1); + _ownerApps = new WeakReferenceList(1); } else if (_ownerApps.Contains(app) && ContainsCycle(this)) { @@ -1628,32 +1628,28 @@ internal void RemoveOwner(DispatcherObject owner) RemoveOwnerFromAllMergedDictionaries(owner); } - // Check if the given is an owner to this dictionary - internal bool ContainsOwner(DispatcherObject owner) + /// + /// Checks if the given is owning this dictionary + /// + internal bool ContainsOwner(FrameworkElement owner) { - FrameworkElement fe = owner as FrameworkElement; - if (fe != null) - { - return (_ownerFEs != null && _ownerFEs.Contains(fe)); - } - else - { - FrameworkContentElement fce = owner as FrameworkContentElement; - if (fce != null) - { - return (_ownerFCEs != null && _ownerFCEs.Contains(fce)); - } - else - { - Application app = owner as Application; - if (app != null) - { - return (_ownerApps != null && _ownerApps.Contains(app)); - } - } - } + return _ownerFEs?.Contains(owner) ?? false; + } - return false; + /// + /// Checks if the given is owning this dictionary + /// + internal bool ContainsOwner(FrameworkContentElement owner) + { + return _ownerFCEs?.Contains(owner) ?? false; + } + + /// + /// Checks if the given is owning this dictionary + /// + internal bool ContainsOwner(Application owner) + { + return _ownerApps?.Contains(owner) ?? false; } // Helper method that tries to set IsInitialized to true if BeginInit hasn't been called before this. @@ -1681,62 +1677,50 @@ private void NotifyOwners(ResourcesChangeInfo info) if (shouldInvalidate || hasImplicitStyles) { // Invalidate all FE owners - if (_ownerFEs != null) + if (_ownerFEs is not null) { - foreach (Object o in _ownerFEs) + foreach (FrameworkElement fe in _ownerFEs) { - FrameworkElement fe = o as FrameworkElement; - if (fe != null) - { - // Set the HasImplicitStyles flag on the owner - if (hasImplicitStyles) - fe.ShouldLookupImplicitStyles = true; - - // If this dictionary has been initialized fire an invalidation - // to let the tree know of this change. - if (shouldInvalidate) - TreeWalkHelper.InvalidateOnResourcesChange(fe, null, info); - } + // Set the HasImplicitStyles flag on the owner + if (hasImplicitStyles) + fe.ShouldLookupImplicitStyles = true; + + // If this dictionary has been initialized fire an invalidation + // to let the tree know of this change. + if (shouldInvalidate) + TreeWalkHelper.InvalidateOnResourcesChange(fe, null, info); } } // Invalidate all FCE owners - if (_ownerFCEs != null) + if (_ownerFCEs is not null) { - foreach (Object o in _ownerFCEs) + foreach (FrameworkContentElement fce in _ownerFCEs) { - FrameworkContentElement fce = o as FrameworkContentElement; - if (fce != null) - { - // Set the HasImplicitStyles flag on the owner - if (hasImplicitStyles) - fce.ShouldLookupImplicitStyles = true; - - // If this dictionary has been initialized fire an invalidation - // to let the tree know of this change. - if (shouldInvalidate) - TreeWalkHelper.InvalidateOnResourcesChange(null, fce, info); - } + // Set the HasImplicitStyles flag on the owner + if (hasImplicitStyles) + fce.ShouldLookupImplicitStyles = true; + + // If this dictionary has been initialized fire an invalidation + // to let the tree know of this change. + if (shouldInvalidate) + TreeWalkHelper.InvalidateOnResourcesChange(null, fce, info); } } // Invalidate all App owners - if (_ownerApps != null) + if (_ownerApps is not null) { - foreach (Object o in _ownerApps) + foreach (Application app in _ownerApps) { - Application app = o as Application; - if (app != null) - { - // Set the HasImplicitStyles flag on the owner - if (hasImplicitStyles) - app.HasImplicitStylesInResources = true; - - // If this dictionary has been initialized fire an invalidation - // to let the tree know of this change. - if (shouldInvalidate) - app.InvalidateResourceReferences(info); - } + // Set the HasImplicitStyles flag on the owner + if (hasImplicitStyles) + app.HasImplicitStylesInResources = true; + + // If this dictionary has been initialized fire an invalidation + // to let the tree know of this change. + if (shouldInvalidate) + app.InvalidateResourceReferences(info); } } } @@ -1805,14 +1789,14 @@ private object FetchResource( return GetValue(resourceKey, out canCache); } - private WeakReferenceList GetOrCreateWeakReferenceList(object resourceKey) + private WeakReferenceList GetOrCreateWeakReferenceList(object resourceKey) { - this._weakDeferredResourceReferencesMap ??= new(); + _weakDeferredResourceReferencesMap ??= new Dictionary>(); - if (!this._weakDeferredResourceReferencesMap.TryGetValue(resourceKey, out var weakDeferredResourceReferences)) + if (!_weakDeferredResourceReferencesMap.TryGetValue(resourceKey, out var weakDeferredResourceReferences)) { - weakDeferredResourceReferences = new WeakReferenceList(); - this._weakDeferredResourceReferencesMap[resourceKey] = weakDeferredResourceReferences; + weakDeferredResourceReferences = new WeakReferenceList(syncRoot: null); + _weakDeferredResourceReferencesMap[resourceKey] = weakDeferredResourceReferences; } return weakDeferredResourceReferences; @@ -1820,8 +1804,7 @@ private WeakReferenceList GetOrCreateWeakReferenceList(object resourceKey) internal void RemoveDeferredResourceReference(DeferredResourceReference deferredResourceReference) { - - if (this._weakDeferredResourceReferencesMap?.TryGetValue(deferredResourceReference.Key, out var weakDeferredResourceReferences) is true) + if (_weakDeferredResourceReferencesMap?.TryGetValue(deferredResourceReference.Key, out var weakDeferredResourceReferences) is true) { weakDeferredResourceReferences.Remove(deferredResourceReference); } @@ -1833,7 +1816,6 @@ internal void RemoveDeferredResourceReference(DeferredResourceReference deferred /// private void ValidateDeferredResourceReferences(object resourceKey) { - if (_weakDeferredResourceReferencesMap is null) { return; @@ -1841,36 +1823,29 @@ private void ValidateDeferredResourceReferences(object resourceKey) if (resourceKey is null) { - foreach (var weakDeferredResourceReferences in _weakDeferredResourceReferencesMap.Values) + foreach (WeakReferenceList weakReferencesList in _weakDeferredResourceReferencesMap.Values) { - foreach (var weakResourceReference in weakDeferredResourceReferences) + foreach (DeferredResourceReference weakReference in weakReferencesList) { - DeferredResourceReference deferredResourceReference = weakResourceReference as DeferredResourceReference; - - Inflate(deferredResourceReference); + Inflate(weakReference); } } } else { - if (_weakDeferredResourceReferencesMap.TryGetValue(resourceKey, out var weakDeferredResourceReferences)) + if (_weakDeferredResourceReferencesMap.TryGetValue(resourceKey, out WeakReferenceList weakReferencesList)) { - foreach (var weakResourceReference in weakDeferredResourceReferences) + foreach (DeferredResourceReference weakReference in weakReferencesList) { - DeferredResourceReference deferredResourceReference = weakResourceReference as DeferredResourceReference; - - Inflate(deferredResourceReference); + Inflate(weakReference); } } } - return; - - void Inflate(DeferredResourceReference deferredResourceReference) + static void Inflate(DeferredResourceReference deferredResourceReference) { - // This will inflate the deferred reference, causing it - // to be removed from the list. The list may also be - // purged of dead references. + // This will inflate the deferred reference, causing it to be removed from the list. + // The list may also be purged of dead references. deferredResourceReference?.GetValue(BaseValueSourceInternal.Unknown); } } @@ -2012,14 +1987,12 @@ private void PropagateParentOwners(ResourceDictionary mergedDictionary) if (mergedDictionary._ownerFEs == null) { - mergedDictionary._ownerFEs = new WeakReferenceList(_ownerFEs.Count); + mergedDictionary._ownerFEs = new WeakReferenceList(_ownerFEs.Count); } - foreach (object o in _ownerFEs) + foreach (FrameworkElement fe in _ownerFEs) { - FrameworkElement fe = o as FrameworkElement; - if (fe != null) - mergedDictionary.AddOwner(fe); + mergedDictionary.AddOwner(fe); } } @@ -2029,14 +2002,12 @@ private void PropagateParentOwners(ResourceDictionary mergedDictionary) if (mergedDictionary._ownerFCEs == null) { - mergedDictionary._ownerFCEs = new WeakReferenceList(_ownerFCEs.Count); + mergedDictionary._ownerFCEs = new WeakReferenceList(_ownerFCEs.Count); } - foreach (object o in _ownerFCEs) + foreach (FrameworkContentElement fce in _ownerFCEs) { - FrameworkContentElement fce = o as FrameworkContentElement; - if (fce != null) - mergedDictionary.AddOwner(fce); + mergedDictionary.AddOwner(fce); } } @@ -2046,14 +2017,12 @@ private void PropagateParentOwners(ResourceDictionary mergedDictionary) if (mergedDictionary._ownerApps == null) { - mergedDictionary._ownerApps = new WeakReferenceList(_ownerApps.Count); + mergedDictionary._ownerApps = new WeakReferenceList(_ownerApps.Count); } - foreach (object o in _ownerApps) + foreach (Application app in _ownerApps) { - Application app = o as Application; - if (app != null) - mergedDictionary.AddOwner(app); + mergedDictionary.AddOwner(app); } } } @@ -2067,37 +2036,31 @@ private void PropagateParentOwners(ResourceDictionary mergedDictionary) /// internal void RemoveParentOwners(ResourceDictionary mergedDictionary) { - if (_ownerFEs != null) + if (_ownerFEs is not null) { - foreach (Object o in _ownerFEs) + foreach (FrameworkElement fe in _ownerFEs) { - FrameworkElement fe = o as FrameworkElement; mergedDictionary.RemoveOwner(fe); - } } - if (_ownerFCEs != null) + if (_ownerFCEs is not null) { Invariant.Assert(_ownerFCEs.Count > 0); - foreach (Object o in _ownerFCEs) + foreach (FrameworkContentElement fec in _ownerFCEs) { - FrameworkContentElement fec = o as FrameworkContentElement; mergedDictionary.RemoveOwner(fec); - } } - if (_ownerApps != null) + if (_ownerApps is not null) { Invariant.Assert(_ownerApps.Count > 0); - foreach (Object o in _ownerApps) + foreach (Application app in _ownerApps) { - Application app = o as Application; mergedDictionary.RemoveOwner(app); - } } } @@ -2118,19 +2081,19 @@ private bool ContainsCycle(ResourceDictionary origin) // three properties used by ResourceDictionaryDiagnostics - internal WeakReferenceList FrameworkElementOwners + internal WeakReferenceList FrameworkElementOwners { - get { return _ownerFEs; } + get => _ownerFEs; } - internal WeakReferenceList FrameworkContentElementOwners + internal WeakReferenceList FrameworkContentElementOwners { - get { return _ownerFCEs; } + get => _ownerFCEs; } - internal WeakReferenceList ApplicationOwners + internal WeakReferenceList ApplicationOwners { - get { return _ownerApps; } + get => _ownerApps; } #endregion HelperMethods @@ -2619,11 +2582,12 @@ private enum FallbackState #region Data + private WeakReferenceList _ownerFEs; + private WeakReferenceList _ownerFCEs; + private WeakReferenceList _ownerApps; + private Dictionary> _weakDeferredResourceReferencesMap; + private Hashtable _baseDictionary = null; - private WeakReferenceList _ownerFEs = null; - private WeakReferenceList _ownerFCEs = null; - private WeakReferenceList _ownerApps = null; - private Dictionary _weakDeferredResourceReferencesMap = null; private ObservableCollection _mergedDictionaries = null; private Uri _source = null; private Uri _baseUri = null; diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/SystemResources.cs b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/SystemResources.cs index 4650ba7de11..cdb0ba523b3 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/SystemResources.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/SystemResources.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. // @@ -1791,10 +1791,8 @@ internal virtual void RemoveFromDictionary() internal void AddInflatedListener(ResourceReferenceExpression listener) { - if (_inflatedList == null) - { - _inflatedList = new WeakReferenceList(this); - } + _inflatedList ??= new WeakReferenceList(this); + _inflatedList.Add(listener); } @@ -1837,7 +1835,7 @@ internal bool IsInflated private ResourceDictionary _dictionary; protected object _key; protected object _value; - private WeakReferenceList _inflatedList; + private WeakReferenceList _inflatedList; #endregion Data } diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs index 80e6db8f9b3..0b5ecd01719 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs @@ -1,191 +1,207 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Collections; +#nullable enable -namespace MS.Internal +using System.Collections.ObjectModel; +using System.Runtime.CompilerServices; + +namespace MS.Internal; + +/// +/// This is a ThreadSafe that uses Copy On Write to support consistency. +/// - When the "List" property is requested a readonly reference to the +/// list is returned and a reference to the readonly list is cached. +/// - If the "List" is requested again, the same cached reference is returned. +/// - When the list is modified, if a readonly reference is present in the +/// cache then the list is copied before it is modified and the readonly list is +/// released from the cache. +/// +internal abstract class CopyOnWriteList where T : class { + private ReadOnlyCollection? _readonlyWrapper; + private List _liveList; + + private readonly object _syncRoot; + /// - /// This is a ThreadSafe ArrayList that uses Copy On Write to support consistency. - /// - When the "List" property is requested a readonly reference to the - /// list is returned and a reference to the readonly list is cached. - /// - If the "List" is requested again, the same cached reference is returned. - /// - When the list is modified, if a readonly reference is present in the - /// cache then the list is copied before it is modified and the readonly list is - /// released from the cache. + /// Creates a new WeakReferenceList with the default capacity. /// - internal class CopyOnWriteList + /// The initial capacity of the list. + public CopyOnWriteList(int capacity) { - public CopyOnWriteList() : this(null) - { - } + _syncRoot = new object(); + _liveList = new List(capacity); + } + + /// + /// Creates a new CopyOnWriteList with the specified sync root. + /// + /// The synchronization object used to manage access to the list. + /// In case is , a private instance will be created. + public CopyOnWriteList(object? syncRoot) + { + syncRoot ??= new object(); - public CopyOnWriteList(object syncRoot) + _syncRoot = syncRoot; + _liveList = new List(); + } + + /// + /// Return a readonly wrapper of the list. + /// A non-null _readonlyWrapper is a "Copy on Write" flag. + /// Methods that change the list (eg. Add() and Remove()) are responsible for: + /// 1) Checking _readonlyWrapper and copying the list before modifying it. + /// 2) Clearing _readonlyWrapper. + /// + /// Note: This is NOT a copy. + public ReadOnlyCollection List + { + get { - if(syncRoot == null) + ReadOnlyCollection tempList; + + lock (_syncRoot) { - syncRoot = new Object(); + _readonlyWrapper ??= _liveList.AsReadOnly(); + + tempList = _readonlyWrapper; } - _syncRoot = syncRoot; + + return tempList; } + } - /// - /// Return a readonly wrapper of the list. Note: this is NOT a copy. - /// A non-null _readonlyWrapper is a "Copy on Write" flag. - /// Methods that change the list (eg. Add() and Remove()) are - /// responsible for: - /// 1) Checking _readonlyWrapper and copying the list before modifing it. - /// 2) Clearing _readonlyWrapper. - /// - public ArrayList List + /// + /// Add an object to the List. + /// Returns true if successfully added. + /// Returns false if object is already on the list. + /// + protected bool Add(T obj) + { + Debug.Assert(obj is not null, "CopyOnWriteList.Add() should not be passed null."); + + lock (_syncRoot) { - get - { - ArrayList tempList; - - lock(_syncRoot) - { - if(null == _readonlyWrapper) - _readonlyWrapper = ArrayList.ReadOnly(_LiveList); - tempList = _readonlyWrapper; - } - return tempList; - } + int index = Find(obj); + + return index < 0 && Internal_Add(obj); } + } - /// - /// Add an object to the List. - /// Returns true if successfully added. - /// Returns false if object is already on the list. - /// - public virtual bool Add(object obj) + /// + /// Remove an object from the List. + /// Returns true if successfully removed. + /// Returns false if object was not on the list. + /// + protected bool Remove(T obj) + { + Debug.Assert(obj is not null, "CopyOnWriteList.Remove() should not be passed null."); + lock (_syncRoot) { - Debug.Assert(null!=obj, "CopyOnWriteList.Add() should not be passed null."); - lock(_syncRoot) - { - int index = Find(obj); - - if(index >= 0) - return false; + int index = Find(obj); - return Internal_Add(obj); - } + // If the object is not on the list then we are done. + return index >= 0 && RemoveAt(index); } + } - /// - /// Remove an object from the List. - /// Returns true if successfully removed. - /// Returns false if object was not on the list. - /// - public virtual bool Remove(object obj) - { - Debug.Assert(null!=obj, "CopyOnWriteList.Remove() should not be passed null."); - lock(_syncRoot) - { - int index = Find(obj); + /// + /// This allows derived classes to take the lock. This is mostly used + /// to extend Add() and Remove() etc. + /// + protected object SyncRoot + { + get => _syncRoot; + } - // If the object is not on the list then - // we are done. (return false) - if(index < 0) - return false; + /// + /// This is protected and the caller can get into real serious trouble + /// using this. Because this points at the real current list without + /// any copy on write protection. So the caller must really know what + /// they are doing. + /// + protected List LiveList + { + get => _liveList; + } - return RemoveAt(index); - } - } + /// + /// Add an object to the List. + /// Without any error checks. + /// For use by derived classes that implement their own error checks. + /// + protected bool Internal_Add(T obj) + { + DoCopyOnWriteCheck(); - /// - /// This allows derived classes to take the lock. This is mostly used - /// to extend Add() and Remove() etc. - /// - protected object SyncRoot - { - get{ return _syncRoot; } - } + _liveList.Add(obj); - /// - /// This is protected and the caller can get into real serious trouble - /// using this. Because this points at the real current list without - /// any copy on write protection. So the caller must really know what - /// they are doing. - /// - protected ArrayList LiveList - { - get{ return _LiveList; } - } + return true; + } - /// - /// Add an object to the List. - /// Without any error checks. - /// For use by derived classes that implement there own error checks. - /// - protected bool Internal_Add(object obj) - { - DoCopyOnWriteCheck(); - _LiveList.Add(obj); - return true; - } + /// + /// Insert an object into the List at the given index. + /// Without any error checks. + /// For use by derived classes that implement there own error checks. + /// + protected bool Internal_Insert(int index, T obj) + { + DoCopyOnWriteCheck(); - /// - /// Insert an object into the List at the given index. - /// Without any error checks. - /// For use by derived classes that implement there own error checks. - /// - protected bool Internal_Insert(int index, object obj) - { - DoCopyOnWriteCheck(); - _LiveList.Insert(index, obj); - return true; - } + _liveList.Insert(index, obj); + + return true; + } - /// - /// Find an object on the List. - /// - private int Find(object obj) + /// + /// Find an object on the List. + /// + private int Find(T obj) + { + // syncRoot Lock MUST be held by the caller. + for (int i = 0; i < _liveList.Count; i++) { - // syncRoot Lock MUST be held by the caller. - for(int i = 0; i < _LiveList.Count; i++) + if (obj == _liveList[i]) { - if(obj == _LiveList[i]) - { - return i; - } + return i; } - return -1; } - /// - /// Remove the object at a given index from the List. - /// Returns true if successfully removed. - /// Returns false if index is outside the range of the list. - /// - /// This is protected because it operates on the LiveList - /// - protected bool RemoveAt(int index) - { - // syncRoot Lock MUST be held by the caller. - if(index <0 || index >= _LiveList.Count ) - return false; + return -1; + } - DoCopyOnWriteCheck(); - _LiveList.RemoveAt(index); - return true; - } + /// + /// Remove the object at a given index from the List. + /// Returns true if successfully removed. + /// Returns false if index is outside the range of the list. + /// + /// This is protected because it operates on the . + protected bool RemoveAt(int index) + { + // syncRoot Lock MUST be held by the caller. + if ((uint)index >= (uint)_liveList.Count) + return false; + + DoCopyOnWriteCheck(); + + _liveList.RemoveAt(index); + + return true; + } - protected void DoCopyOnWriteCheck() + /// + /// This must be called under lock held. + /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] + protected void DoCopyOnWriteCheck() + { + // If we have exposed (given out) a readonly reference to this version of the list, + // then clone a new internal copy and cut the old version free. + if (_readonlyWrapper is not null) { - // syncRoot Lock MUST be held by the caller. - // If we have exposed (given out) a readonly reference to this - // version of the list, then clone a new internal copy and cut - // the old version free. - if(null != _readonlyWrapper) - { - _LiveList = (ArrayList)_LiveList.Clone(); - _readonlyWrapper = null; - } + _liveList = new List(_liveList); + _readonlyWrapper = null; } - - private object _syncRoot; - private ArrayList _LiveList = new ArrayList(); - private ArrayList _readonlyWrapper; } } diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceEnumerator.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceEnumerator.cs index a08c90fd421..6ba7a5be587 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceEnumerator.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceEnumerator.cs @@ -2,67 +2,82 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections; +using System.Collections.ObjectModel; -namespace MS.Internal +#nullable enable + +namespace MS.Internal; + +/// +/// This allows callers to "foreach" through a WeakReferenceList. +/// Each weak reference is checked for liveness and "current" +/// actually returns a strong reference to the current element. +/// +/// +/// +/// Due to the way enumerators function, this enumerator often +/// holds a cached strong reference to the "Current" element. +/// This should not be a problem unless the caller stops enumerating +/// before the end of the list AND holds the enumerator alive forever. +/// +/// Explicitly calling Dispose, this removes the strong reference. +/// +internal struct WeakReferenceListEnumerator : IEnumerator where T : class { - /// - /// This allows callers to "foreach" through a WeakReferenceList. - /// Each weakreference is checked for liveness and "current" - /// actually returns a strong reference to the current element. - /// - /// - /// Due to the way enumerators function, this enumerator often - /// holds a cached strong reference to the "Current" element. - /// This should not be a problem unless the caller stops enumerating - /// before the end of the list AND holds the enumerator alive forever. - /// - internal struct WeakReferenceListEnumerator : IEnumerator + private readonly ReadOnlyCollection> _backingList; + + private T? _strongReference; + private int _index; + + internal WeakReferenceListEnumerator(ReadOnlyCollection> backingList) { - public WeakReferenceListEnumerator( ArrayList List) - { - _i = 0; - _List = List; - _StrongReference = null; - } + _index = 0; + _backingList = backingList; + _strongReference = null; + } - object IEnumerator.Current - { get{ return Current; } } - - public object Current - { - get - { - if( null == _StrongReference ) - { - throw new System.InvalidOperationException(SR.Enumerator_VerifyContext); - } - return _StrongReference; - } - } + readonly T IEnumerator.Current + { + get => Current; + } - public bool MoveNext() - { - object obj=null; - while( _i < _List.Count ) - { - WeakReference weakRef = (WeakReference) _List[ _i++ ]; - obj = weakRef.Target; - if(null != obj) - break; - } - _StrongReference = obj; - return (null != obj); - } + readonly object IEnumerator.Current + { + get => Current; + } + + public readonly T Current + { + get => _strongReference ?? throw new InvalidOperationException(SR.Enumerator_VerifyContext); + } - public void Reset() + public bool MoveNext() + { + T? element = null; + + while (_index < _backingList.Count) { - _i = 0; - _StrongReference = null; + WeakReference weakRef = _backingList[_index++]; + + if (weakRef.TryGetTarget(out element)) + break; } - private int _i; - private ArrayList _List; - private object _StrongReference; + _strongReference = element; + + return element is not null; + } + + public void Reset() + { + _index = 0; + _strongReference = null; + } + + public void Dispose() + { + // Clean up the instance to avoid holding strong references to elements + _strongReference = null; } } diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs index 15d712abd52..58ee2b80a9e 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs @@ -1,254 +1,267 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +#nullable enable + using System.Collections; -namespace MS.Internal +namespace MS.Internal; + +/// +/// This is a Cached ThreadSafe of WeakReferences. +/// - When the "List" property is requested a readonly reference to the +/// list is returned and a reference to the readonly list is cached. +/// - If the "List" is requested again, the same cached reference is returned. +/// - When the list is modified, if a readonly reference is present in the +/// cache then the list is copied before it is modified and the readonly list is +/// released from the cache. +/// +internal sealed class WeakReferenceList : CopyOnWriteList>, IEnumerable where T : class { /// - /// This is a Cached ThreadSafe ArrayList of WeakReferences. - /// - When the "List" property is requested a readonly reference to the - /// list is returned and a reference to the readonly list is cached. - /// - If the "List" is requested again, the same cached reference is returned. - /// - When the list is modified, if a readonly reference is present in the - /// cache then the list is copied before it is modified and the readonly list is - /// released from the cache. + /// Retrieves the count of weak references in the list. /// - internal class WeakReferenceList : CopyOnWriteList, IEnumerable + /// This can include any dead references currently present in the list. + public int Count { - public WeakReferenceList():base(null) + get { + lock (SyncRoot) + { + return LiveList.Count; + } } + } - public WeakReferenceList(object syncRoot):base(syncRoot) - { - } + /// + /// Creates a new WeakReferenceList with the default capacity. + /// + /// The initial capacity of the list. + public WeakReferenceList(int capacity) : base(capacity) + { + } - public WeakReferenceListEnumerator GetEnumerator() - { - return new WeakReferenceListEnumerator(List); - } + /// + /// Creates a new WeakReferenceList with the specified sync root. + /// + /// The synchronization object used to manage access to the list. + /// In case is , a private instance will be created. + public WeakReferenceList(object? syncRoot) : base(syncRoot) + { + } + + /// + /// Retrieves a struct-based enumerator for the list. + /// + /// A struct-based enumerator instance. + public WeakReferenceListEnumerator GetEnumerator() + { + return new WeakReferenceListEnumerator(List); + } + + IEnumerator IEnumerable.GetEnumerator() + { + return GetEnumerator(); + } + + IEnumerator IEnumerable.GetEnumerator() + { + return GetEnumerator(); + } + + public bool Contains(T item) + { + Debug.Assert(item is not null, "WeakReferenceList.Contains() should not be passed null."); - IEnumerator IEnumerable.GetEnumerator() + lock (SyncRoot) { - return GetEnumerator(); + // If the object is already on the list then return true + return FindWeakReference(item) >= 0; } + } - public bool Contains(object item) - { - Debug.Assert(null != item, "WeakReferenceList.Contains() should not be passed null."); - lock (base.SyncRoot) - { - int index = FindWeakReference(item); + /// + /// Add a weak reference to the List. + /// Returns true if successfully added. + /// Returns false if object is already on the list. + /// + public bool Add(T obj) + { + Debug.Assert(obj is not null, "WeakReferenceList.Add() should not be passed null."); - // If the object is already on the list then - // return true - if (index >= 0) - return true; + return Add(obj, skipFind: false); + } - return false; - } - } + //Will insert a new WeakReference into the list. + //The object bein inserted MUST be unique as there is no check for it. + public bool Add(T obj, bool skipFind) + { + Debug.Assert(obj is not null, "WeakReferenceList.Add() should not be passed null."); - public int Count + lock (SyncRoot) { - get + if (skipFind) { - int count = 0; - lock (base.SyncRoot) + // before growing the list, purge it of dead entries. + // The expense of purging amortizes to O(1) per entry, because + // the list doubles its capacity when it grows. + if (LiveList.Count == LiveList.Capacity) { - count = base.LiveList.Count; + Purge(); } - return count; } -} + else if (FindWeakReference(obj) >= 0) + { + return false; + } + + return Internal_Add(new WeakReference(obj)); + } + } - /// - /// Add a weak reference to the List. - /// Returns true if successfully added. - /// Returns false if object is already on the list. - /// - public override bool Add(object obj) + /// + /// Remove a weak reference to the List. + /// Returns true if successfully removed. + /// Returns false if object is not in the list. + /// + public bool Remove(T obj) + { + Debug.Assert(obj is not null, "WeakReferenceList.Remove() should not be passed null."); + + lock (SyncRoot) { - Debug.Assert(null!=obj, "WeakReferenceList.Add() should not be passed null."); - return Add(obj, false /*skipFind*/); -} + int index = FindWeakReference(obj); + + // If the object is not on the list then we are done. + return index >= 0 && RemoveAt(index); + } + } + + /// + /// Insert a weak reference into the List. + /// Returns true if successfully inserted. + /// Returns false if object is already on the list. + /// + public bool Insert(int index, T obj) + { + Debug.Assert(obj is not null, "WeakReferenceList.Add() should not be passed null."); - //Will insert a new WeakREference into the list. - //The object bein inserted MUST be unique as there is no check for it. - public bool Add(object obj, bool skipFind) + lock (SyncRoot) { - Debug.Assert(null!=obj, "WeakReferenceList.Add() should not be passed null."); - lock(base.SyncRoot) + int existingIndex = FindWeakReference(obj); + + // If the object is already on the list then we are done. + return existingIndex < 0 && Internal_Insert(index, new WeakReference(obj)); + } + } + + /// + /// Find an object on the List. + /// Also cleans up dead weak references. + /// + private int FindWeakReference(T obj) + { + // syncRoot Lock MUST be held by the caller. + // Search the LiveList looking for the object, also remove any + // dead references we find. + // + // We use the "LiveList" to avoid snapping a Clone every time we + // Change something. + // To do this correctly you need to understand how the base class + // virtualizes the Copy On Write. + bool foundDeadReferences = true; // so that the while loop runs the first time + int foundItem = -1; + + while (foundDeadReferences) + { + foundDeadReferences = false; + List> list = LiveList; + + for (int i = 0; i < list.Count; i++) { - if (skipFind) + WeakReference weakRef = list[i]; + + if (weakRef.TryGetTarget(out T? target)) { - // before growing the list, purge it of dead entries. - // The expense of purging amortizes to O(1) per entry, because - // the the list doubles its capacity when it grows. - if (LiveList.Count == LiveList.Capacity) + if (target == obj) { - Purge(); + foundItem = i; + break; } } - else if (FindWeakReference(obj) >= 0) + else { - return false; + foundDeadReferences = true; } - - return base.Internal_Add(new WeakReference(obj)); } - } - /// - /// Remove a weak reference to the List. - /// Returns true if successfully added. - /// Returns false if object is already on the list. - /// - public override bool Remove(object obj) - { - Debug.Assert(null!=obj, "WeakReferenceList.Remove() should not be passed null."); - lock(base.SyncRoot) + if (foundDeadReferences) { - int index = FindWeakReference(obj); - - // If the object is not on the list then - // we are done. (return false) - if(index < 0) - return false; - - return base.RemoveAt(index); + // if there were dead references, take this opportunity + // to clean up _all_ the dead references. After doing this, + // the foundItem index is no longer valid, so we just + // compute it again. + // Most of the time we expect no dead references, so the while + // loop runs once and the for loop walks the list once. + // Occasionally there will be dead references - the while loop + // runs twice and the for loop walks the list twice. Purge + // also walks the list once, for a total of three times. + Purge(); } } - /// - /// Insert a weak reference into the List. - /// Returns true if successfully inserted. - /// Returns false if object is already on the list. - /// - public bool Insert(int index, object obj) - { - Debug.Assert(null!=obj, "WeakReferenceList.Add() should not be passed null."); - lock(base.SyncRoot) - { - int existingIndex = FindWeakReference(obj); + return foundItem; + } - // If the object is already on the list then - // we are done. (return false) - if(existingIndex >= 0) - return false; + // purge the list of dead references + // caller is expected to lock the SyncRoot + private void Purge() + { + List> list = LiveList; + int destIndex; + int n = list.Count; - return base.Internal_Insert(index, new WeakReference(obj)); - } + // skip over valid entries at the beginning of the list + for (destIndex = 0; destIndex < n; ++destIndex) + { + WeakReference wr = list[destIndex]; + if (!wr.TryGetTarget(out _)) + break; } - /// - /// Find an object on the List. - /// Also cleans up dead weakreferences. - /// - private int FindWeakReference(object obj) - { - // syncRoot Lock MUST be held by the caller. - // Search the LiveList looking for the object, also remove any - // dead references we find. - // - // We use the "LiveList" to avoid snapping a Clone everytime we - // Change something. - // To do this correctly you need to understand how the base class - // virtualizes the Copy On Write. - bool foundDeadReferences = true; // so that the while loop runs the first time - int foundItem = -1; - - while (foundDeadReferences) - { - foundDeadReferences = false; - ArrayList list = base.LiveList; - - for(int i = 0; i < list.Count; i++) - { - WeakReference weakRef = (WeakReference) list[i]; - if(weakRef.IsAlive) - { - if(obj == weakRef.Target) - { - foundItem = i; - break; - } - } - else - { - foundDeadReferences = true; - } - } - - if (foundDeadReferences) - { - // if there were dead references, take this opportunity - // to clean up _all_ the dead references. After doing this, - // the foundItem index is no longer valid, so we just - // compute it again. - // Most of the time we expect no dead references, so the while - // loop runs once and the for loop walks the list once. - // Occasionally there will be dead references - the while loop - // runs twice and the for loop walks the list twice. Purge - // also walks the list once, for a total of three times. - Purge(); - } - } - - return foundItem; - } - - // purge the list of dead references - // caller is expected to lock the SyncRoot - private void Purge() - { - ArrayList list = base.LiveList; - int destIndex; - int n = list.Count; - - // skip over valid entries at the beginning of the list - for (destIndex=0; destIndex= n) + return; - // there may be nothing to purge - if (destIndex >= n) - return; + // but if there is, check for copy-on-write + DoCopyOnWriteCheck(); + list = LiveList; - // but if there is, check for copy-on-write - DoCopyOnWriteCheck(); - list = base.LiveList; - - // move remaining valid entries toward the beginning, into one - // contiguous block - for (int i=destIndex+1; i wr = list[i]; + if (wr.TryGetTarget(out _)) { - WeakReference wr = (WeakReference)list[i]; - if (wr.IsAlive) - { - list[destIndex++] = list[i]; - } + list[destIndex++] = list[i]; } + } - // remove the remaining entries and shrink the list - if (destIndex < n) - { - list.RemoveRange(destIndex, n - destIndex); + // remove the remaining entries and shrink the list + if (destIndex < n) + { + list.RemoveRange(destIndex, n - destIndex); - // shrink the list if it would be less than half full otherwise. - // This is more liberal than List.TrimExcess(), because we're - // probably in the situation where additions to the list are common. - int newCapacity = destIndex << 1; - if (newCapacity < list.Capacity) - { - list.Capacity = newCapacity; - } + // shrink the list if it would be less than half full otherwise. + // This is more liberal than List.TrimExcess(), because we're + // probably in the situation where additions to the list are common. + int newCapacity = destIndex << 1; + if (newCapacity < list.Capacity) + { + list.Capacity = newCapacity; } - } + } } } diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Win32/HwndWrapper.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Win32/HwndWrapper.cs index cc7cc630e1f..87a9b95aeac 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Win32/HwndWrapper.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Win32/HwndWrapper.cs @@ -220,13 +220,13 @@ private void Dispose(bool disposing, bool isHwndBeingDestroyed) public void AddHook(HwndWrapperHook hook) { - _hooks ??= []; + _hooks ??= new WeakReferenceList(syncRoot: null); _hooks.Insert(0, hook); } internal void AddHookLast(HwndWrapperHook hook) { - _hooks ??= []; + _hooks ??= new WeakReferenceList(syncRoot: null); _hooks.Add(hook); } @@ -357,7 +357,7 @@ public DestroyWindowArgs(IntPtr handle, ushort classAtom) private IntPtr _handle; private UInt16 _classAtom; - private WeakReferenceList _hooks; + private WeakReferenceList _hooks; private int _ownerThreadID; private HwndWrapperHook _wndProc;