Skip to content

Commit

Permalink
style: Update StyleSource to use ArcUnion.
Browse files Browse the repository at this point in the history
Bug: 1455784
Reviewed-by: Manishearth
MozReview-Commit-ID: AT4sud9goGV
  • Loading branch information
bholley authored and emilio committed Apr 29, 2018
1 parent cbbefeb commit 48558e3
Show file tree
Hide file tree
Showing 7 changed files with 99 additions and 109 deletions.
2 changes: 1 addition & 1 deletion components/style/applicable_declarations.rs
Expand Up @@ -116,7 +116,7 @@ impl ApplicableDeclarationBlock {
level: CascadeLevel,
) -> Self {
ApplicableDeclarationBlock {
source: StyleSource::Declarations(declarations),
source: StyleSource::from_declarations(declarations),
bits: ApplicableDeclarationBits::new(0, level, 0),
specificity: 0,
}
Expand Down
2 changes: 1 addition & 1 deletion components/style/properties/properties.mako.rs
Expand Up @@ -3424,7 +3424,7 @@ where
let source = node.style_source();

let declarations = if source.is_some() {
source.read(cascade_level.guard(guards)).declaration_importance_iter()
source.as_ref().unwrap().read(cascade_level.guard(guards)).declaration_importance_iter()
} else {
// The root node has no style source.
DeclarationImportanceIterator::new(&[], &empty)
Expand Down
22 changes: 12 additions & 10 deletions components/style/rule_cache.rs
Expand Up @@ -8,7 +8,7 @@
use fnv::FnvHashMap;
use logical_geometry::WritingMode;
use properties::{ComputedValues, StyleBuilder};
use rule_tree::{StrongRuleNode, StyleSource};
use rule_tree::StrongRuleNode;
use selector_parser::PseudoElement;
use servo_arc::Arc;
use shared_lock::StylesheetGuards;
Expand Down Expand Up @@ -97,16 +97,18 @@ impl RuleCache {
mut rule_node: Option<&'r StrongRuleNode>,
) -> Option<&'r StrongRuleNode> {
while let Some(node) = rule_node {
match *node.style_source() {
StyleSource::Declarations(ref decls) => {
let cascade_level = node.cascade_level();
let decls = decls.read_with(cascade_level.guard(guards));
if decls.contains_any_reset() {
break;
}
match node.style_source() {
Some(s) => match s.as_declarations() {
Some(decls) => {
let cascade_level = node.cascade_level();
let decls = decls.read_with(cascade_level.guard(guards));
if decls.contains_any_reset() {
break;
}
},
None => break,
},
StyleSource::None => {},
StyleSource::Style(_) => break,
None => {},
}
rule_node = node.parent();
}
Expand Down
158 changes: 72 additions & 86 deletions components/style/rule_tree/mod.rs
Expand Up @@ -12,7 +12,7 @@ use gecko::selector_parser::PseudoElement;
#[cfg(feature = "gecko")]
use malloc_size_of::{MallocSizeOf, MallocSizeOfOps};
use properties::{Importance, LonghandIdSet, PropertyDeclarationBlock};
use servo_arc::{Arc, ArcBorrow, NonZeroPtrMut};
use servo_arc::{Arc, ArcBorrow, ArcUnion, ArcUnionBorrow, NonZeroPtrMut};
use shared_lock::{Locked, SharedRwLockReadGuard, StylesheetGuards};
use smallvec::SmallVec;
use std::io::{self, Write};
Expand Down Expand Up @@ -89,40 +89,27 @@ impl MallocSizeOf for RuleTree {
/// more debuggability, and also the ability of show those selectors to
/// devtools.
#[derive(Clone, Debug)]
pub enum StyleSource {
/// A style rule stable pointer.
Style(Arc<Locked<StyleRule>>),
/// A declaration block stable pointer.
Declarations(Arc<Locked<PropertyDeclarationBlock>>),
/// Indicates no style source. Used to save an Option wrapper around the stylesource in
/// RuleNode
None,
}
pub struct StyleSource(ArcUnion<Locked<StyleRule>, Locked<PropertyDeclarationBlock>>);

impl PartialEq for StyleSource {
fn eq(&self, other: &Self) -> bool {
self.ptr_equals(other)
ArcUnion::ptr_eq(&self.0, &other.0)
}
}

impl StyleSource {
#[inline]
fn ptr_equals(&self, other: &Self) -> bool {
use self::StyleSource::*;
match (self, other) {
(&Style(ref one), &Style(ref other)) => Arc::ptr_eq(one, other),
(&Declarations(ref one), &Declarations(ref other)) => Arc::ptr_eq(one, other),
(&None, _) | (_, &None) => {
panic!("Should not check for equality between null StyleSource objects")
},
_ => false,
}
/// Creates a StyleSource from a StyleRule.
pub fn from_rule(rule: Arc<Locked<StyleRule>>) -> Self {
StyleSource(ArcUnion::from_first(rule))
}

fn dump<W: Write>(&self, guard: &SharedRwLockReadGuard, writer: &mut W) {
use self::StyleSource::*;
/// Creates a StyleSource from a PropertyDeclarationBlock.
pub fn from_declarations(decls: Arc<Locked<PropertyDeclarationBlock>>) -> Self {
StyleSource(ArcUnion::from_second(decls))
}

if let Style(ref rule) = *self {
fn dump<W: Write>(&self, guard: &SharedRwLockReadGuard, writer: &mut W) {
if let Some(ref rule) = self.0.as_first() {
let rule = rule.read_with(guard);
let _ = write!(writer, "{:?}", rule.selectors);
}
Expand All @@ -134,20 +121,31 @@ impl StyleSource {
/// underlying property declaration block.
#[inline]
pub fn read<'a>(&'a self, guard: &'a SharedRwLockReadGuard) -> &'a PropertyDeclarationBlock {
let block = match *self {
StyleSource::Style(ref rule) => &rule.read_with(guard).block,
StyleSource::Declarations(ref block) => block,
StyleSource::None => panic!("Cannot call read on StyleSource::None"),
let block: &Locked<PropertyDeclarationBlock> = match self.0.borrow() {
ArcUnionBorrow::First(ref rule) => &rule.get().read_with(guard).block,
ArcUnionBorrow::Second(ref block) => block.get(),
};
block.read_with(guard)
}

/// Indicates if this StyleSource has a value
pub fn is_some(&self) -> bool {
match *self {
StyleSource::None => false,
_ => true,
}
/// Indicates if this StyleSource is a style rule.
pub fn is_rule(&self) -> bool {
self.0.is_first()
}

/// Indicates if this StyleSource is a PropertyDeclarationBlock.
pub fn is_declarations(&self) -> bool {
self.0.is_second()
}

/// Returns the style rule if applicable, otherwise None.
pub fn as_rule(&self) -> Option<ArcBorrow<Locked<StyleRule>>> {
self.0.as_first()
}

/// Returns the declaration block if applicable, otherwise None.
pub fn as_declarations(&self) -> Option<ArcBorrow<Locked<PropertyDeclarationBlock>>> {
self.0.as_second()
}
}

Expand Down Expand Up @@ -248,11 +246,12 @@ impl RuleTree {
last_cascade_order = shadow_cascade_order;
important_inner_shadow.push(SmallVec::new());
}
important_inner_shadow.last_mut().unwrap().push(source.clone())
}
SameTreeAuthorNormal => {
important_same_tree.push(source.clone())
important_inner_shadow
.last_mut()
.unwrap()
.push(source.clone())
},
SameTreeAuthorNormal => important_same_tree.push(source.clone()),
UANormal => important_ua.push(source.clone()),
UserNormal => important_user.push(source.clone()),
StyleAttributeNormal => {
Expand Down Expand Up @@ -391,7 +390,10 @@ impl RuleTree {
// First walk up until the first less-or-equally specific rule.
let mut children = SmallVec::<[_; 10]>::new();
while current.get().level > level {
children.push((current.get().source.clone(), current.get().level));
children.push((
current.get().source.as_ref().unwrap().clone(),
current.get().level,
));
current = current.parent().unwrap().clone();
}

Expand All @@ -418,13 +420,14 @@ impl RuleTree {
// also equally valid. This is less likely, and would require an
// in-place mutation of the source, which is, at best, fiddly,
// so let's skip it for now.
let is_here_already = match current.get().source {
StyleSource::Declarations(ref already_here) => {
pdb.with_arc(|arc| Arc::ptr_eq(arc, already_here))
},
_ => unreachable!("Replacing non-declarations style?"),
};

let current_decls = current
.get()
.source
.as_ref()
.unwrap()
.as_declarations()
.expect("Replacing non-declarations style?");
let is_here_already = ArcBorrow::ptr_eq(&pdb, &current_decls);
if is_here_already {
debug!("Picking the fast path in rule replacement");
return None;
Expand All @@ -447,15 +450,15 @@ impl RuleTree {
if pdb.read_with(level.guard(guards)).any_important() {
current = current.ensure_child(
self.root.downgrade(),
StyleSource::Declarations(pdb.clone_arc()),
StyleSource::from_declarations(pdb.clone_arc()),
level,
);
}
} else {
if pdb.read_with(level.guard(guards)).any_normal() {
current = current.ensure_child(
self.root.downgrade(),
StyleSource::Declarations(pdb.clone_arc()),
StyleSource::from_declarations(pdb.clone_arc()),
level,
);
}
Expand Down Expand Up @@ -491,7 +494,10 @@ impl RuleTree {
let mut children = SmallVec::<[_; 10]>::new();
for node in iter {
if !node.cascade_level().is_animation() {
children.push((node.get().source.clone(), node.cascade_level()));
children.push((
node.get().source.as_ref().unwrap().clone(),
node.cascade_level(),
));
}
last = node;
}
Expand Down Expand Up @@ -689,7 +695,9 @@ pub struct RuleNode {

/// The actual style source, either coming from a selector in a StyleRule,
/// or a raw property declaration block (like the style attribute).
source: StyleSource,
///
/// None for the root node.
source: Option<StyleSource>,

/// The cascade level this rule is positioned at.
level: CascadeLevel,
Expand Down Expand Up @@ -775,7 +783,7 @@ impl RuleNode {
RuleNode {
root: Some(root),
parent: Some(parent),
source: source,
source: Some(source),
level: level,
refcount: AtomicUsize::new(1),
first_child: AtomicPtr::new(ptr::null_mut()),
Expand All @@ -789,7 +797,7 @@ impl RuleNode {
RuleNode {
root: None,
parent: None,
source: StyleSource::None,
source: None,
level: CascadeLevel::UANormal,
refcount: AtomicUsize::new(1),
first_child: AtomicPtr::new(ptr::null_mut()),
Expand Down Expand Up @@ -884,7 +892,10 @@ impl RuleNode {
}

if self.source.is_some() {
self.source.dump(self.level.guard(guards), writer);
self.source
.as_ref()
.unwrap()
.dump(self.level.guard(guards), writer);
} else {
if indent != 0 {
warn!("How has this happened?");
Expand Down Expand Up @@ -986,7 +997,7 @@ impl StrongRuleNode {
// WeakRuleNode, and implementing this on WeakRuleNode itself...
for child in self.get().iter_children() {
let child_node = unsafe { &*child.ptr() };
if child_node.level == level && child_node.source.ptr_equals(&source) {
if child_node.level == level && child_node.source.as_ref().unwrap() == &source {
return child.upgrade();
}
last = Some(child);
Expand Down Expand Up @@ -1026,7 +1037,7 @@ impl StrongRuleNode {
// we accessed `last`.
next = WeakRuleNode::from_ptr(existing);

if unsafe { &*next.ptr() }.source.ptr_equals(&source) {
if unsafe { &*next.ptr() }.source.as_ref().unwrap() == &source {
// That node happens to be for the same style source, use
// that, and let node fall out of scope.
return next.upgrade();
Expand Down Expand Up @@ -1054,8 +1065,8 @@ impl StrongRuleNode {
/// Get the style source corresponding to this rule node. May return `None`
/// if it's the root node, which means that the node hasn't matched any
/// rules.
pub fn style_source(&self) -> &StyleSource {
&self.get().source
pub fn style_source(&self) -> Option<&StyleSource> {
self.get().source.as_ref()
}

/// The cascade level for this node
Expand Down Expand Up @@ -1317,6 +1328,8 @@ impl StrongRuleNode {
let source = node.style_source();
let declarations = if source.is_some() {
source
.as_ref()
.unwrap()
.read(node.cascade_level().guard(guards))
.declaration_importance_iter()
} else {
Expand Down Expand Up @@ -1444,7 +1457,7 @@ impl StrongRuleNode {
.take_while(|node| node.cascade_level() > CascadeLevel::Animations);
let mut result = (LonghandIdSet::new(), false);
for node in iter {
let style = node.style_source();
let style = node.style_source().unwrap();
for (decl, important) in style
.read(node.cascade_level().guard(guards))
.declaration_importance_iter()
Expand All @@ -1464,33 +1477,6 @@ impl StrongRuleNode {
}
result
}

/// Returns PropertyDeclarationBlock for this node.
/// This function must be called only for animation level node.
fn get_animation_style(&self) -> &Arc<Locked<PropertyDeclarationBlock>> {
debug_assert!(
self.cascade_level().is_animation(),
"The cascade level should be an animation level"
);
match *self.style_source() {
StyleSource::Declarations(ref block) => block,
StyleSource::Style(_) => unreachable!("animating style should not be a style rule"),
StyleSource::None => unreachable!("animating style should not be none"),
}
}

/// Returns SMIL override declaration block if exists.
pub fn get_smil_animation_rule(&self) -> Option<&Arc<Locked<PropertyDeclarationBlock>>> {
if cfg!(feature = "servo") {
// Servo has no knowledge of a SMIL rule, so just avoid looking for it.
return None;
}

self.self_and_ancestors()
.take_while(|node| node.cascade_level() >= CascadeLevel::SMILOverride)
.find(|node| node.cascade_level() == CascadeLevel::SMILOverride)
.map(|node| node.get_animation_style())
}
}

/// An iterator over a rule node and its ancestors.
Expand Down
6 changes: 3 additions & 3 deletions components/style/stylist.rs
Expand Up @@ -1523,7 +1523,7 @@ impl Stylist {
// just avoid allocating it and calling `apply_declarations` directly,
// maybe...
let rule_node = self.rule_tree.insert_ordered_rules(iter::once((
StyleSource::Declarations(declarations),
StyleSource::from_declarations(declarations),
CascadeLevel::StyleAttributeNormal,
)));

Expand Down Expand Up @@ -2177,7 +2177,7 @@ impl CascadeData {
.expect("Expected precomputed declarations for the UA level")
.get_or_insert_with(&pseudo.canonical(), Vec::new)
.push(ApplicableDeclarationBlock::new(
StyleSource::Style(locked.clone()),
StyleSource::from_rule(locked.clone()),
self.rules_source_order,
CascadeLevel::UANormal,
selector.specificity(),
Expand Down Expand Up @@ -2480,7 +2480,7 @@ impl Rule {
level: CascadeLevel,
shadow_cascade_order: ShadowCascadeOrder,
) -> ApplicableDeclarationBlock {
let source = StyleSource::Style(self.style_rule.clone());
let source = StyleSource::from_rule(self.style_rule.clone());
ApplicableDeclarationBlock::new(source, self.source_order, level, self.specificity(), shadow_cascade_order)
}

Expand Down

0 comments on commit 48558e3

Please sign in to comment.