fix(avatar): decode and save the remote avatar off the main thread

app/src/main/java/com/nextcloud/talk/utils/PickImage.kt

@@ -8,36 +8,85 @@
 
 package com.nextcloud.talk.utils
 
-import android.app.Activity
 import android.content.Intent
 import android.graphics.Bitmap
 import android.graphics.BitmapFactory
 import android.net.Uri
 import android.os.Bundle
 import android.provider.MediaStore
 import android.util.Log
+import android.widget.Toast
+import androidx.activity.ComponentActivity
 import androidx.activity.result.ActivityResultLauncher
+import androidx.lifecycle.lifecycleScope
 import androidx.core.content.FileProvider
 import autodagger.AutoInjector
 import com.github.dhaval2404.imagepicker.ImagePicker
 import com.github.dhaval2404.imagepicker.constant.ImageProvider
+import com.nextcloud.talk.R
 import com.nextcloud.talk.api.NcApi
 import com.nextcloud.talk.application.NextcloudTalkApplication
 import com.nextcloud.talk.data.user.model.User
 import com.nextcloud.talk.remotefilebrowser.activities.RemoteFileBrowserActivity
 import com.nextcloud.talk.utils.bundle.BundleKeys
 import com.nextcloud.talk.utils.permissions.PlatformPermissionUtil
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.Job
+import kotlinx.coroutines.launch
+import kotlinx.coroutines.withContext
 import okhttp3.ResponseBody
 import retrofit2.Call
 import retrofit2.Callback
 import retrofit2.Response
 import java.io.File
 import java.io.FileOutputStream
 import java.io.IOException
+import java.io.InputStream
 import javax.inject.Inject
 
+private const val FULL_QUALITY: Int = 100
+
+/**
+ * Launches [block] and closes [body] when the job completes, including when the scope is already cancelled
+ * (e.g. the activity was destroyed) and [block] never starts. Closing twice is safe.
+ */
+internal fun CoroutineScope.launchClosing(body: ResponseBody?, block: suspend CoroutineScope.() -> Unit): Job {
+    val job = launch(block = block)
+    job.invokeOnCompletion { body?.close() }
+    return job
+}
+
+/**
+ * Decodes the avatar thumbnail and writes it as PNG. Blocking, call off the main thread.
+ * Returns null and logs the cause if the body is missing, does not decode or cannot be written.
+ */
+internal fun decodeAvatarToFile(
+    body: ResponseBody?,
+    decode: (InputStream) -> Bitmap? = { BitmapFactory.decodeStream(it) },
+    createFile: () -> File
+): File? {
+    val bitmap = body?.use { decode(it.byteStream()) }
+    if (bitmap == null) {
+        val cause = if (body == null) "request returned no body" else "could not be decoded"
+        Log.e("PickImage", "Avatar thumbnail $cause")
+        return null
+    }
+    return try {
+        val file = createFile()
+        val written = FileOutputStream(file).use { out ->
+            bitmap.compress(Bitmap.CompressFormat.PNG, FULL_QUALITY, out)
+        }
+        if (!written) throw IOException("Bitmap.compress returned false")
+        file
+    } catch (e: IOException) {
+        Log.e("PickImage", "Error saving the avatar image", e)
+        null
+    }
+}
+
 @AutoInjector(NextcloudTalkApplication::class)
-class PickImage(private val activity: Activity, private var currentUser: User?) {
+class PickImage(private val activity: ComponentActivity, private var currentUser: User?) {
 
     @Inject
     lateinit var ncApi: NcApi
@@ -112,42 +161,28 @@ class PickImage(private val activity: Activity, private var currentUser: User?)
         )
         downloadCall.enqueue(object : Callback<ResponseBody> {
             override fun onResponse(call: Call<ResponseBody>, response: Response<ResponseBody>) {
-                saveBitmapAndPassToImagePicker(
-                    startImagePickerForResult,
-                    BitmapFactory.decodeStream(response.body()!!.byteStream())
-                )
+                val body = response.body()
+                activity.lifecycleScope.launchClosing(body) {
+                    val file = withContext(Dispatchers.IO) {
+                        decodeAvatarToFile(body) { createTempFileForAvatar() }
+                    }
+                    if (file != null) {
+                        selectLocal(startImagePickerForResult, file)
+                    } else {
+                        showAvatarError()
+                    }
+                }
             }
 
             override fun onFailure(call: Call<ResponseBody>, t: Throwable) {
-                // unused atm
+                Log.e(TAG, "Failed to download the avatar thumbnail", t)
+                showAvatarError()
             }
         })
     }
 
