2
0
mirror of https://github.com/fork-maintainers/iceraven-browser synced 2024-11-15 18:12:54 +00:00

Revert "For #26511: Parallelize work for setting the wallpaper"

This reverts commit ce3a7152f9.
This commit is contained in:
Christian Sadilek 2022-09-11 10:37:24 -04:00 committed by Ryan VanderMeulen
parent d39759e7ea
commit 9cae449e06
3 changed files with 139 additions and 203 deletions

View File

@ -217,7 +217,6 @@ class HomeFragment : Fragment() {
if (shouldEnableWallpaper()) {
wallpapersObserver = WallpapersObserver(
appStore = components.appStore,
settings = requireContext().settings(),
wallpapersUseCases = components.useCases.wallpaperUseCases,
wallpaperImageView = binding.wallpaperImageView,
).also {
@ -419,6 +418,7 @@ 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,
@ -765,12 +765,6 @@ class HomeFragment : Fragment() {
lifecycleScope.launch(IO) {
requireComponents.reviewPromptController.promptReview(requireActivity())
}
if (shouldEnableWallpaper()) {
runBlockingIncrement {
wallpapersObserver?.applyCurrentWallpaper()
}
}
}
private fun dispatchModeChanges(mode: Mode) {

View File

@ -4,27 +4,21 @@
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
@ -34,45 +28,19 @@ 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<AppState, AppAction>? = 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 currentWallpaperImage: Bitmap? = null
/**
* Listener for when the first observed wallpaper is loaded and available to be set.
*/
@VisibleForTesting
internal val isWallpaperLoaded = CompletableDeferred<Unit>()
internal var wallpapersScope = CoroutineScope(Dispatchers.Main.immediate)
init {
observeWallpaperUpdates()
@ -82,20 +50,8 @@ class WallpapersObserver(
* Immediately apply the current wallpaper automatically adjusted to support
* the current configuration - portrait or landscape.
*/
internal suspend fun applyCurrentWallpaper() {
isWallpaperLoaded.await()
withContext(Dispatchers.Main.immediate) {
with(currentWallpaperImage) {
when (this) {
null -> wallpaperImageView.isVisible = false
else -> {
scaleToBottomOfView(wallpaperImageView)
wallpaperImageView.isVisible = true
}
}
}
}
fun applyCurrentWallpaper() {
showWallpaper()
}
override fun onDestroy(owner: LifecycleOwner) {
@ -108,42 +64,37 @@ 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
wallpapersScope.launch {
loadWallpaper(currentValue)
applyCurrentWallpaper()
}
showWallpaper(currentValue)
}
}.also {
it.resume()
}
}
/**
* Load the bitmap of [wallpaper] and cache it in [currentWallpaperImage].
*/
@WorkerThread
@VisibleForTesting
internal suspend fun loadWallpaper(wallpaper: Wallpaper) {
currentWallpaperImage = when (wallpaper) {
Wallpaper.Default -> null
else -> wallpapersUseCases.loadBitmap(wallpaper)
}
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)
isWallpaperLoaded.complete(Unit)
bitmap?.let {
it.scaleToBottomOfView(wallpaperImageView)
wallpaperImageView.isVisible = true
}
}
}
}
}
}

View File

@ -7,115 +7,55 @@ 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.assertNull
import org.junit.Rule
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.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 = getObserver(appStore)
val observer = WallpapersObserver(appStore, mockk(), mockk())
assertNotNull(observer.observeWallpapersStoreSubscription)
}
@Test
fun `GIVEN a certain wallpaper is chosen WHEN the state is updated with that wallpaper THEN load it it`() = runTestOnMain {
fun `WHEN asked to apply the wallpaper THEN show it`() {
val appStore = AppStore()
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)),
)
// 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()
coVerify { observer.loadWallpaper(any()) }
coVerify { observer.applyCurrentWallpaper() }
}
@Test
fun `GIVEN a certain wallpaper is chosen WHEN the state updated with another wallpaper THEN avoid loading it`() = runTestOnMain {
val appStore = AppStore()
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()
appStore.dispatch(UpdateCurrentWallpaper(otherWallpaper)).joinBlocking()
appStore.waitUntilIdle()
coVerify(exactly = 0) { observer.loadWallpaper(any()) }
coVerify(exactly = 0) { observer.applyCurrentWallpaper() }
}
@Test
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 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
val observer = spyk(WallpapersObserver(appStore, mockk(), mockk())) {
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()
appStore.dispatch(UpdateCurrentWallpaper(wallpaper)).joinBlocking()
appStore.waitUntilIdle()
coVerify(exactly = 1) { observer.loadWallpaper(any()) }
coVerify(exactly = 1) { observer.applyCurrentWallpaper() }
observer.applyCurrentWallpaper()
appStore.dispatch(UpdateCurrentWallpaper(wallpaper)).joinBlocking()
appStore.waitUntilIdle()
coVerify(exactly = 1) { observer.loadWallpaper(any()) }
coVerify(exactly = 1) { 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 = getObserver(mockk(relaxed = true))
val observer = WallpapersObserver(mockk(relaxed = true), mockk(), mockk())
observer.observeWallpapersStoreSubscription = mockk(relaxed = true)
observer.wallpapersScope = mockk {
every { cancel() } just Runs
@ -128,65 +68,116 @@ class WallpapersObserverTest {
}
@Test
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.applyCurrentWallpaper()
verify { imageView.setImageBitmap(image) }
verify { imageView.isVisible = true }
}
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.applyCurrentWallpaper()
verify { imageView.isVisible = false }
verify(exactly = 0) { imageView.setImageBitmap(any()) }
}
@Test
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.loadWallpaper(Wallpaper.Default)
assertNull(observer.currentWallpaperImage)
}
@Test
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
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
}
val observer = getObserver(wallpapersUseCases = usecases)
observer.loadWallpaper(wallpaper)
// 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()
assertEquals(bitmap, observer.currentWallpaperImage)
val newWallpaper: Wallpaper = mockk(relaxed = true)
appStore.dispatch(UpdateCurrentWallpaper(newWallpaper))
appStore.waitUntilIdle()
verify { observer.showWallpaper(newWallpaper) }
}
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,
)
@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"))
}
}