Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixing issues with DynamicResource usage #9393

Merged
merged 1 commit into from
Jul 19, 2024
Merged

Fixing issues with DynamicResource usage #9393

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 29 additions & 20 deletions src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ private void CopyToWithoutLock(DictionaryEntry[] array, int arrayIndex)
entry.Value = value; // refresh the entry value in case it was changed in the previous call
}
}

// This is set when the RD is loaded from unsafe xps doc. This will be checked while creating reader for RD source.
internal bool IsUnsafe { get; set; }

Expand Down Expand Up @@ -157,7 +157,7 @@ public Uri Source
// that it is being passed down by the Baml parsing code, and it is trying to give us more
// information to avoid possible ambiguities in assembly resolving. Use the VersionedUri
// to resolve, and the set _source to the OriginalUri so we don't change the return of Source property.
// The versioned Uri is not stored, if the version info is needed while debugging, once this method
// The versioned Uri is not stored, if the version info is needed while debugging, once this method
// returns _reader should be set, from there BamlSchemaContext.LocalAssembly contains the version info.
if (uriWrapper == null)
{
Expand All @@ -169,10 +169,10 @@ public Uri Source
_source = uriWrapper.OriginalUri;
sourceUri = uriWrapper.VersionedUri;
}

Clear();


Uri uri = BindUriHelper.GetResolvedUri(_baseUri, sourceUri);

WebRequest request = WpfWebRequestHelper.CreateRequest(uri);
Expand Down Expand Up @@ -1737,10 +1737,7 @@ private object FetchResource(
{
// Cache the deferredResourceReference so that it can be validated
// in case of a dictionary change prior to its inflation
if (_deferredResourceReferences == null)
{
_deferredResourceReferences = new DeferredResourceReferenceList();
}
_deferredResourceReferences ??= new DeferredResourceReferenceList();

if (_deferredResourceReferences.Get(resourceKey) is { } existingDeferredResourceReference
&& existingDeferredResourceReference.Dictionary == this)
Expand All @@ -1749,14 +1746,7 @@ private object FetchResource(
}
else
{
if (_ownerApps != null)
{
deferredResourceReference = new DeferredAppResourceReference(this, resourceKey);
}
else
{
deferredResourceReference = new DeferredResourceReference(this, resourceKey);
}
deferredResourceReference = _ownerApps is not null ? new DeferredAppResourceReference(this, resourceKey) : new DeferredResourceReference(this, resourceKey);

_deferredResourceReferences.AddOrSet(deferredResourceReference);
}
Expand All @@ -1781,10 +1771,29 @@ private object FetchResource(
/// </summary>
private void ValidateDeferredResourceReferences(object resourceKey)
{
if (_deferredResourceReferences != null)
if (_deferredResourceReferences is null)
{
return;
}

if (resourceKey is null)
{
foreach (DeferredResourceReference deferredResourceReference in _deferredResourceReferences)
{
Inflate(deferredResourceReference);
}
}
else
{
DeferredResourceReference deferredResourceReference = _deferredResourceReferences.Get(resourceKey);

Inflate(deferredResourceReference);
}

return;

void Inflate(DeferredResourceReference deferredResourceReference)
{
if (deferredResourceReference is not null)
{
// This will inflate the deferred reference, causing it
Expand Down Expand Up @@ -2504,9 +2513,9 @@ private enum PrivateFlags : byte

/// <summary>
/// This wrapper class exists so SourceUriTypeConverterMarkupExtension can pass
/// a more complete Uri to help resolve to the correct assembly, while also passing
/// a more complete Uri to help resolve to the correct assembly, while also passing
/// the original Uri so that ResourceDictionary.Source still returns the original value.
/// </summary>
/// </summary>
internal class ResourceDictionarySourceUriWrapper : Uri
{
public ResourceDictionarySourceUriWrapper(Uri originalUri, Uri versionedUri) : base(originalUri.OriginalString, UriKind.RelativeOrAbsolute)
Expand Down
21 changes: 12 additions & 9 deletions ...rosoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceReferenceExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,10 @@ private void InvalidateCacheValue()
}
}

deferredResourceReference.RemoveFromDictionary();
// 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);
singhashish-wpf marked this conversation as resolved.
Show resolved Hide resolved
}

StopListeningForFreezableChanges(resource);
Expand Down Expand Up @@ -341,8 +344,8 @@ private void InvalidateMentorCache()
internal void InvalidateExpressionValue(object sender, EventArgs e)
{
// VS has a scenario where a TreeWalk invalidates all reference expressions on a DependencyObject.
// If there is a dependency between RRE's,
// invalidating one RRE could cause _targetObject to be null on the other RRE. Hence this check.
// If there is a dependency between RRE's,
// invalidating one RRE could cause _targetObject to be null on the other RRE. Hence this check.
if (_targetObject == null)
{
return;
Expand Down Expand Up @@ -404,7 +407,7 @@ private void ListenForFreezableChanges(object resource)
{
_weakContainerRRE = new ResourceReferenceExpressionWeakContainer(this);
}

// Hook up the event to the weak container to prevent memory leaks (Bug436021)
_weakContainerRRE.AddChangedHandler(resourceAsFreezable);
WriteInternalState(InternalState.IsListeningForFreezableChanges, true);
Expand Down Expand Up @@ -435,7 +438,7 @@ private void StopListeningForFreezableChanges(object resource)
}
}

// It is possible that a freezable was unfrozen during the call to ListForFreezableChanges
// It is possible that a freezable was unfrozen during the call to ListForFreezableChanges
// but was frozen before the call to StopListeningForFreezableChanges
WriteInternalState(InternalState.IsListeningForFreezableChanges, false);
}
Expand Down Expand Up @@ -512,8 +515,8 @@ private enum InternalState : byte
#region ResourceReferenceExpressionWeakContainer

/// <summary>
/// ResourceReferenceExpressionWeakContainer handles the Freezable.Changed event
/// without holding a strong reference to ResourceReferenceExpression.
/// ResourceReferenceExpressionWeakContainer handles the Freezable.Changed event
/// without holding a strong reference to ResourceReferenceExpression.
/// </summary>
private class ResourceReferenceExpressionWeakContainer : WeakReference
{
Expand Down Expand Up @@ -542,7 +545,7 @@ public void AddChangedHandler(Freezable resource)
}

_resource = resource;

Debug.Assert(!_resource.IsFrozen);
_resource.Changed += new EventHandler(this.InvalidateTargetSubProperty);
}
Expand All @@ -558,7 +561,7 @@ public void RemoveChangedHandler()

private Freezable _resource;
}
#endregion
#endregion
}

