Reviews onDispose calls to make sure the onDispose is disposing the data from the previous composable, since it can happen AFTER a new composition already took place. So the references must match the old composition.

This commit is contained in:
Vitor Pamplona
2025-07-03 10:27:47 -04:00
parent 5bcaf065a3
commit 69db9bdcd5
13 changed files with 97 additions and 80 deletions

View File

@@ -20,6 +20,7 @@
*/ */
package com.vitorpamplona.amethyst.service.playback.composable package com.vitorpamplona.amethyst.service.playback.composable
import android.view.View
import androidx.compose.runtime.Composable import androidx.compose.runtime.Composable
import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.DisposableEffect
import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.LaunchedEffect
@@ -67,23 +68,30 @@ fun ControlWhenPlayerIsActive(
// Keeps the screen on while playing and viewing videos. // Keeps the screen on while playing and viewing videos.
DisposableEffect(key1 = controller, key2 = view) { DisposableEffect(key1 = controller, key2 = view) {
val listener = val listener = PlayerEventListener(view)
object : Player.Listener {
override fun onIsPlayingChanged(isPlaying: Boolean) {
// doesn't consider the mutex because the screen can turn off if the video
// being played in the mutex is not visible.
if (view.keepScreenOn != isPlaying) {
view.keepScreenOn = isPlaying
}
}
}
controller.addListener(listener) controller.addListener(listener)
onDispose { onDispose {
if (view.keepScreenOn) {
view.keepScreenOn = false
}
controller.removeListener(listener) controller.removeListener(listener)
listener.destroy()
}
}
}
class PlayerEventListener(
val view: View,
) : Player.Listener {
override fun onIsPlayingChanged(isPlaying: Boolean) {
// doesn't consider the mutex because the screen can turn off if the video
// being played in the mutex is not visible.
if (view.keepScreenOn != isPlaying) {
view.keepScreenOn = isPlaying
}
}
fun destroy() {
if (view.keepScreenOn) {
view.keepScreenOn = false
} }
} }
} }

View File

