Make foreground notifications much more reliable

app-common/src/main/java/im/angry/openeuicc/service/EuiccChannelManagerService.kt

@@ -24,6 +24,8 @@ import kotlinx.coroutines.flow.takeWhile
 import kotlinx.coroutines.flow.transformWhile
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
+import kotlinx.coroutines.withTimeoutOrNull
+import kotlinx.coroutines.yield
 import net.typeblog.lpac_jni.ProfileDownloadCallback
 
 /**
@@ -101,7 +103,7 @@ class EuiccChannelManagerService : LifecycleService(), OpenEuiccContextMarker {
         }
     }
 
-    private fun updateForegroundNotification(title: String, iconRes: Int) {
+    private suspend fun updateForegroundNotification(title: String, iconRes: Int) {
         val channel =
             NotificationChannelCompat.Builder(CHANNEL_ID, NotificationManagerCompat.IMPORTANCE_LOW)
                 .setName(getString(R.string.task_notification))
@@ -117,6 +119,7 @@ class EuiccChannelManagerService : LifecycleService(), OpenEuiccContextMarker {
                 .setProgress(100, state.progress, state.progress == 0)
                 .setSmallIcon(iconRes)
                 .setPriority(NotificationCompat.PRIORITY_LOW)
+                .setOngoing(true)
                 .build()
 
             if (state.progress == 0) {
@@ -124,6 +127,10 @@ class EuiccChannelManagerService : LifecycleService(), OpenEuiccContextMarker {
             } else if (checkSelfPermission(android.Manifest.permission.POST_NOTIFICATIONS) == PackageManager.PERMISSION_GRANTED) {
                 NotificationManagerCompat.from(this).notify(FOREGROUND_ID, notification)
             }
+
+            // Yield out so that the main looper can handle the notification event
+            // Without this yield, the notification sent above will not be shown in time.
+            yield()
         } else {
             stopForeground(STOP_FOREGROUND_REMOVE)
         }
@@ -133,11 +140,14 @@ class EuiccChannelManagerService : LifecycleService(), OpenEuiccContextMarker {
      * Launch a potentially blocking foreground task in this service's lifecycle context.
      * This function does not block, but returns a Flow that emits ForegroundTaskState
      * updates associated with this task. The last update the returned flow will emit is
-     * always ForegroundTaskState.Done.
+     * always ForegroundTaskState.Done. The returned flow MUST be started in order for the
+     * foreground task to run.
      *
      * The task closure is expected to update foregroundTaskState whenever appropriate.
      * If a foreground task is already running, this function returns null.
      *
+     * To wait for foreground tasks to be available, use waitForForegroundTask().
+     *
      * The function will set the state back to Idle once it sees ForegroundTaskState.Done.
      */
     private fun launchForegroundTask(
@@ -158,21 +168,32 @@ class EuiccChannelManagerService : LifecycleService(), OpenEuiccContextMarker {
         lifecycleScope.launch(Dispatchers.Main) {
             // Wait until our self-start command has succeeded.
             // We can only call startForeground() after that
+            val res = withTimeoutOrNull(30 * 1000) {
                 foregroundStarted.first()
+            }
+
+            if (res == null) {
+                // The only case where the wait above could time out is if the subscriber
+                // to the flow is stuck. Or we failed to start foreground.
+                // In that case, we should just set our state back to Idle -- setting it
+                // to Done wouldn't help much because nothing is going to then set it Idle.
+                foregroundTaskState.value = ForegroundTaskState.Idle
+                return@launch
+            }
+
             updateForegroundNotification(title, iconRes)
 
             try {
                 withContext(Dispatchers.IO) {
                     this@EuiccChannelManagerService.task()
                 }
+                // This update will be sent by the subscriber (as shown below)
                 foregroundTaskState.value = ForegroundTaskState.Done(null)
             } catch (t: Throwable) {
                 foregroundTaskState.value = ForegroundTaskState.Done(t)
             } finally {
                 stopSelf()
             }
-
-            updateForegroundNotification(title, iconRes)
         }
 
         // We should be the only task running, so we can subscribe to foregroundTaskState
@@ -182,20 +203,26 @@ class EuiccChannelManagerService : LifecycleService(), OpenEuiccContextMarker {
         // it has been completed by that point.
         return foregroundTaskState.transformWhile {
             // Also update our notification when we see an update
+            // But ignore the first progress = 0 update -- that is the current value.
+            // we need that to be handled by the main coroutine after it finishes.
+            if (it !is ForegroundTaskState.InProgress || it.progress != 0) {
                 withContext(Dispatchers.Main) {
                     updateForegroundNotification(title, iconRes)
                 }
+            }
             emit(it)
             it !is ForegroundTaskState.Done
         }.onStart {
             // When this Flow is started, we unblock the coroutine launched above by
             // self-starting as a foreground service.
+            withContext(Dispatchers.Main) {
                 startForegroundService(
                     Intent(
                         this@EuiccChannelManagerService,
                         this@EuiccChannelManagerService::class.java
                     )
                 )
+            }
         }.onCompletion { foregroundTaskState.value = ForegroundTaskState.Idle }
     }
 
@@ -262,4 +289,23 @@ class EuiccChannelManagerService : LifecycleService(), OpenEuiccContextMarker {
                 throw RuntimeException("Profile not renamed")
             }
         }
+
+    fun launchProfileDeleteTask(
+        slotId: Int,
+        portId: Int,
+        iccid: String
+    ): Flow<ForegroundTaskState>? =
+        launchForegroundTask(
+            getString(R.string.task_profile_delete),
+            R.drawable.ic_task_delete
+        ) {
+            euiccChannelManager.beginTrackedOperationBlocking(slotId, portId) {
+                euiccChannelManager.findEuiccChannelByPort(
+                    slotId,
+                    portId
+                )!!.lpa.deleteProfile(iccid)
+
+                preferenceRepository.notificationDeleteFlow.first()
+            }
+        }
 }

