Simplify the availability check and data provider interface

Author: 0nko

browser-mode/browser-mode-api/src/main/java/com/duckduckgo/browsermode/api/BrowserModeDataProvider.kt

@@ -23,8 +23,6 @@ package com.duckduckgo.browsermode.api
  * [FIRE][BrowserMode.FIRE] variants without referencing either backing implementation directly.
  */
 interface BrowserModeDataProvider<T> {
-    /**
-     * Returns the [T] bound to [mode]. Suspends until the underlying provider is ready.
-     */
-    suspend fun forMode(mode: BrowserMode): T
+    /** Returns the [T] bound to [mode]. */
+    fun forMode(mode: BrowserMode): T
 }

browser-mode/browser-mode-impl/src/main/java/com/duckduckgo/browsermode/impl/RealFireModeAvailability.kt

@@ -31,7 +31,7 @@ import kotlinx.coroutines.launch
 import javax.inject.Inject
 
 @SingleInstanceIn(AppScope::class)
-@ContributesBinding(AppScope::class)
+@ContributesBinding(AppScope::class, boundType = FireModeAvailability::class)
 @ContributesMultibinding(AppScope::class, boundType = MainProcessLifecycleObserver::class)
 class RealFireModeAvailability @Inject constructor(
     private val fireModeFeature: FireModeFeature,
@@ -48,10 +48,11 @@ class RealFireModeAvailability @Inject constructor(
 
     override fun isAvailable(): Boolean = cachedAvailability ?: computeAndCache()
 
-    private fun computeAndCache(): Boolean =
-        (
-            WebViewFeature.isFeatureSupported(WebViewFeature.MULTI_PROFILE) &&
+    private fun computeAndCache(): Boolean {
+        cachedAvailability?.let { return it }
+        val value = WebViewFeature.isFeatureSupported(WebViewFeature.MULTI_PROFILE) &&
                 fireModeFeature.fireTabs().isEnabled()
-            )
-            .also { cachedAvailability = it }
+        cachedAvailability = value
+        return value
+    }
 }

browser-mode/browser-mode-impl/src/main/java/com/duckduckgo/browsermode/impl/WebStorageProvider.kt

@@ -18,26 +18,23 @@ package com.duckduckgo.browsermode.impl
 
 import android.annotation.SuppressLint
 import android.webkit.WebStorage
+import androidx.annotation.UiThread
 import androidx.webkit.ProfileStore
 import com.duckduckgo.browsermode.api.BrowserMode
 import com.duckduckgo.browsermode.api.BrowserModeDataProvider
 import com.duckduckgo.browsermode.api.FireModeAvailability
-import com.duckduckgo.common.utils.DispatcherProvider
 import com.duckduckgo.di.scopes.AppScope
 import dagger.SingleInstanceIn
-import kotlinx.coroutines.withContext
 import javax.inject.Inject
 
 @SuppressLint("RequiresFeature")
 @SingleInstanceIn(AppScope::class)
 class WebStorageProvider @Inject constructor(
     private val fireModeAvailability: FireModeAvailability,
-    private val dispatchers: DispatcherProvider,
 ) : BrowserModeDataProvider<WebStorage> {
-    override suspend fun forMode(mode: BrowserMode): WebStorage {
+    @UiThread
+    override fun forMode(mode: BrowserMode): WebStorage {
         if (!fireModeAvailability.isAvailable()) return WebStorage.getInstance()
-        return withContext(dispatchers.main()) {
-            ProfileStore.getInstance().getOrCreateProfile(mode.profileName).webStorage
-        }
+        return ProfileStore.getInstance().getOrCreateProfile(mode.profileName).webStorage
     }
 }

browser-mode/browser-mode-impl/src/test/kotlin/com/duckduckgo/browsermode/impl/RealFireModeAvailabilityTest.kt

@@ -17,74 +17,42 @@
 package com.duckduckgo.browsermode.impl
 
 import android.annotation.SuppressLint
-import com.duckduckgo.app.browser.api.WebViewCapabilityChecker
-import com.duckduckgo.app.browser.api.WebViewCapabilityChecker.WebViewCapability.MultiProfile
+import androidx.test.ext.junit.runners.AndroidJUnit4
 import com.duckduckgo.common.test.CoroutineTestRule
 import com.duckduckgo.feature.toggles.api.FakeFeatureToggleFactory
 import com.duckduckgo.feature.toggles.api.Toggle
-import kotlinx.coroutines.test.runTest
 import org.junit.Assert.assertFalse
-import org.junit.Assert.assertTrue
 import org.junit.Rule
 import org.junit.Test
-import org.mockito.kotlin.mock
-import org.mockito.kotlin.stub
-import org.mockito.kotlin.times
-import org.mockito.kotlin.verifyBlocking
+import org.junit.runner.RunWith
 
+@RunWith(AndroidJUnit4::class)
 @SuppressLint("DenyListedApi")
 class RealFireModeAvailabilityTest {
 
     @get:Rule val coroutineRule = CoroutineTestRule()
 
     private val fireModeFeature: FireModeFeature = FakeFeatureToggleFactory.create(FireModeFeature::class.java)
-    private val webViewCapabilityChecker: WebViewCapabilityChecker = mock()
 
-    private val testee = RealFireModeAvailability(fireModeFeature, webViewCapabilityChecker, coroutineRule.testDispatcherProvider)
+    private val testee = RealFireModeAvailability(
+        fireModeFeature,
+        coroutineRule.testDispatcherProvider,
+        coroutineRule.testScope,
+    )
 
     @Test
-    fun `is unavailable when fireTabs flag is disabled`() = runTest {
+    fun `is unavailable when fireTabs flag is disabled`() {
         fireModeFeature.fireTabs().setRawStoredState(Toggle.State(enable = false))
-        webViewCapabilityChecker.stub { onBlocking { isSupported(MultiProfile) }.thenReturn(true) }
 
         assertFalse(testee.isAvailable())
     }
 
     @Test
-    fun `is unavailable when WebView does not support MultiProfile`() = runTest {
-        fireModeFeature.fireTabs().setRawStoredState(Toggle.State(enable = true))
-        webViewCapabilityChecker.stub { onBlocking { isSupported(MultiProfile) }.thenReturn(false) }
-
-        assertFalse(testee.isAvailable())
-    }
-
-    @Test
-    fun `is available when fireTabs is enabled and WebView supports MultiProfile`() = runTest {
-        fireModeFeature.fireTabs().setRawStoredState(Toggle.State(enable = true))
-        webViewCapabilityChecker.stub { onBlocking { isSupported(MultiProfile) }.thenReturn(true) }
-
-        assertTrue(testee.isAvailable())
-    }
-
-    @Test
-    fun `availability checks are cached across calls`() = runTest {
-        fireModeFeature.fireTabs().setRawStoredState(Toggle.State(enable = true))
-        webViewCapabilityChecker.stub { onBlocking { isSupported(MultiProfile) }.thenReturn(true) }
-
-        repeat(5) { testee.isAvailable() }
-
-        verifyBlocking(webViewCapabilityChecker, times(1)) { isSupported(MultiProfile) }
-    }
-
-    @Test
-    fun `availability is frozen on first computation even if flag flips later`() = runTest {
+    fun `availability is frozen on first computation even if flag flips later`() {
         fireModeFeature.fireTabs().setRawStoredState(Toggle.State(enable = false))
-        webViewCapabilityChecker.stub { onBlocking { isSupported(MultiProfile) }.thenReturn(true) }
-
         assertFalse(testee.isAvailable())
 
         fireModeFeature.fireTabs().setRawStoredState(Toggle.State(enable = true))
-
         assertFalse(testee.isAvailable())
     }
 }

browser-mode/browser-mode-impl/src/test/kotlin/com/duckduckgo/browsermode/impl/WebStorageProviderTest.kt

@@ -19,28 +19,21 @@ package com.duckduckgo.browsermode.impl
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import com.duckduckgo.browsermode.api.BrowserMode
 import com.duckduckgo.browsermode.api.FireModeAvailability
-import com.duckduckgo.common.test.CoroutineTestRule
 import junit.framework.TestCase.assertNotNull
-import kotlinx.coroutines.test.runTest
-import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.kotlin.mock
-import org.mockito.kotlin.stub
+import org.mockito.kotlin.whenever
 
 @RunWith(AndroidJUnit4::class)
 class WebStorageProviderTest {
 
-    @get:Rule
-    val coroutineRule = CoroutineTestRule()
-
     private val fireModeAvailability: FireModeAvailability = mock()
-    private val testee =
-        WebStorageProvider(fireModeAvailability, coroutineRule.testDispatcherProvider)
+    private val testee = WebStorageProvider(fireModeAvailability)
 
     @Test
-    fun `forMode returns default WebStorage when multi-profile is unavailable`() = runTest {
-        fireModeAvailability.stub { onBlocking { isAvailable() }.thenReturn(false) }
+    fun `forMode returns default WebStorage when multi-profile is unavailable`() {
+        whenever(fireModeAvailability.isAvailable()).thenReturn(false)
 
         assertNotNull(testee.forMode(BrowserMode.REGULAR))
         assertNotNull(testee.forMode(BrowserMode.FIRE))