Commit eabcd742 authored by Sebastian Kaspari's avatar Sebastian Kaspari
Browse files

Issue #7737: Remove findResults from Session.

parent bac84265
......@@ -11,11 +11,8 @@ 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
......@@ -101,7 +98,6 @@ class Session(
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
/**
......@@ -186,15 +182,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,23 +363,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.
*/
......
......@@ -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()
}
......@@ -169,11 +171,18 @@ internal class EngineObserver(
}
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(
......
......@@ -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].
*/
......
......@@ -149,7 +149,6 @@ class SelectionAwareSessionObserverTest {
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))
......
......@@ -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
......@@ -915,32 +914,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()
......
......@@ -14,7 +14,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
......@@ -35,7 +34,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
......@@ -627,59 +625,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)
......@@ -772,7 +717,6 @@ class SessionTest {
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))
......
......@@ -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
......@@ -388,32 +390,47 @@ class EngineObserverTest {
@Test
fun engineObserverClearsFindResults() {
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 result1 = Session.FindResult(0, 1, false)
val result2 = Session.FindResult(1, 2, true)
observer.onFindResult(0, 1, false)
observer.onFindResult(1, 2, true)
assertEquals(listOf(result1, result2), session.findResults)
verify(store).dispatch(ContentAction.AddFindResultAction(
"test-id", FindResultState(0, 1, false)
))
reset(store)
observer.onFind("mozilla")
assertEquals(emptyList<Session.FindResult>(), session.findResults)
verify(store).dispatch(
ContentAction.ClearFindResultsAction("test-id")
)
}
@Test
fun engineObserverClearsFindResultIfNewPageStartsLoading() {
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 result1 = Session.FindResult(0, 1, false)
val result2 = Session.FindResult(1, 2, true)
observer.onFindResult(0, 1, false)
verify(store).dispatch(ContentAction.AddFindResultAction(
"test-id", FindResultState(0, 1, false)
))
reset(store)
observer.onFindResult(1, 2, true)
assertEquals(listOf(result1, result2), session.findResults)
verify(store).dispatch(ContentAction.AddFindResultAction(
"test-id", FindResultState(1, 2, true)
))
reset(store)
observer.onLoadingStateChange(true)
assertEquals(emptyList<String>(), session.findResults)
verify(store).dispatch(ContentAction.ClearFindResultsAction("test-id"))
}
@Test
......
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