diff --git a/components/style/dom.rs b/components/style/dom.rs index 4e699017c64..7716a8a1e38 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -501,13 +501,20 @@ pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone + unsafe fn unset_animation_only_dirty_descendants(&self) { } - /// Clear all bits related to dirty descendant. + /// Clear all bits related describing the dirtiness of descendants. /// /// In Gecko, this corresponds to the regular dirty descendants bit, the /// animation-only dirty descendants bit, and the lazy frame construction /// descendants bit. unsafe fn clear_descendants_bits(&self) { self.unset_dirty_descendants(); } + /// Clear all element flags related to dirtiness. + /// + /// In Gecko, this corresponds to the regular dirty descendants bit, the + /// animation-only dirty descendants bit, the lazy frame construction bit, + /// and the lazy frame construction descendants bit. + unsafe fn clear_dirty_bits(&self) { self.unset_dirty_descendants(); } + /// Returns true if this element is a visited link. /// /// Servo doesn't support visited styles yet. @@ -718,28 +725,6 @@ pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone + -> bool; } -/// Trait abstracting over different kinds of dirty-descendants bits. -pub trait DescendantsBit { - /// Returns true if the Element has the bit. - fn has(el: E) -> bool; - /// Sets the bit on the Element. - unsafe fn set(el: E); -} - -/// Implementation of DescendantsBit for the regular dirty descendants bit. -pub struct DirtyDescendants; -impl DescendantsBit for DirtyDescendants { - fn has(el: E) -> bool { el.has_dirty_descendants() } - unsafe fn set(el: E) { el.set_dirty_descendants(); } -} - -/// Implementation of DescendantsBit for the animation-only dirty descendants bit. -pub struct AnimationOnlyDirtyDescendants; -impl DescendantsBit for AnimationOnlyDirtyDescendants { - fn has(el: E) -> bool { el.has_animation_only_dirty_descendants() } - unsafe fn set(el: E) { el.set_animation_only_dirty_descendants(); } -} - /// TNode and TElement aren't Send because we want to be careful and explicit /// about our parallel traversal. However, there are certain situations /// (including but not limited to the traversal) where we need to send DOM diff --git a/components/style/gecko/data.rs b/components/style/gecko/data.rs index 39cfddd3738..28857a3c010 100644 --- a/components/style/gecko/data.rs +++ b/components/style/gecko/data.rs @@ -152,7 +152,7 @@ impl PerDocumentStyleDataImpl { &mut self, guard: &SharedRwLockReadGuard, document_element: Option, - ) + ) -> bool where E: TElement, { diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 926533d9c90..4b7e5993334 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -61,6 +61,7 @@ use gecko_bindings::structs::ELEMENT_HAS_DIRTY_DESCENDANTS_FOR_SERVO; use gecko_bindings::structs::ELEMENT_HAS_SNAPSHOT; use gecko_bindings::structs::EffectCompositor_CascadeLevel as CascadeLevel; use gecko_bindings::structs::NODE_DESCENDANTS_NEED_FRAMES; +use gecko_bindings::structs::NODE_NEEDS_FRAME; use gecko_bindings::structs::nsChangeHint; use gecko_bindings::structs::nsIDocument_DocumentTheme as DocumentTheme; use gecko_bindings::structs::nsRestyleHint; @@ -493,6 +494,33 @@ impl<'le> GeckoElement<'le> { unsafe { Gecko_UnsetNodeFlags(self.as_node().0, flags) } } + /// Returns true if this element has descendants for lazy frame construction. + pub fn descendants_need_frames(&self) -> bool { + self.flags() & (NODE_DESCENDANTS_NEED_FRAMES as u32) != 0 + } + + /// Returns true if this element needs lazy frame construction. + pub fn needs_frame(&self) -> bool { + self.flags() & (NODE_NEEDS_FRAME as u32) != 0 + } + + /// Returns true if a traversal starting from this element requires a post-traversal. + pub fn needs_post_traversal(&self) -> bool { + debug!("needs_post_traversal: dd={}, aodd={}, lfcd={}, lfc={}, restyle={:?}", + self.has_dirty_descendants(), + self.has_animation_only_dirty_descendants(), + self.descendants_need_frames(), + self.needs_frame(), + self.borrow_data().unwrap().restyle); + + let has_flag = + self.flags() & (ELEMENT_HAS_DIRTY_DESCENDANTS_FOR_SERVO as u32 | + ELEMENT_HAS_ANIMATION_ONLY_DIRTY_DESCENDANTS_FOR_SERVO as u32 | + NODE_DESCENDANTS_NEED_FRAMES as u32 | + NODE_NEEDS_FRAME as u32) != 0; + has_flag || self.borrow_data().unwrap().restyle.contains_restyle_data() + } + /// Returns true if this element has a shadow root. fn has_shadow_root(&self) -> bool { self.get_extended_slots().map_or(false, |slots| !slots.mShadowRoot.mRawPtr.is_null()) @@ -1070,6 +1098,13 @@ impl<'le> TElement for GeckoElement<'le> { } #[inline] + unsafe fn clear_dirty_bits(&self) { + self.unset_flags(ELEMENT_HAS_DIRTY_DESCENDANTS_FOR_SERVO as u32 | + ELEMENT_HAS_ANIMATION_ONLY_DIRTY_DESCENDANTS_FOR_SERVO as u32 | + NODE_DESCENDANTS_NEED_FRAMES as u32 | + NODE_NEEDS_FRAME as u32) + } + fn is_visited_link(&self) -> bool { use element_state::IN_VISITED_STATE; self.get_state().intersects(IN_VISITED_STATE) diff --git a/components/style/invalidation/stylesheets.rs b/components/style/invalidation/stylesheets.rs index 71a1b29e5c0..3c071fc4a4d 100644 --- a/components/style/invalidation/stylesheets.rs +++ b/components/style/invalidation/stylesheets.rs @@ -117,13 +117,17 @@ impl StylesheetInvalidationSet { /// Clears the invalidation set, invalidating elements as needed if /// `document_element` is provided. - pub fn flush(&mut self, document_element: Option) + /// + /// Returns true if any invalidations ocurred. + pub fn flush(&mut self, document_element: Option) -> bool where E: TElement, { - if let Some(e) = document_element { - self.process_invalidations(e); - } + let have_invalidations = match document_element { + Some(e) => self.process_invalidations(e), + None => false, + }; self.clear(); + have_invalidations } /// Clears the invalidation set without processing. diff --git a/components/style/stylesheet_set.rs b/components/style/stylesheet_set.rs index 91e87cff239..8548ac53f01 100644 --- a/components/style/stylesheet_set.rs +++ b/components/style/stylesheet_set.rs @@ -182,25 +182,28 @@ where /// Flush the current set, unmarking it as dirty, and returns an iterator /// over the new stylesheet list. + /// + /// Returns true if any elements were invalidated. pub fn flush( &mut self, document_element: Option, - ) -> (StylesheetIterator, OriginSet) + ) -> (StylesheetIterator, OriginSet, bool) where E: TElement, { debug!("StylesheetSet::flush"); let mut origins = OriginSet::empty(); + let mut have_invalidations = false; for (data, origin) in self.invalidation_data.iter_mut_origins() { if data.dirty { - data.invalidations.flush(document_element); + have_invalidations |= data.invalidations.flush(document_element); data.dirty = false; origins |= origin; } } - (self.iter(), origins) + (self.iter(), origins, have_invalidations) } /// Flush stylesheets, but without running any of the invalidation passes. diff --git a/components/style/stylist.rs b/components/style/stylist.rs index dc6924c6c5b..2ea8d5313ac 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -481,19 +481,20 @@ impl Stylist { ua_sheets: Option<&UserAgentStylesheets>, extra_data: &mut PerOrigin, document_element: Option, - ) + ) -> bool where E: TElement, { if !self.stylesheets.has_changed() { - return; + return false; } let author_style_disabled = self.stylesheets.author_style_disabled(); - let (doc_stylesheets, origins_to_rebuild) = self.stylesheets.flush(document_element); + let (doc_stylesheets, origins_to_rebuild, have_invalidations) = + self.stylesheets.flush(document_element); if origins_to_rebuild.is_empty() { - return; + return have_invalidations; } self.num_rebuilds += 1; @@ -538,6 +539,8 @@ impl Stylist { extra_data, origins_to_rebuild, ); + + have_invalidations } /// Insert a given stylesheet before another stylesheet in the document. diff --git a/components/style/traversal.rs b/components/style/traversal.rs index 5379f0cdee8..2882d7d4d5f 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -185,10 +185,16 @@ pub trait DomTraversal : Sync { let should_traverse = Self::element_needs_traversal( root, flags, - data.map(|d| &*d), + data.as_mut().map(|d| &**d), parent_data.as_ref().map(|d| &**d) ); + // If we're not going to traverse at all, we may need to clear some state + // off the root (which would normally be done at the end of recalc_style_at). + if !should_traverse && data.is_some() { + clear_state_after_traversing(root, data.unwrap(), flags); + } + PreTraverseToken(should_traverse) } @@ -592,6 +598,31 @@ where ); } + // FIXME(bholley): Make these assertions pass for servo. + if cfg!(feature = "gecko") && cfg!(debug_assertions) && data.styles.is_display_none() { + debug_assert!(!element.has_dirty_descendants()); + debug_assert!(!element.has_animation_only_dirty_descendants()); + } + + debug_assert!(flags.for_animation_only() || + !flags.contains(ClearDirtyBits) || + !element.has_animation_only_dirty_descendants(), + "Should have cleared animation bits already"); + clear_state_after_traversing(element, data, flags); + + context.thread_local.end_element(element); +} + +fn clear_state_after_traversing( + element: E, + data: &mut ElementData, + flags: TraversalFlags +) +where + E: TElement, +{ + use traversal_flags::*; + // If we are in a forgetful traversal, drop the existing restyle // data here, since we won't need to perform a post-traversal to pick up // any change hints. @@ -599,32 +630,16 @@ where data.clear_restyle_flags_and_damage(); } - // Optionally clear the descendants bits. - if data.styles.is_display_none() { - // When this element is the root of a display:none subtree, we want to clear - // the bits even if the style didn't change (since, if the style did change, - // we'd have already cleared it above). - // - // This keeps the tree in a valid state without requiring the DOM to check - // display:none on the parent when inserting new children (which can be - // moderately expensive). Instead, DOM implementations can unconditionally - // set the dirty descendants bit on any styled parent, and let the traversal - // sort it out. - // - // Note that the NODE_DESCENDANTS_NEED_FRAMES bit should generally only be set - // when appending content beneath an element with a frame (i.e. not - // display:none), so clearing it here isn't strictly necessary, but good - // belt-and-suspenders. - unsafe { element.clear_descendants_bits(); } - } else if flags.for_animation_only() { - if flags.contains(ClearAnimationOnlyDirtyDescendants) { + // Clear dirty bits as appropriate. + if flags.for_animation_only() { + if flags.intersects(ClearDirtyBits | ClearAnimationOnlyDirtyDescendants) { unsafe { element.unset_animation_only_dirty_descendants(); } } - } else if flags.contains(ClearDirtyDescendants) { - unsafe { element.unset_dirty_descendants(); } + } else if flags.contains(ClearDirtyBits) { + // The animation traversal happens first, so we don't need to guard against + // clearing the animation bit on the regular traversal. + unsafe { element.clear_dirty_bits(); } } - - context.thread_local.end_element(element); } fn compute_style( @@ -855,6 +870,11 @@ where } } } - p.clear_descendants_bits(); + if p == root { + // Make sure not to clear NODE_NEEDS_FRAME on the root. + p.clear_descendants_bits(); + } else { + p.clear_dirty_bits(); + } } } diff --git a/components/style/traversal_flags.rs b/components/style/traversal_flags.rs index 341f21016a9..674c9a49631 100644 --- a/components/style/traversal_flags.rs +++ b/components/style/traversal_flags.rs @@ -27,8 +27,8 @@ bitflags! { /// Actively seeks out and clears change hints that may have been posted into /// the tree. Nonsensical without also passing Forgetful. const AggressivelyForgetful = 1 << 4, - /// Clears the dirty descendants bit in the subtree. - const ClearDirtyDescendants = 1 << 5, + /// Clears all the dirty bits on the elements traversed. + const ClearDirtyBits = 1 << 5, /// Clears the animation-only dirty descendants bit in the subtree. const ClearAnimationOnlyDirtyDescendants = 1 << 6, /// Allows the traversal to run in parallel if there are sufficient cores on @@ -67,7 +67,7 @@ pub fn assert_traversal_flags_match() { ServoTraversalFlags_UnstyledOnly => UnstyledOnly, ServoTraversalFlags_Forgetful => Forgetful, ServoTraversalFlags_AggressivelyForgetful => AggressivelyForgetful, - ServoTraversalFlags_ClearDirtyDescendants => ClearDirtyDescendants, + ServoTraversalFlags_ClearDirtyBits => ClearDirtyBits, ServoTraversalFlags_ClearAnimationOnlyDirtyDescendants => ClearAnimationOnlyDirtyDescendants, ServoTraversalFlags_ParallelTraversal => ParallelTraversal, diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 7388314ce0c..bdbf1d319d7 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -25,6 +25,7 @@ use style::gecko::restyle_damage::GeckoRestyleDamage; use style::gecko::selector_parser::PseudoElement; use style::gecko::traversal::RecalcStyleOnly; use style::gecko::wrapper::GeckoElement; +use style::gecko_bindings::bindings; use style::gecko_bindings::bindings::{RawGeckoElementBorrowed, RawGeckoElementBorrowedOrNull}; use style::gecko_bindings::bindings::{RawGeckoKeyframeListBorrowed, RawGeckoKeyframeListBorrowedMut}; use style::gecko_bindings::bindings::{RawServoDeclarationBlockBorrowed, RawServoDeclarationBlockStrong}; @@ -291,14 +292,14 @@ pub extern "C" fn Servo_TraverseSubtree(root: RawGeckoElementBorrowed, traversal_flags, unsafe { &*snapshots }); - debug!("Servo_TraverseSubtree complete (dd={}, aodd={}, restyle={:?})", + debug!("Servo_TraverseSubtree complete (dd={}, aodd={}, lfcd={}, lfc={}, restyle={:?})", element.has_dirty_descendants(), element.has_animation_only_dirty_descendants(), + element.descendants_need_frames(), + element.needs_frame(), element.borrow_data().unwrap().restyle); - element.has_dirty_descendants() || - element.has_animation_only_dirty_descendants() || - element.borrow_data().unwrap().restyle.contains_restyle_data() + element.needs_post_traversal() } /// Checks whether the rule tree has crossed its threshold for unused nodes, and @@ -826,6 +827,14 @@ pub extern "C" fn Servo_Element_GetPseudoComputedValues(element: RawGeckoElement .clone().into() } +#[no_mangle] +pub extern "C" fn Servo_Element_IsDisplayNone(element: RawGeckoElementBorrowed) -> bool +{ + let element = GeckoElement(element); + let data = element.borrow_data().expect("Invoking Servo_Element_IsDisplayNone on unstyled element"); + data.styles.is_display_none() +} + #[no_mangle] pub extern "C" fn Servo_StyleSheet_Empty(mode: SheetParsingMode) -> RawServoStyleSheetContentsStrong { let global_style_data = &*GLOBAL_STYLE_DATA; @@ -991,7 +1000,12 @@ pub extern "C" fn Servo_StyleSet_FlushStyleSheets( let guard = global_style_data.shared_lock.read(); let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut(); let doc_element = doc_element.map(GeckoElement); - data.flush_stylesheets(&guard, doc_element); + let have_invalidations = data.flush_stylesheets(&guard, doc_element); + if have_invalidations && doc_element.is_some() { + // The invalidation machinery propagates the bits up, but we still + // need to tell the gecko restyle root machinery about it. + unsafe { bindings::Gecko_NoteDirtyElement(doc_element.unwrap().0); } + } } #[no_mangle]