Merge #6909 #6948

6909: Close #6842: Migrate to browser-state in BrowserThumbnails r=csadilek a=jonalmeida



6948: For #1159: Enable to download blob files in GeckoViewFetchClient r=Amejia481 a=kglazko
Co-authored-by: Jonathan Almeida <jalmeida@mozilla.com>
Co-authored-by: Kate Glazko <kglazko@Kates-MacBook-Pro.local>

components/browser/thumbnails/build.gradle

@@ -26,18 +26,30 @@ android {
     }
 }
 
+tasks.withType(org.jetbrains.kotlin.gradle.tasks.KotlinCompile).all {
+    kotlinOptions.freeCompilerArgs += [
+            "-Xuse-experimental=kotlinx.coroutines.ExperimentalCoroutinesApi"
+    ]
+}
+
 dependencies {
-    implementation project(':browser-session')
+    implementation project(':browser-state')
     implementation project(':concept-engine')
     implementation project(':support-ktx')
 
     implementation Dependencies.androidx_annotation
+    implementation Dependencies.androidx_core_ktx
+    implementation Dependencies.kotlin_coroutines
+    implementation Dependencies.kotlin_stdlib
 
     testImplementation project(':support-test')
+    testImplementation project(':support-test-libstate')
 
+    testImplementation Dependencies.androidx_test_core
     testImplementation Dependencies.androidx_test_junit
     testImplementation Dependencies.testing_mockito
     testImplementation Dependencies.testing_robolectric
+    testImplementation Dependencies.testing_coroutines
 }
 
 apply from: '../../../publish.gradle'

components/browser/thumbnails/src/main/java/mozilla/components/browser/thumbnails/BrowserThumbnails.kt

@@ -6,69 +6,71 @@ package mozilla.components.browser.thumbnails
 
 import android.content.Context
 import androidx.annotation.VisibleForTesting
-import mozilla.components.browser.session.SelectionAwareSessionObserver
-import mozilla.components.browser.session.Session
-import mozilla.components.browser.session.SessionManager
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.cancel
+import kotlinx.coroutines.flow.map
+import kotlinx.coroutines.flow.collect
+import mozilla.components.browser.state.action.ContentAction
+import mozilla.components.browser.state.selector.selectedTab
+import mozilla.components.browser.state.store.BrowserStore
+import mozilla.components.browser.state.state.ContentState
 import mozilla.components.concept.engine.EngineView
+import mozilla.components.lib.state.ext.flowScoped
 import mozilla.components.support.base.feature.LifecycleAwareFeature
 import mozilla.components.support.ktx.android.content.isOSOnLowMemory
