Skip to content

Commit

Permalink
Bug 1818151 - Record attribute dependencies within the selector list …
Browse files Browse the repository at this point in the history
…of :nth-child(... of <selector list>) r=emilio

There are separate filters for IDs, classes, attribute local names, and
element state.

Also, we invalidate siblings of elements matched against the selector
list of :nth-child(... of <selector list>) by marking matched elements
with NODE_HAS_SLOW_SELECTOR_NTH_OF.

The only remaining invalidation case invalidation case is
`:nth-child(An+B of :has())` (bug 1818155), which should not block
shipping `layout.css.nth-child-of.enabled`, because :has(...) is still
being implemented (bug 418039).

Depends on D172352

Differential Revision: https://phabricator.services.mozilla.com/D171936
  • Loading branch information
zrhoffman committed Mar 15, 2023
1 parent 1a899fc commit 2d8b2e6
Show file tree
Hide file tree
Showing 37 changed files with 830 additions and 51 deletions.
29 changes: 18 additions & 11 deletions dom/base/nsINode.h
Original file line number Diff line number Diff line change
Expand Up @@ -163,41 +163,48 @@ enum {
// child's later siblings must also be restyled.
NODE_HAS_SLOW_SELECTOR_LATER_SIBLINGS = NODE_FLAG_BIT(9),

// A child of this node might be matched by :nth-child(.. of <selector>) or
// :nth-last-child(.. of <selector>). If a DOM mutation may have caused the
// selector to either match or no longer match that child, the child's
// siblings are restyled.
NODE_HAS_SLOW_SELECTOR_NTH_OF = NODE_FLAG_BIT(10),

NODE_ALL_SELECTOR_FLAGS = NODE_HAS_EMPTY_SELECTOR | NODE_HAS_SLOW_SELECTOR |
NODE_HAS_EDGE_CHILD_SELECTOR |
NODE_HAS_SLOW_SELECTOR_LATER_SIBLINGS,
NODE_HAS_SLOW_SELECTOR_LATER_SIBLINGS |
NODE_HAS_SLOW_SELECTOR_NTH_OF,

// This node needs to go through frame construction to get a frame (or
// undisplayed entry).
NODE_NEEDS_FRAME = NODE_FLAG_BIT(10),
NODE_NEEDS_FRAME = NODE_FLAG_BIT(11),

// At least one descendant in the flattened tree has NODE_NEEDS_FRAME set.
// This should be set on every node on the flattened tree path between the
// node(s) with NODE_NEEDS_FRAME and the root content.
NODE_DESCENDANTS_NEED_FRAMES = NODE_FLAG_BIT(11),
NODE_DESCENDANTS_NEED_FRAMES = NODE_FLAG_BIT(12),

// Set if the node has the accesskey attribute set.
NODE_HAS_ACCESSKEY = NODE_FLAG_BIT(12),
NODE_HAS_ACCESSKEY = NODE_FLAG_BIT(13),

// Set if the node has right-to-left directionality
NODE_HAS_DIRECTION_RTL = NODE_FLAG_BIT(13),
NODE_HAS_DIRECTION_RTL = NODE_FLAG_BIT(14),

// Set if the node has left-to-right directionality
NODE_HAS_DIRECTION_LTR = NODE_FLAG_BIT(14),
NODE_HAS_DIRECTION_LTR = NODE_FLAG_BIT(15),

NODE_ALL_DIRECTION_FLAGS = NODE_HAS_DIRECTION_LTR | NODE_HAS_DIRECTION_RTL,

NODE_HAS_BEEN_IN_UA_WIDGET = NODE_FLAG_BIT(15),
NODE_HAS_BEEN_IN_UA_WIDGET = NODE_FLAG_BIT(16),

// Set if the node has a nonce value and a header delivered CSP.
NODE_HAS_NONCE_AND_HEADER_CSP = NODE_FLAG_BIT(16),
NODE_HAS_NONCE_AND_HEADER_CSP = NODE_FLAG_BIT(17),

NODE_KEEPS_DOMARENA = NODE_FLAG_BIT(17),
NODE_KEEPS_DOMARENA = NODE_FLAG_BIT(18),

NODE_MAY_HAVE_ELEMENT_CHILDREN = NODE_FLAG_BIT(18),
NODE_MAY_HAVE_ELEMENT_CHILDREN = NODE_FLAG_BIT(19),

// Remaining bits are node type specific.
NODE_TYPE_SPECIFIC_BITS_OFFSET = 19
NODE_TYPE_SPECIFIC_BITS_OFFSET = 20
};

