From d3e4d293687d51504f68323228bfbab972aa0f2f Mon Sep 17 00:00:00 2001 From: Clark Gaebel Date: Wed, 3 Dec 2014 11:17:38 -0800 Subject: Fixed #4170 - Incremental reflow wasn't being aggressive enough when nodes get reparented. When inserting a node that was already dirtied, the dirtying logic would short circuit: "This node is already dirty? Great! Then its parents must be HAS_DIRTY_DESCENDANTS, too! Let's skip that step." This isn't appropriate when nodes move around the tree. In that case, the node may be marked HAS_CHANGED, but ancestors may not yet have the HAS_DIRTY_DESCENDANTS flag set. This patch adds a `content_and_heritage_changed` hook in the document, to deal with these cases appropriately. --- components/script/dom/node.rs | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) (limited to 'components/script/dom/node.rs') diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index 2c02ebc9b1e..5ad856aa75c 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -282,7 +282,7 @@ impl<'a> PrivateNodeHelpers for JSRef<'a, Node> { let parent = self.parent_node().root(); parent.map(|parent| vtable_for(&*parent).child_inserted(self)); - document.content_changed(self); + document.content_and_heritage_changed(self); } // http://dom.spec.whatwg.org/#node-is-removed @@ -436,6 +436,16 @@ pub trait NodeHelpers<'a> { /// descendants as `IS_DIRTY`. fn dirty(self); + /// Similar to `dirty`, but will always walk the ancestors to mark them dirty, + /// too. This is useful when a node is reparented. The node will frequently + /// already be marked as `changed` to skip double-dirties, but the ancestors + /// still need to be marked as `HAS_DIRTY_DESCENDANTS`. + /// + /// See #4170 + fn force_dirty_ancestors(self); + + fn dirty_impl(self, force_ancestors: bool); + fn dump(self); fn dump_indent(self, indent: uint); fn debug_str(self) -> String; @@ -616,11 +626,19 @@ impl<'a> NodeHelpers<'a> for JSRef<'a, Node> { self.set_flag(HAS_DIRTY_DESCENDANTS, state) } + fn force_dirty_ancestors(self) { + self.dirty_impl(true) + } + fn dirty(self) { + self.dirty_impl(false) + } + + fn dirty_impl(self, force_ancestors: bool) { // 1. Dirty self. self.set_has_changed(true); - if self.get_is_dirty() { + if self.get_is_dirty() && !force_ancestors { return } @@ -654,7 +672,7 @@ impl<'a> NodeHelpers<'a> for JSRef<'a, Node> { // 4. Dirty ancestors. for ancestor in self.ancestors() { - if ancestor.get_has_dirty_descendants() { break } + if !force_ancestors && ancestor.get_has_dirty_descendants() { break } ancestor.set_has_dirty_descendants(true); } } @@ -1384,7 +1402,6 @@ impl Node { // http://dom.spec.whatwg.org/#concept-node-replace-all fn replace_all(node: Option>, parent: JSRef) { - // Step 1. match node { Some(node) => { -- cgit v1.2.3