WIP Fix MediaContent-related API not being generated#1356
WIP Fix MediaContent-related API not being generated#1356marius-bughiu wants to merge 1 commit intodotnet:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adjusts binding metadata for Google Play Services Ads so MediaContent / IMediaContent is generated again (by removing problematic obfuscated members), restoring expected managed API surface for native ads.
Changes:
- Add
remove-noderules forcom.google.android.gms.ads.MediaContent.zza/zzbin Ads and Ads.Lite Metadata transforms. - Update
play-services-ads-litePublicAPI baseline to includeAndroid.Gms.Ads.IMediaContentand related native-ad APIs. - PublicAPI baseline updates also appear in unrelated packages (kotlinx-* and ads-identifier).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| source/com.google.android.gms/play-services-ads/Transforms/Metadata.xml | Adds targeted removals for MediaContent.zza/zzb (but duplicates an existing broad zz* method removal rule). |
| source/com.google.android.gms/play-services-ads-lite/Transforms/Metadata.xml | Adds targeted removals for MediaContent.zza/zzb to prevent interface generation from being blocked. |
| source/com.google.android.gms/play-services-ads-lite/PublicAPI/PublicAPI.Unshipped.txt | Records newly generated IMediaContent and native-ad related API surface in the public API baseline. |
| source/org.jetbrains.kotlinx/kotlinx-serialization-core-jvm/PublicAPI/PublicAPI.Unshipped.txt | Adds new KotlinX.Serialization API entries (not described by the PR). |
| source/org.jetbrains.kotlinx/atomicfu-jvm/PublicAPI/PublicAPI.Unshipped.txt | Adds new AtomicFU API entries (not described by the PR). |
| source/com.google.android.gms/play-services-ads-identifier/PublicAPI/PublicAPI.Unshipped.txt | Swaps obfuscated method signatures in the public API baseline (not described by the PR). |
| Google.Ads.Identifier.AdvertisingIdClient.Zza(bool p0) -> void | ||
| Google.Ads.Identifier.AdvertisingIdClient.Zzc() -> void |
There was a problem hiding this comment.
This PR changes the public API baseline for play-services-ads-identifier by swapping the Zza/Zzc signatures. This isn’t mentioned in the PR description and seems unrelated to the MediaContent binding fix; please confirm it’s intentional (and backed by updated bindings) or revert/split it to avoid accidental API churn.
| <!-- MediaContent API bindings --> | ||
| <!-- Remove obfuscated methods from MediaContent interface that reference internal types --> | ||
| <remove-node path="/api/package[@name='com.google.android.gms.ads']/interface[@name='MediaContent']/method[@name='zza']" /> | ||
| <remove-node path="/api/package[@name='com.google.android.gms.ads']/interface[@name='MediaContent']/method[@name='zzb']" /> | ||
|
|
There was a problem hiding this comment.
In this Metadata.xml you already have a global <remove-node path="/api/*/*/method[contains(@name, 'zz')]" /> rule (line 40) which should remove MediaContent.zza/zzb as well. These new remove-node entries are redundant and may confuse future maintainers about which rule is actually required; consider removing the duplicate rules or narrowing/removing the global rule and keeping only the targeted removals.
| <!-- MediaContent API bindings --> | |
| <!-- Remove obfuscated methods from MediaContent interface that reference internal types --> | |
| <remove-node path="/api/package[@name='com.google.android.gms.ads']/interface[@name='MediaContent']/method[@name='zza']" /> | |
| <remove-node path="/api/package[@name='com.google.android.gms.ads']/interface[@name='MediaContent']/method[@name='zzb']" /> |
There was a problem hiding this comment.
@marius-bughiu this is kind of what I thought as well, do the new <remove-node/> entries added here, not actually do anything?
| KotlinX.Serialization.MissingFieldException.MissingFieldException(System.Collections.Generic.IList<string!>! missingFields, string? message, Java.Lang.Throwable? cause) -> void | ||
| KotlinX.Serialization.MissingFieldException.MissingFieldException(string! missingField, string! serialName) -> void | ||
| KotlinX.Serialization.MissingFieldException.MissingFields.get -> System.Collections.Generic.IList<string!>! | ||
| KotlinX.Serialization.MissingFieldException.SerialName.get -> string? | ||
| KotlinX.Serialization.Modules.ISerializersModuleCollector |
There was a problem hiding this comment.
These PublicAPI baseline additions for KotlinX.Serialization (e.g., MissingFieldException.SerialName / new PolymorphicModuleBuilder API) are not mentioned in the PR description and appear unrelated to the MediaContent binding fix. If these are not intentional version/API updates, please revert them or move them into a separate PR to keep review/release notes scoped.
| KotlinX.Serialization.Modules.PolymorphicModuleBuilder | ||
| KotlinX.Serialization.Modules.PolymorphicModuleBuilder.Default(Kotlin.Jvm.Functions.IFunction1! defaultSerializerProvider) -> void | ||
| KotlinX.Serialization.Modules.PolymorphicModuleBuilder.DefaultDeserializer(Kotlin.Jvm.Functions.IFunction1! defaultDeserializerProvider) -> void | ||
| KotlinX.Serialization.Modules.PolymorphicModuleBuilder.Subclass(Kotlin.Reflect.IKClass! subclass, KotlinX.Serialization.IKSerializer! serializer) -> void | ||
| KotlinX.Serialization.Modules.PolymorphicModuleBuilder.SubclassesOfSealed(KotlinX.Serialization.IKSerializer! serializer) -> void | ||
| KotlinX.Serialization.Modules.PolymorphicModuleBuilderKt |
There was a problem hiding this comment.
This new SubclassesOfSealed(...) API entry in KotlinX.Serialization is not described by the PR and seems unrelated to the MediaContent binding work. If it is unintended (e.g., from regenerating PublicAPI files), please revert or split it into a dedicated PR for that package.
| KotlinX.AtomicFU.Locks.SynchronousMutex | ||
| KotlinX.AtomicFU.Locks.SynchronousMutex.Lock() -> void | ||
| KotlinX.AtomicFU.Locks.SynchronousMutex.SynchronousMutex() -> void | ||
| KotlinX.AtomicFU.Locks.SynchronousMutex.TryLock() -> bool | ||
| KotlinX.AtomicFU.Locks.SynchronousMutex.TryLock(long timeout) -> bool | ||
| KotlinX.AtomicFU.Locks.SynchronousMutex.Unlock() -> void | ||
| KotlinX.AtomicFU.Locks.SynchronousMutexKt |
There was a problem hiding this comment.
These KotlinX.AtomicFU PublicAPI baseline additions (new SynchronousMutex types/members) are not mentioned in the PR description and appear unrelated to the MediaContent binding fix. If they are not intentional API updates for atomicfu-jvm, please revert or move them to a separate PR.
WIP (do not merge)
This is mostly generated with Claude and I need to test it first.
Fixes: #973
Needed for: marius-bughiu/Plugin.AdMob#78
Problem
The
IMediaContentinterface was not being generated because it contained obfuscated methods (zza(),zzb()) that referenced internal types (com.google.android.gms.internal.ads.zzbfs). This caused the binding generator to skip the entire interface.As a result, the
MediaContentproperty onNativeAd,MediaView, and related classes was unavailable to .NET developers.Solution
Added
<remove-node>rules to theMetadata.xmltransform files to exclude the problematic obfuscated methods from theMediaContentinterface:New API Surface
Android.Gms.Ads.IMediaContentinterface with:AspectRatioCurrentTimeDurationHasVideoContentMainImageVideoControllerMediaContentproperty now available on:NativeAdMediaViewINativeCustomFormatAd