[Android] Fix app crash caused by dynamic template switching in ListView #24808

BagavathiPerumal · 2024-09-17T15:49:15Z

Exception I – Object reference not set to an instance of an object at GraphicsExtensions.AsColor :

During the execution of the device test case - ChangingTemplateTypeDoesNotCrash, a fatal exception was thrown, causing a crash.

Root Cause

The root cause of the NullReferenceException was identified as a failure to check for a valid context or color before calling the AsColor method. Specifically, the line of code responsible for this was:

var color = Current?.Windows?.Count > 0 
            ? Current.Windows[0].MauiContext.Context?.GetAccentColor() 
            : null;

In the UpdateSeparatorColor method, when returning null for Application.AccentColor , the method requests:
bline.SetBackgroundColor(separatorColor.ToPlatform(Application.AccentColor)); Since no default separator color was set, this led to the issue. The failure was particularly observed in our test case that enabled grouping, which is why only the ChangingTemplateTypeDoesNotCrash test case was failing.

Description of Change

To resolve this issue, the following modifications were implemented in the relevant code segment:

Default Color Implementation: A fallback mechanism was implemented to provide a default color (e.g., Color.FromRgba(50, 79, 133, 255)) when the window.count resource is unavailable. This approach prevents the application from crashing and ensures that the UI continues to function smoothly even in command line device test cases.

Exception II – The specified child already has a parent. you must call `removeView()` on the child’s parent first at `ViewGroup.addViewInner`.

Root cause

In the ViewCellRenderer class, the platform view was being recreated for each cell without properly removing the existing parent view (i.e., the ViewCellContainer ) before attempting to add the new view. This caused an exception:'The specified child already has a parent. You must call removeView() on the child's parent first. 'The issue arose due to improper handling of the platform view's parent when switching between different DataTemplates in a ListView using the RetainElement caching strategy in .NET MAUI.

Description of Issue Fix

The fix involved checking if the platformView already had a parent (i.e., ViewGroup) and ensuring that the parent was removed before adding the platformView to the new container. By introducing this conditional check, the system prevents the platform view from being associated with multiple parents, which caused the "The specified child already has a parent" exception. This change ensures that dynamic template changes in the ListView are handled correctly, improving view recycling and maintaining stable rendering without conflicts.

Tested the behavior in the following platforms.

Android
Windows
iOS
Mac

Issues Fixed

Fixes #24701

Output

Before fix:

After fix:

Testcase:

The Testcase is already available for this fix. Please refer to the PR link below for reference.

Clear out prototype cell when changing ItemSource by PureWeen · Pull Request #24700 · dotnet/maui (github.com)

PureWeen · 2024-09-18T13:09:04Z

/azp run

azure-pipelines · 2024-09-18T13:09:27Z

Azure Pipelines successfully started running 3 pipeline(s).

PureWeen

Can you re-enable this device test for android?

https://github.com/dotnet/maui/pull/24700/files#diff-9ef75bbb1d7dbaf152e43bba50d7dfc6ed46b9db1b351e0ff5da3272718f811aR36-R41

BagavathiPerumal · 2024-09-23T06:49:14Z

Can you re-enable this device test for android?

https://github.com/dotnet/maui/pull/24700/files#diff-9ef75bbb1d7dbaf152e43bba50d7dfc6ed46b9db1b351e0ff5da3272718f811aR36-R41

@PureWeen, I have modified changes. Could you please validate it once and let me know if there are any further concerns.

azure-pipelines · 2024-09-23T07:02:09Z

Azure Pipelines successfully started running 3 pipeline(s).

PureWeen

Looks like the test is failing

