diff --git a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt index d88c4b743..8b32ed7fb 100644 --- a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt @@ -213,6 +213,7 @@ class HomeFragment : Fragment() { if (shouldEnableWallpaper()) { wallpapersObserver = WallpapersObserver( appStore = components.appStore, + settings = requireContext().settings(), wallpapersUseCases = components.useCases.wallpaperUseCases, wallpaperImageView = binding.wallpaperImageView, ).also { @@ -412,7 +413,6 @@ class HomeFragment : Fragment() { getMenuButton()?.dismissMenu() if (shouldEnableWallpaper()) { - // Setting the wallpaper is a potentially expensive operation - can take 100ms. // Running this on the Main thread helps to ensure that the just updated configuration // will be used when the wallpaper is scaled to match. // Otherwise the portrait wallpaper may remain shown on landscape, @@ -757,6 +757,12 @@ class HomeFragment : Fragment() { lifecycleScope.launch(IO) { requireComponents.reviewPromptController.promptReview(requireActivity()) } + + if (shouldEnableWallpaper()) { + runBlockingIncrement { + wallpapersObserver?.applyCurrentWallpaper() + } + } } private fun dispatchModeChanges(mode: Mode) { diff --git a/app/src/main/java/org/mozilla/fenix/home/WallpapersObserver.kt b/app/src/main/java/org/mozilla/fenix/home/WallpapersObserver.kt index 25456f43f..83d7bb9af 100644 --- a/app/src/main/java/org/mozilla/fenix/home/WallpapersObserver.kt +++ b/app/src/main/java/org/mozilla/fenix/home/WallpapersObserver.kt @@ -4,21 +4,27 @@ package org.mozilla.fenix.home +import android.graphics.Bitmap import android.widget.ImageView import androidx.annotation.VisibleForTesting +import androidx.annotation.WorkerThread import androidx.core.view.isVisible import androidx.lifecycle.DefaultLifecycleObserver import androidx.lifecycle.LifecycleObserver import androidx.lifecycle.LifecycleOwner +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.cancel import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import mozilla.components.lib.state.Store import org.mozilla.fenix.components.AppStore import org.mozilla.fenix.components.appstate.AppAction import org.mozilla.fenix.components.appstate.AppState import org.mozilla.fenix.ext.scaleToBottomOfView +import org.mozilla.fenix.utils.Settings import org.mozilla.fenix.wallpapers.Wallpaper import org.mozilla.fenix.wallpapers.WallpapersUseCases @@ -28,18 +34,44 @@ import org.mozilla.fenix.wallpapers.WallpapersUseCases * when the [LifecycleOwner] is destroyed. * * @param appStore Holds the details abut the current wallpaper. + * @param settings Used for checking user's option for what wallpaper to use. * @param wallpapersUseCases Used for interacting with the wallpaper feature. * @param wallpaperImageView Serves as the target when applying wallpapers. + * @param backgroundWorkDispatcher Used for scheduling the wallpaper update when the state is updated + * with a new wallpaper. */ class WallpapersObserver( private val appStore: AppStore, + private val settings: Settings, private val wallpapersUseCases: WallpapersUseCases, private val wallpaperImageView: ImageView, + backgroundWorkDispatcher: CoroutineDispatcher = Dispatchers.IO, ) : DefaultLifecycleObserver { @VisibleForTesting internal var observeWallpapersStoreSubscription: Store.Subscription? = null + + /** + * Coroutine scope for updating the wallpapers when an update is observed. + * Allows for easy cleanup when the client of this is destroyed. + */ + @VisibleForTesting + internal var wallpapersScope = CoroutineScope(backgroundWorkDispatcher) + + /** + * Setting the wallpaper assumes two steps: + * - load - running on IO + * - set - running on Main + * This property caches the result of [loadWallpaper] to be later used by [applyCurrentWallpaper]. + */ + @Volatile @VisibleForTesting - internal var wallpapersScope = CoroutineScope(Dispatchers.Main.immediate) + internal var currentWallpaperImage: Bitmap? = null + + /** + * Listener for when the first observed wallpaper is loaded and available to be set. + */ + @VisibleForTesting + internal val isWallpaperLoaded = CompletableDeferred() init { observeWallpaperUpdates() @@ -49,8 +81,20 @@ class WallpapersObserver( * Immediately apply the current wallpaper automatically adjusted to support * the current configuration - portrait or landscape. */ - fun applyCurrentWallpaper() { - showWallpaper() + internal suspend fun applyCurrentWallpaper() { + isWallpaperLoaded.await() + + withContext(Dispatchers.Main.immediate) { + with(currentWallpaperImage) { + when (this) { + null -> wallpaperImageView.isVisible = false + else -> { + scaleToBottomOfView(wallpaperImageView) + wallpaperImageView.isVisible = true + } + } + } + } } override fun onDestroy(owner: LifecycleOwner) { @@ -63,37 +107,42 @@ class WallpapersObserver( var lastObservedValue: Wallpaper? = null observeWallpapersStoreSubscription = appStore.observeManually { state -> val currentValue = state.wallpaperState.currentWallpaper + + // Use the persisted wallpaper name to wait until a state update + // that contains the wallpaper that the user chose. + // Avoids setting the AppState default wallpaper if we know that another wallpaper is chosen. + if (currentValue.name != settings.currentWallpaperName) { + return@observeManually + } + // Use the wallpaper name to differentiate between updates to properly support // the restored from settings wallpaper being the same as the one downloaded // case in which details like "collection" may be different. + // Avoids setting the same wallpaper twice. if (currentValue.name != lastObservedValue?.name) { lastObservedValue = currentValue - showWallpaper(currentValue) + wallpapersScope.launch { + loadWallpaper(currentValue) + applyCurrentWallpaper() + } } }.also { it.resume() } } + /** + * Load the bitmap of [wallpaper] and cache it in [currentWallpaperImage]. + */ + @WorkerThread @VisibleForTesting - internal fun showWallpaper(wallpaper: Wallpaper = appStore.state.wallpaperState.currentWallpaper) { - wallpapersScope.launch { - when (wallpaper) { - // We only want to update the wallpaper when it's different from the default one - // as the default is applied already on xml by default. - Wallpaper.Default -> { - wallpaperImageView.isVisible = false - } - else -> { - val bitmap = wallpapersUseCases.loadBitmap(wallpaper) - - bitmap?.let { - it.scaleToBottomOfView(wallpaperImageView) - wallpaperImageView.isVisible = true - } - } - } + internal suspend fun loadWallpaper(wallpaper: Wallpaper) { + currentWallpaperImage = when (wallpaper) { + Wallpaper.Default -> null + else -> wallpapersUseCases.loadBitmap(wallpaper) } + + isWallpaperLoaded.complete(Unit) } } diff --git a/app/src/test/java/org/mozilla/fenix/home/WallpapersObserverTest.kt b/app/src/test/java/org/mozilla/fenix/home/WallpapersObserverTest.kt index dfb9fe0a1..08aff3c0c 100644 --- a/app/src/test/java/org/mozilla/fenix/home/WallpapersObserverTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/WallpapersObserverTest.kt @@ -7,177 +7,186 @@ package org.mozilla.fenix.home import android.graphics.Bitmap import android.widget.ImageView import androidx.core.view.isVisible -import io.mockk.Called import io.mockk.Runs import io.mockk.coEvery +import io.mockk.coVerify import io.mockk.every import io.mockk.just import io.mockk.mockk import io.mockk.spyk import io.mockk.verify +import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.cancel +import mozilla.components.support.test.ext.joinBlocking import mozilla.components.support.test.libstate.ext.waitUntilIdle +import mozilla.components.support.test.rule.MainCoroutineRule +import mozilla.components.support.test.rule.runTestOnMain +import org.junit.Assert.assertEquals import org.junit.Assert.assertNotNull -import org.junit.Assert.assertTrue -import org.junit.Ignore +import org.junit.Assert.assertNull +import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.mozilla.fenix.components.AppStore import org.mozilla.fenix.components.appstate.AppAction.WallpaperAction.UpdateCurrentWallpaper import org.mozilla.fenix.helpers.FenixRobolectricTestRunner +import org.mozilla.fenix.utils.Settings import org.mozilla.fenix.wallpapers.Wallpaper import org.mozilla.fenix.wallpapers.WallpapersUseCases @RunWith(FenixRobolectricTestRunner::class) class WallpapersObserverTest { + @get:Rule + val coroutinesTestRule = MainCoroutineRule() + @Test fun `WHEN the observer is created THEN start observing the store`() { val appStore: AppStore = mockk(relaxed = true) { every { observeManually(any()) } answers { mockk(relaxed = true) } } - val observer = WallpapersObserver(appStore, mockk(), mockk()) + val observer = getObserver(appStore) assertNotNull(observer.observeWallpapersStoreSubscription) } @Test - fun `WHEN asked to apply the wallpaper THEN show it`() { + fun `GIVEN a certain wallpaper is chosen WHEN the state is updated with that wallpaper THEN load it it`() = runTestOnMain { val appStore = AppStore() - val observer = spyk(WallpapersObserver(appStore, mockk(), mockk())) { - every { showWallpaper(any()) } just Runs - } - - observer.applyCurrentWallpaper() - - verify { observer.showWallpaper(any()) } - } - - @Test - fun `GIVEN the store was observed for updates WHEN the lifecycle owner is destroyed THEN stop observing the store`() { - val observer = WallpapersObserver(mockk(relaxed = true), mockk(), mockk()) - observer.observeWallpapersStoreSubscription = mockk(relaxed = true) - observer.wallpapersScope = mockk { - every { cancel() } just Runs - } + val settings: Settings = mockk { every { currentWallpaperName } returns "Test" } + val wallpaper: Wallpaper = mockk { every { name } returns "Test" } + val observer = spyk( + getObserver(appStore, settings, mockk(relaxed = true), mockk(relaxed = true)), + ) - observer.onDestroy(mockk()) + // Ignore the call on the real instance and call again "observeWallpaperUpdates" + // on the spy to be able to verify the "showWallpaper" call in the spy. + observer.observeWallpaperUpdates() + appStore.dispatch(UpdateCurrentWallpaper(wallpaper)).joinBlocking() + appStore.waitUntilIdle() - verify { observer.wallpapersScope.cancel() } - verify { observer.observeWallpapersStoreSubscription!!.unsubscribe() } + coVerify { observer.loadWallpaper(any()) } + coVerify { observer.applyCurrentWallpaper() } } @Test - fun `WHEN the wallpaper is updated THEN show the wallpaper`() { + fun `GIVEN a certain wallpaper is chosen WHEN the state updated with another wallpaper THEN avoid loading it`() = runTestOnMain { val appStore = AppStore() - val observer = spyk(WallpapersObserver(appStore, mockk(relaxed = true), mockk(relaxed = true))) { - every { showWallpaper(any()) } just Runs - } + val settings: Settings = mockk { every { currentWallpaperName } returns "Test" } + val otherWallpaper: Wallpaper = mockk { every { name } returns "Other" } + val observer = spyk( + getObserver(appStore, settings, mockk(relaxed = true), mockk(relaxed = true)), + ) // Ignore the call on the real instance and call again "observeWallpaperUpdates" // on the spy to be able to verify the "showWallpaper" call in the spy. observer.observeWallpaperUpdates() - - val newWallpaper: Wallpaper = mockk(relaxed = true) - appStore.dispatch(UpdateCurrentWallpaper(newWallpaper)) + appStore.dispatch(UpdateCurrentWallpaper(otherWallpaper)).joinBlocking() appStore.waitUntilIdle() - verify { observer.showWallpaper(newWallpaper) } + + coVerify(exactly = 0) { observer.loadWallpaper(any()) } + coVerify(exactly = 0) { observer.applyCurrentWallpaper() } } @Test - @Ignore("Intermittent test: https://github.com/mozilla-mobile/fenix/issues/26760") - fun `WHEN the wallpaper is updated to a new one THEN show the wallpaper`() { + fun `GIVEN a wallpaper is SHOWN WHEN the wallpaper is updated to the current one THEN don't try showing the same wallpaper again`() { val appStore = AppStore() - val wallpapersUseCases: WallpapersUseCases = mockk { - coEvery { loadBitmap(any()) } returns null - } - val observer = spyk(WallpapersObserver(appStore, wallpapersUseCases, mockk(relaxed = true))) { - every { showWallpaper(any()) } just Runs + val settings: Settings = mockk { every { currentWallpaperName } returns "Test" } + val wallpaper: Wallpaper = mockk { every { name } returns "Test" } + val wallpapersUseCases: WallpapersUseCases = mockk { coEvery { loadBitmap(any()) } returns null } + val observer = spyk(getObserver(appStore, settings, wallpapersUseCases, mockk(relaxed = true))) { + coEvery { loadWallpaper(any()) } just Runs + coEvery { applyCurrentWallpaper() } just Runs } // Ignore the call on the real instance and call again "observeWallpaperUpdates" // on the spy to be able to verify the "showWallpaper" call in the spy. observer.observeWallpaperUpdates() - verify { observer.showWallpaper(Wallpaper.Default) } + appStore.dispatch(UpdateCurrentWallpaper(wallpaper)).joinBlocking() + appStore.waitUntilIdle() + coVerify(exactly = 1) { observer.loadWallpaper(any()) } + coVerify(exactly = 1) { observer.applyCurrentWallpaper() } - val wallpaper: Wallpaper = mockk(relaxed = true) - appStore.dispatch(UpdateCurrentWallpaper(wallpaper)) + appStore.dispatch(UpdateCurrentWallpaper(wallpaper)).joinBlocking() appStore.waitUntilIdle() - verify { observer.showWallpaper(wallpaper) } + coVerify(exactly = 1) { observer.loadWallpaper(any()) } + coVerify(exactly = 1) { observer.applyCurrentWallpaper() } } @Test - fun `WHEN the wallpaper is updated to the current one THEN don't try showing the same wallpaper again`() { - val appStore = AppStore() - val wallpapersUseCases: WallpapersUseCases = mockk { - coEvery { loadBitmap(any()) } returns null - } - val observer = spyk(WallpapersObserver(appStore, wallpapersUseCases, mockk(relaxed = true))) { - every { showWallpaper(any()) } just Runs + fun `GIVEN the store was observed for updates WHEN the lifecycle owner is destroyed THEN stop observing the store`() { + val observer = getObserver(mockk(relaxed = true)) + observer.observeWallpapersStoreSubscription = mockk(relaxed = true) + observer.wallpapersScope = mockk { + every { cancel() } just Runs } - // Ignore the call on the real instance and call again "observeWallpaperUpdates" - // on the spy to be able to verify the "showWallpaper" call in the spy. - observer.observeWallpaperUpdates() - val wallpaper: Wallpaper = mockk(relaxed = true) - appStore.dispatch(UpdateCurrentWallpaper(wallpaper)) - appStore.waitUntilIdle() - verify { observer.showWallpaper(wallpaper) } + observer.onDestroy(mockk()) - appStore.dispatch(UpdateCurrentWallpaper(wallpaper)) - appStore.waitUntilIdle() - verify(exactly = 1) { observer.showWallpaper(wallpaper) } + verify { observer.wallpapersScope.cancel() } + verify { observer.observeWallpapersStoreSubscription!!.unsubscribe() } } @Test - fun `GIVEN no wallpaper is provided WHEN asked to show the wallpaper THEN show the current one`() { - val wallpaper: Wallpaper = mockk() - val appStore: AppStore = mockk(relaxed = true) { - every { state.wallpaperState.currentWallpaper } returns wallpaper - } - val observer = spyk(WallpapersObserver(appStore, mockk(relaxed = true), mockk(relaxed = true))) + fun `GIVEN a wallpaper image is available WHEN asked to apply the current wallpaper THEN show set it to the wallpaper ImageView`() = runTestOnMain { + val imageView: ImageView = mockk(relaxed = true) + val observer = getObserver(wallpaperImageView = imageView) + val image: Bitmap = mockk() + observer.currentWallpaperImage = image + observer.isWallpaperLoaded.complete(Unit) - observer.showWallpaper() + observer.applyCurrentWallpaper() - verify { observer.showWallpaper(wallpaper) } + verify { imageView.setImageBitmap(image) } + verify { imageView.isVisible = true } } - fun `GiVEN the current wallpaper is the default one WHEN showing it THEN hide the wallpaper view`() { - val wallpapersUseCases: WallpapersUseCases = mockk() - val wallpaperView: ImageView = mockk(relaxed = true) - val observer = WallpapersObserver(mockk(relaxed = true), wallpapersUseCases, wallpaperView) + fun `GIVEN no wallpaper image is available WHEN asked to apply the current wallpaper THEN hide the wallpaper ImageView`() = runTestOnMain { + val imageView: ImageView = mockk() + val observer = getObserver(wallpaperImageView = imageView) + observer.isWallpaperLoaded.complete(Unit) - observer.showWallpaper(Wallpaper.Default) + observer.applyCurrentWallpaper() - verify { wallpaperView.isVisible = false } - verify { wallpapersUseCases wasNot Called } + verify { imageView.isVisible = false } + verify(exactly = 0) { imageView.setImageBitmap(any()) } } @Test - fun `GiVEN the current wallpaper is different than the default one WHEN showing it THEN load it's bitmap in the visible wallpaper view`() { - val wallpaper: Wallpaper = mockk() - val bitmap: Bitmap = mockk() - val wallpapersUseCases: WallpapersUseCases = mockk { - coEvery { loadBitmap(any()) } returns bitmap - } - val wallpaperView: ImageView = mockk(relaxed = true) - val observer = WallpapersObserver(mockk(relaxed = true), wallpapersUseCases, wallpaperView) + fun `GIVEN the default wallpaper WHEN asked to load it THEN cache that the current image is null`() = runTestOnMain { + val observer = getObserver() + observer.currentWallpaperImage = mockk() - observer.showWallpaper(wallpaper) + observer.loadWallpaper(Wallpaper.Default) - verify { wallpaperView.isVisible = true } - verify { wallpaperView.setImageBitmap(bitmap) } + assertNull(observer.currentWallpaperImage) } @Test - fun `GIVEN the observer THEN use the main thread for showing the wallpaper`() { - val wallpapersUseCases: WallpapersUseCases = mockk() - val wallpaperView: ImageView = mockk(relaxed = true) + fun `GIVEN a custom wallpaper WHEN asked to load it THEN cache it's bitmap`() = runTestOnMain { + val bitmap: Bitmap = mockk() + val wallpaper: Wallpaper = mockk() + val usecases: WallpapersUseCases = mockk { + coEvery { loadBitmap(wallpaper) } returns bitmap + } + val observer = getObserver(wallpapersUseCases = usecases) - val observer = WallpapersObserver(mockk(relaxed = true), wallpapersUseCases, wallpaperView) + observer.loadWallpaper(wallpaper) - // Check that the context that would be used is Dispatchers.Main.immediate - // Unfortunately I could not also test that this is actually used when "showWallpaper" is called. - assertTrue(observer.wallpapersScope.toString().contains("Dispatchers.Main.immediate")) + assertEquals(bitmap, observer.currentWallpaperImage) } + + private fun getObserver( + appStore: AppStore = mockk(relaxed = true), + settings: Settings = mockk(), + wallpapersUseCases: WallpapersUseCases = mockk(), + wallpaperImageView: ImageView = mockk(), + backgroundWorkDispatcher: CoroutineDispatcher = coroutinesTestRule.testDispatcher, + ) = WallpapersObserver( + appStore = appStore, + settings = settings, + wallpapersUseCases = wallpapersUseCases, + wallpaperImageView = wallpaperImageView, + backgroundWorkDispatcher = backgroundWorkDispatcher, + ) }