Skip to content

DynamicResource support for app resource changes#6354

Open
miloush wants to merge 2 commits intodotnet:mainfrom
miloush:DynamicResource-MonitorAppSystem
Open

DynamicResource support for app resource changes#6354
miloush wants to merge 2 commits intodotnet:mainfrom
miloush:DynamicResource-MonitorAppSystem

Conversation

@miloush
Copy link
Contributor

@miloush miloush commented Apr 4, 2022

Fixes #6302

Description

Changes in application resources are not picked up by DynamicResource extension if the target object is not in the visual tree (i.e. has no mentor). It already has code to access App/System resources in the absence of mentor which suggests this was meant to be a supported scenario.

This PR introduces internal ResourcesChanged instance event on the Application object and subscribes the ResourceReferenceExpression to the changes when the target element does not have a mentor.

Customer Impact

DynamicResource extension on DependencyObjects such as DataGridColumns or other user types will not track the changes to the application resources. However, the extension reads them correct the first time, making this issue difficult to diagnose.

Regression

No.

Testing

Built an updated PresentationFramework.dll and tested with the repro project in #6302 on 6.0.2 x86 app.
Built again and tested on 9.0.0 Preview 6 with changing system colors.

Risk

No changes to public API. No changes to behavior when the resource does not come from App/System resources, or when the target object has a mentor. At runtime, each instance of ResourceReferenceExpression will create a strong reference to the Application instance, but only in the affected case.

@ghost ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Apr 4, 2022
@ghost ghost requested review from SamBent, dipeshmsft and singhashish-wpf April 4, 2022 14:15
@ghost ghost added Community Contribution A label for all community Contributions draft labels Apr 4, 2022
@anjaligupta-dev anjaligupta-dev self-assigned this May 27, 2022
@ghost ghost assigned miloush May 27, 2022
@anjaligupta-dev anjaligupta-dev self-requested a review May 27, 2022 07:59
anjaligupta-dev
anjaligupta-dev previously approved these changes Jun 1, 2022
@anjaligupta-dev anjaligupta-dev self-requested a review June 1, 2022 04:52
@batzen
Copy link
Contributor

batzen commented Jun 1, 2022

Doesn't the event subscription create a strong reference between application and expression, thus holding all expressions in memory for the lifetime of the application? Before the change the expression could be garbage collected as soon as the mentor could be. I guess this change will cause a massive memory leak.

@batzen
Copy link
Contributor

batzen commented Jun 1, 2022

Ah, i was blind...

@miloush
Copy link
Contributor Author

miloush commented Oct 7, 2022

How do we feel about this one, do we want this issue to be addressed and is this a reasonable way to do so? (It appears this is waiting on me for being a draft.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Community Contribution A label for all community Contributions Included in test pass PR metadata: Label to tag PRs, to facilitate with triage Status:Completed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DynamicResource is not working for Header property in DataGridTextColumn

4 participants