09-23 03:31:56.106  4471  4471 E AndroidRuntime: FATAL EXCEPTION: main
09-23 03:31:56.106  4471  4471 E AndroidRuntime: Process: com.microsoft.maui.controls.devicetests, PID: 4471
09-23 03:31:56.106  4471  4471 E AndroidRuntime: android.runtime.JavaProxyThrowable: [System.NullReferenceException]: Object reference not set to an instance of an object.
09-23 03:31:56.106  4471  4471 E AndroidRuntime: 	at Microsoft.Maui.Graphics.Platform.GraphicsExtensions.AsColor(Unknown Source:0)
09-23 03:31:56.106  4471  4471 E AndroidRuntime: 	at Microsoft.Maui.Platform.ColorExtensions.ToPlatform(Unknown Source:0)
09-23 03:31:56.106  4471  4471 E AndroidRuntime: 	at Microsoft.Maui.Platform.ColorExtensions.ToPlatform(Unknown Source:0)
09-23 03:31:56.106  4471  4471 E AndroidRuntime: 	at Microsoft.Maui.Controls.Handlers.Compatibility.ListViewAdapter.UpdateSeparatorColor(Unknown Source:0)
09-23 03:31:56.106  4471  4471 E AndroidRuntime: 	at Microsoft.Maui.Controls.Handlers.Compatibility.ListViewAdapter.GetView(Unknown Source:0)
09-23 03:31:56.106  4471  4471 E AndroidRuntime: 	at Microsoft.Maui.Controls.Handlers.Compatibility.ListViewRenderer.GetDesiredSize(Unknown Source:0)
09-23 03:31:56.106  4471  4471 E AndroidRuntime: 	at Microsoft.Maui.Controls.Handlers.Compatibility.VisualElementRenderer`1[[Microsoft.Maui.Controls.ListView, Microsoft.Maui.Controls, Version=1.0.0.0, Culture=neutral,

azure-pipelines · 2024-09-25T11:42:18Z

Azure Pipelines successfully started running 3 pipeline(s).

jfversluis · 2024-09-27T12:07:41Z

/rebase

azure-pipelines · 2024-09-27T12:13:27Z

Azure Pipelines successfully started running 3 pipeline(s).

praveenkumarkarunanithi · 2024-10-03T11:14:59Z

/rebase

azure-pipelines · 2024-10-03T12:34:12Z

Azure Pipelines successfully started running 3 pipeline(s).

jsuarezruiz · 2024-10-09T11:57:09Z

src/Controls/src/Core/Compatibility/Handlers/ListView/Android/ViewCellRenderer.cs

+
+				if (platformView.Parent != null && platformView.Parent is ViewGroup parentViewGroup)
+				{
+					parentViewGroup.RemoveView(platformView);


The RemoveFromParent extension method can be used here.

platformView.RemoveFromParent();

@jsuarezruiz, Thank you for your suggestion regarding the use of the RemoveFromParent extension method. I implemented this fix; however, the device test case is still experiencing app crashes.

Upon further investigation, we discovered that the crash occurs during the execution of the ChangingTemplateTypeDoesNotCrash test case. The issue arises because grouping is enabled, but the data provided is not in a grouped format, leading to the app crashing when running the command-line device test case.
We validated this by running a standalone sample with the fix applied, and the same test case also passed without any issues when checked manually. The crashes seem to only occur during command-line execution.

We are currently delving deeper into this issue and would greatly appreciate any insights you may have.

BagavathiPerumal · 2024-10-11T09:44:45Z

Looks like the test is failing

09-23 03:31:56.106  4471  4471 E AndroidRuntime: FATAL EXCEPTION: main
09-23 03:31:56.106  4471  4471 E AndroidRuntime: Process: com.microsoft.maui.controls.devicetests, PID: 4471
09-23 03:31:56.106  4471  4471 E AndroidRuntime: android.runtime.JavaProxyThrowable: [System.NullReferenceException]: Object reference not set to an instance of an object.
09-23 03:31:56.106  4471  4471 E AndroidRuntime: 	at Microsoft.Maui.Graphics.Platform.GraphicsExtensions.AsColor(Unknown Source:0)
09-23 03:31:56.106  4471  4471 E AndroidRuntime: 	at Microsoft.Maui.Platform.ColorExtensions.ToPlatform(Unknown Source:0)
09-23 03:31:56.106  4471  4471 E AndroidRuntime: 	at Microsoft.Maui.Platform.ColorExtensions.ToPlatform(Unknown Source:0)
09-23 03:31:56.106  4471  4471 E AndroidRuntime: 	at Microsoft.Maui.Controls.Handlers.Compatibility.ListViewAdapter.UpdateSeparatorColor(Unknown Source:0)
09-23 03:31:56.106  4471  4471 E AndroidRuntime: 	at Microsoft.Maui.Controls.Handlers.Compatibility.ListViewAdapter.GetView(Unknown Source:0)
09-23 03:31:56.106  4471  4471 E AndroidRuntime: 	at Microsoft.Maui.Controls.Handlers.Compatibility.ListViewRenderer.GetDesiredSize(Unknown Source:0)
09-23 03:31:56.106  4471  4471 E AndroidRuntime: 	at Microsoft.Maui.Controls.Handlers.Compatibility.VisualElementRenderer`1[[Microsoft.Maui.Controls.ListView, Microsoft.Maui.Controls, Version=1.0.0.0, Culture=neutral,

@PureWeen, The test failure is caused by the UpdateSeparatorColor method, which calls the default separator color through
Application.AccentColor. We have addressed this issue by implementing a fix at this line of code, where a default color is now provided to prevent a NullReferenceException.

jsuarezruiz · 2024-10-11T11:29:37Z

/azp run

azure-pipelines · 2024-10-11T11:29:55Z

Azure Pipelines successfully started running 3 pipeline(s).

praveenkumarkarunanithi · 2024-10-14T09:11:08Z

/rebase

…e-at-device-test

jsuarezruiz · 2024-10-14T10:02:47Z

/azp run

azure-pipelines · 2024-10-14T10:03:08Z

Azure Pipelines successfully started running 3 pipeline(s).

PureWeen · 2024-10-15T17:55:29Z

src/Controls/src/Core/Compatibility/Handlers/ListView/Android/ViewCellRenderer.cs

@@ -244,6 +244,12 @@ public void Update(ViewCell cell)

 				var platformView = _viewCell.View.ToPlatform(Element.FindMauiContext());
 				_viewHandler = (IPlatformViewHandler)_viewCell.View.Handler;
+
+				if (platformView?.Parent is ViewGroup)


If you're using RemoveFromParent I don't think you need the if statement here

@PureWeen, I have modified the code based on your concerns, could you please validate once and let me know if there are any concerns.

…n RemoveFromParent().

azure-pipelines · 2024-10-16T13:26:27Z

Azure Pipelines successfully started running 3 pipeline(s).

PureWeen · 2024-10-17T21:36:10Z

src/Controls/src/Core/Application/Application.cs

-			return null;
+			return Current?.Windows?.Count > 0
+			? Current.Windows[0].MauiContext.Context?.GetAccentColor()
+			: Color.FromRgba(50, 79, 133, 255);


What's this color based on?

Can the code that's calling this be fixed to handle a null value better?

@PureWeen , I've updated the code to handle null values more effectively. Specifically, we are now using a default fallback color of "#ff33b5e5" when both the current accent color and the passed defaultColor are null. This fallback color is based on the reference code from the GetAccentColor method in ContextExtensions.cs here, ensuring consistency with the default Android accent color in working scenarios.
Could you please review and let me know if there are any further concerns?

…fix-24701

jsuarezruiz · 2024-10-24T14:27:07Z

/azp run

azure-pipelines · 2024-10-24T14:27:30Z

Azure Pipelines successfully started running 3 pipeline(s).

jsuarezruiz · 2024-10-25T10:03:46Z

src/Core/src/Platform/Android/ColorExtensions.cs

@@ -17,7 +17,7 @@ public static AColor ToPlatform(this Color self, int defaultColorResourceId, Con
 			=> self?.ToPlatform() ?? new AColor(ContextCompat.GetColor(context, defaultColorResourceId));

 		public static AColor ToPlatform(this Color? self, Color defaultColor)


This extension is used every time we convert a MAUI Color to an Android Color (everywhere, in any View). If the color is null (default value) and not pass a default value, will use this #ff33b5e5 color

And could cause unexpected behaviors.

In this case, the separator line must use the color from the SeparatorColor property, but if it is not set and is null, a color from an Android Resource such as global::Android.Resource.Attribute.ColorAccent should be used. So, we need to identify why the AccentColor is null.

@jfversluis, Thank you for the feedback! I've updated the code to address the potential null values effectively and to avoid unintended defaulting to #ff33b5e5 across views. Specifically, I’ve ensured that the color retrieval method now uses the primary context when available via Current.Windows[0].MauiContext.Context, and if not, it falls back to Current?.FindMauiContext()?.Context (During device tests).

Please review and let us know if any further changes are required.

jsuarezruiz · 2024-10-28T11:15:25Z

/azp run

azure-pipelines · 2024-10-28T11:15:46Z

Azure Pipelines successfully started running 3 pipeline(s).

jsuarezruiz · 2024-10-29T12:38:25Z

src/Controls/src/Core/Application/Application.cs

-				return Current.Windows[0].MauiContext.Context?.GetAccentColor();
+			var context = Current?.Windows?.Count > 0 ? Current.Windows[0].MauiContext.Context : Current?.FindMauiContext()?.Context;
+			if (context is not null)
+				return context?.GetAccentColor();


If the current Window MauiContext is null, we keep having the same null Color exception, right?
We could fallback to Application.Current.FindMauiContext() in that case.

@jsuarezruiz , Thank you for the feedback. I’ve added a fallback to ensure context is retrieved effectively in all cases, even when the primary window context might not be accessible.

Please review and let us know if any further changes are required.

jsuarezruiz · 2024-10-30T08:58:54Z

/azp run

azure-pipelines · 2024-10-30T08:59:14Z

Azure Pipelines successfully started running 3 pipeline(s).

praveenkumarkarunanithi · 2024-11-04T08:09:23Z

/rebase

…fix-24701

jsuarezruiz · 2024-11-06T10:23:09Z

/azp run

azure-pipelines · 2024-11-06T10:23:30Z

Azure Pipelines successfully started running 3 pipeline(s).

dotnet-policy-service bot added the community ✨ Community Contribution label Sep 17, 2024

BagavathiPerumal marked this pull request as ready for review September 17, 2024 16:33

BagavathiPerumal requested a review from a team as a code owner September 17, 2024 16:33

BagavathiPerumal requested review from jfversluis and tj-devel709 September 17, 2024 16:33

PureWeen requested changes Sep 20, 2024

View reviewed changes

PureWeen requested changes Sep 23, 2024

View reviewed changes

github-actions bot force-pushed the fix-24701 branch from 09f3833 to aec2b55 Compare September 27, 2024 12:08

github-actions bot force-pushed the fix-24701 branch from aec2b55 to 07f8c3b Compare October 3, 2024 11:15

PureWeen marked this pull request as draft October 4, 2024 22:16

jsuarezruiz reviewed Oct 9, 2024

View reviewed changes

BagavathiPerumal marked this pull request as ready for review October 11, 2024 09:47

BagavathiPerumal added 3 commits October 14, 2024 09:11

fix-24701--Removed-Parent-from-PlatformView-Before-Adding-to-View.

92e77a3

Fix-24701-Re-enabled the device test for android.

8d3aa8c

fix-24701-Default-color-set-when-current-device-count-is-not-availabl…

403de20

…e-at-device-test

github-actions bot force-pushed the fix-24701 branch from c55e1fc to 403de20 Compare October 14, 2024 09:11

PureWeen requested changes Oct 15, 2024

View reviewed changes

fix-24701-Removed a redundant if condition that was already checked i…

ec5b543

…n RemoveFromParent().

PureWeen requested changes Oct 17, 2024

View reviewed changes

Merge branch 'main' of https://github.com/BagavathiPerumal/maui into …

f3215dc

…fix-24701

jsuarezruiz added platform/android 🤖 area-controls-listview ListView and TableView labels Oct 23, 2024

SuthiYuvaraj and others added 2 commits October 23, 2024 15:21

Merge branch 'dotnet:main' into fix-24701

7f8452e

commit to handle null value effectively

7d45b24

jsuarezruiz requested changes Oct 25, 2024

View reviewed changes

Commit for review concerns.

71674d1

jsuarezruiz requested changes Oct 29, 2024

View reviewed changes

SuthiYuvaraj added 2 commits October 29, 2024 19:58

Update Application.cs

23c1768

Update Application.cs

d1f1f7d

PureWeen added this to the .NET 9 Servicing milestone Oct 31, 2024

Merge branch 'main' of https://github.com/BagavathiPerumal/maui into …

bc32100

…fix-24701

		@@ -17,7 +17,7 @@ public static AColor ToPlatform(this Color self, int defaultColorResourceId, Con
		=> self?.ToPlatform() ?? new AColor(ContextCompat.GetColor(context, defaultColorResourceId));

		public static AColor ToPlatform(this Color? self, Color defaultColor)

[Android] Fix app crash caused by dynamic template switching in ListView #24808

Are you sure you want to change the base?

[Android] Fix app crash caused by dynamic template switching in ListView #24808

Conversation

BagavathiPerumal commented Sep 17, 2024 • edited Loading

Exception I – Object reference not set to an instance of an object at GraphicsExtensions.AsColor :

Root Cause

Description of Change

Exception II – The specified child already has a parent. you must call removeView() on the child’s parent first at ViewGroup.addViewInner.

Root cause

Description of Issue Fix

Issues Fixed

Output

Testcase:

PureWeen commented Sep 18, 2024 • edited by jfversluis Loading

azure-pipelines bot commented Sep 18, 2024

PureWeen left a comment

Choose a reason for hiding this comment

BagavathiPerumal commented Sep 23, 2024

azure-pipelines bot commented Sep 23, 2024

PureWeen left a comment

Choose a reason for hiding this comment

azure-pipelines bot commented Sep 25, 2024

jfversluis commented Sep 27, 2024

azure-pipelines bot commented Sep 27, 2024

praveenkumarkarunanithi commented Oct 3, 2024

azure-pipelines bot commented Oct 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BagavathiPerumal commented Oct 11, 2024

jsuarezruiz commented Oct 11, 2024

azure-pipelines bot commented Oct 11, 2024

praveenkumarkarunanithi commented Oct 14, 2024

jsuarezruiz commented Oct 14, 2024 • edited by jfversluis Loading

azure-pipelines bot commented Oct 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

azure-pipelines bot commented Oct 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsuarezruiz commented Oct 24, 2024

azure-pipelines bot commented Oct 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsuarezruiz commented Oct 28, 2024

azure-pipelines bot commented Oct 28, 2024

Choose a reason for hiding this comment

SuthiYuvaraj Oct 29, 2024 • edited Loading

Choose a reason for hiding this comment

jsuarezruiz commented Oct 30, 2024

azure-pipelines bot commented Oct 30, 2024

praveenkumarkarunanithi commented Nov 4, 2024

jsuarezruiz commented Nov 6, 2024

azure-pipelines bot commented Nov 6, 2024

BagavathiPerumal commented Sep 17, 2024 •

edited

Loading

Exception II – The specified child already has a parent. you must call `removeView()` on the child’s parent first at `ViewGroup.addViewInner`.

PureWeen commented Sep 18, 2024 •

edited by jfversluis

Loading

jsuarezruiz commented Oct 14, 2024 •

edited by jfversluis

Loading

SuthiYuvaraj Oct 29, 2024 •

edited

Loading