style: Remove the never_matches field from attr selectors

It's an extra branch which in practice we almost never take, plus extra
checks during parsing.

Differential Revision: https://phabricator.services.mozilla.com/D180529
This commit is contained in:
Emilio Cobos Álvarez 2023-06-14 21:08:22 +00:00 committed by Martin Robinson
parent 54a783db17
commit 3da2db1c53
5 changed files with 28 additions and 57 deletions

View file

@ -19,7 +19,6 @@ pub struct AttrSelectorWithOptionalNamespace<Impl: SelectorImpl> {
pub local_name_lower: Impl::LocalName, pub local_name_lower: Impl::LocalName,
#[cfg_attr(feature = "shmem", shmem(field_bound))] #[cfg_attr(feature = "shmem", shmem(field_bound))]
pub operation: ParsedAttrSelectorOperation<Impl::AttrValue>, pub operation: ParsedAttrSelectorOperation<Impl::AttrValue>,
pub never_matches: bool,
} }
impl<Impl: SelectorImpl> AttrSelectorWithOptionalNamespace<Impl> { impl<Impl: SelectorImpl> AttrSelectorWithOptionalNamespace<Impl> {
@ -47,7 +46,7 @@ pub enum ParsedAttrSelectorOperation<AttrValue> {
WithValue { WithValue {
operator: AttrSelectorOperator, operator: AttrSelectorOperator,
case_sensitivity: ParsedCaseSensitivity, case_sensitivity: ParsedCaseSensitivity,
expected_value: AttrValue, value: AttrValue,
}, },
} }
@ -57,7 +56,7 @@ pub enum AttrSelectorOperation<AttrValue> {
WithValue { WithValue {
operator: AttrSelectorOperator, operator: AttrSelectorOperator,
case_sensitivity: CaseSensitivity, case_sensitivity: CaseSensitivity,
expected_value: AttrValue, value: AttrValue,
}, },
} }
@ -71,10 +70,10 @@ impl<AttrValue> AttrSelectorOperation<AttrValue> {
AttrSelectorOperation::WithValue { AttrSelectorOperation::WithValue {
operator, operator,
case_sensitivity, case_sensitivity,
ref expected_value, ref value,
} => operator.eval_str( } => operator.eval_str(
element_attr_value, element_attr_value,
expected_value.as_ref(), value.as_ref(),
case_sensitivity, case_sensitivity,
), ),
} }

View file

@ -736,25 +736,18 @@ where
ref value, ref value,
operator, operator,
case_sensitivity, case_sensitivity,
never_matches,
} => { } => {
if never_matches {
return false;
}
element.attr_matches( element.attr_matches(
&NamespaceConstraint::Specific(&crate::parser::namespace_empty_string::<E::Impl>()), &NamespaceConstraint::Specific(&crate::parser::namespace_empty_string::<E::Impl>()),
local_name, local_name,
&AttrSelectorOperation::WithValue { &AttrSelectorOperation::WithValue {
operator, operator,
case_sensitivity: to_unconditional_case_sensitivity(case_sensitivity, element), case_sensitivity: to_unconditional_case_sensitivity(case_sensitivity, element),
expected_value: value, value,
}, },
) )
}, },
Component::AttributeOther(ref attr_sel) => { Component::AttributeOther(ref attr_sel) => {
if attr_sel.never_matches {
return false;
}
let empty_string; let empty_string;
let namespace = match attr_sel.namespace() { let namespace = match attr_sel.namespace() {
Some(ns) => ns, Some(ns) => ns,
@ -771,14 +764,14 @@ where
ParsedAttrSelectorOperation::WithValue { ParsedAttrSelectorOperation::WithValue {
operator, operator,
case_sensitivity, case_sensitivity,
ref expected_value, ref value,
} => AttrSelectorOperation::WithValue { } => AttrSelectorOperation::WithValue {
operator, operator,
case_sensitivity: to_unconditional_case_sensitivity( case_sensitivity: to_unconditional_case_sensitivity(
case_sensitivity, case_sensitivity,
element, element,
), ),
expected_value, value,
}, },
}, },
) )

View file

