Stop using Ref::map for style().

It's not possible to implement a Ref::map equivalent method on AtomicRefCell
while having AtomicRefCell implemented on top of RwArc. We could potentially
reimplement AtomicRefCell to be more like RefCell and add a Ref::map equivalent
method, but I (and pcwalton) think we should try just cloning a few extra
Arcs at these callsites instead.

MozReview-Commit-ID: 6H8vAWguO3z
This commit is contained in:
Bobby Holley 2016-09-29 17:43:04 -07:00
parent 518324cff6
commit 18d552a1e9
5 changed files with 20 additions and 28 deletions

View file

@ -218,7 +218,7 @@ impl InlineFragmentsAccumulator {
enclosing_node: Some(InlineFragmentNodeInfo { enclosing_node: Some(InlineFragmentNodeInfo {
address: node.opaque(), address: node.opaque(),
pseudo: node.get_pseudo_element_type().strip(), pseudo: node.get_pseudo_element_type().strip(),
style: node.style(style_context).clone(), style: node.style(style_context),
selected_style: node.selected_style(style_context).clone(), selected_style: node.selected_style(style_context).clone(),
flags: InlineFragmentNodeFlags::empty(), flags: InlineFragmentNodeFlags::empty(),
}), }),
@ -355,7 +355,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode>
let style_context = self.style_context(); let style_context = self.style_context();
if child.is_table_cell() { if child.is_table_cell() {
let mut style = child_node.style(style_context).clone(); let mut style = child_node.style(style_context);
properties::modify_style_for_anonymous_table_object(&mut style, display::T::table_row); properties::modify_style_for_anonymous_table_object(&mut style, display::T::table_row);
let fragment = Fragment::from_opaque_node_and_style(child_node.opaque(), let fragment = Fragment::from_opaque_node_and_style(child_node.opaque(),
PseudoElementType::Normal, PseudoElementType::Normal,
@ -369,7 +369,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode>
*child = new_child *child = new_child
} }
if child.is_table_row() || child.is_table_rowgroup() { if child.is_table_row() || child.is_table_rowgroup() {
let mut style = child_node.style(style_context).clone(); let mut style = child_node.style(style_context);
properties::modify_style_for_anonymous_table_object(&mut style, display::T::table); properties::modify_style_for_anonymous_table_object(&mut style, display::T::table);
let fragment = Fragment::from_opaque_node_and_style(child_node.opaque(), let fragment = Fragment::from_opaque_node_and_style(child_node.opaque(),
PseudoElementType::Normal, PseudoElementType::Normal,
@ -383,7 +383,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode>
*child = new_child *child = new_child
} }
if child.is_table() { if child.is_table() {
let mut style = child_node.style(style_context).clone(); let mut style = child_node.style(style_context);
properties::modify_style_for_anonymous_table_object(&mut style, display::T::table); properties::modify_style_for_anonymous_table_object(&mut style, display::T::table);
let fragment = let fragment =
Fragment::from_opaque_node_and_style(child_node.opaque(), Fragment::from_opaque_node_and_style(child_node.opaque(),
@ -477,7 +477,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode>
let (ascent, descent) = let (ascent, descent) =
inline_flow.compute_minimum_ascent_and_descent(&mut self.layout_context inline_flow.compute_minimum_ascent_and_descent(&mut self.layout_context
.font_context(), .font_context(),
&**node.style(self.style_context())); &node.style(self.style_context()));
inline_flow.minimum_block_size_above_baseline = ascent; inline_flow.minimum_block_size_above_baseline = ascent;
inline_flow.minimum_depth_below_baseline = descent; inline_flow.minimum_depth_below_baseline = descent;
} }
@ -692,7 +692,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode>
} }
} }
let mut style = node.style(self.style_context()).clone(); let mut style = node.style(self.style_context());
if node_is_input_or_text_area { if node_is_input_or_text_area {
style = self.style_context().stylist. style = self.style_context().stylist.
precomputed_values_for_pseudo(&PseudoElement::ServoInputText, Some(&style)).unwrap(); precomputed_values_for_pseudo(&PseudoElement::ServoInputText, Some(&style)).unwrap();
@ -806,7 +806,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode>
-> ConstructionResult { -> ConstructionResult {
let mut opt_inline_block_splits: LinkedList<InlineBlockSplit> = LinkedList::new(); let mut opt_inline_block_splits: LinkedList<InlineBlockSplit> = LinkedList::new();
let mut fragment_accumulator = InlineFragmentsAccumulator::from_inline_node(node, self.style_context()); let mut fragment_accumulator = InlineFragmentsAccumulator::from_inline_node(node, self.style_context());
fragment_accumulator.bidi_control_chars = bidi_control_chars(&*node.style(self.style_context())); fragment_accumulator.bidi_control_chars = bidi_control_chars(&node.style(self.style_context()));
let mut abs_descendants = AbsoluteDescendants::new(); let mut abs_descendants = AbsoluteDescendants::new();
@ -949,13 +949,13 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode>
return ConstructionResult::ConstructionItem(ConstructionItem::Whitespace( return ConstructionResult::ConstructionItem(ConstructionItem::Whitespace(
node.opaque(), node.opaque(),
node.get_pseudo_element_type().strip(), node.get_pseudo_element_type().strip(),
node.style(self.style_context()).clone(), node.style(self.style_context()),
node.restyle_damage())) node.restyle_damage()))
} }
// Modify the style as necessary. (See the comment in // Modify the style as necessary. (See the comment in
// `properties::modify_style_for_replaced_content()`.) // `properties::modify_style_for_replaced_content()`.)
let mut style = node.style(self.style_context()).clone(); let mut style = node.style(self.style_context());
match node.get_pseudo_element_type() { match node.get_pseudo_element_type() {
PseudoElementType::Before(_) | PseudoElementType::Before(_) |
PseudoElementType::After(_) => {} PseudoElementType::After(_) => {}
@ -993,7 +993,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode>
}; };
let style_context = self.style_context(); let style_context = self.style_context();
let mut modified_style = (*node.style(self.style_context())).clone(); let mut modified_style = node.style(self.style_context());
properties::modify_style_for_outer_inline_block_fragment(&mut modified_style); properties::modify_style_for_outer_inline_block_fragment(&mut modified_style);
let fragment_info = SpecificFragmentInfo::InlineBlock(InlineBlockFragmentInfo::new( let fragment_info = SpecificFragmentInfo::InlineBlock(InlineBlockFragmentInfo::new(
block_flow)); block_flow));
@ -1030,7 +1030,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode>
let fragment_info = SpecificFragmentInfo::InlineAbsoluteHypothetical( let fragment_info = SpecificFragmentInfo::InlineAbsoluteHypothetical(
InlineAbsoluteHypotheticalFragmentInfo::new(block_flow)); InlineAbsoluteHypotheticalFragmentInfo::new(block_flow));
let style_context = self.style_context(); let style_context = self.style_context();
let mut style = node.style(style_context).clone(); let mut style = node.style(style_context);
properties::modify_style_for_inline_absolute_hypothetical_fragment(&mut style); properties::modify_style_for_inline_absolute_hypothetical_fragment(&mut style);
let fragment = Fragment::from_opaque_node_and_style(node.opaque(), let fragment = Fragment::from_opaque_node_and_style(node.opaque(),
PseudoElementType::Normal, PseudoElementType::Normal,
@ -1412,7 +1412,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode>
let mut set_has_newly_constructed_flow_flag = false; let mut set_has_newly_constructed_flow_flag = false;
let result = { let result = {
let mut style = node.style(self.style_context()).clone(); let mut style = node.style(self.style_context());
let mut data = node.mutate_layout_data().unwrap(); let mut data = node.mutate_layout_data().unwrap();
let damage = data.restyle_damage; let damage = data.restyle_damage;

View file

@ -1272,7 +1272,7 @@ impl<'a> ImmutableFlowUtils for &'a Flow {
/// as it's harder to understand. /// as it's harder to understand.
fn generate_missing_child_flow<N: ThreadSafeLayoutNode>(self, node: &N, ctx: &LayoutContext) -> FlowRef { fn generate_missing_child_flow<N: ThreadSafeLayoutNode>(self, node: &N, ctx: &LayoutContext) -> FlowRef {
let style_context = ctx.style_context(); let style_context = ctx.style_context();
let mut style = node.style(style_context).clone(); let mut style = node.style(style_context);
match self.class() { match self.class() {
FlowClass::Table | FlowClass::TableRowGroup => { FlowClass::Table | FlowClass::TableRowGroup => {
properties::modify_style_for_anonymous_table_object( properties::modify_style_for_anonymous_table_object(

View file

@ -856,7 +856,7 @@ impl Fragment {
/// Constructs a new `Fragment` instance. /// Constructs a new `Fragment` instance.
pub fn new<N: ThreadSafeLayoutNode>(node: &N, specific: SpecificFragmentInfo, ctx: &LayoutContext) -> Fragment { pub fn new<N: ThreadSafeLayoutNode>(node: &N, specific: SpecificFragmentInfo, ctx: &LayoutContext) -> Fragment {
let style_context = ctx.style_context(); let style_context = ctx.style_context();
let style = node.style(style_context).clone(); let style = node.style(style_context);
let writing_mode = style.writing_mode; let writing_mode = style.writing_mode;
let mut restyle_damage = node.restyle_damage(); let mut restyle_damage = node.restyle_damage();

View file

@ -240,12 +240,11 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + NodeInfo + PartialEq + Sized {
/// ///
/// Unlike the version on TNode, this handles pseudo-elements. /// Unlike the version on TNode, this handles pseudo-elements.
#[inline] #[inline]
fn style(&self, context: &SharedStyleContext) -> Ref<Arc<ServoComputedValues>> { fn style(&self, context: &SharedStyleContext) -> Arc<ServoComputedValues> {
match self.get_pseudo_element_type() { match self.get_pseudo_element_type() {
PseudoElementType::Normal => { PseudoElementType::Normal => {
Ref::map(self.get_style_data().unwrap().borrow(), |data| { self.get_style_data().unwrap().borrow()
data.style_data.style.as_ref().unwrap() .style_data.style.as_ref().unwrap().clone()
})
}, },
other => { other => {
// Precompute non-eagerly-cascaded pseudo-element styles if not // Precompute non-eagerly-cascaded pseudo-element styles if not
@ -289,9 +288,9 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + NodeInfo + PartialEq + Sized {
} }
} }
Ref::map(self.get_style_data().unwrap().borrow(), |data| { self.get_style_data().unwrap().borrow()
data.style_data.per_pseudo.get(&style_pseudo).unwrap() .style_data.per_pseudo.get(&style_pseudo)
}) .unwrap().clone()
} }
} }
} }

View file

@ -167,13 +167,6 @@ pub trait TNode : Sized + Copy + Clone + NodeInfo {
fn next_sibling(&self) -> Option<Self>; fn next_sibling(&self) -> Option<Self>;
/// Returns the style results for the given node. If CSS selector matching
/// has not yet been performed, fails.
fn style(&self, _context: &SharedStyleContext) -> Ref<Arc<ComputedValues>> {
Ref::map(self.borrow_data().unwrap(), |data| data.style.as_ref().unwrap())
}
/// Removes the style from this node. /// Removes the style from this node.
fn unstyle(self) { fn unstyle(self) {
self.mutate_data().unwrap().style = None; self.mutate_data().unwrap().style = None;