Bug 1729289 - Fix out of order child layer registration, and enable the tests. r=boris

This makes layer order use a fixed set of bits per nesting level, to "reserve"
bits for children before they are registered.

See the comment in LayerOrder for the implementation limits it imposes, and
potential alternatives if these limits are not enough (but I think they should
be).

Enable the tests, as they mostly pass now (commit incoming to fix the remaining
ones).

Differential Revision: https://phabricator.services.mozilla.com/D124620
This commit is contained in:
Emilio Cobos Álvarez 2021-09-08 11:17:04 +00:00
parent b62a05a36a
commit f4095b3242
4 changed files with 134 additions and 173 deletions

View File

@ -19,6 +19,97 @@ use smallvec::SmallVec;
use std::fmt::{self, Write};
use style_traits::{CssWriter, ParseError, ToCss};
/// The order of a given layer. We encode in a 32-bit integer as follows:
///
/// * 0 is reserved for the initial (top-level) layer.
/// * Top 7 bits are for top level layer order.
/// * The 25 remaining bits are split in 5 chunks of 5 bits each, for each
/// nesting level.
///
/// This scheme this gives up to 127 layers in the top level, and up to 31
/// children layers in nested levels, with a max of 6 nesting levels over all.
///
/// This seemingly complicated scheme is to avoid fixing up layer orders after
/// the cascade data rebuild.
///
/// An alternative approach that would allow improving those limits would be to
/// make layers have a sequential identifier, and sort layer order after the
/// fact. But that complicates incremental cascade data rebuild.
#[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq, PartialOrd, Ord)]
pub struct LayerOrder(u32);
impl LayerOrder {
const FIRST_LEVEL_BITS: usize = 7;
const CHILD_BITS: usize = 5;
const FIRST_LEVEL_MASK: u32 = 0b11111110_00000000_00000000_00000000;
const CHILD_MASK: u32 = 0b00011111;
/// Get the raw value.
pub fn raw(self) -> u32 {
self.0
}
/// The top level layer (implicit) is zero.
#[inline]
pub const fn top_level() -> Self {
Self(0)
}
/// The first layer order.
#[inline]
pub const fn first() -> Self {
Self(1 << (32 - Self::FIRST_LEVEL_BITS))
}
fn child_bit_offset(self) -> usize {
if self.0 & (Self::CHILD_MASK << 5) != 0 {
return 0; // We're at the last or next-to-last level.
}
if self.0 & (Self::CHILD_MASK << 10) != 0 {
return 5;
}
if self.0 & (Self::CHILD_MASK << 15) != 0 {
return 10;
}
if self.0 & (Self::CHILD_MASK << 20) != 0 {
return 15;
}
if self.0 != 0 {
return 20;
}
return 25;
}
fn sibling_bit_mask_max_and_offset(self) -> (u32, u32, u32) {
debug_assert_ne!(self.0, 0, "Top layer should have no siblings");
for offset in &[0, 5, 10, 15, 20] {
let mask = Self::CHILD_MASK << *offset;
if self.0 & mask != 0 {
return (mask, (1 << Self::CHILD_BITS) - 1, *offset);
}
}
return (Self::FIRST_LEVEL_MASK, (1 << Self::FIRST_LEVEL_BITS) - 1, 25);
}
/// Generate the layer order for our first child.
pub fn for_child(self) -> Self {
Self(self.0 | (1 << self.child_bit_offset()))
}
/// Generate the layer order for our next sibling. Might return the same
/// order when our limits overflow.
pub fn for_next_sibling(self) -> Self {
let (mask, max_index, offset) = self.sibling_bit_mask_max_and_offset();
let self_index = (self.0 & mask) >> offset;
let next_index = if self_index == max_index {
self_index
} else {
self_index + 1
};
Self((self.0 & !mask) | (next_index << offset))
}
}
/// A `<layer-name>`: https://drafts.csswg.org/css-cascade-5/#typedef-layer-name
#[derive(Clone, Debug, Eq, Hash, MallocSizeOf, PartialEq, ToShmem)]
pub struct LayerName(pub SmallVec<[AtomIdent; 1]>);

View File

