[fenix] For https://github.com/mozilla-mobile/fenix/issues/26723 - Ensure wallpapers are set on the main thread

Using Dispatchers.IO allowed the observer to react faster to wallpaper state
updates but caused issues with updating the wallpaper in fragment layout.

Using Dispatchers.Main.immediate gives us a bit slower reaction time but still
faster than Dispatchers.Main and allows us to still execute before the fragment
is visible.
pull/600/head
Mugurell 2 years ago committed by mergify[bot]
parent aa0ed7ad4a
commit 4662dbd8ab

@ -39,7 +39,7 @@ class WallpapersObserver(
@VisibleForTesting @VisibleForTesting
internal var observeWallpapersStoreSubscription: Store.Subscription<AppState, AppAction>? = null internal var observeWallpapersStoreSubscription: Store.Subscription<AppState, AppAction>? = null
@VisibleForTesting @VisibleForTesting
internal var wallpapersScope = CoroutineScope(Dispatchers.IO) internal var wallpapersScope = CoroutineScope(Dispatchers.Main.immediate)
init { init {
observeWallpaperUpdates() observeWallpaperUpdates()
@ -49,7 +49,7 @@ class WallpapersObserver(
* Immediately apply the current wallpaper automatically adjusted to support * Immediately apply the current wallpaper automatically adjusted to support
* the current configuration - portrait or landscape. * the current configuration - portrait or landscape.
*/ */
suspend fun applyCurrentWallpaper() { fun applyCurrentWallpaper() {
showWallpaper() showWallpaper()
} }
@ -69,7 +69,7 @@ class WallpapersObserver(
if (currentValue.name != lastObservedValue?.name) { if (currentValue.name != lastObservedValue?.name) {
lastObservedValue = currentValue lastObservedValue = currentValue
wallpapersScope.launch { showWallpaper(currentValue) } showWallpaper(currentValue)
} }
}.also { }.also {
it.resume() it.resume()
@ -77,7 +77,8 @@ class WallpapersObserver(
} }
@VisibleForTesting @VisibleForTesting
internal suspend fun showWallpaper(wallpaper: Wallpaper = appStore.state.wallpaperState.currentWallpaper) { internal fun showWallpaper(wallpaper: Wallpaper = appStore.state.wallpaperState.currentWallpaper) {
wallpapersScope.launch {
when (wallpaper) { when (wallpaper) {
// We only want to update the wallpaper when it's different from the default one // 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. // as the default is applied already on xml by default.
@ -86,6 +87,7 @@ class WallpapersObserver(
} }
else -> { else -> {
val bitmap = wallpapersUseCases.loadBitmap(wallpaper) val bitmap = wallpapersUseCases.loadBitmap(wallpaper)
bitmap?.let { bitmap?.let {
it.scaleToBottomOfView(wallpaperImageView) it.scaleToBottomOfView(wallpaperImageView)
wallpaperImageView.isVisible = true wallpaperImageView.isVisible = true
@ -94,3 +96,4 @@ class WallpapersObserver(
} }
} }
} }
}

@ -10,7 +10,6 @@ import androidx.core.view.isVisible
import io.mockk.Called import io.mockk.Called
import io.mockk.Runs import io.mockk.Runs
import io.mockk.coEvery import io.mockk.coEvery
import io.mockk.coVerify
import io.mockk.every import io.mockk.every
import io.mockk.just import io.mockk.just
import io.mockk.mockk import io.mockk.mockk
@ -18,10 +17,8 @@ import io.mockk.spyk
import io.mockk.verify import io.mockk.verify
import kotlinx.coroutines.cancel import kotlinx.coroutines.cancel
import mozilla.components.support.test.libstate.ext.waitUntilIdle 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.assertNotNull import org.junit.Assert.assertNotNull
import org.junit.Rule import org.junit.Assert.assertTrue
import org.junit.Test import org.junit.Test
import org.junit.runner.RunWith import org.junit.runner.RunWith
import org.mozilla.fenix.components.AppStore import org.mozilla.fenix.components.AppStore
@ -32,9 +29,6 @@ import org.mozilla.fenix.wallpapers.WallpapersUseCases
@RunWith(FenixRobolectricTestRunner::class) @RunWith(FenixRobolectricTestRunner::class)
class WallpapersObserverTest { class WallpapersObserverTest {
@get:Rule
val coroutinesTestRule = MainCoroutineRule()
@Test @Test
fun `WHEN the observer is created THEN start observing the store`() { fun `WHEN the observer is created THEN start observing the store`() {
val appStore: AppStore = mockk(relaxed = true) { val appStore: AppStore = mockk(relaxed = true) {
@ -47,15 +41,15 @@ class WallpapersObserverTest {
} }
@Test @Test
fun `WHEN asked to apply the wallpaper THEN show it`() = runTestOnMain { fun `WHEN asked to apply the wallpaper THEN show it`() {
val appStore = AppStore() val appStore = AppStore()
val observer = spyk(WallpapersObserver(appStore, mockk(), mockk())) { val observer = spyk(WallpapersObserver(appStore, mockk(), mockk())) {
coEvery { showWallpaper(any()) } just Runs every { showWallpaper(any()) } just Runs
} }
observer.applyCurrentWallpaper() observer.applyCurrentWallpaper()
coVerify { observer.showWallpaper(any()) } verify { observer.showWallpaper(any()) }
} }
@Test @Test
@ -73,10 +67,10 @@ class WallpapersObserverTest {
} }
@Test @Test
fun `WHEN the wallpaper is updated THEN show the wallpaper`() = runTestOnMain { fun `WHEN the wallpaper is updated THEN show the wallpaper`() {
val appStore = AppStore() val appStore = AppStore()
val observer = spyk(WallpapersObserver(appStore, mockk(relaxed = true), mockk(relaxed = true))) { val observer = spyk(WallpapersObserver(appStore, mockk(relaxed = true), mockk(relaxed = true))) {
coEvery { showWallpaper(any()) } just Runs every { showWallpaper(any()) } just Runs
} }
// Ignore the call on the real instance and call again "observeWallpaperUpdates" // Ignore the call on the real instance and call again "observeWallpaperUpdates"
@ -86,38 +80,38 @@ class WallpapersObserverTest {
val newWallpaper: Wallpaper = mockk(relaxed = true) val newWallpaper: Wallpaper = mockk(relaxed = true)
appStore.dispatch(UpdateCurrentWallpaper(newWallpaper)) appStore.dispatch(UpdateCurrentWallpaper(newWallpaper))
appStore.waitUntilIdle() appStore.waitUntilIdle()
coVerify { observer.showWallpaper(newWallpaper) } verify { observer.showWallpaper(newWallpaper) }
} }
@Test @Test
fun `WHEN the wallpaper is updated to a new one THEN show the wallpaper`() = runTestOnMain { fun `WHEN the wallpaper is updated to a new one THEN show the wallpaper`() {
val appStore = AppStore() val appStore = AppStore()
val wallpapersUseCases: WallpapersUseCases = mockk { val wallpapersUseCases: WallpapersUseCases = mockk {
coEvery { loadBitmap(any()) } returns null coEvery { loadBitmap(any()) } returns null
} }
val observer = spyk(WallpapersObserver(appStore, wallpapersUseCases, mockk(relaxed = true))) { val observer = spyk(WallpapersObserver(appStore, wallpapersUseCases, mockk(relaxed = true))) {
coEvery { showWallpaper(any()) } just Runs every { showWallpaper(any()) } just Runs
} }
// Ignore the call on the real instance and call again "observeWallpaperUpdates" // 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. // on the spy to be able to verify the "showWallpaper" call in the spy.
observer.observeWallpaperUpdates() observer.observeWallpaperUpdates()
coVerify { observer.showWallpaper(Wallpaper.Default) } verify { observer.showWallpaper(Wallpaper.Default) }
val wallpaper: Wallpaper = mockk(relaxed = true) val wallpaper: Wallpaper = mockk(relaxed = true)
appStore.dispatch(UpdateCurrentWallpaper(wallpaper)) appStore.dispatch(UpdateCurrentWallpaper(wallpaper))
appStore.waitUntilIdle() appStore.waitUntilIdle()
coVerify { observer.showWallpaper(wallpaper) } verify { observer.showWallpaper(wallpaper) }
} }
@Test @Test
fun `WHEN the wallpaper is updated to the current one THEN don't try showing the same wallpaper again`() = runTestOnMain { fun `WHEN the wallpaper is updated to the current one THEN don't try showing the same wallpaper again`() {
val appStore = AppStore() val appStore = AppStore()
val wallpapersUseCases: WallpapersUseCases = mockk { val wallpapersUseCases: WallpapersUseCases = mockk {
coEvery { loadBitmap(any()) } returns null coEvery { loadBitmap(any()) } returns null
} }
val observer = spyk(WallpapersObserver(appStore, wallpapersUseCases, mockk(relaxed = true))) { val observer = spyk(WallpapersObserver(appStore, wallpapersUseCases, mockk(relaxed = true))) {
coEvery { showWallpaper(any()) } just Runs every { showWallpaper(any()) } just Runs
} }
// Ignore the call on the real instance and call again "observeWallpaperUpdates" // 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. // on the spy to be able to verify the "showWallpaper" call in the spy.
@ -126,15 +120,15 @@ class WallpapersObserverTest {
val wallpaper: Wallpaper = mockk(relaxed = true) val wallpaper: Wallpaper = mockk(relaxed = true)
appStore.dispatch(UpdateCurrentWallpaper(wallpaper)) appStore.dispatch(UpdateCurrentWallpaper(wallpaper))
appStore.waitUntilIdle() appStore.waitUntilIdle()
coVerify { observer.showWallpaper(wallpaper) } verify { observer.showWallpaper(wallpaper) }
appStore.dispatch(UpdateCurrentWallpaper(wallpaper)) appStore.dispatch(UpdateCurrentWallpaper(wallpaper))
appStore.waitUntilIdle() appStore.waitUntilIdle()
coVerify(exactly = 1) { observer.showWallpaper(wallpaper) } verify(exactly = 1) { observer.showWallpaper(wallpaper) }
} }
@Test @Test
fun `GIVEN no wallpaper is provided WHEN asked to show the wallpaper THEN show the current one`() = runTestOnMain { fun `GIVEN no wallpaper is provided WHEN asked to show the wallpaper THEN show the current one`() {
val wallpaper: Wallpaper = mockk() val wallpaper: Wallpaper = mockk()
val appStore: AppStore = mockk(relaxed = true) { val appStore: AppStore = mockk(relaxed = true) {
every { state.wallpaperState.currentWallpaper } returns wallpaper every { state.wallpaperState.currentWallpaper } returns wallpaper
@ -143,10 +137,10 @@ class WallpapersObserverTest {
observer.showWallpaper() observer.showWallpaper()
coVerify { observer.showWallpaper(wallpaper) } verify { observer.showWallpaper(wallpaper) }
} }
fun `GiVEN the current wallpaper is the default one WHEN showing it THEN hide the wallpaper view`() = runTestOnMain { fun `GiVEN the current wallpaper is the default one WHEN showing it THEN hide the wallpaper view`() {
val wallpapersUseCases: WallpapersUseCases = mockk() val wallpapersUseCases: WallpapersUseCases = mockk()
val wallpaperView: ImageView = mockk(relaxed = true) val wallpaperView: ImageView = mockk(relaxed = true)
val observer = WallpapersObserver(mockk(relaxed = true), wallpapersUseCases, wallpaperView) val observer = WallpapersObserver(mockk(relaxed = true), wallpapersUseCases, wallpaperView)
@ -158,7 +152,7 @@ class WallpapersObserverTest {
} }
@Test @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`() = runTestOnMain { 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 wallpaper: Wallpaper = mockk()
val bitmap: Bitmap = mockk() val bitmap: Bitmap = mockk()
val wallpapersUseCases: WallpapersUseCases = mockk { val wallpapersUseCases: WallpapersUseCases = mockk {
@ -172,4 +166,16 @@ class WallpapersObserverTest {
verify { wallpaperView.isVisible = true } verify { wallpaperView.isVisible = true }
verify { wallpaperView.setImageBitmap(bitmap) } 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"))
}
} }

Loading…
Cancel
Save