script: Improve dirty propagation and fix script-layout synchronization.

This fixes race conditions whereby layout and script could be running
simultaneously.

components/script/dom/document.rs

@@ -49,7 +49,7 @@ use dom::mouseevent::MouseEvent;
 use dom::keyboardevent::KeyboardEvent;
 use dom::messageevent::MessageEvent;
 use dom::node::{Node, ElementNodeTypeId, DocumentNodeTypeId, NodeHelpers};
-use dom::node::{CloneChildren, DoNotCloneChildren};
+use dom::node::{CloneChildren, DoNotCloneChildren, NodeDamage, OtherNodeDamage};
 use dom::nodelist::NodeList;
 use dom::text::Text;
 use dom::processinginstruction::ProcessingInstruction;
@@ -176,10 +176,8 @@ pub trait DocumentHelpers<'a> {
     fn set_quirks_mode(self, mode: QuirksMode);
     fn set_last_modified(self, value: DOMString);
     fn set_encoding_name(self, name: DOMString);
-    fn content_changed(self, node: JSRef<Node>);
+    fn content_changed(self, node: JSRef<Node>, damage: NodeDamage);
-    fn content_and_heritage_changed(self, node: JSRef<Node>);
+    fn content_and_heritage_changed(self, node: JSRef<Node>, damage: NodeDamage);
-    fn reflow(self);
-    fn wait_until_safe_to_modify_dom(self);
     fn unregister_named_element(self, to_unregister: JSRef<Element>, id: Atom);
     fn register_named_element(self, element: JSRef<Element>, id: Atom);
     fn load_anchor_href(self, href: DOMString);
@@ -190,6 +188,7 @@ pub trait DocumentHelpers<'a> {
     fn request_focus(self, elem: JSRef<Element>);
     fn commit_focus_transaction(self);
     fn send_title_to_compositor(self);
+    fn dirty_all_nodes(self);
 }
 
 impl<'a> DocumentHelpers<'a> for JSRef<'a, Document> {
@@ -228,24 +227,14 @@ impl<'a> DocumentHelpers<'a> for JSRef<'a, Document> {
         *self.encoding_name.borrow_mut() = name;
     }
 
-    fn content_changed(self, node: JSRef<Node>) {
+    fn content_changed(self, node: JSRef<Node>, damage: NodeDamage) {
-        debug!("content_changed on {}", node.debug_str());
+        node.dirty(damage);
-        node.dirty();
-        self.reflow();
     }
 
-    fn content_and_heritage_changed(self, node: JSRef<Node>) {
+    fn content_and_heritage_changed(self, node: JSRef<Node>, damage: NodeDamage) {
         debug!("content_and_heritage_changed on {}", node.debug_str());
-        node.force_dirty_ancestors();
+        node.force_dirty_ancestors(damage);
-        self.reflow();
+        node.dirty(damage);
-    }
-
-    fn reflow(self) {
-        self.window.root().reflow();
-    }
-
-    fn wait_until_safe_to_modify_dom(self) {
-        self.window.root().wait_until_safe_to_modify_dom();
     }
 
     /// Remove any existing association between the provided id and any elements in this document.
@@ -376,6 +365,13 @@ impl<'a> DocumentHelpers<'a> for JSRef<'a, Document> {
         let window = self.window().root();
         window.page().send_title_to_compositor();
     }
+
+    fn dirty_all_nodes(self) {
+        let root: JSRef<Node> = NodeCast::from_ref(self);
+        for node in root.traverse_preorder() {
+            node.dirty(OtherNodeDamage)
+        }
+    }
 }
 
 #[deriving(PartialEq)]

components/script/dom/element.rs

@@ -33,8 +33,9 @@ use dom::htmlcollection::HTMLCollection;
 use dom::htmlinputelement::{HTMLInputElement, RawLayoutHTMLInputElementHelpers};
 use dom::htmlserializer::serialize;
 use dom::htmltablecellelement::{HTMLTableCellElement, HTMLTableCellElementHelpers};
-use dom::node::{ElementNodeTypeId, Node, NodeHelpers, NodeIterator, document_from_node, CLICK_IN_PROGRESS};
+use dom::node::{CLICK_IN_PROGRESS, ElementNodeTypeId, Node, NodeHelpers, NodeIterator};
-use dom::node::{window_from_node, LayoutNodeHelpers};
+use dom::node::{document_from_node, window_from_node, LayoutNodeHelpers, NodeStyleDamaged};
+use dom::node::{OtherNodeDamage};
 use dom::nodelist::NodeList;
 use dom::virtualmethods::{VirtualMethods, vtable_for};
 use devtools_traits::AttrInfo;
