For #20402 - Re-enable "in progress media tab"

The crash for when media starts playing in a custom tab is now resolved in AC.
upstream-sync
Mugurell 3 years ago committed by mergify[bot]
parent 973070ab50
commit 2c8c6d29ea

@ -32,6 +32,7 @@ import mozilla.components.feature.customtabs.store.CustomTabsServiceStore
import mozilla.components.feature.downloads.DownloadMiddleware
import mozilla.components.feature.logins.exceptions.LoginExceptionStorage
import mozilla.components.feature.media.MediaSessionFeature
import mozilla.components.feature.media.middleware.LastMediaAccessMiddleware
import mozilla.components.feature.media.middleware.RecordingDevicesMiddleware
import mozilla.components.feature.prompts.PromptMiddleware
import mozilla.components.feature.pwa.ManifestStorage
@ -208,8 +209,8 @@ class Core(
),
RecordingDevicesMiddleware(context),
PromptMiddleware(),
AdsTelemetryMiddleware(adsTelemetry)
// LastMediaAccessMiddleware() // disabled to avoid a nightly crash in #20402
AdsTelemetryMiddleware(adsTelemetry),
LastMediaAccessMiddleware()
)
if (FeatureFlags.historyMetadataFeature) {

@ -8,6 +8,7 @@ import mozilla.components.browser.state.selector.normalTabs
import mozilla.components.browser.state.selector.selectedNormalTab
import mozilla.components.browser.state.state.BrowserState
import mozilla.components.browser.state.state.TabSessionState
import mozilla.components.feature.tabs.ext.hasMediaPlayed
/**
* Get the last opened normal tab and the last tab with in progress media, if available.
@ -19,12 +20,11 @@ fun BrowserState.asRecentTabs(): List<TabSessionState> {
return mutableListOf<TabSessionState>().apply {
val lastOpenedNormalTab = lastOpenedNormalTab
lastOpenedNormalTab?.let { add(it) }
// disabled to avoid a nightly crash in #20402
// inProgressMediaTab
// ?.takeUnless { it == lastOpenedNormalTab }
// ?.let {
// add(it)
// }
inProgressMediaTab
?.takeUnless { it == lastOpenedNormalTab }
?.let {
add(it)
}
}
}
@ -40,5 +40,5 @@ val BrowserState.lastOpenedNormalTab: TabSessionState?
*/
val BrowserState.inProgressMediaTab: TabSessionState?
get() = normalTabs
.filter { it.lastMediaAccess > 0 }
.maxByOrNull { it.lastMediaAccess }
.filter { it.hasMediaPlayed() }
.maxByOrNull { it.lastMediaAccessState.lastMediaAccess }

@ -6,17 +6,20 @@ package org.mozilla.fenix.ext
import io.mockk.mockk
import mozilla.components.browser.state.state.BrowserState
import mozilla.components.browser.state.state.LastMediaAccessState
import mozilla.components.browser.state.state.createTab
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNull
import org.junit.Ignore
import org.junit.Test
class BrowserStateTest {
@Test
fun `GIVEN a tab which had media playing WHEN inProgressMediaTab is called THEN return that tab`() {
val inProgressMediaTab = createTab(url = "mediaUrl", id = "2", lastMediaAccess = 123)
val inProgressMediaTab = createTab(
url = "mediaUrl", id = "2",
lastMediaAccessState = LastMediaAccessState("https://mozilla.com", 123, true)
)
val browserState = BrowserState(
tabs = listOf(mockk(relaxed = true), inProgressMediaTab, mockk(relaxed = true))
)
@ -27,17 +30,17 @@ class BrowserStateTest {
@Test
fun `GIVEN no tab which had media playing exists WHEN inProgressMediaTab is called THEN return null`() {
val browserState = BrowserState(
tabs = listOf(mockk(relaxed = true), mockk(relaxed = true), mockk(relaxed = true))
tabs = listOf(createTab("tab1"), createTab("tab2"), createTab("tab3"))
)
assertNull(browserState.inProgressMediaTab)
}
@Test
fun `GIVEN the selected tab is a normal tab and no media tab WHEN asRecentTabs is called THEN return a list of that tab`() {
fun `GIVEN the selected tab is a normal tab and no media tab exists WHEN asRecentTabs is called THEN return a list of that tab`() {
val selectedTab = createTab(url = "url", id = "3")
val browserState = BrowserState(
tabs = listOf(mockk(relaxed = true), selectedTab, mockk(relaxed = true)),
tabs = listOf(createTab("tab1"), selectedTab, createTab("tab3")),
selectedTabId = selectedTab.id
)
@ -48,11 +51,15 @@ class BrowserStateTest {
}
@Test
fun `GIVEN the selected tab is a private tab and no media tab WHEN asRecentTabs is called THEN return a list of the last accessed normal tab`() {
fun `GIVEN the selected tab is a private tab and no media tab exists WHEN asRecentTabs is called THEN return a list of the last accessed normal tab`() {
val selectedPrivateTab = createTab(url = "url", id = "1", lastAccess = 1, private = true)
val lastAccessedNormalTab = createTab(url = "url2", id = "2", lastAccess = 2)
val browserState = BrowserState(
tabs = listOf(mockk(relaxed = true), lastAccessedNormalTab, selectedPrivateTab),
tabs = listOf(
createTab("https://mozilla.org"),
lastAccessedNormalTab,
selectedPrivateTab
),
selectedTabId = selectedPrivateTab.id
)
@ -62,11 +69,13 @@ class BrowserStateTest {
assertEquals(lastAccessedNormalTab, result[0])
}
@Ignore("Temporarily disabled. See #20402.")
@Test
fun `GIVEN the selected tab is a normal tab and another media tab exists WHEN asRecentTabs is called THEN return a list of these tabs`() {
val selectedTab = createTab(url = "url", id = "3")
val mediaTab = createTab("mediaUrl", id = "23", lastMediaAccess = 123)
val mediaTab = createTab(
"mediaUrl", id = "23",
lastMediaAccessState = LastMediaAccessState("https://mozilla.com", 123, true)
)
val browserState = BrowserState(
tabs = listOf(mockk(relaxed = true), selectedTab, mediaTab),
selectedTabId = selectedTab.id
@ -79,14 +88,21 @@ class BrowserStateTest {
assertEquals(mediaTab, result[1])
}
@Ignore("Temporarily disabled. See #20402.")
@Test
fun `GIVEN the selected tab is a private tab and another media tab exists WHEN asRecentTabs is called THEN return a list of the last normal tab and the media tab`() {
val lastAccessedNormalTab = createTab(url = "url2", id = "2", lastAccess = 2)
val selectedPrivateTab = createTab(url = "url", id = "1", lastAccess = 1, private = true)
val mediaTab = createTab("mediaUrl", id = "12", lastAccess = 0, lastMediaAccess = 123)
val mediaTab = createTab(
"mediaUrl", id = "12", lastAccess = 0,
lastMediaAccessState = LastMediaAccessState("https://mozilla.com", 123, true)
)
val browserState = BrowserState(
tabs = listOf(mockk(relaxed = true), lastAccessedNormalTab, selectedPrivateTab, mediaTab),
tabs = listOf(
mockk(relaxed = true),
lastAccessedNormalTab,
selectedPrivateTab,
mediaTab
),
selectedTabId = selectedPrivateTab.id
)
@ -101,7 +117,10 @@ class BrowserStateTest {
fun `GIVEN the selected tab is a private tab and the media tab is the last accessed normal tab WHEN asRecentTabs is called THEN a list of the media tab`() {
val selectedPrivateTab = createTab(url = "url", id = "1", lastAccess = 1, private = true)
val normalTab = createTab(url = "url2", id = "2", lastAccess = 2)
val mediaTab = createTab("mediaUrl", id = "12", lastAccess = 20, lastMediaAccess = 123)
val mediaTab = createTab(
"mediaUrl", id = "12", lastAccess = 20,
lastMediaAccessState = LastMediaAccessState("https://mozilla.com", 123, true)
)
val browserState = BrowserState(
tabs = listOf(mockk(relaxed = true), normalTab, selectedPrivateTab, mediaTab),
selectedTabId = selectedPrivateTab.id

@ -12,6 +12,7 @@ import mozilla.components.browser.state.action.ContentAction.UpdateTitleAction
import mozilla.components.browser.state.action.MediaSessionAction
import mozilla.components.browser.state.action.TabListAction
import mozilla.components.browser.state.state.BrowserState
import mozilla.components.browser.state.state.LastMediaAccessState
import mozilla.components.browser.state.state.createTab
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.engine.mediasession.MediaSession
@ -26,7 +27,6 @@ import org.junit.Assert.assertNotNull
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Ignore
import org.junit.Rule
import org.junit.Test
import org.mozilla.fenix.home.HomeFragmentAction.RecentTabsChange
@ -115,10 +115,12 @@ class RecentTabsListFeatureTest {
assertEquals(1, homeStore.state.recentTabs.size)
}
@Ignore("Temporarily disabled. See #20402.")
@Test
fun `GIVEN a valid inProgressMediaTabId and another selected tab exists WHEN the feature starts THEN dispatch both as as a recent tabs list`() {
val mediaTab = createTab("https://mozilla.com", id = "42", lastMediaAccess = 123)
val mediaTab = createTab(
url = "https://mozilla.com", id = "42",
lastMediaAccessState = LastMediaAccessState("https://mozilla.com", 123)
)
val selectedTab = createTab("https://mozilla.com", id = "43")
val browserStore = BrowserStore(BrowserState(
tabs = listOf(mediaTab, selectedTab),
@ -139,7 +141,10 @@ class RecentTabsListFeatureTest {
@Test
fun `GIVEN a valid inProgressMediaTabId exists and that is the selected tab WHEN the feature starts THEN dispatch just one tab as the recent tabs list`() {
val selectedMediaTab = createTab("https://mozilla.com", id = "42", lastMediaAccess = 123)
val selectedMediaTab = createTab(
"https://mozilla.com", id = "42",
lastMediaAccessState = LastMediaAccessState("https://mozilla.com", 123)
)
val browserStore = BrowserStore(BrowserState(
tabs = listOf(selectedMediaTab),
selectedTabId = "42"
@ -193,11 +198,16 @@ class RecentTabsListFeatureTest {
assertEquals(tab2, homeStore.state.recentTabs[0])
}
@Ignore("Temporarily disabled. See #20402.")
@Test
fun `WHEN the browser state has an in progress media tab THEN dispatch the new recent tab list`() {
val initialMediaTab = createTab(url = "https://mozilla.com", id = "1", lastMediaAccess = 123)
val newMediaTab = createTab(url = "http://mozilla.org", id = "2", lastMediaAccess = 100)
val initialMediaTab = createTab(
url = "https://mozilla.com", id = "1",
lastMediaAccessState = LastMediaAccessState("https://mozilla.com", 123)
)
val newMediaTab = createTab(
url = "http://mozilla.org", id = "2",
lastMediaAccessState = LastMediaAccessState("https://mozilla.com", 100)
)
val browserStore = BrowserStore(
initialState = BrowserState(
tabs = listOf(initialMediaTab, newMediaTab),
@ -222,10 +232,15 @@ class RecentTabsListFeatureTest {
assertEquals(2, homeStore.state.recentTabs.size)
assertEquals(initialMediaTab, homeStore.state.recentTabs[0])
// UpdateMediaPlaybackStateAction would set the current timestamp as the new value for lastMediaAccess
val updatedLastMediaAccess = homeStore.state.recentTabs[1].lastMediaAccess
val updatedLastMediaAccess = homeStore.state.recentTabs[1].lastMediaAccessState.lastMediaAccess
assertTrue("expected lastMediaAccess ($updatedLastMediaAccess) > 100", updatedLastMediaAccess > 100)
assertEquals("http://mozilla.org", homeStore.state.recentTabs[1].lastMediaAccessState.lastMediaUrl)
// Check that the media tab is updated ignoring just the lastMediaAccess property.
assertEquals(newMediaTab, homeStore.state.recentTabs[1].copy(lastMediaAccess = 100))
assertEquals(
newMediaTab, homeStore.state.recentTabs[1].copy(
lastMediaAccessState = LastMediaAccessState("https://mozilla.com", 100)
)
)
}
@Test

Loading…
Cancel
Save