/// <summary>
Expand Down
39 changes: 29 additions & 10 deletions src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/SystemResources.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1412,7 +1412,7 @@ private static IntPtr SystemThemeFilterMessage(IntPtr hwnd, int msg, IntPtr wPar
}

SystemParameters.InvalidateWindowFrameThicknessProperties();

if(ThemeManager.IsFluentThemeEnabled)
{
ThemeManager.ApplySystemTheme();
Expand Down Expand Up @@ -1733,8 +1733,7 @@ internal override object GetValue(BaseValueSourceInternal valueSource)
// the dictionary else just retun the cached value
if (_dictionary != null)
{
bool canCache;
object value = _dictionary.GetValue(_keyOrValue, out canCache);
object value = _dictionary.GetValue(_keyOrValue, out bool canCache);
if (canCache)
{
// Note that we are replacing the _keyorValue field
Expand Down Expand Up @@ -1790,8 +1789,7 @@ internal override Type GetValueType()
{
// Take a peek at the element type of the ElementStartRecord
// within the ResourceDictionary's deferred content.
bool found;
return _dictionary.GetValueType(_keyOrValue, out found);
return _dictionary.GetValueType(_keyOrValue, out bool _);
}
else
{
Expand All @@ -1800,7 +1798,7 @@ internal override Type GetValueType()
}

// remove this DeferredResourceReference from its ResourceDictionary
internal virtual void RemoveFromDictionary()
protected virtual void RemoveFromDictionary()
{
if (_dictionary != null)
{
Expand Down Expand Up @@ -1970,7 +1968,7 @@ internal override Type GetValueType()
}

// remove this DeferredResourceReference from its ResourceDictionary
internal override void RemoveFromDictionary()
protected override void RemoveFromDictionary()
{
// DeferredThemeResourceReferences are never added to the dictionary's
// list of deferred references, so they don't need to be removed.
Expand Down Expand Up @@ -2034,11 +2032,11 @@ internal override bool IsUnset
#endregion Properties
}

internal class DeferredResourceReferenceList
internal class DeferredResourceReferenceList : IEnumerable<DeferredResourceReference>
{
private readonly object _syncRoot = new();
private readonly Dictionary<object, WeakReference<DeferredResourceReference>> _entries = new();
private int _potentiallyDeadEntryCount = 0;
private int _potentiallyDeadEntryCount;

public void AddOrSet(DeferredResourceReference deferredResourceReference)
{
Expand Down Expand Up @@ -2111,6 +2109,11 @@ private void PurgeIfRequired()
}

private void Purge()
{
Purge(null);
}

private void Purge(List<DeferredResourceReference> aliveItems)
{
lock (_syncRoot)
{
Expand All @@ -2119,10 +2122,14 @@ private void Purge()

foreach (KeyValuePair<object, WeakReference<DeferredResourceReference>> entry in _entries)
{
if (entry.Value.TryGetTarget(out _) == false)
if (entry.Value.TryGetTarget(out var item) is false)
{
deadKeys.Add(entry.Key);
}
else
{
aliveItems?.Add(item);
}
}

foreach (object deadKey in deadKeys)
Expand All @@ -2131,5 +2138,17 @@ private void Purge()
}
}
}

public IEnumerator<DeferredResourceReference> GetEnumerator()
{
var aliveItems = new List<DeferredResourceReference>(_entries.Count);
Purge(aliveItems);
return aliveItems.GetEnumerator();
}

IEnumerator IEnumerable.GetEnumerator()
{
return GetEnumerator();
}
}
}
5 changes: 3 additions & 2 deletions src/Microsoft.DotNet.Wpf/src/WindowsBase/System/Windows/DependencyObject.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -863,7 +863,7 @@ internal bool ProvideSelfAsInheritanceContext( DependencyObject doValue, Depende
// on side-effects from setting the "Freezable context". Freezable's
// implementation does its own checks of the conditions omitted here.
// Enhancement suggestion: Freezable should follow the same rules for
// InheritanceContext as everyone else
// InheritanceContext as everyone else


if (doValue != null &&
Expand Down Expand Up @@ -910,7 +910,7 @@ internal bool RemoveSelfAsInheritanceContext( DependencyObject doValue, Dependen
// on side-effects from setting the "Freezable context". Freezable's
// implementation does its own checks of the conditions omitted here.
// Enhancement suggestion: Freezable should follow the same rules for
// InheritanceContext as everyone else
// InheritanceContext as everyone else


if (doValue != null &&
Expand Down Expand Up @@ -3534,6 +3534,7 @@ internal enum UpdateResult
}

[FriendAccessAllowed] // Built into Base, also used by Framework.
[Flags]
internal enum RequestFlags
{
FullyResolved = 0x00,
Expand Down
Loading