diff --git a/app/src/main/java/org/mozilla/fenix/components/Core.kt b/app/src/main/java/org/mozilla/fenix/components/Core.kt index 87d43aa61..f722b3711 100644 --- a/app/src/main/java/org/mozilla/fenix/components/Core.kt +++ b/app/src/main/java/org/mozilla/fenix/components/Core.kt @@ -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) { diff --git a/app/src/main/java/org/mozilla/fenix/ext/BrowserState.kt b/app/src/main/java/org/mozilla/fenix/ext/BrowserState.kt index 3fe768a4e..a76b802b3 100644 --- a/app/src/main/java/org/mozilla/fenix/ext/BrowserState.kt +++ b/app/src/main/java/org/mozilla/fenix/ext/BrowserState.kt @@ -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 { return mutableListOf().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 } diff --git a/app/src/test/java/org/mozilla/fenix/ext/BrowserStateTest.kt b/app/src/test/java/org/mozilla/fenix/ext/BrowserStateTest.kt index ca85c1946..c453987db 100644 --- a/app/src/test/java/org/mozilla/fenix/ext/BrowserStateTest.kt +++ b/app/src/test/java/org/mozilla/fenix/ext/BrowserStateTest.kt @@ -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 diff --git a/app/src/test/java/org/mozilla/fenix/home/RecentTabsListFeatureTest.kt b/app/src/test/java/org/mozilla/fenix/home/RecentTabsListFeatureTest.kt index 2d552bb56..29054a983 100644 --- a/app/src/test/java/org/mozilla/fenix/home/RecentTabsListFeatureTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/RecentTabsListFeatureTest.kt @@ -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