Don't send a load event when a loaded image is actually the placeholder

The image cache returns an `ImageCacheResult::ImageAvailable `the second
time you try getting the placeholder. This means that in some cases, the
loading of an image would fail, then the same image would get fetched
from the cache, the placeholder would be loaded from that but would be
seen as a normal image, firing a load event.

This made the tests in
`fetch/cross-origin-resource-policy/image-loads.html` fail depending on
their order.
This commit is contained in:
Bastien Orivel 2020-05-08 15:36:55 +02:00
parent 8249be38c9
commit bdbfde9ec0
6 changed files with 40 additions and 15 deletions

View file

@ -163,7 +163,7 @@ impl<'a> LayoutContext<'a> {
}
match self.get_or_request_image_or_meta(node, url.clone(), use_placeholder) {
Some(ImageOrMetadataAvailable::ImageAvailable(image, _)) => {
Some(ImageOrMetadataAvailable::ImageAvailable { image, .. }) => {
let image_info = WebRenderImageInfo::from_image(&*image);
if image_info.key.is_none() {
Some(image_info)

View file

@ -431,8 +431,8 @@ impl ImageFragmentInfo {
layout_context
.get_or_request_image_or_meta(node.opaque(), url, UsePlaceholder::Yes)
.map(|result| match result {
ImageOrMetadataAvailable::ImageAvailable(i, _) => {
ImageOrMetadata::Image(i)
ImageOrMetadataAvailable::ImageAvailable { image, .. } => {
ImageOrMetadata::Image(image)
},
ImageOrMetadataAvailable::MetadataAvailable(m) => {
ImageOrMetadata::Metadata(m)

View file

@ -466,9 +466,12 @@ impl ImageCache for ImageCacheImpl {
match result {
Ok((image, image_url)) => {
debug!("{} is available", url);
return ImageCacheResult::Available(ImageOrMetadataAvailable::ImageAvailable(
image, image_url,
));
let is_placeholder = image_url == store.placeholder_url;
return ImageCacheResult::Available(ImageOrMetadataAvailable::ImageAvailable {
image,
url: image_url,
is_placeholder,
});
},
Err(()) => {
debug!("{} is not available", url);
@ -515,9 +518,14 @@ impl ImageCache for ImageCacheImpl {
// TODO: make this behaviour configurable according to the caller's needs.
store.handle_decoder(decoded);
match store.get_completed_image_if_available(url, origin, cors_setting, use_placeholder) {
Some(Ok((image, image_url))) => ImageCacheResult::Available(
ImageOrMetadataAvailable::ImageAvailable(image, image_url),
),
Some(Ok((image, image_url))) => {
let is_placeholder = image_url == store.placeholder_url;
ImageCacheResult::Available(ImageOrMetadataAvailable::ImageAvailable {
image,
url: image_url,
is_placeholder,
})
},
_ => ImageCacheResult::LoadError,
}
}

View file

@ -17,7 +17,12 @@ use std::sync::Arc;
/// Indicating either entire image or just metadata availability
#[derive(Clone, Debug, Deserialize, MallocSizeOf, Serialize)]
pub enum ImageOrMetadataAvailable {
ImageAvailable(#[ignore_malloc_size_of = "Arc"] Arc<Image>, ServoUrl),
ImageAvailable {
#[ignore_malloc_size_of = "Arc"]
image: Arc<Image>,
url: ServoUrl,
is_placeholder: bool,
},
MetadataAvailable(ImageMetadata),
}

View file

@ -329,8 +329,16 @@ impl HTMLImageElement {
);
match cache_result {
ImageCacheResult::Available(ImageOrMetadataAvailable::ImageAvailable(image, url)) => {
self.process_image_response(ImageResponse::Loaded(image, url))
ImageCacheResult::Available(ImageOrMetadataAvailable::ImageAvailable {
image,
url,
is_placeholder,
}) => {
if is_placeholder {
self.process_image_response(ImageResponse::PlaceholderLoaded(image, url))
} else {
self.process_image_response(ImageResponse::Loaded(image, url))
}
},
ImageCacheResult::Available(ImageOrMetadataAvailable::MetadataAvailable(m)) => {
self.process_image_response(ImageResponse::MetadataLoaded(m))
@ -1109,7 +1117,7 @@ impl HTMLImageElement {
);
match cache_result {
ImageCacheResult::Available(ImageOrMetadataAvailable::ImageAvailable(_image, _url)) => {
ImageCacheResult::Available(ImageOrMetadataAvailable::ImageAvailable { .. }) => {
// Step 15
self.finish_reacting_to_environment_change(
selected_source,

View file

@ -166,8 +166,12 @@ impl HTMLVideoElement {
);
match cache_result {
ImageCacheResult::Available(ImageOrMetadataAvailable::ImageAvailable(img, url)) => {
self.process_image_response(ImageResponse::Loaded(img, url));
ImageCacheResult::Available(ImageOrMetadataAvailable::ImageAvailable {
image,
url,
..
}) => {
self.process_image_response(ImageResponse::Loaded(image, url));
},
ImageCacheResult::ReadyForRequest(id) => {
self.do_fetch_poster_frame(poster_url, id, cancel_receiver)