servo: Merge #16968 - Stylist accessors (from HeyZoos:stylist-accessors); r=emilio
authorheyzoos <jbracho@uchicago.edu>
Mon, 22 May 2017 20:12:46 -0500
changeset 360057 621439cb44b1bcc9cc8415ee076deee48ef8431c
parent 360056 90d9d225e54f33ee8d9ae50b43c070ebafce6782
child 360058 15c96c8b05cc08498d2b6fbad03ea3dd230c580e
push id43197
push userservo-vcs-sync@mozilla.com
push dateTue, 23 May 2017 04:17:38 +0000
treeherderautoland@621439cb44b1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs16968, 16857
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
servo: Merge #16968 - Stylist accessors (from HeyZoos:stylist-accessors); r=emilio <!-- Please describe your changes on the following line: --> Add accessor methods for the `device` and `ruleset` fields in the `Stylist` struct. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #16857 (github issue number if applicable). <!-- Either: --> - [X] There are tests for these changes <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 1306b16d5a170074b4f42046a7b1a3a43952b346
servo/components/layout_thread/lib.rs
servo/components/style/animation.rs
servo/components/style/context.rs
servo/components/style/gecko/data.rs
servo/components/style/matching.rs
servo/components/style/stylist.rs
servo/ports/geckolib/glue.rs
servo/tests/unit/style/stylist.rs
--- a/servo/components/layout_thread/lib.rs
+++ b/servo/components/layout_thread/lib.rs
@@ -457,17 +457,17 @@ impl LayoutThread {
 
         let stylist = Stylist::new(device, QuirksMode::NoQuirks);
         let outstanding_web_fonts_counter = Arc::new(AtomicUsize::new(0));
         let ua_stylesheets = &*UA_STYLESHEETS;
         let guard = ua_stylesheets.shared_lock.read();
         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);
         }
 
         LayoutThread {
             id: id,
             top_level_browsing_context_id: top_level_browsing_context_id,
@@ -790,20 +790,20 @@ impl LayoutThread {
     fn handle_add_stylesheet<'a, 'b>(&self,
                                      stylesheet: StyleArc<Stylesheet>,
                                      possibly_locked_rw_data: &mut RwData<'a, 'b>) {
         // Find all font-face rules and notify the font cache of them.
         // GWTODO: Need to handle unloading web fonts.
 
         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);
         }
 
         possibly_locked_rw_data.block(rw_data);
     }
 