+import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifChanged
 
 /**
  * Feature implementation for automatically taking thumbnails of sites.
  * The feature will take a screenshot when the page finishes loading,
- * and will add it to the [Session.thumbnail] property.
+ * and will add it to the [ContentState.thumbnail] property.
  *
  * If the OS is under low memory conditions, the screenshot will be not taken.
- * Ideally, this should be used in conjunction with [SessionManager.onLowMemory] to allow
- * free up some [Session.thumbnail] from memory.
+ * Ideally, this should be used in conjunction with `SessionManager.onLowMemory` to allow
+ * free up some [ContentState.thumbnail] from memory.
  */
 class BrowserThumbnails(
     private val context: Context,
     private val engineView: EngineView,
-    sessionManager: SessionManager
+    private val store: BrowserStore
 ) : LifecycleAwareFeature {
 
-    private val observer = ThumbnailsRequestObserver(sessionManager)
+    private var scope: CoroutineScope? = null
 
     /**
-     * Starts observing the selected session to listen for when a session finish loading.
+     * Starts observing the selected session to listen for when a session finishes loading.
      */
     override fun start() {
-        observer.observeSelected()
-    }
-
-    /**
-     * Stops observing the selected session.
-     */
-    override fun stop() {
-        observer.stop()
-    }
-
-    internal inner class ThumbnailsRequestObserver(
-        sessionManager: SessionManager
-    ) : SelectionAwareSessionObserver(sessionManager) {
-
-        override fun onLoadingStateChanged(session: Session, loading: Boolean) {
-            if (!loading) {
-                requestScreenshot(session)
-            }
-        }
-
-        override fun onTitleChanged(session: Session, title: String) {
-            requestScreenshot(session)
+        scope = store.flowScoped { flow ->
+            flow.map { it.selectedTab }
+                .ifChanged { it?.content?.loading }
+                .collect { state ->
+                    if (state?.content?.loading == false) {
+                        requestScreenshot()
+                    }
+                }
         }
     }
 
-    private fun requestScreenshot(session: Session) {
+    @VisibleForTesting
+    internal fun requestScreenshot() {
         if (!isLowOnMemory()) {
             engineView.captureThumbnail {
-                session.thumbnail = it
+                val bitmap = it ?: return@captureThumbnail
+                val tabId = store.state.selectedTabId ?: return@captureThumbnail
+
+                store.dispatch(ContentAction.UpdateThumbnailAction(tabId, bitmap))
             }
-        } else {
-            session.thumbnail = null
         }
     }
 
+    /**
+     * Stops observing the selected session.
+     */
+    override fun stop() {
+        scope?.cancel()
+    }
+
     @VisibleForTesting
     internal var testLowMemory = false
 

components/browser/thumbnails/src/test/java/mozilla/components/browser/thumbnails/BrowserThumbnailsTest.kt

@@ -4,98 +4,143 @@
 
 package mozilla.components.browser.thumbnails
 
+import android.graphics.Bitmap
 import androidx.test.ext.junit.runners.AndroidJUnit4
-import mozilla.components.browser.session.Session
-import mozilla.components.browser.session.SessionManager
-import mozilla.components.concept.engine.Engine
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.test.TestCoroutineDispatcher
+import kotlinx.coroutines.test.resetMain
+import kotlinx.coroutines.test.setMain
+import mozilla.components.browser.state.action.ContentAction
+import mozilla.components.browser.state.state.BrowserState
+import mozilla.components.browser.state.state.createTab
+import mozilla.components.browser.state.store.BrowserStore
 import mozilla.components.concept.engine.EngineView
 import mozilla.components.support.test.any
+import mozilla.components.support.test.ext.joinBlocking
 import mozilla.components.support.test.mock
 import mozilla.components.support.test.robolectric.testContext
-import org.junit.Assert.assertNull
+import org.junit.After
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.mockito.Mockito.`when`
 import org.mockito.Mockito.never
 import org.mockito.Mockito.spy
 import org.mockito.Mockito.verify
+import org.mockito.Mockito.verifyNoMoreInteractions
+import org.mockito.Mockito.verifyZeroInteractions
 
 @RunWith(AndroidJUnit4::class)
 class BrowserThumbnailsTest {
 
-    private lateinit var mockSessionManager: SessionManager
-    private lateinit var mockEngineView: EngineView
+    private val testDispatcher = TestCoroutineDispatcher()
+
+    private lateinit var store: BrowserStore
+    private lateinit var engineView: EngineView
     private lateinit var thumbnails: BrowserThumbnails
+    private val tabId = "test-tab"
 
     @Before
     fun setup() {
-        val engine = mock<Engine>()
-        mockSessionManager = spy(SessionManager(engine))
-        mockEngineView = mock()
-        thumbnails = BrowserThumbnails(testContext, mockEngineView, mockSessionManager)
+        Dispatchers.setMain(testDispatcher)
+        store = spy(BrowserStore(BrowserState(
+            tabs = listOf(
+                createTab("https://www.mozilla.org", id = tabId)
+            ),
+            selectedTabId = tabId
+        )))
+        engineView = mock()
+        thumbnails = BrowserThumbnails(testContext, engineView, store)
+    }
+
+    @After
+    fun tearDown() {
+        Dispatchers.resetMain()
+        testDispatcher.cleanupTestCoroutines()
     }
 
     @Test
-    fun `when feature is stop must not capture thumbnail when a site finish loading`() {
+    fun `do not capture thumbnail when feature is stopped and a site finishes loading`() {
         thumbnails.start()
         thumbnails.stop()
 
-        val session = getSelectedSession()
+        // Initial loading state is false
+        verify(engineView).captureThumbnail(any())
 
-        session.notifyObservers {
-            onLoadingStateChanged(session, false)
-        }
+        store.dispatch(ContentAction.UpdateThumbnailAction(tabId, mock())).joinBlocking()
 
-        verify(mockEngineView, never()).captureThumbnail(any())
+        verifyNoMoreInteractions(engineView)
     }
 
+    @Suppress("UNCHECKED_CAST")
     @Test
-    fun `feature must capture thumbnail when a site finish loading`() {
+    fun `feature must capture thumbnail when a site finishes loading`() {
+        val bitmap: Bitmap? = mock()
+
+        store.dispatch(ContentAction.UpdateLoadingStateAction(tabId, true)).joinBlocking()
+
         thumbnails.start()
 
-        val session = getSelectedSession()
+        `when`(engineView.captureThumbnail(any()))
+            .thenAnswer { // if engineView responds with a bitmap
+                (it.arguments[0] as (Bitmap?) -> Unit).invoke(bitmap)
+            }
+
+        verify(store, never()).dispatch(ContentAction.UpdateThumbnailAction(tabId, bitmap!!))
 
-        session.notifyObservers {
-            onLoadingStateChanged(session, false)
-        }
+        store.dispatch(ContentAction.UpdateLoadingStateAction(tabId, false)).joinBlocking()
 
-        verify(mockEngineView).captureThumbnail(any())
+        verify(store).dispatch(ContentAction.UpdateThumbnailAction(tabId, bitmap))
     }
 
+    @Suppress("UNCHECKED_CAST")
     @Test
-    fun `feature must capture thumbnail when title changes`() {
-        thumbnails.start()
+    fun `feature never updates the store if there is no thumbnail bitmap`() {
+        val store: BrowserStore = mock()
+        val state: BrowserState = mock()
+        val engineView: EngineView = mock()
+        val feature = BrowserThumbnails(testContext, engineView, store)
 
-        val session = getSelectedSession()
+        `when`(store.state).thenReturn(state)
+        `when`(engineView.captureThumbnail(any()))
+            .thenAnswer { // if engineView responds with a bitmap
+                (it.arguments[0] as (Bitmap?) -> Unit).invoke(null)
+            }
 
-        session.notifyObservers {
-            onTitleChanged(session, "test")
-        }
+        feature.requestScreenshot()
 
-        verify(mockEngineView).captureThumbnail(any())
+        verifyZeroInteractions(store)
     }
 
+    @Suppress("UNCHECKED_CAST")
     @Test
-    fun `when a page is loaded and the os is in low memory condition none thumbnail should be captured`() {
-        thumbnails.start()
+    fun `feature never updates the store if there is no tab ID`() {
+        val store: BrowserStore = mock()
+        val state: BrowserState = mock()
+        val engineView: EngineView = mock()
+        val feature = BrowserThumbnails(testContext, engineView, store)
+        val bitmap: Bitmap = mock()
+
+        `when`(store.state).thenReturn(state)
+        `when`(state.selectedTabId).thenReturn(tabId)
+        `when`(engineView.captureThumbnail(any()))
+            .thenAnswer { // if engineView responds with a bitmap
+                (it.arguments[0] as (Bitmap?) -> Unit).invoke(bitmap)
+            }
+
+        feature.requestScreenshot()
+
+        verify(store).dispatch(ContentAction.UpdateThumbnailAction(tabId, bitmap))
+    }
 
-        val session = getSelectedSession()
-        session.thumbnail = mock()
+    @Test
+    fun `when a page is loaded and the os is in low memory condition thumbnail should not be captured`() {
+        store.dispatch(ContentAction.UpdateThumbnailAction(tabId, mock())).joinBlocking()
 
         thumbnails.testLowMemory = true
 
-        session.notifyObservers {
-            onLoadingStateChanged(session, false)
-        }
-
-        verify(mockEngineView, never()).captureThumbnail(any())
-        assertNull(session.thumbnail)
-    }
+        thumbnails.start()
 
-    private fun getSelectedSession(): Session {
-        val session = Session("https://www.mozilla.org")
-        mockSessionManager.add(session)
-        mockSessionManager.select(session)
-        return session
+        verify(engineView, never()).captureThumbnail(any())
     }
 }

components/feature/downloads/src/main/java/mozilla/components/feature/downloads/manager/FetchDownloadManager.kt

@@ -55,10 +55,7 @@ class FetchDownloadManager<T : AbstractFetchDownloadService>(
      * @return the id reference of the scheduled download.
      */
     override fun download(download: DownloadState, cookie: String): Long? {
-        if (!download.isScheme(listOf("http", "https", "data"))) {
-            // We are ignoring everything that is not http(s) or data. This is a limitation of
-            // GeckoView: https://bugzilla.mozilla.org/show_bug.cgi?id=1501735 and
-            // https://bugzilla.mozilla.org/show_bug.cgi?id=1432949
+        if (!download.isScheme(listOf("http", "https", "data", "blob"))) {
             return null
         }
 

components/feature/downloads/src/test/java/mozilla/components/feature/downloads/manager/FetchDownloadManagerTest.kt

@@ -22,15 +22,16 @@ import mozilla.components.support.test.any
 import mozilla.components.support.test.mock
 import mozilla.components.support.test.robolectric.grantPermission
 import mozilla.components.support.test.robolectric.testContext
-import org.junit.Assert.assertEquals
-import org.junit.Assert.assertNull
-import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.Mockito.verify
 import mozilla.components.feature.downloads.AbstractFetchDownloadService.DownloadJobStatus
 import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Assert.assertNull
+import org.junit.Assert.assertNotNull
+import org.junit.Assert.assertEquals
 import org.mockito.Mockito.times
 
 @RunWith(AndroidJUnit4::class)
@@ -136,6 +137,15 @@ class FetchDownloadManagerTest {
         assertNull(id)
     }
 
+    @Test
+    fun `trying to download a file with a blob scheme should trigger a download`() {
+        val validBlobDownload = download.copy(url = "blob:https://ipv4.download.thinkbroadband.com/5MB.zip")
+        grantPermissions()
+
+        val id = downloadManager.download(validBlobDownload)!!
+        assertNotNull(id)
+    }
+
     @Test
     fun `sendBroadcast with valid downloadID must call onDownloadStopped after download`() {
         var downloadCompleted = false

docs/changelog.md

@@ -35,6 +35,9 @@ permalink: /changelog/
 * **browser-toolbar**
   * It will only be animated for vertical scrolls inside the EngineView. Not for horizontal scrolls. Not for zoom gestures.
 
+* **browser-thumbnails**
+  * ⚠️ **This is a breaking change**: Migrated this component to use `browser-state` instead of `browser-session`. It is now required to pass a `BrowserStore` instance (instead of `SessionManager`) to `BrowserThumnails`.
+
 # 40.0.0
 
 * [Commits](https://github.com/mozilla-mobile/android-components/compare/v39.0.0...v40.0.0)
@@ -82,7 +85,11 @@ permalink: /changelog/
 * **feature-syncedtabs**
   * Moved `SyncedTabsFeature` to `SyncedTabsStorage`.
   * ⚠️ **This is a breaking change**: The new `SyncedTabsFeature` now orchestrates the correct state needed for consumers to handle by implementing the `SyncedTabsView`.
-
+  
+* **browser-thumbnails**
+  * 🆕 New component for capturing browser thumbnails.
+  * `ThumbnailsFeature` will be deprecated for the new `BrowserThumbnails` in a future.
+  
 # 39.0.0
 
 * [Commits](https://github.com/mozilla-mobile/android-components/compare/v38.0.0...v39.0.0)

samples/browser/src/main/java/org/mozilla/samples/browser/BrowserFragment.kt

@@ -81,7 +81,7 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
         )
 
         thumbnailsFeature.set(
-            feature = BrowserThumbnails(requireContext(), layout.engineView, components.sessionManager),
+            feature = BrowserThumbnails(requireContext(), layout.engineView, components.store),
             owner = this,
             view = layout
         )