mirror of
https://github.com/servo/servo.git
synced 2025-08-03 20:50:07 +01:00
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.
This commit is contained in:
parent
873ca6cadd
commit
d3e4d29368
3 changed files with 31 additions and 5 deletions
|
@ -223,7 +223,8 @@ impl<'ln> LayoutNode<'ln> {
|
||||||
}
|
}
|
||||||
|
|
||||||
fn debug_str(self) -> String {
|
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 {
|
pub fn flow_debug_id(self) -> uint {
|
||||||
|
|
|
@ -176,6 +176,7 @@ pub trait DocumentHelpers<'a> {
|
||||||
fn set_last_modified(self, value: DOMString);
|
fn set_last_modified(self, value: DOMString);
|
||||||
fn set_encoding_name(self, name: DOMString);
|
fn set_encoding_name(self, name: DOMString);
|
||||||
fn content_changed(self, node: JSRef<Node>);
|
fn content_changed(self, node: JSRef<Node>);
|
||||||
|
fn content_and_heritage_changed(self, node: JSRef<Node>);
|
||||||
fn reflow(self);
|
fn reflow(self);
|
||||||
fn wait_until_safe_to_modify_dom(self);
|
fn wait_until_safe_to_modify_dom(self);
|
||||||
fn unregister_named_element(self, to_unregister: JSRef<Element>, id: Atom);
|
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>) {
|
fn content_changed(self, node: JSRef<Node>) {
|
||||||
|
debug!("content_changed on {}", node.debug_str());
|
||||||
node.dirty();
|
node.dirty();
|
||||||
self.reflow();
|
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) {
|
fn reflow(self) {
|
||||||
self.window.root().reflow();
|
self.window.root().reflow();
|
||||||
}
|
}
|
||||||
|
|
|
@ -282,7 +282,7 @@ impl<'a> PrivateNodeHelpers for JSRef<'a, Node> {
|
||||||
let parent = self.parent_node().root();
|
let parent = self.parent_node().root();
|
||||||
parent.map(|parent| vtable_for(&*parent).child_inserted(self));
|
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
|
// http://dom.spec.whatwg.org/#node-is-removed
|
||||||
|
@ -436,6 +436,16 @@ pub trait NodeHelpers<'a> {
|
||||||
/// descendants as `IS_DIRTY`.
|
/// descendants as `IS_DIRTY`.
|
||||||
fn dirty(self);
|
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(self);
|
||||||
fn dump_indent(self, indent: uint);
|
fn dump_indent(self, indent: uint);
|
||||||
fn debug_str(self) -> String;
|
fn debug_str(self) -> String;
|
||||||
|
@ -616,11 +626,19 @@ impl<'a> NodeHelpers<'a> for JSRef<'a, Node> {
|
||||||
self.set_flag(HAS_DIRTY_DESCENDANTS, state)
|
self.set_flag(HAS_DIRTY_DESCENDANTS, state)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn force_dirty_ancestors(self) {
|
||||||
|
self.dirty_impl(true)
|
||||||
|
}
|
||||||
|
|
||||||
fn dirty(self) {
|
fn dirty(self) {
|
||||||
|
self.dirty_impl(false)
|
||||||
|
}
|
||||||
|
|
||||||
|
fn dirty_impl(self, force_ancestors: bool) {
|
||||||
// 1. Dirty self.
|
// 1. Dirty self.
|
||||||
self.set_has_changed(true);
|
self.set_has_changed(true);
|
||||||
|
|
||||||
if self.get_is_dirty() {
|
if self.get_is_dirty() && !force_ancestors {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -654,7 +672,7 @@ impl<'a> NodeHelpers<'a> for JSRef<'a, Node> {
|
||||||
|
|
||||||
// 4. Dirty ancestors.
|
// 4. Dirty ancestors.
|
||||||
for ancestor in self.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);
|
ancestor.set_has_dirty_descendants(true);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -1384,7 +1402,6 @@ impl Node {
|
||||||
|
|
||||||
// http://dom.spec.whatwg.org/#concept-node-replace-all
|
// http://dom.spec.whatwg.org/#concept-node-replace-all
|
||||||
fn replace_all(node: Option<JSRef<Node>>, parent: JSRef<Node>) {
|
fn replace_all(node: Option<JSRef<Node>>, parent: JSRef<Node>) {
|
||||||
|
|
||||||
// Step 1.
|
// Step 1.
|
||||||
match node {
|
match node {
|
||||||
Some(node) => {
|
Some(node) => {
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue