Skip to content
GitLab
Commit d61f7e12 authored by Sebastian Kaspari's avatar Sebastian Kaspari Committed by Christian Sadilek
Browse files

Closes #2048: Rank icons before trying to load them.

parent ba364488
Hide whitespace changes
Inline Side-by-side
components/browser/icons/src/main/java/mozilla/components/browser/icons/BrowserIcons.kt
View file @ d61f7e12
......@@ -19,6 +19,7 @@ import mozilla.components.browser.icons.generator.IconGenerator
import mozilla.components.browser.icons.loader.DataUriIconLoader
import mozilla.components.browser.icons.loader.HttpIconLoader
import mozilla.components.browser.icons.loader.IconLoader
import mozilla.components.browser.icons.pipeline.IconResourceComparator
import mozilla.components.concept.engine.Engine
import mozilla.components.concept.engine.webextension.WebExtension
import mozilla.components.concept.fetch.Client
......@@ -91,9 +92,7 @@ class BrowserIcons(
}
private fun load(request: IconRequest, loaders: List<IconLoader>): Pair<ByteArray, Icon.Source>? {
// We are just looping over the resources here. We need to rank them first to try the best icon first.
// https://github.com/mozilla-mobile/android-components/issues/2048
request.resources.forEach { resource ->
request.resources.toMutableList().sortedWith(IconResourceComparator).forEach { resource ->
loaders.forEach { loader ->
val data = loader.load(request, resource)
if (data != null) {
......
components/browser/icons/src/main/java/mozilla/components/browser/icons/pipeline/IconResourceComparator.kt 0 → 100644
View file @ d61f7e12
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
package mozilla.components.browser.icons.pipeline
import mozilla.components.browser.icons.IconRequest
import kotlin.math.min
/**
* This [Comparator] implementations compares [IconRequest.Resource] objects to determine which icon to try to load
* first.
*/
internal object IconResourceComparator : Comparator<IconRequest.Resource> {
@Suppress("ComplexMethod", "ReturnCount")
override fun compare(resource: IconRequest.Resource, other: IconRequest.Resource): Int {
if (resource.url == other.url) {
// Two resources pointing to the same URL are always referencing the same icon. So treat them as equal.
return 0
}
if (resource.type != other.type) {
return if (resource.type.rank() > other.type.rank()) -1 else 1
}
if (resource.maxSize != other.maxSize) {
return if (resource.maxSize > other.maxSize) -1 else 1
}
// If there's no other way to choose, we prefer container types. They *might* contain an image larger than the
// size given in the <link> tag.
val isResourceContainerType = resource.isContainerType
val isOtherContainerType = other.isContainerType
if (isResourceContainerType != isOtherContainerType) {
return if (isResourceContainerType) -1 else 1
}
// There's no way to know which icon might be better. However we need to pick a consistent one to avoid breaking
// set implementations (See Fennec Bug 1331808). Therefore we are picking one by just comparing the URLs.
return resource.url.compareTo(other.url)
}
}
@Suppress("MagicNumber")
private fun IconRequest.Resource.Type.rank(): Int {
return when (this) {
// We prefer touch icons because they tend to have a higher resolution than ordinary favicons.
IconRequest.Resource.Type.APPLE_TOUCH_ICON -> 15
IconRequest.Resource.Type.FAVICON -> 10
// Fallback icon types:
IconRequest.Resource.Type.IMAGE_SRC -> 6
IconRequest.Resource.Type.FLUID_ICON -> 5
IconRequest.Resource.Type.OPENGRAPH -> 4
IconRequest.Resource.Type.TWITTER -> 3
IconRequest.Resource.Type.MICROSOFT_TILE -> 2
}
}
private val IconRequest.Resource.maxSize: Int
get() {
return sizes.maxBy { size ->
min(size.width, size.height)
}?.let { min(it.width, it.height) } ?: 0
}
private val IconRequest.Resource.isContainerType: Boolean
get() = mimeType != null && containerTypes.contains(mimeType)
private val containerTypes = listOf(
"image/vnd.microsoft.icon",
"image/ico",
"image/icon",
"image/x-icon",
"text/ico",
"application/ico"
)
components/browser/icons/src/test/java/mozilla/components/browser/icons/BrowserIconsTest.kt
View file @ d61f7e12
......@@ -87,7 +87,7 @@ class BrowserIconsTest {
assertNotNull(icon.bitmap!!)
val serverRequest = server.takeRequest()
assertEquals("/icon64.png", serverRequest.requestUrl.encodedPath())
assertEquals("/icon128.png", serverRequest.requestUrl.encodedPath())
} finally {
server.shutdown()
}
......
components/browser/icons/src/test/java/mozilla/components/browser/icons/pipeline/IconResourceComparatorTest.kt 0 → 100644
View file @ d61f7e12
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
package mozilla.components.browser.icons.pipeline
import mozilla.components.browser.icons.IconRequest
import org.junit.Assert.assertEquals
import org.junit.Test
class IconResourceComparatorTest {
@Test
fun `compare mozilla-org icons`() {
val resources = listOf(
IconRequest.Resource(
url = "https://www.mozilla.org/media/img/favicon/favicon-196x196.c80e6abe0767.png",
type = IconRequest.Resource.Type.FAVICON,
sizes = listOf(IconRequest.Resource.Size(196, 196))),
IconRequest.Resource(
url = "https://www.mozilla.org/media/img/favicon.d4f1f46b91f4.ico",
type = IconRequest.Resource.Type.FAVICON),
IconRequest.Resource(
url = "https://www.mozilla.org/media/img/favicon/apple-touch-icon-180x180.8772ec154918.png",
type = IconRequest.Resource.Type.APPLE_TOUCH_ICON,
sizes = listOf(IconRequest.Resource.Size(180, 180))),
IconRequest.Resource(
url = "https://www.mozilla.org/media/img/mozorg/mozilla-256.4720741d4108.jpg",
type = IconRequest.Resource.Type.OPENGRAPH)
)
val urls = resources.sortedWith(IconResourceComparator).map { it.url }
assertEquals(
listOf(
"https://www.mozilla.org/media/img/favicon/apple-touch-icon-180x180.8772ec154918.png",
"https://www.mozilla.org/media/img/favicon/favicon-196x196.c80e6abe0767.png",
"https://www.mozilla.org/media/img/favicon.d4f1f46b91f4.ico",
"https://www.mozilla.org/media/img/mozorg/mozilla-256.4720741d4108.jpg"
),
urls
)
}
@Test
fun `compare m-youtube-com icons`() {
val resources = listOf(
IconRequest.Resource(
url = "https://s.ytimg.com/yts/img/favicon-vfl8qSV2F.ico",
type = IconRequest.Resource.Type.FAVICON,
mimeType = "image/x-icon"
),
IconRequest.Resource(
url = "https://s.ytimg.com/yts/img/favicon-vfl8qSV2F.ico",
type = IconRequest.Resource.Type.FAVICON,
mimeType = "image/x-icon"
)
)
val urls = resources.sortedWith(IconResourceComparator).map { it.url }
assertEquals(
listOf(
"https://s.ytimg.com/yts/img/favicon-vfl8qSV2F.ico",
"https://s.ytimg.com/yts/img/favicon-vfl8qSV2F.ico"
),
urls
)
}
@Test
fun `compare m-facebook-com icons`() {
val resources = listOf(
IconRequest.Resource(
url = "https://static.xx.fbcdn.net/rsrc.php/v3/ya/r/O2aKM2iSbOw.png",
type = IconRequest.Resource.Type.FAVICON,
sizes = listOf(IconRequest.Resource.Size(196, 196))
)
)
val urls = resources.sortedWith(IconResourceComparator).map { it.url }
assertEquals(
listOf(
"https://static.xx.fbcdn.net/rsrc.php/v3/ya/r/O2aKM2iSbOw.png"
),
urls
)
}
@Test
fun `compare baidu-com icons`() {
val resources = listOf(
IconRequest.Resource(
url = "http://sm.bdimg.com/static/wiseindex/img/favicon64.ico",
type = IconRequest.Resource.Type.FAVICON
),
IconRequest.Resource(
url = "http://sm.bdimg.com/static/wiseindex/img/screen_icon_new.png",
type = IconRequest.Resource.Type.APPLE_TOUCH_ICON
)
)
val urls = resources.sortedWith(IconResourceComparator).map { it.url }
assertEquals(
listOf(
"http://sm.bdimg.com/static/wiseindex/img/screen_icon_new.png",
"http://sm.bdimg.com/static/wiseindex/img/favicon64.ico"
),
urls
)
}
@Test
fun `compare wikipedia-org icons`() {
val resources = listOf(
IconRequest.Resource(
url = "https://www.wikipedia.org/static/favicon/wikipedia.ico",
type = IconRequest.Resource.Type.FAVICON
),
IconRequest.Resource(
url = "https://www.wikipedia.org/static/apple-touch/wikipedia.png",
type = IconRequest.Resource.Type.APPLE_TOUCH_ICON
)
)
val urls = resources.sortedWith(IconResourceComparator).map { it.url }
assertEquals(
listOf(
"https://www.wikipedia.org/static/apple-touch/wikipedia.png",
"https://www.wikipedia.org/static/favicon/wikipedia.ico"
),
urls
)
}
@Test
fun `compare amazon-com icons`() {
val resources = listOf(
IconRequest.Resource(
url = "https://images-na.ssl-images-amazon.com/images/G/01/anywhere/a_smile_57x57._CB368212015_.png",
type = IconRequest.Resource.Type.APPLE_TOUCH_ICON,
sizes = listOf(IconRequest.Resource.Size(57, 57))
),
IconRequest.Resource(
url = "https://images-na.ssl-images-amazon.com/images/G/01/anywhere/a_smile_72x72._CB368212002_.png",
type = IconRequest.Resource.Type.APPLE_TOUCH_ICON,
sizes = listOf(IconRequest.Resource.Size(72, 72))
),
IconRequest.Resource(
url = "https://images-na.ssl-images-amazon.com/images/G/01/anywhere/a_smile_114x114._CB368212020_.png",
type = IconRequest.Resource.Type.APPLE_TOUCH_ICON,
sizes = listOf(IconRequest.Resource.Size(114, 114))
),
IconRequest.Resource(
url = "https://images-na.ssl-images-amazon.com/images/G/01/anywhere/a_smile_120x120._CB368246573_.png",
type = IconRequest.Resource.Type.APPLE_TOUCH_ICON,
sizes = listOf(IconRequest.Resource.Size(120, 120))
),
IconRequest.Resource(
url = "https://images-na.ssl-images-amazon.com/images/G/01/anywhere/a_smile_144x144._CB368211973_.png",
type = IconRequest.Resource.Type.APPLE_TOUCH_ICON,
sizes = listOf(IconRequest.Resource.Size(144, 144))
),
IconRequest.Resource(
url = "https://images-na.ssl-images-amazon.com/images/G/01/anywhere/a_smile_152x152._CB368246573_.png",
type = IconRequest.Resource.Type.APPLE_TOUCH_ICON,
sizes = listOf(IconRequest.Resource.Size(152, 152))
),
IconRequest.Resource(
url = "https://images-na.ssl-images-amazon.com/images/G/01/anywhere/a_smile_196x196._CB368246573_.png",
type = IconRequest.Resource.Type.APPLE_TOUCH_ICON,
sizes = listOf(IconRequest.Resource.Size(196, 196))
)
)
val urls = resources.sortedWith(IconResourceComparator).map { it.url }
assertEquals(
listOf(
"https://images-na.ssl-images-amazon.com/images/G/01/anywhere/a_smile_196x196._CB368246573_.png",
"https://images-na.ssl-images-amazon.com/images/G/01/anywhere/a_smile_152x152._CB368246573_.png",
"https://images-na.ssl-images-amazon.com/images/G/01/anywhere/a_smile_144x144._CB368211973_.png",
"https://images-na.ssl-images-amazon.com/images/G/01/anywhere/a_smile_120x120._CB368246573_.png",
"https://images-na.ssl-images-amazon.com/images/G/01/anywhere/a_smile_114x114._CB368212020_.png",
"https://images-na.ssl-images-amazon.com/images/G/01/anywhere/a_smile_72x72._CB368212002_.png",
"https://images-na.ssl-images-amazon.com/images/G/01/anywhere/a_smile_57x57._CB368212015_.png"
),
urls
)
}
@Test
fun `compare twitter-com icons`() {
val resources = listOf(
IconRequest.Resource(
url = "https://abs.twimg.com/favicons/favicon.ico",
type = IconRequest.Resource.Type.FAVICON),
IconRequest.Resource(
url = "https://abs.twimg.com/responsive-web/web/icon-ios.8ea219d08eafdfa41.png",
type = IconRequest.Resource.Type.APPLE_TOUCH_ICON))
val urls = resources.sortedWith(IconResourceComparator).map { it.url }
assertEquals(
listOf(
"https://abs.twimg.com/responsive-web/web/icon-ios.8ea219d08eafdfa41.png",
"https://abs.twimg.com/favicons/favicon.ico"
),
urls
)
}
@Test
fun `compare github-com icons`() {
val resources = listOf(
IconRequest.Resource(
url = "https://github.githubassets.com/favicon.ico",
type = IconRequest.Resource.Type.FAVICON),
IconRequest.Resource(
url = "https://github.com/fluidicon.png",
type = IconRequest.Resource.Type.FLUID_ICON),
IconRequest.Resource(
url = "https://github.githubassets.com/images/modules/open_graph/github-logo.png",
type = IconRequest.Resource.Type.OPENGRAPH),
IconRequest.Resource(
url = "https://github.githubassets.com/images/modules/open_graph/github-mark.png",
type = IconRequest.Resource.Type.OPENGRAPH),
IconRequest.Resource(
url = "https://github.githubassets.com/images/modules/open_graph/github-octocat.png",
type = IconRequest.Resource.Type.OPENGRAPH))
val urls = resources.sortedWith(IconResourceComparator).map { it.url }
assertEquals(
listOf(
"https://github.githubassets.com/favicon.ico",
"https://github.com/fluidicon.png",
"https://github.githubassets.com/images/modules/open_graph/github-logo.png",
"https://github.githubassets.com/images/modules/open_graph/github-mark.png",
"https://github.githubassets.com/images/modules/open_graph/github-octocat.png"
),
urls
)
}
@Test
fun `compare theverge-com icons`() {
val resources = listOf(
IconRequest.Resource(
url = "https://cdn.vox-cdn.com/uploads/chorus_asset/file/7395367/favicon-16x16.0.png",
type = IconRequest.Resource.Type.FAVICON,
sizes = listOf(IconRequest.Resource.Size(16, 16))),
IconRequest.Resource(
url = "https://cdn.vox-cdn.com/uploads/chorus_asset/file/7395363/favicon-32x32.0.png",
type = IconRequest.Resource.Type.FAVICON,
sizes = listOf(IconRequest.Resource.Size(32, 32))),
IconRequest.Resource(
url = "https://cdn.vox-cdn.com/uploads/chorus_asset/file/7395365/favicon-96x96.0.png",
type = IconRequest.Resource.Type.FAVICON,
sizes = listOf(IconRequest.Resource.Size(96, 96))),
IconRequest.Resource(
url = "https://cdn.vox-cdn.com/uploads/chorus_asset/file/7395351/android-chrome-192x192.0.png",
type = IconRequest.Resource.Type.FAVICON,
sizes = listOf(IconRequest.Resource.Size(192, 192))),
IconRequest.Resource(
url = "https://cdn.vox-cdn.com/uploads/chorus_asset/file/7395361/favicon-64x64.0.ico",
type = IconRequest.Resource.Type.FAVICON),
IconRequest.Resource(
url = "https://cdn.vox-cdn.com/uploads/chorus_asset/file/7395359/ios-icon.0.png",
type = IconRequest.Resource.Type.APPLE_TOUCH_ICON,
sizes = listOf(IconRequest.Resource.Size(180, 180))),
IconRequest.Resource(
url = "https://cdn.vox-cdn.com/uploads/chorus_asset/file/9672633/VergeOG.0_1200x627.0.png",
type = IconRequest.Resource.Type.OPENGRAPH),
IconRequest.Resource(
url = "https://cdn.vox-cdn.com/community_logos/52803/VER_Logomark_175x92..png",
type = IconRequest.Resource.Type.TWITTER),
IconRequest.Resource(
url = "https://cdn.vox-cdn.com/uploads/chorus_asset/file/7396113/221a67c8-a10f-11e6-8fae-983107008690.0.png",
type = IconRequest.Resource.Type.MICROSOFT_TILE))
val urls = resources.sortedWith(IconResourceComparator).map { it.url }
assertEquals(
listOf(
"https://cdn.vox-cdn.com/uploads/chorus_asset/file/7395359/ios-icon.0.png",
"https://cdn.vox-cdn.com/uploads/chorus_asset/file/7395351/android-chrome-192x192.0.png",
"https://cdn.vox-cdn.com/uploads/chorus_asset/file/7395365/favicon-96x96.0.png",
"https://cdn.vox-cdn.com/uploads/chorus_asset/file/7395363/favicon-32x32.0.png",
"https://cdn.vox-cdn.com/uploads/chorus_asset/file/7395367/favicon-16x16.0.png",
"https://cdn.vox-cdn.com/uploads/chorus_asset/file/7395361/favicon-64x64.0.ico",
"https://cdn.vox-cdn.com/uploads/chorus_asset/file/9672633/VergeOG.0_1200x627.0.png",
"https://cdn.vox-cdn.com/community_logos/52803/VER_Logomark_175x92..png",
"https://cdn.vox-cdn.com/uploads/chorus_asset/file/7396113/221a67c8-a10f-11e6-8fae-983107008690.0.png"
),
urls
)
}
}
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