From 4196156bb97cbdb6da8cd09b4809727e1e51f265 Mon Sep 17 00:00:00 2001 From: heyzoos Date: Thu, 18 May 2017 22:18:16 -0500 Subject: [PATCH 1/3] Change device and rule_set references to accessors --- components/layout_thread/lib.rs | 10 +++++----- components/style/animation.rs | 2 +- components/style/context.rs | 2 +- components/style/gecko/data.rs | 4 ++-- components/style/matching.rs | 16 ++++++++-------- components/style/stylist.rs | 19 +++++++++++++++++-- ports/geckolib/glue.rs | 6 +++--- 7 files changed, 37 insertions(+), 22 deletions(-) diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index 8185aa6e409..136a61033b7 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -457,7 +457,7 @@ impl LayoutThread { for stylesheet in &ua_stylesheets.user_or_user_agent_stylesheets { add_font_face_rules(stylesheet, &guard, - &stylist.device, + stylist.device(), &font_cache_thread, &ipc_font_cache_sender, &outstanding_web_fonts_counter); @@ -789,10 +789,10 @@ impl LayoutThread { let rw_data = possibly_locked_rw_data.lock(); let guard = stylesheet.shared_lock.read(); - if stylesheet.is_effective_for_device(&self.stylist.device, &guard) { + if stylesheet.is_effective_for_device(self.stylist.device(), &guard) { add_font_face_rules(&*stylesheet, &guard, - &self.stylist.device, + self.stylist.device(), &self.font_cache_thread, &self.font_cache_sender, &self.outstanding_web_fonts); @@ -1255,11 +1255,11 @@ impl LayoutThread { } if opts::get().dump_rule_tree { - layout_context.style_context.stylist.rule_tree.dump_stdout(&guards); + layout_context.style_context.stylist.rule_tree().dump_stdout(&guards); } // GC the rule tree if some heuristics are met. - unsafe { layout_context.style_context.stylist.rule_tree.maybe_gc(); } + unsafe { layout_context.style_context.stylist.rule_tree().maybe_gc(); } // Perform post-style recalculation layout passes. if let Some(mut root_flow) = self.root_flow.borrow().clone() { diff --git a/components/style/animation.rs b/components/style/animation.rs index 5f53508ba4e..4761fde0f28 100644 --- a/components/style/animation.rs +++ b/components/style/animation.rs @@ -476,7 +476,7 @@ fn compute_style_for_animation_step(context: &SharedStyleContext, }; let computed = - properties::apply_declarations(&context.stylist.device, + properties::apply_declarations(context.stylist.device(), /* is_root = */ false, iter, previous_style, diff --git a/components/style/context.rs b/components/style/context.rs index a628ef1377f..4ac9b64c80e 100644 --- a/components/style/context.rs +++ b/components/style/context.rs @@ -150,7 +150,7 @@ pub struct SharedStyleContext<'a> { impl<'a> SharedStyleContext<'a> { /// Return a suitable viewport size in order to be used for viewport units. pub fn viewport_size(&self) -> Size2D { - self.stylist.device.au_viewport_size() + self.stylist.device().au_viewport_size() } } diff --git a/components/style/gecko/data.rs b/components/style/gecko/data.rs index 4108e073945..b95e938d3a2 100644 --- a/components/style/gecko/data.rs +++ b/components/style/gecko/data.rs @@ -95,7 +95,7 @@ impl PerDocumentStyleDataImpl { /// /// Implies also a stylesheet flush. pub fn reset_device(&mut self, guard: &SharedRwLockReadGuard) { - Arc::get_mut(&mut self.stylist.device).unwrap().reset(); + Arc::get_mut(self.stylist.device_mut()).unwrap().reset(); self.stylesheets.force_dirty(); self.flush_stylesheets(guard); } @@ -123,7 +123,7 @@ impl PerDocumentStyleDataImpl { /// Get the default computed values for this document. pub fn default_computed_values(&self) -> &Arc { - self.stylist.device.default_computed_values_arc() + self.stylist.device().default_computed_values_arc() } /// Clear the stylist. This will be a no-op if the stylist is diff --git a/components/style/matching.rs b/components/style/matching.rs index 289a6fb1975..d80870394a4 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -476,7 +476,7 @@ trait PrivateMatchMethods: TElement { // Invoke the cascade algorithm. let values = - Arc::new(cascade(&shared_context.stylist.device, + Arc::new(cascade(shared_context.stylist.device(), rule_node, &shared_context.guards, style_to_inherit_from, @@ -603,7 +603,7 @@ trait PrivateMatchMethods: TElement { -> Option> { let rule_node = &primary_style.rules; let without_transition_rules = - context.shared.stylist.rule_tree.remove_transition_rule_if_applicable(rule_node); + context.shared.stylist.rule_tree().remove_transition_rule_if_applicable(rule_node); if without_transition_rules == *rule_node { // We don't have transition rule in this case, so return None to let the caller // use the original ComputedValues. @@ -955,7 +955,7 @@ pub trait MatchMethods : TElement { // Handle animations here. if let Some(animation_rule) = animation_rules.0 { let animation_rule_node = - context.shared.stylist.rule_tree + context.shared.stylist.rule_tree() .update_rule_at_level(CascadeLevel::Animations, Some(&animation_rule), &mut rules, @@ -967,7 +967,7 @@ pub trait MatchMethods : TElement { if let Some(animation_rule) = animation_rules.1 { let animation_rule_node = - context.shared.stylist.rule_tree + context.shared.stylist.rule_tree() .update_rule_at_level(CascadeLevel::Transitions, Some(&animation_rule), &mut rules, @@ -1011,7 +1011,7 @@ pub trait MatchMethods : TElement { *relations = matching_context.relations; let primary_rule_node = - compute_rule_node::(&stylist.rule_tree, + compute_rule_node::(stylist.rule_tree(), &mut applicable_declarations, &context.shared.guards); @@ -1044,7 +1044,7 @@ pub trait MatchMethods : TElement { // at us later in the closure. let stylist = &context.shared.stylist; let guards = &context.shared.guards; - let rule_tree = &stylist.rule_tree; + let rule_tree = stylist.rule_tree(); let bloom_filter = context.thread_local.bloom_filter.filter(); let mut matching_context = @@ -1172,7 +1172,7 @@ pub trait MatchMethods : TElement { let mut replace_rule_node = |level: CascadeLevel, pdb: Option<&Arc>>, path: &mut StrongRuleNode| { - let new_node = context.shared.stylist.rule_tree + let new_node = context.shared.stylist.rule_tree() .update_rule_at_level(level, pdb, path, &context.shared.guards); if let Some(n) = new_node { *path = n; @@ -1430,7 +1430,7 @@ pub trait MatchMethods : TElement { let relevant_style = pseudo_style.unwrap_or(primary_style); let rule_node = &relevant_style.rules; let without_animation_rules = - shared_context.stylist.rule_tree.remove_animation_rules(rule_node); + shared_context.stylist.rule_tree().remove_animation_rules(rule_node); if without_animation_rules == *rule_node { // Note that unwrapping here is fine, because the style is // only incomplete during the styling process. diff --git a/components/style/stylist.rs b/components/style/stylist.rs index a5c103c5a55..5ae2e1afa3f 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -88,7 +88,7 @@ pub struct Stylist { /// /// In both cases, the device is actually _owned_ by the Stylist, and it's /// only an `Arc` so we can implement `add_stylesheet` more idiomatically. - pub device: Arc, + device: Arc, /// Viewport constraints based on the current device. viewport_constraints: Option, @@ -110,7 +110,7 @@ pub struct Stylist { /// The rule tree, that stores the results of selector matching. /// /// FIXME(emilio): Not `pub`! - pub rule_tree: RuleTree, + rule_tree: RuleTree, /// The selector maps corresponding to a given pseudo-element /// (depending on the implementation) @@ -1065,6 +1065,21 @@ impl Stylist { CascadeFlags::empty(), self.quirks_mode)) } + + /// Accessor for a shared reference to the device. + pub fn device(&self) -> &Arc { + &self.device + } + + /// Accessor for a mutable reference to the device. + pub fn device_mut(&mut self) -> &mut Arc { + &mut self.device + } + + /// Accessor for a shared reference to the rule tree. + pub fn rule_tree(&self) -> &RuleTree { + &self.rule_tree + } } impl Drop for Stylist { diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index d04625727aa..92bb4e46a06 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -1390,7 +1390,7 @@ pub extern "C" fn Servo_MediaList_Matches(list: RawServoMediaListBorrowed, -> bool { let per_doc_data = PerDocumentStyleData::from_ffi(raw_data).borrow(); read_locked_arc(list, |list: &MediaList| { - list.evaluate(&per_doc_data.stylist.device, per_doc_data.stylist.quirks_mode()) + list.evaluate(per_doc_data.stylist.device(), per_doc_data.stylist.quirks_mode()) }) } @@ -2074,7 +2074,7 @@ pub extern "C" fn Servo_GetComputedKeyframeValues(keyframes: RawGeckoKeyframeLis let mut context = Context { is_root_element: false, - device: &data.stylist.device, + device: data.stylist.device(), inherited_style: parent_style.unwrap_or(default_values), layout_parent_style: parent_style.unwrap_or(default_values), style: StyleBuilder::for_derived_style(&style), @@ -2152,7 +2152,7 @@ pub extern "C" fn Servo_AnimationValue_Compute(declarations: RawServoDeclaration let metrics = get_metrics_provider_for_product(); let mut context = Context { is_root_element: false, - device: &data.stylist.device, + device: data.stylist.device(), inherited_style: parent_style.unwrap_or(default_values), layout_parent_style: parent_style.unwrap_or(default_values), style: StyleBuilder::for_derived_style(&style), From e3d9c7449a975f83c38dc7df54086ec63e235993 Mon Sep 17 00:00:00 2001 From: heyzoos Date: Sat, 20 May 2017 11:11:30 -0500 Subject: [PATCH 2/3] Add tests for the device and rule_tree accessors --- tests/unit/style/stylist.rs | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/tests/unit/style/stylist.rs b/tests/unit/style/stylist.rs index 3c136edcef3..16feb1cec4b 100644 --- a/tests/unit/style/stylist.rs +++ b/tests/unit/style/stylist.rs @@ -3,10 +3,13 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use cssparser::SourceLocation; +use euclid::TypedSize2D; use html5ever::LocalName; use selectors::parser::LocalName as LocalNameSelector; use selectors::parser::Selector; use servo_atoms::Atom; +use style::context::QuirksMode; +use style::media_queries::{Device, MediaType}; use style::properties::{PropertyDeclarationBlock, PropertyDeclaration}; use style::properties::{longhands, Importance}; use style::rule_tree::CascadeLevel; @@ -15,7 +18,7 @@ use style::shared_lock::SharedRwLock; use style::stylearc::Arc; use style::stylesheets::StyleRule; use style::stylist; -use style::stylist::{Rule, SelectorMap}; +use style::stylist::{Rule, SelectorMap, Stylist}; use style::stylist::needs_revalidation; use style::thread_state; @@ -218,3 +221,23 @@ fn test_get_universal_rules() { assert_eq!(decls.len(), 1, "{:?}", decls); } + +fn mock_stylist() -> Stylist { + let device = Device::new(MediaType::Screen, TypedSize2D::new(0f32, 0f32)); + Stylist::new(device, QuirksMode::NoQuirks) +} + +#[test] +fn test_stylist_device_accessors() { + let stylist = mock_stylist(); + assert_eq!(stylist.device().media_type(), MediaType::Screen); + let mut stylist_mut = mock_stylist(); + assert_eq!(stylist_mut.device_mut().media_type(), MediaType::Screen); +} + +#[test] +fn test_stylist_rule_tree_accessors() { + let stylist = mock_stylist(); + stylist.rule_tree(); + stylist.rule_tree().root(); +} From fb755838ca440b2a616689723381714e54fc2898 Mon Sep 17 00:00:00 2001 From: heyzoos Date: Sat, 20 May 2017 18:52:53 -0500 Subject: [PATCH 3/3] Return &Device instead of &Arc --- components/style/stylist.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 5ae2e1afa3f..2a69dcf433c 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -108,8 +108,6 @@ pub struct Stylist { element_map: PerPseudoElementSelectorMap, /// The rule tree, that stores the results of selector matching. - /// - /// FIXME(emilio): Not `pub`! rule_tree: RuleTree, /// The selector maps corresponding to a given pseudo-element @@ -1067,7 +1065,7 @@ impl Stylist { } /// Accessor for a shared reference to the device. - pub fn device(&self) -> &Arc { + pub fn device(&self) -> &Device { &self.device }