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/layout/wrapper.rs

@@ -223,7 +223,8 @@ impl<'ln> LayoutNode<'ln> {
     }
 
     fn debug_str(self) -> String {
-        format!("{}: dirty={}", self.type_id(), self.is_dirty())
+        format!("{}: changed={} dirty={} dirty_descendants={}",
+                self.type_id(), self.has_changed(), self.is_dirty(), self.has_dirty_descendants())
     }
 
     pub fn flow_debug_id(self) -> uint {

components/script/dom/document.rs

@@ -176,6 +176,7 @@ pub trait DocumentHelpers<'a> {
     fn set_last_modified(self, value: DOMString);
     fn set_encoding_name(self, name: DOMString);
     fn content_changed(self, node: JSRef<Node>);
+    fn content_and_heritage_changed(self, node: JSRef<Node>);
     fn reflow(self);
     fn wait_until_safe_to_modify_dom(self);
     fn unregister_named_element(self, to_unregister: JSRef<Element>, id: Atom);
@@ -226,10 +227,17 @@ impl<'a> DocumentHelpers<'a> for JSRef<'a, Document> {
     }
 
     fn content_changed(self, node: JSRef<Node>) {
+        debug!("content_changed on {}", node.debug_str());
         node.dirty();
         self.reflow();
     }
 
+    fn content_and_heritage_changed(self, node: JSRef<Node>) {
+        debug!("content_and_heritage_changed on {}", node.debug_str());
+        node.force_dirty_ancestors();
+        self.reflow();
+    }
+
     fn reflow(self) {
         self.window.root().reflow();
     }

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<JSRef<Node>>, parent: JSRef<Node>) {
-
         // Step 1.
         match node {
             Some(node) => {