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

Commit

Permalink
Merge #7221
Browse files Browse the repository at this point in the history 
7221: Closes #7220: Add expiry to browser package names discovery r=pocmo a=rocketsroger



Co-authored-by: Roger Yang <royang@mozilla.com>
  • Loading branch information
@MozLando @rocketsroger
MozLando and rocketsroger committed Jun 3, 2020
2 parents 3cd07e5 + 77cb077 commit cdb6b04
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 23 deletions.
46 changes: 34 additions & 12 deletions .../feature/app-links/src/main/java/mozilla/components/feature/app/links/AppLinksUseCases.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import android.content.Intent
import android.content.pm.PackageManager
import android.content.pm.ResolveInfo
import android.net.Uri
import android.os.SystemClock
import androidx.annotation.VisibleForTesting
import mozilla.components.support.base.log.logger.Logger
import mozilla.components.support.ktx.android.net.isHttpOrHttps
Expand Down Expand Up @@ -44,17 +45,10 @@ internal const val APP_LINKS_CACHE_INTERVAL = 30 * 1000L // 30 seconds
class AppLinksUseCases(
private val context: Context,
private val launchInApp: () -> Boolean = { false },
browserPackageNames: Set<String>? = null,
unguessableWebUrl: String = "https://${UUID.randomUUID()}.net",
private val browserPackageNames: Set<String>? = null,
private val unguessableWebUrl: String = "https://${UUID.randomUUID()}.net",
private val alwaysDeniedSchemes: Set<String> = ALWAYS_DENY_SCHEMES
) {
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
internal val browserPackageNames: Lazy<Set<String>>

init {
this.browserPackageNames = lazy { browserPackageNames ?: findExcludedPackages(unguessableWebUrl) }
}

@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
internal fun findActivities(intent: Intent): List<ResolveInfo> {
return context.packageManager
Expand All @@ -73,7 +67,8 @@ class AppLinksUseCases(
}
}

private fun findExcludedPackages(randomWebURLString: String): Set<String> {
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
internal fun findExcludedPackages(randomWebURLString: String): Set<String> {
val intent = safeParseUri(randomWebURLString, 0) ?: return emptySet()
// We generate a URL is not likely to be opened by a native app
// but will fallback to a browser.
Expand All @@ -83,6 +78,24 @@ class AppLinksUseCases(
.toHashSet()
}

@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
internal fun getBrowserPackageNames(): Set<String> {
if (browserPackageNames != null) {
return browserPackageNames
}

val currentTimeStamp = SystemClock.elapsedRealtime()
val cache = browserNamesCache
if (cache != null && currentTimeStamp <= cache.cacheTimeStamp + APP_LINKS_CACHE_INTERVAL) {
return cache.cachedBrowserNames
}

val browserNames = findExcludedPackages(unguessableWebUrl)
browserNamesCache = AppLinkBrowserNamesCache(currentTimeStamp, browserNames)

return browserNames
}

/**
* Parse a URL and check if it can be handled by an app elsewhere on the Android device.
* If that app is not available, then a market place intent is also provided.
Expand All @@ -104,7 +117,7 @@ class AppLinksUseCases(
) {
operator fun invoke(url: String): AppLinkRedirect {
val urlHash = (url + includeHttpAppLinks + ignoreDefaultBrowser + includeHttpAppLinks).hashCode()
val currentTimeStamp = System.currentTimeMillis()
val currentTimeStamp = SystemClock.elapsedRealtime()
// since redirectCache is mutable, get the latest
val cache = redirectCache
if (cache != null && urlHash == cache.cachedUrlHash &&
Expand Down Expand Up @@ -142,7 +155,7 @@ class AppLinksUseCases(
private fun getNonBrowserActivities(intent: Intent): List<ResolveInfo> {
return findActivities(intent)
.map { it.activityInfo.packageName to it }
.filter { !browserPackageNames.value.contains(it.first) || intent.`package` == it.first }
.filter { intent.`package` == it.first || !getBrowserPackageNames().contains(it.first) }
.map { it.second }
}

Expand Down Expand Up @@ -260,10 +273,19 @@ class AppLinksUseCases(
var cachedAppLinkRedirect: AppLinkRedirect
)

@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
internal data class AppLinkBrowserNamesCache(
var cacheTimeStamp: Long,
var cachedBrowserNames: Set<String>
)

companion object {
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
internal var redirectCache: AppLinkRedirectCache? = null

@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
internal var browserNamesCache: AppLinkBrowserNamesCache? = null

@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
// list of scheme from https://searchfox.org/mozilla-central/source/netwerk/build/components.conf
internal val ENGINE_SUPPORTED_SCHEMES: Set<String> = setOf("about", "data", "file", "ftp", "http",
Expand Down
42 changes: 31 additions & 11 deletions ...ture/app-links/src/test/java/mozilla/components/feature/app/links/AppLinksUseCasesTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import kotlinx.coroutines.test.TestCoroutineDispatcher
import kotlinx.coroutines.test.TestCoroutineScope
import mozilla.components.support.test.mock
import mozilla.components.support.test.robolectric.testContext
import mozilla.components.support.test.whenever
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNotNull
Expand Down Expand Up @@ -52,6 +53,7 @@ class AppLinksUseCasesTest {
@Before
fun setup() {
AppLinksUseCases.redirectCache = null
AppLinksUseCases.browserNamesCache = null
}

private fun createContext(vararg urlToPackages: Pair<String, String>, default: Boolean = false): Context {
Expand Down Expand Up @@ -206,24 +208,14 @@ class AppLinksUseCasesTest {
assertFalse(menuRedirect.hasExternalApp())
}

@Test
fun `browser package names is lazily initialized`() {
val unguessable = "https://unguessable-test-url.com"
val context = createContext(unguessable to browserPackage)
val subject = AppLinksUseCases(context, unguessableWebUrl = unguessable)
assertFalse(subject.browserPackageNames.isInitialized())
}

@Test
fun `A list of browser package names can be generated if not supplied`() {
val unguessable = "https://unguessable-test-url.com"
val context = createContext(unguessable to browserPackage)
val subject = AppLinksUseCases(context, unguessableWebUrl = unguessable)
assertFalse(subject.browserPackageNames.isInitialized())

subject.appLinkRedirect(unguessable)
assertTrue(subject.browserPackageNames.isInitialized())
assertEquals(subject.browserPackageNames.value, setOf(browserPackage))
assertEquals(subject.getBrowserPackageNames(), setOf(browserPackage))
}

@Test
Expand Down Expand Up @@ -348,6 +340,34 @@ class AppLinksUseCasesTest {
}
}

@Test
fun `AppLinksUsecases uses browser names cache`() {
val testDispatcher = TestCoroutineDispatcher()
TestCoroutineScope(testDispatcher).launch {
val context = createContext(appUrl to appPackage)

var subject = AppLinksUseCases(context, { true })
whenever(subject.findExcludedPackages(any())).thenReturn(emptySet())
var browserNames = subject.getBrowserPackageNames()
assertTrue(browserNames.isEmpty())
val timestamp = AppLinksUseCases.browserNamesCache?.cacheTimeStamp

whenever(subject.findExcludedPackages(any())).thenReturn(setOf(appPackage))
testDispatcher.advanceTimeBy(APP_LINKS_CACHE_INTERVAL / 2)
subject = AppLinksUseCases(context, { true })
browserNames = subject.getBrowserPackageNames()
assertTrue(browserNames.isEmpty())
assert(timestamp == AppLinksUseCases.browserNamesCache?.cacheTimeStamp)

testDispatcher.advanceTimeBy(APP_LINKS_CACHE_INTERVAL / 2 + 1)
subject = AppLinksUseCases(context, { true })
browserNames = subject.getBrowserPackageNames()
assertFalse(browserNames.isEmpty())
assertFalse(browserNames.contains(appPackage))
assert(timestamp != AppLinksUseCases.browserNamesCache?.cacheTimeStamp)
}
}

@Test
fun `OpenAppLinkRedirect should not try to open files`() {
val context = spy(createContext())
Expand Down

0 comments on commit cdb6b04

Please sign in to comment.