Add core-compose module (#1239)#1304
Conversation
PierfrancescoSoffritti
left a comment
There was a problem hiding this comment.
thanks for the PR! what is the drawback of adding compose support directly in the core module?
| /** | ||
| * Compose-native entry point for the YouTube player. | ||
| * | ||
| * Renders the [YouTubePlayerView] owned by [state]. Because the view lives inside the |
There was a problem hiding this comment.
remove this bit from kdoc, unnecessary details
* Renders the [YouTubePlayerView] owned by [state]. Because the view lives inside the
* [rememberYouTubePlayerState] scope rather than being recreated on every recomposition, the
* player's internal network observer is registered only once per state instance — which is the
* Compose-safe way to avoid `ConnectivityManager$TooManyRequestsException`.
There was a problem hiding this comment.
Done — removed the network-observer paragraph, kept only the param docs.
| fun YouTubePlayer( | ||
| state: YouTubePlayerState, | ||
| modifier: Modifier = Modifier, | ||
| fullscreenListener: FullscreenListener? = null, |
There was a problem hiding this comment.
how did you handle going full screen in compose? would be nice to see in the sample
There was a problem hiding this comment.
Good idea!
Added ComposeFullscreenExampleActivity to the sample app — uses the new fullscreenListener parameter and attaches the fullscreen view to the activity's decorView, same pattern as the existing FullscreenExampleActivity.
| * Owns a single [YouTubePlayerView] across Compose recompositions. | ||
| * | ||
| * Instances must be created with [rememberYouTubePlayerState] and passed to the [YouTubePlayer] | ||
| * composable. The underlying view is created once inside the enclosing `remember` scope and |
There was a problem hiding this comment.
The underlying view is created once inside the enclosing `remember` scope and
* released when the state leaves composition, so the internal network observer registered by the
* player is registered at most once per state instance. This avoids the
* `ConnectivityManager$TooManyRequestsException` that can occur when rapid recompositions recreate
* the view and pile up `registerDefaultNetworkCallback` calls above the per-UID system limit.
remove
| @@ -0,0 +1,15 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
There was a problem hiding this comment.
this can be done programmatically
There was a problem hiding this comment.
Good catch! Added a YouTubePlayerView(context, enableAutomaticInitialization: Boolean) constructor in :core so rememberYouTubePlayerState can construct the view programmatically. XML layout and res/ directory removed.
The existing constructor reads enableAutomaticInitialization from styleable attributes and defaults to true, so the init block has already auto-initialized (and registered a network callback) by the time a caller can flip the property from Kotlin. Add an additive constructor that takes the flag explicitly and applies it before the init block runs, enabling programmatic construction without inflating from XML.
…State Use the new YouTubePlayerView(context, enableAutomaticInitialization = false) constructor instead of inflating from an XML layout. Removes the layout resource and the now-unused res/ directory, and trims the verbose KDoc on YouTubePlayerState.
Demonstrates the FullscreenListener parameter on the YouTubePlayer composable by attaching the fullscreen view to the host activity's decorView. Wraps content in a Scaffold so status/navigation bar insets are applied to the inline player, while the fullscreen overlay stays outside the Scaffold to cover the system bar area.
@PierfrancescoSoffritti
Happy to fold it into |
Fixes #1239.
What
Adds a new
core-composemodule that provides a Compose-native API for the YouTubeplayer:
rememberYouTubePlayerState(...)— owns a singleYouTubePlayerViewacrossrecompositions, wires it to the current
LifecycleOwner, and releases it ondispose.
YouTubePlayer(state, modifier, fullscreenListener)— thinAndroidViewwrapper that renders the view owned by the state.
The module is published as
core-composeand depends on:core, so existingusers are unaffected.
Why
Issue #1239 reports a sporadic
android.net.ConnectivityManager$TooManyRequestsExceptionthrown from
NetworkObserver.doObserveNetworkwhen the player is used fromCompose. The root cause is that
NetworkObservercallsConnectivityManager.registerDefaultNetworkCallback, which is subject to aper-UID limit (~100 concurrent callbacks). In Compose, two patterns blow that
limit:
View recreated on recomposition. The natural Compose pattern is to
construct
YouTubePlayerViewinsideAndroidView'sfactory. Lists(
LazyColumn), navigation, or configuration changes create/release viewsrapidly — each
initialize()call registers another network callback, andthey accumulate faster than the system releases them.
Callback registered during view construction. Setting
enableAutomaticInitialization = falseafterYouTubePlayerView(ctx)comes too late — the view's
initblock already read the styleableattribute as its default (
true) and calledinitialize(..., handleNetworkEvents = true, ...), which registers anetwork callback. A subsequent manual
initialize(listener, options)overload hardcodes
handleNetworkEvents = trueinternally(
YouTubePlayerView.kt:151), so it registers a second callback. To makematters worse,
NetworkObserver.doObserveNetworkoverwrites its ownnetworkCallbackfield without unregistering the previous one, so thefirst callback leaks permanently.
This PR fixes both by:
app:enableAutomaticInitialization="false"before the init block runs, sono auto-initialization happens at construction time and we control the
single
initialize(...)call ourselves.remember { ... }so it survives recompositions and iscreated exactly once per state instance.
DisposableEffectattaches thelifecycle observer and calls
release()on dispose, which unregisters thenetwork callback deterministically.
Sample app
Adds a
LazyColumnExampleActivitythat scrolls 30 autoplaying videos in aLazyColumnwithhandleNetworkEvents = true. This reproduces the worst casefrom #1239 (rapid view create/release + real network traffic) and is the
regression test I used to verify the fix.
Tested on
minutes without hitting the crash. On
devthe same scenario reproducesTooManyRequestsExceptionwithin a few scroll passes.Notes for review
ComposeExampleActivityincore-sample-appis left in placeas a low-level
AndroidViewexample. Happy to migrate it to the new API ifpreferred.
:core. This is purely additive.