Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow using CORS filtered image responses as WebGL textures #24340

Merged
merged 6 commits into from Oct 8, 2019
Next

Double key image cache by requesting origin, and store CORS status wi…

…th cached images.
  • Loading branch information
jdm committed Oct 4, 2019
commit 81a67aed9e0cc866f4689e8d7a5541198d75e445
@@ -22,6 +22,7 @@ use ipc_channel::ipc;
use libc::c_void;
use msg::constellation_msg::{PipelineId, PipelineIndex, PipelineNamespaceId};
use net_traits::image::base::Image;
use net_traits::image_cache::CorsStatus;
use num_traits::FromPrimitive;
#[cfg(feature = "gl")]
use pixels::PixelFormat;
@@ -1384,6 +1385,7 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
format: PixelFormat::RGB8,
bytes: ipc::IpcSharedMemory::from_bytes(&*img),
id: None,
cors_status: CorsStatus::Safe,
})
},
#[cfg(feature = "gl")]
@@ -18,7 +18,7 @@ use script_layout_interface::{PendingImage, PendingImageState};
use script_traits::Painter;
use script_traits::UntrustedNodeAddress;
use servo_atoms::Atom;
use servo_url::ServoUrl;
use servo_url::{ImmutableOrigin, ServoUrl};
use std::cell::{RefCell, RefMut};
use std::collections::HashMap;
use std::hash::BuildHasherDefault;
@@ -60,6 +60,9 @@ pub struct LayoutContext<'a> {
/// The pipeline id of this LayoutContext.
pub id: PipelineId,

/// The origin of this layout context.
pub origin: ImmutableOrigin,

/// Bits shared by the layout and style system.
pub style_context: SharedStyleContext<'a>,

@@ -120,9 +123,12 @@ impl<'a> LayoutContext<'a> {
};