@ -3,8 +3,7 @@
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
use crate::attr::{AttrSelectorOperator, AttrSelectorWithOptionalNamespace}; use crate::attr::{AttrSelectorOperator, AttrSelectorWithOptionalNamespace};
use crate::attr::{NamespaceConstraint, ParsedAttrSelectorOperation}; use crate::attr::{NamespaceConstraint, ParsedAttrSelectorOperation, ParsedCaseSensitivity};
use crate::attr::{ParsedCaseSensitivity, SELECTOR_WHITESPACE};
use crate::bloom::BLOOM_HASH_MASK; use crate::bloom::BLOOM_HASH_MASK;
use crate::builder::{ use crate::builder::{
relative_selector_list_specificity_and_flags, selector_list_specificity_and_flags, relative_selector_list_specificity_and_flags, selector_list_specificity_and_flags,
@ -624,6 +623,7 @@ impl AncestorHashes {
} }
} }
#[inline]
pub fn namespace_empty_string<Impl: SelectorImpl>() -> Impl::NamespaceUrl { pub fn namespace_empty_string<Impl: SelectorImpl>() -> Impl::NamespaceUrl {
// Rust types default, not default namespace // Rust types default, not default namespace
Impl::NamespaceUrl::default() Impl::NamespaceUrl::default()
@ -1658,7 +1658,6 @@ pub enum Component<Impl: SelectorImpl> {
#[cfg_attr(feature = "shmem", shmem(field_bound))] #[cfg_attr(feature = "shmem", shmem(field_bound))]
value: Impl::AttrValue, value: Impl::AttrValue,
case_sensitivity: ParsedCaseSensitivity, case_sensitivity: ParsedCaseSensitivity,
never_matches: bool,
}, },
// Use a Box in the less common cases with more data to keep size_of::<Component>() small. // Use a Box in the less common cases with more data to keep size_of::<Component>() small.
AttributeOther(Box<AttrSelectorWithOptionalNamespace<Impl>>), AttributeOther(Box<AttrSelectorWithOptionalNamespace<Impl>>),
@ -1836,11 +1835,7 @@ impl<Impl: SelectorImpl> Component<Impl> {
return false; return false;
} }
}, },
AttributeInNoNamespace { AttributeInNoNamespace { ref local_name, .. } => {
ref local_name,
never_matches,
..
} if !never_matches => {
if !visitor.visit_attribute_selector( if !visitor.visit_attribute_selector(
&NamespaceConstraint::Specific(&namespace_empty_string::<Impl>()), &NamespaceConstraint::Specific(&namespace_empty_string::<Impl>()),
local_name, local_name,
@ -1849,7 +1844,7 @@ impl<Impl: SelectorImpl> Component<Impl> {
return false; return false;
} }
}, },
AttributeOther(ref attr_selector) if !attr_selector.never_matches => { AttributeOther(ref attr_selector) => {
let empty_string; let empty_string;
let namespace = match attr_selector.namespace() { let namespace = match attr_selector.namespace() {
Some(ns) => ns, Some(ns) => ns,
@ -2291,11 +2286,11 @@ impl<Impl: SelectorImpl> ToCss for AttrSelectorWithOptionalNamespace<Impl> {
ParsedAttrSelectorOperation::WithValue { ParsedAttrSelectorOperation::WithValue {
operator, operator,
case_sensitivity, case_sensitivity,
ref expected_value, ref value,
} => { } => {
operator.to_css(dest)?; operator.to_css(dest)?;
dest.write_char('"')?; dest.write_char('"')?;
expected_value.to_css(dest)?; value.to_css(dest)?;
dest.write_char('"')?; dest.write_char('"')?;
match case_sensitivity { match case_sensitivity {
ParsedCaseSensitivity::CaseSensitive | ParsedCaseSensitivity::CaseSensitive |
@ -2663,7 +2658,6 @@ where
local_name, local_name,
local_name_lower, local_name_lower,
operation: ParsedAttrSelectorOperation::Exists, operation: ParsedAttrSelectorOperation::Exists,
never_matches: false,
}, },
))); )));
} else { } else {
@ -2701,18 +2695,8 @@ where
}) => return Err(location.new_custom_error(SelectorParseErrorKind::BadValueInAttr(t))), }) => return Err(location.new_custom_error(SelectorParseErrorKind::BadValueInAttr(t))),
Err(e) => return Err(e.into()), Err(e) => return Err(e.into()),
}; };
let never_matches = match operator {
AttrSelectorOperator::Equal | AttrSelectorOperator::DashMatch => false,
AttrSelectorOperator::Includes => value.is_empty() || value.contains(SELECTOR_WHITESPACE),
AttrSelectorOperator::Prefix |
AttrSelectorOperator::Substring |
AttrSelectorOperator::Suffix => value.is_empty(),
};
let attribute_flags = parse_attribute_flags(input)?; let attribute_flags = parse_attribute_flags(input)?;
let value = value.as_ref().into(); let value = value.as_ref().into();
let local_name_lower; let local_name_lower;
let local_name_is_ascii_lowercase; let local_name_is_ascii_lowercase;
@ -2731,11 +2715,10 @@ where
namespace, namespace,
local_name, local_name,
local_name_lower, local_name_lower,
never_matches,
operation: ParsedAttrSelectorOperation::WithValue { operation: ParsedAttrSelectorOperation::WithValue {
operator, operator,
case_sensitivity, case_sensitivity,
expected_value: value, value,
}, },
}, },
))) )))
@ -2745,7 +2728,6 @@ where
operator, operator,
value, value,
case_sensitivity, case_sensitivity,
never_matches,
}) })
} }
} }
@ -3906,7 +3888,6 @@ pub mod tests {
local_name: DummyAtom::from("attr"), local_name: DummyAtom::from("attr"),
operator: AttrSelectorOperator::DashMatch, operator: AttrSelectorOperator::DashMatch,
value: DummyAttrValue::from("foo"), value: DummyAttrValue::from("foo"),
never_matches: false,
case_sensitivity: ParsedCaseSensitivity::CaseSensitive, case_sensitivity: ParsedCaseSensitivity::CaseSensitive,
}], }],
specificity(0, 1, 0), specificity(0, 1, 0),

