Commit d098d318 authored by Fredrik Söderquist's avatar Fredrik Söderquist Committed by Commit Bot

Simplify/improve ElementStyleResources "is sprite" heuristic

The HasBackgroundImage check is redundant because the helper function is
called within an equivalent check.
Also change so that current layer is checked, rather than always
checking the first (top-most) layer. Update the test to include a case
where doing this will help.
Rename the helper method to BackgroundLayerMayBeSprite since it's no
longer operating on a ComputedStyle.

Change-Id: Ibfabfc4a3c68d042ab356f43cb6031697d4daf98
Reviewed-on: https://chromium-review.googlesource.com/852255Reviewed-by: default avatarrajendrant <rajendrant@chromium.org>
Reviewed-by: default avatarScott Little <sclittle@chromium.org>
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#536998}
parent 60a76867
<!DOCTYPE html>
<style>
div {
width: 40px;
height: 40px;
}
</style>
<body>
<!-- This test verifies if CSS sprite image is shown, instead of placeholder image. -->
<div id="sprite1"></div>
......@@ -7,9 +13,9 @@
<div id="sprite4"></div>
<script>
window.internals.settings.setFetchImagePlaceholders(true);
document.getElementById("sprite1").style.cssText = 'width: 40px; height: 40px; background: url(https://127.0.0.1:8443/resources/square200.png) 0 0;';
document.getElementById("sprite2").style.cssText = 'width: 40px; height: 40px; background: url(https://127.0.0.1:8443/resources/square200.png) 0 10px;';
document.getElementById("sprite3").style.cssText = 'width: 40px; height: 40px; background: url(https://127.0.0.1:8443/resources/square200.png) 10px 0;';
document.getElementById("sprite4").style.cssText = 'width: 40px; height: 40px; background: url(https://127.0.0.1:8443/resources/square200.png) 10px 10px;';
document.getElementById("sprite1").style.cssText = 'background: url(https://127.0.0.1:8443/resources/square200.png) 0 0;';
document.getElementById("sprite2").style.cssText = 'background: url(https://127.0.0.1:8443/resources/square200.png) 0 10px;';
document.getElementById("sprite3").style.cssText = 'background: url(https://127.0.0.1:8443/resources/square200.png) 10px 0;';
document.getElementById("sprite4").style.cssText = 'background: none, url(https://127.0.0.1:8443/resources/square200.png) 10px 10px;';
</script>
</body>
......@@ -115,15 +115,14 @@ void ElementStyleResources::LoadPendingSVGDocuments(
}
}
static bool ComputedStyleMayBeCSSSpriteBackgroundImage(
const ComputedStyle& style) {
// Simple heuristic to guess if CSS background image is used to create CSS
// sprites. For a legit background image it's very likely the X and the Y
// position will not be explicitly specifed. For CSS sprite image,
static bool BackgroundLayerMayBeSprite(const FillLayer& background_layer) {
// Simple heuristic to guess if a CSS background image layer is used to
// create CSS sprites. For a legit background image it's very likely the X
// and the Y position will not be explicitly specifed. For CSS sprite image,
// background X or Y position will probably be specified.
const FillLayer& background = style.BackgroundLayers();
return style.HasBackgroundImage() &&
(background.PositionX().IsFixed() || background.PositionY().IsFixed());
DCHECK(background_layer.GetImage());
return background_layer.PositionX().IsFixed() ||
background_layer.PositionY().IsFixed();
}
StyleImage* ElementStyleResources::LoadPendingImage(
......@@ -186,7 +185,7 @@ void ElementStyleResources::LoadPendingImages(ComputedStyle* style) {
background_layer->GetImage()->IsPendingImage()) {
background_layer->SetImage(LoadPendingImage(
style, ToStylePendingImage(background_layer->GetImage()),
ComputedStyleMayBeCSSSpriteBackgroundImage(*style)
BackgroundLayerMayBeSprite(*background_layer)
? FetchParameters::kDisallowPlaceholder
: FetchParameters::kAllowPlaceholder));
}
......
Markdown is supported
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