From 5ac12b5df4406dbde1ceeb6be36be5c3162401a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 22 Jan 2018 21:49:18 +0100 Subject: [PATCH 1/3] style: Make the TElement type arrive to the `cascade` function. Not super-proud of this one, but it's the easiest way I could think of. The changeset looks bigger than what it is, because while at it I've rewrapped a fair amount of functions around to use proper block indentation. Alternatives are parameterizing Stylist by , which is not fun, or moving the concrete element from layout_thread to layout, but that implies layout depending on script, which isn't fun either. Other alternative is implementing an empty enum and making anon boxes work on it. It has the advantage of removing the annoying type parameter, but the disadvantage of instantiating `cascade` twice, which isn't great, and having to maintain all the boilerplate of a `TElement` implementation that just does nothing. --- components/layout/animation.rs | 49 +-- components/layout/construct.rs | 331 ++++++++++++------ components/script_layout_interface/lib.rs | 1 + .../script_layout_interface/wrapper_traits.rs | 13 +- components/style/animation.rs | 97 ++--- components/style/matching.rs | 10 +- .../style/properties/properties.mako.rs | 14 +- components/style/style_resolver.rs | 1 + components/style/stylist.rs | 93 +++-- ports/geckolib/glue.rs | 23 +- 10 files changed, 425 insertions(+), 207 deletions(-) diff --git a/components/layout/animation.rs b/components/layout/animation.rs index a3c364a7851..27497811489 100644 --- a/components/layout/animation.rs +++ b/components/layout/animation.rs @@ -15,6 +15,7 @@ use script_traits::{AnimationState, ConstellationControlMsg, LayoutMsg as Conste use script_traits::UntrustedNodeAddress; use std::sync::mpsc::Receiver; use style::animation::{Animation, update_style_for_animation}; +use style::dom::TElement; use style::font_metrics::ServoMetricsProvider; use style::selector_parser::RestyleDamage; use style::timer::Timer; @@ -22,14 +23,19 @@ use style::timer::Timer; /// Processes any new animations that were discovered after style recalculation. /// Also expire any old animations that have completed, inserting them into /// `expired_animations`. -pub fn update_animation_state(constellation_chan: &IpcSender, - script_chan: &IpcSender, - running_animations: &mut FnvHashMap>, - expired_animations: &mut FnvHashMap>, - mut newly_transitioning_nodes: Option<&mut Vec>, - new_animations_receiver: &Receiver, - pipeline_id: PipelineId, - timer: &Timer) { +pub fn update_animation_state( + constellation_chan: &IpcSender, + script_chan: &IpcSender, + running_animations: &mut FnvHashMap>, + expired_animations: &mut FnvHashMap>, + mut newly_transitioning_nodes: Option<&mut Vec>, + new_animations_receiver: &Receiver, + pipeline_id: PipelineId, + timer: &Timer, +) +where + E: TElement, +{ let mut new_running_animations = vec![]; while let Ok(animation) = new_animations_receiver.try_recv() { let mut should_push = true; @@ -144,22 +150,25 @@ pub fn update_animation_state(constellation_chan: &IpcSender, /// Recalculates style for a set of animations. This does *not* run with the DOM /// lock held. -// NB: This is specific for SelectorImpl, since the layout context and the -// flows are SelectorImpl specific too. If that goes away at some point, -// this should be made generic. -pub fn recalc_style_for_animations(context: &LayoutContext, - flow: &mut Flow, - animations: &FnvHashMap>) { +pub fn recalc_style_for_animations( + context: &LayoutContext, + flow: &mut Flow, + animations: &FnvHashMap>, +) +where + E: TElement, +{ let mut damage = RestyleDamage::empty(); flow.mutate_fragments(&mut |fragment| { if let Some(ref animations) = animations.get(&fragment.node) { for animation in animations.iter() { let old_style = fragment.style.clone(); - update_style_for_animation(&context.style_context, - animation, - &mut fragment.style, - &ServoMetricsProvider); + update_style_for_animation::( + &context.style_context, + animation, + &mut fragment.style, + &ServoMetricsProvider, + ); let difference = RestyleDamage::compute_style_difference( &old_style, @@ -173,6 +182,6 @@ pub fn recalc_style_for_animations(context: &LayoutContext, let base = flow.mut_base(); base.restyle_damage.insert(damage); for kid in base.children.iter_mut() { - recalc_style_for_animations(context, kid, animations) + recalc_style_for_animations::(context, kid, animations) } } diff --git a/components/layout/construct.rs b/components/layout/construct.rs index 807bac242cc..bbbd9287e13 100644 --- a/components/layout/construct.rs +++ b/components/layout/construct.rs @@ -51,6 +51,7 @@ use style::computed_values::float::T as Float; use style::computed_values::list_style_position::T as ListStylePosition; use style::computed_values::position::T as Position; use style::context::SharedStyleContext; +use style::dom::TElement; use style::logical_geometry::Direction; use style::properties::ComputedValues; use style::properties::longhands::list_style_image; @@ -170,11 +171,12 @@ pub struct InlineBlockSplit { impl InlineBlockSplit { /// Flushes the given accumulator to the new split and makes a new accumulator to hold any /// subsequent fragments. - fn new(fragment_accumulator: &mut InlineFragmentsAccumulator, - node: &ConcreteThreadSafeLayoutNode, - style_context: &SharedStyleContext, - flow: FlowRef) - -> InlineBlockSplit { + fn new( + fragment_accumulator: &mut InlineFragmentsAccumulator, + node: &ConcreteThreadSafeLayoutNode, + style_context: &SharedStyleContext, + flow: FlowRef, + ) -> InlineBlockSplit { fragment_accumulator.enclosing_node.as_mut().expect( "enclosing_node is None; Are {ib} splits being generated outside of an inline node?" ).flags.remove(InlineFragmentNodeFlags::LAST_FRAGMENT_OF_ELEMENT); @@ -183,7 +185,9 @@ impl InlineBlockSplit { predecessors: mem::replace( fragment_accumulator, InlineFragmentsAccumulator::from_inline_node( - node, style_context)).to_intermediate_inline_fragments(style_context), + node, + style_context, + )).to_intermediate_inline_fragments::(style_context), flow: flow, }; @@ -280,8 +284,13 @@ impl InlineFragmentsAccumulator { self.fragments.absolute_descendants.push_descendants(fragments.absolute_descendants); } - fn to_intermediate_inline_fragments(self, context: &SharedStyleContext) - -> IntermediateInlineFragments { + fn to_intermediate_inline_fragments( + self, + context: &SharedStyleContext, + ) -> IntermediateInlineFragments + where + N: ThreadSafeLayoutNode, + { let InlineFragmentsAccumulator { mut fragments, enclosing_node, @@ -308,9 +317,21 @@ impl InlineFragmentsAccumulator { if let Some((start, end)) = bidi_control_chars { fragments.fragments.push_front( - control_chars_to_fragment(&enclosing_node, context, start, restyle_damage)); + control_chars_to_fragment::( + &enclosing_node, + context, + start, + restyle_damage, + ) + ); fragments.fragments.push_back( - control_chars_to_fragment(&enclosing_node, context, end, restyle_damage)); + control_chars_to_fragment::( + &enclosing_node, + context, + end, + restyle_damage, + ) + ); } } fragments @@ -402,13 +423,18 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> /// `#[inline(always)]` because this is performance critical and LLVM will not inline it /// otherwise. #[inline(always)] - fn flush_inline_fragments_to_flow(&mut self, - fragment_accumulator: InlineFragmentsAccumulator, - flow: &mut FlowRef, - absolute_descendants: &mut AbsoluteDescendants, - legalizer: &mut Legalizer, - node: &ConcreteThreadSafeLayoutNode) { - let mut fragments = fragment_accumulator.to_intermediate_inline_fragments(self.style_context()); + fn flush_inline_fragments_to_flow( + &mut self, + fragment_accumulator: InlineFragmentsAccumulator, + flow: &mut FlowRef, + absolute_descendants: &mut AbsoluteDescendants, + legalizer: &mut Legalizer, + node: &ConcreteThreadSafeLayoutNode, + ) { + let mut fragments = + fragment_accumulator.to_intermediate_inline_fragments::( + self.style_context(), + ); if fragments.is_empty() { return }; @@ -479,7 +505,11 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> } inline_flow_ref.finish(); - legalizer.add_child(self.style_context(), flow, inline_flow_ref) + legalizer.add_child::( + self.style_context(), + flow, + inline_flow_ref, + ) } fn build_block_flow_using_construction_result_of_child( @@ -512,7 +542,11 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> legalizer, node); } - legalizer.add_child(self.style_context(), flow, kid_flow) + legalizer.add_child::( + self.style_context(), + flow, + kid_flow, + ) } abs_descendants.push_descendants(kid_abs_descendants); } @@ -546,7 +580,11 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> node); // Push the flow generated by the {ib} split onto our list of flows. - legalizer.add_child(self.style_context(), flow, kid_flow) + legalizer.add_child::( + self.style_context(), + flow, + kid_flow, + ) } // Add the fragments to the list we're maintaining. @@ -666,11 +704,17 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> let context = self.style_context(); let mut style = node.style(context); - style = context.stylist.style_for_anonymous( - &context.guards, &PseudoElement::ServoText, &style); + style = context.stylist.style_for_anonymous::( + &context.guards, + &PseudoElement::ServoText, + &style, + ); if node_is_input_or_text_area { - style = context.stylist.style_for_anonymous( - &context.guards, &PseudoElement::ServoInputText, &style) + style = context.stylist.style_for_anonymous::( + &context.guards, + &PseudoElement::ServoInputText, + &style, + ) } self.create_fragments_for_node_text_content(&mut fragments, node, &style) @@ -878,7 +922,9 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> let construction_item = ConstructionItem::InlineFragments( InlineFragmentsConstructionResult { splits: opt_inline_block_splits, - fragments: fragment_accumulator.to_intermediate_inline_fragments(self.style_context()), + fragments: fragment_accumulator.to_intermediate_inline_fragments::( + self.style_context(), + ), }); ConstructionResult::ConstructionItem(construction_item) } else { @@ -902,9 +948,13 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> return ConstructionResult::ConstructionItem(ConstructionItem::Whitespace( node.opaque(), node.get_pseudo_element_type(), - context.stylist.style_for_anonymous( - &context.guards, &PseudoElement::ServoText, &style), - node.restyle_damage())) + context.stylist.style_for_anonymous::( + &context.guards, + &PseudoElement::ServoText, + &style, + ), + node.restyle_damage(), + )) } // If this is generated content, then we need to initialize the accumulator with the @@ -913,8 +963,11 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> let mut fragments = IntermediateInlineFragments::new(); match (node.get_pseudo_element_type(), node.type_id()) { (_, Some(LayoutNodeType::Text)) => { - let text_style = context.stylist.style_for_anonymous( - &context.guards, &PseudoElement::ServoText, &style); + let text_style = context.stylist.style_for_anonymous::( + &context.guards, + &PseudoElement::ServoText, + &style, + ); self.create_fragments_for_node_text_content(&mut fragments, node, &text_style) } (PseudoElementType::Normal, _) => { @@ -949,8 +1002,11 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> let context = self.style_context(); let style = node.style(context); - let style = context.stylist.style_for_anonymous( - &context.guards, &PseudoElement::ServoInlineBlockWrapper, &style); + let style = context.stylist.style_for_anonymous::( + &context.guards, + &PseudoElement::ServoInlineBlockWrapper, + &style, + ); let fragment_info = SpecificFragmentInfo::InlineBlock(InlineBlockFragmentInfo::new( block_flow)); let fragment = Fragment::from_opaque_node_and_style(node.opaque(), @@ -967,7 +1023,8 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> let construction_item = ConstructionItem::InlineFragments(InlineFragmentsConstructionResult { splits: LinkedList::new(), - fragments: fragment_accumulator.to_intermediate_inline_fragments(context), + fragments: fragment_accumulator + .to_intermediate_inline_fragments::(context), }); ConstructionResult::ConstructionItem(construction_item) } @@ -987,8 +1044,11 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> InlineAbsoluteHypotheticalFragmentInfo::new(block_flow)); let style_context = self.style_context(); let style = node.style(style_context); - let style = style_context.stylist.style_for_anonymous( - &style_context.guards, &PseudoElement::ServoInlineAbsolute, &style); + let style = style_context.stylist.style_for_anonymous::( + &style_context.guards, + &PseudoElement::ServoInlineAbsolute, + &style, + ); let fragment = Fragment::from_opaque_node_and_style(node.opaque(), PseudoElementType::Normal, style, @@ -1003,7 +1063,8 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> let construction_item = ConstructionItem::InlineFragments(InlineFragmentsConstructionResult { splits: LinkedList::new(), - fragments: fragment_accumulator.to_intermediate_inline_fragments(style_context), + fragments: fragment_accumulator + .to_intermediate_inline_fragments::(style_context), }); ConstructionResult::ConstructionItem(construction_item) } @@ -1097,8 +1158,11 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> { let context = self.style_context(); table_style = node.style(context); - wrapper_style = context.stylist.style_for_anonymous( - &context.guards, &PseudoElement::ServoTableWrapper, &table_style); + wrapper_style = context.stylist.style_for_anonymous::( + &context.guards, + &PseudoElement::ServoTableWrapper, + &table_style, + ); } let wrapper_fragment = Fragment::from_opaque_node_and_style(node.opaque(), @@ -1128,7 +1192,11 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> CaptionSide::Top); if let ConstructionResult::Flow(table_flow, table_abs_descendants) = construction_result { - legalizer.add_child(self.style_context(), &mut wrapper_flow, table_flow); + legalizer.add_child::( + self.style_context(), + &mut wrapper_flow, + table_flow, + ); abs_descendants.push_descendants(table_abs_descendants); } @@ -1821,16 +1889,24 @@ fn bidi_control_chars(style: &ServoArc) -> Option<(&'static str, } } -fn control_chars_to_fragment(node: &InlineFragmentNodeInfo, - context: &SharedStyleContext, - text: &str, - restyle_damage: RestyleDamage) - -> Fragment { +fn control_chars_to_fragment( + node: &InlineFragmentNodeInfo, + context: &SharedStyleContext, + text: &str, + restyle_damage: RestyleDamage, +) -> Fragment +where + E: TElement, +{ let info = SpecificFragmentInfo::UnscannedText( Box::new(UnscannedTextFragmentInfo::new(String::from(text), None)) ); - let text_style = context.stylist.style_for_anonymous( - &context.guards, &PseudoElement::ServoText, &node.style); + let text_style = context.stylist.style_for_anonymous::( + &context.guards, + &PseudoElement::ServoText, + &node.style, + ); + Fragment::from_opaque_node_and_style(node.address, node.pseudo, text_style, @@ -1887,16 +1963,24 @@ impl Legalizer { /// Makes the `child` flow a new child of `parent`. Anonymous flows are automatically inserted /// to keep the tree legal. - fn add_child(&mut self, context: &SharedStyleContext, parent: &mut FlowRef, mut child: FlowRef) { + fn add_child( + &mut self, + context: &SharedStyleContext, + parent: &mut FlowRef, + mut child: FlowRef, + ) + where + E: TElement, + { while !self.stack.is_empty() { - if self.try_to_add_child(context, parent, &mut child) { + if self.try_to_add_child::(context, parent, &mut child) { return } self.flush_top_of_stack(parent) } - while !self.try_to_add_child(context, parent, &mut child) { - self.push_next_anonymous_flow(context, parent) + while !self.try_to_add_child::(context, parent, &mut child) { + self.push_next_anonymous_flow::(context, parent) } } @@ -1913,8 +1997,15 @@ impl Legalizer { /// This method attempts to create anonymous blocks in between `parent` and `child` if and only /// if those blocks will only ever have `child` as their sole child. At present, this is only /// true for anonymous block children of flex flows. - fn try_to_add_child(&mut self, context: &SharedStyleContext, parent: &mut FlowRef, child: &mut FlowRef) - -> bool { + fn try_to_add_child( + &mut self, + context: &SharedStyleContext, + parent: &mut FlowRef, + child: &mut FlowRef, + ) -> bool + where + E: TElement, + { let parent = self.stack.last_mut().unwrap_or(parent); let (parent_class, child_class) = (parent.class(), child.class()); match (parent_class, child_class) { @@ -1944,12 +2035,14 @@ impl Legalizer { (FlowClass::Flex, FlowClass::Inline) => { FlowRef::deref_mut(child).mut_base().flags.insert(FlowFlags::MARGINS_CANNOT_COLLAPSE); - let mut block_wrapper = - Legalizer::create_anonymous_flow(context, - parent, - &[PseudoElement::ServoAnonymousBlock], - SpecificFragmentInfo::Generic, - BlockFlow::from_fragment); + let mut block_wrapper = Legalizer::create_anonymous_flow::( + context, + parent, + &[PseudoElement::ServoAnonymousBlock], + SpecificFragmentInfo::Generic, + BlockFlow::from_fragment, + ); + { let flag = if parent.as_flex().main_mode() == Direction::Inline { FragmentFlags::IS_INLINE_FLEX_ITEM @@ -1997,54 +2090,76 @@ impl Legalizer { /// Adds the anonymous flow that would be necessary to make an illegal child of `parent` legal /// to the stack. - fn push_next_anonymous_flow(&mut self, context: &SharedStyleContext, parent: &FlowRef) { + fn push_next_anonymous_flow( + &mut self, + context: &SharedStyleContext, + parent: &FlowRef, + ) + where + E: TElement, + { let parent_class = self.stack.last().unwrap_or(parent).class(); match parent_class { FlowClass::TableRow => { - self.push_new_anonymous_flow(context, - parent, - &[PseudoElement::ServoAnonymousTableCell], - SpecificFragmentInfo::TableCell, - TableCellFlow::from_fragment) + self.push_new_anonymous_flow::( + context, + parent, + &[PseudoElement::ServoAnonymousTableCell], + SpecificFragmentInfo::TableCell, + TableCellFlow::from_fragment, + ) } FlowClass::Table | FlowClass::TableRowGroup => { - self.push_new_anonymous_flow(context, - parent, - &[PseudoElement::ServoAnonymousTableRow], - SpecificFragmentInfo::TableRow, - TableRowFlow::from_fragment) + self.push_new_anonymous_flow::( + context, + parent, + &[PseudoElement::ServoAnonymousTableRow], + SpecificFragmentInfo::TableRow, + TableRowFlow::from_fragment, + ) } FlowClass::TableWrapper => { - self.push_new_anonymous_flow(context, - parent, - &[PseudoElement::ServoAnonymousTable], - SpecificFragmentInfo::Table, - TableFlow::from_fragment) + self.push_new_anonymous_flow::( + context, + parent, + &[PseudoElement::ServoAnonymousTable], + SpecificFragmentInfo::Table, + TableFlow::from_fragment, + ) } _ => { - self.push_new_anonymous_flow(context, - parent, - &[PseudoElement::ServoTableWrapper, - PseudoElement::ServoAnonymousTableWrapper], - SpecificFragmentInfo::TableWrapper, - TableWrapperFlow::from_fragment) + self.push_new_anonymous_flow::( + context, + parent, + &[PseudoElement::ServoTableWrapper, + PseudoElement::ServoAnonymousTableWrapper], + SpecificFragmentInfo::TableWrapper, + TableWrapperFlow::from_fragment, + ) } } } /// Creates an anonymous flow and pushes it onto the stack. - fn push_new_anonymous_flow(&mut self, - context: &SharedStyleContext, - reference: &FlowRef, - pseudos: &[PseudoElement], - specific_fragment_info: SpecificFragmentInfo, - constructor: extern "Rust" fn(Fragment) -> F) - where F: Flow { - let new_flow = Legalizer::create_anonymous_flow(context, - reference, - pseudos, - specific_fragment_info, - constructor); + fn push_new_anonymous_flow( + &mut self, + context: &SharedStyleContext, + reference: &FlowRef, + pseudos: &[PseudoElement], + specific_fragment_info: SpecificFragmentInfo, + constructor: extern "Rust" fn(Fragment) -> F, + ) + where + E: TElement, + F: Flow, + { + let new_flow = Self::create_anonymous_flow::( + context, + reference, + pseudos, + specific_fragment_info, + constructor, + ); self.stack.push(new_flow) } @@ -2053,21 +2168,31 @@ impl Legalizer { /// /// This method invokes the supplied constructor function on the given specific fragment info /// in order to actually generate the flow. - fn create_anonymous_flow(context: &SharedStyleContext, - reference: &FlowRef, - pseudos: &[PseudoElement], - specific_fragment_info: SpecificFragmentInfo, - constructor: extern "Rust" fn(Fragment) -> F) - -> FlowRef - where F: Flow { + fn create_anonymous_flow( + context: &SharedStyleContext, + reference: &FlowRef, + pseudos: &[PseudoElement], + specific_fragment_info: SpecificFragmentInfo, + constructor: extern "Rust" fn(Fragment) -> F, + ) -> FlowRef + where + E: TElement, + F: Flow, + { let reference_block = reference.as_block(); let mut new_style = reference_block.fragment.style.clone(); for pseudo in pseudos { - new_style = context.stylist.style_for_anonymous(&context.guards, pseudo, &new_style) + new_style = context.stylist.style_for_anonymous::( + &context.guards, + pseudo, + &new_style, + ); } - let fragment = reference_block.fragment - .create_similar_anonymous_fragment(new_style, - specific_fragment_info); + let fragment = + reference_block.fragment.create_similar_anonymous_fragment( + new_style, + specific_fragment_info, + ); FlowRef::new(Arc::new(constructor(fragment))) } } diff --git a/components/script_layout_interface/lib.rs b/components/script_layout_interface/lib.rs index 0538aabafd0..45c82ee1089 100644 --- a/components/script_layout_interface/lib.rs +++ b/components/script_layout_interface/lib.rs @@ -7,6 +7,7 @@ //! to depend on script. #![deny(unsafe_code)] +#![feature(associated_type_defaults)] extern crate app_units; extern crate atomic_refcell; diff --git a/components/script_layout_interface/wrapper_traits.rs b/components/script_layout_interface/wrapper_traits.rs index dc1f5db4c54..17a9172cd71 100644 --- a/components/script_layout_interface/wrapper_traits.rs +++ b/components/script_layout_interface/wrapper_traits.rs @@ -19,7 +19,7 @@ use std::fmt::Debug; use style::attr::AttrValue; use style::context::SharedStyleContext; use style::data::ElementData; -use style::dom::{LayoutIterator, NodeInfo, TNode}; +use style::dom::{LayoutIterator, NodeInfo, TElement, TNode}; use style::dom::OpaqueNode; use style::font_metrics::ServoMetricsProvider; use style::properties::{CascadeFlags, ComputedValues}; @@ -148,6 +148,8 @@ impl Iterator for TreeIterator /// node does not allow any parents or siblings of nodes to be accessed, to avoid races. pub trait ThreadSafeLayoutNode: Clone + Copy + Debug + GetLayoutData + NodeInfo + PartialEq + Sized { type ConcreteNode: LayoutNode; + type ConcreteElement: TElement = ::ConcreteElement; + type ConcreteThreadSafeLayoutElement: ThreadSafeLayoutElement + ::selectors::Element; @@ -291,6 +293,10 @@ pub trait ThreadSafeLayoutElement { type ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode; + /// This type alias is just a hack to avoid writing the monstrosity after it + /// twice. + type ConcreteElement: TElement = ::ConcreteElement; + fn as_node(&self) -> Self::ConcreteThreadSafeLayoutNode; /// Creates a new `ThreadSafeLayoutElement` for the same `LayoutElement` @@ -307,8 +313,7 @@ pub trait ThreadSafeLayoutElement /// /// We need this so that the functions defined on this trait can call /// lazily_compute_pseudo_element_style, which operates on TElement. - unsafe fn unsafe_get(self) -> - <::ConcreteNode as TNode>::ConcreteElement; + unsafe fn unsafe_get(self) -> Self::ConcreteElement; #[inline] fn get_attr(&self, namespace: &Namespace, name: &LocalName) -> Option<&str>; @@ -382,7 +387,7 @@ pub trait ThreadSafeLayoutElement .unwrap().clone() }, PseudoElementCascadeType::Precomputed => { - context.stylist.precomputed_values_for_pseudo( + context.stylist.precomputed_values_for_pseudo::( &context.guards, &style_pseudo, Some(data.styles.primary()), diff --git a/components/style/animation.rs b/components/style/animation.rs index ada5ccad40f..ab482862358 100644 --- a/components/style/animation.rs +++ b/components/style/animation.rs @@ -8,7 +8,7 @@ use Atom; use bezier::Bezier; use context::SharedStyleContext; -use dom::OpaqueNode; +use dom::{OpaqueNode, TElement}; use font_metrics::FontMetricsProvider; use properties::{self, CascadeFlags, ComputedValues, LonghandId}; use properties::animated_properties::{AnimatedProperty, TransitionProperty}; @@ -458,12 +458,16 @@ pub fn start_transitions_if_applicable( had_animations } -fn compute_style_for_animation_step(context: &SharedStyleContext, - step: &KeyframesStep, - previous_style: &ComputedValues, - style_from_cascade: &Arc, - font_metrics_provider: &FontMetricsProvider) - -> Arc { +fn compute_style_for_animation_step( + context: &SharedStyleContext, + step: &KeyframesStep, + previous_style: &ComputedValues, + style_from_cascade: &Arc, + font_metrics_provider: &FontMetricsProvider, +) -> Arc +where + E: TElement, +{ match step.value { KeyframesStepValue::ComputedValues => style_from_cascade.clone(), KeyframesStepValue::Declarations { block: ref declarations } => { @@ -482,20 +486,23 @@ fn compute_style_for_animation_step(context: &SharedStyleContext, // This currently ignores visited styles, which seems acceptable, // as existing browsers don't appear to animate visited styles. let computed = - properties::apply_declarations(context.stylist.device(), - /* pseudo = */ None, - previous_style.rules(), - &context.guards, - iter, - Some(previous_style), - Some(previous_style), - Some(previous_style), - /* visited_style = */ None, - font_metrics_provider, - CascadeFlags::empty(), - context.quirks_mode(), - /* rule_cache = */ None, - &mut Default::default()); + properties::apply_declarations::( + context.stylist.device(), + /* pseudo = */ None, + previous_style.rules(), + &context.guards, + iter, + Some(previous_style), + Some(previous_style), + Some(previous_style), + /* visited_style = */ None, + font_metrics_provider, + CascadeFlags::empty(), + context.quirks_mode(), + /* rule_cache = */ None, + &mut Default::default(), + /* element = */ None, + ); computed } } @@ -503,11 +510,12 @@ fn compute_style_for_animation_step(context: &SharedStyleContext, /// Triggers animations for a given node looking at the animation property /// values. -pub fn maybe_start_animations(context: &SharedStyleContext, - new_animations_sender: &Sender, - node: OpaqueNode, - new_style: &Arc) - -> bool { +pub fn maybe_start_animations( + context: &SharedStyleContext, + new_animations_sender: &Sender, + node: OpaqueNode, + new_style: &Arc, +) -> bool { let mut had_animations = false; let box_style = new_style.get_box(); @@ -599,10 +607,15 @@ pub fn update_style_for_animation_frame(mut new_style: &mut Arc, } /// Updates a single animation and associated style based on the current time. /// If `damage` is provided, inserts the appropriate restyle damage. -pub fn update_style_for_animation(context: &SharedStyleContext, - animation: &Animation, - style: &mut Arc, - font_metrics_provider: &FontMetricsProvider) { +pub fn update_style_for_animation( + context: &SharedStyleContext, + animation: &Animation, + style: &mut Arc, + font_metrics_provider: &FontMetricsProvider, +) +where + E: TElement, +{ debug!("update_style_for_animation: entering"); debug_assert!(!animation.is_expired()); @@ -724,11 +737,13 @@ pub fn update_style_for_animation(context: &SharedStyleContext, let relative_progress = (now - last_keyframe_ended_at) / relative_duration; // TODO: How could we optimise it? Is it such a big deal? - let from_style = compute_style_for_animation_step(context, - last_keyframe, - &**style, - &state.cascade_style, - font_metrics_provider); + let from_style = compute_style_for_animation_step::( + context, + last_keyframe, + &**style, + &state.cascade_style, + font_metrics_provider, + ); // NB: The spec says that the timing function can be overwritten // from the keyframe style. @@ -739,11 +754,13 @@ pub fn update_style_for_animation(context: &SharedStyleContext, timing_function = from_style.get_box().animation_timing_function_at(0); } - let target_style = compute_style_for_animation_step(context, - target_keyframe, - &from_style, - &state.cascade_style, - font_metrics_provider); + let target_style = compute_style_for_animation_step::( + context, + target_keyframe, + &from_style, + &state.cascade_style, + font_metrics_provider, + ); let mut new_style = (*style).clone(); diff --git a/components/style/matching.rs b/components/style/matching.rs index 37694bc0538..5145e26ec36 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -460,10 +460,12 @@ trait PrivateMatchMethods: TElement { // See #12171 and the associated PR for an example where this // happened while debugging other release panic. if !running_animation.is_expired() { - animation::update_style_for_animation(context, - running_animation, - style, - font_metrics); + animation::update_style_for_animation::( + context, + running_animation, + style, + font_metrics, + ); if let Animation::Transition(_, _, ref frame, _) = *running_animation { possibly_expired_animations.push(frame.property_animation.clone()) } diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index a30fa6c0e0a..8e551312044 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -12,6 +12,7 @@ #[cfg(feature = "servo")] use app_units::Au; +use dom::TElement; use custom_properties::CustomPropertiesBuilder; use servo_arc::{Arc, UniqueArc}; use smallbitvec::SmallBitVec; @@ -3188,7 +3189,7 @@ bitflags! { /// Returns the computed values. /// * `flags`: Various flags. /// -pub fn cascade( +pub fn cascade( device: &Device, pseudo: Option<<&PseudoElement>, rule_node: &StrongRuleNode, @@ -3202,7 +3203,11 @@ pub fn cascade( quirks_mode: QuirksMode, rule_cache: Option<<&RuleCache>, rule_cache_conditions: &mut RuleCacheConditions, -) -> Arc { + element: Option, +) -> Arc +where + E: TElement, +{ debug_assert_eq!(parent_style.is_some(), parent_style_ignoring_first_line.is_some()); let empty = SmallBitVec::new(); @@ -3261,12 +3266,13 @@ pub fn cascade( quirks_mode, rule_cache, rule_cache_conditions, + element, ) } /// NOTE: This function expects the declaration with more priority to appear /// first. -pub fn apply_declarations<'a, F, I>( +pub fn apply_declarations<'a, E, F, I>( device: &Device, pseudo: Option<<&PseudoElement>, rules: &StrongRuleNode, @@ -3281,8 +3287,10 @@ pub fn apply_declarations<'a, F, I>( quirks_mode: QuirksMode, rule_cache: Option<<&RuleCache>, rule_cache_conditions: &mut RuleCacheConditions, + _element: Option, ) -> Arc where + E: TElement, F: Fn() -> I, I: Iterator, { diff --git a/components/style/style_resolver.rs b/components/style/style_resolver.rs index ea3e8a6b9e3..2a30aa1659b 100644 --- a/components/style/style_resolver.rs +++ b/components/style/style_resolver.rs @@ -592,6 +592,7 @@ where self.context.shared.quirks_mode(), Some(&self.context.thread_local.rule_cache), &mut conditions, + Some(self.element), ); self.context diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 88689d92890..d43f7ed6740 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -642,14 +642,20 @@ impl Stylist { /// parent; otherwise, non-inherited properties are reset to their initial /// values. The flow constructor uses this flag when constructing anonymous /// flows. - pub fn precomputed_values_for_pseudo( + /// + /// TODO(emilio): The type parameter could go away with a void type + /// implementing TElement. + pub fn precomputed_values_for_pseudo( &self, guards: &StylesheetGuards, pseudo: &PseudoElement, parent: Option<&ComputedValues>, cascade_flags: CascadeFlags, - font_metrics: &FontMetricsProvider - ) -> Arc { + font_metrics: &FontMetricsProvider, + ) -> Arc + where + E: TElement, + { debug_assert!(pseudo.is_precomputed()); let rule_node = self.rule_node_for_precomputed_pseudo( @@ -658,7 +664,7 @@ impl Stylist { None, ); - self.precomputed_values_for_pseudo_with_rule_node( + self.precomputed_values_for_pseudo_with_rule_node::( guards, pseudo, parent, @@ -670,7 +676,10 @@ impl Stylist { /// Computes the style for a given "precomputed" pseudo-element with /// given rule node. - pub fn precomputed_values_for_pseudo_with_rule_node( + /// + /// TODO(emilio): The type parameter could go away with a void type + /// implementing TElement. + pub fn precomputed_values_for_pseudo_with_rule_node( &self, guards: &StylesheetGuards, pseudo: &PseudoElement, @@ -678,8 +687,11 @@ impl Stylist { cascade_flags: CascadeFlags, font_metrics: &FontMetricsProvider, rule_node: StrongRuleNode - ) -> Arc { - self.compute_pseudo_element_style_with_inputs( + ) -> Arc + where + E: TElement, + { + self.compute_pseudo_element_style_with_inputs::( &CascadeInputs { rules: Some(rule_node), visited_rules: None, @@ -689,6 +701,7 @@ impl Stylist { parent, font_metrics, cascade_flags, + None, ).unwrap() } @@ -730,13 +743,19 @@ impl Stylist { } /// Returns the style for an anonymous box of the given type. + /// + /// TODO(emilio): The type parameter could go away with a void type + /// implementing TElement. #[cfg(feature = "servo")] - pub fn style_for_anonymous( + pub fn style_for_anonymous( &self, guards: &StylesheetGuards, pseudo: &PseudoElement, - parent_style: &ComputedValues - ) -> Arc { + parent_style: &ComputedValues, + ) -> Arc + where + E: TElement, + { use font_metrics::ServoMetricsProvider; // For most (but not all) pseudo-elements, we inherit all values from the parent. @@ -771,12 +790,12 @@ impl Stylist { if inherit_all { cascade_flags.insert(CascadeFlags::INHERIT_ALL); } - self.precomputed_values_for_pseudo( + self.precomputed_values_for_pseudo::( guards, &pseudo, Some(parent_style), cascade_flags, - &ServoMetricsProvider + &ServoMetricsProvider, ) } @@ -818,6 +837,7 @@ impl Stylist { Some(parent_style), font_metrics, CascadeFlags::empty(), + Some(element), ) } @@ -825,7 +845,7 @@ impl Stylist { /// This can be used for truly lazy pseudo-elements or to avoid redoing /// selector matching for eager pseudo-elements when we need to recompute /// their style with a new parent style. - pub fn compute_pseudo_element_style_with_inputs( + pub fn compute_pseudo_element_style_with_inputs( &self, inputs: &CascadeInputs, pseudo: &PseudoElement, @@ -833,7 +853,11 @@ impl Stylist { parent_style: Option<&ComputedValues>, font_metrics: &FontMetricsProvider, cascade_flags: CascadeFlags, - ) -> Option> { + element: Option, + ) -> Option> + where + E: TElement, + { // We may have only visited rules in cases when we are actually // resolving, not probing, pseudo-element style. if inputs.rules.is_none() && inputs.visited_rules.is_none() { @@ -861,6 +885,7 @@ impl Stylist { parent_style, font_metrics, cascade_flags, + element, )) } @@ -879,7 +904,7 @@ impl Stylist { /// /// is_link should be true if we're computing style for a link; that affects /// how :visited handling is done. - pub fn compute_style_with_inputs( + pub fn compute_style_with_inputs( &self, inputs: &CascadeInputs, pseudo: Option<&PseudoElement>, @@ -888,8 +913,12 @@ impl Stylist { parent_style_ignoring_first_line: Option<&ComputedValues>, layout_parent_style: Option<&ComputedValues>, font_metrics: &FontMetricsProvider, - cascade_flags: CascadeFlags - ) -> Arc { + cascade_flags: CascadeFlags, + element: Option, + ) -> Arc + where + E: TElement, + { // We need to compute visited values if we have visited rules or if our // parent has visited values. let mut visited_values = None; @@ -928,7 +957,7 @@ impl Stylist { }); } - visited_values = Some(properties::cascade( + visited_values = Some(properties::cascade::( &self.device, pseudo, rule_node, @@ -942,6 +971,7 @@ impl Stylist { self.quirks_mode, /* rule_cache = */ None, &mut Default::default(), + element, )); } @@ -953,7 +983,7 @@ impl Stylist { // difficult to assert that display: contents nodes never arrive here // (tl;dr: It doesn't apply for replaced elements and such, but the // computed value is still "contents"). - properties::cascade( + properties::cascade::( &self.device, pseudo, rules, @@ -967,6 +997,7 @@ impl Stylist { self.quirks_mode, /* rule_cache = */ None, &mut Default::default(), + None, ) } @@ -1515,14 +1546,28 @@ impl Stylist { } /// Computes styles for a given declaration with parent_style. - pub fn compute_for_declarations( + /// + /// FIXME(emilio): the lack of pseudo / cascade flags look quite dubious, + /// hopefully this is only used for some canvas font stuff. + /// + /// TODO(emilio): The type parameter can go away when + /// https://github.com/rust-lang/rust/issues/35121 is fixed. + pub fn compute_for_declarations( &self, guards: &StylesheetGuards, parent_style: &ComputedValues, declarations: Arc>, - ) -> Arc { + ) -> Arc + where + E: TElement, + { use font_metrics::get_metrics_provider_for_product; + // FIXME(emilio): Why do we even need the rule node? We should probably + // just avoid allocating it and calling `apply_declarations` directly, + // maybe... + // + // Also the `vec!` is super-wasteful. let v = vec![ApplicableDeclarationBlock::from_declarations( declarations.clone(), CascadeLevel::StyleAttributeNormal @@ -1537,7 +1582,7 @@ impl Stylist { let metrics = get_metrics_provider_for_product(); // FIXME(emilio): the pseudo bit looks quite dubious! - properties::cascade( + properties::cascade::( &self.device, /* pseudo = */ None, &rule_node, @@ -1551,20 +1596,24 @@ impl Stylist { self.quirks_mode, /* rule_cache = */ None, &mut Default::default(), + None, ) } /// Accessor for a shared reference to the device. + #[inline] pub fn device(&self) -> &Device { &self.device } /// Accessor for a mutable reference to the device. + #[inline] pub fn device_mut(&mut self) -> &mut Device { &mut self.device } /// Accessor for a shared reference to the rule tree. + #[inline] pub fn rule_tree(&self) -> &RuleTree { &self.rule_tree } diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 06c76f3e7f0..fdaa9ab09a4 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -2040,7 +2040,7 @@ pub extern "C" fn Servo_ComputedValues_GetForAnonymousBox(parent_style_or_null: ); let cascade_flags = CascadeFlags::SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP; - data.stylist.precomputed_values_for_pseudo_with_rule_node( + data.stylist.precomputed_values_for_pseudo_with_rule_node::( &guards, &pseudo, parent_style_or_null.map(|x| &*x), @@ -2231,15 +2231,15 @@ fn get_pseudo_style( let guards = StylesheetGuards::same(guard); let metrics = get_metrics_provider_for_product(); let inputs = CascadeInputs::new_from_style(pseudo_styles); - doc_data.stylist - .compute_pseudo_element_style_with_inputs( - &inputs, - pseudo, - &guards, - Some(inherited_styles), - &metrics, - CascadeFlags::empty(), - ) + doc_data.stylist.compute_pseudo_element_style_with_inputs( + &inputs, + pseudo, + &guards, + Some(inherited_styles), + &metrics, + CascadeFlags::empty(), + Some(element), + ) }) }, _ => { @@ -3655,6 +3655,7 @@ pub extern "C" fn Servo_ReparentStyle( Some(layout_parent_style), &metrics, cascade_flags, + element, ).into() } @@ -4297,7 +4298,7 @@ pub extern "C" fn Servo_StyleSet_ResolveForDeclarations( let declarations = Locked::::as_arc(&declarations); - doc_data.stylist.compute_for_declarations( + doc_data.stylist.compute_for_declarations::( &guards, parent_style, declarations.clone_arc(), From 104f5c2553606d0b26d7cf2ad9b90eb2efd94792 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 22 Jan 2018 23:50:40 +0100 Subject: [PATCH 2/3] style: Derive debug for CascadeInputs. It no longer has anything than rules. --- components/layout_thread/dom_wrapper.rs | 2 ++ components/layout_thread/lib.rs | 25 +++++++++++-------- .../script_layout_interface/wrapper_traits.rs | 10 +++++--- components/style/context.rs | 11 +------- 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/components/layout_thread/dom_wrapper.rs b/components/layout_thread/dom_wrapper.rs index 6eef1106c3d..f900854d1c4 100644 --- a/components/layout_thread/dom_wrapper.rs +++ b/components/layout_thread/dom_wrapper.rs @@ -860,6 +860,7 @@ impl<'ln> NodeInfo for ServoThreadSafeLayoutNode<'ln> { impl<'ln> ThreadSafeLayoutNode for ServoThreadSafeLayoutNode<'ln> { type ConcreteNode = ServoLayoutNode<'ln>; type ConcreteThreadSafeLayoutElement = ServoThreadSafeLayoutElement<'ln>; + type ConcreteElement = ServoLayoutElement<'ln>; type ChildrenIterator = ThreadSafeLayoutNodeChildrenIterator; fn opaque(&self) -> OpaqueNode { @@ -1084,6 +1085,7 @@ pub struct ServoThreadSafeLayoutElement<'le> { impl<'le> ThreadSafeLayoutElement for ServoThreadSafeLayoutElement<'le> { type ConcreteThreadSafeLayoutNode = ServoThreadSafeLayoutNode<'le>; + type ConcreteElement = ServoLayoutElement<'le>; fn as_node(&self) -> ServoThreadSafeLayoutNode<'le> { ServoThreadSafeLayoutNode { diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index 3fb7d0c8c73..fcd790fe6ec 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -1491,8 +1491,11 @@ impl LayoutThread { self.profiler_metadata(), self.time_profiler_chan.clone(), || { - animation::recalc_style_for_animations( - &layout_context, FlowRef::deref_mut(&mut root_flow), &animations) + animation::recalc_style_for_animations::( + &layout_context, + FlowRef::deref_mut(&mut root_flow), + &animations, + ) }); } self.perform_post_style_recalc_layout_passes(&mut root_flow, @@ -1522,14 +1525,16 @@ impl LayoutThread { .as_mut() .map(|nodes| &mut **nodes); // Kick off animations if any were triggered, expire completed ones. - animation::update_animation_state(&self.constellation_chan, - &self.script_chan, - &mut *self.running_animations.write(), - &mut *self.expired_animations.write(), - newly_transitioning_nodes, - &self.new_animations_receiver, - self.id, - &self.timer); + animation::update_animation_state::( + &self.constellation_chan, + &self.script_chan, + &mut *self.running_animations.write(), + &mut *self.expired_animations.write(), + newly_transitioning_nodes, + &self.new_animations_receiver, + self.id, + &self.timer, + ); } profile(time::ProfilerCategory::LayoutRestyleDamagePropagation, diff --git a/components/script_layout_interface/wrapper_traits.rs b/components/script_layout_interface/wrapper_traits.rs index 17a9172cd71..ffdc9aa95ed 100644 --- a/components/script_layout_interface/wrapper_traits.rs +++ b/components/script_layout_interface/wrapper_traits.rs @@ -148,7 +148,7 @@ impl Iterator for TreeIterator /// node does not allow any parents or siblings of nodes to be accessed, to avoid races. pub trait ThreadSafeLayoutNode: Clone + Copy + Debug + GetLayoutData + NodeInfo + PartialEq + Sized { type ConcreteNode: LayoutNode; - type ConcreteElement: TElement = ::ConcreteElement; + type ConcreteElement: TElement; type ConcreteThreadSafeLayoutElement: ThreadSafeLayoutElement @@ -293,9 +293,11 @@ pub trait ThreadSafeLayoutElement { type ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode; - /// This type alias is just a hack to avoid writing the monstrosity after it - /// twice. - type ConcreteElement: TElement = ::ConcreteElement; + /// This type alias is just a work-around to avoid writing + /// + /// ::ConcreteElement + /// + type ConcreteElement: TElement; fn as_node(&self) -> Self::ConcreteThreadSafeLayoutNode; diff --git a/components/style/context.rs b/components/style/context.rs index 6c9ba7b47f6..3768289da0e 100644 --- a/components/style/context.rs +++ b/components/style/context.rs @@ -203,7 +203,7 @@ impl<'a> SharedStyleContext<'a> { /// within the `CurrentElementInfo`. At the end of the cascade, they are folded /// down into the main `ComputedValues` to reduce memory usage per element while /// still remaining accessible. -#[derive(Clone, Default)] +#[derive(Clone, Debug, Default)] pub struct CascadeInputs { /// The rule node representing the ordered list of rules matched for this /// node. @@ -226,15 +226,6 @@ impl CascadeInputs { } } -// We manually implement Debug for CascadeInputs so that we can avoid the -// verbose stringification of ComputedValues for normal logging. -impl fmt::Debug for CascadeInputs { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "CascadeInputs {{ rules: {:?}, visited_rules: {:?}, .. }}", - self.rules, self.visited_rules) - } -} - /// A list of cascade inputs for eagerly-cascaded pseudo-elements. /// The list is stored inline. #[derive(Debug)] From cd04664fb987ebfab063cbbff1a2516bd16b8cd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 22 Jan 2018 23:53:03 +0100 Subject: [PATCH 3/3] style: Use CascadeFlags for what they're for. Now that we have an Element around on cascade, we can stop using the cascade flags mechanism to pass various element-related state, like "is this element the root", or "should it use the item-based display fixup". That fixes handwaviness in the handling of those flags from style reparenting, and code duplication to handle tricky stuff like :visited. There are a number of other changes that are worth noticing: * skip_root_and_item_based_display_fixup is renamed to skip_item_display_fixup: TElement::is_root() already implies being the document element, which by definition is not native anonymous and not a pseudo-element. Thus, you never get fixed-up if your NAC or a pseudo, which is what the code tried to avoid, so the only fixup with a point is the item one, which is necessary. * The pseudo-element probing code was refactored to return early a Option::::None, which is nicer than what it was doing. * The visited_links_enabled check has moved to selector-matching time. The rest of the checks aren't based on whether the element is a link, or are properly guarded by parent_style.visited_style().is_some() or visited_rules.is_some(). Thus you can transitively infer that no element will end up with a :visited style, not even from style reparenting. Anyway, the underlying reason why I want the element in StyleAdjuster is because we're going to implement an adjustment in there depending on the tag of the element (converting display: contents to display: none depending on the tag), so computing that information eagerly, including a hash lookup, wouldn't be nice. --- components/layout_thread/dom_wrapper.rs | 2 +- .../script_layout_interface/wrapper_traits.rs | 3 +- components/style/dom.rs | 2 +- components/style/gecko/pseudo_element.rs | 13 +- components/style/gecko/wrapper.rs | 14 +- .../style/properties/properties.mako.rs | 35 +---- components/style/servo/selector_parser.rs | 46 +++++- components/style/style_adjuster.rs | 92 ++++++++--- components/style/style_resolver.rs | 145 +++++------------ components/style/stylist.rs | 146 ++++++------------ ports/geckolib/glue.rs | 33 ++-- 11 files changed, 233 insertions(+), 298 deletions(-) diff --git a/components/layout_thread/dom_wrapper.rs b/components/layout_thread/dom_wrapper.rs index f900854d1c4..0110704edad 100644 --- a/components/layout_thread/dom_wrapper.rs +++ b/components/layout_thread/dom_wrapper.rs @@ -455,7 +455,7 @@ impl<'le> TElement for ServoLayoutElement<'le> { } } - fn skip_root_and_item_based_display_fixup(&self) -> bool { + fn skip_item_display_fixup(&self) -> bool { false } diff --git a/components/script_layout_interface/wrapper_traits.rs b/components/script_layout_interface/wrapper_traits.rs index ffdc9aa95ed..0b04d4256a6 100644 --- a/components/script_layout_interface/wrapper_traits.rs +++ b/components/script_layout_interface/wrapper_traits.rs @@ -22,7 +22,7 @@ use style::data::ElementData; use style::dom::{LayoutIterator, NodeInfo, TElement, TNode}; use style::dom::OpaqueNode; use style::font_metrics::ServoMetricsProvider; -use style::properties::{CascadeFlags, ComputedValues}; +use style::properties::ComputedValues; use style::selector_parser::{PseudoElement, PseudoElementCascadeType, SelectorImpl}; use style::stylist::RuleInclusion; use webrender_api::ClipId; @@ -393,7 +393,6 @@ pub trait ThreadSafeLayoutElement &context.guards, &style_pseudo, Some(data.styles.primary()), - CascadeFlags::empty(), &ServoMetricsProvider, ) } diff --git a/components/style/dom.rs b/components/style/dom.rs index bfa0b6078ac..88cd7a82829 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -680,7 +680,7 @@ pub trait TElement /// Whether we should skip any root- or item-based display property /// blockification on this element. (This function exists so that Gecko /// native anonymous content can opt out of this style fixup.) - fn skip_root_and_item_based_display_fixup(&self) -> bool; + fn skip_item_display_fixup(&self) -> bool; /// Sets selector flags, which indicate what kinds of selectors may have /// matched on this element and therefore what kind of work may need to diff --git a/components/style/gecko/pseudo_element.rs b/components/style/gecko/pseudo_element.rs index 852498ab9ec..ab38bdff798 100644 --- a/components/style/gecko/pseudo_element.rs +++ b/components/style/gecko/pseudo_element.rs @@ -10,7 +10,7 @@ use cssparser::{ToCss, serialize_identifier}; use gecko_bindings::structs::{self, CSSPseudoElementType}; -use properties::{ComputedValues, PropertyFlags}; +use properties::{CascadeFlags, ComputedValues, PropertyFlags}; use properties::longhands::display::computed_value::T as Display; use selector_parser::{NonTSPseudoClass, PseudoElementCascadeType, SelectorImpl}; use std::fmt; @@ -51,6 +51,15 @@ impl PseudoElement { PseudoElementCascadeType::Lazy } + /// The CascadeFlags needed to cascade this pseudo-element. + /// + /// This is only needed to support the broken INHERIT_ALL pseudo mode for + /// Servo. + #[inline] + pub fn cascade_flags(&self) -> CascadeFlags { + CascadeFlags::empty() + } + /// Whether the pseudo-element should inherit from the default computed /// values instead of from the parent element. /// @@ -128,7 +137,7 @@ impl PseudoElement { /// Whether this pseudo-element skips flex/grid container display-based /// fixup. #[inline] - pub fn skip_item_based_display_fixup(&self) -> bool { + pub fn skip_item_display_fixup(&self) -> bool { (self.flags() & structs::CSS_PSEUDO_ELEMENT_IS_FLEX_OR_GRID_ITEM) == 0 } diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index de4855c4ed4..92aedc3b5d7 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -1285,15 +1285,11 @@ impl<'le> TElement for GeckoElement<'le> { } #[inline] - fn skip_root_and_item_based_display_fixup(&self) -> bool { - if !self.is_native_anonymous() { - return false; - } - - if let Some(p) = self.implemented_pseudo_element() { - return p.skip_item_based_display_fixup(); - } - + fn skip_item_display_fixup(&self) -> bool { + debug_assert!( + self.implemented_pseudo_element().is_none(), + "Just don't call me if I'm a pseudo, you should know the answer already" + ); self.is_root_of_native_anonymous_subtree() } diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 8e551312044..e87f6758b3c 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -3148,30 +3148,8 @@ bitflags! { /// present, non-inherited styles are reset to their initial values. const INHERIT_ALL = 1; - /// Whether to skip any display style fixup for root element, flex/grid - /// item, and ruby descendants. - const SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP = 1 << 1; - /// Whether to only cascade properties that are visited dependent. - const VISITED_DEPENDENT_ONLY = 1 << 2; - - /// Whether the given element we're styling is the document element, - /// that is, matches :root. - /// - /// Not set for native anonymous content since some NAC form their own - /// root, but share the device. - /// - /// This affects some style adjustments, like blockification, and means - /// that it may affect global state, like the Device's root font-size. - const IS_ROOT_ELEMENT = 1 << 3; - - /// Whether we're computing the style of a link, either visited or - /// unvisited. - const IS_LINK = 1 << 4; - - /// Whether we're computing the style of a link element that happens to - /// be visited. - const IS_VISITED_LINK = 1 << 5; + const VISITED_DEPENDENT_ONLY = 1 << 1; } } @@ -3287,7 +3265,7 @@ pub fn apply_declarations<'a, E, F, I>( quirks_mode: QuirksMode, rule_cache: Option<<&RuleCache>, rule_cache_conditions: &mut RuleCacheConditions, - _element: Option, + element: Option, ) -> Arc where E: TElement, @@ -3326,7 +3304,7 @@ where }; let mut context = computed::Context { - is_root_element: flags.contains(CascadeFlags::IS_ROOT_ELEMENT), + is_root_element: pseudo.is_none() && element.map_or(false, |e| e.is_root()), // We'd really like to own the rules here to avoid refcount traffic, but // animation's usage of `apply_declarations` make this tricky. See bug // 1375525. @@ -3610,8 +3588,11 @@ where builder.clear_modified_reset(); - StyleAdjuster::new(&mut builder) - .adjust(layout_parent_style, flags); + StyleAdjuster::new(&mut builder).adjust( + layout_parent_style, + element, + flags, + ); if builder.modified_reset() || !apply_reset { // If we adjusted any reset structs, we can't cache this ComputedValues. diff --git a/components/style/servo/selector_parser.rs b/components/style/servo/selector_parser.rs index bc6cc31cf7d..04c164e290d 100644 --- a/components/style/servo/selector_parser.rs +++ b/components/style/servo/selector_parser.rs @@ -14,8 +14,7 @@ use element_state::{DocumentState, ElementState}; use fnv::FnvHashMap; use invalidation::element::document_state::InvalidationMatchingData; use invalidation::element::element_wrapper::ElementSnapshot; -use properties::ComputedValues; -use properties::PropertyFlags; +use properties::{CascadeFlags, ComputedValues, PropertyFlags}; use properties::longhands::display::computed_value::T as Display; use selector_parser::{AttrValue as SelectorAttrValue, PseudoElementCascadeType, SelectorParser}; use selectors::attr::{AttrSelectorOperation, NamespaceConstraint, CaseSensitivity}; @@ -174,10 +173,10 @@ impl PseudoElement { self.is_precomputed() } - /// Whether this pseudo-element skips flex/grid container - /// display-based fixup. + /// Whether this pseudo-element skips flex/grid container display-based + /// fixup. #[inline] - pub fn skip_item_based_display_fixup(&self) -> bool { + pub fn skip_item_display_fixup(&self) -> bool { !self.is_before_or_after() } @@ -213,6 +212,43 @@ impl PseudoElement { } } + /// For most (but not all) anon-boxes, we inherit all values from the + /// parent, this is the hook in the style system to allow this. + /// + /// FIXME(emilio): It's likely that this is broken in a variety of + /// situations, and what it really wants is just inherit some reset + /// properties... Also, I guess it just could do all: inherit on the + /// stylesheet, though chances are that'd be kinda slow if we don't cache + /// them... + pub fn cascade_flags(&self) -> CascadeFlags { + match *self { + PseudoElement::After | + PseudoElement::Before | + PseudoElement::Selection | + PseudoElement::DetailsContent | + PseudoElement::DetailsSummary => CascadeFlags::empty(), + // Anonymous table flows shouldn't inherit their parents properties in order + // to avoid doubling up styles such as transformations. + PseudoElement::ServoAnonymousTableCell | + PseudoElement::ServoAnonymousTableRow | + PseudoElement::ServoText | + PseudoElement::ServoInputText => CascadeFlags::empty(), + + // For tables, we do want style to inherit, because TableWrapper is + // responsible for handling clipping and scrolling, while Table is + // responsible for creating stacking contexts. + // + // StackingContextCollectionFlags makes sure this is processed + // properly. + PseudoElement::ServoAnonymousTable | + PseudoElement::ServoAnonymousTableWrapper | + PseudoElement::ServoTableWrapper | + PseudoElement::ServoAnonymousBlock | + PseudoElement::ServoInlineBlockWrapper | + PseudoElement::ServoInlineAbsolute => CascadeFlags::INHERIT_ALL, + } + } + /// Covert non-canonical pseudo-element to canonical one, and keep a /// canonical one as it is. pub fn canonical(&self) -> PseudoElement { diff --git a/components/style/style_adjuster.rs b/components/style/style_adjuster.rs index 052d80dbe22..7f805bb3f5c 100644 --- a/components/style/style_adjuster.rs +++ b/components/style/style_adjuster.rs @@ -6,6 +6,7 @@ //! a computed style needs in order for it to adhere to the CSS spec. use app_units::Au; +use dom::TElement; use properties::{self, CascadeFlags, ComputedValues, StyleBuilder}; use properties::longhands::display::computed_value::T as Display; use properties::longhands::float::computed_value::T as Float; @@ -50,13 +51,30 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { } } + /// Whether we should skip any item-based display property blockification on + /// this element. + fn skip_item_display_fixup(&self, element: Option) -> bool + where + E: TElement, + { + if let Some(pseudo) = self.style.pseudo { + return pseudo.skip_item_display_fixup(); + } + + element.map_or(false, |e| e.skip_item_display_fixup()) + } + + /// Apply the blockification rules based on the table in CSS 2.2 section 9.7. /// - fn blockify_if_necessary( + fn blockify_if_necessary( &mut self, layout_parent_style: &ComputedValues, - flags: CascadeFlags, - ) { + element: Option, + ) + where + E: TElement, + { let mut blockify = false; macro_rules! blockify_if { ($if_what:expr) => { @@ -66,8 +84,9 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { } } - if !flags.contains(CascadeFlags::SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP) { - blockify_if!(flags.contains(CascadeFlags::IS_ROOT_ELEMENT)); + let is_root = self.style.pseudo.is_none() && element.map_or(false, |e| e.is_root()); + blockify_if!(is_root); + if !self.skip_item_display_fixup(element) { blockify_if!(layout_parent_style.get_box().clone_display().is_item_container()); } @@ -81,8 +100,7 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { } let display = self.style.get_box().clone_display(); - let blockified_display = - display.equivalent_block_display(flags.contains(CascadeFlags::IS_ROOT_ELEMENT)); + let blockified_display = display.equivalent_block_display(is_root); if display != blockified_display { self.style.mutate_box().set_adjusted_display( blockified_display, @@ -477,12 +495,14 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { /// * suppress border and padding for ruby level containers, /// * correct unicode-bidi. #[cfg(feature = "gecko")] - fn adjust_for_ruby( + fn adjust_for_ruby( &mut self, layout_parent_style: &ComputedValues, - flags: CascadeFlags, - ) { - use properties::CascadeFlags; + element: Option, + ) + where + E: TElement, + { use properties::computed_value_flags::ComputedValueFlags; use properties::longhands::unicode_bidi::computed_value::T as UnicodeBidi; @@ -491,7 +511,7 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { if self.should_suppress_linebreak(layout_parent_style) { self.style.flags.insert(ComputedValueFlags::SHOULD_SUPPRESS_LINEBREAK); // Inlinify the display type if allowed. - if !flags.contains(CascadeFlags::SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP) { + if !self.skip_item_display_fixup(element) { let inline_display = self_display.inlinify(); if self_display != inline_display { self.style.mutate_box().set_adjusted_display(inline_display, false); @@ -531,16 +551,22 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { /// /// FIXME(emilio): This isn't technically a style adjustment thingie, could /// it move somewhere else? - fn adjust_for_visited(&mut self, flags: CascadeFlags) { - use properties::CascadeFlags; + fn adjust_for_visited(&mut self, element: Option) + where + E: TElement, + { use properties::computed_value_flags::ComputedValueFlags; if !self.style.has_visited_style() { return; } - let relevant_link_visited = if flags.contains(CascadeFlags::IS_LINK) { - flags.contains(CascadeFlags::IS_VISITED_LINK) + let is_link_element = + self.style.pseudo.is_none() && + element.map_or(false, |e| e.is_link()); + + let relevant_link_visited = if is_link_element { + element.unwrap().is_visited_link() } else { self.style.inherited_flags().contains(ComputedValueFlags::IS_RELEVANT_LINK_VISITED) }; @@ -586,11 +612,35 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { /// When comparing to Gecko, this is similar to the work done by /// `nsStyleContext::ApplyStyleFixups`, plus some parts of /// `nsStyleSet::GetContext`. - pub fn adjust( + pub fn adjust( &mut self, layout_parent_style: &ComputedValues, + element: Option, flags: CascadeFlags, - ) { + ) + where + E: TElement, + { + if cfg!(debug_assertions) { + if element.and_then(|e| e.implemented_pseudo_element()).is_some() { + // It'd be nice to assert `self.style.pseudo == Some(&pseudo)`, + // but we do resolve ::-moz-list pseudos on ::before / ::after + // content, sigh. + debug_assert!( + self.style.pseudo.is_some(), + "Someone really messed up" + ); + } + } + // FIXME(emilio): The apply_declarations callsite in Servo's + // animation, and the font stuff for Gecko + // (Stylist::compute_for_declarations) should pass an element to + // cascade(), then we can make this assertion hold everywhere. + // debug_assert!( + // element.is_some() || self.style.pseudo.is_some(), + // "Should always have an element around for non-pseudo styles" + // ); + // Don't adjust visited styles, visited-dependent properties aren't // affected by these adjustments and it'd be just wasted work anyway. // @@ -600,14 +650,14 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { return; } - self.adjust_for_visited(flags); + self.adjust_for_visited(element); #[cfg(feature = "gecko")] { self.adjust_for_prohibited_display_contents(); self.adjust_for_fieldset_content(layout_parent_style); } self.adjust_for_top_layer(); - self.blockify_if_necessary(layout_parent_style, flags); + self.blockify_if_necessary(layout_parent_style, element); self.adjust_for_position(); self.adjust_for_overflow(); #[cfg(feature = "gecko")] @@ -627,7 +677,7 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { self.adjust_for_text_decoration_lines(layout_parent_style); #[cfg(feature = "gecko")] { - self.adjust_for_ruby(layout_parent_style, flags); + self.adjust_for_ruby(layout_parent_style, element); } #[cfg(feature = "servo")] { diff --git a/components/style/style_resolver.rs b/components/style/style_resolver.rs index 2a30aa1659b..b07dfb487b9 100644 --- a/components/style/style_resolver.rs +++ b/components/style/style_resolver.rs @@ -9,9 +9,8 @@ use context::{CascadeInputs, ElementCascadeInputs, StyleContext}; use data::{ElementStyles, EagerPseudoStyles}; use dom::TElement; use log::LogLevel::Trace; -use matching::{CascadeVisitedMode, MatchMethods}; -use properties::{AnimationRules, CascadeFlags, ComputedValues}; -use properties::cascade; +use matching::MatchMethods; +use properties::{AnimationRules, ComputedValues}; use properties::longhands::display::computed_value::T as Display; use rule_tree::StrongRuleNode; use selector_parser::{PseudoElement, SelectorImpl}; @@ -161,7 +160,8 @@ where parent_style.map_or(false, |s| s.visited_style().is_some()); let visited_rules = - if inside_link || self.element.is_link() { + if self.context.shared.visited_styles_enabled && + (inside_link || self.element.is_link()) { let visited_matching_results = self.match_primary(VisitedHandlingMode::RelevantLinkVisited); Some(visited_matching_results.rule_node) @@ -291,29 +291,37 @@ where layout_parent_style: Option<&ComputedValues>, pseudo: Option<&PseudoElement>, ) -> ResolvedStyle { - let mut style_if_visited = None; - if parent_style.map_or(false, |s| s.visited_style().is_some()) || - inputs.visited_rules.is_some() { - style_if_visited = Some(self.cascade_style( - inputs.visited_rules.as_ref().or(inputs.rules.as_ref()), - /* style_if_visited = */ None, - parent_style, - layout_parent_style, - CascadeVisitedMode::Visited, - pseudo, - )); - } + debug_assert!( + self.element.implemented_pseudo_element().is_none() || pseudo.is_none(), + "Pseudo-elements can't have other pseudos!" + ); + debug_assert!(pseudo.map_or(true, |p| p.is_eager())); - ResolvedStyle( - self.cascade_style( - inputs.rules.as_ref(), - style_if_visited, - parent_style, - layout_parent_style, - CascadeVisitedMode::Unvisited, - pseudo, - ) - ) + let implemented_pseudo = self.element.implemented_pseudo_element(); + let pseudo = pseudo.or(implemented_pseudo.as_ref()); + + let mut conditions = Default::default(); + let values = self.context.shared.stylist.cascade_style_and_visited( + Some(self.element), + pseudo, + inputs, + &self.context.shared.guards, + parent_style, + parent_style, + layout_parent_style, + &self.context.thread_local.font_metrics_provider, + Some(&self.context.thread_local.rule_cache), + &mut conditions, + ); + + self.context.thread_local.rule_cache.insert_if_possible( + &self.context.shared.guards, + &values, + pseudo, + &conditions + ); + + ResolvedStyle(values) } /// Cascade the element and pseudo-element styles with the default parents. @@ -469,7 +477,7 @@ where ) -> Option { debug!("Match pseudo {:?} for {:?}, visited: {:?}", self.element, pseudo_element, visited_handling); - debug_assert!(pseudo_element.is_eager() || pseudo_element.is_lazy()); + debug_assert!(pseudo_element.is_eager()); debug_assert!(self.element.implemented_pseudo_element().is_none(), "Element pseudos can't have any other pseudo."); @@ -524,87 +532,4 @@ where Some(rule_node) } - - fn cascade_style( - &mut self, - rules: Option<&StrongRuleNode>, - style_if_visited: Option>, - mut parent_style: Option<&ComputedValues>, - layout_parent_style: Option<&ComputedValues>, - cascade_visited: CascadeVisitedMode, - pseudo: Option<&PseudoElement>, - ) -> Arc { - debug_assert!( - self.element.implemented_pseudo_element().is_none() || pseudo.is_none(), - "Pseudo-elements can't have other pseudos!" - ); - debug_assert!(pseudo.map_or(true, |p| p.is_eager())); - - let mut cascade_flags = CascadeFlags::empty(); - - if self.element.skip_root_and_item_based_display_fixup() || - pseudo.map_or(false, |p| p.skip_item_based_display_fixup()) { - cascade_flags.insert(CascadeFlags::SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP); - } - - if pseudo.is_none() && self.element.is_link() { - cascade_flags.insert(CascadeFlags::IS_LINK); - if self.element.is_visited_link() && - self.context.shared.visited_styles_enabled { - cascade_flags.insert(CascadeFlags::IS_VISITED_LINK); - } - } - - if cascade_visited.visited_dependent_only() { - // If this element is a link, we want its visited style to inherit - // from the regular style of its parent, because only the - // visitedness of the relevant link should influence style. - if pseudo.is_some() || !self.element.is_link() { - parent_style = parent_style.map(|s| { - s.visited_style().unwrap_or(s) - }); - } - cascade_flags.insert(CascadeFlags::VISITED_DEPENDENT_ONLY); - } - if !self.element.is_native_anonymous() && - pseudo.is_none() && - self.element.is_root() - { - cascade_flags.insert(CascadeFlags::IS_ROOT_ELEMENT); - } - - let implemented_pseudo = self.element.implemented_pseudo_element(); - let pseudo = pseudo.or(implemented_pseudo.as_ref()); - - let mut conditions = Default::default(); - let values = - cascade( - self.context.shared.stylist.device(), - pseudo, - rules.unwrap_or(self.context.shared.stylist.rule_tree().root()), - &self.context.shared.guards, - parent_style, - parent_style, - layout_parent_style, - style_if_visited, - &self.context.thread_local.font_metrics_provider, - cascade_flags, - self.context.shared.quirks_mode(), - Some(&self.context.thread_local.rule_cache), - &mut conditions, - Some(self.element), - ); - - self.context - .thread_local - .rule_cache - .insert_if_possible( - &self.context.shared.guards, - &values, - pseudo, - &conditions - ); - - values - } } diff --git a/components/style/stylist.rs b/components/style/stylist.rs index d43f7ed6740..6ed2b36ac89 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -22,6 +22,7 @@ use malloc_size_of::MallocUnconditionalShallowSizeOf; use media_queries::Device; use properties::{self, CascadeFlags, ComputedValues}; use properties::{AnimationRules, PropertyDeclarationBlock}; +use rule_cache::{RuleCache, RuleCacheConditions}; use rule_tree::{CascadeLevel, RuleTree, StrongRuleNode, StyleSource}; use selector_map::{PrecomputedHashMap, SelectorMap, SelectorMapEntry}; use selector_parser::{SelectorImpl, PerPseudoElementMap, PseudoElement}; @@ -650,7 +651,6 @@ impl Stylist { guards: &StylesheetGuards, pseudo: &PseudoElement, parent: Option<&ComputedValues>, - cascade_flags: CascadeFlags, font_metrics: &FontMetricsProvider, ) -> Arc where @@ -668,7 +668,6 @@ impl Stylist { guards, pseudo, parent, - cascade_flags, font_metrics, rule_node ) @@ -684,25 +683,23 @@ impl Stylist { guards: &StylesheetGuards, pseudo: &PseudoElement, parent: Option<&ComputedValues>, - cascade_flags: CascadeFlags, font_metrics: &FontMetricsProvider, - rule_node: StrongRuleNode + rules: StrongRuleNode ) -> Arc where E: TElement, { self.compute_pseudo_element_style_with_inputs::( - &CascadeInputs { - rules: Some(rule_node), + CascadeInputs { + rules: Some(rules), visited_rules: None, }, pseudo, guards, parent, font_metrics, - cascade_flags, None, - ).unwrap() + ) } /// Returns the rule node for given precomputed pseudo-element. @@ -757,44 +754,10 @@ impl Stylist { E: TElement, { use font_metrics::ServoMetricsProvider; - - // For most (but not all) pseudo-elements, we inherit all values from the parent. - let inherit_all = match *pseudo { - // Anonymous table flows shouldn't inherit their parents properties in order - // to avoid doubling up styles such as transformations. - PseudoElement::ServoAnonymousTableCell | - PseudoElement::ServoAnonymousTableRow | - PseudoElement::ServoText | - PseudoElement::ServoInputText => false, - PseudoElement::ServoAnonymousBlock | - - // For tables, we do want style to inherit, because TableWrapper is responsible - // for handling clipping and scrolling, while Table is responsible for creating - // stacking contexts. StackingContextCollectionFlags makes sure this is processed - // properly. - PseudoElement::ServoAnonymousTable | - PseudoElement::ServoAnonymousTableWrapper | - - PseudoElement::ServoTableWrapper | - PseudoElement::ServoInlineBlockWrapper | - PseudoElement::ServoInlineAbsolute => true, - PseudoElement::Before | - PseudoElement::After | - PseudoElement::Selection | - PseudoElement::DetailsSummary | - PseudoElement::DetailsContent => { - unreachable!("That pseudo doesn't represent an anonymous box!") - } - }; - let mut cascade_flags = CascadeFlags::empty(); - if inherit_all { - cascade_flags.insert(CascadeFlags::INHERIT_ALL); - } self.precomputed_values_for_pseudo::( guards, &pseudo, Some(parent_style), - cascade_flags, &ServoMetricsProvider, ) } @@ -828,17 +791,16 @@ impl Stylist { is_probe, rule_inclusion, matching_fn - ); + )?; - self.compute_pseudo_element_style_with_inputs( - &cascade_inputs, + Some(self.compute_pseudo_element_style_with_inputs( + cascade_inputs, pseudo, guards, Some(parent_style), font_metrics, - CascadeFlags::empty(), Some(element), - ) + )) } /// Computes a pseudo-element style lazily using the given CascadeInputs. @@ -847,23 +809,16 @@ impl Stylist { /// their style with a new parent style. pub fn compute_pseudo_element_style_with_inputs( &self, - inputs: &CascadeInputs, + inputs: CascadeInputs, pseudo: &PseudoElement, guards: &StylesheetGuards, parent_style: Option<&ComputedValues>, font_metrics: &FontMetricsProvider, - cascade_flags: CascadeFlags, element: Option, - ) -> Option> + ) -> Arc where E: TElement, { - // We may have only visited rules in cases when we are actually - // resolving, not probing, pseudo-element style. - if inputs.rules.is_none() && inputs.visited_rules.is_none() { - return None - } - // FIXME(emilio): The lack of layout_parent_style here could be // worrying, but we're probably dropping the display fixup for // pseudos other than before and after, so it's probably ok. @@ -876,17 +831,18 @@ impl Stylist { //
. That is, the computed value of // display for the fieldset is "contents", even though it's not the used // value, so we don't need to adjust in a different way anyway. - Some(self.compute_style_with_inputs( - inputs, + self.cascade_style_and_visited( + element, Some(pseudo), + inputs, guards, parent_style, parent_style, parent_style, font_metrics, - cascade_flags, - element, - )) + /* rule_cache = */ None, + &mut RuleCacheConditions::default(), + ) } /// Computes a style using the given CascadeInputs. This can be used to @@ -904,38 +860,43 @@ impl Stylist { /// /// is_link should be true if we're computing style for a link; that affects /// how :visited handling is done. - pub fn compute_style_with_inputs( + pub fn cascade_style_and_visited( &self, - inputs: &CascadeInputs, + element: Option, pseudo: Option<&PseudoElement>, + inputs: CascadeInputs, guards: &StylesheetGuards, parent_style: Option<&ComputedValues>, parent_style_ignoring_first_line: Option<&ComputedValues>, layout_parent_style: Option<&ComputedValues>, font_metrics: &FontMetricsProvider, - cascade_flags: CascadeFlags, - element: Option, + rule_cache: Option<&RuleCache>, + rule_cache_conditions: &mut RuleCacheConditions, ) -> Arc where E: TElement, { + debug_assert!(pseudo.is_some() || element.is_some(), "Huh?"); + + let cascade_flags = + pseudo.map_or(CascadeFlags::empty(), |p| p.cascade_flags()); + // We need to compute visited values if we have visited rules or if our // parent has visited values. let mut visited_values = None; if inputs.visited_rules.is_some() || parent_style.and_then(|s| s.visited_style()).is_some() { - // At this point inputs may have visited rules, or rules, or both, - // or neither (e.g. if it's a text style it may have neither). So - // we have to be a bit careful here. + // At this point inputs may have visited rules, or rules. let rule_node = match inputs.visited_rules.as_ref() { Some(rules) => rules, - None => inputs.rules.as_ref().unwrap_or(self.rule_tree().root()), + None => inputs.rules.as_ref().unwrap_or(self.rule_tree.root()), }; + let inherited_style; let inherited_style_ignoring_first_line; let layout_parent_style_for_visited; - if cascade_flags.contains(CascadeFlags::IS_LINK) { + if pseudo.is_none() && element.unwrap().is_link() { // We just want to use our parent style as our parent. inherited_style = parent_style; inherited_style_ignoring_first_line = parent_style_ignoring_first_line; @@ -969,24 +930,22 @@ impl Stylist { font_metrics, cascade_flags | CascadeFlags::VISITED_DEPENDENT_ONLY, self.quirks_mode, - /* rule_cache = */ None, - &mut Default::default(), + rule_cache, + rule_cache_conditions, element, )); } - // We may not have non-visited rules, if we only had visited ones. In - // that case we want to use the root rulenode for our non-visited rules. - let rules = inputs.rules.as_ref().unwrap_or(self.rule_tree.root()); - // Read the comment on `precomputed_values_for_pseudo` to see why it's // difficult to assert that display: contents nodes never arrive here // (tl;dr: It doesn't apply for replaced elements and such, but the // computed value is still "contents"). + // + // FIXME(emilio): We should assert that it holds if pseudo.is_none()! properties::cascade::( &self.device, pseudo, - rules, + inputs.rules.as_ref().unwrap_or(self.rule_tree.root()), guards, parent_style, parent_style_ignoring_first_line, @@ -995,9 +954,9 @@ impl Stylist { font_metrics, cascade_flags, self.quirks_mode, - /* rule_cache = */ None, - &mut Default::default(), - None, + rule_cache, + rule_cache_conditions, + element, ) } @@ -1014,7 +973,7 @@ impl Stylist { is_probe: bool, rule_inclusion: RuleInclusion, matching_fn: Option<&Fn(&PseudoElement) -> bool>, - ) -> CascadeInputs + ) -> Option where E: TElement { @@ -1051,7 +1010,6 @@ impl Stylist { } }; - let mut inputs = CascadeInputs::default(); let mut declarations = ApplicableDeclarationList::new(); let mut matching_context = MatchingContext::new( MatchingMode::ForStatelessPseudoElement, @@ -1074,19 +1032,14 @@ impl Stylist { &mut set_selector_flags ); - if !declarations.is_empty() { - let rule_node = - self.rule_tree.compute_rule_node(&mut declarations, guards); - debug_assert!(rule_node != *self.rule_tree.root()); - inputs.rules = Some(rule_node); + if declarations.is_empty() && is_probe { + return None; } - if is_probe && inputs.rules.is_none() { - // When probing, don't compute visited styles if we have no - // unvisited styles. - return inputs; - } + let rules = + self.rule_tree.compute_rule_node(&mut declarations, guards); + let mut visited_rules = None; if parent_style.visited_style().is_some() { let mut declarations = ApplicableDeclarationList::new(); let mut matching_context = @@ -1114,14 +1067,15 @@ impl Stylist { let rule_node = self.rule_tree.insert_ordered_rules_with_important( declarations.drain().map(|a| a.order_and_level()), - guards); + guards, + ); if rule_node != *self.rule_tree.root() { - inputs.visited_rules = Some(rule_node); + visited_rules = Some(rule_node); } } } - inputs + Some(CascadeInputs { rules: Some(rules), visited_rules }) } /// Set a given device, which may change the styles that apply to the @@ -1596,7 +1550,7 @@ impl Stylist { self.quirks_mode, /* rule_cache = */ None, &mut Default::default(), - None, + /* element = */ None, ) } diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index fdaa9ab09a4..4a93d43a83a 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -6,7 +6,7 @@ use cssparser::{ParseErrorKind, Parser, ParserInput}; use cssparser::ToCss as ParserToCss; use env_logger::LogBuilder; use malloc_size_of::MallocSizeOfOps; -use selectors::{Element, NthIndexCache}; +use selectors::NthIndexCache; use selectors::matching::{MatchingContext, MatchingMode, matches_selector}; use servo_arc::{Arc, ArcBorrow, RawOffsetArc}; use smallvec::SmallVec; @@ -122,7 +122,7 @@ use style::gecko_properties; use style::invalidation::element::restyle_hints; use style::media_queries::{Device, MediaList, parse_media_query_list}; use style::parser::{Parse, ParserContext, self}; -use style::properties::{CascadeFlags, ComputedValues, DeclarationSource, Importance}; +use style::properties::{ComputedValues, DeclarationSource, Importance}; use style::properties::{LonghandId, LonghandIdSet, PropertyDeclaration, PropertyDeclarationBlock, PropertyId}; use style::properties::{PropertyDeclarationId, ShorthandId}; use style::properties::{SourcePropertyDeclaration, StyleBuilder}; @@ -2039,12 +2039,10 @@ pub extern "C" fn Servo_ComputedValues_GetForAnonymousBox(parent_style_or_null: page_decls, ); - let cascade_flags = CascadeFlags::SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP; data.stylist.precomputed_values_for_pseudo_with_rule_node::( &guards, &pseudo, parent_style_or_null.map(|x| &*x), - cascade_flags, &metrics, rule_node ).into() @@ -2220,7 +2218,7 @@ fn get_pseudo_style( PseudoElementCascadeType::Eager => { match *pseudo { PseudoElement::FirstLetter => { - styles.pseudos.get(&pseudo).and_then(|pseudo_styles| { + styles.pseudos.get(&pseudo).map(|pseudo_styles| { // inherited_styles can be None when doing lazy resolution // (e.g. for computed style) or when probing. In that case // we just inherit from our element, which is what Gecko @@ -2232,12 +2230,11 @@ fn get_pseudo_style( let metrics = get_metrics_provider_for_product(); let inputs = CascadeInputs::new_from_style(pseudo_styles); doc_data.stylist.compute_pseudo_element_style_with_inputs( - &inputs, + inputs, pseudo, &guards, Some(inherited_styles), &metrics, - CascadeFlags::empty(), Some(element), ) }) @@ -3633,29 +3630,17 @@ pub extern "C" fn Servo_ReparentStyle( let pseudo = style_to_reparent.pseudo(); let element = element.map(GeckoElement); - let mut cascade_flags = CascadeFlags::empty(); - if style_to_reparent.is_anon_box() { - cascade_flags.insert(CascadeFlags::SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP); - } - if let Some(element) = element { - if element.is_link() { - cascade_flags.insert(CascadeFlags::IS_LINK); - if element.is_visited_link() && doc_data.visited_styles_enabled() { - cascade_flags.insert(CascadeFlags::IS_VISITED_LINK); - } - }; - } - - doc_data.stylist.compute_style_with_inputs( - &inputs, + doc_data.stylist.cascade_style_and_visited( + element, pseudo.as_ref(), + inputs, &StylesheetGuards::same(&guard), Some(parent_style), Some(parent_style_ignoring_first_line), Some(layout_parent_style), &metrics, - cascade_flags, - element, + /* rule_cache = */ None, + &mut RuleCacheConditions::default(), ).into() }