@ -26,7 +26,7 @@ use crate::shared_lock::{Locked, SharedRwLockReadGuard, StylesheetGuards};
use crate::stylesheet_set::{DataValidity, DocumentStylesheetSet, SheetRebuildKind};
use crate::stylesheet_set::{DocumentStylesheetFlusher, SheetCollectionFlusher};
use crate::stylesheets::keyframes_rule::KeyframesAnimation;
use crate::stylesheets::layer_rule::LayerName;
use crate::stylesheets::layer_rule::{LayerName, LayerOrder};
use crate::stylesheets::viewport_rule::{self, MaybeNew, ViewportRule};
use crate::stylesheets::{StyleRule, StylesheetInDocument, StylesheetContents};
#[cfg(feature = "gecko")]
@ -1892,6 +1892,14 @@ impl PartElementAndPseudoRules {
}
}
#[derive(Debug, Clone, MallocSizeOf)]
struct LayerOrderState {
/// The order for this layer.
order: LayerOrder,
/// The order for the next registered child layer.
next_child: LayerOrder,
}
/// Data resulting from performing the CSS cascade that is specific to a given
/// origin.
///
@ -1958,10 +1966,10 @@ pub struct CascadeData {
animations: PrecomputedHashMap<Atom, KeyframesAnimation>,
/// A map from cascade layer name to layer order.
layer_order: FxHashMap<LayerName, u32>,
layer_order: FxHashMap<LayerName, LayerOrderState>,
/// The next layer order for this cascade data.
next_layer_order: u32,
/// The next layer order for the top level cascade data.
next_layer_order: LayerOrder,
/// Effective media query results cached from the last rebuild.
effective_media_query_results: EffectiveMediaQueryResults,
@ -2004,7 +2012,7 @@ impl CascadeData {
selectors_for_cache_revalidation: SelectorMap::new_without_attribute_bucketing(),
animations: Default::default(),
layer_order: Default::default(),
next_layer_order: 1, // 0 reserved for the root scope.
next_layer_order: LayerOrder::first(),
extra_data: ExtraStyleData::default(),
effective_media_query_results: EffectiveMediaQueryResults::new(),
rules_source_order: 0,
@ -2168,7 +2176,7 @@ impl CascadeData {
guard: &SharedRwLockReadGuard,
rebuild_kind: SheetRebuildKind,
current_layer: &mut LayerName,
current_layer_order: u32,
current_layer_order: LayerOrder,
mut precomputed_pseudo_element_decls: Option<&mut PrecomputedPseudoElementDeclarations>,
) -> Result<(), FailedAllocationError>
where
@ -2201,7 +2209,7 @@ impl CascadeData {
self.rules_source_order,
CascadeLevel::UANormal,
selector.specificity(),
current_layer_order,
current_layer_order.raw(),
));
continue;
}
@ -2355,16 +2363,31 @@ impl CascadeData {
}
fn maybe_register_layer(data: &mut CascadeData, layer: &LayerName) -> u32 {
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
// case.
if let Some(order) = data.layer_order.get(layer) {
return *order;
if let Some(ref mut state) = data.layer_order.get(layer) {
return state.order;
}
let order = data.next_layer_order;
data.layer_order.insert(layer.clone(), order);
data.next_layer_order += 1;
// If the layer is not top-level, find the relevant parent.
let order = if layer.layer_names().len() > 1 {
let mut parent = layer.clone();
parent.0.pop();
let mut parent_state = data.layer_order.get_mut(&parent).expect("Parent layers should be registered before child layers");
let order = parent_state.next_child;
parent_state.next_child = order.for_next_sibling();
order
} else {
let order = data.next_layer_order;
data.next_layer_order = order.for_next_sibling();
order
};
data.layer_order.insert(layer.clone(), LayerOrderState {
order,
next_child: order.for_child(),
});
order
}
@ -2473,7 +2496,7 @@ impl CascadeData {
guard,
rebuild_kind,
&mut current_layer,
/* current_layer_order = */ 0,
LayerOrder::top_level(),
precomputed_pseudo_element_decls.as_deref_mut(),
)?;
@ -2592,7 +2615,7 @@ impl CascadeData {
}
self.animations.clear();
self.layer_order.clear();
self.next_layer_order = 1;
self.next_layer_order = LayerOrder::first();
self.extra_data.clear();
self.rules_source_order = 0;
self.num_selectors = 0;
@ -2682,7 +2705,7 @@ pub struct Rule {
pub source_order: u32,
/// The current layer order of this style rule.
pub layer_order: u32,
pub layer_order: LayerOrder,
/// The actual style rule.
#[cfg_attr(
@ -2712,7 +2735,7 @@ impl Rule {
level: CascadeLevel,
) -> ApplicableDeclarationBlock {
let source = StyleSource::from_rule(self.style_rule.clone());
ApplicableDeclarationBlock::new(source, self.source_order, level, self.specificity(), self.layer_order)
ApplicableDeclarationBlock::new(source, self.source_order, level, self.specificity(), self.layer_order.raw())
}
/// Creates a new Rule.
@ -2721,7 +2744,7 @@ impl Rule {
hashes: AncestorHashes,
style_rule: Arc<Locked<StyleRule>>,
source_order: u32,
layer_order: u32,
layer_order: LayerOrder,
) -> Self {
Rule {
selector,

View File

@ -1,96 +1,4 @@
[layer-basic.html]
[A2 Anonymous layers]
expected: FAIL
[A3 Anonymous layers]
expected: FAIL
[A4 Anonymous layers]
expected: FAIL
[A5 Anonymous layers]
expected: FAIL
[A6 Anonymous layers]
expected: FAIL
[A7 Anonymous layers]
expected: FAIL
[A8 Anonymous layers]
expected: FAIL
[A9 Anonymous layers]
expected: FAIL
[B2 Named layers]
expected: FAIL
[B3 Named layers]
expected: FAIL
[B4 Named layers]
expected: FAIL
[B5 Named layers]
expected: FAIL
[B6 Named layers]
expected: FAIL
[B7 Named layers]
expected: FAIL
[B8 Named layers]
expected: FAIL
[B9 Named layers]
expected: FAIL
[B10 Named layers]
expected: FAIL
[C1 Named layers shorthand]
expected: FAIL
[C2 Named layers shorthand]
expected: FAIL
[C3 Named layers shorthand]
expected: FAIL
[C4 Named layers shorthand]
expected: FAIL
[C5 Named layers shorthand]
expected: FAIL
[D1 Mixed named and anonymous layers]
expected: FAIL
[D2 Mixed named and anonymous layers]
expected: FAIL
[D3 Mixed named and anonymous layers]
expected: FAIL
[D4 Mixed named and anonymous layers]
expected: FAIL
[D5 Mixed named and anonymous layers]
expected: FAIL
[E1 Statement syntax]
expected: FAIL
[E2 Statement syntax]
expected: FAIL
[E3 Statement syntax]
expected: FAIL
prefs: [layout.css.cascade-layers.enabled:true]
[E4 Statement syntax]
expected: FAIL
[E5 Statement syntax]
expected: FAIL

View File

@ -1,66 +1,5 @@
[layer-import.html]
[A1 Layer rules with import]
expected: FAIL
[A2 Layer rules with import]
expected: FAIL
[A3 Layer rules with import]
expected: FAIL
[A4 Layer rules with import]
expected: FAIL
[B1 Anonymous imports]
expected: FAIL
[B2 Anonymous imports]
expected: FAIL
[B3 Anonymous imports]
expected: FAIL
[B4 Anonymous imports]
expected: FAIL
[C1 Named imports]
expected: FAIL
[C2 Named imports]
expected: FAIL
[C3 Named imports]
expected: FAIL
[C4 Named imports]
expected: FAIL
[C5 Named imports]
expected: FAIL
[C6 Named imports]
expected: FAIL
[C7 Named imports]
expected: FAIL
[C8 Named imports]
expected: FAIL
[C9 Named imports]
expected: FAIL
[D1 Layer statement with imports]
expected: FAIL
[D2 Layer statement with imports]
expected: FAIL
[D3 Layer statement with imports]
expected: FAIL
[D4 Layer statement with imports]
expected: FAIL
prefs: [layout.css.cascade-layers.enabled:true]
[D5 Layer statement with imports]
expected: FAIL