From ae59d32f3b62a694df94085024306bb7b9a18f24 Mon Sep 17 00:00:00 2001 From: David Perez Date: Thu, 16 May 2024 10:06:07 -0500 Subject: [PATCH] Fix memory leak by using activity lifecycle scope (#1378) --- .../data/autofill/di/ActivityAutofillModule.kt | 14 +++++++++++--- .../manager/AutofillActivityManagerImpl.kt | 15 +++++---------- .../manager/AutofillActivityManagerTest.kt | 10 ++++++++-- 3 files changed, 24 insertions(+), 15 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/di/ActivityAutofillModule.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/di/ActivityAutofillModule.kt index 74471ec051..e5d4cba3b1 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/di/ActivityAutofillModule.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/di/ActivityAutofillModule.kt @@ -2,11 +2,13 @@ package com.x8bit.bitwarden.data.autofill.di import android.app.Activity import android.view.autofill.AutofillManager +import androidx.lifecycle.LifecycleCoroutineScope +import androidx.lifecycle.LifecycleOwner +import androidx.lifecycle.lifecycleScope import com.x8bit.bitwarden.data.autofill.manager.AutofillActivityManager import com.x8bit.bitwarden.data.autofill.manager.AutofillActivityManagerImpl import com.x8bit.bitwarden.data.autofill.manager.AutofillEnabledManager import com.x8bit.bitwarden.data.platform.manager.AppForegroundManager -import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager import dagger.Module import dagger.Provides import dagger.hilt.InstallIn @@ -27,13 +29,13 @@ object ActivityAutofillModule { @ActivityScopedManager autofillManager: AutofillManager, appForegroundManager: AppForegroundManager, autofillEnabledManager: AutofillEnabledManager, - dispatcherManager: DispatcherManager, + lifecycleScope: LifecycleCoroutineScope, ): AutofillActivityManager = AutofillActivityManagerImpl( autofillManager = autofillManager, appForegroundManager = appForegroundManager, autofillEnabledManager = autofillEnabledManager, - dispatcherManager = dispatcherManager, + lifecycleScope = lifecycleScope, ) /** @@ -46,4 +48,10 @@ object ActivityAutofillModule { fun provideActivityScopedAutofillManager( activity: Activity, ): AutofillManager = activity.getSystemService(AutofillManager::class.java) + + @ActivityScoped + @Provides + fun provideLifecycleCoroutineScope( + activity: Activity, + ): LifecycleCoroutineScope = (activity as LifecycleOwner).lifecycleScope } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/manager/AutofillActivityManagerImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/manager/AutofillActivityManagerImpl.kt index 7f7fdb22dd..e01a7713d0 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/manager/AutofillActivityManagerImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/manager/AutofillActivityManagerImpl.kt @@ -1,9 +1,8 @@ package com.x8bit.bitwarden.data.autofill.manager import android.view.autofill.AutofillManager +import androidx.lifecycle.LifecycleCoroutineScope import com.x8bit.bitwarden.data.platform.manager.AppForegroundManager -import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager -import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach @@ -12,23 +11,19 @@ import kotlinx.coroutines.flow.onEach */ class AutofillActivityManagerImpl( private val autofillManager: AutofillManager, - private val appForegroundManager: AppForegroundManager, private val autofillEnabledManager: AutofillEnabledManager, - private val dispatcherManager: DispatcherManager, + appForegroundManager: AppForegroundManager, + lifecycleScope: LifecycleCoroutineScope, ) : AutofillActivityManager { private val isAutofillEnabledAndSupported: Boolean get() = autofillManager.isEnabled && autofillManager.hasEnabledAutofillServices() && autofillManager.isAutofillSupported - private val unconfinedScope = CoroutineScope(dispatcherManager.unconfined) - init { appForegroundManager .appForegroundStateFlow - .onEach { - autofillEnabledManager.isAutofillEnabled = isAutofillEnabledAndSupported - } - .launchIn(unconfinedScope) + .onEach { autofillEnabledManager.isAutofillEnabled = isAutofillEnabledAndSupported } + .launchIn(lifecycleScope) } } diff --git a/app/src/test/java/com/x8bit/bitwarden/data/autofill/manager/AutofillActivityManagerTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/autofill/manager/AutofillActivityManagerTest.kt index 1ec1923800..525f4d4ab7 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/autofill/manager/AutofillActivityManagerTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/autofill/manager/AutofillActivityManagerTest.kt @@ -1,20 +1,23 @@ package com.x8bit.bitwarden.data.autofill.manager import android.view.autofill.AutofillManager +import androidx.lifecycle.LifecycleCoroutineScope import app.cash.turbine.test -import com.x8bit.bitwarden.data.platform.base.FakeDispatcherManager import com.x8bit.bitwarden.data.platform.manager.AppForegroundManager import com.x8bit.bitwarden.data.platform.manager.model.AppForegroundState import io.mockk.every import io.mockk.just import io.mockk.mockk import io.mockk.runs +import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.Assertions.assertFalse import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test +@OptIn(ExperimentalCoroutinesApi::class) class AutofillActivityManagerTest { private val autofillManager: AutofillManager = mockk { every { hasEnabledAutofillServices() } answers { isAutofillEnabledAndSupported } @@ -28,6 +31,9 @@ class AutofillActivityManagerTest { private val appForegroundManager: AppForegroundManager = mockk { every { appForegroundStateFlow } returns mutableAppForegroundStateFlow } + private val lifecycleScope = mockk { + every { coroutineContext } returns UnconfinedTestDispatcher() + } // We will construct an instance here just to hook the various dependencies together internally @Suppress("unused") @@ -35,7 +41,7 @@ class AutofillActivityManagerTest { autofillManager = autofillManager, appForegroundManager = appForegroundManager, autofillEnabledManager = autofillEnabledManager, - dispatcherManager = FakeDispatcherManager(), + lifecycleScope = lifecycleScope, ) private var isAutofillEnabledAndSupported = false