@@ -54,6 +54,10 @@ fun GetVideoController(
val scope = rememberCoroutineScope() val scope = rememberCoroutineScope()
// Prepares a VideoPlayer from the foreground service. // Prepares a VideoPlayer from the foreground service.
//
// TODO: Review this code because a new Disposable Effect can run
// before the onDispose of the previous composable and the onDispose
// sometimes affects the new variables, not the old ones.
DisposableEffect(key1 = mediaItem.src.videoUri) { DisposableEffect(key1 = mediaItem.src.videoUri) {
// If it is not null, the user might have come back from a playing video, like clicking on // If it is not null, the user might have come back from a playing video, like clicking on
// the notification of the video player. // the notification of the video player.

View File

@@ -56,21 +56,25 @@ fun Waveform(
val restartFlow = remember { mutableIntStateOf(0) } val restartFlow = remember { mutableIntStateOf(0) }
val myController = mediaControllerState.controller
// Keeps the screen on while playing and viewing videos. // Keeps the screen on while playing and viewing videos.
DisposableEffect(key1 = mediaControllerState.controller) { if (myController != null) {
val listener = DisposableEffect(key1 = myController) {
object : Player.Listener { val listener =
override fun onIsPlayingChanged(isPlaying: Boolean) { object : Player.Listener {
// doesn't consider the mutex because the screen can turn off if the video override fun onIsPlayingChanged(isPlaying: Boolean) {
// being played in the mutex is not visible. // doesn't consider the mutex because the screen can turn off if the video
if (isPlaying) { // being played in the mutex is not visible.
restartFlow.intValue += 1 if (isPlaying) {
restartFlow.intValue += 1
}
} }
} }
}
mediaControllerState.controller?.addListener(listener) myController.addListener(listener)
onDispose { mediaControllerState.controller?.removeListener(listener) } onDispose { myController.removeListener(listener) }
}
} }
LaunchedEffect(key1 = restartFlow.intValue) { LaunchedEffect(key1 = restartFlow.intValue) {

View File

@@ -39,7 +39,7 @@ object BackgroundMedia {
fun removeBackgroundControllerAndReleaseIt() { fun removeBackgroundControllerAndReleaseIt() {
bgInstance.value?.let { bgInstance.value?.let {
PlaybackServiceClient.removeController(it) PlaybackServiceClient.removeController(it)
clearBackground() bgInstance.tryEmit(null)
} }
} }
@@ -47,7 +47,9 @@ object BackgroundMedia {
bgInstance.tryEmit(mediaControllerState) bgInstance.tryEmit(mediaControllerState)
} }
fun clearBackground() { fun clearBackground(mediaControllerState: MediaControllerState) {
bgInstance.tryEmit(null) if (bgInstance.value == mediaControllerState) {
bgInstance.tryEmit(null)
}
} }
} }

View File

@@ -58,9 +58,8 @@ fun rememberIsInPipMode(): Boolean {
Consumer<PictureInPictureModeChangedInfo> { info -> Consumer<PictureInPictureModeChangedInfo> { info ->
pipMode = info.isInPictureInPictureMode pipMode = info.isInPictureInPictureMode
} }
activity.addOnPictureInPictureModeChangedListener(
observer, activity.addOnPictureInPictureModeChangedListener(observer)
)
onDispose { activity.removeOnPictureInPictureModeChangedListener(observer) } onDispose { activity.removeOnPictureInPictureModeChangedListener(observer) }
} }
return pipMode return pipMode

View File

@@ -81,7 +81,7 @@ fun PipVideo(controller: MediaControllerState) {
DisposableEffect(controller) { DisposableEffect(controller) {
BackgroundMedia.switchKeepPlaying(controller) BackgroundMedia.switchKeepPlaying(controller)
onDispose { onDispose {
BackgroundMedia.clearBackground() BackgroundMedia.clearBackground(controller)
} }
} }

View File

@@ -40,7 +40,6 @@ import androidx.compose.material3.Text
import androidx.compose.material3.TopAppBar import androidx.compose.material3.TopAppBar
import androidx.compose.material3.TopAppBarDefaults import androidx.compose.material3.TopAppBarDefaults
import androidx.compose.runtime.Composable import androidx.compose.runtime.Composable
import androidx.compose.runtime.DisposableEffect
import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.LaunchedEffect
import androidx.compose.ui.Alignment import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier import androidx.compose.ui.Modifier
@@ -74,12 +73,6 @@ fun NewUserMetadataScreen(
postViewModel.load(accountViewModel.account) postViewModel.load(accountViewModel.account)
} }
DisposableEffect(Unit) {
onDispose {
postViewModel.clear()
}
}
Scaffold( Scaffold(
topBar = { topBar = {
TopAppBar( TopAppBar(

View File

@@ -104,7 +104,6 @@ fun keyboardAsState(): State<Keyboard> {
} }
} }
view.viewTreeObserver.addOnGlobalLayoutListener(onGlobalListener) view.viewTreeObserver.addOnGlobalLayoutListener(onGlobalListener)
onDispose { view.viewTreeObserver.removeOnGlobalLayoutListener(onGlobalListener) } onDispose { view.viewTreeObserver.removeOnGlobalLayoutListener(onGlobalListener) }
} }

View File

@@ -22,7 +22,6 @@ package com.vitorpamplona.amethyst.ui.screen
import androidx.compose.runtime.Composable import androidx.compose.runtime.Composable
import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.CompositionLocalProvider
import androidx.compose.runtime.DisposableEffect
import androidx.compose.runtime.Stable import androidx.compose.runtime.Stable
import androidx.lifecycle.ViewModelStore import androidx.lifecycle.ViewModelStore
import androidx.lifecycle.ViewModelStoreOwner import androidx.lifecycle.ViewModelStoreOwner
@@ -55,11 +54,13 @@ fun SetAccountCentricViewModelStore(
content() content()
} }
DisposableEffect(key1 = state) { // moved this clearing activity to the viewmodel account
onDispose { // because the new composable might run before the onDispose.
state.currentViewModelStore.viewModelStore.clear() // DisposableEffect(key1 = state) {
} // onDispose {
} // state.currentViewModelStore.viewModelStore.clear()
// }
// }
} }
class AccountCentricViewModelStore : ViewModelStoreOwner { class AccountCentricViewModelStore : ViewModelStoreOwner {

View File

@@ -21,6 +21,7 @@
package com.vitorpamplona.amethyst.ui.screen.loggedIn package com.vitorpamplona.amethyst.ui.screen.loggedIn
import android.app.Activity import android.app.Activity
import android.content.Intent
import android.util.Log import android.util.Log
import androidx.activity.compose.rememberLauncherForActivityResult import androidx.activity.compose.rememberLauncherForActivityResult
import androidx.activity.result.contract.ActivityResultContracts import androidx.activity.result.contract.ActivityResultContracts
@@ -216,25 +217,27 @@ private fun ListenToExternalSignerIfNeeded(accountViewModel: AccountViewModel) {
} }
} }
val launcher: (Intent) -> Unit = {
try {
launcher.launch(it)
} catch (e: Exception) {
if (e is CancellationException) throw e
Log.e("Signer", "Error opening Signer app", e)
accountViewModel.toastManager.toast(
R.string.error_opening_external_signer,
R.string.error_opening_external_signer_description,
)
}
}
lifeCycleOwner.lifecycle.addObserver(observer) lifeCycleOwner.lifecycle.addObserver(observer)
accountViewModel.account.signer.launcher.registerLauncher( accountViewModel.account.signer.launcher.registerLauncher(
launcher = { launcher = launcher,
try {
launcher.launch(it)
} catch (e: Exception) {
if (e is CancellationException) throw e
Log.e("Signer", "Error opening Signer app", e)
accountViewModel.toastManager.toast(
R.string.error_opening_external_signer,
R.string.error_opening_external_signer_description,
)
}
},
contentResolver = Amethyst.instance::contentResolverFn, contentResolver = Amethyst.instance::contentResolverFn,
) )
onDispose { onDispose {
accountViewModel.account.signer.launcher accountViewModel.account.signer.launcher
.clearLauncher() .clearLauncherIf(launcher)
lifeCycleOwner.lifecycle.removeObserver(observer) lifeCycleOwner.lifecycle.removeObserver(observer)
} }
} }