@@ -1256,21 +1256,21 @@ impl LayoutThread {
 
         layout_context = traversal.destroy();
 
         if opts::get().dump_style_tree {
             println!("{:?}", ShowSubtreeDataAndPrimaryValues(element.as_node()));
         }
 
         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() {
             self.perform_post_style_recalc_layout_passes(&mut root_flow,
                                                          &data.reflow_info,
                                                          Some(&data.query_type),
                                                          Some(&document),
                                                          &mut rw_data,
--- a/servo/components/style/animation.rs
+++ b/servo/components/style/animation.rs
@@ -471,17 +471,17 @@ fn compute_style_for_animation_step(cont
             debug_assert!(guard.declarations().iter()
                             .all(|&(_, importance)| importance == Importance::Normal));
 
             let iter = || {
                 guard.declarations().iter().rev().map(|&(ref decl, _importance)| decl)
             };
 
             let computed =
-                properties::apply_declarations(&context.stylist.device,
+                properties::apply_declarations(context.stylist.device(),
                                                /* is_root = */ false,
                                                iter,
                                                previous_style,
                                                previous_style,
                                                /* cascade_info = */ None,
                                                &*context.error_reporter,
                                                font_metrics_provider,
                                                CascadeFlags::empty(),
--- a/servo/components/style/context.rs
+++ b/servo/components/style/context.rs
@@ -145,17 +145,17 @@ pub struct SharedStyleContext<'a> {
     #[cfg(feature = "servo")]
     pub local_context_creation_data: Mutex<ThreadLocalStyleContextCreationInfo>,
 
 }
 
 impl<'a> SharedStyleContext<'a> {
     /// Return a suitable viewport size in order to be used for viewport units.
     pub fn viewport_size(&self) -> Size2D<Au> {
-        self.stylist.device.au_viewport_size()
+        self.stylist.device().au_viewport_size()
     }
 }
 
 /// Information about the current element being processed. We group this together
 /// into a single struct within ThreadLocalStyleContext so that we can instantiate
 /// and destroy it easily at the beginning and end of element processing.
 pub struct CurrentElementInfo {
     /// The element being processed. Currently we use an OpaqueNode since we only
--- a/servo/components/style/gecko/data.rs
+++ b/servo/components/style/gecko/data.rs
@@ -90,17 +90,17 @@ impl PerDocumentStyleData {
     }
 }
 
 impl PerDocumentStyleDataImpl {
     /// Reset the device state because it may have changed.
     ///
     /// 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);
     }
 
     /// Recreate the style data if the stylesheets have changed.
     pub fn flush_stylesheets(&mut self, guard: &SharedRwLockReadGuard) {
         if !self.stylesheets.has_changed() {
             return;
@@ -118,17 +118,17 @@ impl PerDocumentStyleDataImpl {
                              /* ua_sheets = */ None,
                              /* stylesheets_changed = */ true,
                              author_style_disabled,
                              &mut extra_data);
     }
 
     /// Get the default computed values for this document.
     pub fn default_computed_values(&self) -> &Arc<ComputedValues> {
-        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
     /// already cleared; the stylist handles that.
     pub fn clear_stylist(&mut self) {
         self.stylist.clear();
     }
 }
--- a/servo/components/style/matching.rs
+++ b/servo/components/style/matching.rs
@@ -218,17 +218,17 @@ trait PrivateMatchMethods: TElement {
             let can_be_fragmented =
                 p.is_multicol() ||
                 layout_parent_el.as_ref().unwrap().as_node().can_be_fragmented();
             unsafe { self.as_node().set_can_be_fragmented(can_be_fragmented); }
         }
 
         // 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,
                              layout_parent_style,
                              Some(&mut cascade_info),
                              &*shared_context.error_reporter,
                              font_metrics_provider,
                              cascade_flags,
@@ -345,17 +345,17 @@ trait PrivateMatchMethods: TElement {
     /// If there is no transition rule in the ComputedValues, it returns None.
     #[cfg(feature = "gecko")]
     fn get_after_change_style(&self,
                               context: &mut StyleContext<Self>,
                               primary_style: &ComputedStyle)
                               -> Option<Arc<ComputedValues>> {
         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.
             return None;
         }
 
         Some(self.cascade_with_rules(context.shared,
                                      &context.thread_local.font_metrics_provider,
@@ -693,29 +693,29 @@ pub trait MatchMethods : TElement {
                 let pseudo_style =
                     parent_data.styles().pseudos.get(&pseudo).unwrap();
                 let mut rules = pseudo_style.rules.clone();
                 let animation_rules = self.get_animation_rules();
 
                 // 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,
                                                   &context.shared.guards);
                     if let Some(node) = animation_rule_node {
                         rules = node;
                     }
                 }
 
                 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,
                                                   &context.shared.guards);
                     if let Some(node) = animation_rule_node {
                         rules = node;
                     }
                 }
@@ -758,17 +758,17 @@ pub trait MatchMethods : TElement {
                                              animation_rules,
                                              &mut applicable_declarations,
                                              &mut matching_context,
                                              &mut set_selector_flags);
 
         *relations = matching_context.relations;
 
         let primary_rule_node =
-            compute_rule_node::<Self>(&stylist.rule_tree,
+            compute_rule_node::<Self>(stylist.rule_tree(),
                                       &mut applicable_declarations,
                                       &context.shared.guards);
 
         if log_enabled!(Trace) {
             trace!("Matched rules:");
             for rn in primary_rule_node.self_and_ancestors() {
                 if let Some(source) = rn.style_source() {
                     trace!(" > {:?}", source);
@@ -811,17 +811,17 @@ pub trait MatchMethods : TElement {
         let mut set_selector_flags = |element: &Self, flags: ElementSelectorFlags| {
             self.apply_selector_flags(map, element, flags);
         };
 
         // Borrow the stuff we need here so the borrow checker doesn't get mad
         // 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 =
             MatchingContext::new(MatchingMode::ForStatelessPseudoElement,
                                  Some(bloom_filter));
 
         // Compute rule nodes for eagerly-cascaded pseudo-elements.
         let mut matches_different_pseudos = false;
@@ -977,17 +977,17 @@ pub trait MatchMethods : TElement {
         let element_styles = &mut data.styles_mut();
         let primary_rules = &mut element_styles.primary.rules;
         let mut result = RulesChanged::empty();
 
         {
             let mut replace_rule_node = |level: CascadeLevel,
                                          pdb: Option<&Arc<Locked<PropertyDeclarationBlock>>>,
                                          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;
                     if level.is_important() {
                         result.insert(IMPORTANT_RULES_CHANGED);
                     } else {
                         result.insert(NORMAL_RULES_CHANGED);
                     }
@@ -1133,17 +1133,17 @@ pub trait MatchMethods : TElement {
                       shared_context: &SharedStyleContext,
                       font_metrics_provider: &FontMetricsProvider,
                       primary_style: &ComputedStyle,
                       pseudo_style: Option<&ComputedStyle>)
                       -> Arc<ComputedValues> {
         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.
             return relevant_style.values.as_ref().unwrap().clone();
         }
 
         self.cascade_with_rules(shared_context,
                                 font_metrics_provider,
--- a/servo/components/style/stylist.rs
+++ b/servo/components/style/stylist.rs
@@ -81,17 +81,17 @@ pub struct Stylist {
     /// device.
     ///
     /// On Servo, on the other hand, the device is a really cheap representation
     /// that is recreated each time some constraint changes and calling
     /// `set_device`.
     ///
     /// 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>,
+    device: Arc<Device>,
 
     /// Viewport constraints based on the current device.
     viewport_constraints: Option<ViewportConstraints>,
 
     /// If true, the quirks-mode stylesheet is applied.
     quirks_mode: QuirksMode,
 
     /// If true, the device has changed, and the stylist needs to be updated.
@@ -101,19 +101,17 @@ pub struct Stylist {
     /// had clear() called on it with no following rebuild()).
     is_cleared: bool,
 
     /// The current selector maps, after evaluating media
     /// rules against the current device.
     element_map: PerPseudoElementSelectorMap,
 
     /// 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)
     pseudos_map: FnvHashMap<PseudoElement, PerPseudoElementSelectorMap>,
 
     /// A map with all the animations indexed by name.
     animations: FnvHashMap<Atom, KeyframesAnimation>,
 
@@ -1059,16 +1057,31 @@ impl Stylist {
                                      Some(parent_style),
                                      Some(parent_style),
                                      None,
                                      &RustLogReporter,
                                      &metrics,
                                      CascadeFlags::empty(),
                                      self.quirks_mode))
     }
+
+    /// Accessor for a shared reference to the device.
+    pub fn device(&self) -> &Device {
+        &self.device
+    }
+
+    /// Accessor for a mutable reference to the device.
+    pub fn device_mut(&mut self) -> &mut Arc<Device> {
+        &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 {
     fn drop(&mut self) {
         // This is the last chance to GC the rule tree.  If we have dropped all
         // strong rule node references before the Stylist is dropped, then this
         // will cause the rule tree to be destroyed correctly.  If we haven't
         // dropped all strong rule node references before now, then we will
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -1587,17 +1587,17 @@ pub extern "C" fn Servo_MediaList_DeepCl
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_MediaList_Matches(list: RawServoMediaListBorrowed,
                                           raw_data: RawServoStyleSetBorrowed)
                                           -> 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())
     })
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_DeclarationBlock_HasCSSWideKeyword(declarations: RawServoDeclarationBlockBorrowed,
                                                            property: nsCSSPropertyID) -> bool {
     let property_id = get_property_id_from_nscsspropertyid!(property, false);
     read_locked_arc(declarations, |decls: &PropertyDeclarationBlock| {
@@ -2285,17 +2285,17 @@ pub extern "C" fn Servo_GetComputedKeyfr
     let style = ComputedValues::as_arc(&style);
     let parent_style = parent_style.as_ref().map(|r| &**ComputedValues::as_arc(&r));
 
     let default_values = data.default_computed_values();
     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),
         font_metrics_provider: &metrics,
         cached_system_font: None,
         in_media_query: false,
         quirks_mode: data.stylist.quirks_mode(),
     };
@@ -2363,17 +2363,17 @@ pub extern "C" fn Servo_AnimationValue_C
 
     let data = PerDocumentStyleData::from_ffi(raw_data).borrow();
     let style = ComputedValues::as_arc(&style);
     let parent_style = parent_style.as_ref().map(|r| &**ComputedValues::as_arc(&r));
     let default_values = data.default_computed_values();
     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),
         font_metrics_provider: &metrics,
         cached_system_font: None,
         in_media_query: false,
         quirks_mode: data.stylist.quirks_mode(),
     };
--- a/servo/tests/unit/style/stylist.rs
+++ b/servo/tests/unit/style/stylist.rs
@@ -1,26 +1,29 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * 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;
 use style::selector_parser::{SelectorImpl, SelectorParser};
 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;
 
 /// Helper method to get some Rules from selector strings.
 /// Each sublist of the result contains the Rules for one StyleRule.
 fn get_mock_rules(css_selectors: &[&str]) -> (Vec<Vec<Rule>>, SharedRwLock) {
     let shared_lock = SharedRwLock::new();
     (css_selectors.iter().enumerate().map(|(i, selectors)| {
@@ -213,8 +216,28 @@ fn test_insert() {
 fn test_get_universal_rules() {
     thread_state::initialize(thread_state::LAYOUT);
     let (map, _shared_lock) = get_mock_map(&["*|*", "#foo > *|*", "*|* > *|*", ".klass", "#id"]);
 
     let decls = map.get_universal_rules(CascadeLevel::UserNormal);
 
     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();
+}