From ca4d52d8681be7b567ca2ab051e11ffd6df24f95 Mon Sep 17 00:00:00 2001 From: Jonatan Rhodin Date: Fri, 19 Jan 2024 15:40:38 +0100 Subject: [PATCH 1/2] Handle exceptions when getting app icon in SplitTunnelingScreen --- .../compose/screen/SplitTunnelingScreen.kt | 4 ++-- .../util/PackageManagerExtensions.kt | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) create mode 100644 android/app/src/main/kotlin/net/mullvad/mullvadvpn/util/PackageManagerExtensions.kt diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SplitTunnelingScreen.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SplitTunnelingScreen.kt index 339673949178..5164fe0a3dba 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SplitTunnelingScreen.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SplitTunnelingScreen.kt @@ -23,7 +23,6 @@ import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.platform.LocalFocusManager import androidx.compose.ui.res.stringResource import androidx.compose.ui.tooling.preview.Preview -import androidx.core.graphics.drawable.toBitmapOrNull import com.ramcosta.composedestinations.annotation.Destination import com.ramcosta.composedestinations.navigation.DestinationsNavigator import net.mullvad.mullvadvpn.R @@ -42,6 +41,7 @@ import net.mullvad.mullvadvpn.compose.state.SplitTunnelingUiState import net.mullvad.mullvadvpn.compose.transitions.SlideInFromRightTransition import net.mullvad.mullvadvpn.lib.theme.AppTheme import net.mullvad.mullvadvpn.lib.theme.Dimens +import net.mullvad.mullvadvpn.util.getApplicationIconBitmapOrNull import net.mullvad.mullvadvpn.viewmodel.SplitTunnelingViewModel import org.koin.androidx.compose.koinViewModel @@ -93,7 +93,7 @@ fun SplitTunneling(navigator: DestinationsNavigator) { onIncludeAppClick = viewModel::onIncludeAppClick, onBackClick = navigator::navigateUp, onResolveIcon = { packageName -> - packageManager.getApplicationIcon(packageName).toBitmapOrNull() + packageManager.getApplicationIconBitmapOrNull(packageName) } ) } diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/util/PackageManagerExtensions.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/util/PackageManagerExtensions.kt new file mode 100644 index 000000000000..ea8ce1f4e1ec --- /dev/null +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/util/PackageManagerExtensions.kt @@ -0,0 +1,18 @@ +package net.mullvad.mullvadvpn.util + +import android.content.pm.PackageManager +import android.graphics.Bitmap +import androidx.core.graphics.drawable.toBitmapOrNull + +fun PackageManager.getApplicationIconBitmapOrNull(packageName: String): Bitmap? = + try { + getApplicationIcon(packageName).toBitmapOrNull() + } catch (e: Exception) { + // Name not found is thrown if the application is not installed + // IllegalArgumentException is thrown if the application has an invalid icon + when (e) { + is PackageManager.NameNotFoundException, + is IllegalArgumentException -> null + else -> throw e + } + } From 57d13750893a99bfdfc99fddfd170f554ce4e60f Mon Sep 17 00:00:00 2001 From: Jonatan Rhodin Date: Fri, 19 Jan 2024 15:40:17 +0100 Subject: [PATCH 2/2] Remove unused ApplicationsIconManager --- .../screen/SplitTunnelingScreenTest.kt | 17 --- .../applist/ApplicationsIconManager.kt | 28 ----- .../net/mullvad/mullvadvpn/di/UiModule.kt | 3 - .../applist/ApplicationsIconManagerTest.kt | 110 ------------------ 4 files changed, 158 deletions(-) delete mode 100644 android/app/src/main/kotlin/net/mullvad/mullvadvpn/applist/ApplicationsIconManager.kt delete mode 100644 android/app/src/test/kotlin/net/mullvad/mullvadvpn/applist/ApplicationsIconManagerTest.kt diff --git a/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/SplitTunnelingScreenTest.kt b/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/SplitTunnelingScreenTest.kt index 12e06b009dcf..a4f6d1ab4a38 100644 --- a/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/SplitTunnelingScreenTest.kt +++ b/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/SplitTunnelingScreenTest.kt @@ -1,49 +1,32 @@ package net.mullvad.mullvadvpn.compose.screen -import android.graphics.Bitmap import androidx.compose.ui.test.ExperimentalTestApi import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick import de.mannodermaus.junit5.compose.createComposeExtension import io.mockk.MockKAnnotations -import io.mockk.every import io.mockk.mockk import io.mockk.unmockkAll import io.mockk.verify import net.mullvad.mullvadvpn.applist.AppData -import net.mullvad.mullvadvpn.applist.ApplicationsIconManager import net.mullvad.mullvadvpn.compose.setContentWithTheme import net.mullvad.mullvadvpn.compose.state.SplitTunnelingUiState import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.RegisterExtension -import org.koin.core.context.loadKoinModules -import org.koin.core.context.unloadKoinModules -import org.koin.dsl.module @OptIn(ExperimentalTestApi::class) class SplitTunnelingScreenTest { @JvmField @RegisterExtension val composeExtension = createComposeExtension() - private val mockBitmap: Bitmap = Bitmap.createBitmap(10, 10, Bitmap.Config.ARGB_8888) - private val testModule = module { - single { - mockk().apply { - every { getAppIcon(any()) } returns mockBitmap - } - } - } - @BeforeEach fun setup() { MockKAnnotations.init(this) - loadKoinModules(testModule) } @AfterEach fun tearDown() { - unloadKoinModules(testModule) unmockkAll() } diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/applist/ApplicationsIconManager.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/applist/ApplicationsIconManager.kt deleted file mode 100644 index e1ff07022cbb..000000000000 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/applist/ApplicationsIconManager.kt +++ /dev/null @@ -1,28 +0,0 @@ -package net.mullvad.mullvadvpn.applist - -import android.content.pm.PackageManager -import android.graphics.Bitmap -import android.os.Looper -import androidx.annotation.WorkerThread -import androidx.collection.LruCache -import androidx.core.graphics.drawable.toBitmap - -class ApplicationsIconManager(private val packageManager: PackageManager) { - private val iconsCache = LruCache(500) - - @WorkerThread - @Throws(PackageManager.NameNotFoundException::class) - fun getAppIcon(packageName: String): Bitmap { - check(!Looper.getMainLooper().isCurrentThread) { "Should not be called from MainThread" } - iconsCache.get(packageName)?.let { - return it - } - return packageManager.getApplicationIcon(packageName).toBitmap().also { - iconsCache.put(packageName, it) - } - } - - fun dispose() { - iconsCache.evictAll() - } -} diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/di/UiModule.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/di/UiModule.kt index 56d6b71a2dd3..5e51cc99d4b7 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/di/UiModule.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/di/UiModule.kt @@ -7,7 +7,6 @@ import android.os.Messenger import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.MainScope import net.mullvad.mullvadvpn.BuildConfig -import net.mullvad.mullvadvpn.applist.ApplicationsIconManager import net.mullvad.mullvadvpn.applist.ApplicationsProvider import net.mullvad.mullvadvpn.constant.IS_PLAY_BUILD import net.mullvad.mullvadvpn.dataproxy.MullvadProblemReport @@ -67,7 +66,6 @@ import org.koin.androidx.viewmodel.dsl.viewModel import org.koin.core.qualifier.named import org.koin.dsl.bind import org.koin.dsl.module -import org.koin.dsl.onClose val uiModule = module { single(named(APP_PREFERENCES_NAME)) { @@ -78,7 +76,6 @@ val uiModule = module { single(named(SELF_PACKAGE_NAME)) { androidContext().packageName } viewModel { SplitTunnelingViewModel(get(), get(), Dispatchers.Default) } - single { ApplicationsIconManager(get()) } onClose { it?.dispose() } single { ApplicationsProvider(get(), get(named(SELF_PACKAGE_NAME))) } single { (messenger: Messenger, dispatcher: EventDispatcher) -> diff --git a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/applist/ApplicationsIconManagerTest.kt b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/applist/ApplicationsIconManagerTest.kt deleted file mode 100644 index 3296243c562d..000000000000 --- a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/applist/ApplicationsIconManagerTest.kt +++ /dev/null @@ -1,110 +0,0 @@ -package net.mullvad.mullvadvpn.applist - -import android.content.pm.PackageManager -import android.graphics.Bitmap -import android.graphics.drawable.Drawable -import android.os.Looper -import androidx.core.graphics.drawable.toBitmap -import io.mockk.every -import io.mockk.mockk -import io.mockk.mockkStatic -import io.mockk.unmockkAll -import io.mockk.verify -import kotlin.test.assertEquals -import kotlin.test.assertFails -import org.junit.jupiter.api.AfterEach -import org.junit.jupiter.api.BeforeEach -import org.junit.jupiter.api.Test - -class ApplicationsIconManagerTest { - private val mockedPackageManager = mockk() - private val mockedMainLooper = mockk() - private val testSubject = ApplicationsIconManager(mockedPackageManager) - - @BeforeEach - fun setUp() { - mockkStatic(Looper::class) - mockkStatic(DRAWABLE_EXTENSION_CLASS) - every { Looper.getMainLooper() } returns mockedMainLooper - } - - @AfterEach - fun tearDown() { - unmockkAll() - } - - @Test - fun test_first_time_load_icon_from_PM() { - val testPackageName = "test" - val mockedBitmap = mockk() - val mockedDrawable = mockk().apply { every { toBitmap() } returns mockedBitmap } - every { mockedPackageManager.getApplicationIcon(testPackageName) } returns mockedDrawable - every { mockedMainLooper.isCurrentThread } returns false - every { mockedDrawable.intrinsicWidth } returns 0 - every { mockedDrawable.intrinsicHeight } returns 0 - - val result = testSubject.getAppIcon(testPackageName) - - assertEquals(mockedBitmap, result) - verify { - mockedMainLooper.isCurrentThread - mockedPackageManager.getApplicationIcon(testPackageName) - } - } - - @Test - fun test_second_time_load_icon_from_cache() { - val testPackageName = "test" - val mockedBitmap = mockk() - val mockedDrawable = mockk().apply { every { toBitmap() } returns mockedBitmap } - every { mockedPackageManager.getApplicationIcon(testPackageName) } returns mockedDrawable - every { mockedMainLooper.isCurrentThread } returns false - every { mockedDrawable.intrinsicWidth } returns 0 - every { mockedDrawable.intrinsicHeight } returns 0 - - val result = testSubject.getAppIcon(testPackageName) - val result2 = testSubject.getAppIcon(testPackageName) - - assertEquals(mockedBitmap, result) - assertEquals(mockedBitmap, result2) - verify(exactly = 2) { mockedMainLooper.isCurrentThread } - verify(exactly = 1) { mockedPackageManager.getApplicationIcon(testPackageName) } - } - - @Test - fun test_second_time_load_icon_from_PM_after_clear() { - val testPackageName = "test" - val mockedBitmap = mockk() - val mockedDrawable = mockk().apply { every { toBitmap() } returns mockedBitmap } - every { mockedPackageManager.getApplicationIcon(testPackageName) } returns mockedDrawable - every { mockedMainLooper.isCurrentThread } returns false - every { mockedDrawable.intrinsicWidth } returns 0 - every { mockedDrawable.intrinsicHeight } returns 0 - - val result = testSubject.getAppIcon(testPackageName) - testSubject.dispose() - val result2 = testSubject.getAppIcon(testPackageName) - - assertEquals(mockedBitmap, result) - assertEquals(mockedBitmap, result2) - verify(exactly = 2) { - mockedMainLooper.isCurrentThread - mockedPackageManager.getApplicationIcon(testPackageName) - } - } - - @Test - fun test_throw_exception_when_invoke_from_MainThread() { - val testPackageName = "test" - every { mockedMainLooper.isCurrentThread } returns true - - assertFails("Should not be called from MainThread") { - testSubject.getAppIcon(testPackageName) - } - verify { mockedMainLooper.isCurrentThread } - } - - companion object { - private const val DRAWABLE_EXTENSION_CLASS = "androidx.core.graphics.drawable.DrawableKt" - } -}