From 1c11bd09dfc64b2aab5f47c12c9c3cb1f6c211d8 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Fri, 6 Nov 2015 21:41:04 +0100 Subject: [PATCH 1/9] Remove unused SharedLayoutContext::layout_chan. --- components/layout/context.rs | 6 +----- components/layout/layout_task.rs | 1 - 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/components/layout/context.rs b/components/layout/context.rs index f3db3d80a01..66c919e7e44 100644 --- a/components/layout/context.rs +++ b/components/layout/context.rs @@ -20,7 +20,7 @@ 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; @@ -91,9 +91,6 @@ pub struct SharedLayoutContext { /// 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, @@ -130,7 +127,6 @@ pub struct SharedLayoutContext { // 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 diff --git a/components/layout/layout_task.rs b/components/layout/layout_task.rs index b72bdbb23bc..73393b2894f 100644 --- a/components/layout/layout_task.rs +++ b/components/layout/layout_task.rs @@ -456,7 +456,6 @@ impl LayoutTask { 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, From bcd541c02c7aa9fa1bb14090cde3180dc7dc016b Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Fri, 6 Nov 2015 21:55:47 +0100 Subject: [PATCH 2/9] Wrap SharedLayoutContext::image_cache_sender in a Mutex. --- components/layout/context.rs | 9 ++++----- components/layout/layout_task.rs | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/components/layout/context.rs b/components/layout/context.rs index 66c919e7e44..283074ae3d9 100644 --- a/components/layout/context.rs +++ b/components/layout/context.rs @@ -25,8 +25,8 @@ 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; @@ -80,7 +80,7 @@ pub struct SharedLayoutContext { 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, /// The current viewport size. pub viewport_size: Size2D, @@ -125,7 +125,6 @@ pub struct SharedLayoutContext { // 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 font_cache_task // XXX UNSOUND!!! for stylist @@ -196,8 +195,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 diff --git a/components/layout/layout_task.rs b/components/layout/layout_task.rs index 73393b2894f..71c6ac48d4d 100644 --- a/components/layout/layout_task.rs +++ b/components/layout/layout_task.rs @@ -452,7 +452,7 @@ 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(), From 32bb4044fd51f5e39e39f9cc21f4e64907ebee69 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Fri, 6 Nov 2015 22:38:25 +0100 Subject: [PATCH 3/9] Remove unused SharedLayoutContext::constellation_chan. --- components/layout/context.rs | 5 ----- components/layout/layout_task.rs | 1 - 2 files changed, 6 deletions(-) diff --git a/components/layout/context.rs b/components/layout/context.rs index 283074ae3d9..e7e0ab070b8 100644 --- a/components/layout/context.rs +++ b/components/layout/context.rs @@ -16,7 +16,6 @@ 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}; @@ -88,9 +87,6 @@ pub struct SharedLayoutContext { /// Screen sized changed? pub screen_size_changed: bool, - /// A channel up to the constellation. - pub constellation_chan: ConstellationChan, - /// Interface to the font cache task. pub font_cache_task: FontCacheTask, @@ -125,7 +121,6 @@ pub struct SharedLayoutContext { // FIXME(#6569) This implementations is unsound: // XXX UNSOUND!!! for image_cache_task -// XXX UNSOUND!!! for constellation_chan // XXX UNSOUND!!! for font_cache_task // XXX UNSOUND!!! for stylist // XXX UNSOUND!!! for new_animations_sender diff --git a/components/layout/layout_task.rs b/components/layout/layout_task.rs index 71c6ac48d4d..58e4a6f5639 100644 --- a/components/layout/layout_task.rs +++ b/components/layout/layout_task.rs @@ -455,7 +455,6 @@ impl LayoutTask { 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(), font_cache_task: self.font_cache_task.clone(), canvas_layers_sender: self.canvas_layers_sender.clone(), stylist: &*rw_data.stylist, From 11e760d582a1302651b9eb5f8ad277ce7ec72a9d Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Fri, 6 Nov 2015 22:40:20 +0100 Subject: [PATCH 4/9] Remove unused Flow::remove_compositor_layers. The caller was removed in c72d0c2ed04f6c5f9fa22a3f576e1df5878f941d. --- components/layout/flow.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/components/layout/flow.rs b/components/layout/flow.rs index 0396737822a..6902341edae 100644 --- a/components/layout/flow.rs +++ b/components/layout/flow.rs @@ -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); - - /// Remove any compositor layers associated with this flow - fn remove_compositor_layers(&self, _: ConstellationChan) {} } // Base access From 552a03fde66b5869d77892d52da4f2a3df3f4cdf Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Fri, 6 Nov 2015 23:01:18 +0100 Subject: [PATCH 5/9] Wrap SharedLayoutContext::font_cache_task in a Mutex. --- components/layout/context.rs | 6 +++--- components/layout/layout_task.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/components/layout/context.rs b/components/layout/context.rs index e7e0ab070b8..5a7bbdfed8f 100644 --- a/components/layout/context.rs +++ b/components/layout/context.rs @@ -62,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()), }); @@ -88,7 +89,7 @@ pub struct SharedLayoutContext { pub screen_size_changed: bool, /// Interface to the font cache task. - pub font_cache_task: FontCacheTask, + pub font_cache_task: Mutex, /// The CSS selector stylist. /// @@ -121,7 +122,6 @@ pub struct SharedLayoutContext { // FIXME(#6569) This implementations is unsound: // XXX UNSOUND!!! for image_cache_task -// XXX UNSOUND!!! for font_cache_task // XXX UNSOUND!!! for stylist // XXX UNSOUND!!! for new_animations_sender // XXX UNSOUND!!! for canvas_layers_sender diff --git a/components/layout/layout_task.rs b/components/layout/layout_task.rs index 58e4a6f5639..edf408cf937 100644 --- a/components/layout/layout_task.rs +++ b/components/layout/layout_task.rs @@ -455,7 +455,7 @@ impl LayoutTask { image_cache_sender: Mutex::new(self.image_cache_sender.clone()), viewport_size: rw_data.viewport_size.clone(), screen_size_changed: screen_size_changed, - font_cache_task: self.font_cache_task.clone(), + font_cache_task: Mutex::new(self.font_cache_task.clone()), canvas_layers_sender: self.canvas_layers_sender.clone(), stylist: &*rw_data.stylist, url: (*url).clone(), From 8fc75704c972453804d8ec69184f584cd3313dc3 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Fri, 6 Nov 2015 23:01:38 +0100 Subject: [PATCH 6/9] Wrap SharedLayoutContext::new_animations_sender in a Mutex. --- components/layout/animation.rs | 6 +++--- components/layout/context.rs | 3 +-- components/layout/css/matching.rs | 10 +++++----- components/layout/layout_task.rs | 2 +- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/components/layout/animation.rs b/components/layout/animation.rs index 27bf118bfda..7b58e435681 100644 --- a/components/layout/animation.rs +++ b/components/layout/animation.rs @@ -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, +pub fn start_transitions_if_applicable(new_animations_sender: &Mutex>, node: OpaqueNode, old_style: &ComputedValues, new_style: &mut ComputedValues) { @@ -37,7 +37,7 @@ pub fn start_transitions_if_applicable(new_animations_sender: &Sender 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, diff --git a/components/layout/context.rs b/components/layout/context.rs index 5a7bbdfed8f..eb5c19cc596 100644 --- a/components/layout/context.rs +++ b/components/layout/context.rs @@ -105,7 +105,7 @@ pub struct SharedLayoutContext { /// A channel on which new animations that have been triggered by style recalculation can be /// sent. - pub new_animations_sender: Sender, + pub new_animations_sender: Mutex>, /// A channel to send canvas renderers to paint task, in order to correctly paint the layers pub canvas_layers_sender: Sender<(LayerId, IpcSender)>, @@ -123,7 +123,6 @@ pub struct SharedLayoutContext { // FIXME(#6569) This implementations is unsound: // XXX UNSOUND!!! for image_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 {} diff --git a/components/layout/css/matching.rs b/components/layout/css/matching.rs index 456f8cbe88f..d0260fa5c98 100644 --- a/components/layout/css/matching.rs +++ b/components/layout/css/matching.rs @@ -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, applicable_declarations: &ApplicableDeclarations, applicable_declarations_cache: &mut ApplicableDeclarationsCache, - new_animations_sender: &Sender); + new_animations_sender: &Mutex>); } trait PrivateMatchMethods { @@ -417,7 +417,7 @@ trait PrivateMatchMethods { style: &mut Option>, applicable_declarations_cache: &mut ApplicableDeclarationsCache, - new_animations_sender: &Sender, + new_animations_sender: &Mutex>, shareable: bool, animate_properties: bool) -> RestyleDamage; @@ -438,7 +438,7 @@ impl<'ln> PrivateMatchMethods for LayoutNode<'ln> { style: &mut Option>, applicable_declarations_cache: &mut ApplicableDeclarationsCache, - new_animations_sender: &Sender, + new_animations_sender: &Mutex>, shareable: bool, animate_properties: bool) -> RestyleDamage { @@ -655,7 +655,7 @@ impl<'ln> MatchMethods for LayoutNode<'ln> { parent: Option, applicable_declarations: &ApplicableDeclarations, applicable_declarations_cache: &mut ApplicableDeclarationsCache, - new_animations_sender: &Sender) { + new_animations_sender: &Mutex>) { // Get our parent's style. This must be unsafe so that we don't touch the parent's // borrow flags. // diff --git a/components/layout/layout_task.rs b/components/layout/layout_task.rs index edf408cf937..604a0743d34 100644 --- a/components/layout/layout_task.rs +++ b/components/layout/layout_task.rs @@ -461,7 +461,7 @@ impl LayoutTask { 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(), } From e9b77628ce1caf4c9bb77d98c95fe7ff71dddb32 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Fri, 6 Nov 2015 23:08:05 +0100 Subject: [PATCH 7/9] Wrap SharedLayoutContext::canvas_layers_sender in a Mutex. --- components/layout/context.rs | 3 +-- components/layout/display_list_builder.rs | 2 +- components/layout/layout_task.rs | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/components/layout/context.rs b/components/layout/context.rs index eb5c19cc596..86d98e8a4fa 100644 --- a/components/layout/context.rs +++ b/components/layout/context.rs @@ -108,7 +108,7 @@ pub struct SharedLayoutContext { pub new_animations_sender: Mutex>, /// A channel to send canvas renderers to paint task, in order to correctly paint the layers - pub canvas_layers_sender: Sender<(LayerId, IpcSender)>, + pub canvas_layers_sender: Mutex)>>, /// The visible rects for each layer, as reported to us by the compositor. pub visible_rects: Arc, DefaultState>>, @@ -123,7 +123,6 @@ pub struct SharedLayoutContext { // FIXME(#6569) This implementations is unsound: // XXX UNSOUND!!! for image_cache_task // XXX UNSOUND!!! for stylist -// XXX UNSOUND!!! for canvas_layers_sender #[allow(unsafe_code)] unsafe impl Sync for SharedLayoutContext {} diff --git a/components/layout/display_list_builder.rs b/components/layout/display_list_builder.rs index 907bb5b129d..52c481f504e 100644 --- a/components/layout/display_list_builder.rs +++ b/components/layout/display_list_builder.rs @@ -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 diff --git a/components/layout/layout_task.rs b/components/layout/layout_task.rs index 604a0743d34..d39f6c06a7a 100644 --- a/components/layout/layout_task.rs +++ b/components/layout/layout_task.rs @@ -456,7 +456,7 @@ impl LayoutTask { viewport_size: rw_data.viewport_size.clone(), screen_size_changed: screen_size_changed, font_cache_task: Mutex::new(self.font_cache_task.clone()), - canvas_layers_sender: self.canvas_layers_sender.clone(), + canvas_layers_sender: Mutex::new(self.canvas_layers_sender.clone()), stylist: &*rw_data.stylist, url: (*url).clone(), visible_rects: rw_data.visible_rects.clone(), From 647232a495ddeaa403ea3a846ff7699a3d04266b Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Fri, 6 Nov 2015 23:15:41 +0100 Subject: [PATCH 8/9] Wrap SharedLayoutContext::stylist in a wrapper to make it Sync. --- components/layout/context.rs | 9 +++++++-- components/layout/layout_task.rs | 4 ++-- components/layout/traversal.rs | 2 +- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/components/layout/context.rs b/components/layout/context.rs index 86d98e8a4fa..3d5196fac84 100644 --- a/components/layout/context.rs +++ b/components/layout/context.rs @@ -74,6 +74,12 @@ 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. @@ -94,7 +100,7 @@ pub struct SharedLayoutContext { /// 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, @@ -122,7 +128,6 @@ pub struct SharedLayoutContext { // FIXME(#6569) This implementations is unsound: // XXX UNSOUND!!! for image_cache_task -// XXX UNSOUND!!! for stylist #[allow(unsafe_code)] unsafe impl Sync for SharedLayoutContext {} diff --git a/components/layout/layout_task.rs b/components/layout/layout_task.rs index d39f6c06a7a..8881a174dec 100644 --- a/components/layout/layout_task.rs +++ b/components/layout/layout_task.rs @@ -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; @@ -457,7 +457,7 @@ impl LayoutTask { screen_size_changed: screen_size_changed, font_cache_task: Mutex::new(self.font_cache_task.clone()), canvas_layers_sender: Mutex::new(self.canvas_layers_sender.clone()), - stylist: &*rw_data.stylist, + stylist: StylistWrapper(&*rw_data.stylist), url: (*url).clone(), visible_rects: rw_data.visible_rects.clone(), generation: rw_data.generation, diff --git a/components/layout/traversal.rs b/components/layout/traversal.rs index 34427b9d8d9..3a1fcf38bc6 100644 --- a/components/layout/traversal.rs +++ b/components/layout/traversal.rs @@ -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) { From 5462afaa8f53492261e34bb113486ce43bb41c5d Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Fri, 6 Nov 2015 23:17:00 +0100 Subject: [PATCH 9/9] Remove the unsafe Sync implementation for SharedLayoutContext. --- components/layout/context.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/components/layout/context.rs b/components/layout/context.rs index 3d5196fac84..f6058eaaf44 100644 --- a/components/layout/context.rs +++ b/components/layout/context.rs @@ -126,11 +126,6 @@ pub struct SharedLayoutContext { pub goal: ReflowGoal, } -// FIXME(#6569) This implementations is unsound: -// XXX UNSOUND!!! for image_cache_task -#[allow(unsafe_code)] -unsafe impl Sync for SharedLayoutContext {} - pub struct LayoutContext<'a> { pub shared: &'a SharedLayoutContext, cached_local_layout_context: Rc,