Skip to content
GitLab
Commit 3ee03f28 authored by Sebastian Kaspari's avatar Sebastian Kaspari
Browse files

Issue #7737: Remove hitResult from Session.

parent eabcd742
Hide whitespace changes
Inline Side-by-side
components/browser/session/src/main/java/mozilla/components/browser/session/Session.kt
View file @ 3ee03f28
......@@ -13,13 +13,11 @@ import mozilla.components.browser.session.engine.request.LoadRequestOption
import mozilla.components.browser.session.ext.syncDispatch
import mozilla.components.browser.session.ext.toSecurityInfoState
import mozilla.components.browser.session.ext.toTabSessionState
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
......@@ -36,7 +34,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
......@@ -97,7 +94,6 @@ 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 onDesktopModeChanged(session: Session, enabled: Boolean) = Unit
fun onFullScreenChanged(session: Session, enabled: Boolean) = Unit
/**
......@@ -363,24 +359,6 @@ class Session(
}
}
/**
* 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.
*/
......
components/browser/session/src/main/java/mozilla/components/browser/session/engine/EngineObserver.kt
View file @ 3ee03f28
......@@ -167,7 +167,9 @@ 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) {
......
components/browser/session/src/test/java/mozilla/components/browser/session/SelectionAwareSessionObserverTest.kt
View file @ 3ee03f28
......@@ -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,7 +147,6 @@ class SelectionAwareSessionObserverTest {
observer.onCustomTabConfigChanged(session, null)
observer.onTrackerBlockingEnabledChanged(session, true)
observer.onTrackerBlocked(session, Tracker(""), emptyList())
observer.onLongPress(session, Mockito.mock(HitResult::class.java))
observer.onDesktopModeChanged(session, true)
observer.onFullScreenChanged(session, true)
observer.onThumbnailChanged(session, Mockito.spy(Bitmap::class.java))
......
components/browser/session/src/test/java/mozilla/components/browser/session/SessionManagerMigrationTest.kt
View file @ 3ee03f28
......@@ -13,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
......@@ -800,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()
......
components/browser/session/src/test/java/mozilla/components/browser/session/SessionTest.kt
View file @ 3ee03f28
......@@ -21,7 +21,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
......@@ -449,64 +448,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)
......@@ -716,7 +657,6 @@ class SessionTest {
defaultObserver.onCustomTabConfigChanged(session, null)
defaultObserver.onTrackerBlockingEnabledChanged(session, true)
defaultObserver.onTrackerBlocked(session, mock(), emptyList())
defaultObserver.onLongPress(session, mock(HitResult::class.java))
defaultObserver.onDesktopModeChanged(session, true)
defaultObserver.onFullScreenChanged(session, true)
defaultObserver.onThumbnailChanged(session, spy(Bitmap::class.java))
......
components/browser/session/src/test/java/mozilla/components/browser/session/engine/EngineObserverTest.kt
View file @ 3ee03f28
......@@ -375,17 +375,16 @@ 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
......
components/feature/contextmenu/src/main/java/mozilla/components/feature/contextmenu/ContextMenuUseCases.kt
View file @ 3ee03f28
......@@ -4,8 +4,6 @@
package mozilla.components.feature.contextmenu
import mozilla.components.browser.session.Session
import mozilla.components.browser.session.SessionManager
import mozilla.components.browser.state.action.ContentAction
import mozilla.components.browser.state.state.content.DownloadState
import mozilla.components.browser.state.store.BrowserStore
......@@ -14,22 +12,19 @@ import mozilla.components.concept.engine.HitResult
/**
* Contains use cases related to the context menu feature.
*
* @param sessionManager the application's [SessionManager].
* @param store the application's [BrowserStore].
*/
class ContextMenuUseCases(
sessionManager: SessionManager,
store: BrowserStore
) {
class ConsumeHitResultUseCase(
private val sessionManager: SessionManager
private val store: BrowserStore
) {
/**
* Consumes the [HitResult] from the [Session] with the given [tabId].
* Consumes the [HitResult] from the [BrowserStore] with the given [tabId].
*/
operator fun invoke(tabId: String) {
sessionManager.findSessionById(tabId)?.let {
it.hitResult.consume { true }
}
store.dispatch(ContentAction.ConsumeHitResultAction(tabId))
}
}
......@@ -37,7 +32,7 @@ class ContextMenuUseCases(
private val store: BrowserStore
) {
/**
* Adds a [Download] to the [Session] with the given [tabId].
* Adds a [DownloadState] to the [BrowserStore] with the given [tabId].
*
* This is a hacky workaround. After we have migrated everything from browser-session to
* browser-state we should revisits this and find a better solution.
......@@ -49,6 +44,6 @@ class ContextMenuUseCases(
}
}
val consumeHitResult = ConsumeHitResultUseCase(sessionManager)
val consumeHitResult = ConsumeHitResultUseCase(store)
val injectDownload = InjectDownloadUseCase(store)
}
components/feature/contextmenu/src/test/java/mozilla/components/feature/contextmenu/ContextMenuCandidateTest.kt
View file @ 3ee03f28
......@@ -396,7 +396,7 @@ class ContextMenuCandidateTest {
val saveImage = ContextMenuCandidate.createSaveImageCandidate(
testContext,
ContextMenuUseCases(sessionManager, store))
ContextMenuUseCases(store))
// showFor
......@@ -448,7 +448,7 @@ class ContextMenuCandidateTest {
val saveVideoAudio = ContextMenuCandidate.createSaveVideoAudioCandidate(
testContext,
ContextMenuUseCases(sessionManager, store))
ContextMenuUseCases(store))
// showFor
......@@ -503,7 +503,8 @@ class ContextMenuCandidateTest {
val downloadLink = ContextMenuCandidate.createDownloadLinkCandidate(
testContext,
ContextMenuUseCases(sessionManager, store))
ContextMenuUseCases(store)
)
// showFor
......
components/feature/contextmenu/src/test/java/mozilla/components/feature/contextmenu/ContextMenuFeatureTest.kt
View file @ 3ee03f28
......@@ -26,9 +26,9 @@ import mozilla.components.concept.engine.HitResult
import mozilla.components.support.base.Component
import mozilla.components.support.base.facts.Action
import mozilla.components.support.base.facts.processor.CollectionProcessor
import mozilla.components.support.base.observer.Consumable
import mozilla.components.support.test.any
import mozilla.components.support.test.ext.joinBlocking
import mozilla.components.support.test.libstate.ext.waitUntilIdle
import mozilla.components.support.test.mock
import mozilla.components.support.test.robolectric.testContext
import org.junit.After
......@@ -234,7 +234,7 @@ class ContextMenuFeatureTest {
store,
listOf(candidate),
engineView,
ContextMenuUseCases(mock(), mock())
ContextMenuUseCases(mock())
)
feature.showContextMenu(
......@@ -252,9 +252,13 @@ class ContextMenuFeatureTest {
val sessionManager = SessionManager(engine = mock(), store = store)
Session("https://www.mozilla.org", id = "test-tab").also {
sessionManager.add(it)
it.hitResult = Consumable.from(HitResult.UNKNOWN("https://www.mozilla.org"))
}
store.dispatch(ContentAction.UpdateHitResultAction(
"test-tab",
HitResult.UNKNOWN("https://www.mozilla.org")
)).joinBlocking()
val (engineView, _) = mockEngineView()
val feature = ContextMenuFeature(
......@@ -262,13 +266,14 @@ class ContextMenuFeatureTest {
store,
ContextMenuCandidate.defaultCandidates(testContext, mock(), mock(), mock()),
engineView,
ContextMenuUseCases(sessionManager, store)
ContextMenuUseCases(store)
)
assertNotNull(store.state.findTab("test-tab")!!.content.hitResult)
feature.onMenuCancelled("test-tab")
store.waitUntilIdle()
testDispatcher.advanceUntilIdle()
assertNull(store.state.findTab("test-tab")!!.content.hitResult)
......@@ -281,9 +286,13 @@ class ContextMenuFeatureTest {
val sessionManager = SessionManager(engine = mock(), store = store)
Session("https://www.mozilla.org", id = "test-tab").also {
sessionManager.add(it)
it.hitResult = Consumable.from(HitResult.UNKNOWN("https://www.mozilla.org"))
}
store.dispatch(ContentAction.UpdateHitResultAction(
"test-tab",
HitResult.UNKNOWN("https://www.mozilla.org")
)).joinBlocking()
val (engineView, view) = mockEngineView()
var actionInvoked = false
......@@ -298,7 +307,7 @@ class ContextMenuFeatureTest {
store,
listOf(candidate),
engineView,
ContextMenuUseCases(sessionManager, store)
ContextMenuUseCases(store)
)
testDispatcher.advanceUntilIdle()
......@@ -322,9 +331,13 @@ class ContextMenuFeatureTest {
val sessionManager = SessionManager(engine = mock(), store = store)
Session("https://www.mozilla.org", id = "test-tab").also {
sessionManager.add(it)
it.hitResult = Consumable.from(HitResult.UNKNOWN("https://www.mozilla.org"))
}
store.dispatch(ContentAction.UpdateHitResultAction(
"test-tab",
HitResult.UNKNOWN("https://www.mozilla.org")
)).joinBlocking()
val (engineView, _) = mockEngineView()
val candidate = ContextMenuCandidate(
id = "test-id",
......@@ -337,7 +350,7 @@ class ContextMenuFeatureTest {
store,
listOf(candidate),
engineView,
ContextMenuUseCases(sessionManager, store)
ContextMenuUseCases(store)
)
CollectionProcessor.withFactCollection { facts ->
......
samples/browser/src/main/java/org/mozilla/samples/browser/DefaultComponents.kt
View file @ 3ee03f28
......@@ -377,5 +377,5 @@ open class DefaultComponents(private val applicationContext: Context) {
val tabsUseCases: TabsUseCases by lazy { TabsUseCases(store, sessionManager) }
val downloadsUseCases: DownloadsUseCases by lazy { DownloadsUseCases(store) }
val contextMenuUseCases: ContextMenuUseCases by lazy { ContextMenuUseCases(sessionManager, store) }
val contextMenuUseCases: ContextMenuUseCases by lazy { ContextMenuUseCases(store) }
}
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment