Auto merge of #8382 - Ms2ger:SharedLayoutContext-sync, r=eefriedman

Remove the unsafe Sync implementation for SharedLayoutContext.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8382)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2015-11-07 16:19:43 +05:30
commit 36cd186608
7 changed files with 33 additions and 51 deletions

View file

@ -14,14 +14,14 @@ use script::layout_interface::Animation;
use script_traits::ConstellationControlMsg;
use std::collections::HashMap;
use std::collections::hash_map::Entry;
use std::sync::Arc;
use std::sync::mpsc::Sender;
use std::sync::{Arc, Mutex};
use style::animation::{GetMod, PropertyAnimation};
use style::properties::ComputedValues;
/// Inserts transitions into the queue of running animations as applicable for the given style
/// difference. This is called from the layout worker threads.
pub fn start_transitions_if_applicable(new_animations_sender: &Sender<Animation>,
pub fn start_transitions_if_applicable(new_animations_sender: &Mutex<Sender<Animation>>,
node: OpaqueNode,
old_style: &ComputedValues,
new_style: &mut ComputedValues) {
@ -37,7 +37,7 @@ pub fn start_transitions_if_applicable(new_animations_sender: &Sender<Animation>
let animation_style = new_style.get_animation();
let start_time =
now + (animation_style.transition_delay.0.get_mod(i).seconds() as f64);
new_animations_sender.send(Animation {
new_animations_sender.lock().unwrap().send(Animation {
node: node.id(),
property_animation: property_animation,
start_time: start_time,

View file

@ -16,17 +16,16 @@ use gfx::font_cache_task::FontCacheTask;
use gfx::font_context::FontContext;
use ipc_channel::ipc::{self, IpcSender};
use msg::compositor_msg::LayerId;
use msg::constellation_msg::ConstellationChan;
use net_traits::image::base::Image;
use net_traits::image_cache_task::{ImageCacheChan, ImageCacheTask, ImageResponse, ImageState};
use net_traits::image_cache_task::{UsePlaceholder};
use script::layout_interface::{Animation, LayoutChan, ReflowGoal};
use script::layout_interface::{Animation, ReflowGoal};
use std::cell::{RefCell, RefMut};
use std::collections::HashMap;
use std::collections::hash_state::DefaultState;
use std::rc::Rc;
use std::sync::Arc;
use std::sync::mpsc::{Sender, channel};
use std::sync::{Arc, Mutex};
use style::selector_matching::Stylist;
use url::Url;
use util::mem::HeapSizeOf;
@ -63,8 +62,9 @@ fn create_or_get_local_context(shared_layout_context: &SharedLayoutContext)
}
context
} else {
let font_cache_task = shared_layout_context.font_cache_task.lock().unwrap().clone();
let context = Rc::new(LocalLayoutContext {
font_context: RefCell::new(FontContext::new(shared_layout_context.font_cache_task.clone())),
font_context: RefCell::new(FontContext::new(font_cache_task)),
applicable_declarations_cache: RefCell::new(ApplicableDeclarationsCache::new()),
style_sharing_candidate_cache: RefCell::new(StyleSharingCandidateCache::new()),
});
@ -74,13 +74,19 @@ fn create_or_get_local_context(shared_layout_context: &SharedLayoutContext)
})
}
pub struct StylistWrapper(pub *const Stylist);
// FIXME(#6569) This implementation is unsound.
#[allow(unsafe_code)]
unsafe impl Sync for StylistWrapper {}
/// Layout information shared among all workers. This must be thread-safe.
pub struct SharedLayoutContext {
/// The shared image cache task.
pub image_cache_task: ImageCacheTask,
/// A channel for the image cache to send responses to.
pub image_cache_sender: ImageCacheChan,
pub image_cache_sender: Mutex<ImageCacheChan>,
/// The current viewport size.
pub viewport_size: Size2D<Au>,
@ -88,19 +94,13 @@ pub struct SharedLayoutContext {
/// Screen sized changed?
pub screen_size_changed: bool,
/// A channel up to the constellation.
pub constellation_chan: ConstellationChan,
/// A channel up to the layout task.
pub layout_chan: LayoutChan,
/// Interface to the font cache task.
pub font_cache_task: FontCacheTask,
pub font_cache_task: Mutex<FontCacheTask>,
/// The CSS selector stylist.
///
/// FIXME(#2604): Make this no longer an unsafe pointer once we have fast `RWArc`s.
pub stylist: *const Stylist,
pub stylist: StylistWrapper,
/// The URL.
pub url: Url,
@ -111,10 +111,10 @@ pub struct SharedLayoutContext {
/// A channel on which new animations that have been triggered by style recalculation can be
/// sent.
pub new_animations_sender: Sender<Animation>,
pub new_animations_sender: Mutex<Sender<Animation>>,
/// A channel to send canvas renderers to paint task, in order to correctly paint the layers
pub canvas_layers_sender: Sender<(LayerId, IpcSender<CanvasMsg>)>,
pub canvas_layers_sender: Mutex<Sender<(LayerId, IpcSender<CanvasMsg>)>>,
/// The visible rects for each layer, as reported to us by the compositor.
pub visible_rects: Arc<HashMap<LayerId, Rect<Au>, DefaultState<FnvHasher>>>,
@ -126,18 +126,6 @@ pub struct SharedLayoutContext {
pub goal: ReflowGoal,
}
// FIXME(#6569) This implementations is unsound:
// XXX UNSOUND!!! for image_cache_task
// XXX UNSOUND!!! for image_cache_sender
// XXX UNSOUND!!! for constellation_chan
// XXX UNSOUND!!! for layout_chan
// XXX UNSOUND!!! for font_cache_task
// XXX UNSOUND!!! for stylist
// XXX UNSOUND!!! for new_animations_sender
// XXX UNSOUND!!! for canvas_layers_sender
#[allow(unsafe_code)]
unsafe impl Sync for SharedLayoutContext {}
pub struct LayoutContext<'a> {
pub shared: &'a SharedLayoutContext,
cached_local_layout_context: Rc<LocalLayoutContext>,
@ -200,8 +188,8 @@ impl<'a> LayoutContext<'a> {
}
// Not yet requested, async mode - request image from the cache
(ImageState::NotRequested, false) => {
self.shared.image_cache_task
.request_image(url, self.shared.image_cache_sender.clone(), None);
let sender = self.shared.image_cache_sender.lock().unwrap().clone();
self.shared.image_cache_task.request_image(url, sender, None);
None
}
// Image has been requested, is still pending. Return no image

View file

@ -21,8 +21,8 @@ use smallvec::SmallVec;
use std::borrow::ToOwned;
use std::hash::{Hash, Hasher};
use std::slice::Iter;
use std::sync::Arc;
use std::sync::mpsc::Sender;
use std::sync::{Arc, Mutex};
use string_cache::{Atom, Namespace};
use style::node::TElementAttributes;
use style::properties::{ComputedValues, PropertyDeclaration, cascade};
@ -406,7 +406,7 @@ pub trait MatchMethods {
parent: Option<LayoutNode>,
applicable_declarations: &ApplicableDeclarations,
applicable_declarations_cache: &mut ApplicableDeclarationsCache,
new_animations_sender: &Sender<Animation>);
new_animations_sender: &Mutex<Sender<Animation>>);
}
trait PrivateMatchMethods {
@ -417,7 +417,7 @@ trait PrivateMatchMethods {
style: &mut Option<Arc<ComputedValues>>,
applicable_declarations_cache:
&mut ApplicableDeclarationsCache,
new_animations_sender: &Sender<Animation>,
new_animations_sender: &Mutex<Sender<Animation>>,
shareable: bool,
animate_properties: bool)
-> RestyleDamage;
@ -438,7 +438,7 @@ impl<'ln> PrivateMatchMethods for LayoutNode<'ln> {
style: &mut Option<Arc<ComputedValues>>,
applicable_declarations_cache:
&mut ApplicableDeclarationsCache,
new_animations_sender: &Sender<Animation>,
new_animations_sender: &Mutex<Sender<Animation>>,
shareable: bool,
animate_properties: bool)
-> RestyleDamage {
@ -655,7 +655,7 @@ impl<'ln> MatchMethods for LayoutNode<'ln> {
parent: Option<LayoutNode>,
applicable_declarations: &ApplicableDeclarations,
applicable_declarations_cache: &mut ApplicableDeclarationsCache,
new_animations_sender: &Sender<Animation>) {
new_animations_sender: &Mutex<Sender<Animation>>) {
// Get our parent's style. This must be unsafe so that we don't touch the parent's
// borrow flags.
//

View file

@ -1150,7 +1150,7 @@ impl FragmentDisplayListBuilding for Fragment {
let data = receiver.recv().unwrap();
// Propagate the layer and the renderer to the paint task.
layout_context.shared.canvas_layers_sender.send(
layout_context.shared.canvas_layers_sender.lock().unwrap().send(
(layer_id, (*ipc_renderer).clone())).unwrap();
data

View file

@ -38,7 +38,6 @@ use incremental::{self, RECONSTRUCT_FLOW, REFLOW, REFLOW_OUT_OF_FLOW, RestyleDam
use inline::InlineFlow;
use model::{CollapsibleMargins, IntrinsicISizes, MarginCollapseInfo};
use msg::compositor_msg::{LayerId, LayerType};
use msg::constellation_msg::ConstellationChan;
use multicol::MulticolFlow;
use parallel::FlowParallelInfo;
use rustc_serialize::{Encodable, Encoder};
@ -363,9 +362,6 @@ pub trait Flow: fmt::Debug + Sync + Send + 'static {
/// Attempts to perform incremental fixup of this flow by replacing its fragment's style with
/// the new style. This can only succeed if the flow has exactly one fragment.
fn repair_style(&mut self, new_style: &Arc<ComputedValues>);
/// Remove any compositor layers associated with this flow
fn remove_compositor_layers(&self, _: ConstellationChan) {}
}
// Base access

View file

@ -12,7 +12,7 @@ use app_units::Au;
use azure::azure::AzColor;
use canvas_traits::CanvasMsg;
use construct::ConstructionResult;
use context::{SharedLayoutContext, heap_size_of_local_context};
use context::{SharedLayoutContext, StylistWrapper, heap_size_of_local_context};
use cssparser::ToCss;
use data::LayoutDataWrapper;
use display_list_builder::ToGfxColor;
@ -452,18 +452,16 @@ impl LayoutTask {
-> SharedLayoutContext {
SharedLayoutContext {
image_cache_task: rw_data.image_cache_task.clone(),
image_cache_sender: self.image_cache_sender.clone(),
image_cache_sender: Mutex::new(self.image_cache_sender.clone()),
viewport_size: rw_data.viewport_size.clone(),
screen_size_changed: screen_size_changed,
constellation_chan: rw_data.constellation_chan.clone(),
layout_chan: self.chan.clone(),
font_cache_task: self.font_cache_task.clone(),
canvas_layers_sender: self.canvas_layers_sender.clone(),
stylist: &*rw_data.stylist,
font_cache_task: Mutex::new(self.font_cache_task.clone()),
canvas_layers_sender: Mutex::new(self.canvas_layers_sender.clone()),
stylist: StylistWrapper(&*rw_data.stylist),
url: (*url).clone(),
visible_rects: rw_data.visible_rects.clone(),
generation: rw_data.generation,
new_animations_sender: rw_data.new_animations_sender.clone(),
new_animations_sender: Mutex::new(rw_data.new_animations_sender.clone()),
goal: goal,
running_animations: rw_data.running_animations.clone(),
}

View file

@ -188,7 +188,7 @@ impl<'a> PreorderDomTraversal for RecalcStyleForNode<'a> {
let shareable_element = match node.as_element() {
Some(element) => {
// Perform the CSS selector matching.
let stylist = unsafe { &*self.layout_context.shared.stylist };
let stylist = unsafe { &*self.layout_context.shared.stylist.0 };
if element.match_element(stylist,
Some(&*bf),
&mut applicable_declarations) {