diff --git a/Cargo.lock b/Cargo.lock index f7065091220..f4a1d7d52f6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5328,9 +5328,9 @@ checksum = "c111b5bd5695e56cffe5129854aa230b39c93a305372fdbb2668ca2394eea9f8" [[package]] name = "smallbitvec" -version = "2.3.0" +version = "2.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1764fe2b30ee783bfe3b9b37b2649d8d590b3148bb12e0079715d4d5c673562e" +checksum = "797a4eaffb90d896f29698d45676f9f940a71936d7574996a7df54593ba209fa" [[package]] name = "smallvec" diff --git a/components/derive_common/cg.rs b/components/derive_common/cg.rs index c1b6588c749..c51c0d7750b 100644 --- a/components/derive_common/cg.rs +++ b/components/derive_common/cg.rs @@ -85,9 +85,22 @@ pub fn add_predicate(where_clause: &mut Option, pred: WherePre .push(pred); } -pub fn fmap_match(input: &DeriveInput, bind_style: BindStyle, mut f: F) -> TokenStream +pub fn fmap_match(input: &DeriveInput, bind_style: BindStyle, f: F) -> TokenStream where - F: FnMut(BindingInfo) -> TokenStream, + F: FnMut(&BindingInfo) -> TokenStream, +{ + fmap2_match(input, bind_style, f, |_| None) +} + +pub fn fmap2_match( + input: &DeriveInput, + bind_style: BindStyle, + mut f: F, + mut g: G, +) -> TokenStream +where + F: FnMut(&BindingInfo) -> TokenStream, + G: FnMut(&BindingInfo) -> Option, { let mut s = synstructure::Structure::new(input); s.variants_mut().iter_mut().for_each(|v| { @@ -95,12 +108,20 @@ where }); s.each_variant(|variant| { let (mapped, mapped_fields) = value(variant, "mapped"); - let fields_pairs = variant.bindings().iter().zip(mapped_fields); + let fields_pairs = variant.bindings().iter().zip(mapped_fields.iter()); let mut computations = quote!(); computations.append_all(fields_pairs.map(|(field, mapped_field)| { - let expr = f(field.clone()); + let expr = f(field); quote! { let #mapped_field = #expr; } })); + computations.append_all( + mapped_fields + .iter() + .map(|mapped_field| match g(mapped_field) { + Some(expr) => quote! { let #mapped_field = #expr; }, + None => quote!(), + }), + ); computations.append_all(mapped); Some(computations) }) diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index b9af35b8cac..157829412ce 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -3331,7 +3331,7 @@ impl Document { pub fn element_state_will_change(&self, el: &Element) { let mut entry = self.ensure_pending_restyle(el); if entry.snapshot.is_none() { - entry.snapshot = Some(Snapshot::new(el.html_element_in_html_document())); + entry.snapshot = Some(Snapshot::new()); } let snapshot = entry.snapshot.as_mut().unwrap(); if snapshot.state.is_none() { @@ -3347,7 +3347,7 @@ impl Document { // could in theory do it in the DOM I think. let mut entry = self.ensure_pending_restyle(el); if entry.snapshot.is_none() { - entry.snapshot = Some(Snapshot::new(el.html_element_in_html_document())); + entry.snapshot = Some(Snapshot::new()); } if attr.local_name() == &local_name!("style") { entry.hint.insert(RestyleHint::RESTYLE_STYLE_ATTRIBUTE); @@ -3359,12 +3359,21 @@ impl Document { let snapshot = entry.snapshot.as_mut().unwrap(); if attr.local_name() == &local_name!("id") { + if snapshot.id_changed { + return; + } snapshot.id_changed = true; } else if attr.local_name() == &local_name!("class") { + if snapshot.class_changed { + return; + } snapshot.class_changed = true; } else { snapshot.other_attributes_changed = true; } + if !snapshot.changed_attrs.contains(attr.local_name()) { + snapshot.changed_attrs.push(attr.local_name().clone()); + } if snapshot.attrs.is_none() { let attrs = el .attrs() diff --git a/components/selectors/matching.rs b/components/selectors/matching.rs index d8d4ad59946..3414f9d6efa 100644 --- a/components/selectors/matching.rs +++ b/components/selectors/matching.rs @@ -322,13 +322,7 @@ where }, } - // The only other parser-allowed Component in this sequence is a state - // class. We just don't match in that case. - if let Some(s) = iter.next() { - debug_assert!( - matches!(*s, Component::NonTSPseudoClass(..)), - "Someone messed up pseudo-element parsing" - ); + if !iter.matches_for_stateless_pseudo_element() { return false; } diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs index 0a3dd43ac3c..9980bdeeb60 100644 --- a/components/selectors/parser.rs +++ b/components/selectors/parser.rs @@ -104,41 +104,57 @@ bitflags! { /// disallowed. If this flag is set, `AFTER_PSEUDO_ELEMENT` must be set /// as well. const AFTER_NON_STATEFUL_PSEUDO_ELEMENT = 1 << 4; + /// Whether we are after any of the pseudo-like things. const AFTER_PSEUDO = Self::AFTER_PART.bits | Self::AFTER_SLOTTED.bits | Self::AFTER_PSEUDO_ELEMENT.bits; + + /// Whether we explicitly disallow combinators. + const DISALLOW_COMBINATORS = 1 << 5; + + /// Whether we explicitly disallow pseudo-element-like things. + const DISALLOW_PSEUDOS = 1 << 6; } } impl SelectorParsingState { #[inline] - fn allows_functional_pseudo_classes(self) -> bool { - !self.intersects(SelectorParsingState::AFTER_PSEUDO) + fn allows_pseudos(self) -> bool { + // NOTE(emilio): We allow pseudos after ::part and such. + !self.intersects(Self::AFTER_PSEUDO_ELEMENT | Self::DISALLOW_PSEUDOS) } #[inline] fn allows_slotted(self) -> bool { - !self.intersects(SelectorParsingState::AFTER_PSEUDO) + !self.intersects(Self::AFTER_PSEUDO | Self::DISALLOW_PSEUDOS) } - // TODO(emilio): Should we allow other ::part()s after ::part()? - // - // See https://github.com/w3c/csswg-drafts/issues/3841 #[inline] fn allows_part(self) -> bool { - !self.intersects(SelectorParsingState::AFTER_PSEUDO) + !self.intersects(Self::AFTER_PSEUDO | Self::DISALLOW_PSEUDOS) + } + + // TODO(emilio): Maybe some of these should be allowed, but this gets us on + // the safe side for now, matching previous behavior. Gotta be careful with + // the ones like :-moz-any, which allow nested selectors but don't carry the + // state, and so on. + #[inline] + fn allows_custom_functional_pseudo_classes(self) -> bool { + !self.intersects(Self::AFTER_PSEUDO) } #[inline] fn allows_non_functional_pseudo_classes(self) -> bool { - !self.intersects( - SelectorParsingState::AFTER_SLOTTED | - SelectorParsingState::AFTER_NON_STATEFUL_PSEUDO_ELEMENT, - ) + !self.intersects(Self::AFTER_SLOTTED | Self::AFTER_NON_STATEFUL_PSEUDO_ELEMENT) } #[inline] fn allows_tree_structural_pseudo_classes(self) -> bool { - !self.intersects(SelectorParsingState::AFTER_PSEUDO) + !self.intersects(Self::AFTER_PSEUDO) + } + + #[inline] + fn allows_combinators(self) -> bool { + !self.intersects(Self::DISALLOW_COMBINATORS) } } @@ -146,7 +162,6 @@ pub type SelectorParseError<'i> = ParseError<'i, SelectorParseErrorKind<'i>>; #[derive(Clone, Debug, PartialEq)] pub enum SelectorParseErrorKind<'i> { - PseudoElementInComplexSelector, NoQualifiedNameInAttributeSelector(Token<'i>), EmptySelector, DanglingCombinator, @@ -321,15 +336,25 @@ impl SelectorList { parser: &P, input: &mut CssParser<'i, 't>, ) -> Result> + where + P: Parser<'i, Impl = Impl>, + { + Self::parse_with_state(parser, input, SelectorParsingState::empty()) + } + + fn parse_with_state<'i, 't, P>( + parser: &P, + input: &mut CssParser<'i, 't>, + state: SelectorParsingState, + ) -> Result> where P: Parser<'i, Impl = Impl>, { let mut values = SmallVec::new(); loop { - values.push( - input - .parse_until_before(Delimiter::Comma, |input| parse_selector(parser, input))?, - ); + values.push(input.parse_until_before(Delimiter::Comma, |input| { + parse_selector(parser, input, state) + })?); match input.next() { Err(_) => return Ok(SelectorList(values)), Ok(&Token::Comma) => continue, @@ -344,30 +369,21 @@ impl SelectorList { } } -/// Parses one compound selector suitable for nested stuff like ::-moz-any, etc. +/// Parses one compound selector suitable for nested stuff like :-moz-any, etc. fn parse_inner_compound_selector<'i, 't, P, Impl>( parser: &P, input: &mut CssParser<'i, 't>, + state: SelectorParsingState, ) -> Result, ParseError<'i, P::Error>> where P: Parser<'i, Impl = Impl>, Impl: SelectorImpl, { - let location = input.current_source_location(); - let selector = parse_selector(parser, input)?; - - // Ensure they're actually all compound selectors without pseudo-elements. - if selector.has_pseudo_element() { - return Err( - location.new_custom_error(SelectorParseErrorKind::PseudoElementInComplexSelector) - ); - } - - if selector.iter_raw_match_order().any(|s| s.is_combinator()) { - return Err(location.new_custom_error(SelectorParseErrorKind::NonCompoundSelector)); - } - - Ok(selector) + parse_selector( + parser, + input, + state | SelectorParsingState::DISALLOW_PSEUDOS | SelectorParsingState::DISALLOW_COMBINATORS, + ) } /// Parse a comma separated list of compound selectors. @@ -380,7 +396,9 @@ where Impl: SelectorImpl, { input - .parse_comma_separated(|input| parse_inner_compound_selector(parser, input)) + .parse_comma_separated(|input| { + parse_inner_compound_selector(parser, input, SelectorParsingState::empty()) + }) .map(|selectors| selectors.into_boxed_slice()) } @@ -404,18 +422,66 @@ pub struct AncestorHashes { pub packed_hashes: [u32; 3], } +fn collect_ancestor_hashes( + iter: SelectorIter, + quirks_mode: QuirksMode, + hashes: &mut [u32; 4], + len: &mut usize, +) -> bool +where + Impl::Identifier: PrecomputedHash, + Impl::ClassName: PrecomputedHash, + Impl::LocalName: PrecomputedHash, + Impl::NamespaceUrl: PrecomputedHash, +{ + for component in AncestorIter::new(iter) { + let hash = match *component { + Component::LocalName(LocalName { + ref name, + ref lower_name, + }) => { + // Only insert the local-name into the filter if it's all + // lowercase. Otherwise we would need to test both hashes, and + // our data structures aren't really set up for that. + if name != lower_name { + continue; + } + name.precomputed_hash() + }, + Component::DefaultNamespace(ref url) | Component::Namespace(_, ref url) => { + url.precomputed_hash() + }, + // In quirks mode, class and id selectors should match + // case-insensitively, so just avoid inserting them into the filter. + Component::ID(ref id) if quirks_mode != QuirksMode::Quirks => id.precomputed_hash(), + Component::Class(ref class) if quirks_mode != QuirksMode::Quirks => { + class.precomputed_hash() + }, + Component::Is(ref list) | Component::Where(ref list) => { + // :where and :is OR their selectors, so we can't put any hash + // in the filter if there's more than one selector, as that'd + // exclude elements that may match one of the other selectors. + if list.len() == 1 { + if !collect_ancestor_hashes(list[0].iter(), quirks_mode, hashes, len) { + return false; + } + } + continue; + }, + _ => continue, + }; + + hashes[*len] = hash & BLOOM_HASH_MASK; + *len += 1; + if *len == hashes.len() { + return false; + } + } + true +} + impl AncestorHashes { pub fn new(selector: &Selector, quirks_mode: QuirksMode) -> Self - where - Impl::Identifier: PrecomputedHash, - Impl::ClassName: PrecomputedHash, - Impl::LocalName: PrecomputedHash, - Impl::NamespaceUrl: PrecomputedHash, - { - Self::from_iter(selector.iter(), quirks_mode) - } - - fn from_iter(iter: SelectorIter, quirks_mode: QuirksMode) -> Self where Impl::Identifier: PrecomputedHash, Impl::ClassName: PrecomputedHash, @@ -424,18 +490,14 @@ impl AncestorHashes { { // Compute ancestor hashes for the bloom filter. let mut hashes = [0u32; 4]; - let mut hash_iter = AncestorIter::new(iter).filter_map(|x| x.ancestor_hash(quirks_mode)); - for i in 0..4 { - hashes[i] = match hash_iter.next() { - Some(x) => x & BLOOM_HASH_MASK, - None => break, - } - } + let mut len = 0; + collect_ancestor_hashes(selector.iter(), quirks_mode, &mut hashes, &mut len); + debug_assert!(len <= 4); // Now, pack the fourth hash (if it exists) into the upper byte of each of // the other three hashes. - let fourth = hashes[3]; - if fourth != 0 { + if len == 4 { + let fourth = hashes[3]; hashes[0] |= (fourth & 0x000000ff) << 24; hashes[1] |= (fourth & 0x0000ff00) << 16; hashes[2] |= (fourth & 0x00ff0000) << 8; @@ -741,6 +803,33 @@ impl<'a, Impl: 'a + SelectorImpl> SelectorIter<'a, Impl> { self.next_sequence().is_none() } + #[inline] + pub(crate) fn matches_for_stateless_pseudo_element(&mut self) -> bool { + let first = match self.next() { + Some(c) => c, + // Note that this is the common path that we keep inline: the + // pseudo-element not having anything to its right. + None => return true, + }; + self.matches_for_stateless_pseudo_element_internal(first) + } + + #[inline(never)] + fn matches_for_stateless_pseudo_element_internal(&mut self, first: &Component) -> bool { + if !first.matches_for_stateless_pseudo_element() { + return false; + } + for component in self { + // The only other parser-allowed Components in this sequence are + // state pseudo-classes, or one of the other things that can contain + // them. + if !component.matches_for_stateless_pseudo_element() { + return false; + } + } + true + } + /// Returns remaining count of the simple selectors and combinators in the Selector. #[inline] pub fn selector_length(&self) -> usize { @@ -980,43 +1069,6 @@ pub enum Component { } impl Component { - /// Compute the ancestor hash to check against the bloom filter. - fn ancestor_hash(&self, quirks_mode: QuirksMode) -> Option - where - Impl::Identifier: PrecomputedHash, - Impl::ClassName: PrecomputedHash, - Impl::LocalName: PrecomputedHash, - Impl::NamespaceUrl: PrecomputedHash, - { - match *self { - Component::LocalName(LocalName { - ref name, - ref lower_name, - }) => { - // Only insert the local-name into the filter if it's all - // lowercase. Otherwise we would need to test both hashes, and - // our data structures aren't really set up for that. - if name == lower_name { - Some(name.precomputed_hash()) - } else { - None - } - }, - Component::DefaultNamespace(ref url) | Component::Namespace(_, ref url) => { - Some(url.precomputed_hash()) - }, - // In quirks mode, class and id selectors should match - // case-insensitively, so just avoid inserting them into the filter. - Component::ID(ref id) if quirks_mode != QuirksMode::Quirks => { - Some(id.precomputed_hash()) - }, - Component::Class(ref class) if quirks_mode != QuirksMode::Quirks => { - Some(class.precomputed_hash()) - }, - _ => None, - } - } - /// Returns true if this is a combinator. pub fn is_combinator(&self) -> bool { matches!(*self, Component::Combinator(_)) @@ -1030,6 +1082,52 @@ impl Component { } } + /// Whether this component is valid after a pseudo-element. Only intended + /// for sanity-checking. + pub fn maybe_allowed_after_pseudo_element(&self) -> bool { + match *self { + Component::NonTSPseudoClass(..) => true, + Component::Negation(ref components) => components + .iter() + .all(|c| c.maybe_allowed_after_pseudo_element()), + Component::Is(ref selectors) | Component::Where(ref selectors) => { + selectors.iter().all(|selector| { + selector + .iter_raw_match_order() + .all(|c| c.maybe_allowed_after_pseudo_element()) + }) + }, + _ => false, + } + } + + /// Whether a given selector should match for stateless pseudo-elements. + /// + /// This is a bit subtle: Only selectors that return true in + /// `maybe_allowed_after_pseudo_element` should end up here, and + /// `NonTSPseudoClass` never matches (as it is a stateless pseudo after + /// all). + fn matches_for_stateless_pseudo_element(&self) -> bool { + debug_assert!( + self.maybe_allowed_after_pseudo_element(), + "Someone messed up pseudo-element parsing: {:?}", + *self + ); + match *self { + Component::Negation(ref components) => !components + .iter() + .all(|c| c.matches_for_stateless_pseudo_element()), + Component::Is(ref selectors) | Component::Where(ref selectors) => { + selectors.iter().any(|selector| { + selector + .iter_raw_match_order() + .all(|c| c.matches_for_stateless_pseudo_element()) + }) + }, + _ => false, + } + } + pub fn visit(&self, visitor: &mut V) -> bool where V: SelectorVisitor, @@ -1533,6 +1631,7 @@ fn display_to_css_identifier(x: &T, dest: &mut W) -> fn parse_selector<'i, 't, P, Impl>( parser: &P, input: &mut CssParser<'i, 't>, + mut state: SelectorParsingState, ) -> Result, ParseError<'i, P::Error>> where P: Parser<'i, Impl = Impl>, @@ -1545,16 +1644,14 @@ where let mut part = false; 'outer_loop: loop { // Parse a sequence of simple selectors. - let state = match parse_compound_selector(parser, input, &mut builder)? { - Some(state) => state, - None => { - return Err(input.new_custom_error(if builder.has_combinators() { - SelectorParseErrorKind::DanglingCombinator - } else { - SelectorParseErrorKind::EmptySelector - })); - }, - }; + let empty = parse_compound_selector(parser, &mut state, input, &mut builder)?; + if empty { + return Err(input.new_custom_error(if builder.has_combinators() { + SelectorParseErrorKind::DanglingCombinator + } else { + SelectorParseErrorKind::EmptySelector + })); + } if state.intersects(SelectorParsingState::AFTER_PSEUDO) { has_pseudo_element = state.intersects(SelectorParsingState::AFTER_PSEUDO_ELEMENT); @@ -1595,6 +1692,11 @@ where }, } } + + if !state.allows_combinators() { + return Err(input.new_custom_error(SelectorParseErrorKind::InvalidState)); + } + builder.push_combinator(combinator); } @@ -1611,7 +1713,7 @@ impl Selector { where P: Parser<'i, Impl = Impl>, { - parse_selector(parser, input) + parse_selector(parser, input, SelectorParsingState::empty()) } } @@ -1621,6 +1723,7 @@ impl Selector { fn parse_type_selector<'i, 't, P, Impl, S>( parser: &P, input: &mut CssParser<'i, 't>, + state: SelectorParsingState, sink: &mut S, ) -> Result> where @@ -1635,6 +1738,9 @@ where }) | Ok(OptionalQName::None(_)) => Ok(false), Ok(OptionalQName::Some(namespace, local_name)) => { + if state.intersects(SelectorParsingState::AFTER_PSEUDO) { + return Err(input.new_custom_error(SelectorParseErrorKind::InvalidState)); + } match namespace { QNamePrefix::ImplicitAnyNamespace => {}, QNamePrefix::ImplicitDefaultNamespace(url) => { @@ -2006,11 +2112,14 @@ fn parse_attribute_flags<'i, 't>( fn parse_negation<'i, 't, P, Impl>( parser: &P, input: &mut CssParser<'i, 't>, + state: SelectorParsingState, ) -> Result, ParseError<'i, P::Error>> where P: Parser<'i, Impl = Impl>, Impl: SelectorImpl, { + let state = state | SelectorParsingState::INSIDE_NEGATION; + // We use a sequence because a type selector may be represented as two Components. let mut sequence = SmallVec::<[Component; 2]>::new(); @@ -2018,7 +2127,7 @@ where // Get exactly one simple selector. The parse logic in the caller will verify // that there are no trailing tokens after we're done. - let is_type_sel = match parse_type_selector(parser, input, &mut sequence) { + let is_type_sel = match parse_type_selector(parser, input, state, &mut sequence) { Ok(result) => result, Err(ParseError { kind: ParseErrorKind::Basic(BasicParseErrorKind::EndOfInput), @@ -2027,7 +2136,7 @@ where Err(e) => return Err(e.into()), }; if !is_type_sel { - match parse_one_simple_selector(parser, input, SelectorParsingState::INSIDE_NEGATION)? { + match parse_one_simple_selector(parser, input, state)? { Some(SimpleSelectorParseResult::SimpleSelector(s)) => { sequence.push(s); }, @@ -2054,12 +2163,13 @@ where /// | [ HASH | class | attrib | pseudo | negation ]+ /// /// `Err(())` means invalid selector. -/// `Ok(None)` is an empty selector +/// `Ok(true)` is an empty selector fn parse_compound_selector<'i, 't, P, Impl>( parser: &P, + state: &mut SelectorParsingState, input: &mut CssParser<'i, 't>, builder: &mut SelectorBuilder, -) -> Result, ParseError<'i, P::Error>> +) -> Result> where P: Parser<'i, Impl = Impl>, Impl: SelectorImpl, @@ -2067,13 +2177,12 @@ where input.skip_whitespace(); let mut empty = true; - if parse_type_selector(parser, input, builder)? { + if parse_type_selector(parser, input, *state, builder)? { empty = false; } - let mut state = SelectorParsingState::empty(); loop { - let result = match parse_one_simple_selector(parser, input, state)? { + let result = match parse_one_simple_selector(parser, input, *state)? { None => break, Some(result) => result, }; @@ -2132,17 +2241,13 @@ where }, } } - if empty { - // An empty selector is invalid. - Ok(None) - } else { - Ok(Some(state)) - } + Ok(empty) } fn parse_is_or_where<'i, 't, P, Impl>( parser: &P, input: &mut CssParser<'i, 't>, + state: SelectorParsingState, component: impl FnOnce(Box<[Selector]>) -> Component, ) -> Result, ParseError<'i, P::Error>> where @@ -2150,15 +2255,16 @@ where Impl: SelectorImpl, { debug_assert!(parser.parse_is_and_where()); - let inner = SelectorList::parse(parser, input)?; // https://drafts.csswg.org/selectors/#matches-pseudo: // // Pseudo-elements cannot be represented by the matches-any // pseudo-class; they are not valid within :is(). // - if inner.0.iter().any(|i| i.has_pseudo_element()) { - return Err(input.new_custom_error(SelectorParseErrorKind::InvalidPseudoElementInsideWhere)); - } + let inner = SelectorList::parse_with_state( + parser, + input, + state | SelectorParsingState::DISALLOW_PSEUDOS, + )?; Ok(component(inner.0.into_vec().into_boxed_slice())) } @@ -2172,40 +2278,51 @@ where P: Parser<'i, Impl = Impl>, Impl: SelectorImpl, { - if !state.allows_functional_pseudo_classes() { - return Err(input.new_custom_error(SelectorParseErrorKind::InvalidState)); - } - debug_assert!(state.allows_tree_structural_pseudo_classes()); match_ignore_ascii_case! { &name, - "nth-child" => return Ok(parse_nth_pseudo_class(input, Component::NthChild)?), - "nth-of-type" => return Ok(parse_nth_pseudo_class(input, Component::NthOfType)?), - "nth-last-child" => return Ok(parse_nth_pseudo_class(input, Component::NthLastChild)?), - "nth-last-of-type" => return Ok(parse_nth_pseudo_class(input, Component::NthLastOfType)?), - "is" if parser.parse_is_and_where() => return parse_is_or_where(parser, input, Component::Is), - "where" if parser.parse_is_and_where() => return parse_is_or_where(parser, input, Component::Where), - "host" => return Ok(Component::Host(Some(parse_inner_compound_selector(parser, input)?))), + "nth-child" => return parse_nth_pseudo_class(parser, input, state, Component::NthChild), + "nth-of-type" => return parse_nth_pseudo_class(parser, input, state, Component::NthOfType), + "nth-last-child" => return parse_nth_pseudo_class(parser, input, state, Component::NthLastChild), + "nth-last-of-type" => return parse_nth_pseudo_class(parser, input, state, Component::NthLastOfType), + "is" if parser.parse_is_and_where() => return parse_is_or_where(parser, input, state, Component::Is), + "where" if parser.parse_is_and_where() => return parse_is_or_where(parser, input, state, Component::Where), + "host" => { + if !state.allows_tree_structural_pseudo_classes() { + return Err(input.new_custom_error(SelectorParseErrorKind::InvalidState)); + } + return Ok(Component::Host(Some(parse_inner_compound_selector(parser, input, state)?))); + }, "not" => { if state.intersects(SelectorParsingState::INSIDE_NEGATION) { return Err(input.new_custom_error( SelectorParseErrorKind::UnexpectedIdent("not".into()) )); } - debug_assert!(state.is_empty()); - return parse_negation(parser, input) + return parse_negation(parser, input, state) }, _ => {} } + + if !state.allows_custom_functional_pseudo_classes() { + return Err(input.new_custom_error(SelectorParseErrorKind::InvalidState)); + } + P::parse_non_ts_functional_pseudo_class(parser, name, input).map(Component::NonTSPseudoClass) } -fn parse_nth_pseudo_class<'i, 't, Impl, F>( +fn parse_nth_pseudo_class<'i, 't, P, Impl, F>( + _: &P, input: &mut CssParser<'i, 't>, + state: SelectorParsingState, selector: F, -) -> Result, BasicParseError<'i>> +) -> Result, ParseError<'i, P::Error>> where + P: Parser<'i, Impl = Impl>, Impl: SelectorImpl, F: FnOnce(i32, i32) -> Component, { + if !state.allows_tree_structural_pseudo_classes() { + return Err(input.new_custom_error(SelectorParseErrorKind::InvalidState)); + } let (a, b) = parse_nth(input)?; Ok(selector(a, b)) } @@ -2290,7 +2407,7 @@ where }; let is_pseudo_element = !is_single_colon || is_css2_pseudo_element(&name); if is_pseudo_element { - if state.intersects(SelectorParsingState::AFTER_PSEUDO_ELEMENT) { + if !state.allows_pseudos() { return Err(input.new_custom_error(SelectorParseErrorKind::InvalidState)); } let pseudo_element = if is_functional { @@ -2317,7 +2434,7 @@ where ); } let selector = input.parse_nested_block(|input| { - parse_inner_compound_selector(parser, input) + parse_inner_compound_selector(parser, input, state) })?; return Ok(Some(SimpleSelectorParseResult::SlottedPseudo(selector))); } diff --git a/components/style/animation.rs b/components/style/animation.rs index 152692a0b28..a0d684f1a4b 100644 --- a/components/style/animation.rs +++ b/components/style/animation.rs @@ -978,7 +978,13 @@ where } => { let guard = declarations.read_with(context.guards.author); - let iter = || { + // 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, // It's possible to have !important properties in keyframes // so we have to filter them out. // See the spec issue https://github.com/w3c/csswg-drafts/issues/1824 @@ -986,17 +992,7 @@ where guard .normal_declaration_iter() .filter(|declaration| declaration.is_animatable()) - .map(|decl| (decl, Origin::Author)) - }; - - // 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, + .map(|decl| (decl, Origin::Author)), Some(previous_style), Some(previous_style), Some(previous_style), diff --git a/components/style/build_gecko.rs b/components/style/build_gecko.rs index a502b7d45f9..dfd2a213d99 100644 --- a/components/style/build_gecko.rs +++ b/components/style/build_gecko.rs @@ -131,6 +131,7 @@ impl BuilderExt for Builder { // them. let mut builder = Builder::default() .rust_target(RustTarget::Stable_1_25) + .size_t_is_usize(true) .disable_untagged_union(); let rustfmt_path = env::var_os("RUSTFMT") diff --git a/components/style/dom_apis.rs b/components/style/dom_apis.rs index 8f764dc8958..f4e814acd11 100644 --- a/components/style/dom_apis.rs +++ b/components/style/dom_apis.rs @@ -7,6 +7,7 @@ use crate::context::QuirksMode; use crate::dom::{TDocument, TElement, TNode, TShadowRoot}; +use crate::invalidation::element::invalidation_map::Dependency; use crate::invalidation::element::invalidator::{DescendantInvalidationLists, Invalidation}; use crate::invalidation::element::invalidator::{InvalidationProcessor, InvalidationVector}; use crate::Atom; @@ -130,7 +131,7 @@ where { results: &'a mut Q::Output, matching_context: MatchingContext<'a, E::Impl>, - selector_list: &'a SelectorList, + dependencies: &'a [Dependency], } impl<'a, E, Q> InvalidationProcessor<'a, E> for QuerySelectorProcessor<'a, E, Q> @@ -143,6 +144,14 @@ where true } + fn check_outer_dependency(&mut self, _: &Dependency, _: E) -> bool { + debug_assert!( + false, + "How? We should only have parent-less dependencies here!" + ); + true + } + fn collect_invalidations( &mut self, element: E, @@ -171,11 +180,10 @@ where self_invalidations }; - for selector in self.selector_list.0.iter() { + for dependency in self.dependencies.iter() { target_vector.push(Invalidation::new( - selector, + dependency, self.matching_context.current_host.clone(), - 0, )) } @@ -642,10 +650,15 @@ pub fn query_selector( if root_element.is_some() || !invalidation_may_be_useful { query_selector_slow::(root, selector_list, results, &mut matching_context); } else { + let dependencies = selector_list + .0 + .iter() + .map(|selector| Dependency::for_full_selector_invalidation(selector.clone())) + .collect::>(); let mut processor = QuerySelectorProcessor:: { results, matching_context, - selector_list, + dependencies: &dependencies, }; for node in root.dom_children() { diff --git a/components/style/element_state.rs b/components/style/element_state.rs index cf943cc3c4e..027ebc839b4 100644 --- a/components/style/element_state.rs +++ b/components/style/element_state.rs @@ -87,9 +87,9 @@ bitflags! { /// const IN_OUTOFRANGE_STATE = 1 << 28; /// - const IN_MOZ_READONLY_STATE = 1 << 29; + const IN_READONLY_STATE = 1 << 29; /// - const IN_MOZ_READWRITE_STATE = 1 << 30; + const IN_READWRITE_STATE = 1 << 30; /// const IN_DEFAULT_STATE = 1 << 31; /// Non-standard: https://developer.mozilla.org/en-US/docs/Web/CSS/:-moz-submit-invalid @@ -141,6 +141,10 @@ bitflags! { /// /// https://drafts.csswg.org/selectors-4/#the-focus-visible-pseudo const IN_FOCUS_VISIBLE_STATE = 1 << 52; + /// State that dialog element is modal, for centered alignment + /// + /// https://html.spec.whatwg.org/multipage/#centered-alignment + const IN_MODAL_DIALOG_STATE = 1 << 53; } } diff --git a/components/style/gecko/media_features.rs b/components/style/gecko/media_features.rs index e3de8dbfaaa..4d60dc2ff26 100644 --- a/components/style/gecko/media_features.rs +++ b/components/style/gecko/media_features.rs @@ -8,8 +8,9 @@ use crate::gecko_bindings::bindings; use crate::gecko_bindings::structs; use crate::media_queries::media_feature::{AllowsRanges, ParsingRequirements}; use crate::media_queries::media_feature::{Evaluator, MediaFeatureDescription}; -use crate::media_queries::media_feature_expression::{AspectRatio, RangeOrOperator}; +use crate::media_queries::media_feature_expression::RangeOrOperator; use crate::media_queries::{Device, MediaType}; +use crate::values::computed::position::Ratio; use crate::values::computed::CSSPixelLength; use crate::values::computed::Resolution; use crate::Atom; @@ -91,7 +92,7 @@ fn eval_device_height( fn eval_aspect_ratio_for( device: &Device, - query_value: Option, + query_value: Option, range_or_operator: Option, get_size: F, ) -> bool @@ -104,14 +105,14 @@ where }; let size = get_size(device); - let value = AspectRatio(size.width.0 as f32, size.height.0 as f32); + let value = Ratio::new(size.width.0 as f32, size.height.0 as f32); RangeOrOperator::evaluate_with_query_value(range_or_operator, query_value, value) } /// https://drafts.csswg.org/mediaqueries-4/#aspect-ratio fn eval_aspect_ratio( device: &Device, - query_value: Option, + query_value: Option, range_or_operator: Option, ) -> bool { eval_aspect_ratio_for(device, query_value, range_or_operator, viewport_size) @@ -120,7 +121,7 @@ fn eval_aspect_ratio( /// https://drafts.csswg.org/mediaqueries-4/#device-aspect-ratio fn eval_device_aspect_ratio( device: &Device, - query_value: Option, + query_value: Option, range_or_operator: Option, ) -> bool { eval_aspect_ratio_for(device, query_value, range_or_operator, device_size) @@ -537,225 +538,222 @@ macro_rules! system_metric_feature { }}; } -lazy_static! { - /// Adding new media features requires (1) adding the new feature to this - /// array, with appropriate entries (and potentially any new code needed - /// to support new types in these entries and (2) ensuring that either - /// nsPresContext::MediaFeatureValuesChanged is called when the value that - /// would be returned by the evaluator function could change. - pub static ref MEDIA_FEATURES: [MediaFeatureDescription; 53] = [ - feature!( - atom!("width"), - AllowsRanges::Yes, - Evaluator::Length(eval_width), - ParsingRequirements::empty(), - ), - feature!( - atom!("height"), - AllowsRanges::Yes, - Evaluator::Length(eval_height), - ParsingRequirements::empty(), - ), - feature!( - atom!("aspect-ratio"), - AllowsRanges::Yes, - Evaluator::NumberRatio(eval_aspect_ratio), - ParsingRequirements::empty(), - ), - feature!( - atom!("orientation"), - AllowsRanges::No, - keyword_evaluator!(eval_orientation, Orientation), - ParsingRequirements::empty(), - ), - feature!( - atom!("device-width"), - AllowsRanges::Yes, - Evaluator::Length(eval_device_width), - ParsingRequirements::empty(), - ), - feature!( - atom!("device-height"), - AllowsRanges::Yes, - Evaluator::Length(eval_device_height), - ParsingRequirements::empty(), - ), - feature!( - atom!("device-aspect-ratio"), - AllowsRanges::Yes, - Evaluator::NumberRatio(eval_device_aspect_ratio), - ParsingRequirements::empty(), - ), - feature!( - atom!("-moz-device-orientation"), - AllowsRanges::No, - keyword_evaluator!(eval_device_orientation, Orientation), - ParsingRequirements::empty(), - ), - // Webkit extensions that we support for de-facto web compatibility. - // -webkit-{min|max}-device-pixel-ratio (controlled with its own pref): - feature!( - atom!("device-pixel-ratio"), - AllowsRanges::Yes, - Evaluator::Float(eval_device_pixel_ratio), - ParsingRequirements::WEBKIT_PREFIX, - ), - // -webkit-transform-3d. - feature!( - atom!("transform-3d"), - AllowsRanges::No, - Evaluator::BoolInteger(eval_transform_3d), - ParsingRequirements::WEBKIT_PREFIX, - ), - feature!( - atom!("-moz-device-pixel-ratio"), - AllowsRanges::Yes, - Evaluator::Float(eval_device_pixel_ratio), - ParsingRequirements::empty(), - ), - feature!( - atom!("resolution"), - AllowsRanges::Yes, - Evaluator::Resolution(eval_resolution), - ParsingRequirements::empty(), - ), - feature!( - atom!("display-mode"), - AllowsRanges::No, - keyword_evaluator!(eval_display_mode, DisplayMode), - ParsingRequirements::empty(), - ), - feature!( - atom!("grid"), - AllowsRanges::No, - Evaluator::BoolInteger(eval_grid), - ParsingRequirements::empty(), - ), - feature!( - atom!("scan"), - AllowsRanges::No, - keyword_evaluator!(eval_scan, Scan), - ParsingRequirements::empty(), - ), - feature!( - atom!("color"), - AllowsRanges::Yes, - Evaluator::Integer(eval_color), - ParsingRequirements::empty(), - ), - feature!( - atom!("color-index"), - AllowsRanges::Yes, - Evaluator::Integer(eval_color_index), - ParsingRequirements::empty(), - ), - feature!( - atom!("monochrome"), - AllowsRanges::Yes, - Evaluator::Integer(eval_monochrome), - ParsingRequirements::empty(), - ), - feature!( - atom!("prefers-reduced-motion"), - AllowsRanges::No, - keyword_evaluator!(eval_prefers_reduced_motion, PrefersReducedMotion), - ParsingRequirements::empty(), - ), - feature!( - atom!("overflow-block"), - AllowsRanges::No, - keyword_evaluator!(eval_overflow_block, OverflowBlock), - ParsingRequirements::empty(), - ), - feature!( - atom!("overflow-inline"), - AllowsRanges::No, - keyword_evaluator!(eval_overflow_inline, OverflowInline), - ParsingRequirements::empty(), - ), - feature!( - atom!("prefers-color-scheme"), - AllowsRanges::No, - keyword_evaluator!(eval_prefers_color_scheme, PrefersColorScheme), - ParsingRequirements::empty(), - ), - feature!( - atom!("pointer"), - AllowsRanges::No, - keyword_evaluator!(eval_pointer, Pointer), - ParsingRequirements::empty(), - ), - feature!( - atom!("any-pointer"), - AllowsRanges::No, - keyword_evaluator!(eval_any_pointer, Pointer), - ParsingRequirements::empty(), - ), - feature!( - atom!("hover"), - AllowsRanges::No, - keyword_evaluator!(eval_hover, Hover), - ParsingRequirements::empty(), - ), - feature!( - atom!("any-hover"), - AllowsRanges::No, - keyword_evaluator!(eval_any_hover, Hover), - ParsingRequirements::empty(), - ), - - // Internal -moz-is-glyph media feature: applies only inside SVG glyphs. - // Internal because it is really only useful in the user agent anyway - // and therefore not worth standardizing. - feature!( - atom!("-moz-is-glyph"), - AllowsRanges::No, - Evaluator::BoolInteger(eval_moz_is_glyph), - ParsingRequirements::CHROME_AND_UA_ONLY, - ), - feature!( - atom!("-moz-is-resource-document"), - AllowsRanges::No, - Evaluator::BoolInteger(eval_moz_is_resource_document), - ParsingRequirements::CHROME_AND_UA_ONLY, - ), - feature!( - atom!("-moz-os-version"), - AllowsRanges::No, - Evaluator::Ident(eval_moz_os_version), - ParsingRequirements::CHROME_AND_UA_ONLY, - ), - system_metric_feature!(atom!("-moz-scrollbar-start-backward")), - system_metric_feature!(atom!("-moz-scrollbar-start-forward")), - system_metric_feature!(atom!("-moz-scrollbar-end-backward")), - system_metric_feature!(atom!("-moz-scrollbar-end-forward")), - system_metric_feature!(atom!("-moz-scrollbar-thumb-proportional")), - system_metric_feature!(atom!("-moz-overlay-scrollbars")), - system_metric_feature!(atom!("-moz-windows-default-theme")), - system_metric_feature!(atom!("-moz-mac-graphite-theme")), - system_metric_feature!(atom!("-moz-mac-yosemite-theme")), - system_metric_feature!(atom!("-moz-windows-accent-color-in-titlebar")), - system_metric_feature!(atom!("-moz-windows-compositor")), - system_metric_feature!(atom!("-moz-windows-classic")), - system_metric_feature!(atom!("-moz-windows-glass")), - system_metric_feature!(atom!("-moz-menubar-drag")), - system_metric_feature!(atom!("-moz-swipe-animation-enabled")), - system_metric_feature!(atom!("-moz-gtk-csd-available")), - system_metric_feature!(atom!("-moz-gtk-csd-hide-titlebar-by-default")), - system_metric_feature!(atom!("-moz-gtk-csd-transparent-background")), - system_metric_feature!(atom!("-moz-gtk-csd-minimize-button")), - system_metric_feature!(atom!("-moz-gtk-csd-maximize-button")), - system_metric_feature!(atom!("-moz-gtk-csd-close-button")), - system_metric_feature!(atom!("-moz-gtk-csd-reversed-placement")), - system_metric_feature!(atom!("-moz-system-dark-theme")), - // This is the only system-metric media feature that's accessible to - // content as of today. - // FIXME(emilio): Restrict (or remove?) when bug 1035774 lands. - feature!( - atom!("-moz-touch-enabled"), - AllowsRanges::No, - Evaluator::BoolInteger(eval_moz_touch_enabled), - ParsingRequirements::empty(), - ), - ]; -} +/// Adding new media features requires (1) adding the new feature to this +/// array, with appropriate entries (and potentially any new code needed +/// to support new types in these entries and (2) ensuring that either +/// nsPresContext::MediaFeatureValuesChanged is called when the value that +/// would be returned by the evaluator function could change. +pub static MEDIA_FEATURES: [MediaFeatureDescription; 53] = [ + feature!( + atom!("width"), + AllowsRanges::Yes, + Evaluator::Length(eval_width), + ParsingRequirements::empty(), + ), + feature!( + atom!("height"), + AllowsRanges::Yes, + Evaluator::Length(eval_height), + ParsingRequirements::empty(), + ), + feature!( + atom!("aspect-ratio"), + AllowsRanges::Yes, + Evaluator::NumberRatio(eval_aspect_ratio), + ParsingRequirements::empty(), + ), + feature!( + atom!("orientation"), + AllowsRanges::No, + keyword_evaluator!(eval_orientation, Orientation), + ParsingRequirements::empty(), + ), + feature!( + atom!("device-width"), + AllowsRanges::Yes, + Evaluator::Length(eval_device_width), + ParsingRequirements::empty(), + ), + feature!( + atom!("device-height"), + AllowsRanges::Yes, + Evaluator::Length(eval_device_height), + ParsingRequirements::empty(), + ), + feature!( + atom!("device-aspect-ratio"), + AllowsRanges::Yes, + Evaluator::NumberRatio(eval_device_aspect_ratio), + ParsingRequirements::empty(), + ), + feature!( + atom!("-moz-device-orientation"), + AllowsRanges::No, + keyword_evaluator!(eval_device_orientation, Orientation), + ParsingRequirements::empty(), + ), + // Webkit extensions that we support for de-facto web compatibility. + // -webkit-{min|max}-device-pixel-ratio (controlled with its own pref): + feature!( + atom!("device-pixel-ratio"), + AllowsRanges::Yes, + Evaluator::Float(eval_device_pixel_ratio), + ParsingRequirements::WEBKIT_PREFIX, + ), + // -webkit-transform-3d. + feature!( + atom!("transform-3d"), + AllowsRanges::No, + Evaluator::BoolInteger(eval_transform_3d), + ParsingRequirements::WEBKIT_PREFIX, + ), + feature!( + atom!("-moz-device-pixel-ratio"), + AllowsRanges::Yes, + Evaluator::Float(eval_device_pixel_ratio), + ParsingRequirements::empty(), + ), + feature!( + atom!("resolution"), + AllowsRanges::Yes, + Evaluator::Resolution(eval_resolution), + ParsingRequirements::empty(), + ), + feature!( + atom!("display-mode"), + AllowsRanges::No, + keyword_evaluator!(eval_display_mode, DisplayMode), + ParsingRequirements::empty(), + ), + feature!( + atom!("grid"), + AllowsRanges::No, + Evaluator::BoolInteger(eval_grid), + ParsingRequirements::empty(), + ), + feature!( + atom!("scan"), + AllowsRanges::No, + keyword_evaluator!(eval_scan, Scan), + ParsingRequirements::empty(), + ), + feature!( + atom!("color"), + AllowsRanges::Yes, + Evaluator::Integer(eval_color), + ParsingRequirements::empty(), + ), + feature!( + atom!("color-index"), + AllowsRanges::Yes, + Evaluator::Integer(eval_color_index), + ParsingRequirements::empty(), + ), + feature!( + atom!("monochrome"), + AllowsRanges::Yes, + Evaluator::Integer(eval_monochrome), + ParsingRequirements::empty(), + ), + feature!( + atom!("prefers-reduced-motion"), + AllowsRanges::No, + keyword_evaluator!(eval_prefers_reduced_motion, PrefersReducedMotion), + ParsingRequirements::empty(), + ), + feature!( + atom!("overflow-block"), + AllowsRanges::No, + keyword_evaluator!(eval_overflow_block, OverflowBlock), + ParsingRequirements::empty(), + ), + feature!( + atom!("overflow-inline"), + AllowsRanges::No, + keyword_evaluator!(eval_overflow_inline, OverflowInline), + ParsingRequirements::empty(), + ), + feature!( + atom!("prefers-color-scheme"), + AllowsRanges::No, + keyword_evaluator!(eval_prefers_color_scheme, PrefersColorScheme), + ParsingRequirements::empty(), + ), + feature!( + atom!("pointer"), + AllowsRanges::No, + keyword_evaluator!(eval_pointer, Pointer), + ParsingRequirements::empty(), + ), + feature!( + atom!("any-pointer"), + AllowsRanges::No, + keyword_evaluator!(eval_any_pointer, Pointer), + ParsingRequirements::empty(), + ), + feature!( + atom!("hover"), + AllowsRanges::No, + keyword_evaluator!(eval_hover, Hover), + ParsingRequirements::empty(), + ), + feature!( + atom!("any-hover"), + AllowsRanges::No, + keyword_evaluator!(eval_any_hover, Hover), + ParsingRequirements::empty(), + ), + // Internal -moz-is-glyph media feature: applies only inside SVG glyphs. + // Internal because it is really only useful in the user agent anyway + // and therefore not worth standardizing. + feature!( + atom!("-moz-is-glyph"), + AllowsRanges::No, + Evaluator::BoolInteger(eval_moz_is_glyph), + ParsingRequirements::CHROME_AND_UA_ONLY, + ), + feature!( + atom!("-moz-is-resource-document"), + AllowsRanges::No, + Evaluator::BoolInteger(eval_moz_is_resource_document), + ParsingRequirements::CHROME_AND_UA_ONLY, + ), + feature!( + atom!("-moz-os-version"), + AllowsRanges::No, + Evaluator::Ident(eval_moz_os_version), + ParsingRequirements::CHROME_AND_UA_ONLY, + ), + system_metric_feature!(atom!("-moz-scrollbar-start-backward")), + system_metric_feature!(atom!("-moz-scrollbar-start-forward")), + system_metric_feature!(atom!("-moz-scrollbar-end-backward")), + system_metric_feature!(atom!("-moz-scrollbar-end-forward")), + system_metric_feature!(atom!("-moz-scrollbar-thumb-proportional")), + system_metric_feature!(atom!("-moz-overlay-scrollbars")), + system_metric_feature!(atom!("-moz-windows-default-theme")), + system_metric_feature!(atom!("-moz-mac-graphite-theme")), + system_metric_feature!(atom!("-moz-mac-yosemite-theme")), + system_metric_feature!(atom!("-moz-windows-accent-color-in-titlebar")), + system_metric_feature!(atom!("-moz-windows-compositor")), + system_metric_feature!(atom!("-moz-windows-classic")), + system_metric_feature!(atom!("-moz-windows-glass")), + system_metric_feature!(atom!("-moz-menubar-drag")), + system_metric_feature!(atom!("-moz-swipe-animation-enabled")), + system_metric_feature!(atom!("-moz-gtk-csd-available")), + system_metric_feature!(atom!("-moz-gtk-csd-hide-titlebar-by-default")), + system_metric_feature!(atom!("-moz-gtk-csd-transparent-background")), + system_metric_feature!(atom!("-moz-gtk-csd-minimize-button")), + system_metric_feature!(atom!("-moz-gtk-csd-maximize-button")), + system_metric_feature!(atom!("-moz-gtk-csd-close-button")), + system_metric_feature!(atom!("-moz-gtk-csd-reversed-placement")), + system_metric_feature!(atom!("-moz-system-dark-theme")), + // This is the only system-metric media feature that's accessible to + // content as of today. + // FIXME(emilio): Restrict (or remove?) when bug 1035774 lands. + feature!( + atom!("-moz-touch-enabled"), + AllowsRanges::No, + Evaluator::BoolInteger(eval_moz_touch_enabled), + ParsingRequirements::empty(), + ), +]; diff --git a/components/style/gecko/non_ts_pseudo_class_list.rs b/components/style/gecko/non_ts_pseudo_class_list.rs index ea5db98030a..7b737f4d9cf 100644 --- a/components/style/gecko/non_ts_pseudo_class_list.rs +++ b/components/style/gecko/non_ts_pseudo_class_list.rs @@ -50,6 +50,7 @@ macro_rules! apply_non_ts_list { ("-moz-devtools-highlighted", MozDevtoolsHighlighted, IN_DEVTOOLS_HIGHLIGHTED_STATE, PSEUDO_CLASS_ENABLED_IN_UA_SHEETS), ("-moz-styleeditor-transitioning", MozStyleeditorTransitioning, IN_STYLEEDITOR_TRANSITIONING_STATE, PSEUDO_CLASS_ENABLED_IN_UA_SHEETS), ("fullscreen", Fullscreen, IN_FULLSCREEN_STATE, _), + ("-moz-modal-dialog", MozModalDialog, IN_MODAL_DIALOG_STATE, PSEUDO_CLASS_ENABLED_IN_UA_SHEETS), // TODO(emilio): This is inconsistently named (the capital R). ("-moz-focusring", MozFocusRing, IN_FOCUSRING_STATE, _), ("-moz-broken", MozBroken, IN_BROKEN_STATE, _), @@ -79,8 +80,8 @@ macro_rules! apply_non_ts_list { ("out-of-range", OutOfRange, IN_OUTOFRANGE_STATE, _), ("default", Default, IN_DEFAULT_STATE, _), ("placeholder-shown", PlaceholderShown, IN_PLACEHOLDER_SHOWN_STATE, _), - ("-moz-read-only", MozReadOnly, IN_MOZ_READONLY_STATE, _), - ("-moz-read-write", MozReadWrite, IN_MOZ_READWRITE_STATE, _), + ("read-only", ReadOnly, IN_READONLY_STATE, _), + ("read-write", ReadWrite, IN_READWRITE_STATE, _), ("-moz-submit-invalid", MozSubmitInvalid, IN_MOZ_SUBMITINVALID_STATE, _), ("-moz-ui-valid", MozUIValid, IN_MOZ_UI_VALID_STATE, _), ("-moz-ui-invalid", MozUIInvalid, IN_MOZ_UI_INVALID_STATE, _), diff --git a/components/style/gecko/pseudo_element.rs b/components/style/gecko/pseudo_element.rs index d989380c02e..16e3ab312f4 100644 --- a/components/style/gecko/pseudo_element.rs +++ b/components/style/gecko/pseudo_element.rs @@ -37,7 +37,8 @@ impl ::selectors::parser::PseudoElement for PseudoElement { PseudoElement::Before | PseudoElement::After | PseudoElement::Marker | - PseudoElement::Placeholder + PseudoElement::Placeholder | + PseudoElement::FileChooserButton ) } @@ -92,6 +93,12 @@ impl PseudoElement { EAGER_PSEUDOS[i].clone() } + /// Whether the current pseudo element is animatable. + #[inline] + pub fn is_animatable(&self) -> bool { + matches!(*self, Self::Before | Self::After | Self::Marker) + } + /// Whether the current pseudo element is ::before or ::after. #[inline] pub fn is_before_or_after(&self) -> bool { @@ -153,7 +160,15 @@ impl PseudoElement { /// Whether this pseudo-element is enabled for all content. pub fn enabled_in_content(&self) -> bool { - (self.flags() & structs::CSS_PSEUDO_ELEMENT_ENABLED_IN_UA_SHEETS_AND_CHROME) == 0 + match *self { + PseudoElement::MozFocusOuter => { + static_prefs::pref!("layout.css.moz-focus-outer.enabled") + }, + PseudoElement::FileChooserButton => { + static_prefs::pref!("layout.css.file-chooser-button.enabled") + }, + _ => (self.flags() & structs::CSS_PSEUDO_ELEMENT_ENABLED_IN_UA_SHEETS_AND_CHROME) == 0, + } } /// Whether this pseudo is enabled explicitly in UA sheets. diff --git a/components/style/gecko/selector_parser.rs b/components/style/gecko/selector_parser.rs index 4b96aa2148c..fd0494fa4b4 100644 --- a/components/style/gecko/selector_parser.rs +++ b/components/style/gecko/selector_parser.rs @@ -119,6 +119,8 @@ impl NonTSPseudoClass { match_ignore_ascii_case! { &name, $($css => Some(NonTSPseudoClass::$name),)* "-moz-full-screen" => Some(NonTSPseudoClass::Fullscreen), + "-moz-read-only" => Some(NonTSPseudoClass::ReadOnly), + "-moz-read-write" => Some(NonTSPseudoClass::ReadWrite), _ => None, } } @@ -227,17 +229,6 @@ impl NonTSPseudoClass { NonTSPseudoClass::MozLWThemeDarkText ) } - - /// Returns true if the evaluation of the pseudo-class depends on the - /// element's attributes. - pub fn is_attr_based(&self) -> bool { - matches!( - *self, - NonTSPseudoClass::MozTableBorderNonzero | - NonTSPseudoClass::MozBrowserFrame | - NonTSPseudoClass::Lang(..) - ) - } } impl ::selectors::parser::NonTSPseudoClass for NonTSPseudoClass { @@ -352,7 +343,8 @@ impl<'a, 'i> ::selectors::Parser<'i> for SelectorParser<'a> { #[inline] fn parse_is_and_where(&self) -> bool { - static_prefs::pref!("layout.css.is-where-selectors.enabled") + self.in_user_agent_stylesheet() || + static_prefs::pref!("layout.css.is-where-selectors.enabled") } #[inline] diff --git a/components/style/gecko/snapshot.rs b/components/style/gecko/snapshot.rs index b2a66f709e2..26b3e1393dd 100644 --- a/components/style/gecko/snapshot.rs +++ b/components/style/gecko/snapshot.rs @@ -70,11 +70,15 @@ impl GeckoElementSnapshot { self.mClassAttributeChanged() } - /// Returns true if the snapshot recorded an attribute change which isn't a - /// class / id + /// Executes the callback once for each attribute that changed. #[inline] - pub fn other_attr_changed(&self) -> bool { - self.mOtherAttributeChanged() + pub fn each_attr_changed(&self, mut callback: F) + where + F: FnMut(&Atom), + { + for attr in self.mChangedAttrNames.iter() { + unsafe { Atom::with(attr.mRawPtr, &mut callback) } + } } /// selectors::Element::attr_matches diff --git a/components/style/gecko/url.rs b/components/style/gecko/url.rs index 27f3c60884c..678adb39b09 100644 --- a/components/style/gecko/url.rs +++ b/components/style/gecko/url.rs @@ -18,7 +18,7 @@ use std::fmt::{self, Write}; use std::mem::ManuallyDrop; use std::sync::RwLock; use style_traits::{CssWriter, ParseError, ToCss}; -use to_shmem::{SharedMemoryBuilder, ToShmem}; +use to_shmem::{self, SharedMemoryBuilder, ToShmem}; /// A CSS url() value for gecko. #[css(function = "url")] @@ -241,11 +241,11 @@ impl LoadDataSource { } impl ToShmem for LoadDataSource { - fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> ManuallyDrop { - ManuallyDrop::new(match self { + fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> to_shmem::Result { + Ok(ManuallyDrop::new(match self { LoadDataSource::Owned(..) => LoadDataSource::Lazy, LoadDataSource::Lazy => LoadDataSource::Lazy, - }) + })) } } diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index a5472fadd66..62af71f212b 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -1446,7 +1446,7 @@ impl<'le> TElement for GeckoElement<'le> { #[inline] fn may_have_animations(&self) -> bool { if let Some(pseudo) = self.implemented_pseudo_element() { - if !pseudo.is_before_or_after() { + if !pseudo.is_animatable() { return false; } // FIXME(emilio): When would the parent of a ::before / ::after @@ -2037,8 +2037,8 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { NonTSPseudoClass::MozHandlerCrashed | NonTSPseudoClass::Required | NonTSPseudoClass::Optional | - NonTSPseudoClass::MozReadOnly | - NonTSPseudoClass::MozReadWrite | + NonTSPseudoClass::ReadOnly | + NonTSPseudoClass::ReadWrite | NonTSPseudoClass::FocusWithin | NonTSPseudoClass::FocusVisible | NonTSPseudoClass::MozDragOver | @@ -2062,6 +2062,7 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { NonTSPseudoClass::MozDirAttrRTL | NonTSPseudoClass::MozDirAttrLikeAuto | NonTSPseudoClass::MozAutofill | + NonTSPseudoClass::MozModalDialog | NonTSPseudoClass::Active | NonTSPseudoClass::Hover | NonTSPseudoClass::MozAutofillPreview => { diff --git a/components/style/gecko_bindings/sugar/ns_t_array.rs b/components/style/gecko_bindings/sugar/ns_t_array.rs index e3d4e5b86bd..04ba326c615 100644 --- a/components/style/gecko_bindings/sugar/ns_t_array.rs +++ b/components/style/gecko_bindings/sugar/ns_t_array.rs @@ -5,7 +5,7 @@ //! Rust helpers for Gecko's nsTArray. use crate::gecko_bindings::bindings; -use crate::gecko_bindings::structs::{nsTArray, nsTArrayHeader}; +use crate::gecko_bindings::structs::{nsTArray, nsTArrayHeader, CopyableTArray}; use std::mem; use std::ops::{Deref, DerefMut}; use std::slice; @@ -128,3 +128,16 @@ impl nsTArray { self.iter_mut().zip(iter).for_each(|(r, v)| *r = v); } } + +impl Deref for CopyableTArray { + type Target = nsTArray; + fn deref(&self) -> &Self::Target { + &self._base + } +} + +impl DerefMut for CopyableTArray { + fn deref_mut(&mut self) -> &mut nsTArray { + &mut self._base + } +} diff --git a/components/style/gecko_bindings/sugar/ownership.rs b/components/style/gecko_bindings/sugar/ownership.rs index 5170b7dbc5a..366e4619c68 100644 --- a/components/style/gecko_bindings/sugar/ownership.rs +++ b/components/style/gecko_bindings/sugar/ownership.rs @@ -4,6 +4,7 @@ //! Helpers for different FFI pointer kinds that Gecko's FFI layer uses. +use gecko_bindings::structs::root::mozilla::detail::CopyablePtr; use servo_arc::{Arc, RawOffsetArc}; use std::marker::PhantomData; use std::mem::{forget, transmute}; @@ -332,3 +333,16 @@ impl OwnedOrNull { unsafe { transmute(self) } } } + +impl Deref for CopyablePtr { + type Target = T; + fn deref(&self) -> &Self::Target { + &self.mPtr + } +} + +impl DerefMut for CopyablePtr { + fn deref_mut<'a>(&'a mut self) -> &'a mut T { + &mut self.mPtr + } +} diff --git a/components/style/gecko_string_cache/mod.rs b/components/style/gecko_string_cache/mod.rs index b9e3a0c608a..e9208a5e32e 100644 --- a/components/style/gecko_string_cache/mod.rs +++ b/components/style/gecko_string_cache/mod.rs @@ -3,7 +3,6 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ #![allow(unsafe_code)] - // This is needed for the constants in atom_macro.rs, because we have some // atoms whose names differ only by case, e.g. datetime and dateTime. #![allow(non_upper_case_globals)] @@ -30,7 +29,7 @@ use std::num::NonZeroUsize; use std::ops::Deref; use std::{slice, str}; use style_traits::SpecifiedValueInfo; -use to_shmem::{SharedMemoryBuilder, ToShmem}; +use to_shmem::{self, SharedMemoryBuilder, ToShmem}; #[macro_use] #[allow(improper_ctypes, non_camel_case_types, missing_docs)] @@ -132,14 +131,15 @@ impl Borrow for Atom { } impl ToShmem for Atom { - fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> ManuallyDrop { - assert!( - self.is_static(), - "ToShmem failed for Atom: must be a static atom: {}", - self - ); + fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> to_shmem::Result { + if !self.is_static() { + return Err(format!( + "ToShmem failed for Atom: must be a static atom: {}", + self + )); + } - ManuallyDrop::new(Atom(self.0)) + Ok(ManuallyDrop::new(Atom(self.0))) } } diff --git a/components/style/invalidation/element/document_state.rs b/components/style/invalidation/element/document_state.rs index 7fc51338b1e..9ee97344a4c 100644 --- a/components/style/invalidation/element/document_state.rs +++ b/components/style/invalidation/element/document_state.rs @@ -6,6 +6,7 @@ use crate::dom::TElement; use crate::element_state::DocumentState; +use crate::invalidation::element::invalidation_map::Dependency; use crate::invalidation::element::invalidator::{DescendantInvalidationLists, InvalidationVector}; use crate::invalidation::element::invalidator::{Invalidation, InvalidationProcessor}; use crate::invalidation::element::state_and_attributes; @@ -65,6 +66,14 @@ where E: TElement, I: Iterator, { + fn check_outer_dependency(&mut self, _: &Dependency, _: E) -> bool { + debug_assert!( + false, + "how, we should only have parent-less dependencies here!" + ); + true + } + fn collect_invalidations( &mut self, _element: E, @@ -81,10 +90,14 @@ where // We pass `None` as a scope, as document state selectors aren't // affected by the current scope. + // + // FIXME(emilio): We should really pass the relevant host for + // self.rules, so that we invalidate correctly if the selector + // happens to have something like :host(:-moz-window-inactive) + // for example. self_invalidations.push(Invalidation::new( - &dependency.selector, + &dependency.dependency, /* scope = */ None, - 0, )); } } diff --git a/components/style/invalidation/element/invalidation_map.rs b/components/style/invalidation/element/invalidation_map.rs index dca418703be..6d9b6c1e85d 100644 --- a/components/style/invalidation/element/invalidation_map.rs +++ b/components/style/invalidation/element/invalidation_map.rs @@ -6,7 +6,9 @@ use crate::context::QuirksMode; use crate::element_state::{DocumentState, ElementState}; -use crate::selector_map::{MaybeCaseInsensitiveHashMap, SelectorMap, SelectorMapEntry}; +use crate::selector_map::{ + MaybeCaseInsensitiveHashMap, PrecomputedHashMap, SelectorMap, SelectorMapEntry, +}; use crate::selector_parser::SelectorImpl; use crate::{Atom, LocalName, Namespace}; use fallible::FallibleVec; @@ -47,6 +49,23 @@ pub struct Dependency { /// The offset into the selector that we should match on. pub selector_offset: usize, + + /// The parent dependency for an ancestor selector. For example, consider + /// the following: + /// + /// .foo .bar:where(.baz span) .qux + /// ^ ^ ^ + /// A B C + /// + /// We'd generate: + /// + /// * One dependency for .qux (offset: 0, parent: None) + /// * One dependency for .baz pointing to B with parent being a + /// dependency pointing to C. + /// * One dependency from .bar pointing to C (parent: None) + /// * One dependency from .foo pointing to A (parent: None) + /// + pub parent: Option>, } /// The kind of elements down the tree this dependency may affect. @@ -71,6 +90,22 @@ pub enum DependencyInvalidationKind { } impl Dependency { + /// Creates a dummy dependency to invalidate the whole selector. + /// + /// This is necessary because document state invalidation wants to + /// invalidate all elements in the document. + /// + /// The offset is such as that Invalidation::new(self) returns a zero + /// offset. That is, it points to a virtual "combinator" outside of the + /// selector, so calling combinator() on such a dependency will panic. + pub fn for_full_selector_invalidation(selector: Selector) -> Self { + Self { + selector_offset: selector.len() + 1, + selector, + parent: None, + } + } + /// Returns the combinator to the right of the partial selector this /// dependency represents. /// @@ -130,31 +165,18 @@ impl SelectorMapEntry for StateDependency { /// The same, but for document state selectors. #[derive(Clone, Debug, MallocSizeOf)] pub struct DocumentStateDependency { - /// The selector that is affected. We don't need to track an offset, since - /// when it changes it changes for the whole document anyway. + /// We track `Dependency` even though we don't need to track an offset, + /// since when it changes it changes for the whole document anyway. #[cfg_attr( feature = "gecko", ignore_malloc_size_of = "CssRules have primary refs, we measure there" )] #[cfg_attr(feature = "servo", ignore_malloc_size_of = "Arc")] - pub selector: Selector, + pub dependency: Dependency, /// The state this dependency is affected by. pub state: DocumentState, } -bitflags! { - /// A set of flags that denote whether any invalidations have occurred - /// for a particular attribute selector. - #[derive(MallocSizeOf)] - #[repr(C)] - pub struct InvalidationMapFlags : u8 { - /// Whether [class] or such is used. - const HAS_CLASS_ATTR_SELECTOR = 1 << 0; - /// Whether [id] or such is used. - const HAS_ID_ATTR_SELECTOR = 1 << 1; - } -} - /// A map where we store invalidations. /// /// This is slightly different to a SelectorMap, in the sense of that the same @@ -176,10 +198,8 @@ pub struct InvalidationMap { /// A list of document state dependencies in the rules we represent. pub document_state_selectors: Vec, /// A map of other attribute affecting selectors. - pub other_attribute_affecting_selectors: SelectorMap, - /// A set of flags that contain whether various special attributes are used - /// in this invalidation map. - pub flags: InvalidationMapFlags, + pub other_attribute_affecting_selectors: + PrecomputedHashMap>, } impl InvalidationMap { @@ -190,8 +210,7 @@ impl InvalidationMap { id_to_selector: MaybeCaseInsensitiveHashMap::new(), state_affecting_selectors: SelectorMap::new(), document_state_selectors: Vec::new(), - other_attribute_affecting_selectors: SelectorMap::new(), - flags: InvalidationMapFlags::empty(), + other_attribute_affecting_selectors: PrecomputedHashMap::default(), } } @@ -199,7 +218,9 @@ impl InvalidationMap { pub fn len(&self) -> usize { self.state_affecting_selectors.len() + self.document_state_selectors.len() + - self.other_attribute_affecting_selectors.len() + + self.other_attribute_affecting_selectors + .iter() + .fold(0, |accum, (_, ref v)| accum + v.len()) + self.id_to_selector .iter() .fold(0, |accum, (_, ref v)| accum + v.len()) + @@ -215,7 +236,6 @@ impl InvalidationMap { self.state_affecting_selectors.clear(); self.document_state_selectors.clear(); self.other_attribute_affecting_selectors.clear(); - self.flags = InvalidationMapFlags::empty(); } /// Adds a selector to this `InvalidationMap`. Returns Err(..) to @@ -227,115 +247,60 @@ impl InvalidationMap { ) -> Result<(), FailedAllocationError> { debug!("InvalidationMap::note_selector({:?})", selector); - let mut iter = selector.iter(); - let mut combinator; - let mut index = 0; - let mut document_state = DocumentState::empty(); - loop { - let sequence_start = index; - - let mut compound_visitor = CompoundSelectorDependencyCollector { - classes: SmallVec::new(), - ids: SmallVec::new(), - state: ElementState::empty(), + { + let mut parent_stack = SmallVec::new(); + let mut alloc_error = None; + let mut collector = SelectorDependencyCollector { + map: self, document_state: &mut document_state, - other_attributes: false, - flags: &mut self.flags, + selector, + parent_selectors: &mut parent_stack, + quirks_mode, + compound_state: PerCompoundState::new(0), + alloc_error: &mut alloc_error, }; - // Visit all the simple selectors in this sequence. - // - // Note that this works because we can't have combinators nested - // inside simple selectors (i.e. in :not() or :-moz-any()). - // - // If we ever support that we'll need to visit nested complex - // selectors as well, in order to mark them as affecting descendants - // at least. - for ss in &mut iter { - ss.visit(&mut compound_visitor); - index += 1; // Account for the simple selector. + let visit_result = collector.visit_whole_selector(); + debug_assert_eq!(!visit_result, alloc_error.is_some()); + if let Some(alloc_error) = alloc_error { + return Err(alloc_error); } - - for class in compound_visitor.classes { - self.class_to_selector - .try_entry(class, quirks_mode)? - .or_insert_with(SmallVec::new) - .try_push(Dependency { - selector: selector.clone(), - selector_offset: sequence_start, - })?; - } - - for id in compound_visitor.ids { - self.id_to_selector - .try_entry(id, quirks_mode)? - .or_insert_with(SmallVec::new) - .try_push(Dependency { - selector: selector.clone(), - selector_offset: sequence_start, - })?; - } - - if !compound_visitor.state.is_empty() { - self.state_affecting_selectors.insert( - StateDependency { - dep: Dependency { - selector: selector.clone(), - selector_offset: sequence_start, - }, - state: compound_visitor.state, - }, - quirks_mode, - )?; - } - - if compound_visitor.other_attributes { - self.other_attribute_affecting_selectors.insert( - Dependency { - selector: selector.clone(), - selector_offset: sequence_start, - }, - quirks_mode, - )?; - } - - combinator = iter.next_sequence(); - if combinator.is_none() { - break; - } - - index += 1; // Account for the combinator. } if !document_state.is_empty() { - self.document_state_selectors - .try_push(DocumentStateDependency { - state: document_state, - selector: selector.clone(), - })?; + let dep = DocumentStateDependency { + state: document_state, + dependency: Dependency::for_full_selector_invalidation(selector.clone()), + }; + self.document_state_selectors.try_push(dep)?; } Ok(()) } } -/// A struct that collects invalidations for a given compound selector. -/// -/// FIXME(emilio, bug 1509418): :where is mishandled, figure out the right way -/// to do invalidation for :where when combinators are inside. -/// -/// Simplest feasible idea seems to be: For each :where branch, if there are -/// combinators in the branch, treat as its own selector (basically, call -/// `.note_selector` with the nested selector). That may over-invalidate, but -/// not too much. If there are no combinators, then behave like we do today and -/// use the outer selector as a whole. If we care a lot about that potential -/// over-invalidation where combinators are present then we need more complex -/// data-structures in `Dependency`. -struct CompoundSelectorDependencyCollector<'a> { +struct PerCompoundState { + /// The offset at which our compound starts. + offset: usize, + /// The state this compound selector is affected by. - state: ElementState, + element_state: ElementState, +} + +impl PerCompoundState { + fn new(offset: usize) -> Self { + Self { + offset, + element_state: ElementState::empty(), + } + } +} + +/// A struct that collects invalidations for a given compound selector. +struct SelectorDependencyCollector<'a> { + map: &'a mut InvalidationMap, /// The document this _complex_ selector is affected by. /// @@ -343,76 +308,226 @@ struct CompoundSelectorDependencyCollector<'a> { /// state and it changes for everything. document_state: &'a mut DocumentState, - /// The classes this compound selector is affected by. + /// The current selector and offset we're iterating. + selector: &'a Selector, + + /// The stack of parent selectors that we have, and at which offset of the + /// sequence. /// - /// NB: This will be often a single class, but could be multiple in - /// presence of :not, :-moz-any, .foo.bar.baz, etc. - classes: SmallVec<[Atom; 5]>, + /// This starts empty. It grows when we find nested :is and :where selector + /// lists. + parent_selectors: &'a mut SmallVec<[(Selector, usize); 5]>, - /// The IDs this compound selector is affected by. - /// - /// NB: This will be almost always a single id, but could be multiple in - /// presence of :not, :-moz-any, #foo#bar, etc. - ids: SmallVec<[Atom; 5]>, + /// The quirks mode of the document where we're inserting dependencies. + quirks_mode: QuirksMode, - /// Whether it affects other attribute-dependent selectors that aren't ID or - /// class selectors (NB: We still set this to true in presence of [class] or - /// [id] attribute selectors). - other_attributes: bool, + /// State relevant to a given compound selector. + compound_state: PerCompoundState, - /// The invalidation map flags, that we set when some attribute selectors are present. - flags: &'a mut InvalidationMapFlags, + /// The allocation error, if we OOM. + alloc_error: &'a mut Option, } -impl<'a> SelectorVisitor for CompoundSelectorDependencyCollector<'a> { +impl<'a> SelectorDependencyCollector<'a> { + fn visit_whole_selector(&mut self) -> bool { + let iter = self.selector.iter(); + self.visit_whole_selector_from(iter, 0) + } + + fn visit_whole_selector_from( + &mut self, + mut iter: SelectorIter, + mut index: usize, + ) -> bool { + loop { + // Reset the compound state. + self.compound_state = PerCompoundState::new(index); + + // Visit all the simple selectors in this sequence. + for ss in &mut iter { + if !ss.visit(self) { + return false; + } + index += 1; // Account for the simple selector. + } + + if !self.compound_state.element_state.is_empty() { + let dependency = self.dependency(); + let result = self.map.state_affecting_selectors.insert( + StateDependency { + dep: dependency, + state: self.compound_state.element_state, + }, + self.quirks_mode, + ); + if let Err(alloc_error) = result { + *self.alloc_error = Some(alloc_error); + return false; + } + } + + let combinator = iter.next_sequence(); + if combinator.is_none() { + return true; + } + index += 1; // account for the combinator + } + } + + fn add_attr_dependency(&mut self, name: LocalName) -> bool { + let dependency = self.dependency(); + + let map = &mut self.map.other_attribute_affecting_selectors; + let entry = match map.try_entry(name) { + Ok(entry) => entry, + Err(err) => { + *self.alloc_error = Some(err); + return false; + }, + }; + + match entry.or_insert_with(SmallVec::new).try_push(dependency) { + Ok(..) => true, + Err(err) => { + *self.alloc_error = Some(err); + return false; + }, + } + } + + fn dependency(&self) -> Dependency { + let mut parent = None; + + // TODO(emilio): Maybe we should refcount the parent dependencies, or + // cache them or something. + for &(ref selector, ref selector_offset) in self.parent_selectors.iter() { + debug_assert_ne!( + self.compound_state.offset, 0, + "Shouldn't bother creating nested dependencies for the rightmost compound", + ); + let new_parent = Dependency { + selector: selector.clone(), + selector_offset: *selector_offset, + parent, + }; + parent = Some(Box::new(new_parent)); + } + + Dependency { + selector: self.selector.clone(), + selector_offset: self.compound_state.offset, + parent, + } + } +} + +impl<'a> SelectorVisitor for SelectorDependencyCollector<'a> { type Impl = SelectorImpl; + fn visit_selector_list(&mut self, list: &[Selector]) -> bool { + for selector in list { + // Here we cheat a bit: We can visit the rightmost compound with + // the "outer" visitor, and it'd be fine. This reduces the amount of + // state and attribute invalidations, and we need to check the outer + // selector to the left anyway to avoid over-invalidation, so it + // avoids matching it twice uselessly. + let mut iter = selector.iter(); + let mut index = 0; + + for ss in &mut iter { + if !ss.visit(self) { + return false; + } + index += 1; + } + + let combinator = iter.next_sequence(); + if combinator.is_none() { + continue; + } + + index += 1; // account for the combinator. + + self.parent_selectors + .push((self.selector.clone(), self.compound_state.offset)); + let mut nested = SelectorDependencyCollector { + map: &mut *self.map, + document_state: &mut *self.document_state, + selector, + parent_selectors: &mut *self.parent_selectors, + quirks_mode: self.quirks_mode, + compound_state: PerCompoundState::new(index), + alloc_error: &mut *self.alloc_error, + }; + if !nested.visit_whole_selector_from(iter, index) { + return false; + } + self.parent_selectors.pop(); + } + true + } + fn visit_simple_selector(&mut self, s: &Component) -> bool { - #[cfg(feature = "gecko")] use crate::selector_parser::NonTSPseudoClass; match *s { - Component::ID(ref id) => { - self.ids.push(id.clone()); - }, - Component::Class(ref class) => { - self.classes.push(class.clone()); + Component::ID(ref atom) | Component::Class(ref atom) => { + let dependency = self.dependency(); + let map = match *s { + Component::ID(..) => &mut self.map.id_to_selector, + Component::Class(..) => &mut self.map.class_to_selector, + _ => unreachable!(), + }; + let entry = match map.try_entry(atom.clone(), self.quirks_mode) { + Ok(entry) => entry, + Err(err) => { + *self.alloc_error = Some(err); + return false; + }, + }; + match entry.or_insert_with(SmallVec::new).try_push(dependency) { + Ok(..) => true, + Err(err) => { + *self.alloc_error = Some(err); + return false; + }, + } }, Component::NonTSPseudoClass(ref pc) => { - self.other_attributes |= pc.is_attr_based(); - self.state |= match *pc { + self.compound_state.element_state |= match *pc { #[cfg(feature = "gecko")] NonTSPseudoClass::Dir(ref dir) => dir.element_state(), _ => pc.state_flag(), }; *self.document_state |= pc.document_state_flag(); - }, - _ => {}, - } - true + let attr_name = match *pc { + #[cfg(feature = "gecko")] + NonTSPseudoClass::MozTableBorderNonzero => local_name!("border"), + #[cfg(feature = "gecko")] + NonTSPseudoClass::MozBrowserFrame => local_name!("mozbrowser"), + NonTSPseudoClass::Lang(..) => local_name!("lang"), + _ => return true, + }; + + self.add_attr_dependency(attr_name) + }, + _ => true, + } } fn visit_attribute_selector( &mut self, - constraint: &NamespaceConstraint<&Namespace>, - _local_name: &LocalName, + _: &NamespaceConstraint<&Namespace>, + local_name: &LocalName, local_name_lower: &LocalName, ) -> bool { - self.other_attributes = true; - let may_match_in_no_namespace = match *constraint { - NamespaceConstraint::Any => true, - NamespaceConstraint::Specific(ref ns) => ns.is_empty(), - }; + if !self.add_attr_dependency(local_name.clone()) { + return false; + } - if may_match_in_no_namespace { - if *local_name_lower == local_name!("id") { - self.flags - .insert(InvalidationMapFlags::HAS_ID_ATTR_SELECTOR) - } else if *local_name_lower == local_name!("class") { - self.flags - .insert(InvalidationMapFlags::HAS_CLASS_ATTR_SELECTOR) - } + if local_name != local_name_lower && !self.add_attr_dependency(local_name_lower.clone()) { + return false; } true diff --git a/components/style/invalidation/element/invalidator.rs b/components/style/invalidation/element/invalidator.rs index f0f55595dff..c148b63e3e3 100644 --- a/components/style/invalidation/element/invalidator.rs +++ b/components/style/invalidation/element/invalidator.rs @@ -7,10 +7,10 @@ use crate::context::StackLimitChecker; use crate::dom::{TElement, TNode, TShadowRoot}; -use crate::selector_parser::SelectorImpl; +use crate::invalidation::element::invalidation_map::{Dependency, DependencyInvalidationKind}; use selectors::matching::matches_compound_selector_from; use selectors::matching::{CompoundSelectorMatchingResult, MatchingContext}; -use selectors::parser::{Combinator, Component, Selector}; +use selectors::parser::{Combinator, Component}; use selectors::OpaqueElement; use smallvec::SmallVec; use std::fmt; @@ -34,6 +34,27 @@ where false } + /// When a dependency from a :where or :is selector matches, it may still be + /// the case that we don't need to invalidate the full style. Consider the + /// case of: + /// + /// div .foo:where(.bar *, .baz) .qux + /// + /// We can get to the `*` part after a .bar class change, but you only need + /// to restyle the element if it also matches .foo. + /// + /// Similarly, you only need to restyle .baz if the whole result of matching + /// the selector changes. + /// + /// This function is called to check the result of matching the "outer" + /// dependency that we generate for the parent of the `:where` selector, + /// that is, in the case above it should match + /// `div .foo:where(.bar *, .baz)`. + /// + /// Returning true unconditionally here is over-optimistic and may + /// over-invalidate. + fn check_outer_dependency(&mut self, dependency: &Dependency, element: E) -> bool; + /// The matching context that should be used to process invalidations. fn matching_context(&mut self) -> &mut MatchingContext<'a, E::Impl>; @@ -127,7 +148,11 @@ enum InvalidationKind { /// relative to a current element we are processing, must be restyled. #[derive(Clone)] pub struct Invalidation<'a> { - selector: &'a Selector, + /// The dependency that generated this invalidation. + /// + /// Note that the offset inside the dependency is not really useful after + /// construction. + dependency: &'a Dependency, /// The right shadow host from where the rule came from, if any. /// /// This is needed to ensure that we match the selector with the right @@ -138,6 +163,8 @@ pub struct Invalidation<'a> { /// /// This order is a "parse order" offset, that is, zero is the leftmost part /// of the selector written as parsed / serialized. + /// + /// It is initialized from the offset from `dependency`. offset: usize, /// Whether the invalidation was already matched by any previous sibling or /// ancestor. @@ -149,16 +176,18 @@ pub struct Invalidation<'a> { } impl<'a> Invalidation<'a> { - /// Create a new invalidation for a given selector and offset. - pub fn new( - selector: &'a Selector, - scope: Option, - offset: usize, - ) -> Self { + /// Create a new invalidation for matching a dependency. + pub fn new(dependency: &'a Dependency, scope: Option) -> Self { + debug_assert!( + dependency.selector_offset == dependency.selector.len() + 1 || + dependency.invalidation_kind() != DependencyInvalidationKind::Element, + "No point to this, if the dependency matched the element we should just invalidate it" + ); Self { - selector, + dependency, scope, - offset, + // + 1 to go past the combinator. + offset: dependency.selector.len() + 1 - dependency.selector_offset, matched_by_any_previous: false, } } @@ -174,7 +203,11 @@ impl<'a> Invalidation<'a> { // for the weird pseudos in . // // We should be able to do better here! - match self.selector.combinator_at_parse_order(self.offset - 1) { + match self + .dependency + .selector + .combinator_at_parse_order(self.offset - 1) + { Combinator::Descendant | Combinator::LaterSibling | Combinator::PseudoElement => true, Combinator::Part | Combinator::SlotAssignment | @@ -188,7 +221,11 @@ impl<'a> Invalidation<'a> { return InvalidationKind::Descendant(DescendantInvalidationKind::Dom); } - match self.selector.combinator_at_parse_order(self.offset - 1) { + match self + .dependency + .selector + .combinator_at_parse_order(self.offset - 1) + { Combinator::Child | Combinator::Descendant | Combinator::PseudoElement => { InvalidationKind::Descendant(DescendantInvalidationKind::Dom) }, @@ -206,7 +243,11 @@ impl<'a> fmt::Debug for Invalidation<'a> { use cssparser::ToCss; f.write_str("Invalidation(")?; - for component in self.selector.iter_raw_parse_order_from(self.offset) { + for component in self + .dependency + .selector + .iter_raw_parse_order_from(self.offset) + { if matches!(*component, Component::Combinator(..)) { break; } @@ -763,201 +804,251 @@ where context.current_host = invalidation.scope; matches_compound_selector_from( - &invalidation.selector, + &invalidation.dependency.selector, invalidation.offset, context, &self.element, ) }; - let mut invalidated_self = false; - let mut matched = false; - match matching_result { + let next_invalidation = match matching_result { + CompoundSelectorMatchingResult::NotMatched => { + return SingleInvalidationResult { + invalidated_self: false, + matched: false, + } + }, CompoundSelectorMatchingResult::FullyMatched => { debug!(" > Invalidation matched completely"); - matched = true; - invalidated_self = true; + // We matched completely. If we're an inner selector now we need + // to go outside our selector and carry on invalidating. + let mut cur_dependency = invalidation.dependency; + loop { + cur_dependency = match cur_dependency.parent { + None => { + return SingleInvalidationResult { + invalidated_self: true, + matched: true, + } + }, + Some(ref p) => &**p, + }; + + debug!(" > Checking outer dependency {:?}", cur_dependency); + + // The inner selector changed, now check if the full + // previous part of the selector did, before keeping + // checking for descendants. + if !self + .processor + .check_outer_dependency(cur_dependency, self.element) + { + return SingleInvalidationResult { + invalidated_self: false, + matched: false, + }; + } + + if cur_dependency.invalidation_kind() == DependencyInvalidationKind::Element { + continue; + } + + debug!(" > Generating invalidation"); + break Invalidation::new(cur_dependency, invalidation.scope); + } }, CompoundSelectorMatchingResult::Matched { next_combinator_offset, - } => { - let next_combinator = invalidation - .selector - .combinator_at_parse_order(next_combinator_offset); - matched = true; - - if matches!(next_combinator, Combinator::PseudoElement) { - // This will usually be the very next component, except for - // the fact that we store compound selectors the other way - // around, so there could also be state pseudo-classes. - let pseudo_selector = invalidation - .selector - .iter_raw_parse_order_from(next_combinator_offset + 1) - .skip_while(|c| matches!(**c, Component::NonTSPseudoClass(..))) - .next() - .unwrap(); - - let pseudo = match *pseudo_selector { - Component::PseudoElement(ref pseudo) => pseudo, - _ => unreachable!( - "Someone seriously messed up selector parsing: \ - {:?} at offset {:?}: {:?}", - invalidation.selector, next_combinator_offset, pseudo_selector, - ), - }; - - // FIXME(emilio): This is not ideal, and could not be - // accurate if we ever have stateful element-backed eager - // pseudos. - // - // Ideally, we'd just remove element-backed eager pseudos - // altogether, given they work fine without it. Only gotcha - // is that we wouldn't style them in parallel, which may or - // may not be an issue. - // - // Also, this could be more fine grained now (perhaps a - // RESTYLE_PSEUDOS hint?). - // - // Note that we'll also restyle the pseudo-element because - // it would match this invalidation. - if self.processor.invalidates_on_eager_pseudo_element() { - if pseudo.is_eager() { - invalidated_self = true; - } - // If we start or stop matching some marker rules, and - // don't have a marker, then we need to restyle the - // element to potentially create one. - // - // Same caveats as for other eager pseudos apply, this - // could be more fine-grained. - if pseudo.is_marker() && self.element.marker_pseudo_element().is_none() { - invalidated_self = true; - } - - // FIXME: ::selection doesn't generate elements, so the - // regular invalidation doesn't work for it. We store - // the cached selection style holding off the originating - // element, so we need to restyle it in order to invalidate - // it. This is still not quite correct, since nothing - // triggers a repaint necessarily, but matches old Gecko - // behavior, and the ::selection implementation needs to - // change significantly anyway to implement - // https://github.com/w3c/csswg-drafts/issues/2474. - if pseudo.is_selection() { - invalidated_self = true; - } - } - } - - let next_invalidation = Invalidation { - selector: invalidation.selector, - scope: invalidation.scope, - offset: next_combinator_offset + 1, - matched_by_any_previous: false, - }; - - debug!( - " > Invalidation matched, next: {:?}, ({:?})", - next_invalidation, next_combinator - ); - - let next_invalidation_kind = next_invalidation.kind(); - - // We can skip pushing under some circumstances, and we should - // because otherwise the invalidation list could grow - // exponentially. - // - // * First of all, both invalidations need to be of the same - // kind. This is because of how we propagate them going to - // the right of the tree for sibling invalidations and going - // down the tree for children invalidations. A sibling - // invalidation that ends up generating a children - // invalidation ends up (correctly) in five different lists, - // not in the same list five different times. - // - // * Then, the invalidation needs to be matched by a previous - // ancestor/sibling, in order to know that this invalidation - // has been generated already. - // - // * Finally, the new invalidation needs to be - // `effective_for_next()`, in order for us to know that it is - // still in the list, since we remove the dependencies that - // aren't from the lists for our children / siblings. - // - // To go through an example, let's imagine we are processing a - // dom subtree like: - // - //
- // - // And an invalidation list with a single invalidation like: - // - // [div div div] - // - // When we process the invalidation list for the outer div, we - // match it, and generate a `div div` invalidation, so for the - //
child we have: - // - // [div div div, div div] - // - // With the first of them marked as `matched`. - // - // When we process the
child, we don't match any of - // them, so both invalidations go untouched to our children. - // - // When we process the second
, we match _both_ - // invalidations. - // - // However, when matching the first, we can tell it's been - // matched, and not push the corresponding `div div` - // invalidation, since we know it's necessarily already on the - // list. - // - // Thus, without skipping the push, we'll arrive to the - // innermost
with: - // - // [div div div, div div, div div, div] - // - // While skipping it, we won't arrive here with duplicating - // dependencies: - // - // [div div div, div div, div] - // - let can_skip_pushing = next_invalidation_kind == invalidation_kind && - invalidation.matched_by_any_previous && - next_invalidation.effective_for_next(); - - if can_skip_pushing { - debug!( - " > Can avoid push, since the invalidation had \ - already been matched before" - ); - } else { - match next_invalidation_kind { - InvalidationKind::Descendant(DescendantInvalidationKind::Dom) => { - descendant_invalidations - .dom_descendants - .push(next_invalidation); - }, - InvalidationKind::Descendant(DescendantInvalidationKind::Part) => { - descendant_invalidations.parts.push(next_invalidation); - }, - InvalidationKind::Descendant(DescendantInvalidationKind::Slotted) => { - descendant_invalidations - .slotted_descendants - .push(next_invalidation); - }, - InvalidationKind::Sibling => { - sibling_invalidations.push(next_invalidation); - }, - } - } + } => Invalidation { + dependency: invalidation.dependency, + scope: invalidation.scope, + offset: next_combinator_offset + 1, + matched_by_any_previous: false, }, - CompoundSelectorMatchingResult::NotMatched => {}, + }; + + debug_assert_ne!( + next_invalidation.offset, 0, + "Rightmost selectors shouldn't generate more invalidations", + ); + + let mut invalidated_self = false; + let next_combinator = next_invalidation + .dependency + .selector + .combinator_at_parse_order(next_invalidation.offset - 1); + + if matches!(next_combinator, Combinator::PseudoElement) { + // This will usually be the very next component, except for + // the fact that we store compound selectors the other way + // around, so there could also be state pseudo-classes. + let pseudo = next_invalidation + .dependency + .selector + .iter_raw_parse_order_from(next_invalidation.offset) + .flat_map(|c| { + if let Component::PseudoElement(ref pseudo) = *c { + return Some(pseudo); + } + + // TODO: Would be nice to make this a diagnostic_assert! of + // sorts. + debug_assert!( + c.maybe_allowed_after_pseudo_element(), + "Someone seriously messed up selector parsing: \ + {:?} at offset {:?}: {:?}", + next_invalidation.dependency, + next_invalidation.offset, + c, + ); + + None + }) + .next() + .unwrap(); + + // FIXME(emilio): This is not ideal, and could not be + // accurate if we ever have stateful element-backed eager + // pseudos. + // + // Ideally, we'd just remove element-backed eager pseudos + // altogether, given they work fine without it. Only gotcha + // is that we wouldn't style them in parallel, which may or + // may not be an issue. + // + // Also, this could be more fine grained now (perhaps a + // RESTYLE_PSEUDOS hint?). + // + // Note that we'll also restyle the pseudo-element because + // it would match this invalidation. + if self.processor.invalidates_on_eager_pseudo_element() { + if pseudo.is_eager() { + invalidated_self = true; + } + // If we start or stop matching some marker rules, and + // don't have a marker, then we need to restyle the + // element to potentially create one. + // + // Same caveats as for other eager pseudos apply, this + // could be more fine-grained. + if pseudo.is_marker() && self.element.marker_pseudo_element().is_none() { + invalidated_self = true; + } + + // FIXME: ::selection doesn't generate elements, so the + // regular invalidation doesn't work for it. We store + // the cached selection style holding off the originating + // element, so we need to restyle it in order to invalidate + // it. This is still not quite correct, since nothing + // triggers a repaint necessarily, but matches old Gecko + // behavior, and the ::selection implementation needs to + // change significantly anyway to implement + // https://github.com/w3c/csswg-drafts/issues/2474. + if pseudo.is_selection() { + invalidated_self = true; + } + } + } + + debug!( + " > Invalidation matched, next: {:?}, ({:?})", + next_invalidation, next_combinator + ); + + let next_invalidation_kind = next_invalidation.kind(); + + // We can skip pushing under some circumstances, and we should + // because otherwise the invalidation list could grow + // exponentially. + // + // * First of all, both invalidations need to be of the same + // kind. This is because of how we propagate them going to + // the right of the tree for sibling invalidations and going + // down the tree for children invalidations. A sibling + // invalidation that ends up generating a children + // invalidation ends up (correctly) in five different lists, + // not in the same list five different times. + // + // * Then, the invalidation needs to be matched by a previous + // ancestor/sibling, in order to know that this invalidation + // has been generated already. + // + // * Finally, the new invalidation needs to be + // `effective_for_next()`, in order for us to know that it is + // still in the list, since we remove the dependencies that + // aren't from the lists for our children / siblings. + // + // To go through an example, let's imagine we are processing a + // dom subtree like: + // + //
+ // + // And an invalidation list with a single invalidation like: + // + // [div div div] + // + // When we process the invalidation list for the outer div, we + // match it, and generate a `div div` invalidation, so for the + //
child we have: + // + // [div div div, div div] + // + // With the first of them marked as `matched`. + // + // When we process the
child, we don't match any of + // them, so both invalidations go untouched to our children. + // + // When we process the second
, we match _both_ + // invalidations. + // + // However, when matching the first, we can tell it's been + // matched, and not push the corresponding `div div` + // invalidation, since we know it's necessarily already on the + // list. + // + // Thus, without skipping the push, we'll arrive to the + // innermost
with: + // + // [div div div, div div, div div, div] + // + // While skipping it, we won't arrive here with duplicating + // dependencies: + // + // [div div div, div div, div] + // + let can_skip_pushing = next_invalidation_kind == invalidation_kind && + invalidation.matched_by_any_previous && + next_invalidation.effective_for_next(); + + if can_skip_pushing { + debug!( + " > Can avoid push, since the invalidation had \ + already been matched before" + ); + } else { + match next_invalidation_kind { + InvalidationKind::Descendant(DescendantInvalidationKind::Dom) => { + descendant_invalidations + .dom_descendants + .push(next_invalidation); + }, + InvalidationKind::Descendant(DescendantInvalidationKind::Part) => { + descendant_invalidations.parts.push(next_invalidation); + }, + InvalidationKind::Descendant(DescendantInvalidationKind::Slotted) => { + descendant_invalidations + .slotted_descendants + .push(next_invalidation); + }, + InvalidationKind::Sibling => { + sibling_invalidations.push(next_invalidation); + }, + } } SingleInvalidationResult { invalidated_self, - matched, + matched: true, } } } diff --git a/components/style/invalidation/element/state_and_attributes.rs b/components/style/invalidation/element/state_and_attributes.rs index e906f8d8405..79f1b53ada7 100644 --- a/components/style/invalidation/element/state_and_attributes.rs +++ b/components/style/invalidation/element/state_and_attributes.rs @@ -42,7 +42,6 @@ where descendant_invalidations: &'a mut DescendantInvalidationLists<'selectors>, sibling_invalidations: &'a mut InvalidationVector<'selectors>, invalidates_self: bool, - attr_selector_flags: InvalidationMapFlags, } /// An invalidation processor for style changes due to state and attribute @@ -79,6 +78,39 @@ impl<'a, 'b: 'a, E: TElement + 'b> StateAndAttrInvalidationProcessor<'a, 'b, E> } } +/// Checks a dependency against a given element and wrapper, to see if something +/// changed. +pub fn check_dependency( + dependency: &Dependency, + element: &E, + wrapper: &W, + mut context: &mut MatchingContext<'_, E::Impl>, +) -> bool +where + E: TElement, + W: selectors::Element, +{ + let matches_now = matches_selector( + &dependency.selector, + dependency.selector_offset, + None, + element, + &mut context, + &mut |_, _| {}, + ); + + let matched_then = matches_selector( + &dependency.selector, + dependency.selector_offset, + None, + wrapper, + &mut context, + &mut |_, _| {}, + ); + + matched_then != matches_now +} + /// Whether we should process the descendants of a given element for style /// invalidation. pub fn should_process_descendants(data: &ElementData) -> bool { @@ -133,6 +165,14 @@ where true } + fn check_outer_dependency(&mut self, dependency: &Dependency, element: E) -> bool { + // We cannot assert about `element` having a snapshot here (in fact it + // most likely won't), because it may be an arbitrary descendant or + // later-sibling of the element we started invalidating with. + let wrapper = ElementWrapper::new(element, &*self.shared_context.snapshot_map); + check_dependency(dependency, &element, &wrapper, &mut self.matching_context) + } + fn matching_context(&mut self) -> &mut MatchingContext<'a, E::Impl> { &mut self.matching_context } @@ -156,8 +196,6 @@ where return false; } - let mut attr_selector_flags = InvalidationMapFlags::empty(); - // If we the visited state changed, we force a restyle here. Matching // doesn't depend on the actual visited state at all, so we can't look // at matching results to decide what to do for this case. @@ -175,7 +213,6 @@ where let mut classes_removed = SmallVec::<[Atom; 8]>::new(); let mut classes_added = SmallVec::<[Atom; 8]>::new(); if snapshot.class_changed() { - attr_selector_flags.insert(InvalidationMapFlags::HAS_CLASS_ATTR_SELECTOR); // TODO(emilio): Do this more efficiently! snapshot.each_class(|c| { if !element.has_class(c, CaseSensitivity::CaseSensitive) { @@ -193,7 +230,6 @@ where let mut id_removed = None; let mut id_added = None; if snapshot.id_changed() { - attr_selector_flags.insert(InvalidationMapFlags::HAS_ID_ATTR_SELECTOR); let old_id = snapshot.id_attr(); let current_id = element.id(); @@ -204,10 +240,7 @@ where } if log_enabled!(::log::Level::Debug) { - debug!( - "Collecting changes for: {:?}, flags {:?}", - element, attr_selector_flags - ); + debug!("Collecting changes for: {:?}", element); if !state_changes.is_empty() { debug!(" > state: {:?}", state_changes); } @@ -220,7 +253,11 @@ where classes_added, classes_removed ); } - if snapshot.other_attr_changed() { + let mut attributes_changed = false; + snapshot.each_attr_changed(|_| { + attributes_changed = true; + }); + if attributes_changed { debug!( " > attributes changed, old: {}", snapshot.debug_list_attributes() @@ -255,7 +292,6 @@ where descendant_invalidations, sibling_invalidations, invalidates_self: false, - attr_selector_flags, }; let document_origins = if !matches_document_author_rules { @@ -365,12 +401,13 @@ where } } - let should_examine_attribute_selector_map = - self.snapshot.other_attr_changed() || map.flags.intersects(self.attr_selector_flags); - - if should_examine_attribute_selector_map { - self.collect_dependencies_in_map(&map.other_attribute_affecting_selectors) - } + self.snapshot.each_attr_changed(|attribute| { + if let Some(deps) = map.other_attribute_affecting_selectors.get(attribute) { + for dep in deps { + self.scan_dependency(dep); + } + } + }); let state_changes = self.state_changes; if !state_changes.is_empty() { @@ -378,19 +415,6 @@ where } } - fn collect_dependencies_in_map(&mut self, map: &'selectors SelectorMap) { - map.lookup_with_additional( - self.lookup_element, - self.matching_context.quirks_mode(), - self.removed_id, - self.classes_removed, - |dependency| { - self.scan_dependency(dependency); - true - }, - ); - } - fn collect_state_dependencies( &mut self, map: &'selectors SelectorMap, @@ -412,28 +436,14 @@ where } /// Check whether a dependency should be taken into account. + #[inline] fn check_dependency(&mut self, dependency: &Dependency) -> bool { - let element = &self.element; - let wrapper = &self.wrapper; - let matches_now = matches_selector( - &dependency.selector, - dependency.selector_offset, - None, - element, + check_dependency( + dependency, + &self.element, + &self.wrapper, &mut self.matching_context, - &mut |_, _| {}, - ); - - let matched_then = matches_selector( - &dependency.selector, - dependency.selector_offset, - None, - wrapper, - &mut self.matching_context, - &mut |_, _| {}, - ); - - matched_then != matches_now + ) } fn scan_dependency(&mut self, dependency: &'selectors Dependency) { @@ -456,18 +466,21 @@ where let invalidation_kind = dependency.invalidation_kind(); if matches!(invalidation_kind, DependencyInvalidationKind::Element) { - self.invalidates_self = true; + if let Some(ref parent) = dependency.parent { + // We know something changed in the inner selector, go outwards + // now. + self.scan_dependency(parent); + } else { + self.invalidates_self = true; + } return; } debug_assert_ne!(dependency.selector_offset, 0); debug_assert_ne!(dependency.selector_offset, dependency.selector.len()); - let invalidation = Invalidation::new( - &dependency.selector, - self.matching_context.current_host.clone(), - dependency.selector.len() - dependency.selector_offset + 1, - ); + let invalidation = + Invalidation::new(&dependency, self.matching_context.current_host.clone()); match invalidation_kind { DependencyInvalidationKind::Element => unreachable!(), diff --git a/components/style/lib.rs b/components/style/lib.rs index 7ec3ded3395..52b77a7507e 100644 --- a/components/style/lib.rs +++ b/components/style/lib.rs @@ -272,3 +272,26 @@ where ::is_zero(self) } } + +/// A trait pretty much similar to num_traits::One, but without the need of +/// implementing `Mul`. +pub trait One { + /// Reutrns the one value. + fn one() -> Self; + + /// Returns whether this value is one. + fn is_one(&self) -> bool; +} + +impl One for T +where + T: num_traits::One + PartialEq, +{ + fn one() -> Self { + ::one() + } + + fn is_one(&self) -> bool { + *self == One::one() + } +} diff --git a/components/style/matching.rs b/components/style/matching.rs index 677f024acfa..e8c6e55520f 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -7,7 +7,6 @@ #![allow(unsafe_code)] #![deny(missing_docs)] -use crate::animation::AnimationState; use crate::computed_value_flags::ComputedValueFlags; use crate::context::{ElementCascadeInputs, QuirksMode, SelectorFlagsMap}; use crate::context::{SharedStyleContext, StyleContext}; @@ -438,6 +437,8 @@ trait PrivateMatchMethods: TElement { _restyle_hint: RestyleHint, _important_rules_changed: bool, ) { + use crate::animation::AnimationState; + let this_opaque = self.as_node().opaque(); let shared_context = context.shared; let mut animation_states = shared_context.animation_states.write(); diff --git a/components/style/media_queries/media_feature.rs b/components/style/media_queries/media_feature.rs index a273e7f51be..cad695413c3 100644 --- a/components/style/media_queries/media_feature.rs +++ b/components/style/media_queries/media_feature.rs @@ -4,9 +4,10 @@ //! Media features. -use super::media_feature_expression::{AspectRatio, RangeOrOperator}; +use super::media_feature_expression::RangeOrOperator; use super::Device; use crate::parser::ParserContext; +use crate::values::computed::position::Ratio; use crate::values::computed::{CSSPixelLength, Resolution}; use crate::Atom; use cssparser::Parser; @@ -45,7 +46,7 @@ pub enum Evaluator { Float(MediaFeatureEvaluator), BoolInteger(MediaFeatureEvaluator), /// A non-negative number ratio, such as the one from device-pixel-ratio. - NumberRatio(MediaFeatureEvaluator), + NumberRatio(MediaFeatureEvaluator), /// A resolution. Resolution(MediaFeatureEvaluator), /// A keyword value. diff --git a/components/style/media_queries/media_feature_expression.rs b/components/style/media_queries/media_feature_expression.rs index 4704fab0ffc..9e2fdd2d47b 100644 --- a/components/style/media_queries/media_feature_expression.rs +++ b/components/style/media_queries/media_feature_expression.rs @@ -15,9 +15,8 @@ use crate::parser::{Parse, ParserContext}; #[cfg(feature = "servo")] use crate::servo::media_queries::MEDIA_FEATURES; use crate::str::{starts_with_ignore_ascii_case, string_as_ascii_lowercase}; +use crate::values::computed::position::Ratio; use crate::values::computed::{self, ToComputedValue}; -#[cfg(feature = "gecko")] -use crate::values::specified::NonNegativeNumber; use crate::values::specified::{Integer, Length, Number, Resolution}; use crate::values::{serialize_atom_identifier, CSSFloat}; use crate::{Atom, Zero}; @@ -26,30 +25,6 @@ use std::cmp::{Ordering, PartialOrd}; use std::fmt::{self, Write}; use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss}; -/// An aspect ratio, with a numerator and denominator. -#[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, ToShmem)] -pub struct AspectRatio(pub CSSFloat, pub CSSFloat); - -impl ToCss for AspectRatio { - fn to_css(&self, dest: &mut CssWriter) -> fmt::Result - where - W: fmt::Write, - { - self.0.to_css(dest)?; - dest.write_str(" / ")?; - self.1.to_css(dest) - } -} - -impl PartialOrd for AspectRatio { - fn partial_cmp(&self, other: &AspectRatio) -> Option { - f64::partial_cmp( - &(self.0 as f64 * other.1 as f64), - &(self.1 as f64 * other.0 as f64), - ) - } -} - /// The kind of matching that should be performed on a media feature value. #[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq, ToShmem)] pub enum Range { @@ -460,7 +435,7 @@ pub enum MediaExpressionValue { BoolInteger(bool), /// A single non-negative number or two non-negative numbers separated by '/', /// with optional whitespace on either side of the '/'. - NumberRatio(AspectRatio), + NumberRatio(Ratio), /// A resolution. Resolution(Resolution), /// An enumerated value, defined by the variant keyword table in the @@ -517,24 +492,14 @@ impl MediaExpressionValue { MediaExpressionValue::Float(number.get()) }, Evaluator::NumberRatio(..) => { - #[cfg(feature = "gecko")] - { - if static_prefs::pref!("layout.css.aspect-ratio-number.enabled") { - let a = NonNegativeNumber::parse(context, input)?.0.get(); - let b = match input.try_parse(|input| input.expect_delim('/')) { - Ok(()) => NonNegativeNumber::parse(context, input)?.0.get(), - _ => 1.0, - }; - return Ok(MediaExpressionValue::NumberRatio(AspectRatio(a, b))); - } - } + use crate::values::generics::position::Ratio as GenericRatio; + use crate::values::generics::NonNegative; + use crate::values::specified::position::Ratio; - let a = Integer::parse_positive(context, input)?; - input.expect_delim('/')?; - let b = Integer::parse_positive(context, input)?; - MediaExpressionValue::NumberRatio(AspectRatio( - a.value() as CSSFloat, - b.value() as CSSFloat, + let ratio = Ratio::parse(context, input)?; + MediaExpressionValue::NumberRatio(GenericRatio( + NonNegative(ratio.0.get()), + NonNegative(ratio.1.get()), )) }, Evaluator::Resolution(..) => { diff --git a/components/style/parser.rs b/components/style/parser.rs index c6f7321b25d..26e7cab1ad9 100644 --- a/components/style/parser.rs +++ b/components/style/parser.rs @@ -207,11 +207,20 @@ where } } -impl Parse for UnicodeRange { +impl Parse for crate::OwnedStr { fn parse<'i, 't>( - _context: &ParserContext, + _: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { - UnicodeRange::parse(input).map_err(|e| e.into()) + Ok(input.expect_string()?.as_ref().to_owned().into()) + } +} + +impl Parse for UnicodeRange { + fn parse<'i, 't>( + _: &ParserContext, + input: &mut Parser<'i, 't>, + ) -> Result> { + Ok(UnicodeRange::parse(input)?) } } diff --git a/components/style/properties/cascade.rs b/components/style/properties/cascade.rs index e9a34d6bf34..ab60099b3a4 100644 --- a/components/style/properties/cascade.rs +++ b/components/style/properties/cascade.rs @@ -10,8 +10,8 @@ use crate::dom::TElement; use crate::font_metrics::FontMetricsProvider; use crate::logical_geometry::WritingMode; use crate::media_queries::Device; -use crate::properties::{ComputedValues, StyleBuilder}; -use crate::properties::{LonghandId, LonghandIdSet, CSSWideKeyword}; +use crate::properties::{ComputedValues, StyleBuilder, Importance}; +use crate::properties::{LonghandId, LonghandIdSet, CSSWideKeyword, PropertyFlags}; use crate::properties::{PropertyDeclaration, PropertyDeclarationId, DeclarationImportanceIterator}; use crate::properties::{CASCADE_PROPERTY, ComputedValueFlags}; use crate::rule_cache::{RuleCache, RuleCacheConditions}; @@ -20,7 +20,6 @@ use crate::selector_parser::PseudoElement; use crate::stylesheets::{Origin, PerOrigin}; use servo_arc::Arc; use crate::shared_lock::StylesheetGuards; -use smallbitvec::SmallBitVec; use smallvec::SmallVec; use std::borrow::Cow; use std::cell::RefCell; @@ -108,6 +107,84 @@ where ) } +struct DeclarationIterator<'a> { + // Global to the iteration. + guards: &'a StylesheetGuards<'a>, + restriction: Option, + // The rule we're iterating over. + current_rule_node: Option<&'a StrongRuleNode>, + // Per rule state. + declarations: DeclarationImportanceIterator<'a>, + origin: Origin, + importance: Importance, +} + +impl<'a> DeclarationIterator<'a> { + #[inline] + fn new(rule_node: &'a StrongRuleNode, guards: &'a StylesheetGuards, pseudo: Option<&PseudoElement>) -> Self { + let restriction = pseudo.and_then(|p| p.property_restriction()); + let mut iter = Self { + guards, + current_rule_node: Some(rule_node), + origin: Origin::Author, + importance: Importance::Normal, + declarations: DeclarationImportanceIterator::default(), + restriction, + }; + iter.update_for_node(rule_node); + iter + } + + fn update_for_node(&mut self, node: &'a StrongRuleNode) { + let origin = node.cascade_level().origin(); + self.origin = origin; + self.importance = node.importance(); + let guard = match origin { + Origin::Author => self.guards.author, + Origin::User | Origin::UserAgent => self.guards.ua_or_user, + }; + self.declarations = match node.style_source() { + Some(source) => source.read(guard).declaration_importance_iter(), + None => DeclarationImportanceIterator::default(), + }; + } + +} + +impl<'a> Iterator for DeclarationIterator<'a> { + type Item = (&'a PropertyDeclaration, Origin); + + #[inline] + fn next(&mut self) -> Option { + loop { + if let Some((decl, importance)) = self.declarations.next_back() { + if self.importance != importance { + continue; + } + + let origin = self.origin; + if let Some(restriction) = self.restriction { + // decl.id() is either a longhand or a custom + // property. Custom properties are always allowed, but + // longhands are only allowed if they have our + // restriction flag set. + if let PropertyDeclarationId::Longhand(id) = decl.id() { + if !id.flags().contains(restriction) && origin != Origin::UserAgent { + continue; + } + } + } + + return Some((decl, origin)); + } + + let next_node = self.current_rule_node.take()?.parent()?; + self.current_rule_node = Some(next_node); + self.update_for_node(next_node); + } + } +} + fn cascade_rules( device: &Device, pseudo: Option<&PseudoElement>, @@ -130,54 +207,12 @@ where parent_style.is_some(), parent_style_ignoring_first_line.is_some() ); - let empty = SmallBitVec::new(); - let restriction = pseudo.and_then(|p| p.property_restriction()); - let iter_declarations = || { - rule_node.self_and_ancestors().flat_map(|node| { - let origin = node.cascade_level().origin(); - let node_importance = node.importance(); - let guard = match origin { - Origin::Author => guards.author, - Origin::User | Origin::UserAgent => guards.ua_or_user, - }; - let declarations = match node.style_source() { - Some(source) => source - .read(guard) - .declaration_importance_iter(), - None => DeclarationImportanceIterator::new(&[], &empty), - }; - - declarations - // Yield declarations later in source order (with more precedence) first. - .rev() - .filter_map(move |(declaration, declaration_importance)| { - if let Some(restriction) = restriction { - // declaration.id() is either a longhand or a custom - // property. Custom properties are always allowed, but - // longhands are only allowed if they have our - // restriction flag set. - if let PropertyDeclarationId::Longhand(id) = declaration.id() { - if !id.flags().contains(restriction) && origin != Origin::UserAgent { - return None; - } - } - } - - if declaration_importance == node_importance { - Some((declaration, origin)) - } else { - None - } - }) - }) - }; - apply_declarations( device, pseudo, rule_node, guards, - iter_declarations, + DeclarationIterator::new(rule_node, guards, pseudo), parent_style, parent_style_ignoring_first_line, layout_parent_style, @@ -208,12 +243,12 @@ pub enum CascadeMode<'a> { /// NOTE: This function expects the declaration with more priority to appear /// first. -pub fn apply_declarations<'a, E, F, I>( +pub fn apply_declarations<'a, E, I>( device: &Device, pseudo: Option<&PseudoElement>, rules: &StrongRuleNode, guards: &StylesheetGuards, - iter_declarations: F, + iter: I, parent_style: Option<&ComputedValues>, parent_style_ignoring_first_line: Option<&ComputedValues>, layout_parent_style: Option<&ComputedValues>, @@ -226,7 +261,6 @@ pub fn apply_declarations<'a, E, F, I>( ) -> Arc where E: TElement, - F: Fn() -> I, I: Iterator, { debug_assert!(layout_parent_style.is_none() || parent_style.is_some()); @@ -253,7 +287,7 @@ where device, ); - for (declaration, origin) in iter_declarations() { + for (declaration, origin) in iter { declarations.push((declaration, origin)); if let PropertyDeclaration::Custom(ref declaration) = *declaration { builder.cascade(declaration, origin); @@ -384,10 +418,15 @@ fn tweak_when_ignoring_colors( if color.0.is_transparent() { return; } - let color = builder.device.default_color(); - declarations_to_apply_unless_overriden.push( - PropertyDeclaration::Color(specified::ColorPropertyValue(color.into())) - ) + // If the inherited color would be transparent, but we would + // override this with a non-transparent color, then override it with + // the default color. Otherwise just let it inherit through. + if builder.get_parent_inherited_text().clone_color().alpha == 0 { + let color = builder.device.default_color(); + declarations_to_apply_unless_overriden.push( + PropertyDeclaration::Color(specified::ColorPropertyValue(color.into())) + ) + } }, // We honor url background-images if backplating. #[cfg(feature = "gecko")] @@ -881,8 +920,8 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> { let builder = &mut self.context.builder; - let parent_zoom = builder.get_parent_font().gecko().mAllowZoom; - let zoom = builder.get_font().gecko().mAllowZoom; + let parent_zoom = builder.get_parent_font().gecko().mAllowZoomAndMinSize; + let zoom = builder.get_font().gecko().mAllowZoomAndMinSize; if zoom == parent_zoom { return; } @@ -929,7 +968,7 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> { } let mut min = Au(parent_font.mScriptMinSize); - if font.mAllowZoom { + if font.mAllowZoomAndMinSize { min = builder.device.zoom_text(min); } diff --git a/components/style/properties/data.py b/components/style/properties/data.py index 472d1eb34dd..9ab3a40ca66 100644 --- a/components/style/properties/data.py +++ b/components/style/properties/data.py @@ -328,6 +328,7 @@ class Longhand(object): "AlignItems", "AlignSelf", "Appearance", + "AspectRatio", "BreakBetween", "BreakWithin", "BackgroundRepeat", @@ -356,6 +357,7 @@ class Longhand(object): "JustifyItems", "JustifySelf", "LineBreak", + "MasonryAutoFlow", "MozForceBrokenImageIcon", "MozListReversed", "MozScriptLevel", @@ -363,7 +365,6 @@ class Longhand(object): "MozScriptSizeMultiplier", "TextDecorationSkipInk", "NonNegativeNumber", - "Number", "OffsetRotate", "Opacity", "OutlineStyle", @@ -687,6 +688,7 @@ class PropertyRestrictions: def first_letter(data): props = set([ "color", + "opacity", "float", "initial-letter", @@ -721,6 +723,7 @@ class PropertyRestrictions: props = set([ # Per spec. "color", + "opacity", # Kinda like css-fonts? "-moz-osx-font-smoothing", diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index 53b483887ce..76b7ed864ea 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -110,9 +110,17 @@ pub struct DeclarationImportanceIterator<'a> { iter: Zip, smallbitvec::Iter<'a>>, } +impl<'a> Default for DeclarationImportanceIterator<'a> { + fn default() -> Self { + Self { + iter: [].iter().zip(smallbitvec::Iter::default()), + } + } +} + impl<'a> DeclarationImportanceIterator<'a> { /// Constructor. - pub fn new(declarations: &'a [PropertyDeclaration], important: &'a SmallBitVec) -> Self { + fn new(declarations: &'a [PropertyDeclaration], important: &'a SmallBitVec) -> Self { DeclarationImportanceIterator { iter: declarations.iter().zip(important.iter()), } diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index f3db987d8f9..69d0ac3fb92 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -802,7 +802,8 @@ fn static_assert() { <% skip_position_longhands = " ".join(x.ident for x in SIDES) %> <%self:impl_trait style_struct_name="Position" - skip_longhands="${skip_position_longhands}"> + skip_longhands="${skip_position_longhands} + masonry-auto-flow"> % for side in SIDES: <% impl_split_style_coord(side.ident, "mOffset", side.index) %> % endfor @@ -811,6 +812,7 @@ fn static_assert() { self.gecko.mJustifyItems.computed = v; } + ${impl_simple_type_with_conversion("masonry_auto_flow", "mMasonryAutoFlow")} <% skip_outline_longhands = " ".join("outline-style outline-width".split() + @@ -951,19 +953,19 @@ fn static_assert() { if let Some(info) = v.keyword_info { self.gecko.mFontSizeKeyword = match info.kw { - KeywordSize::XXSmall => structs::NS_STYLE_FONT_SIZE_XXSMALL, - KeywordSize::XSmall => structs::NS_STYLE_FONT_SIZE_XSMALL, - KeywordSize::Small => structs::NS_STYLE_FONT_SIZE_SMALL, - KeywordSize::Medium => structs::NS_STYLE_FONT_SIZE_MEDIUM, - KeywordSize::Large => structs::NS_STYLE_FONT_SIZE_LARGE, - KeywordSize::XLarge => structs::NS_STYLE_FONT_SIZE_XLARGE, - KeywordSize::XXLarge => structs::NS_STYLE_FONT_SIZE_XXLARGE, - KeywordSize::XXXLarge => structs::NS_STYLE_FONT_SIZE_XXXLARGE, - } as u8; + KeywordSize::XXSmall => structs::StyleFontSize::Xxsmall, + KeywordSize::XSmall => structs::StyleFontSize::Xsmall, + KeywordSize::Small => structs::StyleFontSize::Small, + KeywordSize::Medium => structs::StyleFontSize::Medium, + KeywordSize::Large => structs::StyleFontSize::Large, + KeywordSize::XLarge => structs::StyleFontSize::Xxlarge, + KeywordSize::XXLarge => structs::StyleFontSize::Xxlarge, + KeywordSize::XXXLarge => structs::StyleFontSize::Xxxlarge, + }; self.gecko.mFontSizeFactor = info.factor; self.gecko.mFontSizeOffset = info.offset.to_i32_au(); } else { - self.gecko.mFontSizeKeyword = structs::NS_STYLE_FONT_SIZE_NO_KEYWORD as u8; + self.gecko.mFontSizeKeyword = structs::StyleFontSize::NoKeyword; self.gecko.mFontSizeFactor = 1.; self.gecko.mFontSizeOffset = 0; } @@ -972,16 +974,16 @@ fn static_assert() { pub fn clone_font_size(&self) -> FontSize { use crate::values::specified::font::{KeywordInfo, KeywordSize}; let size = Au(self.gecko.mSize).into(); - let kw = match self.gecko.mFontSizeKeyword as u32 { - structs::NS_STYLE_FONT_SIZE_XXSMALL => KeywordSize::XXSmall, - structs::NS_STYLE_FONT_SIZE_XSMALL => KeywordSize::XSmall, - structs::NS_STYLE_FONT_SIZE_SMALL => KeywordSize::Small, - structs::NS_STYLE_FONT_SIZE_MEDIUM => KeywordSize::Medium, - structs::NS_STYLE_FONT_SIZE_LARGE => KeywordSize::Large, - structs::NS_STYLE_FONT_SIZE_XLARGE => KeywordSize::XLarge, - structs::NS_STYLE_FONT_SIZE_XXLARGE => KeywordSize::XXLarge, - structs::NS_STYLE_FONT_SIZE_XXXLARGE => KeywordSize::XXXLarge, - structs::NS_STYLE_FONT_SIZE_NO_KEYWORD => { + let kw = match self.gecko.mFontSizeKeyword { + structs::StyleFontSize::Xxsmall => KeywordSize::XXSmall, + structs::StyleFontSize::Xsmall => KeywordSize::XSmall, + structs::StyleFontSize::Small => KeywordSize::Small, + structs::StyleFontSize::Medium => KeywordSize::Medium, + structs::StyleFontSize::Large => KeywordSize::Large, + structs::StyleFontSize::Xlarge => KeywordSize::XLarge, + structs::StyleFontSize::Xxlarge => KeywordSize::XXLarge, + structs::StyleFontSize::Xxxlarge => KeywordSize::XXXLarge, + structs::StyleFontSize::NoKeyword => { return FontSize { size, keyword_info: None, @@ -1106,12 +1108,12 @@ fn static_assert() { #[allow(non_snake_case)] pub fn set__x_text_zoom(&mut self, v: longhands::_x_text_zoom::computed_value::T) { - self.gecko.mAllowZoom = v.0; + self.gecko.mAllowZoomAndMinSize = v.0; } #[allow(non_snake_case)] pub fn copy__x_text_zoom_from(&mut self, other: &Self) { - self.gecko.mAllowZoom = other.gecko.mAllowZoom; + self.gecko.mAllowZoomAndMinSize = other.gecko.mAllowZoomAndMinSize; } #[allow(non_snake_case)] @@ -1121,7 +1123,7 @@ fn static_assert() { #[allow(non_snake_case)] pub fn clone__x_text_zoom(&self) -> longhands::_x_text_zoom::computed_value::T { - longhands::_x_text_zoom::computed_value::T(self.gecko.mAllowZoom) + longhands::_x_text_zoom::computed_value::T(self.gecko.mAllowZoomAndMinSize) } <% impl_simple_type_with_conversion("font_language_override", "mFont.languageOverride") %> diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index 863da15d65c..f0c9c7cad78 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -68,8 +68,7 @@ pub type AnimationValueMap = FxHashMap; /// /// FIXME: We need to add a path for custom properties, but that's trivial after /// this (is a similar path to that of PropertyDeclaration). -#[cfg_attr(feature = "servo", derive(MallocSizeOf))] -#[derive(Debug)] +#[derive(Debug, MallocSizeOf)] #[repr(u16)] pub enum AnimationValue { % for prop in data.longhands: @@ -420,6 +419,7 @@ impl AnimationValue { /// /// SERVO ONLY: This doesn't properly handle things like updating 'em' units /// when animated font-size. + #[cfg(feature = "servo")] pub fn set_in_style_for_servo(&self, style: &mut ComputedValues) { match self { % for prop in data.longhands: @@ -439,6 +439,11 @@ impl AnimationValue { % endfor } } + + /// As above, but a stub for Gecko. + #[cfg(feature = "gecko")] + pub fn set_in_style_for_servo(&self, _: &mut ComputedValues) { + } } fn animate_discrete(this: &T, other: &T, procedure: Procedure) -> Result { diff --git a/components/style/properties/longhands/box.mako.rs b/components/style/properties/longhands/box.mako.rs index 4a9b85a0883..53aaf453d92 100644 --- a/components/style/properties/longhands/box.mako.rs +++ b/components/style/properties/longhands/box.mako.rs @@ -612,9 +612,7 @@ ${helpers.predefined_type( engines="gecko", animation_value_type="none", flags="CREATES_STACKING_CONTEXT FIXPOS_CB", - gecko_pref="layout.css.contain.enabled", spec="https://drafts.csswg.org/css-contain/#contain-property", - enabled_in="chrome", )} // Non-standard diff --git a/components/style/properties/longhands/position.mako.rs b/components/style/properties/longhands/position.mako.rs index d9403be5864..3af5f86561b 100644 --- a/components/style/properties/longhands/position.mako.rs +++ b/components/style/properties/longhands/position.mako.rs @@ -114,6 +114,17 @@ ${helpers.single_keyword( animation_value_type="discrete", servo_restyle_damage="reflow", )} + + ${helpers.predefined_type( + "justify-tracks", + "JustifyTracks", + "specified::JustifyTracks::default()", + engines="gecko", + gecko_pref="layout.css.grid-template-masonry-value.enabled", + animation_value_type="discrete", + servo_restyle_damage="reflow", + spec="https://github.com/w3c/csswg-drafts/issues/4650", + )} % endif % if engine in ["servo-2013", "servo-2020"]: @@ -151,6 +162,17 @@ ${helpers.single_keyword( servo_restyle_damage="reflow", )} + ${helpers.predefined_type( + "align-tracks", + "AlignTracks", + "specified::AlignTracks::default()", + engines="gecko", + gecko_pref="layout.css.grid-template-masonry-value.enabled", + animation_value_type="discrete", + servo_restyle_damage="reflow", + spec="https://github.com/w3c/csswg-drafts/issues/4650", + )} + ${helpers.predefined_type( "align-items", "AlignItems", @@ -372,6 +394,16 @@ ${helpers.predefined_type( % endfor +${helpers.predefined_type( + "masonry-auto-flow", + "MasonryAutoFlow", + "computed::MasonryAutoFlow::initial()", + engines="gecko", + gecko_pref="layout.css.grid-template-masonry-value.enabled", + animation_value_type="discrete", + spec="https://github.com/w3c/csswg-drafts/issues/4650", +)} + ${helpers.predefined_type( "grid-auto-flow", "GridAutoFlow", @@ -415,17 +447,13 @@ ${helpers.predefined_type( servo_restyle_damage="reflow", )} -// NOTE(emilio): Before exposing this property to content, we probably need to -// change syntax and such, and make it apply to more elements. -// -// For now, it's used only for mapped attributes. ${helpers.predefined_type( "aspect-ratio", - "Number", - "computed::Number::zero()", + "AspectRatio", + "computed::AspectRatio::auto()", engines="gecko servo-2013", - animation_value_type="ComputedValue", - spec="Internal, for now", - enabled_in="", + animation_value_type="discrete", + spec="https://drafts.csswg.org/css-sizing-4/#aspect-ratio", + gecko_pref="layout.css.aspect-ratio.enabled", servo_restyle_damage="reflow", )} diff --git a/components/style/selector_map.rs b/components/style/selector_map.rs index a53aad65fb3..6350caef0cf 100644 --- a/components/style/selector_map.rs +++ b/components/style/selector_map.rs @@ -278,8 +278,7 @@ impl SelectorMap { } impl SelectorMap { - /// Inserts into the correct hash, trying id, class, localname and - /// namespace. + /// Inserts an entry into the correct bucket(s). pub fn insert( &mut self, entry: T, @@ -287,46 +286,91 @@ impl SelectorMap { ) -> Result<(), FailedAllocationError> { self.count += 1; - let vector = match find_bucket(entry.selector()) { - Bucket::Root => &mut self.root, - Bucket::ID(id) => self - .id_hash - .try_entry(id.clone(), quirks_mode)? - .or_insert_with(SmallVec::new), - Bucket::Class(class) => self - .class_hash - .try_entry(class.clone(), quirks_mode)? - .or_insert_with(SmallVec::new), - Bucket::LocalName { name, lower_name } => { - // If the local name in the selector isn't lowercase, insert it - // into the rule hash twice. This means that, during lookup, we - // can always find the rules based on the local name of the - // element, regardless of whether it's an html element in an - // html document (in which case we match against lower_name) or - // not (in which case we match against name). - // - // In the case of a non-html-element-in-html-document with a - // lowercase localname and a non-lowercase selector, the - // rulehash lookup may produce superfluous selectors, but the - // subsequent selector matching work will filter them out. - if name != lower_name { - self.local_name_hash - .try_entry(lower_name.clone())? - .or_insert_with(SmallVec::new) - .try_push(entry.clone())?; + // NOTE(emilio): It'd be nice for this to be a separate function, but + // then the compiler can't reason about the lifetime dependency between + // `entry` and `bucket`, and would force us to clone the rule in the + // common path. + macro_rules! insert_into_bucket { + ($entry:ident, $bucket:expr) => {{ + match $bucket { + Bucket::Root => &mut self.root, + Bucket::ID(id) => self + .id_hash + .try_entry(id.clone(), quirks_mode)? + .or_insert_with(SmallVec::new), + Bucket::Class(class) => self + .class_hash + .try_entry(class.clone(), quirks_mode)? + .or_insert_with(SmallVec::new), + Bucket::LocalName { name, lower_name } => { + // If the local name in the selector isn't lowercase, + // insert it into the rule hash twice. This means that, + // during lookup, we can always find the rules based on + // the local name of the element, regardless of whether + // it's an html element in an html document (in which + // case we match against lower_name) or not (in which + // case we match against name). + // + // In the case of a non-html-element-in-html-document + // with a lowercase localname and a non-lowercase + // selector, the rulehash lookup may produce superfluous + // selectors, but the subsequent selector matching work + // will filter them out. + if name != lower_name { + self.local_name_hash + .try_entry(lower_name.clone())? + .or_insert_with(SmallVec::new) + .try_push($entry.clone())?; + } + self.local_name_hash + .try_entry(name.clone())? + .or_insert_with(SmallVec::new) + }, + Bucket::Namespace(url) => self + .namespace_hash + .try_entry(url.clone())? + .or_insert_with(SmallVec::new), + Bucket::Universal => &mut self.other, } - self.local_name_hash - .try_entry(name.clone())? - .or_insert_with(SmallVec::new) - }, - Bucket::Namespace(url) => self - .namespace_hash - .try_entry(url.clone())? - .or_insert_with(SmallVec::new), - Bucket::Universal => &mut self.other, + .try_push($entry)?; + }}; + } + + let bucket = { + let mut disjoint_buckets = SmallVec::new(); + let bucket = find_bucket(entry.selector(), &mut disjoint_buckets); + + // See if inserting this selector in multiple entries in the + // selector map would be worth it. Consider a case like: + // + // .foo:where(div, #bar) + // + // There, `bucket` would be `Class(foo)`, and disjoint_buckets would + // be `[LocalName { div }, ID(bar)]`. + // + // Here we choose to insert the selector in the `.foo` bucket in + // such a case, as it's likely more worth it than inserting it in + // both `div` and `#bar`. + // + // This is specially true if there's any universal selector in the + // `disjoint_selectors` set, at which point we'd just be doing + // wasted work. + if !disjoint_buckets.is_empty() && + disjoint_buckets + .iter() + .all(|b| b.more_specific_than(&bucket)) + { + for bucket in &disjoint_buckets { + let entry = entry.clone(); + insert_into_bucket!(entry, *bucket); + } + return Ok(()); + } + bucket }; - vector.try_push(entry) + insert_into_bucket!(entry, bucket); + Ok(()) } /// Looks up entries by id, class, local name, namespace, and other (in @@ -457,18 +501,43 @@ impl SelectorMap { } enum Bucket<'a> { - Root, - ID(&'a Atom), - Class(&'a Atom), + Universal, + Namespace(&'a Namespace), LocalName { name: &'a LocalName, lower_name: &'a LocalName, }, - Namespace(&'a Namespace), - Universal, + Class(&'a Atom), + ID(&'a Atom), + Root, } -fn specific_bucket_for<'a>(component: &'a Component) -> Bucket<'a> { +impl<'a> Bucket<'a> { + /// root > id > class > local name > namespace > universal. + #[inline] + fn specificity(&self) -> usize { + match *self { + Bucket::Universal => 0, + Bucket::Namespace(..) => 1, + Bucket::LocalName { .. } => 2, + Bucket::Class(..) => 3, + Bucket::ID(..) => 4, + Bucket::Root => 5, + } + } + + #[inline] + fn more_specific_than(&self, other: &Self) -> bool { + self.specificity() > other.specificity() + } +} + +type DisjointBuckets<'a> = SmallVec<[Bucket<'a>; 5]>; + +fn specific_bucket_for<'a>( + component: &'a Component, + disjoint_buckets: &mut DisjointBuckets<'a>, +) -> Bucket<'a> { match *component { Component::Root => Bucket::Root, Component::ID(ref id) => Bucket::ID(id), @@ -498,14 +567,16 @@ fn specific_bucket_for<'a>(component: &'a Component) -> Bucket<'a> // // So inserting `span` in the rule hash makes sense since we want to // match the slotted . - Component::Slotted(ref selector) => find_bucket(selector.iter()), - Component::Host(Some(ref selector)) => find_bucket(selector.iter()), + Component::Slotted(ref selector) => find_bucket(selector.iter(), disjoint_buckets), + Component::Host(Some(ref selector)) => find_bucket(selector.iter(), disjoint_buckets), Component::Is(ref list) | Component::Where(ref list) => { if list.len() == 1 { - find_bucket(list[0].iter()) + find_bucket(list[0].iter(), disjoint_buckets) } else { - // TODO(emilio): We should handle the multiple element case by - // inserting multiple entries in the map. + for selector in &**list { + let bucket = find_bucket(selector.iter(), disjoint_buckets); + disjoint_buckets.push(bucket); + } Bucket::Universal } }, @@ -515,39 +586,21 @@ fn specific_bucket_for<'a>(component: &'a Component) -> Bucket<'a> /// Searches a compound selector from left to right, and returns the appropriate /// bucket for it. +/// +/// It also populates disjoint_buckets with dependencies from nested selectors +/// with any semantics like :is() and :where(). #[inline(always)] -fn find_bucket<'a>(mut iter: SelectorIter<'a, SelectorImpl>) -> Bucket<'a> { +fn find_bucket<'a>( + mut iter: SelectorIter<'a, SelectorImpl>, + disjoint_buckets: &mut DisjointBuckets<'a>, +) -> Bucket<'a> { let mut current_bucket = Bucket::Universal; loop { - // We basically want to find the most specific bucket, - // where: - // - // root > id > class > local name > namespace > universal. - // for ss in &mut iter { - let new_bucket = specific_bucket_for(ss); - match new_bucket { - Bucket::Root => return new_bucket, - Bucket::ID(..) => { - current_bucket = new_bucket; - }, - Bucket::Class(..) => { - if !matches!(current_bucket, Bucket::ID(..)) { - current_bucket = new_bucket; - } - }, - Bucket::LocalName { .. } => { - if matches!(current_bucket, Bucket::Universal | Bucket::Namespace(..)) { - current_bucket = new_bucket; - } - }, - Bucket::Namespace(..) => { - if matches!(current_bucket, Bucket::Universal) { - current_bucket = new_bucket; - } - }, - Bucket::Universal => {}, + let new_bucket = specific_bucket_for(ss, disjoint_buckets); + if new_bucket.more_specific_than(¤t_bucket) { + current_bucket = new_bucket; } } @@ -558,7 +611,7 @@ fn find_bucket<'a>(mut iter: SelectorIter<'a, SelectorImpl>) -> Bucket<'a> { } } - return current_bucket; + current_bucket } /// Wrapper for PrecomputedHashMap that does ASCII-case-insensitive lookup in quirks mode. diff --git a/components/style/servo/selector_parser.rs b/components/style/servo/selector_parser.rs index 7f8ac65089d..af4dd92fcf7 100644 --- a/components/style/servo/selector_parser.rs +++ b/components/style/servo/selector_parser.rs @@ -391,12 +391,6 @@ impl NonTSPseudoClass { pub fn needs_cache_revalidation(&self) -> bool { self.state_flag().is_empty() } - - /// Returns true if the evaluation of the pseudo-class depends on the - /// element's attributes. - pub fn is_attr_based(&self) -> bool { - matches!(*self, NonTSPseudoClass::Lang(..)) - } } /// The abstract struct we implement the selector parser implementation on top @@ -621,15 +615,14 @@ impl DerefMut for SnapshotMap { } /// Servo's version of an element snapshot. -#[derive(Debug)] -#[cfg_attr(feature = "servo", derive(MallocSizeOf))] +#[derive(Debug, Default, MallocSizeOf)] pub struct ServoElementSnapshot { /// The stored state of the element. pub state: Option, /// The set of stored attributes and its values. pub attrs: Option>, - /// Whether this element is an HTML element in an HTML document. - pub is_html_element_in_html_document: bool, + /// The set of changed attributes and its values. + pub changed_attrs: Vec, /// Whether the class attribute changed or not. pub class_changed: bool, /// Whether the id attribute changed or not. @@ -640,15 +633,8 @@ pub struct ServoElementSnapshot { impl ServoElementSnapshot { /// Create an empty element snapshot. - pub fn new(is_html_element_in_html_document: bool) -> Self { - ServoElementSnapshot { - state: None, - attrs: None, - is_html_element_in_html_document: is_html_element_in_html_document, - class_changed: false, - id_changed: false, - other_attributes_changed: false, - } + pub fn new() -> Self { + Self::default() } /// Returns whether the id attribute changed or not. @@ -675,6 +661,17 @@ impl ServoElementSnapshot { .map(|&(_, ref v)| v) } + /// Executes the callback once for each attribute that changed. + #[inline] + pub fn each_attr_changed(&self, mut callback: F) + where + F: FnMut(&LocalName), + { + for name in &self.changed_attrs { + callback(name) + } + } + fn any_attr_ignore_ns(&self, name: &LocalName, mut f: F) -> bool where F: FnMut(&AttrValue) -> bool, diff --git a/components/style/shared_lock.rs b/components/style/shared_lock.rs index d46370952d3..d151062199f 100644 --- a/components/style/shared_lock.rs +++ b/components/style/shared_lock.rs @@ -15,7 +15,6 @@ use std::cell::UnsafeCell; use std::fmt; #[cfg(feature = "servo")] use std::mem; -use std::mem::ManuallyDrop; #[cfg(feature = "gecko")] use std::ptr; use to_shmem::{SharedMemoryBuilder, ToShmem}; @@ -258,20 +257,20 @@ impl Locked { #[cfg(feature = "gecko")] impl ToShmem for Locked { - fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop { + fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> to_shmem::Result { let guard = self.shared_lock.read(); - ManuallyDrop::new(Locked { + Ok(ManuallyDrop::new(Locked { shared_lock: SharedRwLock::read_only(), data: UnsafeCell::new(ManuallyDrop::into_inner( - self.read_with(&guard).to_shmem(builder), + self.read_with(&guard).to_shmem(builder)?, )), - }) + })) } } #[cfg(feature = "servo")] impl ToShmem for Locked { - fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> ManuallyDrop { + fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> to_shmem::Result { panic!("ToShmem not supported in Servo currently") } } diff --git a/components/style/stylesheets/document_rule.rs b/components/style/stylesheets/document_rule.rs index e779863f031..35c6b443fe2 100644 --- a/components/style/stylesheets/document_rule.rs +++ b/components/style/stylesheets/document_rule.rs @@ -264,10 +264,6 @@ impl DocumentCondition { return true; } - if !pref!("layout.css.moz-document.url-prefix-hack.enabled") { - return false; - } - // Allow a single url-prefix() for compatibility. // // See bug 1446470 and dependencies. diff --git a/components/style/stylesheets/import_rule.rs b/components/style/stylesheets/import_rule.rs index 982572673a9..5efae0a3ee5 100644 --- a/components/style/stylesheets/import_rule.rs +++ b/components/style/stylesheets/import_rule.rs @@ -15,9 +15,8 @@ use crate::stylesheets::{CssRule, Origin, StylesheetInDocument}; use crate::values::CssUrl; use cssparser::SourceLocation; use std::fmt::{self, Write}; -use std::mem::ManuallyDrop; use style_traits::{CssWriter, ToCss}; -use to_shmem::{SharedMemoryBuilder, ToShmem}; +use to_shmem::{self, SharedMemoryBuilder, ToShmem}; /// With asynchronous stylesheet parsing, we can't synchronously create a /// GeckoStyleSheet. So we use this placeholder instead. @@ -184,8 +183,10 @@ pub struct ImportRule { } impl ToShmem for ImportRule { - fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> ManuallyDrop { - panic!("ToShmem failed for ImportRule: cannot handle imported style sheets") + fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> to_shmem::Result { + Err(String::from( + "ToShmem failed for ImportRule: cannot handle imported style sheets", + )) } } diff --git a/components/style/stylesheets/mod.rs b/components/style/stylesheets/mod.rs index daf97f20e3c..12ba46b3e06 100644 --- a/components/style/stylesheets/mod.rs +++ b/components/style/stylesheets/mod.rs @@ -40,7 +40,7 @@ use std::fmt; use std::mem::{self, ManuallyDrop}; use style_traits::ParsingMode; #[cfg(feature = "gecko")] -use to_shmem::{SharedMemoryBuilder, ToShmem}; +use to_shmem::{self, SharedMemoryBuilder, ToShmem}; pub use self::counter_style_rule::CounterStyleRule; pub use self::document_rule::DocumentRule; @@ -117,20 +117,25 @@ impl Drop for UrlExtraData { #[cfg(feature = "gecko")] impl ToShmem for UrlExtraData { - fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> ManuallyDrop { + fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> to_shmem::Result { if self.0 & 1 == 0 { let shared_extra_datas = unsafe { &structs::URLExtraData_sShared }; let self_ptr = self.as_ref() as *const _ as *mut _; let sheet_id = shared_extra_datas .iter() - .position(|r| r.mRawPtr == self_ptr) - .expect( - "ToShmem failed for UrlExtraData: expected sheet's URLExtraData to be in \ - URLExtraData::sShared", - ); - ManuallyDrop::new(UrlExtraData((sheet_id << 1) | 1)) + .position(|r| r.mRawPtr == self_ptr); + let sheet_id = match sheet_id { + Some(id) => id, + None => { + return Err(String::from( + "ToShmem failed for UrlExtraData: expected sheet's URLExtraData to be in \ + URLExtraData::sShared", + )); + }, + }; + Ok(ManuallyDrop::new(UrlExtraData((sheet_id << 1) | 1))) } else { - ManuallyDrop::new(UrlExtraData(self.0)) + Ok(ManuallyDrop::new(UrlExtraData(self.0))) } } } diff --git a/components/style/stylist.rs b/components/style/stylist.rs index e82128f22f7..bb6aec6c52d 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -1318,12 +1318,6 @@ impl Stylist { use crate::font_metrics::get_metrics_provider_for_product; let block = declarations.read_with(guards.author); - let iter_declarations = || { - block - .declaration_importance_iter() - .map(|(declaration, _)| (declaration, Origin::Author)) - }; - let metrics = get_metrics_provider_for_product(); // We don't bother inserting these declarations in the rule tree, since @@ -1332,12 +1326,14 @@ impl Stylist { // TODO(emilio): Now that we fixed bug 1493420, we should consider // reversing this as it shouldn't be slow anymore, and should avoid // generating two instantiations of apply_declarations. - properties::apply_declarations::( + properties::apply_declarations::( &self.device, /* pseudo = */ None, self.rule_tree.root(), guards, - iter_declarations, + block + .declaration_importance_iter() + .map(|(declaration, _)| (declaration, Origin::Author)), Some(parent_style), Some(parent_style), Some(parent_style), diff --git a/components/style/values/animated/color.rs b/components/style/values/animated/color.rs index 14b1d0c539c..9f4fa5c52b5 100644 --- a/components/style/values/animated/color.rs +++ b/components/style/values/animated/color.rs @@ -12,8 +12,7 @@ use crate::values::generics::color::{Color as GenericColor, ComplexColorRatios}; /// /// Unlike in computed values, each component value may exceed the /// range `[0.0, 1.0]`. -#[cfg_attr(feature = "servo", derive(MallocSizeOf))] -#[derive(Clone, Copy, Debug, PartialEq, ToAnimatedZero)] +#[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, ToAnimatedZero)] pub struct RGBA { /// The red component. pub red: f32, diff --git a/components/style/values/computed/align.rs b/components/style/values/computed/align.rs index 45d6cfac323..94847fd80f0 100644 --- a/components/style/values/computed/align.rs +++ b/components/style/values/computed/align.rs @@ -10,7 +10,8 @@ use crate::values::computed::{Context, ToComputedValue}; use crate::values::specified; pub use super::specified::{ - AlignContent, AlignItems, ContentDistribution, JustifyContent, SelfAlignment, + AlignContent, AlignItems, AlignTracks, ContentDistribution, JustifyContent, JustifyTracks, + SelfAlignment, }; pub use super::specified::{AlignSelf, JustifySelf}; @@ -35,7 +36,7 @@ pub use super::specified::{AlignSelf, JustifySelf}; /// sucks :(. /// /// See the discussion in https://bugzil.la/1384542. -#[derive(Clone, Copy, Debug, Eq, PartialEq, ToCss, ToResolvedValue)] +#[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq, ToCss, ToResolvedValue)] #[repr(C)] pub struct ComputedJustifyItems { /// The specified value for the property. Can contain the bare `legacy` diff --git a/components/style/values/computed/font.rs b/components/style/values/computed/font.rs index 0f821123c20..ed00f42d678 100644 --- a/components/style/values/computed/font.rs +++ b/components/style/values/computed/font.rs @@ -32,7 +32,7 @@ use std::mem::{self, ManuallyDrop}; use std::slice; use style_traits::{CssWriter, ParseError, ToCss}; #[cfg(feature = "gecko")] -use to_shmem::{SharedMemoryBuilder, ToShmem}; +use to_shmem::{self, SharedMemoryBuilder, ToShmem}; pub use crate::values::computed::Length as MozScriptMinSize; pub use crate::values::specified::font::{FontSynthesis, MozScriptSizeMultiplier}; @@ -466,19 +466,20 @@ pub enum FontFamilyList { #[cfg(feature = "gecko")] impl ToShmem for FontFamilyList { - fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> ManuallyDrop { + fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> to_shmem::Result { // In practice, the only SharedFontList objects we create from shared // style sheets are ones with a single generic entry. - ManuallyDrop::new(match *self { + Ok(ManuallyDrop::new(match *self { FontFamilyList::SharedFontList(ref r) => { - assert!( - r.mNames.len() == 1 && r.mNames[0].mName.mRawPtr.is_null(), - "ToShmem failed for FontFamilyList: cannot handle non-generic families", - ); + if !(r.mNames.len() == 1 && r.mNames[0].mName.mRawPtr.is_null()) { + return Err(String::from( + "ToShmem failed for FontFamilyList: cannot handle non-generic families", + )); + } FontFamilyList::Generic(r.mNames[0].mGeneric) }, FontFamilyList::Generic(t) => FontFamilyList::Generic(t), - }) + })) } } diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index 841cc3df22a..547c1618f70 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -21,7 +21,7 @@ use crate::media_queries::Device; use crate::properties; use crate::properties::{ComputedValues, LonghandId, StyleBuilder}; use crate::rule_cache::RuleCacheConditions; -use crate::{ArcSlice, Atom}; +use crate::{ArcSlice, Atom, One}; use euclid::default::Size2D; use servo_arc::Arc; use std::cell::RefCell; @@ -29,7 +29,10 @@ use std::cmp; use std::f32; #[cfg(feature = "gecko")] -pub use self::align::{AlignContent, AlignItems, JustifyContent, JustifyItems, SelfAlignment}; +pub use self::align::{ + AlignContent, AlignItems, AlignTracks, JustifyContent, JustifyItems, JustifyTracks, + SelfAlignment, +}; #[cfg(feature = "gecko")] pub use self::align::{AlignSelf, JustifySelf}; pub use self::angle::Angle; @@ -68,7 +71,10 @@ pub use self::list::Quotes; pub use self::motion::{OffsetPath, OffsetRotate}; pub use self::outline::OutlineStyle; pub use self::percentage::{NonNegativePercentage, Percentage}; -pub use self::position::{GridAutoFlow, GridTemplateAreas, Position, PositionOrAuto, ZIndex}; +pub use self::position::AspectRatio; +pub use self::position::{ + GridAutoFlow, GridTemplateAreas, MasonryAutoFlow, Position, PositionOrAuto, ZIndex, +}; pub use self::rect::NonNegativeLengthOrNumberRect; pub use self::resolution::Resolution; pub use self::svg::MozContextProperties; @@ -223,8 +229,8 @@ impl<'a> Context<'a> { pub fn maybe_zoom_text(&self, size: CSSPixelLength) -> CSSPixelLength { // We disable zoom for by unsetting the // -x-text-zoom property, which leads to a false value - // in mAllowZoom - if self.style().get_font().gecko.mAllowZoom { + // in mAllowZoomAndMinSize + if self.style().get_font().gecko.mAllowZoomAndMinSize { self.device().zoom_text(Au::from(size)).into() } else { size @@ -600,6 +606,18 @@ impl From for CSSFloat { } } +impl One for NonNegativeNumber { + #[inline] + fn one() -> Self { + NonNegative(1.0) + } + + #[inline] + fn is_one(&self) -> bool { + self.0 == 1.0 + } +} + /// A wrapper of Number, but the value between 0 and 1 pub type ZeroToOneNumber = ZeroToOne; diff --git a/components/style/values/computed/position.rs b/components/style/values/computed/position.rs index 3eff231de88..9e5fe3be1d2 100644 --- a/components/style/values/computed/position.rs +++ b/components/style/values/computed/position.rs @@ -7,13 +7,16 @@ //! //! [position]: https://drafts.csswg.org/css-backgrounds-3/#position -use crate::values::computed::{Integer, LengthPercentage, Percentage}; +use crate::values::computed::{Integer, LengthPercentage, NonNegativeNumber, Percentage}; +use crate::values::generics::position::AspectRatio as GenericAspectRatio; use crate::values::generics::position::Position as GenericPosition; use crate::values::generics::position::PositionComponent as GenericPositionComponent; use crate::values::generics::position::PositionOrAuto as GenericPositionOrAuto; +use crate::values::generics::position::Ratio as GenericRatio; use crate::values::generics::position::ZIndex as GenericZIndex; -pub use crate::values::specified::position::{GridAutoFlow, GridTemplateAreas}; +pub use crate::values::specified::position::{GridAutoFlow, GridTemplateAreas, MasonryAutoFlow}; use crate::Zero; +use std::cmp::{Ordering, PartialOrd}; use std::fmt::{self, Write}; use style_traits::{CssWriter, ToCss}; @@ -68,3 +71,25 @@ impl GenericPositionComponent for LengthPercentage { /// A computed value for the `z-index` property. pub type ZIndex = GenericZIndex; + +/// A computed value. +pub type Ratio = GenericRatio; + +impl PartialOrd for Ratio { + fn partial_cmp(&self, other: &Self) -> Option { + f64::partial_cmp( + &((self.0).0 as f64 * (other.1).0 as f64), + &((self.1).0 as f64 * (other.0).0 as f64), + ) + } +} + +impl Ratio { + /// Returns a new Ratio. + pub fn new(a: f32, b: f32) -> Self { + GenericRatio(a.into(), b.into()) + } +} + +/// A computed value for the `aspect-ratio` property. +pub type AspectRatio = GenericAspectRatio; diff --git a/components/style/values/generics/basic_shape.rs b/components/style/values/generics/basic_shape.rs index 745d6e07bbf..60619b46e2c 100644 --- a/components/style/values/generics/basic_shape.rs +++ b/components/style/values/generics/basic_shape.rs @@ -270,6 +270,7 @@ pub struct Ellipse { Copy, Debug, MallocSizeOf, + Parse, PartialEq, SpecifiedValueInfo, ToAnimatedValue, diff --git a/components/style/values/generics/calc.rs b/components/style/values/generics/calc.rs index dd77fd28552..cd331818386 100644 --- a/components/style/values/generics/calc.rs +++ b/components/style/values/generics/calc.rs @@ -337,7 +337,7 @@ impl CalcNode { ($slot:expr) => {{ let dummy = Self::MinMax(Default::default(), MinMaxOp::Max); let result = mem::replace($slot, dummy); - let _ = mem::replace(self, result); + *self = result; }}; } match *self { @@ -464,7 +464,7 @@ impl CalcNode { replace_self_with!(&mut children[0]); } else { // Else put our simplified children back. - let _ = mem::replace(children_slot, children.into_boxed_slice().into()); + *children_slot = children.into_boxed_slice().into(); } }, Self::Leaf(ref mut l) => { diff --git a/components/style/values/generics/flex.rs b/components/style/values/generics/flex.rs index adff13b5b42..85b64000f2b 100644 --- a/components/style/values/generics/flex.rs +++ b/components/style/values/generics/flex.rs @@ -5,13 +5,13 @@ //! Generic types for CSS values related to flexbox. /// A generic value for the `flex-basis` property. -#[cfg_attr(feature = "servo", derive(MallocSizeOf))] #[derive( Animate, Clone, ComputeSquaredDistance, Copy, Debug, + MallocSizeOf, Parse, PartialEq, SpecifiedValueInfo, diff --git a/components/style/values/generics/font.rs b/components/style/values/generics/font.rs index 2b29104ff81..237e862d4aa 100644 --- a/components/style/values/generics/font.rs +++ b/components/style/values/generics/font.rs @@ -5,9 +5,9 @@ //! Generic types for font stuff. use crate::parser::{Parse, ParserContext}; +use crate::One; use byteorder::{BigEndian, ReadBytesExt}; use cssparser::Parser; -use num_traits::One; use std::fmt::{self, Write}; use std::io::Cursor; use style_traits::{CssWriter, ParseError}; @@ -42,7 +42,7 @@ where { self.tag.to_css(dest)?; // Don't serialize the default value. - if self.value != Integer::one() { + if !self.value.is_one() { dest.write_char(' ')?; self.value.to_css(dest)?; } diff --git a/components/style/values/generics/grid.rs b/components/style/values/generics/grid.rs index 6f0f155912d..a0970a6fbc0 100644 --- a/components/style/values/generics/grid.rs +++ b/components/style/values/generics/grid.rs @@ -714,7 +714,7 @@ impl Parse for LineNameList { line_names.truncate(MAX_GRID_LINE as usize); } - let (fill_start, fill_len) = fill_data.unwrap_or((usize::MAX, 0)); + let (fill_start, fill_len) = fill_data.unwrap_or((0, 0)); Ok(LineNameList { names: line_names.into(), @@ -733,7 +733,7 @@ impl ToCss for LineNameList { let fill_start = self.fill_start; let fill_len = self.fill_len; for (i, names) in self.names.iter().enumerate() { - if i == fill_start { + if fill_len > 0 && i == fill_start { dest.write_str(" repeat(auto-fill,")?; } @@ -748,7 +748,7 @@ impl ToCss for LineNameList { } dest.write_str("]")?; - if i == fill_start + fill_len - 1 { + if fill_len > 0 && i == fill_start + fill_len - 1 { dest.write_str(")")?; } } @@ -786,6 +786,9 @@ pub enum GenericGridTemplateComponent { /// TODO: Support animations for this after subgrid is addressed in [grid-2] spec. #[animation(error)] Subgrid(Box), + /// `masonry` value. + /// https://github.com/w3c/csswg-drafts/issues/4650 + Masonry, } pub use self::GenericGridTemplateComponent as GridTemplateComponent; diff --git a/components/style/values/generics/length.rs b/components/style/values/generics/length.rs index 4183f40a942..bed574861cb 100644 --- a/components/style/values/generics/length.rs +++ b/components/style/values/generics/length.rs @@ -174,13 +174,13 @@ impl Size { /// A generic value for the `max-width` or `max-height` property. #[allow(missing_docs)] -#[cfg_attr(feature = "servo", derive(MallocSizeOf))] #[derive( Animate, Clone, ComputeSquaredDistance, Copy, Debug, + MallocSizeOf, PartialEq, SpecifiedValueInfo, ToAnimatedValue, diff --git a/components/style/values/generics/position.rs b/components/style/values/generics/position.rs index 00a9a219df4..2b4448f0335 100644 --- a/components/style/values/generics/position.rs +++ b/components/style/values/generics/position.rs @@ -5,6 +5,10 @@ //! Generic types for CSS handling of specified and computed values of //! [`position`](https://drafts.csswg.org/css-backgrounds-3/#position) +use crate::One; +use std::fmt::{self, Write}; +use style_traits::{CssWriter, ToCss}; + /// A generic type for representing a CSS [position](https://drafts.csswg.org/css-values/#position). #[derive( Animate, @@ -150,3 +154,104 @@ impl ZIndex { } } } + +/// A generic value for the `` value. +#[derive( + Animate, + Clone, + ComputeSquaredDistance, + Copy, + Debug, + MallocSizeOf, + PartialEq, + SpecifiedValueInfo, + ToAnimatedZero, + ToComputedValue, + ToResolvedValue, + ToShmem, +)] +#[repr(C)] +pub struct Ratio(pub N, pub N); + +impl ToCss for Ratio +where + N: ToCss + One + std::cmp::PartialEq, +{ + fn to_css(&self, dest: &mut CssWriter) -> fmt::Result + where + W: Write, + { + self.0.to_css(dest)?; + // The second defaults to 1. So if it is one, we omit it in serialization. + if !self.1.is_one() { + dest.write_str(" / ")?; + self.1.to_css(dest)?; + } + Ok(()) + } +} + +/// Ratio or None. +#[derive( + Animate, + Clone, + ComputeSquaredDistance, + Copy, + Debug, + MallocSizeOf, + PartialEq, + SpecifiedValueInfo, + ToAnimatedZero, + ToComputedValue, + ToCss, + ToResolvedValue, + ToShmem, +)] +#[repr(C, u8)] +pub enum PreferredRatio { + /// Without specified ratio + #[css(skip)] + None, + /// With specified ratio + Ratio(#[css(field_bound)] Ratio), +} + +/// A generic value for the `aspect-ratio` property, the value is `auto || `. +#[derive( + Animate, + Clone, + ComputeSquaredDistance, + Copy, + Debug, + MallocSizeOf, + PartialEq, + SpecifiedValueInfo, + ToAnimatedZero, + ToComputedValue, + ToCss, + ToResolvedValue, + ToShmem, +)] +#[repr(C)] +pub struct GenericAspectRatio { + /// Specifiy auto or not. + #[animation(constant)] + #[css(represents_keyword)] + pub auto: bool, + /// The preferred aspect-ratio value. + #[css(field_bound)] + pub ratio: PreferredRatio, +} + +pub use self::GenericAspectRatio as AspectRatio; + +impl AspectRatio { + /// Returns `auto` + #[inline] + pub fn auto() -> Self { + AspectRatio { + auto: true, + ratio: PreferredRatio::None, + } + } +} diff --git a/components/style/values/specified/align.rs b/components/style/values/specified/align.rs index 10f7f3efbfc..51b5e625672 100644 --- a/components/style/values/specified/align.rs +++ b/components/style/values/specified/align.rs @@ -171,16 +171,6 @@ impl ContentDistribution { Self { primary } } - fn from_bits(bits: u16) -> Self { - Self { - primary: AlignFlags::from_bits_truncate(bits as u8), - } - } - - fn as_bits(&self) -> u16 { - self.primary.bits() as u16 - } - /// Returns whether this value is a . pub fn is_baseline_position(&self) -> bool { matches!( @@ -292,6 +282,36 @@ impl SpecifiedValueInfo for AlignContent { } } +/// Value for the `align-tracks` property. +/// +/// +#[derive( + Clone, + Debug, + Default, + Eq, + MallocSizeOf, + PartialEq, + SpecifiedValueInfo, + ToComputedValue, + ToCss, + ToResolvedValue, + ToShmem, +)] +#[repr(transparent)] +#[css(comma)] +pub struct AlignTracks(#[css(iterable, if_empty = "normal")] pub crate::OwnedSlice); + +impl Parse for AlignTracks { + fn parse<'i, 't>( + context: &ParserContext, + input: &mut Parser<'i, 't>, + ) -> Result> { + let values = input.parse_comma_separated(|input| AlignContent::parse(context, input))?; + Ok(AlignTracks(values.into())) + } +} + /// Value for the `justify-content` property. /// /// @@ -329,18 +349,35 @@ impl SpecifiedValueInfo for JustifyContent { ContentDistribution::list_keywords(f, AxisDirection::Inline); } } +/// Value for the `justify-tracks` property. +/// +/// +#[derive( + Clone, + Debug, + Default, + Eq, + MallocSizeOf, + PartialEq, + SpecifiedValueInfo, + ToComputedValue, + ToCss, + ToResolvedValue, + ToShmem, +)] +#[repr(transparent)] +#[css(comma)] +pub struct JustifyTracks( + #[css(iterable, if_empty = "normal")] pub crate::OwnedSlice, +); -#[cfg(feature = "gecko")] -impl From for JustifyContent { - fn from(bits: u16) -> Self { - JustifyContent(ContentDistribution::from_bits(bits)) - } -} - -#[cfg(feature = "gecko")] -impl From for u16 { - fn from(v: JustifyContent) -> u16 { - v.0.as_bits() +impl Parse for JustifyTracks { + fn parse<'i, 't>( + context: &ParserContext, + input: &mut Parser<'i, 't>, + ) -> Result> { + let values = input.parse_comma_separated(|input| JustifyContent::parse(context, input))?; + Ok(JustifyTracks(values.into())) } } diff --git a/components/style/values/specified/basic_shape.rs b/components/style/values/specified/basic_shape.rs index b1782c5b294..135ac85d330 100644 --- a/components/style/values/specified/basic_shape.rs +++ b/components/style/values/specified/basic_shape.rs @@ -263,22 +263,6 @@ impl Ellipse { } } -impl Parse for ShapeRadius { - fn parse<'i, 't>( - context: &ParserContext, - input: &mut Parser<'i, 't>, - ) -> Result> { - if let Ok(lp) = input.try(|i| NonNegativeLengthPercentage::parse(context, i)) { - return Ok(generic::ShapeRadius::Length(lp)); - } - - try_match_ident_ignore_ascii_case! { input, - "closest-side" => Ok(generic::ShapeRadius::ClosestSide), - "farthest-side" => Ok(generic::ShapeRadius::FarthestSide), - } - } -} - impl Parse for Polygon { fn parse<'i, 't>( context: &ParserContext, diff --git a/components/style/values/specified/grid.rs b/components/style/values/specified/grid.rs index 54d31d5826d..9c6bf385d31 100644 --- a/components/style/values/specified/grid.rs +++ b/components/style/values/specified/grid.rs @@ -295,6 +295,18 @@ fn allow_grid_template_subgrids() -> bool { false } +#[cfg(feature = "gecko")] +#[inline] +fn allow_grid_template_masonry() -> bool { + static_prefs::pref!("layout.css.grid-template-masonry-value.enabled") +} + +#[cfg(feature = "servo")] +#[inline] +fn allow_grid_template_masonry() -> bool { + false +} + impl Parse for GridTemplateComponent { fn parse<'i, 't>( context: &ParserContext, @@ -319,7 +331,11 @@ impl GridTemplateComponent { return Ok(GridTemplateComponent::Subgrid(Box::new(t))); } } - + if allow_grid_template_masonry() { + if input.try(|i| i.expect_ident_matching("masonry")).is_ok() { + return Ok(GridTemplateComponent::Masonry); + } + } let track_list = TrackList::parse(context, input)?; Ok(GridTemplateComponent::TrackList(Box::new(track_list))) } diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index 63af3015386..4bb910fc97d 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -18,9 +18,8 @@ use crate::context::QuirksMode; use crate::parser::{Parse, ParserContext}; use crate::values::serialize_atom_identifier; use crate::values::specified::calc::CalcNode; -use crate::{Atom, Namespace, Prefix, Zero}; +use crate::{Atom, Namespace, One, Prefix, Zero}; use cssparser::{Parser, Token}; -use num_traits::One; use std::f32; use std::fmt::{self, Write}; use std::ops::Add; @@ -28,9 +27,9 @@ use style_traits::values::specified::AllowedNumericType; use style_traits::{CssWriter, ParseError, SpecifiedValueInfo, StyleParseErrorKind, ToCss}; #[cfg(feature = "gecko")] -pub use self::align::{AlignContent, AlignItems, AlignSelf, ContentDistribution}; +pub use self::align::{AlignContent, AlignItems, AlignSelf, AlignTracks, ContentDistribution}; #[cfg(feature = "gecko")] -pub use self::align::{JustifyContent, JustifyItems, JustifySelf, SelfAlignment}; +pub use self::align::{JustifyContent, JustifyItems, JustifySelf, JustifyTracks, SelfAlignment}; pub use self::angle::{AllowUnitlessZeroAngle, Angle}; pub use self::background::{BackgroundRepeat, BackgroundSize}; pub use self::basic_shape::FillRule; @@ -72,7 +71,10 @@ pub use self::list::Quotes; pub use self::motion::{OffsetPath, OffsetRotate}; pub use self::outline::OutlineStyle; pub use self::percentage::Percentage; -pub use self::position::{GridAutoFlow, GridTemplateAreas, Position, PositionOrAuto}; +pub use self::position::AspectRatio; +pub use self::position::{ + GridAutoFlow, GridTemplateAreas, MasonryAutoFlow, Position, PositionOrAuto, +}; pub use self::position::{PositionComponent, ZIndex}; pub use self::rect::NonNegativeLengthOrNumberRect; pub use self::resolution::Resolution; @@ -375,11 +377,29 @@ impl Parse for NonNegativeNumber { } } +impl One for NonNegativeNumber { + #[inline] + fn one() -> Self { + NonNegativeNumber::new(1.0) + } + + #[inline] + fn is_one(&self) -> bool { + self.get() == 1.0 + } +} + impl NonNegativeNumber { /// Returns a new non-negative number with the value `val`. pub fn new(val: CSSFloat) -> Self { NonNegative::(Number::new(val.max(0.))) } + + /// Returns the numeric value. + #[inline] + pub fn get(&self) -> f32 { + self.0.get() + } } /// A Number which is >= 1.0. @@ -542,14 +562,10 @@ impl One for Integer { fn one() -> Self { Self::new(1) } -} -// This is not great, because it loses calc-ness, but it's necessary for One. -impl ::std::ops::Mul for Integer { - type Output = Self; - - fn mul(self, other: Self) -> Self { - Self::new(self.value * other.value) + #[inline] + fn is_one(&self) -> bool { + self.value() == 1 } } diff --git a/components/style/values/specified/position.rs b/components/style/values/specified/position.rs index b843de29a41..c2998de328b 100644 --- a/components/style/values/specified/position.rs +++ b/components/style/values/specified/position.rs @@ -12,13 +12,14 @@ use crate::selector_map::PrecomputedHashMap; use crate::str::HTML_SPACE_CHARACTERS; use crate::values::computed::LengthPercentage as ComputedLengthPercentage; use crate::values::computed::{Context, Percentage, ToComputedValue}; +use crate::values::generics::position::AspectRatio as GenericAspectRatio; use crate::values::generics::position::Position as GenericPosition; use crate::values::generics::position::PositionComponent as GenericPositionComponent; use crate::values::generics::position::PositionOrAuto as GenericPositionOrAuto; +use crate::values::generics::position::Ratio as GenericRatio; use crate::values::generics::position::ZIndex as GenericZIndex; -use crate::values::specified::{AllowQuirks, Integer, LengthPercentage}; -use crate::Atom; -use crate::Zero; +use crate::values::specified::{AllowQuirks, Integer, LengthPercentage, NonNegativeNumber}; +use crate::{Atom, One, Zero}; use cssparser::Parser; use selectors::parser::SelectorParseErrorKind; use servo_arc::Arc; @@ -383,6 +384,171 @@ bitflags! { } } +#[derive( + Clone, + Copy, + Debug, + Eq, + MallocSizeOf, + PartialEq, + SpecifiedValueInfo, + ToComputedValue, + ToCss, + ToResolvedValue, + ToShmem, +)] +/// Masonry auto-placement algorithm packing. +pub enum MasonryPlacement { + /// Place the item in the track(s) with the smallest extent so far. + Pack, + /// Place the item after the last item, from start to end. + Next, +} + +#[derive( + Clone, + Copy, + Debug, + Eq, + MallocSizeOf, + PartialEq, + SpecifiedValueInfo, + ToComputedValue, + ToCss, + ToResolvedValue, + ToShmem, +)] +/// Masonry auto-placement algorithm item sorting option. +pub enum MasonryItemOrder { + /// Place all items with a definite placement before auto-placed items. + DefiniteFirst, + /// Place items in `order-modified document order`. + Ordered, +} + +#[derive( + Clone, + Copy, + Debug, + Eq, + MallocSizeOf, + PartialEq, + SpecifiedValueInfo, + ToComputedValue, + ToCss, + ToResolvedValue, + ToShmem, +)] +/// Controls how the Masonry layout algorithm works +/// specifying exactly how auto-placed items get flowed in the masonry axis. +pub struct MasonryAutoFlow { + /// Specify how to pick a auto-placement track. + #[css(contextual_skip_if = "is_pack_with_non_default_order")] + pub placement: MasonryPlacement, + /// Specify how to pick an item to place. + #[css(skip_if = "is_item_order_definite_first")] + pub order: MasonryItemOrder, +} + +#[inline] +fn is_pack_with_non_default_order(placement: &MasonryPlacement, order: &MasonryItemOrder) -> bool { + *placement == MasonryPlacement::Pack && *order != MasonryItemOrder::DefiniteFirst +} + +#[inline] +fn is_item_order_definite_first(order: &MasonryItemOrder) -> bool { + *order == MasonryItemOrder::DefiniteFirst +} + +impl MasonryAutoFlow { + #[inline] + /// Get initial `masonry-auto-flow` value. + pub fn initial() -> MasonryAutoFlow { + MasonryAutoFlow { + placement: MasonryPlacement::Pack, + order: MasonryItemOrder::DefiniteFirst, + } + } +} + +impl Parse for MasonryAutoFlow { + /// [ definite-first | ordered ] || [ pack | next ] + fn parse<'i, 't>( + _context: &ParserContext, + input: &mut Parser<'i, 't>, + ) -> Result> { + let mut value = MasonryAutoFlow::initial(); + let mut got_placement = false; + let mut got_order = false; + while !input.is_exhausted() { + let location = input.current_source_location(); + let ident = input.expect_ident()?; + let success = match_ignore_ascii_case! { &ident, + "pack" if !got_placement => { + got_placement = true; + true + }, + "next" if !got_placement => { + value.placement = MasonryPlacement::Next; + got_placement = true; + true + }, + "definite-first" if !got_order => { + got_order = true; + true + }, + "ordered" if !got_order => { + value.order = MasonryItemOrder::Ordered; + got_order = true; + true + }, + _ => false + }; + if !success { + return Err(location + .new_custom_error(SelectorParseErrorKind::UnexpectedIdent(ident.clone()))); + } + } + + if got_placement || got_order { + Ok(value) + } else { + Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)) + } + } +} + +#[cfg(feature = "gecko")] +impl From for MasonryAutoFlow { + fn from(bits: u8) -> MasonryAutoFlow { + use crate::gecko_bindings::structs; + let mut value = MasonryAutoFlow::initial(); + if bits & structs::NS_STYLE_MASONRY_PLACEMENT_PACK as u8 == 0 { + value.placement = MasonryPlacement::Next; + } + if bits & structs::NS_STYLE_MASONRY_ORDER_DEFINITE_FIRST as u8 == 0 { + value.order = MasonryItemOrder::Ordered; + } + value + } +} + +#[cfg(feature = "gecko")] +impl From for u8 { + fn from(v: MasonryAutoFlow) -> u8 { + use crate::gecko_bindings::structs; + + let mut result: u8 = 0; + if v.placement == MasonryPlacement::Pack { + result |= structs::NS_STYLE_MASONRY_PLACEMENT_PACK as u8; + } + if v.order == MasonryItemOrder::DefiniteFirst { + result |= structs::NS_STYLE_MASONRY_ORDER_DEFINITE_FIRST as u8; + } + result + } +} + impl Parse for GridAutoFlow { /// [ row | column ] || dense fn parse<'i, 't>( @@ -711,3 +877,74 @@ impl GridTemplateAreas { /// A specified value for the `z-index` property. pub type ZIndex = GenericZIndex; + +/// A specified value for the `aspect-ratio` property. +pub type AspectRatio = GenericAspectRatio; + +impl Parse for AspectRatio { + fn parse<'i, 't>( + context: &ParserContext, + input: &mut Parser<'i, 't>, + ) -> Result> { + use crate::values::generics::position::PreferredRatio; + + let location = input.current_source_location(); + let mut auto = input.try(|i| i.expect_ident_matching("auto")); + let ratio = input.try(|i| Ratio::parse(context, i)); + if auto.is_err() { + auto = input.try(|i| i.expect_ident_matching("auto")); + } + + if auto.is_err() && ratio.is_err() { + return Err(location.new_custom_error(StyleParseErrorKind::UnspecifiedError)); + } + + Ok(AspectRatio { + auto: auto.is_ok(), + ratio: match ratio { + Ok(ratio) => PreferredRatio::Ratio(ratio), + Err(..) => PreferredRatio::None, + }, + }) + } +} + +impl AspectRatio { + /// Returns Self by a valid ratio. + pub fn from_mapped_ratio(w: f32, h: f32) -> Self { + use crate::values::generics::position::PreferredRatio; + AspectRatio { + auto: true, + ratio: PreferredRatio::Ratio(GenericRatio( + NonNegativeNumber::new(w), + NonNegativeNumber::new(h), + )), + } + } +} + +/// A specified value. +pub type Ratio = GenericRatio; + +// https://drafts.csswg.org/css-values-4/#ratios +impl Parse for Ratio { + fn parse<'i, 't>( + context: &ParserContext, + input: &mut Parser<'i, 't>, + ) -> Result> { + let a = NonNegativeNumber::parse(context, input)?; + let b = match input.try_parse(|input| input.expect_delim('/')) { + Ok(()) => NonNegativeNumber::parse(context, input)?, + _ => One::one(), + }; + + // The computed value of a is the pair of numbers provided, unless + // both numbers are zero, in which case the computed value is the pair (1, 0) + // (same as 1 / 0). + // https://drafts.csswg.org/css-values-4/#ratios + if a.is_zero() && b.is_zero() { + return Ok(GenericRatio(One::one(), Zero::zero())); + } + return Ok(GenericRatio(a, b)); + } +} diff --git a/components/style/values/specified/text.rs b/components/style/values/specified/text.rs index 703e51a5248..bee3c665840 100644 --- a/components/style/values/specified/text.rs +++ b/components/style/values/specified/text.rs @@ -128,6 +128,7 @@ impl ToComputedValue for LineHeight { Eq, MallocSizeOf, PartialEq, + Parse, SpecifiedValueInfo, ToComputedValue, ToCss, @@ -144,30 +145,6 @@ pub enum TextOverflowSide { String(crate::OwnedStr), } -impl Parse for TextOverflowSide { - fn parse<'i, 't>( - _context: &ParserContext, - input: &mut Parser<'i, 't>, - ) -> Result> { - let location = input.current_source_location(); - match *input.next()? { - Token::Ident(ref ident) => { - match_ignore_ascii_case! { ident, - "clip" => Ok(TextOverflowSide::Clip), - "ellipsis" => Ok(TextOverflowSide::Ellipsis), - _ => Err(location.new_custom_error( - SelectorParseErrorKind::UnexpectedIdent(ident.clone()) - )) - } - }, - Token::QuotedString(ref v) => { - Ok(TextOverflowSide::String(v.as_ref().to_owned().into())) - }, - ref t => Err(location.new_unexpected_token_error(t.clone())), - } - } -} - #[derive(Clone, Debug, Eq, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss, ToShmem)] /// text-overflow. Specifies rendering when inline content overflows its line box edge. pub struct TextOverflow { diff --git a/components/style_derive/parse.rs b/components/style_derive/parse.rs index 80c214bd734..ebf46a3735e 100644 --- a/components/style_derive/parse.rs +++ b/components/style_derive/parse.rs @@ -15,7 +15,14 @@ pub struct ParseVariantAttrs { pub condition: Option, } +#[darling(attributes(parse), default)] +#[derive(Default, FromField)] +pub struct ParseFieldAttrs { + field_bound: bool, +} + fn parse_non_keyword_variant( + where_clause: &mut Option, name: &syn::Ident, variant: &VariantInfo, variant_attrs: &CssVariantAttrs, @@ -32,7 +39,14 @@ fn parse_non_keyword_variant( "We only support deriving parse for simple variants" ); let variant_name = &variant.ast().ident; - let ty = &bindings[0].ast().ty; + let binding_ast = &bindings[0].ast(); + let ty = &binding_ast.ty; + + let field_attrs = cg::parse_field_attrs::(binding_ast); + if field_attrs.field_bound { + cg::add_predicate(where_clause, parse_quote!(#ty: crate::parser::Parse)); + } + let mut parse = if skip_try { quote! { let v = <#ty as crate::parser::Parse>::parse(context, input)?; @@ -67,15 +81,12 @@ fn parse_non_keyword_variant( } pub fn derive(mut input: DeriveInput) -> TokenStream { - { - let mut where_clause = input.generics.where_clause.take(); - for param in input.generics.type_params() { - cg::add_predicate( - &mut where_clause, - parse_quote!(#param: crate::parser::Parse), - ); - } - input.generics.where_clause = where_clause; + let mut where_clause = input.generics.where_clause.take(); + for param in input.generics.type_params() { + cg::add_predicate( + &mut where_clause, + parse_quote!(#param: crate::parser::Parse), + ); } let name = &input.ident; @@ -88,12 +99,13 @@ pub fn derive(mut input: DeriveInput) -> TokenStream { let mut effective_variants = 0; for variant in s.variants().iter() { let css_variant_attrs = cg::parse_variant_attrs_from_ast::(&variant.ast()); - let parse_attrs = cg::parse_variant_attrs_from_ast::(&variant.ast()); if css_variant_attrs.skip { continue; } effective_variants += 1; + let parse_attrs = cg::parse_variant_attrs_from_ast::(&variant.ast()); + saw_condition |= parse_attrs.condition.is_some(); if !variant.bindings().is_empty() { @@ -142,8 +154,14 @@ pub fn derive(mut input: DeriveInput) -> TokenStream { let mut parse_non_keywords = quote! {}; for (i, (variant, css_attrs, parse_attrs)) in non_keywords.iter().enumerate() { let skip_try = !has_keywords && i == non_keywords.len() - 1; - let parse_variant = - parse_non_keyword_variant(name, variant, css_attrs, parse_attrs, skip_try); + let parse_variant = parse_non_keyword_variant( + &mut where_clause, + name, + variant, + css_attrs, + parse_attrs, + skip_try, + ); parse_non_keywords.extend(parse_variant); } @@ -171,6 +189,9 @@ pub fn derive(mut input: DeriveInput) -> TokenStream { quote! { Self::parse(input) } }; + let has_non_keywords = !non_keywords.is_empty(); + + input.generics.where_clause = where_clause; let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); let parse_trait_impl = quote! { @@ -189,7 +210,7 @@ pub fn derive(mut input: DeriveInput) -> TokenStream { return parse_trait_impl; } - assert!(non_keywords.is_empty()); + assert!(!has_non_keywords); // TODO(emilio): It'd be nice to get rid of these, but that makes the // conversion harder... diff --git a/components/style_traits/dom.rs b/components/style_traits/dom.rs index f9844519279..03d5264abf5 100644 --- a/components/style_traits/dom.rs +++ b/components/style_traits/dom.rs @@ -13,8 +13,8 @@ /// Because the script task's GC does not trace layout, node data cannot be safely stored in layout /// data structures. Also, layout code tends to be faster when the DOM is not being accessed, for /// locality reasons. Using `OpaqueNode` enforces this invariant. -#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] -#[cfg_attr(feature = "servo", derive(MallocSizeOf, Deserialize, Serialize))] +#[derive(Clone, Copy, Debug, Eq, Hash, MallocSizeOf, PartialEq)] +#[cfg_attr(feature = "servo", derive(Deserialize, Serialize))] pub struct OpaqueNode(pub usize); impl OpaqueNode { diff --git a/components/style_traits/owned_slice.rs b/components/style_traits/owned_slice.rs index bbce7065196..f6068365360 100644 --- a/components/style_traits/owned_slice.rs +++ b/components/style_traits/owned_slice.rs @@ -13,7 +13,7 @@ use std::marker::PhantomData; use std::ops::{Deref, DerefMut}; use std::ptr::NonNull; use std::{fmt, iter, mem, slice}; -use to_shmem::{SharedMemoryBuilder, ToShmem}; +use to_shmem::{self, SharedMemoryBuilder, ToShmem}; /// A struct that basically replaces a `Box<[T]>`, but which cbindgen can /// understand. @@ -159,10 +159,10 @@ impl MallocSizeOf for OwnedSlice { } impl ToShmem for OwnedSlice { - fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> mem::ManuallyDrop { + fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> to_shmem::Result { unsafe { - let dest = to_shmem::to_shmem_slice(self.iter(), builder); - mem::ManuallyDrop::new(Self::from(Box::from_raw(dest))) + let dest = to_shmem::to_shmem_slice(self.iter(), builder)?; + Ok(mem::ManuallyDrop::new(Self::from(Box::from_raw(dest)))) } } } diff --git a/components/to_shmem/lib.rs b/components/to_shmem/lib.rs index 22b86087bfd..47dc8d17eff 100644 --- a/components/to_shmem/lib.rs +++ b/components/to_shmem/lib.rs @@ -42,6 +42,11 @@ use std::slice; use std::str; use thin_slice::ThinBoxedSlice; +/// Result type for ToShmem::to_shmem. +/// +/// The String is an error message describing why the call failed. +pub type Result = std::result::Result, String>; + // Various pointer arithmetic functions in this file can be replaced with // functions on `Layout` once they have stabilized: // @@ -114,13 +119,13 @@ impl SharedMemoryBuilder { /// a shared memory buffer by calling ToShmem::to_shmem on it. /// /// Panics if there is insufficient space in the buffer. - pub fn write(&mut self, value: &T) -> *mut T { + pub fn write(&mut self, value: &T) -> std::result::Result<*mut T, String> { // Reserve space for the value. let dest: *mut T = self.alloc_value(); // Make a clone of the value with all of its heap allocations // placed in the shared memory buffer. - let value = value.to_shmem(self); + let value = value.to_shmem(self)?; unsafe { // Copy the value into the buffer. @@ -128,7 +133,7 @@ impl SharedMemoryBuilder { } // Return a pointer to the shared value. - dest + Ok(dest) } /// Reserves space in the shared memory buffer to fit a value of type T, @@ -190,7 +195,10 @@ pub trait ToShmem: Sized { /// /// The return type is wrapped in ManuallyDrop to make it harder to /// accidentally invoke the destructor of the value that is produced. - fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop; + /// + /// Returns a Result so that we can gracefully recover from unexpected + /// content. + fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result; } #[macro_export] @@ -201,8 +209,8 @@ macro_rules! impl_trivial_to_shmem { fn to_shmem( &self, _builder: &mut $crate::SharedMemoryBuilder, - ) -> ::std::mem::ManuallyDrop { - ::std::mem::ManuallyDrop::new(*self) + ) -> $crate::Result { + $crate::Result::Ok(::std::mem::ManuallyDrop::new(*self)) } } )* @@ -231,58 +239,60 @@ impl_trivial_to_shmem!(cssparser::SourceLocation); impl_trivial_to_shmem!(cssparser::TokenSerializationType); impl ToShmem for PhantomData { - fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> ManuallyDrop { - ManuallyDrop::new(*self) + fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> Result { + Ok(ManuallyDrop::new(*self)) } } impl ToShmem for Range { - fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop { - ManuallyDrop::new(Range { - start: ManuallyDrop::into_inner(self.start.to_shmem(builder)), - end: ManuallyDrop::into_inner(self.end.to_shmem(builder)), - }) + fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result { + Ok(ManuallyDrop::new(Range { + start: ManuallyDrop::into_inner(self.start.to_shmem(builder)?), + end: ManuallyDrop::into_inner(self.end.to_shmem(builder)?), + })) } } impl ToShmem for cssparser::UnicodeRange { - fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> ManuallyDrop { - ManuallyDrop::new(cssparser::UnicodeRange { + fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> Result { + Ok(ManuallyDrop::new(cssparser::UnicodeRange { start: self.start, end: self.end, - }) + })) } } impl ToShmem for (T, U) { - fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop { - ManuallyDrop::new(( - ManuallyDrop::into_inner(self.0.to_shmem(builder)), - ManuallyDrop::into_inner(self.1.to_shmem(builder)), - )) + fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result { + Ok(ManuallyDrop::new(( + ManuallyDrop::into_inner(self.0.to_shmem(builder)?), + ManuallyDrop::into_inner(self.1.to_shmem(builder)?), + ))) } } impl ToShmem for Wrapping { - fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop { - ManuallyDrop::new(Wrapping(ManuallyDrop::into_inner(self.0.to_shmem(builder)))) + fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result { + Ok(ManuallyDrop::new(Wrapping(ManuallyDrop::into_inner( + self.0.to_shmem(builder)?, + )))) } } impl ToShmem for Box { - fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop { + fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result { // Reserve space for the boxed value. let dest: *mut T = builder.alloc_value(); // Make a clone of the boxed value with all of its heap allocations // placed in the shared memory buffer. - let value = (**self).to_shmem(builder); + let value = (**self).to_shmem(builder)?; unsafe { // Copy the value into the buffer. ptr::write(dest, ManuallyDrop::into_inner(value)); - ManuallyDrop::new(Box::from_raw(dest)) + Ok(ManuallyDrop::new(Box::from_raw(dest))) } } } @@ -293,7 +303,7 @@ unsafe fn to_shmem_slice_ptr<'a, T, I>( src: I, dest: *mut T, builder: &mut SharedMemoryBuilder, -) -> *mut [T] +) -> std::result::Result<*mut [T], String> where T: 'a + ToShmem, I: ExactSizeIterator, @@ -303,15 +313,18 @@ where // Make a clone of each element from the iterator with its own heap // allocations placed in the buffer, and copy that clone into the buffer. for (src, dest) in src.zip(dest.iter_mut()) { - ptr::write(dest, ManuallyDrop::into_inner(src.to_shmem(builder))); + ptr::write(dest, ManuallyDrop::into_inner(src.to_shmem(builder)?)); } - dest + Ok(dest) } /// Writes all the items in `src` into a slice in the shared memory buffer and /// returns a pointer to the slice. -pub unsafe fn to_shmem_slice<'a, T, I>(src: I, builder: &mut SharedMemoryBuilder) -> *mut [T] +pub unsafe fn to_shmem_slice<'a, T, I>( + src: I, + builder: &mut SharedMemoryBuilder, +) -> std::result::Result<*mut [T], String> where T: 'a + ToShmem, I: ExactSizeIterator, @@ -321,16 +334,16 @@ where } impl ToShmem for Box<[T]> { - fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop { + fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result { unsafe { - let dest = to_shmem_slice(self.iter(), builder); - ManuallyDrop::new(Box::from_raw(dest)) + let dest = to_shmem_slice(self.iter(), builder)?; + Ok(ManuallyDrop::new(Box::from_raw(dest))) } } } impl ToShmem for ThinBoxedSlice { - fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop { + fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result { // We could support this if we needed but in practice we will never // need to handle such big ThinBoxedSlices. assert!( @@ -340,14 +353,14 @@ impl ToShmem for ThinBoxedSlice { ); unsafe { - let dest = to_shmem_slice(self.iter(), builder); - ManuallyDrop::new(ThinBoxedSlice::from_raw(dest)) + let dest = to_shmem_slice(self.iter(), builder)?; + Ok(ManuallyDrop::new(ThinBoxedSlice::from_raw(dest))) } } } impl ToShmem for Box { - fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop { + fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result { // Reserve space for the string bytes. let dest: *mut u8 = builder.alloc_array(self.len()); @@ -355,15 +368,15 @@ impl ToShmem for Box { // Copy the value into the buffer. ptr::copy(self.as_ptr(), dest, self.len()); - ManuallyDrop::new(Box::from_raw(str::from_utf8_unchecked_mut( - slice::from_raw_parts_mut(dest, self.len()), + Ok(ManuallyDrop::new(Box::from_raw( + str::from_utf8_unchecked_mut(slice::from_raw_parts_mut(dest, self.len())), ))) } } } impl ToShmem for String { - fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop { + fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result { // Reserve space for the string bytes. let dest: *mut u8 = builder.alloc_array(self.len()); @@ -371,13 +384,17 @@ impl ToShmem for String { // Copy the value into the buffer. ptr::copy(self.as_ptr(), dest, self.len()); - ManuallyDrop::new(String::from_raw_parts(dest, self.len(), self.len())) + Ok(ManuallyDrop::new(String::from_raw_parts( + dest, + self.len(), + self.len(), + ))) } } } impl ToShmem for CString { - fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop { + fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result { let len = self.as_bytes_with_nul().len(); // Reserve space for the string bytes. @@ -387,53 +404,55 @@ impl ToShmem for CString { // Copy the value into the buffer. ptr::copy(self.as_ptr(), dest, len); - ManuallyDrop::new(CString::from_raw(dest)) + Ok(ManuallyDrop::new(CString::from_raw(dest))) } } } impl ToShmem for Vec { - fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop { + fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result { unsafe { - let dest = to_shmem_slice(self.iter(), builder) as *mut T; + let dest = to_shmem_slice(self.iter(), builder)? as *mut T; let dest_vec = Vec::from_raw_parts(dest, self.len(), self.len()); - ManuallyDrop::new(dest_vec) + Ok(ManuallyDrop::new(dest_vec)) } } } impl> ToShmem for SmallVec { - fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop { + fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result { let dest_vec = unsafe { if self.spilled() { // Place the items in a separate allocation in the shared memory // buffer. - let dest = to_shmem_slice(self.iter(), builder) as *mut T; + let dest = to_shmem_slice(self.iter(), builder)? as *mut T; SmallVec::from_raw_parts(dest, self.len(), self.len()) } else { // Place the items inline. let mut s = SmallVec::new(); - to_shmem_slice_ptr(self.iter(), s.as_mut_ptr(), builder); + to_shmem_slice_ptr(self.iter(), s.as_mut_ptr(), builder)?; s.set_len(self.len()); s } }; - ManuallyDrop::new(dest_vec) + Ok(ManuallyDrop::new(dest_vec)) } } impl ToShmem for Option { - fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop { - ManuallyDrop::new( - self.as_ref() - .map(|v| ManuallyDrop::into_inner(v.to_shmem(builder))), - ) + fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result { + let v = match self { + Some(v) => Some(ManuallyDrop::into_inner(v.to_shmem(builder)?)), + None => None, + }; + + Ok(ManuallyDrop::new(v)) } } impl ToShmem for Arc { - fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop { + fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result { // Assert that we don't encounter any shared references to values we // don't expect. Those we expect are those noted by calling // add_allowed_duplication_type, and should be types where we're fine @@ -453,7 +472,7 @@ impl ToShmem for Arc { // Make a clone of the Arc-owned value with all of its heap allocations // placed in the shared memory buffer. - let value = (**self).to_shmem(builder); + let value = (**self).to_shmem(builder)?; // Create a new Arc with the shared value and have it place its // ArcInner in the shared memory buffer. @@ -466,13 +485,13 @@ impl ToShmem for Arc { #[cfg(debug_assertions)] builder.shared_values.insert(self.heap_ptr()); - ManuallyDrop::new(static_arc) + Ok(ManuallyDrop::new(static_arc)) } } } impl ToShmem for ThinArc { - fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop { + fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result { // We don't currently have any shared ThinArc values in stylesheets, // so don't support them for now. #[cfg(debug_assertions)] @@ -484,8 +503,11 @@ impl ToShmem for ThinArc { // Make a clone of the Arc-owned header and slice values with all of // their heap allocations placed in the shared memory buffer. - let header = self.header.header.to_shmem(builder); - let values: Vec> = self.slice.iter().map(|v| v.to_shmem(builder)).collect(); + let header = self.header.header.to_shmem(builder)?; + let mut values = Vec::with_capacity(self.slice.len()); + for v in self.slice.iter() { + values.push(v.to_shmem(builder)?); + } // Create a new ThinArc with the shared value and have it place // its ArcInner in the shared memory buffer. @@ -499,13 +521,13 @@ impl ToShmem for ThinArc { #[cfg(debug_assertions)] builder.shared_values.insert(self.heap_ptr()); - ManuallyDrop::new(static_arc) + Ok(ManuallyDrop::new(static_arc)) } } } impl ToShmem for SmallBitVec { - fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop { + fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result { let storage = match self.clone().into_storage() { InternalStorage::Spilled(vs) => { // Reserve space for the boxed slice values. @@ -524,13 +546,15 @@ impl ToShmem for SmallBitVec { }, InternalStorage::Inline(x) => InternalStorage::Inline(x), }; - ManuallyDrop::new(unsafe { SmallBitVec::from_storage(storage) }) + Ok(ManuallyDrop::new(unsafe { + SmallBitVec::from_storage(storage) + })) } } #[cfg(feature = "string_cache")] impl ToShmem for string_cache::Atom { - fn to_shmem(&self, _: &mut SharedMemoryBuilder) -> ManuallyDrop { + fn to_shmem(&self, _: &mut SharedMemoryBuilder) -> Result { // NOTE(emilio): In practice, this can be implemented trivially if // string_cache could expose the implementation detail of static atoms // being an index into the static table (and panicking in the diff --git a/components/to_shmem_derive/to_shmem.rs b/components/to_shmem_derive/to_shmem.rs index 01ba308e308..c3410bcc7e0 100644 --- a/components/to_shmem_derive/to_shmem.rs +++ b/components/to_shmem_derive/to_shmem.rs @@ -27,13 +27,23 @@ pub fn derive(mut input: syn::DeriveInput) -> TokenStream { input.generics.where_clause = where_clause; - let match_body = cg::fmap_match(&input, BindStyle::Ref, |binding| { - quote! { - ::std::mem::ManuallyDrop::into_inner( - ::to_shmem::ToShmem::to_shmem(#binding, builder) - ) - } - }); + // Do all of the `to_shmem()?` calls before the `ManuallyDrop::into_inner()` + // calls, so that we don't drop a value in the shared memory buffer if one + // of the `to_shmem`s fails. + let match_body = cg::fmap2_match( + &input, + BindStyle::Ref, + |binding| { + quote! { + ::to_shmem::ToShmem::to_shmem(#binding, builder)? + } + }, + |binding| { + Some(quote! { + ::std::mem::ManuallyDrop::into_inner(#binding) + }) + }, + ); let name = &input.ident; let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); @@ -44,12 +54,12 @@ pub fn derive(mut input: syn::DeriveInput) -> TokenStream { fn to_shmem( &self, builder: &mut ::to_shmem::SharedMemoryBuilder, - ) -> ::std::mem::ManuallyDrop { - ::std::mem::ManuallyDrop::new( + ) -> ::to_shmem::Result { + Ok(::std::mem::ManuallyDrop::new( match *self { #match_body } - ) + )) } } } diff --git a/components/url/lib.rs b/components/url/lib.rs index 8a05837a461..f58bbbad3ff 100644 --- a/components/url/lib.rs +++ b/components/url/lib.rs @@ -20,7 +20,6 @@ pub use crate::origin::{ImmutableOrigin, MutableOrigin, OpaqueOrigin}; use std::collections::hash_map::DefaultHasher; use std::fmt; use std::hash::Hasher; -use std::mem::ManuallyDrop; use std::net::IpAddr; use std::ops::{Index, Range, RangeFrom, RangeFull, RangeTo}; use std::path::Path; @@ -34,7 +33,7 @@ pub use url::Host; pub struct ServoUrl(#[ignore_malloc_size_of = "Arc"] Arc); impl ToShmem for ServoUrl { - fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> ManuallyDrop { + fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> to_shmem::Result { unimplemented!("If servo wants to share stylesheets across processes, ToShmem for Url must be implemented") } } diff --git a/tests/unit/style/size_of.rs b/tests/unit/style/size_of.rs index ed435345a28..7306cf059ef 100644 --- a/tests/unit/style/size_of.rs +++ b/tests/unit/style/size_of.rs @@ -6,7 +6,7 @@ use selectors::parser::{SelectorParseError, SelectorParseErrorKind}; use style::invalidation::element::invalidation_map::Dependency; use style::properties; -size_of_test!(test_size_of_dependency, Dependency, 16); +size_of_test!(test_size_of_dependency, Dependency, 24); size_of_test!( test_size_of_property_declaration, diff --git a/tests/unit/style/stylist.rs b/tests/unit/style/stylist.rs index 0d7511f0583..8836436c8be 100644 --- a/tests/unit/style/stylist.rs +++ b/tests/unit/style/stylist.rs @@ -201,13 +201,13 @@ fn test_insert() { 0, selector_map .class_hash - .get(&Atom::from("foo"), QuirksMode::NoQuirks) + .get(&Atom::from("intro"), QuirksMode::NoQuirks) .unwrap()[0] .source_order ); assert!(selector_map .class_hash - .get(&Atom::from("intro"), QuirksMode::NoQuirks) + .get(&Atom::from("foo"), QuirksMode::NoQuirks) .is_none()); }