// Make sure we have space for our bits
Expand Down
72 changes: 72 additions & 0 deletions layout/base/RestyleManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3362,6 +3362,23 @@ void RestyleManager::ElementStateChanged(Element* aElement,
ServoElementSnapshot& snapshot = SnapshotFor(*aElement);
ElementState previousState = aElement->StyleState() ^ aChangedBits;
snapshot.AddState(previousState);

MaybeRestyleForNthOfState(*StyleSet(), aElement, aChangedBits);
}

void RestyleManager::MaybeRestyleForNthOfState(ServoStyleSet& aStyleSet,
Element* aChild,
ElementState aChangedBits) {
const auto* parentNode = aChild->GetParentNode();
MOZ_ASSERT(parentNode);
const auto parentFlags = parentNode->GetFlags();
if (!(parentFlags & NODE_HAS_SLOW_SELECTOR_NTH_OF)) {
return;
}

if (aStyleSet.HasNthOfStateDependency(*aChild, aChangedBits)) {
RestyleSiblings(aChild, parentFlags);
}
}

static inline bool AttributeInfluencesOtherPseudoClassState(
Expand Down Expand Up @@ -3492,6 +3509,8 @@ void RestyleManager::AttributeChanged(Element* aElement, int32_t aNameSpaceID,

changeHint |= aElement->GetAttributeChangeHint(aAttribute, aModType);

MaybeRestyleForNthOfAttribute(aElement, aAttribute, aOldValue);

if (aAttribute == nsGkAtoms::style) {
restyleHint |= RestyleHint::RESTYLE_STYLE_ATTRIBUTE;
} else if (AttributeChangeRequiresSubtreeRestyle(*aElement, aAttribute)) {
Expand Down Expand Up @@ -3539,6 +3558,59 @@ void RestyleManager::AttributeChanged(Element* aElement, int32_t aNameSpaceID,
}
}

void RestyleManager::RestyleSiblings(
Element* aChild, nsBaseContentList::FlagsType aParentFlags) {
const DebugOnly<nsINode*> parentNode = aChild->GetParentNode();
MOZ_ASSERT(parentNode->IsElement() || parentNode->IsShadowRoot());

DebugOnly<bool> restyledSiblings = false;
// NODE_HAS_SLOW_SELECTOR typically indicates restyling the parent, but since
// we know we're restyling for :nth-last-child(.. of <selector>), we can
// restyle only previous siblings without under-invalidating.
if (aParentFlags & NODE_HAS_SLOW_SELECTOR) {
RestylePreviousSiblings(aChild->GetPreviousSibling());
restyledSiblings = true;
}
if (aParentFlags & NODE_HAS_SLOW_SELECTOR_LATER_SIBLINGS) {
RestyleSiblingsStartingWith(aChild->GetNextSibling());
restyledSiblings = true;
}
MOZ_ASSERT(restyledSiblings,
"How can we restyle siblings without a slow selector flag?");
}

void RestyleManager::MaybeRestyleForNthOfAttribute(
Element* aChild, nsAtom* aAttribute, const nsAttrValue* aOldValue) {
const auto* parentNode = aChild->GetParentNode();
MOZ_ASSERT(parentNode);
const auto parentFlags = parentNode->GetFlags();
if (!(parentFlags & NODE_HAS_SLOW_SELECTOR_NTH_OF)) {
return;
}
if (!aChild->HasServoData()) {
return;
}

bool mightHaveNthOfDependency;
auto& styleSet = *StyleSet();
if (aAttribute == nsGkAtoms::id) {
auto* const oldAtom = aOldValue->Type() == nsAttrValue::eAtom
? aOldValue->GetAtomValue()
: nullptr;
mightHaveNthOfDependency =
styleSet.MightHaveNthOfIDDependency(*aChild, oldAtom, aChild->GetID());
} else if (aAttribute == nsGkAtoms::_class) {
mightHaveNthOfDependency = styleSet.MightHaveNthOfClassDependency(*aChild);
} else {
mightHaveNthOfDependency =
styleSet.MightHaveNthOfAttributeDependency(*aChild, aAttribute);
}

if (mightHaveNthOfDependency) {
RestyleSiblings(aChild, parentFlags);
}
}

void RestyleManager::ReparentComputedStyleForFirstLine(nsIFrame* aFrame) {
// This is only called when moving frames in or out of the first-line
// pseudo-element (or one of its descendants). We can't say much about
Expand Down
23 changes: 23 additions & 0 deletions layout/base/RestyleManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -360,13 +360,36 @@ class RestyleManager {
void ProcessAllPendingAttributeAndStateInvalidations();

void ElementStateChanged(Element*, dom::ElementState);

/**
* Posts restyle hints for siblings of an element and their descendants if the
* element's parent has NODE_HAS_SLOW_SELECTOR_NTH_OF and the element has a
* relevant state dependency.
*/
void MaybeRestyleForNthOfState(ServoStyleSet& aStyleSet, dom::Element* aChild,
dom::ElementState aChangedBits);

void AttributeWillChange(Element* aElement, int32_t aNameSpaceID,
nsAtom* aAttribute, int32_t aModType);
void ClassAttributeWillBeChangedBySMIL(dom::Element* aElement);
void AttributeChanged(dom::Element* aElement, int32_t aNameSpaceID,
nsAtom* aAttribute, int32_t aModType,
const nsAttrValue* aOldValue);

/**
* Restyle an element's previous and/or next siblings.
*/
void RestyleSiblings(dom::Element* aChild,
nsBaseContentList::FlagsType aParentFlags);

/**
* Posts restyle hints for siblings of an element and their descendants if the
* element's parent has NODE_HAS_SLOW_SELECTOR_NTH_OF and the element has a
* relevant attribute dependency.
*/
void MaybeRestyleForNthOfAttribute(dom::Element* aChild, nsAtom* aAttribute,
const nsAttrValue* aOldValue);

// This is only used to reparent things when moving them in/out of the
// ::first-line.
void ReparentComputedStyleForFirstLine(nsIFrame*);
Expand Down
24 changes: 24 additions & 0 deletions layout/style/ServoStyleSet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1395,12 +1395,36 @@ bool ServoStyleSet::MightHaveAttributeDependency(const Element& aElement,
aAttribute);
}

bool ServoStyleSet::MightHaveNthOfIDDependency(const Element& aElement,
nsAtom* aOldID,
nsAtom* aNewID) const {
return Servo_StyleSet_MightHaveNthOfIDDependency(mRawSet.get(), &aElement,
aOldID, aNewID);
}

bool ServoStyleSet::MightHaveNthOfClassDependency(const Element& aElement) {
return Servo_StyleSet_MightHaveNthOfClassDependency(mRawSet.get(), &aElement,
&Snapshots());
}

bool ServoStyleSet::MightHaveNthOfAttributeDependency(
const Element& aElement, nsAtom* aAttribute) const {
return Servo_StyleSet_MightHaveNthOfAttributeDependency(
mRawSet.get(), &aElement, aAttribute);
}

bool ServoStyleSet::HasStateDependency(const Element& aElement,
dom::ElementState aState) const {
return Servo_StyleSet_HasStateDependency(mRawSet.get(), &aElement,
aState.GetInternalValue());
}

bool ServoStyleSet::HasNthOfStateDependency(const Element& aElement,
dom::ElementState aState) const {
return Servo_StyleSet_HasNthOfStateDependency(mRawSet.get(), &aElement,
aState.GetInternalValue());
}

bool ServoStyleSet::HasDocumentStateDependency(
dom::DocumentState aState) const {
return Servo_StyleSet_HasDocumentStateDependency(mRawSet.get(),
Expand Down
26 changes: 26 additions & 0 deletions layout/style/ServoStyleSet.h
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,26 @@ class ServoStyleSet {
bool MightHaveAttributeDependency(const dom::Element&,
nsAtom* aAttribute) const;

/**
* Returns true if a modification to an attribute with the specified local
* name might require us to restyle the element's siblings.
*/
bool MightHaveNthOfAttributeDependency(const dom::Element&,
nsAtom* aAttribute) const;

/**
* Returns true if a modification to a class might require us to restyle the
* element's siblings.
*/
bool MightHaveNthOfClassDependency(const dom::Element&);

/**
* Returns true if a modification to an ID might require us to restyle the
* element's siblings.
*/
bool MightHaveNthOfIDDependency(const dom::Element&, nsAtom* aOldID,
nsAtom* aNewID) const;

/**
* Returns true if a change in event state on an element might require
* us to restyle the element.
Expand All @@ -471,6 +491,12 @@ class ServoStyleSet {
*/
bool HasStateDependency(const dom::Element&, dom::ElementState) const;

/**
* Returns true if a change in event state on an element might require
* us to restyle the element's siblings.
*/
bool HasNthOfStateDependency(const dom::Element&, dom::ElementState) const;

/**
* Returns true if a change in document state might require us to restyle the
* document.
Expand Down
23 changes: 19 additions & 4 deletions servo/components/selectors/matching.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,24 @@ bitflags! {
/// :first-of-type, or :nth-of-type.
const HAS_SLOW_SELECTOR_LATER_SIBLINGS = 1 << 1;

/// When a DOM mutation occurs on a child that might be matched by
/// :nth-last-child(.. of <selector list>), earlier children must be
/// restyled, and HAS_SLOW_SELECTOR will be set (which normally
/// indicates that all children will be restyled).
///
/// Similarly, when a DOM mutation occurs on a child that might be
/// matched by :nth-child(.. of <selector list>), later children must be
/// restyled, and HAS_SLOW_SELECTOR_LATER_SIBLINGS will be set.
const HAS_SLOW_SELECTOR_NTH_OF = 1 << 2;

/// When a child is added or removed from the parent, the first and
/// last children must be restyled, because they may match :first-child,
/// :last-child, or :only-child.
const HAS_EDGE_CHILD_SELECTOR = 1 << 2;
const HAS_EDGE_CHILD_SELECTOR = 1 << 3;

/// The element has an empty selector, so when a child is appended we
/// might need to restyle the parent completely.
const HAS_EMPTY_SELECTOR = 1 << 3;
const HAS_EMPTY_SELECTOR = 1 << 4;
}
}

Expand All @@ -56,6 +66,7 @@ impl ElementSelectorFlags {
pub fn for_parent(self) -> ElementSelectorFlags {
self & (ElementSelectorFlags::HAS_SLOW_SELECTOR |
ElementSelectorFlags::HAS_SLOW_SELECTOR_LATER_SIBLINGS |
ElementSelectorFlags::HAS_SLOW_SELECTOR_NTH_OF |
ElementSelectorFlags::HAS_EDGE_CHILD_SELECTOR)
}
}
Expand Down Expand Up @@ -936,13 +947,17 @@ where
let is_edge_child_selector = a == 0 && b == 1 && !is_of_type && selectors.is_empty();

if context.needs_selector_flags() {
element.apply_selector_flags(if is_edge_child_selector {
let mut flags = if is_edge_child_selector {
ElementSelectorFlags::HAS_EDGE_CHILD_SELECTOR
} else if is_from_end {
ElementSelectorFlags::HAS_SLOW_SELECTOR
} else {
ElementSelectorFlags::HAS_SLOW_SELECTOR_LATER_SIBLINGS
});
};
if !selectors.is_empty() {
flags |= ElementSelectorFlags::HAS_SLOW_SELECTOR_NTH_OF;
}
element.apply_selector_flags(flags);
}

if !selectors.is_empty() && !list_matches_complex_selector(selectors, element, context) {
Expand Down
8 changes: 5 additions & 3 deletions servo/components/selectors/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::builder::{
};
use crate::context::QuirksMode;
use crate::sink::Push;
use crate::visitor::SelectorListKind;
pub use crate::visitor::SelectorVisitor;
use cssparser::parse_nth;
use cssparser::{BasicParseError, BasicParseErrorKind, ParseError, ParseErrorKind};
Expand Down Expand Up @@ -1608,14 +1609,15 @@ impl<Impl: SelectorImpl> Component<Impl> {
return false;
}
},

Negation(ref list) | Is(ref list) | Where(ref list) => {
if !visitor.visit_selector_list(&list) {
let list_kind = SelectorListKind::from_component(self);
debug_assert!(!list_kind.is_empty());
if !visitor.visit_selector_list(list_kind, &list) {
return false;
}
},
NthOf(ref nth_of_data) => {
if !visitor.visit_selector_list(nth_of_data.selectors()) {
if !visitor.visit_selector_list(SelectorListKind::NTH_OF, nth_of_data.selectors()) {
return false;
}
},
Expand Down
Loading

0 comments on commit 2d8b2e6

Please sign in to comment.