View File

@@ -64,8 +64,6 @@ fun TabRelays(
lifeCycleOwner.lifecycle.addObserver(observer) lifeCycleOwner.lifecycle.addObserver(observer)
onDispose { onDispose {
lifeCycleOwner.lifecycle.removeObserver(observer) lifeCycleOwner.lifecycle.removeObserver(observer)
println("Profile Relay Dispose")
feedViewModel.unsubscribeTo(user)
} }
} }

View File

@@ -641,26 +641,30 @@ private fun PrepareExternalSignerReceiver(onLogin: (pubkey: String, packageName:
val activity = getActivity() as MainActivity val activity = getActivity() as MainActivity
DisposableEffect(launcher, activity, externalSignerLauncher) { DisposableEffect(launcher, activity, externalSignerLauncher) {
externalSignerLauncher.registerLauncher( val launcher: (Intent) -> Unit = {
launcher = { try {
try { launcher.launch(it)
launcher.launch(it) } catch (e: Exception) {
} catch (e: Exception) { if (e is CancellationException) throw e
if (e is CancellationException) throw e Log.e("Signer", "Error opening Signer app", e)
Log.e("Signer", "Error opening Signer app", e) scope.launch(Dispatchers.Main) {
scope.launch(Dispatchers.Main) { Toast
Toast .makeText(
.makeText( Amethyst.instance,
Amethyst.instance, R.string.error_opening_external_signer,
R.string.error_opening_external_signer, Toast.LENGTH_SHORT,
Toast.LENGTH_SHORT, ).show()
).show()
}
} }
}, }
}
externalSignerLauncher.registerLauncher(
launcher = launcher,
contentResolver = Amethyst.instance::contentResolverFn, contentResolver = Amethyst.instance::contentResolverFn,
) )
onDispose { externalSignerLauncher.clearLauncher() } onDispose {
externalSignerLauncher.clearLauncherIf(launcher)
}
} }
LaunchedEffect(externalSignerLauncher) { LaunchedEffect(externalSignerLauncher) {

View File

@@ -114,9 +114,11 @@ class ExternalSignerLauncher(
} }
/** Call this function when the activity is destroyed or is about to be replaced. */ /** Call this function when the activity is destroyed or is about to be replaced. */
fun clearLauncher() { fun clearLauncherIf(launcher: ((Intent) -> Unit)) {
this.signerAppLauncher = null if (signerAppLauncher == launcher) {
this.contentResolver = null this.signerAppLauncher = null
this.contentResolver = null
}
} }
fun newResult(data: Intent) { fun newResult(data: Intent) {