VisualElement Background/Shadow Notify code creates a strong relationship between the brush and the element

[PureWeen]

Description

Overall, we need to create a generalized strategy for handling Brush changes. There are 5 or 6 places where we use brushes but currently Background and Shape are the only ones that have some notification code.

Currently this is the code we have for Background but it's problematic.

maui/src/Controls/src/Core/VisualElement.cs

Lines 246 to 268 in cd5dd03

	void NotifyBackgroundChanges()
	{
	if (Background != null)
	{
	Background.Parent = this;
	Background.PropertyChanged += OnBackgroundChanged;
	if (Background is GradientBrush gradientBrush)
	gradientBrush.InvalidateGradientBrushRequested += InvalidateGradientBrushRequested;
	}
	}
	void StopNotifyingBackgroundChanges()
	{
	if (Background != null)
	{
	Background.Parent = null;
	Background.PropertyChanged -= OnBackgroundChanged;
	if (Background is GradientBrush gradientBrush)
	gradientBrush.InvalidateGradientBrushRequested -= InvalidateGradientBrushRequested;
	}
	}

• The first low hanging fruit issue here is that the Parent property probably shouldn't get set for every single type of brush. For example, right now if you have the following code

new TabbedPage()
{
     Title = "Tabbed Page",
     Background = SolidColorBrush.Green
};

That brush is an ImmutableBrush that will be reused everywhere that SolidColorBrush.Green is being used.

This can cause undesirable side effects because an element that's no longer on the screen might be set as the Parent of SolidColorBrush. This can cause events to propagate up from the brush to the element that's no longer on the screen. I saw this when running the iOS device tests. Whenever I ran a test for a second time it was causing a threading exception because the previous TabbedPage was set as parent on the ImmutableBrush which would then propagate up to the previous test run.

We can easily cover ImmutableBrush cases.

• The second part here that I'm not sure about is if VisualElement should subscribe to the PropertyChanged event and if Parent should be set on any of the Brushes. Since brushes can be reused (StaticResource) this seems like it will cause undesirable behavior. I think we need a different weaker pattern here for propagating brush changes up.

[ghost]

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.