Skip to content
This repository was archived by the owner on Nov 1, 2022. It is now read-only.

Closes #1126: Rust Places backed implementation of concept-storage #1164

Merged
merged 6 commits into from
Nov 6, 2018
Merged

Closes #1126: Rust Places backed implementation of concept-storage #1164

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c2e704d
Bump app-services dependency to 0.8.1
Oct 31, 2018
3687bb2
Switch HistoryStorage to use Deferred results and suspend functions
Oct 31, 2018
dcaf924
Add 'cleanup' method to HistoryStorage concept
Nov 1, 2018
61c4724
Add 'limit' to HistoryStorage's getSuggestion concept method
Nov 5, 2018
bd3204d
A Rust Places backed implementation of concept-storage
Oct 25, 2018
9a28351
Updated changelog to reflect concept-* and browser-* changes
Nov 6, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions automation/taskcluster/artifacts.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@
public/build/browser.storage-memory.maven.zip:
path: /build/android-components/components/browser/storage-memory/build/target.maven.zip
name: browser-storage-memory
public/build/browser.storage-sync.maven.zip:
path: /build/android-components/components/browser/storage-sync/build/target.maven.zip
name: browser-storage-sync
public/build/browser.domains.maven.zip:
path: /build/android-components/components/browser/domains/build/target.maven.zip
name: browser-domains
Expand Down
3 changes: 2 additions & 1 deletion buildSrc/src/main/java/Dependencies.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ private object Versions {

const val sentry = "1.7.10"

const val mozilla_app_services = "0.7.1"
const val mozilla_app_services = "0.8.1"
const val servo = "0.0.1.20181017.aa95911"
}

