Remove the obsolete AndroidClientHandler class - #11393
Conversation
There was a problem hiding this comment.
Pull request overview
Removes the deprecated Xamarin.Android.Net.AndroidClientHandler class so only AndroidMessageHandler remains as the native HTTP handler. The $(AndroidHttpClientHandlerType) MSBuild property and the XA_HTTP_CLIENT_HANDLER_TYPE environment variable are now ignored (with the property producing a build error if set), and the related MSBuild plumbing, error codes (XA1031/XA1032/XA1033), runtime selection logic, docs, and tests are deleted. A small unrelated tweak in AndroidMessageHandler.HandleRedirect short-circuits cross-scheme redirects to let the client decide.
Changes:
- Delete
AndroidClientHandler(and its.Legacy.cs), update public API baselines on API-35/36/36.1/37, and add API-compat acceptable-breakages entries. - Remove
CheckClientHandlerTypetask,XA1031/1032/1033resources/docs,HttpClientHandlerTypetask properties,XA_HTTP_CLIENT_HANDLER_TYPEenv injection,AndroidEnvironment.GetHttpMessageHandlerselection logic, and related tests. - Refactor
_CheckAndroidHttpClientHandlerTypetarget to emit an MSBuild<Error/>when the property is set; add cross-scheme redirect handling inAndroidMessageHandler; splitHttpClientIntegrationTestsinto a singleAndroidMessageHandlerIntegrationTestsfixture.
Reviewed changes
Copilot reviewed 44 out of 46 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Mono.Android/Xamarin.Android.Net/AndroidClientHandler.cs, AndroidClientHandler.Legacy.cs | Delete the deprecated wrapper and its legacy implementation |
| src/Mono.Android/Xamarin.Android.Net/AndroidMessageHandler.cs | Drop unused *Internal shims; add cross-scheme redirect short-circuit |
| src/Mono.Android/Android.Runtime/AndroidEnvironment.cs | Replace XA_HTTP_CLIENT_HANDLER_TYPE-based selection with a hard-coded new AndroidMessageHandler() |
| src/Mono.Android/Mono.Android.csproj | Drop AndroidClientHandler.cs from compilation |
| src/Mono.Android/PublicAPI/API-3{5,6,6.1,7}/PublicAPI.{Shipped,Unshipped}.txt | Move AndroidClientHandler API surface to REMOVED |
| src/Microsoft.Android.Sdk.ILLink/MarkJavaObjects.cs | Remove custom HTTP handler preservation logic |
| src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets | Drop CheckClientHandlerType UsingTask; replace check target body with an <Error/>; stop forwarding HttpClientHandlerType |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/*.targets | Drop HttpClientHandlerType parameter wiring and the default AndroidHttpClientHandlerType / UseNativeHttpHandler derivation |
| src/Xamarin.Android.Build.Tasks/Tasks/CheckClientHandlerType.cs | Remove obsolete task |
| src/Xamarin.Android.Build.Tasks/Tasks/Generate*Sources.cs | Drop HttpClientHandlerType task property and threading through AddDefaultEnvironmentVariables |
| src/Xamarin.Android.Build.Tasks/Utilities/Environment{Builder,FilesParser}.cs | Remove AddHttpClientHandlerType/HaveHttpMessageHandler plumbing |
| src/Xamarin.Android.Build.Tasks/Properties/Resources.Designer.cs | Remove XA1031/XA1031_HCH/XA1032/XA1033 accessors |
| src/Xamarin.Android.Build.Tasks/Tests/.../CheckClientHandlerTypeTests.cs, LinkerTests.cs, EnvironmentContentTests.cs | Delete tests that exercised the removed plumbing |
| src/profiled-aot/dotnet.aotprofile.txt | Drop IsAcceptableHttpMessageHandlerType entry |
| tests/api-compatibility/acceptable-breakages-vReference-net11.0.txt | Empty out the compat exception list (now reset to just the header) |
| tests/Mono.Android-Tests/.../{HttpClientIntegrationTests.cs,AndroidClientHandlerTests.cs,Android.Runtime/AndroidEnvironmentTest.cs} | Remove handler-agnostic integration tests and AndroidEnvironment unit test |
| tests/Mono.Android-Tests/.../AndroidMessageHandlerIntegrationTests.cs | New consolidated integration test fixture targeting only AndroidMessageHandler |
| tests/Mono.Android-Tests/Mono.Android-Tests/Mono.Android.NET-Tests.csproj | Update compile list and remove AndroidClientHandler category from exclusions |
| tests/Mono.Android-Tests/Mono.Android-Tests/TrimmerRoots.xml | Stop preserving AndroidClientHandler |
| Documentation/docs-mobile/{messages/{xa1031,xa1032,xa1033}.md,index.md,TOC.yml,building-apps/build-properties.md} | Remove docs for retired error codes and the AndroidHttpClientHandlerType property |
| .github/skills/tests/references/test-catalog.md | Drop references to AndroidClientHandler test/category |
Files not reviewed (1)
- src/Xamarin.Android.Build.Tasks/Properties/Resources.Designer.cs: Language not supported
Fix relative redirect handling by checking IsAbsoluteUri before comparing schemes, and escape the UseNativeHttpHandler property reference in the unsupported AndroidHttpClientHandlerType error message. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/review |
|
✅ Android PR Reviewer completed successfully! |
There was a problem hiding this comment.
⚠️ Needs Changes
Clean PR that successfully removes the obsolete AndroidClientHandler class, simplifies GetHttpMessageHandler() to directly instantiate AndroidMessageHandler, and properly redirects users to $(UseNativeHttpHandler). The redirect behavior change for cross-scheme redirects is a nice alignment with SocketsHttpHandler. Public API tracking and API compatibility breakage files look correct.
However, the removal is incomplete — several references to the removed functionality were left behind:
Issues by severity
| Severity | Count |
|---|---|
| ❌ Error | 1 |
| 2 | |
| 💡 Suggestion | 3 |
Key issues
-
❌ Build break:
EnvironmentFilesParserTests.DetectsHttpMessageHandlerstill references the removedHaveHttpMessageHandlerproperty — this will fail to compile. -
⚠️ Orphaned code:GenerateNativeApplicationConfigSources.HttpClientHandlerTypeproperty andEnvironmentBuilder.defaultHttpMessageHandlerfield are unused after this PR (the latter will triggerTreatWarningsAsErrors). -
⚠️ Orphaned resource entries: XA1031, XA1031_HCH, XA1032, XA1033 were removed fromResources.Designer.csbut still exist inResources.resx.
Positive callouts
- The
GetHttpMessageHandler()simplification is a great improvement — direct instantiation eliminates reflection,DynamicDependency,Type.GetType, and theXA_HTTP_CLIENT_HANDLER_TYPEenv var indirection. - Good use of
<Error>in targets to give users a clear migration path. - The redirect scheme change is well-isolated and the
disposeRet = falsepattern correctly delegates to the caller.
Generated by Android PR Reviewer for issue #11393 · ● 29.6M
|
@copilot address review |
Agent-Logs-Url: https://github.com/dotnet/android/sessions/0c11fefb-96ec-4116-b235-d8bd5c4adadd Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
|
@copilot fix build: |
Agent-Logs-Url: https://github.com/dotnet/android/sessions/9ee1cba8-fd60-4ded-8d90-a256e0ca2402 Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/android/sessions/9ee1cba8-fd60-4ded-8d90-a256e0ca2402 Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
... Fixed in 21c1cbd (with follow-up cleanup in f5e7f1f). I restored |
Remove obsolete linker and test leftovers from the AndroidClientHandler removal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the removed HttpClientIntegrationTests fixture with AndroidMessageHandlerIntegrationTests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Document that AndroidHttpClientHandlerType is unsupported in .NET 11 and should be removed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Addressed all Android PR Reviewer comments in follow-up commits.
Agent-Logs-Url: https://github.com/dotnet/android/sessions/496ee485-21a1-4759-a312-5f7b41f6a75c Co-authored-by: jonathanpeppers <840039+jonathanpeppers@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/android/sessions/496ee485-21a1-4759-a312-5f7b41f6a75c Co-authored-by: jonathanpeppers <840039+jonathanpeppers@users.noreply.github.com>
|
/review |
|
✅ Android PR Reviewer completed successfully! |
There was a problem hiding this comment.
Review Summary — ⚠️ Needs attention
Overall: Clean, thorough removal of the obsolete AndroidClientHandler class and its supporting infrastructure. The code deletion is consistent — all references to AndroidClientHandler, XA_HTTP_CLIENT_HANDLER_TYPE, HttpClientHandlerType, CheckClientHandlerType, and error codes XA1031–XA1033 are properly cleaned up. API compatibility breakages are documented, PublicAPI files are updated, and the migration error message in _CheckAndroidHttpClientHandlerType provides clear guidance.
Positive callouts
GetHttpMessageHandlersimplification — The old reflection-based handler resolution with trimming annotations ([DynamicDependency],[UnconditionalSuppressMessage]) is replaced with a directnew AndroidMessageHandler(). Much safer and simpler.- Pre-existing bugs removed — The deleted
ConfigureKeyManagerFactoryInternalandConfigureCustomSSLSocketFactoryInternalmethods were infinitely recursive (calling themselves instead of the non-Internalvariants). Good riddance. - Test restructuring — Renaming
AndroidClientHandlerTests.cs→AndroidHandlerTestBase.csand consolidatingHttpClientIntegrationTests.csintoAndroidMessageHandlerIntegrationTests.csproperly eliminates the dual-handler test hierarchy.
Issues by severity
| Severity | Count | Summary |
|---|---|---|
| 1 | Redirect scheme-change logic is a separate behavior change that blocks HTTP→HTTPS redirects | |
| 💡 Suggestion | 2 | Coded error for migration target, docs expansion |
CI Status
Public CI checks (license/cla, dotnet-android) are ✅ passing.
Generated by Android PR Reviewer for issue #11393 · ● 17.9M
This PR removes the obsolete AndroidClientHandler class and leaves just the AndroidMessageHandler class as the native http handler. The
$(AndroidHttpClientHandlerType)property and theXA_HTTP_CLIENT_HANDLER_TYPEenvironment variable are now ignored.