app-common/src/main/java/im/angry/openeuicc/ui/ProfileDeleteFragment.kt

@@ -3,16 +3,15 @@ package im.angry.openeuicc.ui
 import android.app.Dialog
 import android.os.Bundle
 import android.text.Editable
-import android.util.Log
 import android.widget.EditText
 import androidx.appcompat.app.AlertDialog
 import androidx.fragment.app.DialogFragment
 import androidx.lifecycle.lifecycleScope
 import im.angry.openeuicc.common.R
 import im.angry.openeuicc.util.*
-import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.flow.collect
+import kotlinx.coroutines.flow.onStart
 import kotlinx.coroutines.launch
-import java.lang.Exception
 
 class ProfileDeleteFragment : DialogFragment(), EuiccChannelFragmentMarker {
     companion object {
@@ -67,23 +66,23 @@ class ProfileDeleteFragment : DialogFragment(), EuiccChannelFragmentMarker {
         alertDialog.getButton(AlertDialog.BUTTON_POSITIVE).isEnabled = false
         alertDialog.getButton(AlertDialog.BUTTON_NEGATIVE).isEnabled = false
 
-        lifecycleScope.launch {
+        requireParentFragment().lifecycleScope.launch {
-            try {
+            ensureEuiccChannelManager()
-                doDelete()
+            euiccChannelManagerService.waitForForegroundTask()
-            } catch (e: Exception) {
+
-                Log.d(ProfileDownloadFragment.TAG, "Error deleting profile")
+            euiccChannelManagerService.launchProfileDeleteTask(
-                Log.d(ProfileDownloadFragment.TAG, Log.getStackTraceString(e))
+                slotId,
-            } finally {
+                portId,
+                requireArguments().getString("iccid")!!
+            )!!.onStart {
                 if (parentFragment is EuiccProfilesChangedListener) {
+                    // Trigger a refresh in the parent fragment -- it should wait until
+                    // any foreground task is completed before actually doing a refresh
                     (parentFragment as EuiccProfilesChangedListener).onEuiccProfilesChanged()
                 }
-                dismiss()
-            }
-        }
-    }
 
-    private suspend fun doDelete() = beginTrackedOperation {
+                dismiss()
-        channel.lpa.deleteProfile(requireArguments().getString("iccid")!!)
+            }.collect()
-        preferenceRepository.notificationDeleteFlow.first()
+        }
     }
 }

app-common/src/main/res/drawable/ic_task_delete.xml

@@ -0,0 +1,5 @@
+<vector xmlns:android="http://schemas.android.com/apk/res/android" android:height="24dp" android:tint="#000000" android:viewportHeight="24" android:viewportWidth="24" android:width="24dp">
+      
+    <path android:fillColor="@android:color/white" android:pathData="M6,19c0,1.1 0.9,2 2,2h8c1.1,0 2,-0.9 2,-2V7H6v12zM19,4h-3.5l-1,-1h-5l-1,1H5v2h14V4z"/>
+    
+</vector>

app-common/src/main/res/values/strings.xml

@@ -36,6 +36,7 @@
     <string name="task_notification">Long-running Tasks</string>
     <string name="task_profile_download">Downloading eSIM profile</string>
     <string name="task_profile_rename">Renaming eSIM profile</string>
+    <string name="task_profile_delete">Deleting eSIM profile</string>
 
     <string name="profile_download">New eSIM</string>
     <string name="profile_download_server">Server (RSP / SM-DP+)</string>