Expand Down Expand Up @@ -63,6 +63,7 @@ object Deps {

const val mozilla_fxa = "org.mozilla.fxaclient:fxaclient:${Versions.mozilla_app_services}"
const val mozilla_sync_logins = "org.mozilla.sync15:logins:${Versions.mozilla_app_services}"
const val mozilla_places = "org.mozilla.places:places:${Versions.mozilla_app_services}"
const val mozilla_servo = "org.mozilla.servoview:servoview-armv7:${Versions.servo}"

const val thirdparty_sentry = "io.sentry:sentry-android:${Versions.sentry}"
Expand Down
8 changes: 6 additions & 2 deletions ...ne-gecko-beta/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineSession.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,8 +382,12 @@ class GeckoEngineSession(
override fun onCloseRequest(session: GeckoSession) = Unit

override fun onTitleChange(session: GeckoSession, title: String) {
currentUrl?.let {
settings.historyTrackingDelegate?.onTitleChanged(it, title, privateMode)
currentUrl?.let { url ->
settings.historyTrackingDelegate?.let { delegate ->
runBlocking {
delegate.onTitleChanged(url, title, privateMode)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it need to be blocking?

}
}
notifyObservers { onTitleChange(title) }
}
Expand Down
5 changes: 3 additions & 2 deletions ...ecko-beta/src/test/java/mozilla/components/browser/engine/gecko/GeckoEngineSessionTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package mozilla.components.browser.engine.gecko

import android.os.Handler
import android.os.Message
import kotlinx.coroutines.experimental.runBlocking
import mozilla.components.browser.errorpages.ErrorType
import mozilla.components.concept.engine.DefaultSettings
import mozilla.components.concept.engine.EngineSession
Expand Down Expand Up @@ -366,7 +367,7 @@ class GeckoEngineSessionTest {
}

@Test
fun `WebView client notifies configured history delegate of title changes`() {
fun `WebView client notifies configured history delegate of title changes`() = runBlocking {
val engineSession = GeckoEngineSession(mock(GeckoRuntime::class.java))
val historyDelegate: HistoryTrackingDelegate = mock()

Expand All @@ -386,7 +387,7 @@ class GeckoEngineSessionTest {
}

@Test
fun `WebView client notifies configured history delegate of title changes for private sessions`() {
fun `WebView client notifies configured history delegate of title changes for private sessions`() = runBlocking {
val engineSession = GeckoEngineSession(mock(GeckoRuntime::class.java), privateMode = true)
val historyDelegate: HistoryTrackingDelegate = mock()

Expand Down
8 changes: 6 additions & 2 deletions ...gecko-nightly/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineSession.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,8 +382,12 @@ class GeckoEngineSession(
override fun onCloseRequest(session: GeckoSession) = Unit

override fun onTitleChange(session: GeckoSession, title: String) {
currentUrl?.let {
settings.historyTrackingDelegate?.onTitleChanged(it, title, privateMode)
currentUrl?.let { url ->
settings.historyTrackingDelegate?.let { delegate ->
runBlocking {
delegate.onTitleChanged(url, title, privateMode)
}
}
}
notifyObservers { onTitleChange(title) }
}
Expand Down
5 changes: 3 additions & 2 deletions ...o-nightly/src/test/java/mozilla/components/browser/engine/gecko/GeckoEngineSessionTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package mozilla.components.browser.engine.gecko

import android.os.Handler
import android.os.Message
import kotlinx.coroutines.experimental.runBlocking
import mozilla.components.browser.errorpages.ErrorType
import mozilla.components.concept.engine.DefaultSettings
import mozilla.components.concept.engine.EngineSession
Expand Down Expand Up @@ -366,7 +367,7 @@ class GeckoEngineSessionTest {
}

@Test
fun `WebView client notifies configured history delegate of title changes`() {
fun `WebView client notifies configured history delegate of title changes`() = runBlocking {
val engineSession = GeckoEngineSession(mock(GeckoRuntime::class.java))
val historyDelegate: HistoryTrackingDelegate = mock()

Expand All @@ -386,7 +387,7 @@ class GeckoEngineSessionTest {
}

@Test
fun `WebView client notifies configured history delegate of title changes for private sessions`() {
fun `WebView client notifies configured history delegate of title changes for private sessions`() = runBlocking {
val engineSession = GeckoEngineSession(mock(GeckoRuntime::class.java), privateMode = true)
val historyDelegate: HistoryTrackingDelegate = mock()

Expand Down
8 changes: 6 additions & 2 deletions .../engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineSession.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,8 +384,12 @@ class GeckoEngineSession(
override fun onCloseRequest(session: GeckoSession) = Unit

override fun onTitleChange(session: GeckoSession, title: String) {
currentUrl?.let {
settings.historyTrackingDelegate?.onTitleChanged(it, title, privateMode)
currentUrl?.let { url ->
settings.historyTrackingDelegate?.let { delegate ->
runBlocking {
delegate.onTitleChanged(url, title, privateMode)
}
}
}
notifyObservers { onTitleChange(title) }
}
Expand Down
5 changes: 3 additions & 2 deletions ...ine-gecko/src/test/java/mozilla/components/browser/engine/gecko/GeckoEngineSessionTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package mozilla.components.browser.engine.gecko

import android.os.Handler
import android.os.Message
import kotlinx.coroutines.experimental.runBlocking
import mozilla.components.browser.errorpages.ErrorType
import mozilla.components.concept.engine.DefaultSettings
import mozilla.components.concept.engine.EngineSession
Expand Down Expand Up @@ -378,7 +379,7 @@ class GeckoEngineSessionTest {
}

@Test
fun `WebView client notifies configured history delegate of title changes`() {
fun `WebView client notifies configured history delegate of title changes`() = runBlocking {
val engineSession = GeckoEngineSession(mock(GeckoRuntime::class.java))
val historyDelegate: HistoryTrackingDelegate = mock()

Expand All @@ -398,7 +399,7 @@ class GeckoEngineSessionTest {
}

@Test
fun `WebView client notifies configured history delegate of title changes for private sessions`() {
fun `WebView client notifies configured history delegate of title changes for private sessions`() = runBlocking {
val engineSession = GeckoEngineSession(mock(GeckoRuntime::class.java), privateMode = true)
val historyDelegate: HistoryTrackingDelegate = mock()

Expand Down
23 changes: 15 additions & 8 deletions .../engine-system/src/main/java/mozilla/components/browser/engine/system/SystemEngineView.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import android.webkit.WebView.HitTestResult.SRC_ANCHOR_TYPE
import android.webkit.WebView.HitTestResult.SRC_IMAGE_ANCHOR_TYPE
import android.webkit.WebViewClient
import android.widget.FrameLayout
import kotlinx.coroutines.experimental.runBlocking
import mozilla.components.browser.engine.system.matcher.UrlMatcher
import mozilla.components.browser.errorpages.ErrorType
import mozilla.components.concept.engine.EngineSession
Expand Down Expand Up @@ -119,7 +120,9 @@ class SystemEngineView @JvmOverloads constructor(
override fun doUpdateVisitedHistory(view: WebView, url: String, isReload: Boolean) {
// TODO private browsing not supported for SystemEngine
// https://github.com/mozilla-mobile/android-components/issues/649
session?.settings?.historyTrackingDelegate?.onVisited(url, isReload, privateMode = false)
runBlocking {
session?.settings?.historyTrackingDelegate?.onVisited(url, isReload, privateMode = false)
}
}

override fun onPageStarted(view: WebView?, url: String?, favicon: Bitmap?) {
Expand Down Expand Up @@ -261,9 +264,11 @@ class SystemEngineView @JvmOverloads constructor(
override fun getVisitedHistory(callback: ValueCallback<Array<String>>) {
// TODO private browsing not supported for SystemEngine
// https://github.com/mozilla-mobile/android-components/issues/649
session?.settings?.historyTrackingDelegate?.getVisited({
callback.onReceiveValue(it.toTypedArray())
}, privateMode = false)
session?.settings?.historyTrackingDelegate?.let {
runBlocking {
callback.onReceiveValue(it.getVisited(false).await().toTypedArray())
}
}
}

override fun onProgressChanged(view: WebView?, newProgress: Int) {
Expand All @@ -274,10 +279,12 @@ class SystemEngineView @JvmOverloads constructor(
val titleOrEmpty = title ?: ""
// TODO private browsing not supported for SystemEngine
// https://github.com/mozilla-mobile/android-components/issues/649
if (currentUrl.isNotEmpty()) {
session?.settings?.historyTrackingDelegate?.onTitleChanged(
currentUrl, titleOrEmpty, privateMode = false
)
currentUrl.takeIf { it.isNotEmpty() }?.let { url ->
session?.settings?.historyTrackingDelegate?.let { delegate ->
runBlocking {
delegate.onTitleChanged(url, titleOrEmpty, privateMode = false)
}
}
}
session?.internalNotifyObservers {
onTitleChange(titleOrEmpty)
Expand Down
22 changes: 14 additions & 8 deletions ...ine-system/src/test/java/mozilla/components/browser/engine/system/SystemEngineViewTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ import android.webkit.WebView
import android.webkit.ValueCallback
import android.webkit.WebViewClient
import android.webkit.WebView.HitTestResult
import kotlinx.coroutines.experimental.CompletableDeferred
import kotlinx.coroutines.experimental.Deferred
import kotlinx.coroutines.experimental.runBlocking
import mozilla.components.browser.engine.system.matcher.UrlMatcher
import mozilla.components.browser.errorpages.ErrorType
import mozilla.components.concept.engine.EngineSession
Expand Down Expand Up @@ -227,7 +230,7 @@ class SystemEngineViewTest {
}

@Test
fun `WebView client notifies configured history delegate of url visits`() {
fun `WebView client notifies configured history delegate of url visits`() = runBlocking {
val engineSession = SystemEngineSession()

val engineView = SystemEngineView(RuntimeEnvironment.application)
Expand Down Expand Up @@ -257,20 +260,21 @@ class SystemEngineViewTest {

val engineView = SystemEngineView(RuntimeEnvironment.application)
val historyDelegate = object : HistoryTrackingDelegate {
override fun onVisited(uri: String, isReload: Boolean, privateMode: Boolean) {
override suspend fun onVisited(uri: String, isReload: Boolean, privateMode: Boolean) {
fail()
}

override fun onTitleChanged(uri: String, title: String, privateMode: Boolean) {
override suspend fun onTitleChanged(uri: String, title: String, privateMode: Boolean) {
fail()
}

override fun getVisited(uris: List<String>, callback: (List<Boolean>) -> Unit, privateMode: Boolean) {
override fun getVisited(uris: List<String>, privateMode: Boolean): Deferred<List<Boolean>> {
fail()
return CompletableDeferred(listOf())
}

override fun getVisited(callback: (List<String>) -> Unit, privateMode: Boolean) {
callback(listOf("https://www.mozilla.com"))
override fun getVisited(privateMode: Boolean): Deferred<List<String>> {
return CompletableDeferred(listOf("https://www.mozilla.com"))
}
}

Expand All @@ -285,12 +289,14 @@ class SystemEngineViewTest {
engineSession.settings.historyTrackingDelegate = historyDelegate

val historyValueCallback: ValueCallback<Array<String>> = mock()
engineView.currentWebView.webChromeClient.getVisitedHistory(historyValueCallback)
runBlocking {
engineView.currentWebView.webChromeClient.getVisitedHistory(historyValueCallback)
}
verify(historyValueCallback).onReceiveValue(arrayOf("https://www.mozilla.com"))
}

@Test
fun `WebView client notifies configured history delegate of title changes`() {
fun `WebView client notifies configured history delegate of title changes`() = runBlocking {
val engineSession = SystemEngineSession()

val engineView = SystemEngineView(RuntimeEnvironment.application)
Expand Down
24 changes: 15 additions & 9 deletions ...-memory/src/main/java/mozilla/components/browser/storage/memory/InMemoryHistoryStorage.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
package mozilla.components.browser.storage.memory

import android.support.annotation.VisibleForTesting
import kotlinx.coroutines.experimental.CompletableDeferred
import kotlinx.coroutines.experimental.Deferred
import mozilla.components.concept.storage.HistoryStorage
import mozilla.components.concept.storage.PageObservation
import mozilla.components.concept.storage.SearchResult
Expand All @@ -24,7 +26,7 @@ class InMemoryHistoryStorage : HistoryStorage {
@VisibleForTesting
internal val pageMeta: HashMap<String, PageObservation> = hashMapOf()

override fun recordVisit(uri: String, visitType: VisitType) {
override suspend fun recordVisit(uri: String, visitType: VisitType) {
val now = System.currentTimeMillis()

synchronized(pages) {
Expand All @@ -36,14 +38,14 @@ class InMemoryHistoryStorage : HistoryStorage {
}
}

override fun recordObservation(uri: String, observation: PageObservation) {
override suspend fun recordObservation(uri: String, observation: PageObservation) {
synchronized(pageMeta) {
pageMeta[uri] = observation
}
}

override fun getVisited(uris: List<String>, callback: (List<Boolean>) -> Unit) {
callback(synchronized(pages) {
override fun getVisited(uris: List<String>): Deferred<List<Boolean>> {
return CompletableDeferred(synchronized(pages) {
uris.map {
if (pages[it] != null && pages[it]!!.size > 0) {
return@map true
Expand All @@ -53,13 +55,13 @@ class InMemoryHistoryStorage : HistoryStorage {
})
}

override fun getVisited(callback: (List<String>) -> Unit) {
callback(synchronized(pages) {
(pages.keys.toList())
override fun getVisited(): Deferred<List<String>> {
return CompletableDeferred(synchronized(pages) {
pages.keys.toList()
})
}

override fun getSuggestions(query: String): List<SearchResult> = synchronized(pages + pageMeta) {
override fun getSuggestions(query: String, limit: Int): List<SearchResult> = synchronized(pages + pageMeta) {
data class Hit(val url: String, val score: Int)

val urlMatches = pages.asSequence().map {
Expand All @@ -84,7 +86,7 @@ class InMemoryHistoryStorage : HistoryStorage {
// TODO exclude non-matching results entirely? Score that implies complete mismatch.
matchedUrls.asSequence().sortedBy { it.value }.map {
SearchResult(id = it.key, score = maxScore - it.value, url = it.key, title = pageMeta[it.key]?.title)
}.toList()
}.take(limit).toList()
}

// Borrowed from https://gist.github.com/ademar111190/34d3de41308389a0d0d8
Expand Down Expand Up @@ -115,4 +117,8 @@ class InMemoryHistoryStorage : HistoryStorage {

return cost[lhsLength - 1]
}

override fun cleanup() {
// GC will take care of our internal data structures, so there's nothing to do here.
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now we call cleanup() from an Activity. With the activity going away this doesn't mean the process/engine and therefore the delegate going away though.

  • If we want to call this while parts of the app are still alive: Should we clear the map?
  • If we want to call this when the app process is getting killed: Okay, we do not need to do anything here. But there's no event we could call this from.

}
Loading