style: Fix anonymous name handling in presence of stylesheet sharing

We need to compute the anonymous name on the fly while building the
CascadeData, otherwise we may see the same layer rule in two places due
to stylehseet sharing and make them incorrectly share a name.

Differential Revision: https://phabricator.services.mozilla.com/D125175
This commit is contained in:
Emilio Cobos Álvarez 2023-05-27 07:21:38 +02:00 committed by Oriol Brufau
parent 5fe148d5f1
commit 3e251f50fc
4 changed files with 65 additions and 60 deletions

View file

@ -137,12 +137,10 @@ impl DeepCloneWithLock for ImportSheet {
}
/// The layer keyword or function in an import rule.
#[derive(Debug)]
#[derive(Debug, Clone)]
pub struct ImportLayer {
/// Whether the layer is anonymous.
pub is_anonymous: bool,
/// The layer name.
pub name: LayerName,
/// The layer name, or None for an anonymous layer.
pub name: Option<LayerName>,
}
@ -151,12 +149,13 @@ impl ToCss for ImportLayer {
where
W: Write,
{
if self.is_anonymous {
dest.write_str("layer")
} else {
dest.write_str("layer(")?;
self.name.to_css(dest)?;
dest.write_char(')')
match self.name {
None => dest.write_str("layer"),
Some(ref name) => {
dest.write_str("layer(")?;
name.to_css(dest)?;
dest.write_char(')')
},
}
}
}
@ -199,16 +198,7 @@ impl DeepCloneWithLock for ImportRule {
ImportRule {
url: self.url.clone(),
stylesheet: self.stylesheet.deep_clone_with_lock(lock, guard, params),
layer: self.layer.as_ref().map(|layer| {
ImportLayer {
is_anonymous: layer.is_anonymous,
name: if layer.is_anonymous {
LayerName::new_anonymous()
} else {
layer.name.clone()
},
}
}),
layer: self.layer.clone(),
source_location: self.source_location.clone(),
}
}

View file

@ -202,13 +202,10 @@ impl ToCss for LayerName {
pub enum LayerRuleKind {
/// A block `@layer <name>? { ... }`
Block {
/// The layer name.
name: LayerName,
/// The layer name, or `None` if anonymous.
name: Option<LayerName>,
/// The nested rules.
rules: Arc<Locked<CssRules>>,
/// Whether the layer name is synthesized (and thus shouldn't be
/// serialized).
is_anonymous: bool,
},
/// A statement `@layer <name>, <name>, <name>;`
Statement {
@ -239,9 +236,8 @@ impl ToCssWithGuard for LayerRule {
LayerRuleKind::Block {
ref name,
ref rules,
ref is_anonymous,
} => {
if !*is_anonymous {
if let Some(ref name) = *name {
dest.write_char(' ')?;
name.to_css(&mut CssWriter::new(dest))?;
}
@ -277,13 +273,8 @@ impl DeepCloneWithLock for LayerRule {
LayerRuleKind::Block {
ref name,
ref rules,
ref is_anonymous,
} => LayerRuleKind::Block {
name: if *is_anonymous {
LayerName::new_anonymous()
} else {
name.clone()
},
name: name.clone(),
rules: Arc::new(
lock.wrap(
rules
@ -291,7 +282,6 @@ impl DeepCloneWithLock for LayerRule {
.deep_clone_with_lock(lock, guard, params),
),
),
is_anonymous: *is_anonymous,
},
LayerRuleKind::Statement { ref names } => LayerRuleKind::Statement {
names: names.clone(),

View file

@ -216,8 +216,7 @@ impl<'a, 'i> AtRuleParser<'i> for TopLevelRuleParser<'a> {
None
} else if input.try_parse(|input| input.expect_ident_matching("layer")).is_ok() {
Some(ImportLayer {
is_anonymous: true,
name: LayerName::new_anonymous(),
name: None,
})
} else {
input.try_parse(|input| {
@ -225,8 +224,7 @@ impl<'a, 'i> AtRuleParser<'i> for TopLevelRuleParser<'a> {
input.parse_nested_block(|input| {
LayerName::parse(&self.context, input)
}).map(|name| ImportLayer {
is_anonymous: false,
name,
name: Some(name),
})
}).ok()
};
@ -603,9 +601,8 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> {
))))
},
AtRulePrelude::Layer(names) => {
let (name, is_anonymous) = match names.len() {
0 => (LayerName::new_anonymous(), true),
1 => (names.into_iter().next().unwrap(), false),
let name = match names.len() {
0 | 1 => names.into_iter().next(),
_ => return Err(input.new_error(BasicParseErrorKind::AtRuleBodyInvalid)),
};
Ok(CssRule::Layer(Arc::new(self.shared_lock.wrap(
@ -613,7 +610,6 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> {
kind: LayerRuleKind::Block {
name,
rules: self.parse_nested_rules(input, CssRuleType::Layer),
is_anonymous,
},
source_location: start.source_location(),
},

View file

@ -2150,7 +2150,7 @@ impl CascadeData {
stylesheet: &S,
guard: &SharedRwLockReadGuard,
rebuild_kind: SheetRebuildKind,
current_layer: &mut LayerName,
mut current_layer: &mut LayerName,
current_layer_order: LayerOrder,
mut precomputed_pseudo_element_decls: Option<&mut PrecomputedPseudoElementDeclarations>,
) -> Result<(), FailedAllocationError>
@ -2337,7 +2337,6 @@ impl CascadeData {
continue;
}
fn maybe_register_layer(data: &mut CascadeData, layer: &LayerName) -> LayerOrder {
// TODO: Measure what's more common / expensive, if
// layer.clone() or the double hash lookup in the insert
@ -2366,6 +2365,31 @@ impl CascadeData {
order
}
fn maybe_register_layers(
data: &mut CascadeData,
name: Option<&LayerName>,
current_layer: &mut LayerName,
pushed_layers: &mut usize,
) -> LayerOrder {
let anon_name;
let name = match name {
Some(name) => name,
None => {
anon_name = LayerName::new_anonymous();
&anon_name
},
};
let mut order = LayerOrder::top_level();
for name in name.layer_names() {
current_layer.0.push(name.clone());
order = maybe_register_layer(data, &current_layer);
*pushed_layers += 1;
}
debug_assert_ne!(order, LayerOrder::top_level());
order
}
let mut layer_names_to_pop = 0;
let mut children_layer_order = current_layer_order;
match *rule {
@ -2376,11 +2400,12 @@ impl CascadeData {
.saw_effective(import_rule);
}
if let Some(ref layer) = import_rule.layer {
for name in layer.name.layer_names() {
current_layer.0.push(name.clone());
children_layer_order = maybe_register_layer(self, &current_layer);
layer_names_to_pop += 1;
}
children_layer_order = maybe_register_layers(
self,
layer.name.as_ref(),
&mut current_layer,
&mut layer_names_to_pop,
);
}
},
@ -2396,20 +2421,24 @@ impl CascadeData {
let layer_rule = lock.read_with(guard);
match layer_rule.kind {
LayerRuleKind::Block { ref name, .. } => {
for name in name.layer_names() {
current_layer.0.push(name.clone());
children_layer_order = maybe_register_layer(self, &current_layer);
layer_names_to_pop += 1;
}
children_layer_order = maybe_register_layers(
self,
name.as_ref(),
&mut current_layer,
&mut layer_names_to_pop,
);
}
LayerRuleKind::Statement { ref names } => {
for name in &**names {
let mut pushed = 0;
for name in name.layer_names() {
current_layer.0.push(name.clone());
maybe_register_layer(self, &current_layer);
pushed += 1;
}
// There are no children, so we can ignore the
// return value.
maybe_register_layers(
self,
Some(name),
&mut current_layer,
&mut pushed,
);
for _ in 0..pushed {
current_layer.0.pop();
}