Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Microsoft.Android.Sdk.ILLink] preserve types with IJniNameProviderAttribute #9099

Merged

Conversation

jonathanpeppers
Copy link
Member

Fixes: #8940
Context: TobiasBuchholz/Plugin.Firebase#144

Using the NuGet package:

<PackageReference Include="Plugin.Firebase.CloudMessaging" Version="3.0.0" />

Includes a service:

namespace Plugin.Firebase.CloudMessaging.Platforms.Android;

[Service(Exported = true)]
[IntentFilter(new[] { "com.google.firebase.MESSAGING_EVENT" })]
public class MyFirebaseMessagingService : FirebaseMessagingService

Unfortunately, using TrimMode=full completely trims away the above service, which is required for push notifications to work.

I could reproduce this problem in a test using the above NuGet package.

To fix this, we can modify MarkJavaObjects to preserve types with attributes that implement Java.Interop.IJniNameProviderAttribute, and the new test now passes.

…ttribute`

Fixes: dotnet#8940
Context: TobiasBuchholz/Plugin.Firebase#144

Using the NuGet package:

    <PackageReference Include="Plugin.Firebase.CloudMessaging" Version="3.0.0" />

Includes a service:

    namespace Plugin.Firebase.CloudMessaging.Platforms.Android;

    [Service(Exported = true)]
    [IntentFilter(new[] { "com.google.firebase.MESSAGING_EVENT" })]
    public class MyFirebaseMessagingService : FirebaseMessagingService

Unfortunately, using `TrimMode=full` completely trims away the above
service, which is required for push notifications to work.

I could reproduce this problem in a test using the above NuGet package.

To fix this, we can modify `MarkJavaObjects` to preserve types with
attributes that implement `Java.Interop.IJniNameProviderAttribute`,
and the new test now passes.
Comment on lines +30 to +37
if (Profile.IsSdkAssembly (assembly))
return false;
if (Profile.IsProductAssembly (assembly))
return false;

return assembly.MainModule.HasTypeReference ("System.Net.Http.HttpMessageHandler") ||
assembly.MainModule.HasTypeReference ("Java.Lang.Object") ||
assembly.MainModule.HasTypeReference ("Android.Util.IAttributeSet");
Copy link
Member Author

Choose a reason for hiding this comment

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

Checking IsSdkAssembly(), IsProductAssembly(), and Java.Lang.Object is what the FixAbstractMethodsStep is already doing, so it seemed appropriate here.

@jonathanpeppers jonathanpeppers marked this pull request as ready for review July 12, 2024 01:39
@jonathanpeppers jonathanpeppers merged commit 611c269 into dotnet:main Jul 15, 2024
55 of 57 checks passed
@jonathanpeppers jonathanpeppers deleted the PreserveIJniNameProviderAttribute branch July 15, 2024 13:06
grendello added a commit that referenced this pull request Jul 23, 2024
* main: (23 commits)
  Localized file check-in by OneLocBuild Task (#9129)
  [ci] Disable CodeQL on CI/PR pipelines (#9128)
  Refine 16k page alignment support (#9075)
  [build] fix `ConfigureLocalWorkload` target (#9124)
  Bump to NDK r27 (#9020)
  [ci] Use drop service for SDK insertion artifacts  (#9116)
  Fix up all mapping paths (#9121)
  [ci] Fix maestro publishing for stable packages (#9118)
  Bump to dotnet/sdk@2f14fea98b 9.0.100-preview.7.24367.21 (#9108)
  Missing androidx.window.[extensions|sidecar] warnings (#9085)
  [ci] Use sign-artifacts template for macOS signing (#9091)
  [ci] Use DotNetCoreCLI to sign macOS files (#9102)
  [ci] Disable CodeQL on macOS, Linux, non-main jobs (#9111)
  [tests] re-enable `JavaAbstractMethodTest` (#9097)
  [Microsoft.Android.Sdk.ILLink] preserve types with `IJniNameProviderAttribute` (#9099)
  [Mono.Android] Data sharing and Close() overrides (#9103)
  [AndroidManifest] Add `Android.App.PropertyAttribute` (#9016)
  [Mono.Android] Add support for AndroidMessageHandler ClientCertificates (#8961)
  [Mono.Android] Bind and enumify API-35 (#9043)
  Bump to dotnet/java-interop@7a058c0e (#9066)
  ...
@github-actions github-actions bot locked and limited conversation to collaborators Aug 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TrimMode=full seems to break android push notifications on MAUI .net 9 preview 3
2 participants