Commit 84d7a0f7 authored by MozLando's avatar MozLando
Browse files

Merge #7740



7740: Remove more properties from Session. r=csadilek a=pocmo

Removing a bunch of properties where our code is already reading them exclusively from `BrowserState`. The only change needed in Fenix is to no longer pass `SessionManager` to `ContextMenuUseCases`.
Co-authored-by: default avatarSebastian Kaspari <s.kaspari@gmail.com>
parents dcceed91 2b140bea
......@@ -11,18 +11,12 @@ 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.toFindResultState
import mozilla.components.browser.session.ext.toSecurityInfoState
import mozilla.components.browser.session.ext.toTabSessionState
import mozilla.components.browser.state.action.ContentAction.AddFindResultAction
import mozilla.components.browser.state.action.ContentAction.ClearFindResultsAction
import mozilla.components.browser.state.action.ContentAction.ConsumeHitResultAction
import mozilla.components.browser.state.action.ContentAction.FullScreenChangedAction
import mozilla.components.browser.state.action.ContentAction.RemoveThumbnailAction
import mozilla.components.browser.state.action.ContentAction.RemoveWebAppManifestAction
import mozilla.components.browser.state.action.ContentAction.UpdateBackNavigationStateAction
import mozilla.components.browser.state.action.ContentAction.UpdateForwardNavigationStateAction
import mozilla.components.browser.state.action.ContentAction.UpdateHitResultAction
import mozilla.components.browser.state.action.ContentAction.UpdateLoadingStateAction
import mozilla.components.browser.state.action.ContentAction.UpdateProgressAction
import mozilla.components.browser.state.action.ContentAction.UpdateSearchTermsAction
......@@ -31,7 +25,6 @@ import mozilla.components.browser.state.action.ContentAction.UpdateThumbnailActi
import mozilla.components.browser.state.action.ContentAction.UpdateTitleAction
import mozilla.components.browser.state.action.ContentAction.UpdateUrlAction
import mozilla.components.browser.state.action.ContentAction.UpdateWebAppManifestAction
import mozilla.components.browser.state.action.ContentAction.ViewportFitChangedAction
import mozilla.components.browser.state.action.CustomTabListAction.RemoveCustomTabAction
import mozilla.components.browser.state.action.EngineAction
import mozilla.components.browser.state.action.TabListAction.AddTabAction
......@@ -39,7 +32,6 @@ import mozilla.components.browser.state.action.TrackingProtectionAction
import mozilla.components.browser.state.selector.findTabOrCustomTab
import mozilla.components.browser.state.state.CustomTabConfig
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.RecordingDevice
......@@ -100,14 +92,7 @@ class Session(
fun onTrackerBlockingEnabledChanged(session: Session, blockingEnabled: Boolean) = Unit
fun onTrackerBlocked(session: Session, tracker: Tracker, all: List<Tracker>) = Unit
fun onTrackerLoaded(session: Session, tracker: Tracker, all: List<Tracker>) = Unit
fun onLongPress(session: Session, hitResult: HitResult): Boolean = false
fun onFindResult(session: Session, result: FindResult) = Unit
fun onDesktopModeChanged(session: Session, enabled: Boolean) = Unit
fun onFullScreenChanged(session: Session, enabled: Boolean) = Unit
/**
* @param layoutInDisplayCutoutMode value of defined in https://developer.android.com/reference/android/view/WindowManager.LayoutParams#layoutInDisplayCutoutMode
*/
fun onMetaViewportFitChanged(session: Session, layoutInDisplayCutoutMode: Int) = Unit
fun onThumbnailChanged(session: Session, bitmap: Bitmap?) = Unit
fun onContentPermissionRequested(session: Session, permissionRequest: PermissionRequest): Boolean = false
fun onAppPermissionRequested(session: Session, permissionRequest: PermissionRequest): Boolean = false
......@@ -186,15 +171,6 @@ class Session(
RESTORED
}
/**
* A value type representing a result of a "find in page" operation.
*
* @property activeMatchOrdinal the zero-based ordinal of the currently selected match.
* @property numberOfMatches the match count
* @property isDoneCounting true if the find operation has completed, otherwise false.
*/
data class FindResult(val activeMatchOrdinal: Int, val numberOfMatches: Int, val isDoneCounting: Boolean)
/**
* The currently loading or loaded URL.
*/
......@@ -376,41 +352,6 @@ class Session(
}
}
/**
* List of results of that latest "find in page" operation.
*/
var findResults: List<FindResult> by Delegates.observable(emptyList()) { _, old, new ->
notifyObservers(old, new) {
if (new.isNotEmpty()) {
onFindResult(this@Session, new.last())
}
}
if (new.isNotEmpty()) {
store?.syncDispatch(AddFindResultAction(id, new.last().toFindResultState()))
} else {
store?.syncDispatch(ClearFindResultsAction(id))
}
}
/**
* The target of the latest long click operation.
*/
var hitResult: Consumable<HitResult> by Delegates.vetoable(Consumable.empty()) { _, _, result ->
store?.let {
val hitResult = result.peek()
if (hitResult == null) {
it.syncDispatch(ConsumeHitResultAction(id))
} else {
it.syncDispatch(UpdateHitResultAction(id, hitResult))
result.onConsume { it.syncDispatch(ConsumeHitResultAction(id)) }
}
}
val consumers = wrapConsumers<HitResult> { onLongPress(this@Session, it) }
!result.consumeBy(consumers)
}
/**
* The target of the latest thumbnail.
*/
......@@ -428,23 +369,6 @@ class Session(
notifyObservers(old, new) { onDesktopModeChanged(this@Session, new) }
}
/**
* Exits fullscreen mode if it's in that state.
*/
var fullScreenMode: Boolean by Delegates.observable(false) { _, old, new ->
if (notifyObservers(old, new) { onFullScreenChanged(this@Session, new) }) {
store?.syncDispatch(FullScreenChangedAction(id, new))
}
}
/**
* Display cutout mode state.
*/
var layoutInDisplayCutoutMode: Int by Delegates.observable(0) { _, _, new ->
notifyObservers { onMetaViewportFitChanged(this@Session, new) }
store?.syncDispatch(ViewportFitChangedAction(id, new))
}
/**
* An icon for the currently visible page.
*/
......
......@@ -19,6 +19,7 @@ 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.state.content.FindResultState
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.engine.EngineSession
import mozilla.components.concept.engine.HitResult
......@@ -132,7 +133,8 @@ internal class EngineObserver(
override fun onLoadingStateChange(loading: Boolean) {
session.loading = loading
if (loading) {
session.findResults = emptyList()
store?.dispatch(ContentAction.ClearFindResultsAction(session.id))
session.trackersBlocked = emptyList()
session.trackersLoaded = emptyList()
}
......@@ -165,15 +167,24 @@ internal class EngineObserver(
}
override fun onLongPress(hitResult: HitResult) {
session.hitResult = Consumable.from(hitResult)
store?.dispatch(
ContentAction.UpdateHitResultAction(session.id, hitResult)
)
}
override fun onFind(text: String) {
session.findResults = emptyList()
store?.dispatch(ContentAction.ClearFindResultsAction(session.id))
}
override fun onFindResult(activeMatchOrdinal: Int, numberOfMatches: Int, isDoneCounting: Boolean) {
session.findResults += Session.FindResult(activeMatchOrdinal, numberOfMatches, isDoneCounting)
store?.dispatch(ContentAction.AddFindResultAction(
session.id,
FindResultState(
activeMatchOrdinal,
numberOfMatches,
isDoneCounting
)
))
}
override fun onExternalResource(
......@@ -207,11 +218,15 @@ internal class EngineObserver(
}
override fun onFullScreenChange(enabled: Boolean) {
session.fullScreenMode = enabled
store?.dispatch(ContentAction.FullScreenChangedAction(
session.id, enabled
))
}
override fun onMetaViewportFitChanged(layoutInDisplayCutoutMode: Int) {
session.layoutInDisplayCutoutMode = layoutInDisplayCutoutMode
store?.dispatch(ContentAction.ViewportFitChangedAction(
session.id, layoutInDisplayCutoutMode
))
}
override fun onThumbnailChange(bitmap: Bitmap?) {
......
......@@ -10,7 +10,6 @@ import mozilla.components.browser.state.state.CustomTabSessionState
import mozilla.components.browser.state.state.SecurityInfoState
import mozilla.components.browser.state.state.TabSessionState
import mozilla.components.browser.state.state.TrackingProtectionState
import mozilla.components.browser.state.state.content.FindResultState
/**
* Create a matching [TabSessionState] from a [Session].
......@@ -60,13 +59,6 @@ fun Session.SecurityInfo.toSecurityInfoState(): SecurityInfoState {
return SecurityInfoState(secure, host, issuer)
}
/**
* Creates a matching [FindResultState] from a [Session.FindResult]
*/
fun Session.FindResult.toFindResultState(): FindResultState {
return FindResultState(activeMatchOrdinal, numberOfMatches, isDoneCounting)
}
/**
* Creates a matching [TrackingProtectionState] from a [Session].
*/
......
......@@ -5,7 +5,6 @@
package mozilla.components.browser.session
import android.graphics.Bitmap
import mozilla.components.concept.engine.HitResult
import mozilla.components.concept.engine.content.blocking.Tracker
import mozilla.components.support.test.mock
import org.junit.Assert.assertEquals
......@@ -148,10 +147,7 @@ class SelectionAwareSessionObserverTest {
observer.onCustomTabConfigChanged(session, null)
observer.onTrackerBlockingEnabledChanged(session, true)
observer.onTrackerBlocked(session, Tracker(""), emptyList())
observer.onLongPress(session, Mockito.mock(HitResult::class.java))
observer.onFindResult(session, Mockito.mock(Session.FindResult::class.java))
observer.onDesktopModeChanged(session, true)
observer.onFullScreenChanged(session, true)
observer.onThumbnailChanged(session, Mockito.spy(Bitmap::class.java))
observer.onSessionRemoved(session)
observer.onAllSessionsRemoved()
......
......@@ -5,7 +5,6 @@
package mozilla.components.browser.session
import android.content.ComponentCallbacks2
import mozilla.components.browser.session.ext.toFindResultState
import mozilla.components.browser.state.action.ReaderAction
import mozilla.components.browser.state.selector.findTab
import mozilla.components.browser.state.selector.selectedTab
......@@ -14,9 +13,7 @@ import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.engine.Engine
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.support.base.observer.Consumable
import mozilla.components.support.test.ext.joinBlocking
import mozilla.components.support.test.mock
import org.junit.Assert.assertEquals
......@@ -801,32 +798,6 @@ class SessionManagerMigrationTest {
}
}
@Test
fun `Adding a hit result`() {
val store = BrowserStore()
val manager = SessionManager(engine = mock(), store = store)
val session = Session(id = "session", initialUrl = "https://www.mozilla.org")
manager.add(session)
assertNull(session.hitResult.peek())
assertNull(store.state.findTab("session")!!.content.hitResult)
val hitResult: HitResult = HitResult.UNKNOWN("test")
session.hitResult = Consumable.from(hitResult)
assertEquals(hitResult, session.hitResult.peek())
store.state.findTab("session")!!.also { tab ->
assertNotNull(tab.content.hitResult)
assertSame(hitResult, tab.content.hitResult)
}
session.hitResult.consume { true }
store.state.findTab("session")!!.also { tab ->
assertNull(tab.content.hitResult)
}
}
@Test
fun `Linking session to engine session`() {
val store = BrowserStore()
......@@ -915,32 +886,6 @@ class SessionManagerMigrationTest {
}
}
@Test
fun `Adding a find result`() {
val store = BrowserStore()
val manager = SessionManager(engine = mock(), store = store)
val session = Session(id = "session", initialUrl = "https://www.mozilla.org")
manager.add(session)
assertTrue(session.findResults.isEmpty())
assertTrue(store.state.findTab("session")!!.content.findResults.isEmpty())
val result = Session.FindResult(0, 0, false)
session.findResults += result
assertEquals(1, session.findResults.size)
assertEquals(result, session.findResults.last())
store.state.findTab("session")!!.also { tab ->
assertEquals(1, tab.content.findResults.size)
assertEquals(result.toFindResultState(), tab.content.findResults.last())
}
session.findResults = emptyList()
assertTrue(session.findResults.isEmpty())
assertTrue(store.state.findTab("session")!!.content.findResults.isEmpty())
}
@Test
fun `Trimming memory - RUNNING_LOW - Removes thumbnails`() {
val store = BrowserStore()
......
......@@ -5,7 +5,6 @@
package mozilla.components.browser.session
import android.graphics.Bitmap
import android.view.WindowManager
import kotlinx.coroutines.Dispatchers.IO
import kotlinx.coroutines.GlobalScope
import kotlinx.coroutines.async
......@@ -14,7 +13,6 @@ import mozilla.components.browser.session.Session.Source
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.toFindResultState
import mozilla.components.browser.session.ext.toSecurityInfoState
import mozilla.components.browser.session.ext.toTabSessionState
import mozilla.components.browser.state.action.ContentAction
......@@ -22,7 +20,6 @@ import mozilla.components.browser.state.action.CustomTabListAction
import mozilla.components.browser.state.action.TabListAction
import mozilla.components.browser.state.state.CustomTabConfig
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.Size
import mozilla.components.concept.engine.manifest.WebAppManifest
......@@ -35,7 +32,6 @@ import mozilla.components.support.test.eq
import mozilla.components.support.test.mock
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNotEquals
import org.junit.Assert.assertNotNull
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
......@@ -451,64 +447,6 @@ class SessionTest {
assertEquals("2", session3.contextId)
}
@Test
fun `HitResult will be set on Session`() {
val hitResult: HitResult = mock()
`when`(hitResult.src).thenReturn("https://mozilla.org")
val session = Session("http://firefox.com")
session.hitResult = Consumable.from(hitResult)
assertFalse(session.hitResult.isConsumed())
var hitResultIsSet = false
session.hitResult.consume { consumable ->
hitResultIsSet = consumable.src == "https://mozilla.org"
true
}
assertTrue(hitResultIsSet)
assertTrue(session.hitResult.isConsumed())
}
@Test
fun `HitResult will not be set on Session if consumed by observer`() {
var callbackExecuted = false
val session = Session("https://www.mozilla.org")
session.register(object : Session.Observer {
override fun onLongPress(session: Session, hitResult: HitResult): Boolean {
callbackExecuted = true
return true // Consume HitResult
}
})
val hitResult: HitResult = mock()
session.hitResult = Consumable.from(hitResult)
assertTrue(callbackExecuted)
assertTrue(session.hitResult.isConsumed())
}
@Test
fun `action is dispatched when hit result changes`() {
val store: BrowserStore = mock()
`when`(store.dispatch(any())).thenReturn(mock())
val session = Session("https://www.mozilla.org")
session.store = store
session.hitResult = Consumable.empty()
verify(store).dispatch(ContentAction.ConsumeHitResultAction(session.id))
val hitResult = HitResult.UNKNOWN("test")
session.hitResult = Consumable.from(hitResult)
verify(store).dispatch(ContentAction.UpdateHitResultAction(session.id, hitResult))
session.hitResult.consume { true }
verify(store, times(2)).dispatch(ContentAction.ConsumeHitResultAction(session.id))
}
@Test
fun `observer is notified when title changes`() {
val observer = mock(Session.Observer::class.java)
......@@ -627,59 +565,6 @@ class SessionTest {
assertTrue(session.trackerBlockingEnabled)
}
@Test
fun `observer is notified on find result`() {
val observer = mock(Session.Observer::class.java)
val session = Session("https://www.mozilla.org")
session.register(observer)
val result1 = Session.FindResult(0, 1, false)
session.findResults += result1
verify(observer).onFindResult(
eq(session),
eq(result1)
)
assertEquals(listOf(result1), session.findResults)
result1.copy()
val result2 = result1.copy(1, 2)
session.findResults += result2
verify(observer).onFindResult(
eq(session),
eq(result2)
)
assertEquals(listOf(result1, result2), session.findResults)
assertEquals(session.findResults[0].activeMatchOrdinal, 0)
assertEquals(session.findResults[0].numberOfMatches, 1)
assertFalse(session.findResults[0].isDoneCounting)
assertEquals(session.findResults[1].activeMatchOrdinal, 1)
assertEquals(session.findResults[1].numberOfMatches, 2)
assertFalse(session.findResults[1].isDoneCounting)
assertNotEquals(result1, result2)
session.findResults = emptyList()
verifyNoMoreInteractions(observer)
}
@Test
fun `action is dispatched when find results are updated`() {
val store: BrowserStore = mock()
`when`(store.dispatch(any())).thenReturn(mock())
val session = Session("https://www.mozilla.org")
session.store = store
val result: Session.FindResult = Session.FindResult(0, 1, false)
session.findResults += result
verify(store).dispatch(ContentAction.AddFindResultAction(session.id, result.toFindResultState()))
session.findResults = emptyList()
verify(store).dispatch(ContentAction.ClearFindResultsAction(session.id))
verifyNoMoreInteractions(store)
}
@Test
fun `observer is notified when desktop mode is set`() {
val observer = mock(Session.Observer::class.java)
......@@ -692,34 +577,6 @@ class SessionTest {
assertTrue(session.desktopMode)
}
@Test
fun `observer is notified on fullscreen mode`() {
val observer = mock(Session.Observer::class.java)
val session = Session("https://www.mozilla.org")
session.register(observer)
session.fullScreenMode = true
verify(observer).onFullScreenChanged(session, true)
reset(observer)
session.unregister(observer)
session.fullScreenMode = false
verify(observer, never()).onFullScreenChanged(session, false)
}
@Test
fun `observer is notified on meta viewport fit change`() {
val observer = mock(Session.Observer::class.java)
val session = Session("https://www.mozilla.org")
session.register(observer)
session.layoutInDisplayCutoutMode =
WindowManager.LayoutParams.LAYOUT_IN_DISPLAY_CUTOUT_MODE_DEFAULT
verify(observer).onMetaViewportFitChanged(session,
WindowManager.LayoutParams.LAYOUT_IN_DISPLAY_CUTOUT_MODE_DEFAULT)
reset(observer)
session.unregister(observer)
session.layoutInDisplayCutoutMode = 123
verify(observer, never()).onMetaViewportFitChanged(session, 123)
}
@Test
fun `observer is notified on on thumbnail changed `() {
val observer = mock(Session.Observer::class.java)
......@@ -771,10 +628,7 @@ class SessionTest {
defaultObserver.onCustomTabConfigChanged(session, null)
defaultObserver.onTrackerBlockingEnabledChanged(session, true)
defaultObserver.onTrackerBlocked(session, mock(), emptyList())
defaultObserver.onLongPress(session, mock(HitResult::class.java))
defaultObserver.onFindResult(session, mock(Session.FindResult::class.java))
defaultObserver.onDesktopModeChanged(session, true)
defaultObserver.onFullScreenChanged(session, true)
defaultObserver.onThumbnailChanged(session, spy(Bitmap::class.java))
defaultObserver.onContentPermissionRequested(session, contentPermissionRequest)
defaultObserver.onAppPermissionRequested(session, appPermissionRequest)
......
......@@ -14,6 +14,7 @@ import mozilla.components.browser.session.engine.request.LoadRequestOption
import mozilla.components.browser.state.action.ContentAction
import mozilla.components.browser.state.action.TrackingProtectionAction
import mozilla.components.browser.state.selector.findTab
import mozilla.components.browser.state.state.content.FindResultState
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.engine.EngineSession
import mozilla.components.concept.engine.EngineSessionState
......@@ -39,6 +40,7 @@ import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.Mockito.mock
import org.mockito.Mockito.never
import org.mockito.Mockito.reset
import org.mockito.Mockito.spy
import org.mockito.Mockito.verify
......@@ -373,76 +375,112 @@ class EngineObserverTest {
@Test
fun engineObserverPassingHitResult() {
val session = Session("https://www.mozilla.org")
val observer = EngineObserver(session)
val session = Session("https://www.mozilla.org", id = "test-id")
val store: BrowserStore = mock()
val observer = EngineObserver(session, store)
val hitResult = HitResult.UNKNOWN("data://foobar")
observer.onLongPress(hitResult)
session.hitResult.consume {
assertEquals("data://foobar", it.src)
assertTrue(it is HitResult.UNKNOWN)
true
}
verify(store).dispatch(
ContentAction.UpdateHitResultAction("test-id", hitResult)
)
}
@Test
fun engineObserverClearsFindResults() {
val session = Session("https://www.mozilla.org")
val observer = EngineObserver(session)
val session = Session("https://www.mozilla.org", id = "test-id")