@@ -441,7 +442,6 @@ pub trait AttributeHandlers {
                        value: DOMString) -> AttrValue;
 
     fn remove_attribute(self, namespace: Namespace, name: &str);
-    fn notify_content_changed(self);
     fn has_class(&self, name: &Atom) -> bool;
 
     fn set_atomic_attribute(self, name: &Atom, value: DOMString);
@@ -500,9 +500,6 @@ impl<'a> AttributeHandlers for JSRef<'a, Element> {
         assert!(name.as_slice() == name.as_slice().to_ascii_lower().as_slice());
         assert!(!name.as_slice().contains(":"));
 
-        let node: JSRef<Node> = NodeCast::from_ref(self);
-        node.wait_until_safe_to_modify_dom();
-
         self.do_set_attribute(name.clone(), value, name.clone(),
             ns!(""), None, |attr| *attr.local_name() == *name);
     }
@@ -548,27 +545,25 @@ impl<'a> AttributeHandlers for JSRef<'a, Element> {
         match idx {
             None => (),
             Some(idx) => {
-                let node: JSRef<Node> = NodeCast::from_ref(self);
-                node.wait_until_safe_to_modify_dom();
-
                 if namespace == ns!("") {
                     let attr = (*self.attrs.borrow())[idx].root();
                     vtable_for(&NodeCast::from_ref(self)).before_remove_attr(*attr);
                 }
 
                 self.attrs.borrow_mut().remove(idx);
-                self.notify_content_changed();
-            }
-        };
-    }
 