// See if the image is already available
let result =
self.image_cache
.find_image_or_metadata(url.clone(), use_placeholder, can_request);
let result = self.image_cache.find_image_or_metadata(
url.clone(),
self.origin.clone(),
use_placeholder,
can_request,
);
match result {
Ok(image_or_metadata) => Some(image_or_metadata),
// Image failed to load, so just return nothing
@@ -659,6 +659,7 @@ impl LayoutThread {

LayoutContext {
id: self.id,
origin: self.url.origin(),
style_context: SharedStyleContext {
stylist: &self.stylist,
options: GLOBAL_STYLE_DATA.options.clone(),
@@ -5,12 +5,12 @@
use embedder_traits::resources::{self, Resource};
use immeta::load_from_buf;
use net_traits::image::base::{load_from_memory, Image, ImageMetadata};
use net_traits::image_cache::{CanRequestImages, ImageCache, ImageResponder};
use net_traits::image_cache::{CanRequestImages, CorsStatus, ImageCache, ImageResponder};
use net_traits::image_cache::{ImageOrMetadataAvailable, ImageResponse, ImageState};
use net_traits::image_cache::{PendingImageId, UsePlaceholder};
use net_traits::{FetchMetadata, FetchResponseMsg, NetworkError};
use net_traits::{FetchMetadata, FetchResponseMsg, FilteredMetadata, NetworkError};
use pixels::PixelFormat;
use servo_url::ServoUrl;
use servo_url::{ImmutableOrigin, ServoUrl};
use std::collections::hash_map::Entry::{Occupied, Vacant};
use std::collections::HashMap;
use std::io;
@@ -33,8 +33,8 @@ use webrender_api::units::DeviceIntSize;
// Helper functions.
// ======================================================================

fn decode_bytes_sync(key: LoadKey, bytes: &[u8]) -> DecoderMsg {
let image = load_from_memory(bytes);
fn decode_bytes_sync(key: LoadKey, bytes: &[u8], cors: CorsStatus) -> DecoderMsg {
let image = load_from_memory(bytes, cors);
DecoderMsg {
key: key,
image: image,
@@ -45,7 +45,7 @@ fn get_placeholder_image(
webrender_api: &webrender_api::RenderApi,
data: &[u8],
) -> io::Result<Arc<Image>> {
let mut image = load_from_memory(&data).unwrap();
let mut image = load_from_memory(&data, CorsStatus::Unsafe).unwrap();
set_webrender_image_key(webrender_api, &mut image);
Ok(Arc::new(image))
}
@@ -99,9 +99,9 @@ struct AllPendingLoads {
// for performance reasons.
loads: HashMap<LoadKey, PendingLoad>,

// Get a load key from its url. Used ony when starting and
// Get a load key from its url and requesting origin. Used ony when starting and
// finishing a load or when adding a new listener.
url_to_load_key: HashMap<ServoUrl, LoadKey>,
url_to_load_key: HashMap<(ServoUrl, ImmutableOrigin), LoadKey>,

// A counter used to generate instances of LoadKey
keygen: LoadKeyGenerator,
@@ -123,17 +123,20 @@ impl AllPendingLoads {

fn remove(&mut self, key: &LoadKey) -> Option<PendingLoad> {
self.loads.remove(key).and_then(|pending_load| {
self.url_to_load_key.remove(&pending_load.url).unwrap();
self.url_to_load_key
.remove(&(pending_load.url.clone(), pending_load.load_origin.clone()))
.unwrap();
Some(pending_load)
})
}

fn get_cached<'a>(
&'a mut self,
url: ServoUrl,
origin: ImmutableOrigin,
can_request: CanRequestImages,
) -> CacheResult<'a> {
match self.url_to_load_key.entry(url.clone()) {
match self.url_to_load_key.entry((url.clone(), origin.clone())) {
Occupied(url_entry) => {
let load_key = url_entry.get();
CacheResult::Hit(*load_key, self.loads.get_mut(load_key).unwrap())
@@ -146,7 +149,7 @@ impl AllPendingLoads {
let load_key = self.keygen.next();
url_entry.insert(load_key);

let pending_load = PendingLoad::new(url);
let pending_load = PendingLoad::new(url, origin);
match self.loads.entry(load_key) {
Occupied(_) => unreachable!(),
Vacant(load_entry) => {
@@ -251,33 +254,44 @@ enum LoadResult {
/// Represents an image that is either being loaded
/// by the resource thread, or decoded by a worker thread.
struct PendingLoad {
// The bytes loaded so far. Reset to an empty vector once loading
// is complete and the buffer has been transmitted to the decoder.
/// The bytes loaded so far. Reset to an empty vector once loading
/// is complete and the buffer has been transmitted to the decoder.
bytes: ImageBytes,

// Image metadata, if available.
/// Image metadata, if available.
metadata: Option<ImageMetadata>,

// Once loading is complete, the result of the operation.
/// Once loading is complete, the result of the operation.
result: Option<Result<(), NetworkError>>,

/// The listeners that are waiting for this response to complete.
listeners: Vec<ImageResponder>,

// The url being loaded. Do not forget that this may be several Mb
// if we are loading a data: url.
/// The url being loaded. Do not forget that this may be several Mb
/// if we are loading a data: url.
url: ServoUrl,

/// The origin that requested this load.
load_origin: ImmutableOrigin,

/// The CORS status of this image response.
cors_status: CorsStatus,

/// The URL of the final response that contains a body.
final_url: Option<ServoUrl>,
}

impl PendingLoad {
fn new(url: ServoUrl) -> PendingLoad {
fn new(url: ServoUrl, load_origin: ImmutableOrigin) -> PendingLoad {
PendingLoad {
bytes: ImageBytes::InProgress(vec![]),
metadata: None,
result: None,
listeners: vec![],
url: url,
load_origin,
final_url: None,
cors_status: CorsStatus::Unsafe,
}
}

@@ -294,7 +308,7 @@ struct ImageCacheStore {
pending_loads: AllPendingLoads,

// Images that have finished loading (successful or not)
completed_loads: HashMap<ServoUrl, CompletedLoad>,
completed_loads: HashMap<(ServoUrl, ImmutableOrigin), CompletedLoad>,

// The placeholder image used when an image fails to load
placeholder_image: Option<Arc<Image>>,
@@ -331,8 +345,10 @@ impl ImageCacheStore {
};

let completed_load = CompletedLoad::new(image_response.clone(), key);
self.completed_loads
.insert(pending_load.url.into(), completed_load);
self.completed_loads.insert(
(pending_load.url.into(), pending_load.load_origin),
completed_load,
);

for listener in pending_load.listeners {
listener.respond(image_response.clone());
@@ -343,20 +359,27 @@ impl ImageCacheStore {
/// or the complete load is not fully decoded or is unavailable.
fn get_completed_image_if_available(
&self,
url: &ServoUrl,
url: ServoUrl,
origin: ImmutableOrigin,
placeholder: UsePlaceholder,
) -> Option<Result<ImageOrMetadataAvailable, ImageState>> {
self.completed_loads.get(url).map(|completed_load| {
match (&completed_load.image_response, placeholder) {
(&ImageResponse::Loaded(ref image, ref url), _) |
(&ImageResponse::PlaceholderLoaded(ref image, ref url), UsePlaceholder::Yes) => Ok(
ImageOrMetadataAvailable::ImageAvailable(image.clone(), url.clone()),
),
(&ImageResponse::PlaceholderLoaded(_, _), UsePlaceholder::No) |
(&ImageResponse::None, _) |
(&ImageResponse::MetadataLoaded(_), _) => Err(ImageState::LoadError),
}
})
self.completed_loads
.get(&(url, origin))
.map(
|completed_load| match (&completed_load.image_response, placeholder) {
(&ImageResponse::Loaded(ref image, ref url), _) |
(
&ImageResponse::PlaceholderLoaded(ref image, ref url),
UsePlaceholder::Yes,
) => Ok(ImageOrMetadataAvailable::ImageAvailable(
image.clone(),
url.clone(),
)),
(&ImageResponse::PlaceholderLoaded(_, _), UsePlaceholder::No) |
(&ImageResponse::None, _) |
(&ImageResponse::MetadataLoaded(_), _) => Err(ImageState::LoadError),
},
)
}

/// Handle a message from one of the decoder worker threads or from a sync
@@ -397,23 +420,28 @@ impl ImageCache for ImageCacheImpl {
fn find_image_or_metadata(
&self,
url: ServoUrl,
origin: ImmutableOrigin,
use_placeholder: UsePlaceholder,
can_request: CanRequestImages,
) -> Result<ImageOrMetadataAvailable, ImageState> {
debug!("Find image or metadata for {}", url);
debug!("Find image or metadata for {} ({:?})", url, origin);
let mut store = self.store.lock().unwrap();
if let Some(result) = store.get_completed_image_if_available(&url, use_placeholder) {
if let Some(result) =
store.get_completed_image_if_available(url.clone(), origin.clone(), use_placeholder)
{
debug!("{} is available", url);
return result;
}

let decoded = {
let result = store.pending_loads.get_cached(url.clone(), can_request);
let result = store
.pending_loads
.get_cached(url.clone(), origin.clone(), can_request);
match result {
CacheResult::Hit(key, pl) => match (&pl.result, &pl.metadata) {
(&Some(Ok(_)), _) => {
debug!("Sync decoding {} ({:?})", url, key);
decode_bytes_sync(key, &pl.bytes.as_slice())
decode_bytes_sync(key, &pl.bytes.as_slice(), pl.cors_status)
},
(&None, &Some(ref meta)) => {
debug!("Metadata available for {} ({:?})", url, key);
@@ -440,7 +468,7 @@ impl ImageCache for ImageCacheImpl {
// and ignore the async decode when it finishes later.
// TODO: make this behaviour configurable according to the caller's needs.
store.handle_decoder(decoded);
match store.get_completed_image_if_available(&url, use_placeholder) {
match store.get_completed_image_if_available(url, origin, use_placeholder) {
Some(result) => result,
None => Err(ImageState::LoadError),
}
@@ -472,15 +500,26 @@ impl ImageCache for ImageCacheImpl {
(FetchResponseMsg::ProcessResponse(response), _) => {
let mut store = self.store.lock().unwrap();
let pending_load = store.pending_loads.get_by_key_mut(&id).unwrap();
let metadata = match response {
Ok(meta) => Some(match meta {
FetchMetadata::Unfiltered(m) => m,
FetchMetadata::Filtered { unsafe_, .. } => unsafe_,
}),
Err(_) => None,
let (cors_status, metadata) = match response {
Ok(meta) => match meta {
FetchMetadata::Unfiltered(m) => (CorsStatus::Safe, Some(m)),
FetchMetadata::Filtered { unsafe_, filtered } => (
match filtered {
This conversation was marked as resolved by jdm

This comment has been minimized.

Copy link
@Manishearth

Manishearth Oct 3, 2019

Member

could this branch be placed within braces? it's tricky to read because there are trailing commas meaning two different things here, i was very confused by there being what appeared to be a match arm without a pattern

This comment has been minimized.

Copy link
@jdm

jdm Oct 3, 2019

Author Member

Can you give an example of what you mean?

This comment has been minimized.

Copy link
@Manishearth

Manishearth Oct 3, 2019

Member

The Some(unsafe_) line specifically looks like a new match arm

This comment has been minimized.

Copy link
@Manishearth

Manishearth Oct 3, 2019

Member

I'm basically suggesting

FetchMetadata::Filtered { unsafe_, filtered } => {
    (
        match filtered {...},
        Some(...)
    )
}

This comment has been minimized.

Copy link
@jdm

jdm Oct 3, 2019

Author Member

Rustfmt is unhappy with your suggestion and just reverts it.

FilteredMetadata::Basic(_) | FilteredMetadata::Cors(_) => {
CorsStatus::Safe
},
FilteredMetadata::Opaque | FilteredMetadata::OpaqueRedirect => {
CorsStatus::Unsafe
},
},
Some(unsafe_),
),
},
Err(_) => (CorsStatus::Unsafe, None),
};
let final_url = metadata.as_ref().map(|m| m.final_url.clone());
pending_load.final_url = final_url;
pending_load.cors_status = cors_status;
},
(FetchResponseMsg::ProcessResponseChunk(data), _) => {
debug!("Got some data for {:?}", id);
@@ -506,17 +545,17 @@ impl ImageCache for ImageCacheImpl {
debug!("Received EOF for {:?}", key);
match result {
Ok(_) => {
let bytes = {
let (bytes, cors_status) = {
let mut store = self.store.lock().unwrap();
let pending_load = store.pending_loads.get_by_key_mut(&id).unwrap();
pending_load.result = Some(Ok(()));
debug!("Async decoding {} ({:?})", pending_load.url, key);
pending_load.bytes.mark_complete()
(pending_load.bytes.mark_complete(), pending_load.cors_status)
};

let local_store = self.store.clone();
thread::spawn(move || {
let msg = decode_bytes_sync(key, &*bytes);
let msg = decode_bytes_sync(key, &*bytes, cors_status);
debug!("Image decoded");
local_store.lock().unwrap().handle_decoder(msg);
});
@@ -2,6 +2,7 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */

use crate::image_cache::CorsStatus;
use ipc_channel::ipc::IpcSharedMemory;
use piston_image::{DynamicImage, ImageFormat};
use pixels::PixelFormat;
@@ -16,6 +17,7 @@ pub struct Image {
pub bytes: IpcSharedMemory,
#[ignore_malloc_size_of = "Defined in webrender_api"]
pub id: Option<webrender_api::ImageKey>,
pub cors_status: CorsStatus,
}

impl fmt::Debug for Image {
@@ -37,7 +39,7 @@ pub struct ImageMetadata {
// FIXME: Images must not be copied every frame. Instead we should atomically
// reference count them.

pub fn load_from_memory(buffer: &[u8]) -> Option<Image> {
pub fn load_from_memory(buffer: &[u8], cors_status: CorsStatus) -> Option<Image> {
if buffer.is_empty() {
return None;
}
@@ -61,6 +63,7 @@ pub fn load_from_memory(buffer: &[u8]) -> Option<Image> {
format: PixelFormat::BGRA8,
bytes: IpcSharedMemory::from_bytes(&*rgba),
id: None,
cors_status,
})
},
Err(e) => {
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.