Skip to content

[android] fix DI registration for Frame, ListView, TableView - #16989

Merged
PureWeen merged 1 commit into
dotnet:mainfrom
jonathanpeppers:AndroidFixFrameListViewTableView
Aug 25, 2023
Merged

PureWeen merged 1 commit into
dotnet:mainfrom
jonathanpeppers:AndroidFixFrameListViewTableView

Conversation

@jonathanpeppers

Copy link
Copy Markdown
Member

This partially reverts b0bba51.

When running:

.\bin\dotnet\dotnet.exe build src\Controls\samples\Controls.Sample\Maui.Controls.Sample.csproj -f net7.0-android -t:Run

The app crashes on the "Compatibility" screen with:

System.InvalidOperationException: No service for type 'Android.Content.Context' has been registered.

These are caused by the change:

--handlersCollection.AddHandler(typeof(Frame), typeof(Handlers.Compatibility.FrameRenderer));
++handlersCollection.AddHandler<Frame>(svc => new Handlers.Compatibility.FrameRenderer(svc.GetRequiredService<Android.Content.Context>()));

It turns out this code path only works because:

  1. Microsoft.Extensions.DI throws a MissingMethodException

  2. MAUI has fallback logic to catch this and call:

return (IElementHandler)Extensions.DependencyInjection.
    ActivatorUtilities.CreateInstance(mauiContext.Services, handlerType, mauiContext.Context);

ActivatorUtilities is part of Microsoft.Extensions.DI.

For now, let's just revert this change for Android for these three types: Frame, ListView, and TableView.

I spend some time trying to "unwind" this and avoid the MissingMethodException on startup. I couldn't solve it yet, but plan to come back to it later.

This partially reverts b0bba51.

When running:

    .\bin\dotnet\dotnet.exe build src\Controls\samples\Controls.Sample\Maui.Controls.Sample.csproj -f net7.0-android -t:Run

The app crashes on the "Compatibility" screen with:

    System.InvalidOperationException: No service for type 'Android.Content.Context' has been registered.

These are caused by the change:

    --handlersCollection.AddHandler(typeof(Frame), typeof(Handlers.Compatibility.FrameRenderer));
    ++handlersCollection.AddHandler<Frame>(svc => new Handlers.Compatibility.FrameRenderer(svc.GetRequiredService<Android.Content.Context>()));

It turns out this code path only works because:

1. Microsoft.Extensions.DI throws a `MissingMethodException`

2. MAUI has fallback logic to catch this and call:

    return (IElementHandler)Extensions.DependencyInjection.
        ActivatorUtilities.CreateInstance(mauiContext.Services, handlerType, mauiContext.Context);

`ActivatorUtilities` is part of Microsoft.Extensions.DI.

For now, let's just revert this change for Android for these three
types: `Frame`, `ListView`, and `TableView`.

I spend some time trying to "unwind" this and avoid the
`MissingMethodException` on startup. I couldn't solve it yet, but plan
to come back to it later.
@jonathanpeppers
jonathanpeppers requested a review from a team as a code owner August 24, 2023 19:54
@PureWeen
PureWeen enabled auto-merge (squash) August 24, 2023 19:58
@PureWeen

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 2 pipeline(s).

#if WINDOWS || ANDROID || IOS || MACCATALYST || TIZEN
handlersCollection.AddHandler<ListView>(svc => new Handlers.Compatibility.ListViewRenderer(
#if ANDROID
svc.GetRequiredService<Android.Content.Context>()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if you are able to get the maui context, and then get the android contextfrom that. The maui context should be registered in the services?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I am not sure which maui context it will be - it may be the root app one, or the latest/nearest...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

svc.GetService<IMauContext>() returns null at this point for some reason?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea, the service passed in here is just the MauiHandlerFactory that implements IServiceProvider. It doesn't really have any exciting information or access to the relevant WrappedServiceProvider

Currently all these controls are completely broken so just doing this for now to get unbroken.

This is partially why I was hoping we'd shift to a dependency resolution route. If the ToHandler code used ActivatorUtilities then we could look at dependency resolution to make this work.

I think it's still worth seeing if we can modify this factory approach to provide the WrappedServiceProvider inside this code block before we print NET8.

@PureWeen
PureWeen merged commit 136f21d into dotnet:main Aug 25, 2023
PureWeen added a commit that referenced this pull request Aug 29, 2023
PureWeen added a commit that referenced this pull request Aug 30, 2023
* Revert "[core] factory methods for registering services, part 2 (#17004)"

This reverts commit 79a44ff.

* Revert "[android] fix DI registration for Frame, ListView, TableView (#16989)"

This reverts commit 136f21d.

* Revert "[core] use factory methods for registering services (#16741)"

This reverts commit b0bba51.

# Conflicts:
#	src/Controls/src/Xaml/Hosting/AppHostBuilderExtensions.cs
#	src/Core/src/PublicAPI/net-windows/PublicAPI.Unshipped.txt

* Revert "Revert "[core] factory methods for registering services, part 2 (#17004)""

This reverts commit ed9a1fc.

# Conflicts:
#	eng/BannedSymbols.txt
#	src/BlazorWebView/src/SharedSource/BlazorWebViewServiceCollectionExtensions.cs

* Restore some changes (so hopefully it builds now?)

Diff is also smaller

---------

Co-authored-by: Jonathan Peppers <jonathan.peppers@microsoft.com>
@github-actions github-actions Bot locked and limited conversation to collaborators Dec 7, 2023
@samhouts samhouts added the fixed-in-8.0.0-rc.1.9171 Look for this fix in 8.0.0-rc.1.9171 label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

fixed-in-8.0.0-rc.1.9171 Look for this fix in 8.0.0-rc.1.9171 platform/android

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants