Commit fb8dbbe4 authored by MozLando's avatar MozLando
Browse files

Merge #6362



6362: Issue #4287: Refactor feature-media to use browser-state instead of browser-session. r=csadilek a=pocmo

This is finally ready. :)

With this I completely removed anything related to media from `browser-session` and moved it to `browser-state`. This required a bigger refactoring of `feature-media`. The state machine is gone and that functionality is now provided by a middleware that can be installed on `BrowserStore`.

Note that this also introduces a `Dispatchers` object in `support-base` for creating AC and app specific dispatchers.
Co-authored-by: default avatarSebastian Kaspari <s.kaspari@gmail.com>
parents 12eb86c8 40c0a352
......@@ -40,6 +40,7 @@ dependencies {
api project(':support-base')
testImplementation project(':support-test')
testImplementation project(':support-test-libstate')
testImplementation Dependencies.androidx_test_core
testImplementation Dependencies.androidx_test_junit
......
......@@ -37,7 +37,6 @@ import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.engine.HitResult
import mozilla.components.concept.engine.content.blocking.Tracker
import mozilla.components.concept.engine.manifest.WebAppManifest
import mozilla.components.concept.engine.media.Media
import mozilla.components.concept.engine.media.RecordingDevice
import mozilla.components.concept.engine.permission.PermissionRequest
import mozilla.components.support.base.observer.Consumable
......@@ -102,8 +101,6 @@ class Session(
fun onThumbnailChanged(session: Session, bitmap: Bitmap?) = Unit
fun onContentPermissionRequested(session: Session, permissionRequest: PermissionRequest): Boolean = false
fun onAppPermissionRequested(session: Session, permissionRequest: PermissionRequest): Boolean = false
fun onMediaRemoved(session: Session, media: List<Media>, removed: Media) = Unit
fun onMediaAdded(session: Session, media: List<Media>, added: Media) = Unit
fun onCrashStateChanged(session: Session, crashed: Boolean) = Unit
fun onReaderableStateUpdated(session: Session, readerable: Boolean) = Unit
fun onReaderModeChanged(session: Session, enabled: Boolean) = Unit
......@@ -304,25 +301,6 @@ class Session(
notifyObservers { onWebAppManifestChanged(this@Session, new) }
}
/**
* List of [Media] on the currently visited page.
*/
var media: List<Media> by Delegates.observable(emptyList()) { _, old, new ->
if (old.size > new.size) {
val removed = old - new
require(removed.size == 1) { "Expected only one item to be removed, but was ${removed.size}" }
notifyObservers {
onMediaRemoved(this@Session, new, removed[0])
}
} else if (new.size > old.size) {
val added = new - old
require(added.size == 1) { "Expected only one item to be added, but was ${added.size}" }
notifyObservers {
onMediaAdded(this@Session, new, added[0])
}
}
}
/**
* Tracker blocking state, true if blocking trackers is enabled, otherwise false.
*/
......
......@@ -14,7 +14,9 @@ import mozilla.components.browser.session.engine.request.LaunchIntentMetadata
import mozilla.components.browser.session.engine.request.LoadRequestMetadata
import mozilla.components.browser.session.engine.request.LoadRequestOption
import mozilla.components.browser.session.ext.syncDispatch
import mozilla.components.browser.session.ext.toElement
import mozilla.components.browser.state.action.ContentAction
import mozilla.components.browser.state.action.MediaAction
import mozilla.components.browser.state.action.TrackingProtectionAction
import mozilla.components.browser.state.state.content.DownloadState
import mozilla.components.browser.state.store.BrowserStore
......@@ -38,6 +40,7 @@ internal class EngineObserver(
private val session: Session,
private val store: BrowserStore? = null
) : EngineSession.Observer {
private val mediaMap: MutableMap<Media, MediaObserver> = mutableMapOf()
override fun onNavigateBack() {
session.searchTerms = ""
......@@ -238,16 +241,29 @@ internal class EngineObserver(
}
override fun onMediaAdded(media: Media) {
session.media = session.media.toMutableList().also {
it.add(media)
}
val store = store ?: return
val mediaElement = media.toElement()
store.dispatch(MediaAction.AddMediaAction(session.id, mediaElement))
val observer = MediaObserver(media, mediaElement, store, session.id)
media.register(observer)
mediaMap[media] = observer
}
override fun onMediaRemoved(media: Media) {
session.media = session.media.toMutableList().also {
it.remove(media)
val store = store ?: return
val observer = mediaMap[media]
if (observer != null) {
media.unregister(observer)
store.dispatch(MediaAction.RemoveMediaAction(session.id, observer.element))
}
media.unregisterObservers()
mediaMap.remove(media)
}
override fun onWebAppManifestLoaded(manifest: WebAppManifest) {
......
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
package mozilla.components.browser.session.engine
import mozilla.components.browser.state.action.MediaAction
import mozilla.components.browser.state.state.MediaState
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.engine.media.Media
/**
* [Media.Observer] implementation responsible for dispatching actions updating the state in
* [BrowserStore].
*/
internal class MediaObserver(
val media: Media,
val element: MediaState.Element,
val store: BrowserStore,
val tabId: String
) : Media.Observer {
override fun onStateChanged(media: Media, state: Media.State) {
store.dispatch(MediaAction.UpdateMediaStateAction(
tabId,
element.id,
state
))
}
override fun onPlaybackStateChanged(media: Media, playbackState: Media.PlaybackState) {
store.dispatch(MediaAction.UpdateMediaPlaybackStateAction(
tabId,
element.id,
playbackState
))
}
override fun onMetadataChanged(media: Media, metadata: Media.Metadata) {
store.dispatch(MediaAction.UpdateMediaMetadataAction(
tabId,
element.id,
metadata
))
}
}
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
package mozilla.components.browser.session.ext
import mozilla.components.browser.state.state.MediaState
import mozilla.components.concept.engine.media.Media
internal fun Media.toElement() = MediaState.Element(
state = state,
playbackState = playbackState,
controller = controller,
metadata = metadata
)
......@@ -26,7 +26,6 @@ import mozilla.components.concept.engine.HitResult
import mozilla.components.concept.engine.content.blocking.Tracker
import mozilla.components.concept.engine.manifest.Size
import mozilla.components.concept.engine.manifest.WebAppManifest
import mozilla.components.concept.engine.media.Media
import mozilla.components.concept.engine.media.RecordingDevice
import mozilla.components.concept.engine.permission.PermissionRequest
import mozilla.components.support.base.observer.Consumable
......@@ -721,8 +720,6 @@ class SessionTest {
defaultObserver.onContentPermissionRequested(session, contentPermissionRequest)
defaultObserver.onAppPermissionRequested(session, appPermissionRequest)
defaultObserver.onWebAppManifestChanged(session, mock())
defaultObserver.onMediaAdded(session, emptyList(), mock())
defaultObserver.onMediaRemoved(session, emptyList(), mock())
defaultObserver.onReaderableStateUpdated(session, true)
defaultObserver.onRecordingDevicesChanged(session, emptyList())
}
......@@ -842,99 +839,6 @@ class SessionTest {
assertEquals("Session(my-session-id, https://www.mozilla.org)", session.toString())
}
@Test
fun `observer is notified when media is added`() {
var observedList: List<Media>? = null
var observedMedia: Media? = null
val observer = object : Session.Observer {
override fun onMediaAdded(session: Session, media: List<Media>, added: Media) {
observedList = media
observedMedia = added
}
}
val session = Session("https://www.mozilla.org")
session.register(observer)
val addedMedia1: Media = mock()
session.media = listOf(addedMedia1)
assertEquals(addedMedia1, observedMedia)
assertEquals(listOf(addedMedia1), observedList)
val addedMedia2: Media = mock()
session.media = listOf(addedMedia1, addedMedia2)
assertEquals(addedMedia2, observedMedia)
assertEquals(listOf(addedMedia1, addedMedia2), observedList)
val addedMedia3: Media = mock()
session.media = listOf(addedMedia1, addedMedia3, addedMedia2)
assertEquals(addedMedia3, observedMedia)
assertEquals(listOf(addedMedia1, addedMedia3, addedMedia2), observedList)
}
@Test
fun `observer is notified when media is removed`() {
var observedList: List<Media>? = null
var observedMedia: Media? = null
val observer = object : Session.Observer {
override fun onMediaRemoved(session: Session, media: List<Media>, removed: Media) {
observedList = media
observedMedia = removed
}
}
val media1: Media = mock()
val media2: Media = mock()
val media3: Media = mock()
val session = Session("https://www.mozilla.org")
session.media = listOf(media1)
session.media = listOf(media1, media2)
session.media = listOf(media1, media2, media3)
session.register(observer)
session.media = listOf(media1, media2)
assertEquals(media3, observedMedia)
assertEquals(listOf(media1, media2), observedList)
session.media = listOf(media2)
assertEquals(media1, observedMedia)
assertEquals(listOf(media2), observedList)
session.media = listOf()
assertEquals(media2, observedMedia)
assertEquals(emptyList<Media>(), observedList)
}
@Test(expected = IllegalArgumentException::class)
fun `Session throws if more than one Media object is added at a time`() {
val session = Session("https://www.mozilla.org")
session.media = listOf(mock(), mock())
}
@Test(expected = IllegalArgumentException::class)
fun `Session throws if more than one Media object is removed at a time`() {
val session = Session("https://www.mozilla.org")
val media1: Media = mock()
val media2: Media = mock()
val media3: Media = mock()
session.media = listOf(media1)
session.media = listOf(media1, media2)
session.media = listOf(media1, media2, media3)
session.media = listOf(media1)
}
@Test
fun `WHEN crashed state changes THEN observers are notified`() {
val session = Session("https://www.mozilla.org")
......
......@@ -7,6 +7,7 @@ package mozilla.components.browser.session.engine
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.browser.session.engine.request.LoadRequestOption
import mozilla.components.browser.state.action.ContentAction
import mozilla.components.browser.state.action.TrackingProtectionAction
......@@ -23,6 +24,7 @@ import mozilla.components.concept.engine.prompt.PromptRequest
import mozilla.components.concept.engine.window.WindowRequest
import mozilla.components.support.base.observer.Consumable
import mozilla.components.support.test.any
import mozilla.components.support.test.libstate.ext.waitUntilIdle
import mozilla.components.support.test.mock
import mozilla.components.support.test.whenever
import org.junit.Assert.assertEquals
......@@ -32,6 +34,7 @@ import org.junit.Assert.assertTrue
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.Mockito.mock
import org.mockito.Mockito.never
import org.mockito.Mockito.spy
import org.mockito.Mockito.verify
......@@ -479,50 +482,92 @@ class EngineObserverTest {
}
@Test
fun `onMediaAdded will add media to session`() {
val session = Session("https://www.mozilla.org")
val observer = EngineObserver(session)
fun `onMediaAdded will subscribe to media and add it to store`() {
val store = BrowserStore()
val sessionManager = SessionManager(engine = mock(), store = store)
val session = Session("https://www.mozilla.org", id = "test-tab").also {
sessionManager.add(it)
}
val observer = EngineObserver(session, store)
assertEquals(0, store.state.media.elements.size)
val media1: Media = mock()
val media1: Media = spy(object : Media() {
override val controller: Controller = mock()
override val metadata: Metadata = mock()
})
observer.onMediaAdded(media1)
assertEquals(listOf(media1), session.media)
store.waitUntilIdle()
verify(media1).register(any())
assertEquals(1, store.state.media.elements["test-tab"]?.size)
val media2: Media = mock()
val media2: Media = spy(object : Media() {
override val controller: Controller = mock()
override val metadata: Metadata = mock()
})
observer.onMediaAdded(media2)
assertEquals(listOf(media1, media2), session.media)
store.waitUntilIdle()
verify(media2).register(any())
assertEquals(2, store.state.media.elements["test-tab"]?.size)
val media3: Media = mock()
val media3: Media = spy(object : Media() {
override val controller: Controller = mock()
override val metadata: Metadata = mock()
})
observer.onMediaAdded(media3)
assertEquals(listOf(media1, media2, media3), session.media)
store.waitUntilIdle()
verify(media3).register(any())
assertEquals(3, store.state.media.elements["test-tab"]?.size)
}
@Test
fun `onMediaRemoved will remove media from session`() {
val session = Session("https://www.mozilla.org")
val observer = EngineObserver(session)
fun `onMediaRemoved will unsubscribe and remove it from store`() {
val store = BrowserStore()
val media1: Media = mock()
val media2: Media = mock()
val media3: Media = mock()
val sessionManager = SessionManager(engine = mock(), store = store)
session.media = listOf(media1)
session.media = listOf(media1, media2)
session.media = listOf(media1, media2, media3)
val session = Session("https://www.mozilla.org", id = "test-tab").also {
sessionManager.add(it)
}
val observer = EngineObserver(session, store)
observer.onMediaRemoved(media2)
val media1: Media = spy(object : Media() {
override val controller: Controller = mock()
override val metadata: Metadata = mock()
})
observer.onMediaAdded(media1)
assertEquals(listOf(media1, media3), session.media)
val media2: Media = spy(object : Media() {
override val controller: Controller = mock()
override val metadata: Metadata = mock()
})
observer.onMediaAdded(media2)
store.waitUntilIdle()
assertEquals(2, store.state.media.elements["test-tab"]?.size)
verify(media1, never()).unregister(any())
verify(media2, never()).unregister(any())
observer.onMediaRemoved(media1)
store.waitUntilIdle()
assertEquals(listOf(media3), session.media)
assertEquals(1, store.state.media.elements["test-tab"]?.size)
verify(media1).unregister(any())
verify(media2, never()).unregister(any())
observer.onMediaRemoved(media3)
observer.onMediaRemoved(media2)
store.waitUntilIdle()
assertEquals(emptyList<Media>(), session.media)
assertNull(store.state.media.elements["test-tab"])
verify(media2).unregister(any())
}
@Test
......
......@@ -17,10 +17,12 @@ import mozilla.components.browser.state.state.TrackingProtectionState
import mozilla.components.browser.state.state.WebExtensionState
import mozilla.components.browser.state.state.content.DownloadState
import mozilla.components.browser.state.state.content.FindResultState
import mozilla.components.browser.state.state.MediaState
import mozilla.components.concept.engine.EngineSession
import mozilla.components.concept.engine.EngineSessionState
import mozilla.components.concept.engine.HitResult
import mozilla.components.concept.engine.content.blocking.Tracker
import mozilla.components.concept.engine.media.Media
import mozilla.components.concept.engine.prompt.PromptRequest
import mozilla.components.concept.engine.search.SearchRequest
import mozilla.components.concept.engine.webextension.WebExtensionBrowserAction
......@@ -396,3 +398,60 @@ sealed class ReaderAction : BrowserAction() {
*/
data class UpdateReaderActiveAction(val tabId: String, val active: Boolean) : ReaderAction()
}
/**
* [BrowserAction] implementations related to updating the [MediaState].
*/
sealed class MediaAction : BrowserAction() {
/**
* Adds [media] to the list of [MediaState.Element] for the tab with id [tabId].
*/
data class AddMediaAction(val tabId: String, val media: MediaState.Element) : MediaAction()
/**
* Removes [media] from the list of [MediaState.Element] for the tab with id [tabId].
*/
data class RemoveMediaAction(val tabId: String, val media: MediaState.Element) : MediaAction()
/**
* Removes all [MediaState.Element] for tabs with ids in [tabIds].
*/
data class RemoveTabMediaAction(val tabIds: List<String>) : MediaAction()
/**
* Updates the [Media.State] for the [MediaState.Element] with id [mediaId] owned by the tab
* with id [tabId].
*/
data class UpdateMediaStateAction(
val tabId: String,
val mediaId: String,
val state: Media.State
) : MediaAction()
/**
* Updates the [Media.PlaybackState] for the [MediaState.Element] with id [mediaId] owned by the
* tab with id [tabId].
*/
data class UpdateMediaPlaybackStateAction(
val tabId: String,
val mediaId: String,
val playbackState: Media.PlaybackState
) : MediaAction()
/**
* Updates the [Media.Metadata] for the [MediaState.Element] with id [mediaId] owned by the tab
* with id [tabId].
*/
data class UpdateMediaMetadataAction(
val tabId: String,
val mediaId: String,
val metadata: Media.Metadata
) : MediaAction()
/**
* Updates [MediaState.Aggregate] in the [MediaState].
*/
data class UpdateMediaAggregateAction(
val aggregate: MediaState.Aggregate
) : MediaAction()
}
......@@ -8,6 +8,7 @@ import mozilla.components.browser.state.action.BrowserAction
import mozilla.components.browser.state.action.ContentAction
import mozilla.components.browser.state.action.CustomTabListAction
import mozilla.components.browser.state.action.EngineAction
import mozilla.components.browser.state.action.MediaAction
import mozilla.components.browser.state.action.ReaderAction
import mozilla.components.browser.state.action.SystemAction
import mozilla.components.browser.state.action.TabListAction
......@@ -34,6 +35,7 @@ internal object BrowserStateReducer {
is TabListAction -> TabListReducer.reduce(state, action)
is TrackingProtectionAction -> TrackingProtectionStateReducer.reduce(state, action)
is WebExtensionAction -> WebExtensionReducer.reduce(state, action)
is MediaAction -> MediaReducer.reduce(state, action)
}
}
}
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
package mozilla.components.browser.state.reducer
import mozilla.components.browser.state.action.MediaAction
import mozilla.components.browser.state.state.BrowserState
import mozilla.components.browser.state.state.MediaState
import mozilla.components.browser.state.state.SessionState
internal object MediaReducer {
/**
* [MediaAction] Reducer function for modifying a specific [MediaState] of a [SessionState].
*/
fun reduce(state: BrowserState, action: MediaAction): BrowserState {
return when (action) {
is MediaAction.AddMediaAction -> state.updateMediaList(action.tabId) { list ->
list + action.media
}
is MediaAction.RemoveMediaAction -> state.updateMediaList(action.tabId) { list ->
list.filter { element -> element.id != action.media.id }
}
is MediaAction.RemoveTabMediaAction -> {
val tabIdSet = HashSet<String>().apply { addAll(action.tabIds) }
state.copy(
media = state.media.copy(
elements = state.media.elements.filterKeys { it !in tabIdSet }
)
)
}
is MediaAction.UpdateMediaStateAction -> state.updateMediaElement(action.tabId, action.mediaId) {
it.copy(state = action.state)
}
is MediaAction.UpdateMediaPlaybackStateAction -> state.updateMediaElement(action.tabId, action.mediaId) {