ishan
/
fenix
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

For #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.
This commit is contained in:
Mugurell 2022-08-31 17:44:27 +03:00 committed by mergify[bot]
parent a7b4134e26
commit b7f9457cc4
2 changed files with 49 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
app/src/main/java/org/mozilla/fenix/home/WallpapersObserver.kt
View File

@ -39,7 +39,7 @@ class WallpapersObserver(
@VisibleForTesting
internal var observeWallpapersStoreSubscription: Store.Subscription<AppState, AppAction>? = null
@VisibleForTesting
internal var wallpapersScope = CoroutineScope(Dispatchers.IO)
internal var wallpapersScope = CoroutineScope(Dispatchers.Main.immediate)
init {
observeWallpaperUpdates()
@ -49,7 +49,7 @@ class WallpapersObserver(
* Immediately apply the current wallpaper automatically adjusted to support
* the current configuration - portrait or landscape.
*/
suspend fun applyCurrentWallpaper() {
fun applyCurrentWallpaper() {
showWallpaper()
}
@ -69,7 +69,7 @@ class WallpapersObserver(
if (currentValue.name != lastObservedValue?.name) {
lastObservedValue = currentValue
wallpapersScope.launch { showWallpaper(currentValue) }
showWallpaper(currentValue)
}
}.also {
it.resume()
@ -77,18 +77,21 @@ class WallpapersObserver(
}
@VisibleForTesting
internal suspend fun showWallpaper(wallpaper: Wallpaper = appStore.state.wallpaperState.currentWallpaper) {
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 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
}
}
}
}

56
app/src/test/java/org/mozilla/fenix/home/WallpapersObserverTest.kt
View File

@ -10,7 +10,6 @@ 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
@ -18,10 +17,8 @@ import io.mockk.spyk
import io.mockk.verify
import kotlinx.coroutines.cancel
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.Rule
import org.junit.Assert.assertTrue
import org.junit.Test
import org.junit.runner.RunWith
import org.mozilla.fenix.components.AppStore
@ -32,9 +29,6 @@ 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) {
@ -47,15 +41,15 @@ class WallpapersObserverTest {
}
@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 observer = spyk(WallpapersObserver(appStore, mockk(), mockk())) {
coEvery { showWallpaper(any()) } just Runs
every { showWallpaper(any()) } just Runs
}
observer.applyCurrentWallpaper()
coVerify { observer.showWallpaper(any()) }
verify { observer.showWallpaper(any()) }
}
@Test
@ -73,10 +67,10 @@ class WallpapersObserverTest {
}
@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 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"
@ -86,38 +80,38 @@ class WallpapersObserverTest {
val newWallpaper: Wallpaper = mockk(relaxed = true)
appStore.dispatch(UpdateCurrentWallpaper(newWallpaper))
appStore.waitUntilIdle()
coVerify { observer.showWallpaper(newWallpaper) }
verify { observer.showWallpaper(newWallpaper) }
}
@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 wallpapersUseCases: WallpapersUseCases = mockk {
coEvery { loadBitmap(any()) } returns null
}
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"
// on the spy to be able to verify the "showWallpaper" call in the spy.
observer.observeWallpaperUpdates()
coVerify { observer.showWallpaper(Wallpaper.Default) }
verify { observer.showWallpaper(Wallpaper.Default) }
val wallpaper: Wallpaper = mockk(relaxed = true)
appStore.dispatch(UpdateCurrentWallpaper(wallpaper))
appStore.waitUntilIdle()
coVerify { observer.showWallpaper(wallpaper) }
verify { observer.showWallpaper(wallpaper) }
}
@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 wallpapersUseCases: WallpapersUseCases = mockk {
coEvery { loadBitmap(any()) } returns null
}
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"
// 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)
appStore.dispatch(UpdateCurrentWallpaper(wallpaper))
appStore.waitUntilIdle()
coVerify { observer.showWallpaper(wallpaper) }
verify { observer.showWallpaper(wallpaper) }
appStore.dispatch(UpdateCurrentWallpaper(wallpaper))
appStore.waitUntilIdle()
coVerify(exactly = 1) { observer.showWallpaper(wallpaper) }
verify(exactly = 1) { observer.showWallpaper(wallpaper) }
}
@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 appStore: AppStore = mockk(relaxed = true) {
every { state.wallpaperState.currentWallpaper } returns wallpaper
@ -143,10 +137,10 @@ class WallpapersObserverTest {
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 wallpaperView: ImageView = mockk(relaxed = true)
val observer = WallpapersObserver(mockk(relaxed = true), wallpapersUseCases, wallpaperView)
@ -158,7 +152,7 @@ class WallpapersObserverTest {
}
@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 bitmap: Bitmap = mockk()
val wallpapersUseCases: WallpapersUseCases = mockk {
@ -172,4 +166,16 @@ class WallpapersObserverTest {
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"))
}
}