fixed: race condition when attaching the fragment

This fixes #654
yuriykulikov · May 1, 2024 · 982d676 · 982d676
1 parent 8293b52
commit 982d676
Show file tree

Hide file tree

Showing 7 changed files with 74 additions and 54 deletions.
diff --git a/app/build.gradle.kts b/app/build.gradle.kts
@@ -78,8 +78,8 @@ val acraEmail =
 android {
   compileSdk = 33
   defaultConfig {
-    versionCode = 31504
-    versionName = "3.15.04"
+    versionCode = 31601
+    versionName = "3.16.01"
     applicationId = "com.better.alarm"
     minSdk = 21
     targetSdk = 33

diff --git a/app/src/main/java/com/better/alarm/data/Alarmtone.kt b/app/src/main/java/com/better/alarm/data/Alarmtone.kt
@@ -18,13 +18,13 @@ fun Alarmtone.ringtoneManagerUri(): String? {
 
 @Serializable
 sealed class Alarmtone {
-  @SerialName("Silent") @Serializable object Silent : Alarmtone()
+  @SerialName("Silent") @Serializable data object Silent : Alarmtone()
 
   /** User has selected the same alarm as in the application settings. */
-  @SerialName("Default") @Serializable object Default : Alarmtone()
+  @SerialName("Default") @Serializable data object Default : Alarmtone()
 
   /** The ringtone is a SystemDefault ringtone. */
-  @SerialName("SystemDefault") @Serializable object SystemDefault : Alarmtone()
+  @SerialName("SystemDefault") @Serializable data object SystemDefault : Alarmtone()
 
   @SerialName("Sound") @Serializable data class Sound(val uriString: String) : Alarmtone()
 

diff --git a/app/src/main/java/com/better/alarm/platform/Permissions.kt b/app/src/main/java/com/better/alarm/platform/Permissions.kt
@@ -18,14 +18,16 @@ import com.better.alarm.data.ringtoneManagerUri
 import com.better.alarm.logger.Logger
 import com.better.alarm.services.PlayerWrapper
 import com.better.alarm.ui.ringtonepicker.userFriendlyTitle
+import kotlin.coroutines.coroutineContext
+import kotlinx.coroutines.ensureActive
 
 /** Checks if all ringtones can be played, and requests permissions if it is not the case */
-fun checkPermissions(activity: Activity, tones: List<Alarmtone>) {
+suspend fun checkPermissions(activity: Activity, tones: List<Alarmtone>) {
   checkSetAlarmPermissions(activity)
   checkMediaPermissions(activity, tones)
 }
 
-private fun checkMediaPermissions(activity: Activity, tones: List<Alarmtone>) {
+private suspend fun checkMediaPermissions(activity: Activity, tones: List<Alarmtone>) {
   if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) return
   val mediaPermission =
       when {
@@ -55,6 +57,10 @@ private fun checkMediaPermissions(activity: Activity, tones: List<Alarmtone>) {
             .map { ringtone -> ringtone.userFriendlyTitle(activity) }
 
     if (unplayable.isNotEmpty()) {
+      logger.warning {
+        "Some ringtones are not playable: $unplayable, showing dialog to request permissions"
+      }
+      coroutineContext.ensureActive()
       try {
         AlertDialog.Builder(activity)
             .setTitle(activity.getString(R.string.alert))

diff --git a/app/src/main/java/com/better/alarm/ui/details/AlarmDetailsFragment.kt b/app/src/main/java/com/better/alarm/ui/details/AlarmDetailsFragment.kt
@@ -56,14 +56,13 @@ import io.reactivex.disposables.Disposable
 import io.reactivex.disposables.Disposables
 import java.text.SimpleDateFormat
 import java.util.Calendar
-import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.mapNotNull
 import kotlinx.coroutines.flow.onEach
-import kotlinx.coroutines.withContext
+import kotlinx.coroutines.launch
 import org.koin.android.ext.android.inject
 import org.koin.androidx.viewmodel.ext.android.viewModel
 
@@ -305,19 +304,18 @@ class AlarmDetailsFragment : Fragment() {
               value to defaultRingtone
             }
         .map { (alarmtone, default) ->
-          withContext(Dispatchers.IO) {
-            when {
-              // Default (title)
-              // We need this case otherwise we will have "App default (Default (title))"
-              alarmtone is Alarmtone.Default && default is Alarmtone.SystemDefault ->
-                  default.userFriendlyTitle(requireActivity())
-              // App default (title)
-              alarmtone is Alarmtone.Default ->
-                  getString(
-                      R.string.app_default_ringtone, default.userFriendlyTitle(requireActivity()))
-              // title
-              else -> alarmtone.userFriendlyTitle(requireActivity())
-            }
+          val hostActivity = requireActivity()
+          when {
+            // Default (title)
+            // We need this case otherwise we will have "App default (Default (title))"
+            alarmtone is Alarmtone.Default && default is Alarmtone.SystemDefault ->
+                default.userFriendlyTitle(hostActivity)
+            // App default (title)
+            alarmtone is Alarmtone.Default ->
+                hostActivity.getString(
+                    R.string.app_default_ringtone, default.userFriendlyTitle(hostActivity))
+            // title
+            else -> alarmtone.userFriendlyTitle(hostActivity)
           }
         }
         .onEach { ringtoneSummary.text = it }
@@ -371,7 +369,7 @@ class AlarmDetailsFragment : Fragment() {
   override fun onActivityResult(requestCode: Int, resultCode: Int, data: Intent?) {
     if (data != null && requestCode == ringtonePickerRequestCode) {
       val alarmtone = data.getPickedRingtone()
-      checkPermissions(requireActivity(), listOf(alarmtone))
+      lifecycleScope.launch { checkPermissions(requireActivity(), listOf(alarmtone)) }
       logger.debug { "Picked alarm tone: $alarmtone" }
       modify("Ringtone picker") { prev -> prev.copy(alarmtone = alarmtone, isEnabled = true) }
     }

diff --git a/app/src/main/java/com/better/alarm/ui/main/AlarmsListActivity.kt b/app/src/main/java/com/better/alarm/ui/main/AlarmsListActivity.kt
@@ -55,6 +55,8 @@ import com.google.android.material.snackbar.Snackbar
 import io.reactivex.disposables.Disposables
 import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.onEach
+import kotlinx.coroutines.launch
+import kotlinx.coroutines.rx2.awaitFirst
 import kotlinx.serialization.ExperimentalSerializationApi
 import kotlinx.serialization.protobuf.ProtoBuf
 import org.koin.android.ext.android.inject
@@ -113,11 +115,10 @@ class AlarmsListActivity() : AppCompatActivity() {
 
     setContentView(R.layout.list_activity)
 
-    store
-        .alarms()
-        .take(1)
-        .subscribe { alarms -> checkPermissions(this, alarms.map { it.alarmtone }) }
-        .apply {}
+    val context = this
+    lifecycleScope.launch {
+      checkPermissions(context, store.alarms().awaitFirst().map { it.alarmtone })
+    }
 
     backPresses.onBackPressed(lifecycle) { finish() }
   }

diff --git a/app/src/main/java/com/better/alarm/ui/ringtonepicker/RingtonePicker.kt b/app/src/main/java/com/better/alarm/ui/ringtonepicker/RingtonePicker.kt
@@ -5,12 +5,16 @@ import android.content.Intent
 import android.media.Ringtone
 import android.media.RingtoneManager
 import android.net.Uri
+import android.os.Looper
 import android.widget.Toast
 import androidx.core.net.toUri
 import androidx.fragment.app.Fragment
 import com.better.alarm.R
 import com.better.alarm.data.Alarmtone
 import com.better.alarm.data.ringtoneManagerUri
+import kotlinx.coroutines.CancellationException
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.withContext
 
 /**
  * Shows the ringtone picker.
@@ -88,26 +92,27 @@ fun Intent.getPickedRingtone(): Alarmtone {
   return alarmtone
 }
 
-fun Alarmtone.userFriendlyTitle(context: Context): CharSequence {
-  return runCatching {
-        when (this) {
-          is Alarmtone.Silent -> context.getText(R.string.silent_alarm_summary)
-          else ->
-              RingtoneManager.getRingtone(context, this.ringtoneManagerUri()?.toUri())
-                  .title(context)
-        }
-      }
-      .getOrDefault("")
+suspend fun Alarmtone.userFriendlyTitle(context: Context): CharSequence {
+  checkNotNull(Looper.myLooper()) { "userFriendlyTitle should be called from the main thread" }
+  return when (this) {
+    is Alarmtone.Silent -> context.getText(R.string.silent_alarm_summary)
+    else ->
+        ringtoneManagerUri() //
+            ?.toUri()
+            ?.let { uri -> context.getRingtoneTitleOrNull(uri) }
+            ?: context.getText(R.string.silent_alarm_summary)
+  }
 }
 
-private fun Ringtone.title(context: Context): CharSequence {
-  // this can fail, see
-  // https://github.com/yuriykulikov/AlarmClock/issues/403
-  return try {
-    getTitle(context) ?: context.getText(R.string.silent_alarm_summary)
-  } catch (e: Exception) {
-    context.getText(R.string.silent_alarm_summary)
-  } catch (e: NullPointerException) {
-    null
-  } ?: ""
+/**
+ * Call to [Ringtone.getTitle] can fail, see
+ * [AlarmClock#403](https://github.com/yuriykulikov/AlarmClock/issues/403)
+ */
+private suspend fun Context.getRingtoneTitleOrNull(uri: Uri): CharSequence? {
+  val context = this
+  return withContext(Dispatchers.IO) {
+    runCatching { RingtoneManager.getRingtone(context, uri).getTitle(context) }
+        .onFailure { if (it is CancellationException) throw it }
+        .getOrNull()
+  }
 }
diff --git a/app/src/main/java/com/better/alarm/ui/settings/SettingsFragment.kt b/app/src/main/java/com/better/alarm/ui/settings/SettingsFragment.kt
@@ -8,6 +8,7 @@ import android.os.Handler
 import android.os.Looper
 import android.os.Vibrator
 import android.provider.Settings
+import androidx.lifecycle.lifecycleScope
 import androidx.preference.CheckBoxPreference
 import androidx.preference.ListPreference
 import androidx.preference.Preference
@@ -24,6 +25,11 @@ import com.better.alarm.ui.ringtonepicker.getPickedRingtone
 import com.better.alarm.ui.ringtonepicker.showRingtonePicker
 import com.better.alarm.ui.ringtonepicker.userFriendlyTitle
 import io.reactivex.disposables.CompositeDisposable
+import kotlinx.coroutines.flow.launchIn
+import kotlinx.coroutines.flow.map
+import kotlinx.coroutines.flow.onEach
+import kotlinx.coroutines.launch
+import kotlinx.coroutines.rx2.asFlow
 import org.koin.android.ext.android.inject
 
 /** Created by Yuriy on 24.07.2017. */
@@ -77,10 +83,12 @@ class SettingsFragment : PreferenceFragmentCompat() {
     check(alarmtone !is Alarmtone.Default) { "Default alarmtone is not allowed here" }
     logger.debug { "Picked default alarm tone: $alarmtone" }
     prefs.defaultRingtone.value = alarmtone.asString()
-    checkPermissions(
-        requireActivity(),
-        listOf(alarmtone),
-    )
+    lifecycleScope.launch {
+      checkPermissions(
+          requireActivity(),
+          listOf(alarmtone),
+      )
+    }
   }
 
   override fun onPreferenceTreeClick(preference: Preference): Boolean {
@@ -111,15 +119,17 @@ class SettingsFragment : PreferenceFragmentCompat() {
         val current = Alarmtone.fromString(prefs.defaultRingtone.value)
         showRingtonePicker(current, 43)
       }
+
       prefs.defaultRingtone
           .observe()
+          .asFlow()
           .map { Alarmtone.fromString(it) }
-          .subscribe { alarmtone ->
+          .onEach { alarmtone ->
             val summary = alarmtone.userFriendlyTitle(requireContext())
             logger.debug { "Setting summary to $summary" }
             ringtoneTitle = summary
           }
-          .also { disposables.add(it) }
+          .launchIn(lifecycleScope)
     }
 
     bindListPreference(Prefs.KEY_ALARM_SNOOZE, prefs.snoozeDuration) { duration ->