-    fn notify_content_changed(self) {
                 let node: JSRef<Node> = NodeCast::from_ref(self);
                 if node.is_in_doc() {
                     let document = document_from_node(self).root();
-            document.content_changed(node);
+                    if local_name == atom!("style") {
+                        document.content_changed(node, NodeStyleDamaged);
+                    } else {
+                        document.content_changed(node, OtherNodeDamage);
                     }
                 }
+            }
+        };
+    }
 
     fn has_class(&self, name: &Atom) -> bool {
         self.get_attribute(ns!(""), &atom!("class")).root().map(|attr| {
@@ -755,11 +750,6 @@ impl<'a> ElementMethods for JSRef<'a, Element> {
     fn SetAttribute(self,
                     name: DOMString,
                     value: DOMString) -> ErrorResult {
-        {
-            let node: JSRef<Node> = NodeCast::from_ref(self);
-            node.wait_until_safe_to_modify_dom();
-        }
-
         // Step 1.
         match xml_name_type(name.as_slice()) {
             InvalidXMLName => return Err(InvalidCharacter),
@@ -787,11 +777,6 @@ impl<'a> ElementMethods for JSRef<'a, Element> {
                       namespace_url: Option<DOMString>,
                       name: DOMString,
                       value: DOMString) -> ErrorResult {
-        {
-            let node: JSRef<Node> = NodeCast::from_ref(self);
-            node.wait_until_safe_to_modify_dom();
-        }
-
         // Step 1.
         let namespace = namespace::from_domstring(namespace_url);
 
@@ -999,25 +984,48 @@ impl<'a> VirtualMethods for JSRef<'a, Element> {
 
         match attr.local_name() {
             &atom!("style") => {
+                // Modifying the `style` attribute might change style.
+                let node: JSRef<Node> = NodeCast::from_ref(*self);
                 let doc = document_from_node(*self).root();
                 let base_url = doc.url().clone();
                 let value = attr.value();
                 let style = Some(style::parse_style_attribute(value.as_slice(), &base_url));
                 *self.style_attribute.borrow_mut() = style;
+
+                if node.is_in_doc() {
+                    doc.content_changed(node, NodeStyleDamaged);
+                }
+            }
+            &atom!("class") => {
+                // Modifying a class can change style.
+                let node: JSRef<Node> = NodeCast::from_ref(*self);
+                if node.is_in_doc() {
+                    let document = document_from_node(*self).root();
+                    document.content_changed(node, NodeStyleDamaged);
+                }
             }
             &atom!("id") => {
+                // Modifying an ID might change style.
                 let node: JSRef<Node> = NodeCast::from_ref(*self);
                 let value = attr.value();
-                if node.is_in_doc() && !value.as_slice().is_empty() {
+                if node.is_in_doc() {
                     let doc = document_from_node(*self).root();
+                    if !value.as_slice().is_empty() {
                         let value = Atom::from_slice(value.as_slice());
                         doc.register_named_element(*self, value);
                     }
+                    doc.content_changed(node, NodeStyleDamaged);
+                }
+            }
+            _ => {
+                // Modifying any other attribute might change arbitrary things.
+                let node: JSRef<Node> = NodeCast::from_ref(*self);
+                if node.is_in_doc() {
+                    let document = document_from_node(*self).root();
+                    document.content_changed(node, OtherNodeDamage);
+                }
             }
-            _ => ()
         }
-
-        self.notify_content_changed();
     }
 
     fn before_remove_attr(&self, attr: JSRef<Attr>) {
@@ -1028,21 +1036,45 @@ impl<'a> VirtualMethods for JSRef<'a, Element> {
 
         match attr.local_name() {
             &atom!("style") => {
+                // Modifying the `style` attribute might change style.
                 *self.style_attribute.borrow_mut() = None;
+
+                let node: JSRef<Node> = NodeCast::from_ref(*self);
+                if node.is_in_doc() {
+                    let doc = document_from_node(*self).root();
+                    doc.content_changed(node, NodeStyleDamaged);
+                }
             }
             &atom!("id") => {
+                // Modifying an ID can change style.
                 let node: JSRef<Node> = NodeCast::from_ref(*self);
                 let value = attr.value();
-                if node.is_in_doc() && !value.as_slice().is_empty() {
+                if node.is_in_doc() {
                     let doc = document_from_node(*self).root();
+                    if !value.as_slice().is_empty() {
                         let value = Atom::from_slice(value.as_slice());
                         doc.unregister_named_element(*self, value);
                     }
+                    doc.content_changed(node, NodeStyleDamaged);
+                }
+            }
+            &atom!("class") => {
+                // Modifying a class can change style.
+                let node: JSRef<Node> = NodeCast::from_ref(*self);
+                if node.is_in_doc() {
+                    let document = document_from_node(*self).root();
+                    document.content_changed(node, NodeStyleDamaged);
+                }
+            }
+            _ => {
+                // Modifying any other attribute might change arbitrary things.
+                let node: JSRef<Node> = NodeCast::from_ref(*self);
+                if node.is_in_doc() {
+                    let doc = document_from_node(*self).root();
+                    doc.content_changed(node, OtherNodeDamage);
+                }
             }
-            _ => ()
         }
-
-        self.notify_content_changed();
     }
 
     fn parse_plain_attribute(&self, name: &Atom, value: DOMString) -> AttrValue {

components/script/dom/htmlinputelement.rs

@@ -26,8 +26,10 @@ use dom::event::{Event, Bubbles, NotCancelable, EventHelpers};
 use dom::eventtarget::{EventTarget, NodeTargetTypeId};
 use dom::htmlelement::HTMLElement;
 use dom::keyboardevent::KeyboardEvent;
-use dom::htmlformelement::{InputElement, FormControl, HTMLFormElement, HTMLFormElementHelpers, NotFromFormSubmitMethod};
+use dom::htmlformelement::{InputElement, FormControl, HTMLFormElement, HTMLFormElementHelpers};
-use dom::node::{DisabledStateHelpers, Node, NodeHelpers, ElementNodeTypeId, document_from_node, window_from_node};
+use dom::htmlformelement::{NotFromFormSubmitMethod};
+use dom::node::{DisabledStateHelpers, Node, NodeHelpers, ElementNodeTypeId, OtherNodeDamage};
+use dom::node::{document_from_node, window_from_node};
 use dom::virtualmethods::VirtualMethods;
 use textinput::{Single, TextInput, TriggerDefaultAction, DispatchInput, Nothing};
 
@@ -312,7 +314,7 @@ impl<'a> HTMLInputElementHelpers for JSRef<'a, HTMLInputElement> {
     fn force_relayout(self) {
         let doc = document_from_node(self).root();
         let node: JSRef<Node> = NodeCast::from_ref(self);
-        doc.content_changed(node)
+        doc.content_changed(node, OtherNodeDamage)
     }
 
     fn radio_group_updated(self, group: Option<&str>) {

components/script/dom/htmltextareaelement.rs

@@ -20,7 +20,8 @@ use dom::event::Event;
 use dom::eventtarget::{EventTarget, NodeTargetTypeId};
 use dom::htmlelement::HTMLElement;
 use dom::keyboardevent::KeyboardEvent;
-use dom::node::{DisabledStateHelpers, Node, NodeHelpers, ElementNodeTypeId, document_from_node};
+use dom::node::{DisabledStateHelpers, Node, NodeHelpers, OtherNodeDamage, ElementNodeTypeId};
+use dom::node::{document_from_node};
 use textinput::{Multiple, TextInput, TriggerDefaultAction, DispatchInput, Nothing};
 use dom::virtualmethods::VirtualMethods;
 
@@ -163,7 +164,7 @@ impl<'a> PrivateHTMLTextAreaElementHelpers for JSRef<'a, HTMLTextAreaElement> {
     fn force_relayout(self) {
         let doc = document_from_node(self).root();
         let node: JSRef<Node> = NodeCast::from_ref(self);
-        doc.content_changed(node)
+        doc.content_changed(node, OtherNodeDamage)
     }
 }
 

components/script/dom/node.rs

@@ -287,20 +287,15 @@ impl<'a> PrivateNodeHelpers for JSRef<'a, Node> {
 
         let parent = self.parent_node().root();
         parent.map(|parent| vtable_for(&*parent).child_inserted(self));
-
+        document.content_and_heritage_changed(self, OtherNodeDamage);
-        document.content_and_heritage_changed(self);
     }
 
     // http://dom.spec.whatwg.org/#node-is-removed
     fn node_removed(self, parent_in_doc: bool) {
         assert!(self.parent_node().is_none());
-        let document = document_from_node(self).root();
-
         for node in self.traverse_preorder() {
             vtable_for(&node).unbind_from_tree(parent_in_doc);
         }
-
-        document.content_changed(self);
     }
 
     //
@@ -311,9 +306,6 @@ impl<'a> PrivateNodeHelpers for JSRef<'a, Node> {
     ///
     /// Fails unless `new_child` is disconnected from the tree.
     fn add_child(self, new_child: JSRef<Node>, before: Option<JSRef<Node>>) {
-        let doc = self.owner_doc().root();
-        doc.wait_until_safe_to_modify_dom();
-
         assert!(new_child.parent_node().is_none());
         assert!(new_child.prev_sibling().is_none());
         assert!(new_child.next_sibling().is_none());
@@ -354,9 +346,6 @@ impl<'a> PrivateNodeHelpers for JSRef<'a, Node> {
     ///
     /// Fails unless `child` is a child of this node.
     fn remove_child(self, child: JSRef<Node>) {
-        let doc = self.owner_doc().root();
-        doc.wait_until_safe_to_modify_dom();
-
         assert!(child.parent_node().root().root_ref() == Some(self));
 
         match child.prev_sibling.get().root() {
@@ -428,8 +417,6 @@ pub trait NodeHelpers<'a> {
     fn set_owner_doc(self, document: JSRef<Document>);
     fn is_in_html_doc(self) -> bool;
 
-    fn wait_until_safe_to_modify_dom(self);
-
     fn is_element(self) -> bool;
     fn is_document(self) -> bool;
     fn is_doctype(self) -> bool;
@@ -460,10 +447,11 @@ pub trait NodeHelpers<'a> {
     fn get_has_dirty_descendants(self) -> bool;
     fn set_has_dirty_descendants(self, state: bool);
 
-    /// Marks the given node as `IS_DIRTY`, its siblings as `IS_DIRTY` (to deal
+    /// Marks the given node as `IS_DIRTY`, its siblings as `HAS_DIRTY_SIBLINGS` (to deal with
-    /// with sibling selectors), its ancestors as `HAS_DIRTY_DESCENDANTS`, and its
+    /// sibling selectors), its ancestors as `HAS_DIRTY_DESCENDANTS`, and its descendants as
-    /// descendants as `IS_DIRTY`.
+    /// `IS_DIRTY`. If anything more than the node's style was damaged, this method also sets the
-    fn dirty(self);
+    /// `HAS_CHANGED` flag.
+    fn dirty(self, damage: NodeDamage);
 
     /// 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
@@ -471,9 +459,9 @@ pub trait NodeHelpers<'a> {
     /// still need to be marked as `HAS_DIRTY_DESCENDANTS`.
     ///
     /// See #4170
-    fn force_dirty_ancestors(self);
+    fn force_dirty_ancestors(self, damage: NodeDamage);
 
-    fn dirty_impl(self, force_ancestors: bool);
+    fn dirty_impl(self, damage: NodeDamage, force_ancestors: bool);
 
     fn dump(self);
     fn dump_indent(self, indent: uint);
@@ -656,17 +644,20 @@ impl<'a> NodeHelpers<'a> for JSRef<'a, Node> {
         self.set_flag(HAS_DIRTY_DESCENDANTS, state)
     }
 
-    fn force_dirty_ancestors(self) {
+    fn force_dirty_ancestors(self, damage: NodeDamage) {
-        self.dirty_impl(true)
+        self.dirty_impl(damage, true)
     }
 
-    fn dirty(self) {
+    fn dirty(self, damage: NodeDamage) {
-        self.dirty_impl(false)
+        self.dirty_impl(damage, false)
     }
 
-    fn dirty_impl(self, force_ancestors: bool) {
+    fn dirty_impl(self, damage: NodeDamage, force_ancestors: bool) {
         // 1. Dirty self.
-        self.set_has_changed(true);
+        match damage {
+            NodeStyleDamaged => {}
+            OtherNodeDamage => self.set_has_changed(true),
+        }
 
         if self.get_is_dirty() && !force_ancestors {
             return
@@ -830,11 +821,6 @@ impl<'a> NodeHelpers<'a> for JSRef<'a, Node> {
             .peekable()
     }
 
-    fn wait_until_safe_to_modify_dom(self) {
-        let document = self.owner_doc().root();
-        document.wait_until_safe_to_modify_dom();
-    }
-
     fn remove_self(self) {
         match self.parent_node().root() {
             Some(parent) => parent.remove_child(self),
@@ -1826,14 +1812,12 @@ impl<'a> NodeMethods for JSRef<'a, Node> {
             CommentNodeTypeId |
             TextNodeTypeId |
             ProcessingInstructionNodeTypeId => {
-                self.wait_until_safe_to_modify_dom();
-
                 let characterdata: JSRef<CharacterData> = CharacterDataCast::to_ref(self).unwrap();
                 characterdata.set_data(value);
 
                 // Notify the document that the content of this node is different
                 let document = self.owner_doc().root();
-                document.content_changed(self);
+                document.content_changed(self, OtherNodeDamage);
             }
             DoctypeNodeTypeId |
             DocumentNodeTypeId => {}
@@ -2366,3 +2350,13 @@ impl<'a> DisabledStateHelpers for JSRef<'a, Node> {
         self.set_enabled_state(!has_disabled_attrib);
     }
 }
+
+/// A summary of the changes that happened to a node.
+#[deriving(Clone, PartialEq)]
+pub enum NodeDamage {
+    /// The node's `style` attribute changed.
+    NodeStyleDamaged,
+    /// Other parts of a node changed; attributes, text content, etc.
+    OtherNodeDamage,
+}
+

components/script/dom/window.rs

@@ -21,7 +21,7 @@ use dom::navigator::Navigator;
 use dom::performance::Performance;
 use dom::screen::Screen;
 use dom::storage::Storage;
-use layout_interface::NoQuery;
+use layout_interface::{NoQuery, ReflowForDisplay, ReflowGoal, ReflowQueryType};
 use page::Page;
 use script_task::{ExitWindowMsg, ScriptChan, TriggerLoadMsg, TriggerFragmentMsg};
 use script_task::FromWindow;
@@ -299,9 +299,7 @@ impl Reflectable for Window {
 }
 
 pub trait WindowHelpers {
-    fn reflow(self);
+    fn flush_layout(self, goal: ReflowGoal, query: ReflowQueryType);
-    fn flush_layout(self);
-    fn wait_until_safe_to_modify_dom(self);
     fn init_browser_context(self, doc: JSRef<Document>);
     fn load_url(self, href: DOMString);
     fn handle_fire_timer(self, timer_id: TimerId);
@@ -334,18 +332,8 @@ impl<'a> WindowHelpers for JSRef<'a, Window> {
         })
     }
 
-    fn reflow(self) {
+    fn flush_layout(self, goal: ReflowGoal, query: ReflowQueryType) {
-        self.page().damage();
+        self.page().flush_layout(goal, query);
-    }
-
-    fn flush_layout(self) {
-        self.page().flush_layout(NoQuery);
-    }
-
-    fn wait_until_safe_to_modify_dom(self) {
-        // FIXME: This disables concurrent layout while we are modifying the DOM, since
-        //        our current architecture is entirely unsafe in the presence of races.
-        self.page().join_layout();
     }
 
     fn init_browser_context(self, doc: JSRef<Document>) {
@@ -369,7 +357,7 @@ impl<'a> WindowHelpers for JSRef<'a, Window> {
 
     fn handle_fire_timer(self, timer_id: TimerId) {
         self.timers.fire_timer(timer_id, self.clone());
-        self.flush_layout();
+        self.flush_layout(ReflowForDisplay, NoQuery);
     }
 }
 

components/script/page.rs

@@ -13,9 +13,9 @@ use dom::node::{Node, NodeHelpers};
 use dom::window::Window;
 use layout_interface::{
     ContentBoxQuery, ContentBoxResponse, ContentBoxesQuery, ContentBoxesResponse,
-    GetRPCMsg, HitTestResponse, LayoutChan, LayoutRPC, MouseOverResponse, NoQuery,
+    GetRPCMsg, HitTestResponse, LayoutChan, LayoutRPC, MouseOverResponse, Reflow,
-    Reflow, ReflowForDisplay, ReflowForScriptQuery, ReflowGoal, ReflowMsg,
+    ReflowForScriptQuery, ReflowGoal, ReflowMsg, ReflowQueryType,
-    ReflowQueryType, TrustedNodeAddress
+    TrustedNodeAddress
 };
 use script_traits::{UntrustedNodeAddress, ScriptControlChan};
 
@@ -29,7 +29,7 @@ use servo_net::storage_task::StorageTask;
 use servo_util::geometry::{Au, MAX_RECT};
 use servo_util::geometry;
 use servo_util::str::DOMString;
-use servo_util::smallvec::{SmallVec1, SmallVec};
+use servo_util::smallvec::SmallVec;
 use std::cell::{Cell, Ref, RefMut};
 use std::comm::{channel, Receiver, Empty, Disconnected};
 use std::mem::replace;
@@ -77,9 +77,6 @@ pub struct Page {
     /// Pending resize event, if any.
     pub resize_event: Cell<Option<WindowSizeData>>,
 
-    /// Any nodes that need to be dirtied before the next reflow.
-    pub pending_dirty_nodes: DOMRefCell<SmallVec1<UntrustedNodeAddress>>,
-
     /// Pending scroll to fragment event, if any
     pub fragment_name: DOMRefCell<Option<String>>,
 
@@ -95,15 +92,6 @@ pub struct Page {
     // Child Pages.
     pub children: DOMRefCell<Vec<Rc<Page>>>,
 
-    /// Whether layout needs to be run at all.
-    pub damaged: Cell<bool>,
-
-    /// Number of pending reflows that were sent while layout was active.
-    pub pending_reflows: Cell<int>,
-
-    /// Number of unnecessary potential reflows that were skipped since the last reflow
-    pub avoided_reflows: Cell<int>,
-
     /// An enlarged rectangle around the page contents visible in the viewport, used
     /// to prevent creating display list items for content that is far away from the viewport.
     pub page_clip_rect: Cell<Rect<Au>>,
@@ -165,57 +153,35 @@ impl Page {
             url: DOMRefCell::new(None),
             next_subpage_id: Cell::new(SubpageId(0)),
             resize_event: Cell::new(None),
-            pending_dirty_nodes: DOMRefCell::new(SmallVec1::new()),
             fragment_name: DOMRefCell::new(None),
             last_reflow_id: Cell::new(0),
             resource_task: resource_task,
             storage_task: storage_task,
             constellation_chan: constellation_chan,
             children: DOMRefCell::new(vec!()),
-            damaged: Cell::new(false),
-            pending_reflows: Cell::new(0),
-            avoided_reflows: Cell::new(0),
             page_clip_rect: Cell::new(MAX_RECT),
         }
     }
 
-    pub fn flush_layout(&self, query: ReflowQueryType) {
+    pub fn flush_layout(&self, goal: ReflowGoal, query: ReflowQueryType) {
-        // If we are damaged, we need to force a full reflow, so that queries interact with
-        // an accurate flow tree.
-        let (reflow_goal, force_reflow) = if self.damaged.get() {
-            (ReflowForDisplay, true)
-        } else {
-            match query {
-                ContentBoxQuery(_) | ContentBoxesQuery(_) => (ReflowForScriptQuery, true),
-                NoQuery => (ReflowForDisplay, false),
-            }
-        };
-
-        if force_reflow {
         let frame = self.frame();
         let window = frame.as_ref().unwrap().window.root();
-            self.reflow(reflow_goal, window.control_chan().clone(), &mut **window.compositor(), query);
+        self.reflow(goal, window.control_chan().clone(), &mut **window.compositor(), query);
-        } else {
-            self.avoided_reflows.set(self.avoided_reflows.get() + 1);
-        }
     }
 
     pub fn layout(&self) -> &LayoutRPC {
-        self.flush_layout(NoQuery);
+        &*self.layout_rpc
-        self.join_layout(); //FIXME: is this necessary, or is layout_rpc's mutex good enough?
-        let layout_rpc: &LayoutRPC = &*self.layout_rpc;
-        layout_rpc
     }
 
     pub fn content_box_query(&self, content_box_request: TrustedNodeAddress) -> Rect<Au> {
-        self.flush_layout(ContentBoxQuery(content_box_request));
+        self.flush_layout(ReflowForScriptQuery, ContentBoxQuery(content_box_request));
         self.join_layout(); //FIXME: is this necessary, or is layout_rpc's mutex good enough?
         let ContentBoxResponse(rect) = self.layout_rpc.content_box();
         rect
     }
 
     pub fn content_boxes_query(&self, content_boxes_request: TrustedNodeAddress) -> Vec<Rect<Au>> {
-        self.flush_layout(ContentBoxesQuery(content_boxes_request));
+        self.flush_layout(ReflowForScriptQuery, ContentBoxesQuery(content_boxes_request));
         self.join_layout(); //FIXME: is this necessary, or is layout_rpc's mutex good enough?
         let ContentBoxesResponse(rects) = self.layout_rpc.content_boxes();
         rects
@@ -276,6 +242,13 @@ impl Page {
             }
         }
     }
+
+    pub fn dirty_all_nodes(&self) {
+        match *self.frame.borrow() {
+            None => {}
+            Some(ref frame) => frame.document.root().dirty_all_nodes(),
+        }
+    }
 }
 
 impl Iterator<Rc<Page>> for PageIterator {
@@ -334,7 +307,7 @@ impl Page {
 
     /// Sends a ping to layout and waits for the response. The response will arrive when the
     /// layout task has finished any pending request messages.
-    pub fn join_layout(&self) {
+    fn join_layout(&self) {
         let mut layout_join_port = self.layout_join_port.borrow_mut();
         if layout_join_port.is_some() {
             let join_port = replace(&mut *layout_join_port, None);
@@ -358,11 +331,11 @@ impl Page {
         }
     }
 
-    /// Reflows the page if it's possible to do so. This method will wait until the layout task has
+    /// Reflows the page if it's possible to do so and the page is dirty. This method will wait
-    /// completed its current action, join the layout task, and then request a new layout run. It
+    /// for the layout thread to complete (but see the `TODO` below). If there is no window size
-    /// won't wait for the new layout computation to finish.
+    /// yet, the page is presumed invisible and no reflow is performed.
     ///
-    /// If there is no window size yet, the page is presumed invisible and no reflow is performed.
+    /// TODO(pcwalton): Only wait for style recalc, since we have off-main-thread layout.
     pub fn reflow(&self,
                   goal: ReflowGoal,
                   script_chan: ScriptControlChan,
@@ -375,29 +348,33 @@ impl Page {
             }
         };
 
-        match root.root() {
+        let root = match root.root() {
-            None => {},
+            None => return,
-            Some(root) => {
+            Some(root) => root,
-                debug!("avoided {:d} reflows", self.avoided_reflows.get());
+        };
-                self.avoided_reflows.set(0);
 
         debug!("script: performing reflow for goal {}", goal);
 
-                // Now, join the layout so that they will see the latest changes we have made.
+        let root: JSRef<Node> = NodeCast::from_ref(*root);
-                self.join_layout();
+        if !root.get_has_dirty_descendants() {
+            debug!("root has no dirty descendants; avoiding reflow");
+            return
+        }
+
+        debug!("script: performing reflow for goal {}", goal);
 
         // Layout will let us know when it's done.
         let (join_chan, join_port) = channel();
+
+        {
             let mut layout_join_port = self.layout_join_port.borrow_mut();
             *layout_join_port = Some(join_port);
+        }
 
         let last_reflow_id = &self.last_reflow_id;
         last_reflow_id.set(last_reflow_id.get() + 1);
 
-                let root: JSRef<Node> = NodeCast::from_ref(*root);
-
         let window_size = self.window_size.get();
-                self.damaged.set(false);
 
         // Send new document and relevant styles to layout.
         let reflow = box Reflow {
@@ -416,13 +393,9 @@ impl Page {
         let LayoutChan(ref chan) = self.layout_chan;
         chan.send(ReflowMsg(reflow));
 
-                debug!("script: layout forked")
+        debug!("script: layout forked");
-            }
-        }
-    }
 
-    pub fn damage(&self) {
+        self.join_layout();
-        self.damaged.set(true);
     }
 
     /// Attempt to find a named element in this page's document.

components/script/script_task.rs

@@ -23,8 +23,7 @@ use dom::event::{Event, EventHelpers, Bubbles, DoesNotBubble, Cancelable, NotCan
 use dom::uievent::UIEvent;
 use dom::eventtarget::{EventTarget, EventTargetHelpers};
 use dom::keyboardevent::KeyboardEvent;
-use dom::node;
+use dom::node::{mod, ElementNodeTypeId, Node, NodeHelpers, OtherNodeDamage};
-use dom::node::{ElementNodeTypeId, Node, NodeHelpers};
 use dom::window::{Window, WindowHelpers};
 use dom::worker::{Worker, TrustedWorkerAddress};
 use dom::xmlhttprequest::{TrustedXHRAddress, XMLHttpRequest, XHRProgress};
@@ -40,10 +39,9 @@ use devtools_traits::{DevtoolScriptControlMsg, EvaluateJS, GetDocumentElement};
 use devtools_traits::{GetChildren, GetLayout, ModifyAttribute};
 use script_traits::{CompositorEvent, ResizeEvent, ReflowEvent, ClickEvent, MouseDownEvent};
 use script_traits::{MouseMoveEvent, MouseUpEvent, ConstellationControlMsg, ScriptTaskFactory};
-use script_traits::{ResizeMsg, AttachLayoutMsg, LoadMsg, ViewportMsg, SendEventMsg};
+use script_traits::{ResizeMsg, AttachLayoutMsg, GetTitleMsg, KeyEvent, LoadMsg, ViewportMsg};
 use script_traits::{ResizeInactiveMsg, ExitPipelineMsg, NewLayoutInfo, OpaqueScriptLayoutChannel};
-use script_traits::{ScriptControlChan, ReflowCompleteMsg, UntrustedNodeAddress, KeyEvent};
+use script_traits::{ScriptControlChan, ReflowCompleteMsg, SendEventMsg};
-use script_traits::{GetTitleMsg};
 use servo_msg::compositor_msg::{FinishedLoading, LayerId, Loading, PerformingLayout};
 use servo_msg::compositor_msg::{ScriptListener};
 use servo_msg::constellation_msg::{ConstellationChan, LoadCompleteMsg};
@@ -496,11 +494,7 @@ impl ScriptTask {
                     let page = page.find(id).expect("resize sent to nonexistent pipeline");
                     page.resize_event.set(Some(size));
                 }
-                FromConstellation(SendEventMsg(id, ReflowEvent(node_addresses))) => {
+                FromConstellation(SendEventMsg(id, ReflowEvent(_))) => {
-                    let page = self.page.borrow_mut();
-                    let inner_page = page.find(id).expect("Reflow sent to nonexistent pipeline");
-                    let mut pending = inner_page.pending_dirty_nodes.borrow_mut();
-                    pending.push_all_move(node_addresses);
                     needs_reflow.insert(id);
                 }
                 FromConstellation(ViewportMsg(id, rect)) => {
@@ -666,11 +660,6 @@ impl ScriptTask {
         }
 
         self.compositor.borrow_mut().set_ready_state(pipeline_id, FinishedLoading);
-
-        if page.pending_reflows.get() > 0 {
-            page.pending_reflows.set(0);
-            self.force_reflow(&*page);
-        }
     }
 
     /// Handles a navigate forward or backward message.
@@ -838,8 +827,12 @@ impl ScriptTask {
 
         // Kick off the initial reflow of the page.
         debug!("kicking off initial reflow of {}", final_url);
-        document.content_changed(NodeCast::from_ref(*document));
+        {
-        window.flush_layout();
+            let document_js_ref = (&*document).clone();
+            let document_as_node = NodeCast::from_ref(document_js_ref);
+            document.content_changed(document_as_node, OtherNodeDamage);
+        }
+        window.flush_layout(ReflowForDisplay, NoQuery);
 
         {
             // No more reflow required
@@ -895,18 +888,9 @@ impl ScriptTask {
         self.compositor.borrow_mut().scroll_fragment_point(pipeline_id, LayerId::null(), point);
     }
 
+    /// Reflows non-incrementally.
     fn force_reflow(&self, page: &Page) {
-        {
+        page.dirty_all_nodes();
-            let mut pending = page.pending_dirty_nodes.borrow_mut();
-            let js_runtime = self.js_runtime.deref().ptr;
-
-            for untrusted_node in pending.into_iter() {
-                let node = node::from_untrusted_node_address(js_runtime, untrusted_node).root();
-                node.dirty();
-            }
-        }
-
-        page.damage();
         page.reflow(ReflowForDisplay,
                     self.control_chan.clone(),
                     &mut **self.compositor.borrow_mut(),
@@ -1020,7 +1004,7 @@ impl ScriptTask {
             _ => ()
         }
 
-        window.flush_layout();
+        window.flush_layout(ReflowForDisplay, NoQuery);
     }
 
     /// The entry point for content to notify that a new load has been requested
@@ -1090,14 +1074,9 @@ impl ScriptTask {
         let page = get_page(&*self.page.borrow(), pipeline_id);
         let frame = page.frame();
         if frame.is_some() {
-            let in_layout = page.layout_join_port.borrow().is_some();
-            if in_layout {
-                page.pending_reflows.set(page.pending_reflows.get() + 1);
-            } else {
             self.force_reflow(&*page);
         }
     }
-    }
 
     fn handle_click_event(&self, pipeline_id: PipelineId, _button: uint, point: Point2D<f32>) {
         debug!("ClickEvent: clicked at {}", point);
@@ -1138,7 +1117,7 @@ impl ScriptTask {
                                 el.authentic_click_activation(*event);
 
                                 doc.commit_focus_transaction();
-                                window.flush_layout();
+                                window.flush_layout(ReflowForDisplay, NoQuery);
                             }
                             None => {}
                         }
@@ -1220,8 +1199,6 @@ impl ScriptTask {
 /// Shuts down layout for the given page tree.
 fn shut_down_layout(page_tree: &Rc<Page>, rt: *mut JSRuntime) {
     for page in page_tree.iter() {
-        page.join_layout();
-
         // Tell the layout task to begin shutting down, and wait until it
         // processed this message.
         let (response_chan, response_port) = channel();