From bda73e88c21cdcf5a2e2aae87e93f48e12570c95 Mon Sep 17 00:00:00 2001 From: MatthewTighe Date: Mon, 12 Sep 2022 13:19:15 -0700 Subject: [PATCH] For #26511: apply wallpapers immediately and observe updates --- .../org/mozilla/fenix/home/HomeFragment.kt | 67 ++++--- .../mozilla/fenix/home/WallpapersObserver.kt | 100 ---------- .../mozilla/fenix/home/HomeFragmentTest.kt | 44 ----- .../fenix/home/WallpapersObserverTest.kt | 183 ------------------ 4 files changed, 45 insertions(+), 349 deletions(-) delete mode 100644 app/src/main/java/org/mozilla/fenix/home/WallpapersObserver.kt delete mode 100644 app/src/test/java/org/mozilla/fenix/home/WallpapersObserverTest.kt 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 272cef228fda..f73334586b8a 100644 --- a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt @@ -88,6 +88,7 @@ import org.mozilla.fenix.ext.hideToolbar import org.mozilla.fenix.ext.nav import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.runIfFragmentIsAttached +import org.mozilla.fenix.ext.scaleToBottomOfView import org.mozilla.fenix.ext.settings import org.mozilla.fenix.gleanplumb.DefaultMessageController import org.mozilla.fenix.gleanplumb.MessagingFeature @@ -115,6 +116,7 @@ import org.mozilla.fenix.tabstray.TabsTrayAccessPoint import org.mozilla.fenix.utils.Settings.Companion.TOP_SITES_PROVIDER_MAX_THRESHOLD import org.mozilla.fenix.utils.ToolbarPopupWindow import org.mozilla.fenix.utils.allowUndo +import org.mozilla.fenix.wallpapers.Wallpaper import java.lang.ref.WeakReference import kotlin.math.min @@ -165,8 +167,7 @@ class HomeFragment : Fragment() { private var appBarLayout: AppBarLayout? = null private lateinit var currentMode: CurrentMode - @VisibleForTesting - internal var wallpapersObserver: WallpapersObserver? = null + private var lastAppliedWallpaperName: String = Wallpaper.defaultName private val topSitesFeature = ViewBoundFeatureWrapper() private val messagingFeature = ViewBoundFeatureWrapper() @@ -214,15 +215,8 @@ class HomeFragment : Fragment() { val activity = activity as HomeActivity val components = requireComponents - if (shouldEnableWallpaper()) { - wallpapersObserver = WallpapersObserver( - appStore = components.appStore, - wallpapersUseCases = components.useCases.wallpaperUseCases, - wallpaperImageView = binding.wallpaperImageView, - ).also { - viewLifecycleOwner.lifecycle.addObserver(it) - } - } + val currentWallpaperName = requireContext().settings().currentWallpaperName + applyWallpaper(wallpaperName = currentWallpaperName, orientationChange = false) currentMode = CurrentMode( requireContext(), @@ -417,16 +411,8 @@ 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, - // see https://github.com/mozilla-mobile/fenix/issues/26638 - runBlockingIncrement { - wallpapersObserver?.applyCurrentWallpaper() - } - } + val currentWallpaperName = requireContext().settings().currentWallpaperName + applyWallpaper(wallpaperName = currentWallpaperName, orientationChange = true) } /** @@ -521,6 +507,7 @@ class HomeFragment : Fragment() { observeSearchEngineChanges() observeSearchEngineNameChanges() + observeWallpaperUpdates() HomeMenuBuilder( view = view, @@ -708,7 +695,6 @@ class HomeFragment : Fragment() { _sessionControlInteractor = null sessionControlView = null appBarLayout = null - wallpapersObserver = null _binding = null bundleArgs.clear() } @@ -953,6 +939,43 @@ class HomeFragment : Fragment() { internal fun shouldEnableWallpaper() = (activity as? HomeActivity)?.themeManager?.currentTheme?.isPrivate?.not() ?: false + private fun applyWallpaper(wallpaperName: String, orientationChange: Boolean) { + when { + !shouldEnableWallpaper() || + (wallpaperName == lastAppliedWallpaperName && !orientationChange) -> return + wallpaperName == Wallpaper.defaultName -> { + binding.wallpaperImageView.isVisible = false + lastAppliedWallpaperName = wallpaperName + } + else -> { + runBlockingIncrement { + // loadBitmap does file lookups based on name, so we don't need a fully + // qualified type to load the image + val wallpaper = Wallpaper.Default.copy(name = wallpaperName) + val wallpaperImage = + requireComponents.useCases.wallpaperUseCases.loadBitmap(wallpaper) + wallpaperImage?.let { + it.scaleToBottomOfView(binding.wallpaperImageView) + binding.wallpaperImageView.isVisible = true + lastAppliedWallpaperName = wallpaperName + } ?: run { + binding.wallpaperImageView.isVisible = false + lastAppliedWallpaperName = Wallpaper.defaultName + } + } + } + } + } + + private fun observeWallpaperUpdates() { + consumeFrom(requireComponents.appStore) { + val currentWallpaper = it.wallpaperState.currentWallpaper + if (currentWallpaper.name != lastAppliedWallpaperName) { + applyWallpaper(wallpaperName = currentWallpaper.name, orientationChange = false) + } + } + } + companion object { const val ALL_NORMAL_TABS = "all_normal" const val ALL_PRIVATE_TABS = "all_private" diff --git a/app/src/main/java/org/mozilla/fenix/home/WallpapersObserver.kt b/app/src/main/java/org/mozilla/fenix/home/WallpapersObserver.kt deleted file mode 100644 index 9e01e6faee3f..000000000000 --- a/app/src/main/java/org/mozilla/fenix/home/WallpapersObserver.kt +++ /dev/null @@ -1,100 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -package org.mozilla.fenix.home - -import android.widget.ImageView -import androidx.annotation.VisibleForTesting -import androidx.core.view.isVisible -import androidx.lifecycle.DefaultLifecycleObserver -import androidx.lifecycle.LifecycleObserver -import androidx.lifecycle.LifecycleOwner -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.cancel -import kotlinx.coroutines.launch -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.wallpapers.Wallpaper -import org.mozilla.fenix.wallpapers.WallpapersUseCases - -/** - * [LifecycleObserver] that will immediately start observing the store for wallpapers updates - * to apply them to the passed in [wallpaperImageView] and automatically stop observing for updates - * when the [LifecycleOwner] is destroyed. - * - * @param appStore Holds the details abut the current wallpaper. - * @param wallpapersUseCases Used for interacting with the wallpaper feature. - * @param wallpaperImageView Serves as the target when applying wallpapers. - */ -class WallpapersObserver( - private val appStore: AppStore, - private val wallpapersUseCases: WallpapersUseCases, - private val wallpaperImageView: ImageView, -) : DefaultLifecycleObserver { - @VisibleForTesting - internal var observeWallpapersStoreSubscription: Store.Subscription? = null - - @VisibleForTesting - internal var wallpapersScope = CoroutineScope(Dispatchers.Main.immediate) - - init { - observeWallpaperUpdates() - } - - /** - * Immediately apply the current wallpaper automatically adjusted to support - * the current configuration - portrait or landscape. - */ - fun applyCurrentWallpaper() { - showWallpaper() - } - - override fun onDestroy(owner: LifecycleOwner) { - observeWallpapersStoreSubscription?.unsubscribe() - wallpapersScope.cancel() - } - - @VisibleForTesting - internal fun observeWallpaperUpdates() { - var lastObservedValue: Wallpaper? = null - observeWallpapersStoreSubscription = appStore.observeManually { state -> - val currentValue = state.wallpaperState.currentWallpaper - // 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. - if (currentValue.name != lastObservedValue?.name) { - lastObservedValue = currentValue - - showWallpaper(currentValue) - } - }.also { - it.resume() - } - } - - @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 - } - } - } - } - } -} diff --git a/app/src/test/java/org/mozilla/fenix/home/HomeFragmentTest.kt b/app/src/test/java/org/mozilla/fenix/home/HomeFragmentTest.kt index 4d6d6e8e3a2b..f3245cb4dc4f 100644 --- a/app/src/test/java/org/mozilla/fenix/home/HomeFragmentTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/HomeFragmentTest.kt @@ -5,13 +5,11 @@ package org.mozilla.fenix.home import android.content.Context -import io.mockk.coVerify import io.mockk.every import io.mockk.mockk import io.mockk.mockkStatic import io.mockk.spyk import io.mockk.verify -import kotlinx.coroutines.test.runTest import mozilla.components.browser.menu.view.MenuButton import mozilla.components.browser.state.state.SearchState import mozilla.components.browser.state.state.selectedOrDefaultSearchEngine @@ -19,7 +17,6 @@ import mozilla.components.feature.top.sites.TopSite import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNotNull -import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test @@ -108,37 +105,6 @@ class HomeFragmentTest { verify(exactly = 1) { menuButton.dismissMenu() } } - @Test - fun `GIVEN the user is in normal mode WHEN configuration changes THEN the wallpaper is reapplied`() = runTest { - homeFragment.getMenuButton = { null } - val observer: WallpapersObserver = mockk(relaxed = true) - homeFragment.wallpapersObserver = observer - val activity: HomeActivity = mockk { - every { themeManager.currentTheme.isPrivate } returns false - } - every { homeFragment.activity } returns activity - - homeFragment.onConfigurationChanged(mockk(relaxed = true)) - - coVerify { observer.applyCurrentWallpaper() } - } - - @Test - fun `GIVEN the user is in private mode WHEN configuration changes THEN the wallpaper not updated`() = runTest { - homeFragment.getMenuButton = { null } - val observer: WallpapersObserver = mockk(relaxed = true) - homeFragment.wallpapersObserver = observer - val activity: HomeActivity = mockk { - every { themeManager.currentTheme.isPrivate } returns true - } - every { homeFragment.activity } returns activity - - homeFragment.onConfigurationChanged(mockk(relaxed = true)) - - coVerify(exactly = 0) { observer.applyCurrentWallpaper() } - } - - @Test fun `GIVEN the user is in normal mode WHEN checking if should enable wallpaper THEN return true`() { val activity: HomeActivity = mockk { every { themeManager.currentTheme.isPrivate } returns false @@ -157,14 +123,4 @@ class HomeFragmentTest { assertFalse(homeFragment.shouldEnableWallpaper()) } - - @Test - fun `GIVEN the wallpaper feature is active WHEN the fragment view is destroyed THEN cleanup the wallpaper observer`() { - homeFragment.bundleArgs = mockk(relaxed = true) - homeFragment.wallpapersObserver = mockk() - - homeFragment.onDestroyView() - - assertNull(homeFragment.wallpapersObserver) - } } diff --git a/app/src/test/java/org/mozilla/fenix/home/WallpapersObserverTest.kt b/app/src/test/java/org/mozilla/fenix/home/WallpapersObserverTest.kt deleted file mode 100644 index dfb9fe0a16ae..000000000000 --- a/app/src/test/java/org/mozilla/fenix/home/WallpapersObserverTest.kt +++ /dev/null @@ -1,183 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -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.every -import io.mockk.just -import io.mockk.mockk -import io.mockk.spyk -import io.mockk.verify -import kotlinx.coroutines.cancel -import mozilla.components.support.test.libstate.ext.waitUntilIdle -import org.junit.Assert.assertNotNull -import org.junit.Assert.assertTrue -import org.junit.Ignore -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.wallpapers.Wallpaper -import org.mozilla.fenix.wallpapers.WallpapersUseCases - -@RunWith(FenixRobolectricTestRunner::class) -class WallpapersObserverTest { - @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()) - - assertNotNull(observer.observeWallpapersStoreSubscription) - } - - @Test - fun `WHEN asked to apply the wallpaper THEN show it`() { - 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 - } - - observer.onDestroy(mockk()) - - verify { observer.wallpapersScope.cancel() } - verify { observer.observeWallpapersStoreSubscription!!.unsubscribe() } - } - - @Test - fun `WHEN the wallpaper is updated THEN show the wallpaper`() { - val appStore = AppStore() - val observer = spyk(WallpapersObserver(appStore, mockk(relaxed = true), mockk(relaxed = true))) { - every { showWallpaper(any()) } 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 newWallpaper: Wallpaper = mockk(relaxed = true) - appStore.dispatch(UpdateCurrentWallpaper(newWallpaper)) - appStore.waitUntilIdle() - verify { observer.showWallpaper(newWallpaper) } - } - - @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`() { - 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 - } - - // 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) } - - val wallpaper: Wallpaper = mockk(relaxed = true) - appStore.dispatch(UpdateCurrentWallpaper(wallpaper)) - appStore.waitUntilIdle() - verify { observer.showWallpaper(wallpaper) } - } - - @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 - } - // 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) } - - appStore.dispatch(UpdateCurrentWallpaper(wallpaper)) - appStore.waitUntilIdle() - verify(exactly = 1) { observer.showWallpaper(wallpaper) } - } - - @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))) - - observer.showWallpaper() - - verify { observer.showWallpaper(wallpaper) } - } - - 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) - - observer.showWallpaper(Wallpaper.Default) - - verify { wallpaperView.isVisible = false } - verify { wallpapersUseCases wasNot Called } - } - - @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) - - observer.showWallpaper(wallpaper) - - verify { wallpaperView.isVisible = true } - verify { wallpaperView.setImageBitmap(bitmap) } - } - - @Test - fun `GIVEN the observer THEN use the main thread for showing the wallpaper`() { - val wallpapersUseCases: WallpapersUseCases = mockk() - val wallpaperView: ImageView = mockk(relaxed = true) - - val observer = WallpapersObserver(mockk(relaxed = true), wallpapersUseCases, wallpaperView) - - // 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")) - } -}