View file

@ -99,47 +99,46 @@ impl GeckoElementSnapshot {
AttrSelectorOperation::WithValue { AttrSelectorOperation::WithValue {
operator, operator,
case_sensitivity, case_sensitivity,
expected_value, value,
} => { } => {
let ignore_case = match case_sensitivity { let ignore_case = match case_sensitivity {
CaseSensitivity::CaseSensitive => false, CaseSensitivity::CaseSensitive => false,
CaseSensitivity::AsciiCaseInsensitive => true, CaseSensitivity::AsciiCaseInsensitive => true,
}; };
// FIXME: case sensitivity for operators other than Equal
match operator { match operator {
AttrSelectorOperator::Equal => bindings::Gecko_SnapshotAttrEquals( AttrSelectorOperator::Equal => bindings::Gecko_SnapshotAttrEquals(
self, self,
ns.atom_or_null(), ns.atom_or_null(),
local_name.as_ptr(), local_name.as_ptr(),
expected_value.as_ptr(), value.as_ptr(),
ignore_case, ignore_case,
), ),
AttrSelectorOperator::Includes => bindings::Gecko_SnapshotAttrIncludes( AttrSelectorOperator::Includes => bindings::Gecko_SnapshotAttrIncludes(
self, self,
ns.atom_or_null(), ns.atom_or_null(),
local_name.as_ptr(), local_name.as_ptr(),
expected_value.as_ptr(), value.as_ptr(),
ignore_case, ignore_case,
), ),
AttrSelectorOperator::DashMatch => bindings::Gecko_SnapshotAttrDashEquals( AttrSelectorOperator::DashMatch => bindings::Gecko_SnapshotAttrDashEquals(
self, self,
ns.atom_or_null(), ns.atom_or_null(),
local_name.as_ptr(), local_name.as_ptr(),
expected_value.as_ptr(), value.as_ptr(),
ignore_case, ignore_case,
), ),
AttrSelectorOperator::Prefix => bindings::Gecko_SnapshotAttrHasPrefix( AttrSelectorOperator::Prefix => bindings::Gecko_SnapshotAttrHasPrefix(
self, self,
ns.atom_or_null(), ns.atom_or_null(),
local_name.as_ptr(), local_name.as_ptr(),
expected_value.as_ptr(), value.as_ptr(),
ignore_case, ignore_case,
), ),
AttrSelectorOperator::Suffix => bindings::Gecko_SnapshotAttrHasSuffix( AttrSelectorOperator::Suffix => bindings::Gecko_SnapshotAttrHasSuffix(
self, self,
ns.atom_or_null(), ns.atom_or_null(),
local_name.as_ptr(), local_name.as_ptr(),
expected_value.as_ptr(), value.as_ptr(),
ignore_case, ignore_case,
), ),
AttrSelectorOperator::Substring => { AttrSelectorOperator::Substring => {
@ -147,7 +146,7 @@ impl GeckoElementSnapshot {
self, self,
ns.atom_or_null(), ns.atom_or_null(),
local_name.as_ptr(), local_name.as_ptr(),
expected_value.as_ptr(), value.as_ptr(),
ignore_case, ignore_case,
) )
}, },

View file

@ -1866,54 +1866,53 @@ impl<'le> ::selectors::Element for GeckoElement<'le> {
AttrSelectorOperation::WithValue { AttrSelectorOperation::WithValue {
operator, operator,
case_sensitivity, case_sensitivity,
expected_value, value,
} => { } => {
let ignore_case = match case_sensitivity { let ignore_case = match case_sensitivity {
CaseSensitivity::CaseSensitive => false, CaseSensitivity::CaseSensitive => false,
CaseSensitivity::AsciiCaseInsensitive => true, CaseSensitivity::AsciiCaseInsensitive => true,
}; };
// FIXME: case sensitivity for operators other than Equal
match operator { match operator {
AttrSelectorOperator::Equal => bindings::Gecko_AttrEquals( AttrSelectorOperator::Equal => bindings::Gecko_AttrEquals(
self.0, self.0,
ns.atom_or_null(), ns.atom_or_null(),
local_name.as_ptr(), local_name.as_ptr(),
expected_value.as_ptr(), value.as_ptr(),
ignore_case, ignore_case,
), ),
AttrSelectorOperator::Includes => bindings::Gecko_AttrIncludes( AttrSelectorOperator::Includes => bindings::Gecko_AttrIncludes(
self.0, self.0,
ns.atom_or_null(), ns.atom_or_null(),
local_name.as_ptr(), local_name.as_ptr(),
expected_value.as_ptr(), value.as_ptr(),
ignore_case, ignore_case,
), ),
AttrSelectorOperator::DashMatch => bindings::Gecko_AttrDashEquals( AttrSelectorOperator::DashMatch => bindings::Gecko_AttrDashEquals(
self.0, self.0,
ns.atom_or_null(), ns.atom_or_null(),
local_name.as_ptr(), local_name.as_ptr(),
expected_value.as_ptr(), value.as_ptr(),
ignore_case, ignore_case,
), ),
AttrSelectorOperator::Prefix => bindings::Gecko_AttrHasPrefix( AttrSelectorOperator::Prefix => bindings::Gecko_AttrHasPrefix(
self.0, self.0,
ns.atom_or_null(), ns.atom_or_null(),
local_name.as_ptr(), local_name.as_ptr(),
expected_value.as_ptr(), value.as_ptr(),
ignore_case, ignore_case,
), ),
AttrSelectorOperator::Suffix => bindings::Gecko_AttrHasSuffix( AttrSelectorOperator::Suffix => bindings::Gecko_AttrHasSuffix(
self.0, self.0,
ns.atom_or_null(), ns.atom_or_null(),
local_name.as_ptr(), local_name.as_ptr(),
expected_value.as_ptr(), value.as_ptr(),
ignore_case, ignore_case,
), ),
AttrSelectorOperator::Substring => bindings::Gecko_AttrHasSubstring( AttrSelectorOperator::Substring => bindings::Gecko_AttrHasSubstring(
self.0, self.0,
ns.atom_or_null(), ns.atom_or_null(),
local_name.as_ptr(), local_name.as_ptr(),
expected_value.as_ptr(), value.as_ptr(),
ignore_case, ignore_case,
), ),
} }