-    // only possible with API26
-    private fun saveBitmapAndPassToImagePicker(
-        startImagePickerForResult: ActivityResultLauncher<Intent>,
-        bitmap: Bitmap
-    ) {
-        val file: File = saveBitmapToTempFile(bitmap) ?: return
-        selectLocal(startImagePickerForResult, file)
-    }
-
-    private fun saveBitmapToTempFile(bitmap: Bitmap): File? {
-        try {
-            val file = createTempFileForAvatar()
-            try {
-                FileOutputStream(file).use { out ->
-                    bitmap.compress(Bitmap.CompressFormat.PNG, FULL_QUALITY, out)
-                }
-                return file
-            } catch (e: IOException) {
-                Log.e(TAG, "Error compressing bitmap", e)
-            }
-        } catch (e: IOException) {
-            Log.e(TAG, "Error creating temporary avatar image", e)
-        }
-        return null
+    private fun showAvatarError() {
+        Toast.makeText(activity, R.string.nc_common_error_sorry, Toast.LENGTH_LONG).show()
     }
 
     private fun createTempFileForAvatar(): File {
@@ -195,7 +230,6 @@ class PickImage(private val activity: Activity, private var currentUser: User?)
         private const val MAX_SIZE: Int = 1024
         private const val AVATAR_PATH = "photos/avatar.png"
         private const val CAMERA_CAPTURE_PATH = "photos/camera_capture.jpg"
-        private const val FULL_QUALITY: Int = 100
         const val REQUEST_PERMISSION_CAMERA: Int = 1
     }
 }

app/src/test/java/com/nextcloud/talk/utils/PickImageTest.kt

@@ -0,0 +1,88 @@
+/*
+ * Nextcloud Talk - Android Client
+ *
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: GPL-3.0-or-later
+ */
+
+package com.nextcloud.talk.utils
+
+import android.graphics.Bitmap
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.Job
+import okhttp3.MediaType.Companion.toMediaType
+import okhttp3.ResponseBody
+import okhttp3.ResponseBody.Companion.toResponseBody
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNotNull
+import org.junit.Assert.assertNull
+import org.junit.Assert.assertTrue
+import org.junit.Rule
+import org.junit.Test
+import org.junit.rules.TemporaryFolder
+import org.junit.runner.RunWith
+import org.mockito.Mockito.mock
+import org.mockito.Mockito.verify
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.annotation.Config
+import java.io.ByteArrayOutputStream
+import java.io.File
+import java.io.IOException
+
+@RunWith(RobolectricTestRunner::class)
+@Config(sdk = [33])
+class PickImageTest {
+
+    @get:Rule
+    val tmp = TemporaryFolder()
+
+    private fun pngBody(): ResponseBody {
+        val out = ByteArrayOutputStream()
+        Bitmap.createBitmap(2, 2, Bitmap.Config.ARGB_8888).compress(Bitmap.CompressFormat.PNG, 100, out)
+        return out.toByteArray().toResponseBody("image/png".toMediaType())
+    }
+
+    @Test
+    fun nullBodyYieldsNull() {
+        assertNull(decodeAvatarToFile(null) { tmp.newFile() })
+    }
+
+    @Test
+    fun undecodableBodyYieldsNull() {
+        val body = "not an image".toResponseBody("image/png".toMediaType())
+        assertNull(decodeAvatarToFile(body, decode = { null }) { tmp.newFile() })
+    }
+
+    @Test
+    fun writeFailureYieldsNull() {
+        assertNull(decodeAvatarToFile(pngBody()) { throw IOException("disk full") })
+    }
+
+    @Test
+    fun unwritableTargetYieldsNull() {
+        val dir: File = tmp.newFolder()
+        assertNull(decodeAvatarToFile(pngBody()) { dir })
+    }
+
+    @Test
+    fun validBodyIsSavedAsPng() {
+        val target = tmp.newFile()
+        val result = decodeAvatarToFile(pngBody()) { target }
+        assertNotNull(result)
+        assertEquals(target, result)
+        assertTrue(target.length() > 0)
+    }
+
+    @Test
+    fun bodyIsClosedWhenTheScopeIsAlreadyCancelled() {
+        val body = mock(ResponseBody::class.java)
+        var started = false
+        val scope = CoroutineScope(Job().apply { cancel() } + Dispatchers.Unconfined)
+
+        scope.launchClosing(body) { started = true }
+
+        assertTrue(!started)
+        verify(body).close()
+    }
+}