servo: Merge #3844 - script: Improve dirty propagation and fix script-layout synchronization (from pcwalton:script-layout-synchronization); r=jdm
authorPatrick Walton <pcwalton@mimiga.net>
Mon, 15 Dec 2014 15:31:21 -0700
changeset 473623 5fbc5d79a9dd85e4fb5b460c0c3d740020c93ec0
parent 473622 2e1abcce517dbab9be2d2fd313465c6e2b04db23
child 473624 562a70720103e72469383e37094fe14d242fc7d2
push id44079
push userbmo:gps@mozilla.com
push dateSat, 04 Feb 2017 00:14:49 +0000
reviewersjdm
servo: Merge #3844 - script: Improve dirty propagation and fix script-layout synchronization (from pcwalton:script-layout-synchronization); r=jdm This fixes race conditions whereby layout and script could be running simultaneously. r? @jdm cc @cgaebel Source-Repo: https://github.com/servo/servo Source-Revision: 5f2684d2f81046abd7548fb22d996d1e506a104a
servo/components/layout/layout_task.rs
servo/components/net/image/holder.rs
servo/components/script/dom/document.rs
servo/components/script/dom/element.rs
servo/components/script/dom/htmlimageelement.rs
servo/components/script/dom/htmlinputelement.rs
servo/components/script/dom/htmltextareaelement.rs
servo/components/script/dom/node.rs
servo/components/script/dom/window.rs
servo/components/script/page.rs
servo/components/script/script_task.rs
servo/tests/content/test_getBoundingClientRect.html
--- a/servo/components/layout/layout_task.rs
+++ b/servo/components/layout/layout_task.rs
@@ -585,26 +585,30 @@ impl LayoutTask {
     #[cfg(not(debug))]
     fn verify_flow_tree(&self, _: &mut FlowRef) {
     }
 
     fn process_content_box_request<'a>(&'a self,
                                        requested_node: TrustedNodeAddress,
                                        layout_root: &mut FlowRef,
                                        rw_data: &mut RWGuard<'a>) {
+        // FIXME(pcwalton): This has not been updated to handle the stacking context relative
+        // stuff. So the position is wrong in most cases.
         let requested_node: OpaqueNode = OpaqueNodeMethods::from_script_node(requested_node);
         let mut iterator = UnioningFragmentBoundsIterator::new(requested_node);
         sequential::iterate_through_flow_tree_fragment_bounds(layout_root, &mut iterator);
         rw_data.content_box_response = iterator.rect;
     }
 
     fn process_content_boxes_request<'a>(&'a self,
                                          requested_node: TrustedNodeAddress,
                                          layout_root: &mut FlowRef,
                                          rw_data: &mut RWGuard<'a>) {
+        // FIXME(pcwalton): This has not been updated to handle the stacking context relative
+        // stuff. So the position is wrong in most cases.
         let requested_node: OpaqueNode = OpaqueNodeMethods::from_script_node(requested_node);
         let mut iterator = CollectingFragmentBoundsIterator::new(requested_node);
         sequential::iterate_through_flow_tree_fragment_bounds(layout_root, &mut iterator);
         rw_data.content_boxes_response = iterator.rects;
     }
 
     fn build_display_list_for_reflow<'a>(&'a self,
                                          data: &Reflow,
--- a/servo/components/net/image/holder.rs
+++ b/servo/components/net/image/holder.rs
@@ -2,17 +2,16 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use image::base::Image;
 use image_cache_task::{ImageReady, ImageNotReady, ImageFailed};
 use local_image_cache::LocalImageCache;
 
 use geom::size::Size2D;
-use std::mem;
 use sync::{Arc, Mutex};
 use url::Url;
 
 // FIXME: Nasty coupling here This will be a problem if we want to factor out image handling from
 // the network stack. This should probably be factored out into an interface and use dependency
 // injection.
 
 /// A struct to store image data. The image will be loaded once the first time it is requested,
@@ -82,32 +81,21 @@ impl<NodeAddress: Send> ImageHolder<Node
         // the image and store it for the future
         if self.image.is_none() {
             let port = {
                 let val = self.local_image_cache.lock();
                 let mut local_image_cache = val;
                 local_image_cache.get_image(node_address, &self.url)
             };
             match port.recv() {
-                ImageReady(image) => {
-                    self.image = Some(image);
-                }
-                ImageNotReady => {
-                    debug!("image not ready for {:s}", self.url.serialize());
-                }
-                ImageFailed => {
-                    debug!("image decoding failed for {:s}", self.url.serialize());
-                }
+                ImageReady(image) => self.image = Some(image),
+                ImageNotReady => debug!("image not ready for {:s}", self.url.serialize()),
+                ImageFailed => debug!("image decoding failed for {:s}", self.url.serialize()),
             }
         }
 
-        // Clone isn't pure so we have to swap out the mutable image option
-        let image = mem::replace(&mut self.image, None);
-        let result = image.clone();
-        mem::replace(&mut self.image, image);
-
-        return result;
+        return self.image.clone();
     }
 
     pub fn url(&self) -> &Url {
         &self.url
     }
 }
--- a/servo/components/script/dom/document.rs
+++ b/servo/components/script/dom/document.rs
@@ -44,17 +44,17 @@ use dom::htmlelement::HTMLElement;
 use dom::htmlheadelement::HTMLHeadElement;
 use dom::htmlhtmlelement::HTMLHtmlElement;
 use dom::htmltitleelement::HTMLTitleElement;
 use dom::location::Location;
 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;
 use dom::range::Range;
 use dom::treewalker::TreeWalker;
 use dom::uievent::UIEvent;
 use dom::window::{Window, WindowHelpers};
 use servo_util::namespace;
@@ -171,30 +171,29 @@ pub trait DocumentHelpers<'a> {
     fn window(self) -> Temporary<Window>;
     fn encoding_name(self) -> Ref<'a, DOMString>;
     fn is_html_document(self) -> bool;
     fn url(self) -> &'a Url;
     fn quirks_mode(self) -> QuirksMode;
     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_and_heritage_changed(self, node: JSRef<Node>);
-    fn reflow(self);
-    fn wait_until_safe_to_modify_dom(self);
+    fn content_changed(self, node: JSRef<Node>, damage: NodeDamage);
+    fn content_and_heritage_changed(self, node: JSRef<Node>, damage: NodeDamage);
     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);
     fn find_fragment_node(self, fragid: DOMString) -> Option<Temporary<Element>>;
     fn set_ready_state(self, state: DocumentReadyState);
     fn get_focused_element(self) -> Option<Temporary<Element>>;
     fn begin_focus_transaction(self);
     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> {
     #[inline]
     fn window(self) -> Temporary<Window> {
         Temporary::new(self.window)
     }
 
@@ -223,34 +222,24 @@ impl<'a> DocumentHelpers<'a> for JSRef<'
     fn set_last_modified(self, value: DOMString) {
         *self.last_modified.borrow_mut() = Some(value);
     }
 
     fn set_encoding_name(self, name: DOMString) {
         *self.encoding_name.borrow_mut() = name;
     }
 
-    fn content_changed(self, node: JSRef<Node>) {
-        debug!("content_changed on {}", node.debug_str());
-        node.dirty();
-        self.reflow();
+    fn content_changed(self, node: JSRef<Node>, damage: NodeDamage) {
+        node.dirty(damage);
     }
 
-    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();
-        self.reflow();
-    }
-
-    fn reflow(self) {
-        self.window.root().reflow();
-    }
-
-    fn wait_until_safe_to_modify_dom(self) {
-        self.window.root().wait_until_safe_to_modify_dom();
+        node.force_dirty_ancestors(damage);
+        node.dirty(damage);
     }
 
     /// Remove any existing association between the provided id and any elements in this document.
     fn unregister_named_element(self,
                                 to_unregister: JSRef<Element>,
                                 id: Atom) {
         let mut idmap = self.idmap.borrow_mut();
         let is_empty = match idmap.get_mut(&id) {
@@ -371,16 +360,23 @@ impl<'a> DocumentHelpers<'a> for JSRef<'
         self.focused.assign(self.possibly_focused.get());
     }
 
     /// Sends this document's title to the compositor.
     fn send_title_to_compositor(self) {
         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)]
 pub enum DocumentSource {
     FromParser,
     NotFromParser,
 }
 
--- a/servo/components/script/dom/element.rs
+++ b/servo/components/script/dom/element.rs
@@ -28,18 +28,19 @@ use dom::domrectlist::DOMRectList;
 use dom::document::{Document, DocumentHelpers, LayoutDocumentHelpers};
 use dom::domtokenlist::DOMTokenList;
 use dom::event::Event;
 use dom::eventtarget::{EventTarget, NodeTargetTypeId, EventTargetHelpers};
 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::{window_from_node, LayoutNodeHelpers};
+use dom::node::{CLICK_IN_PROGRESS, ElementNodeTypeId, Node, NodeHelpers, NodeIterator};
+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;
 use style::{IntegerAttribute, LengthAttribute, SizeIntegerAttribute, WidthLengthAttribute};
 use style::{matches, parse_selector_list_from_str};
 use style;
 use servo_util::namespace;
 use servo_util::str::{DOMString, LengthOrPercentageOrAuto};
@@ -436,17 +437,16 @@ pub trait AttributeHandlers {
     fn set_attribute(self, name: &Atom, value: AttrValue);
     fn do_set_attribute(self, local_name: Atom, value: AttrValue,
                         name: Atom, namespace: Namespace,
                         prefix: Option<DOMString>, cb: |JSRef<Attr>| -> bool);
     fn parse_attribute(self, namespace: &Namespace, local_name: &Atom,
                        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);
 
     // http://www.whatwg.org/html/#reflecting-content-attributes-in-idl-attributes
     fn has_attribute(self, name: &Atom) -> bool;
     fn set_bool_attribute(self, name: &Atom, value: bool);
     fn get_url_attribute(self, name: &Atom) -> DOMString;
@@ -495,19 +495,16 @@ impl<'a> AttributeHandlers for JSRef<'a,
         let value = self.parse_attribute(&qname.ns, &qname.local, value);
         self.do_set_attribute(qname.local, value, name, qname.ns, prefix, |_| false)
     }
 
     fn set_attribute(self, name: &Atom, value: AttrValue) {
         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);
     }
 
     fn do_set_attribute(self, local_name: Atom, value: AttrValue,
                         name: Atom, namespace: Namespace,
                         prefix: Option<DOMString>, cb: |JSRef<Attr>| -> bool) {
         let idx = self.attrs.borrow().iter()
@@ -543,38 +540,36 @@ impl<'a> AttributeHandlers for JSRef<'a,
 
         let idx = self.attrs.borrow().iter().map(|attr| attr.root()).position(|attr| {
             *attr.local_name() == local_name
         });
 
         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();
+
+                let node: JSRef<Node> = NodeCast::from_ref(self);
+                if node.is_in_doc() {
+                    let document = document_from_node(self).root();
+                    if local_name == atom!("style") {
+                        document.content_changed(node, NodeStyleDamaged);
+                    } else {
+                        document.content_changed(node, OtherNodeDamage);
+                    }
+                }
             }
         };
     }
 
-    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);
-        }
-    }
-
     fn has_class(&self, name: &Atom) -> bool {
         self.get_attribute(ns!(""), &atom!("class")).root().map(|attr| {
             attr.value().tokens().map(|tokens| {
                 tokens.iter().any(|atom| atom == name)
             }).unwrap_or(false)
         }).unwrap_or(false)
     }
 
@@ -750,21 +745,16 @@ impl<'a> ElementMethods for JSRef<'a, El
         self.get_attribute(namespace, &Atom::from_slice(local_name.as_slice())).root()
                      .map(|attr| attr.Value())
     }
 
     // http://dom.spec.whatwg.org/#dom-element-setattribute
     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),
             _ => {}
         }
 
         // Step 2.
         let name = if self.html_element_in_html_document() {
@@ -782,21 +772,16 @@ impl<'a> ElementMethods for JSRef<'a, El
         Ok(())
     }
 
     // http://dom.spec.whatwg.org/#dom-element-setattributens
     fn SetAttributeNS(self,
                       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);
 
         let name_type = xml_name_type(name.as_slice());
         match name_type {
             // Step 2.
             InvalidXMLName => return Err(InvalidCharacter),
             // Step 3.
@@ -994,60 +979,107 @@ impl<'a> VirtualMethods for JSRef<'a, El
     fn after_set_attr(&self, attr: JSRef<Attr>) {
         match self.super_type() {
             Some(ref s) => s.after_set_attr(attr),
             _ => ()
         }
 
         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();
-                    let value = Atom::from_slice(value.as_slice());
-                    doc.register_named_element(*self, value);
+                    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>) {
         match self.super_type() {
             Some(ref s) => s.before_remove_attr(attr),
             _ => ()
         }
 
         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();
-                    let value = Atom::from_slice(value.as_slice());
-                    doc.unregister_named_element(*self, value);
+                    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 {
         match name {
             &atom!("id") => AttrValue::from_atomic(value),
             &atom!("class") => AttrValue::from_tokenlist(value),
             _ => self.super_type().unwrap().parse_plain_attribute(name, value),
         }
--- a/servo/components/script/dom/htmlimageelement.rs
+++ b/servo/components/script/dom/htmlimageelement.rs
@@ -10,17 +10,17 @@ use dom::bindings::codegen::Bindings::HT
 use dom::bindings::codegen::InheritTypes::{NodeCast, ElementCast, HTMLElementCast, HTMLImageElementDerived};
 use dom::bindings::js::{JS, JSRef, Temporary};
 use dom::bindings::utils::{Reflectable, Reflector};
 use dom::document::{Document, DocumentHelpers};
 use dom::element::{Element, HTMLImageElementTypeId};
 use dom::element::AttributeHandlers;
 use dom::eventtarget::{EventTarget, NodeTargetTypeId};
 use dom::htmlelement::HTMLElement;
-use dom::node::{Node, ElementNodeTypeId, NodeHelpers, window_from_node};
+use dom::node::{Node, ElementNodeTypeId, NodeHelpers, OtherNodeDamage, window_from_node};
 use dom::virtualmethods::VirtualMethods;
 use servo_net::image_cache_task;
 use servo_util::geometry::to_px;
 use servo_util::str::DOMString;
 use string_cache::Atom;
 
 use url::{Url, UrlParser};
 
@@ -107,28 +107,42 @@ impl<'a> HTMLImageElementMethods for JSR
     make_bool_getter!(IsMap)
 
     fn SetIsMap(self, is_map: bool) {
         let element: JSRef<Element> = ElementCast::from_ref(self);
         element.set_string_attribute(&atom!("ismap"), is_map.to_string())
     }
 
     fn Width(self) -> u32 {
+        // FIXME(pcwalton): This is a really nasty thing to do, but the interaction between the
+        // image cache task, the reflow messages that it sends to us via layout, and the image
+        // holders seem to just plain be racy, and this works around it by ensuring that we
+        // recreate the flow (picking up image changes on the way). The image cache task needs a
+        // rewrite to modern Rust.
         let node: JSRef<Node> = NodeCast::from_ref(self);
+        node.dirty(OtherNodeDamage);
+
         let rect = node.get_bounding_content_box();
         to_px(rect.size.width) as u32
     }
 
     fn SetWidth(self, width: u32) {
         let elem: JSRef<Element> = ElementCast::from_ref(self);
         elem.set_uint_attribute(&atom!("width"), width)
     }
 
     fn Height(self) -> u32 {
+        // FIXME(pcwalton): This is a really nasty thing to do, but the interaction between the
+        // image cache task, the reflow messages that it sends to us via layout, and the image
+        // holders seem to just plain be racy, and this works around it by ensuring that we
+        // recreate the flow (picking up image changes on the way). The image cache task needs a
+        // rewrite to modern Rust.
         let node: JSRef<Node> = NodeCast::from_ref(self);
+        node.dirty(OtherNodeDamage);
+
         let rect = node.get_bounding_content_box();
         to_px(rect.size.height) as u32
     }
 
     fn SetHeight(self, height: u32) {
         let elem: JSRef<Element> = ElementCast::from_ref(self);
         elem.set_uint_attribute(&atom!("height"), height)
     }
--- a/servo/components/script/dom/htmlinputelement.rs
+++ b/servo/components/script/dom/htmlinputelement.rs
@@ -21,18 +21,20 @@ use dom::bindings::js::{ResultRootable, 
 use dom::bindings::utils::{Reflectable, Reflector};
 use dom::document::{Document, DocumentHelpers};
 use dom::element::{AttributeHandlers, Element, HTMLInputElementTypeId};
 use dom::element::{RawLayoutElementHelpers, ActivationElementHelpers};
 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::node::{DisabledStateHelpers, Node, NodeHelpers, ElementNodeTypeId, document_from_node, window_from_node};
+use dom::htmlformelement::{InputElement, FormControl, HTMLFormElement, HTMLFormElementHelpers};
+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};
 
 use servo_util::str::DOMString;
 use string_cache::Atom;
 
 use std::ascii::OwnedAsciiExt;
 use std::cell::Cell;
@@ -307,17 +309,17 @@ fn in_same_group<'a,'b>(other: JSRef<'a,
         _ => false
     }
 }
 
 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>) {
         if self.Checked() {
             broadcast_radio_checked(self, group);
         }
     }
 
--- a/servo/components/script/dom/htmltextareaelement.rs
+++ b/servo/components/script/dom/htmltextareaelement.rs
@@ -15,17 +15,18 @@ use dom::bindings::codegen::InheritTypes
 use dom::bindings::js::{JS, JSRef, Temporary};
 use dom::bindings::utils::{Reflectable, Reflector};
 use dom::document::{Document, DocumentHelpers};
 use dom::element::{AttributeHandlers, HTMLTextAreaElementTypeId};
 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;
 
 use servo_util::str::DOMString;
 use string_cache::Atom;
 
 use std::cell::Cell;
 
@@ -158,17 +159,17 @@ impl<'a> HTMLTextAreaElementMethods for 
 trait PrivateHTMLTextAreaElementHelpers {
     fn force_relayout(self);
 }
 
 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)
     }
 }
 
 impl<'a> VirtualMethods for JSRef<'a, HTMLTextAreaElement> {
     fn super_type<'a>(&'a self) -> Option<&'a VirtualMethods> {
         let htmlelement: &JSRef<HTMLElement> = HTMLElementCast::from_borrowed_ref(self);
         Some(htmlelement as &VirtualMethods)
     }
--- a/servo/components/script/dom/node.rs
+++ b/servo/components/script/dom/node.rs
@@ -282,43 +282,35 @@ impl<'a> PrivateNodeHelpers for JSRef<'a
         let is_in_doc = self.is_in_doc();
 
         for node in self.traverse_preorder() {
             vtable_for(&node).bind_to_tree(is_in_doc);
         }
 
         let parent = self.parent_node().root();
         parent.map(|parent| vtable_for(&*parent).child_inserted(self));
-
-        document.content_and_heritage_changed(self);
+        document.content_and_heritage_changed(self, OtherNodeDamage);
     }
 
     // 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);
     }
 
     //
     // Pointer stitching
     //
 
     /// Adds a new child to the end of this node's list of children.
     ///
     /// 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());
         match before {
             Some(ref before) => {
                 assert!(before.parent_node().root().root_ref() == Some(self));
                 match before.prev_sibling().root() {
                     None => {
@@ -349,19 +341,16 @@ impl<'a> PrivateNodeHelpers for JSRef<'a
 
         new_child.parent_node.assign(Some(self));
     }
 
     /// Removes the given child from this node's list of children.
     ///
     /// 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() {
             None => {
                 self.first_child.assign(child.next_sibling.get());
             }
             Some(prev_sibling) => {
                 prev_sibling.next_sibling.assign(child.next_sibling.get());
@@ -423,18 +412,16 @@ pub trait NodeHelpers<'a> {
     fn last_child(self) -> Option<Temporary<Node>>;
     fn prev_sibling(self) -> Option<Temporary<Node>>;
     fn next_sibling(self) -> Option<Temporary<Node>>;
 
     fn owner_doc(self) -> Temporary<Document>;
     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;
     fn is_text(self) -> bool;
     fn is_anchor_element(self) -> bool;
 
     fn get_flag(self, flag: NodeFlags) -> bool;
     fn set_flag(self, flag: NodeFlags, value: bool);
@@ -455,30 +442,31 @@ pub trait NodeHelpers<'a> {
     fn set_is_dirty(self, state: bool);
 
     fn get_has_dirty_siblings(self) -> bool;
     fn set_has_dirty_siblings(self, state: bool);
 
     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
-    /// with sibling selectors), its ancestors as `HAS_DIRTY_DESCENDANTS`, and its
-    /// descendants as `IS_DIRTY`.
-    fn dirty(self);
+    /// Marks the given node as `IS_DIRTY`, its siblings as `HAS_DIRTY_SIBLINGS` (to deal with
+    /// sibling selectors), its ancestors as `HAS_DIRTY_DESCENDANTS`, and its descendants as
+    /// `IS_DIRTY`. If anything more than the node's style was damaged, this method also sets the
+    /// `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
     /// 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 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);
     fn debug_str(self) -> String;
 
     fn traverse_preorder(self) -> TreeIterator<'a>;
     fn inclusively_following_siblings(self) -> NodeChildrenIterator<'a>;
 
@@ -651,27 +639,30 @@ impl<'a> NodeHelpers<'a> for JSRef<'a, N
     fn get_has_dirty_descendants(self) -> bool {
         self.get_flag(HAS_DIRTY_DESCENDANTS)
     }
 
     fn set_has_dirty_descendants(self, state: bool) {
         self.set_flag(HAS_DIRTY_DESCENDANTS, state)
     }
 
-    fn force_dirty_ancestors(self) {
-        self.dirty_impl(true)
+    fn force_dirty_ancestors(self, damage: NodeDamage) {
+        self.dirty_impl(damage, true)
     }
 
-    fn dirty(self) {
-        self.dirty_impl(false)
+    fn dirty(self, damage: NodeDamage) {
+        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
         }
 
         // 2. Dirty descendants.
         fn dirty_subtree(node: JSRef<Node>) {
             // Stop if this subtree is already dirty.
@@ -825,21 +816,16 @@ impl<'a> NodeHelpers<'a> for JSRef<'a, N
     }
 
     fn child_elements(self) -> ChildElementIterator<'a> {
         self.children()
             .filter_map::<JSRef<Element>>(ElementCast::to_ref)
             .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),
             None => ()
         }
     }
 
     fn get_unique_id(self) -> String {
@@ -1821,24 +1807,22 @@ impl<'a> NodeMethods for JSRef<'a, Node>
                 }.root();
 
                 // Step 3.
                 Node::replace_all(node.root_ref(), self);
             }
             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 => {}
         }
     }
 
     // http://dom.spec.whatwg.org/#dom-node-insertbefore
     fn InsertBefore(self, node: JSRef<Node>, child: Option<JSRef<Node>>) -> Fallible<Temporary<Node>> {
@@ -2361,8 +2345,18 @@ impl<'a> DisabledStateHelpers for JSRef<
 
     fn check_disabled_attribute(self) {
         let elem: JSRef<'a, Element> = ElementCast::to_ref(self).unwrap();
         let has_disabled_attrib = elem.has_attribute(&atom!("disabled"));
         self.set_disabled_state(has_disabled_attrib);
         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,
+}
+
--- a/servo/components/script/dom/window.rs
+++ b/servo/components/script/dom/window.rs
@@ -16,17 +16,17 @@ use dom::browsercontext::BrowserContext;
 use dom::console::Console;
 use dom::document::Document;
 use dom::eventtarget::{EventTarget, WindowTypeId, EventTargetHelpers};
 use dom::location::Location;
 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;
 use script_traits::ScriptControlChan;
 use timers::{Interval, NonInterval, TimerId, TimerManager};
 
 use servo_msg::compositor_msg::ScriptListener;
 use servo_msg::constellation_msg::LoadData;
@@ -294,19 +294,17 @@ impl<'a> WindowMethods for JSRef<'a, Win
 
 impl Reflectable for Window {
     fn reflector<'a>(&'a self) -> &'a Reflector {
         self.eventtarget.reflector()
     }
 }
 
 pub trait WindowHelpers {
-    fn reflow(self);
-    fn flush_layout(self);
-    fn wait_until_safe_to_modify_dom(self);
+    fn flush_layout(self, goal: ReflowGoal, query: ReflowQueryType);
     fn init_browser_context(self, doc: JSRef<Document>);
     fn load_url(self, href: DOMString);
     fn handle_fire_timer(self, timer_id: TimerId);
     fn evaluate_js_with_result(self, code: &str) -> JSVal;
     fn evaluate_script_with_result(self, code: &str, filename: &str) -> JSVal;
 }
 
 
@@ -329,28 +327,18 @@ impl<'a> WindowHelpers for JSRef<'a, Win
                                        filename.as_ptr(), 1, &mut rval) == 0 {
                     debug!("error evaluating JS string");
                 }
                 rval
             }
         })
     }
 
-    fn reflow(self) {
-        self.page().damage();
-    }
-
-    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 flush_layout(self, goal: ReflowGoal, query: ReflowQueryType) {
+        self.page().flush_layout(goal, query);
     }
 
     fn init_browser_context(self, doc: JSRef<Document>) {
         *self.browser_context.borrow_mut() = Some(BrowserContext::new(doc));
     }
 
     /// Commence a new URL load which will either replace this window or scroll to a fragment.
     fn load_url(self, href: DOMString) {
@@ -364,17 +352,17 @@ impl<'a> WindowHelpers for JSRef<'a, Win
             script_chan.send(TriggerFragmentMsg(self.page.id, url));
         } else {
             script_chan.send(TriggerLoadMsg(self.page.id, LoadData::new(url)));
         }
     }
 
     fn handle_fire_timer(self, timer_id: TimerId) {
         self.timers.fire_timer(timer_id, self.clone());
-        self.flush_layout();
+        self.flush_layout(ReflowForDisplay, NoQuery);
     }
 }
 
 impl Window {
     pub fn new(cx: *mut JSContext,
                page: Rc<Page>,
                script_chan: ScriptChan,
                control_chan: ScriptControlChan,
--- a/servo/components/script/page.rs
+++ b/servo/components/script/page.rs
@@ -8,33 +8,33 @@ use dom::bindings::codegen::InheritTypes
 use dom::bindings::js::{JS, JSRef, Temporary, OptionalRootable};
 use dom::bindings::utils::GlobalStaticData;
 use dom::document::{Document, DocumentHelpers};
 use dom::element::Element;
 use dom::node::{Node, NodeHelpers};
 use dom::window::Window;
 use layout_interface::{
     ContentBoxQuery, ContentBoxResponse, ContentBoxesQuery, ContentBoxesResponse,
-    GetRPCMsg, HitTestResponse, LayoutChan, LayoutRPC, MouseOverResponse, NoQuery,
-    Reflow, ReflowForDisplay, ReflowForScriptQuery, ReflowGoal, ReflowMsg,
-    ReflowQueryType, TrustedNodeAddress
+    GetRPCMsg, HitTestResponse, LayoutChan, LayoutRPC, MouseOverResponse, Reflow,
+    ReflowForScriptQuery, ReflowGoal, ReflowMsg, ReflowQueryType,
+    TrustedNodeAddress
 };
 use script_traits::{UntrustedNodeAddress, ScriptControlChan};
 
 use geom::{Point2D, Rect, Size2D};
 use js::rust::Cx;
 use servo_msg::compositor_msg::ScriptListener;
 use servo_msg::constellation_msg::{ConstellationChan, WindowSizeData};
 use servo_msg::constellation_msg::{PipelineId, SubpageId};
 use servo_net::resource_task::ResourceTask;
 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;
 use std::num::abs;
 use std::rc::Rc;
 use url::Url;
 
 /// Encapsulates a handle to a frame and its associated layout information.
@@ -72,43 +72,31 @@ pub struct Page {
     /// when reloading.
     url: DOMRefCell<Option<(Url, bool)>>,
 
     next_subpage_id: Cell<SubpageId>,
 
     /// 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>>,
 
     /// Associated resource task for use by DOM objects like XMLHttpRequest
     pub resource_task: ResourceTask,
 
     /// A handle for communicating messages to the storage task.
     pub storage_task: StorageTask,
 
     /// A handle for communicating messages to the constellation task.
     pub constellation_chan: ConstellationChan,
 
     // 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>>,
 }
 
 pub struct PageIterator {
     stack: Vec<Rc<Page>>,
 }
@@ -160,67 +148,45 @@ impl Page {
             layout_chan: layout_chan,
             layout_rpc: layout_rpc,
             layout_join_port: DOMRefCell::new(None),
             window_size: Cell::new(window_size),
             js_info: DOMRefCell::new(Some(js_info)),
             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) {
-        // 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);
-        } else {
-            self.avoided_reflows.set(self.avoided_reflows.get() + 1);
-        }
+    pub fn flush_layout(&self, goal: ReflowGoal, query: ReflowQueryType) {
+        let frame = self.frame();
+        let window = frame.as_ref().unwrap().window.root();
+        self.reflow(goal, window.control_chan().clone(), &mut **window.compositor(), query);
     }
 
-     pub fn layout(&self) -> &LayoutRPC {
-        self.flush_layout(NoQuery);
-        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 layout(&self) -> &LayoutRPC {
+        &*self.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
     }
 
     // must handle root case separately
     pub fn remove(&self, id: PipelineId) -> Option<Rc<Page>> {
         let remove_idx = {
@@ -271,16 +237,23 @@ impl Page {
             None => {}
             Some(ref frame) => {
                 let window = frame.window.root();
                 let document = frame.document.root();
                 window.compositor().set_title(self.id, Some(document.Title()));
             }
         }
     }
+
+    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 {
     fn next(&mut self) -> Option<Rc<Page>> {
         match self.stack.pop() {
             Some(next) => {
                 for child in next.children.borrow().iter() {
                     self.stack.push(child.clone());
@@ -329,17 +302,17 @@ impl Page {
     }
 
     // FIXME(cgaebel): join_layout is racey. What if the compositor triggers a
     // reflow between the "join complete" message and returning from this
     // function?
 
     /// 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);
             match join_port {
                 Some(ref join_port) => {
                     match join_port.try_recv() {
                         Err(Empty) => {
                             info!("script: waiting on layout");
@@ -353,81 +326,81 @@ impl Page {
 
                     debug!("script: layout joined")
                 }
                 None => panic!("reader forked but no join port?"),
             }
         }
     }
 
-    /// Reflows the page if it's possible to do so. This method will wait until the layout task has
-    /// completed its current action, join the layout task, and then request a new layout run. It
-    /// won't wait for the new layout computation to finish.
+    /// Reflows the page if it's possible to do so and the page is dirty. This method will wait
+    /// for the layout thread to complete (but see the `TODO` below). If there is no window size
+    /// 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,
                   _: &mut ScriptListener,
                   query_type: ReflowQueryType) {
         let root = match *self.frame() {
             None => return,
             Some(ref frame) => {
                 frame.document.root().GetDocumentElement()
             }
         };
 
-        match root.root() {
-            None => {},
-            Some(root) => {
-                debug!("avoided {:d} reflows", self.avoided_reflows.get());
-                self.avoided_reflows.set(0);
+        let root = match root.root() {
+            None => return,
+            Some(root) => root,
+        };
 
-                debug!("script: performing reflow for goal {}", goal);
-
-                // Now, join the layout so that they will see the latest changes we have made.
-                self.join_layout();
+        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 root: JSRef<Node> = NodeCast::from_ref(*root);
+        if !root.get_has_dirty_descendants() {
+            debug!("root has no dirty descendants; avoiding reflow");
+            return
+        }
+
+        debug!("script: performing reflow for goal {}", goal);
 
-                let last_reflow_id = &self.last_reflow_id;
-                last_reflow_id.set(last_reflow_id.get() + 1);
+        // Layout will let us know when it's done.
+        let (join_chan, join_port) = channel();
 
-                let root: JSRef<Node> = NodeCast::from_ref(*root);
-
-                let window_size = self.window_size.get();
-                self.damaged.set(false);
+        {
+            let mut layout_join_port = self.layout_join_port.borrow_mut();
+            *layout_join_port = Some(join_port);
+        }
 
-                // Send new document and relevant styles to layout.
-                let reflow = box Reflow {
-                    document_root: root.to_trusted_node_address(),
-                    url: self.get_url(),
-                    iframe: self.subpage_id.is_some(),
-                    goal: goal,
-                    window_size: window_size,
-                    script_chan: script_chan,
-                    script_join_chan: join_chan,
-                    id: last_reflow_id.get(),
-                    query_type: query_type,
-                    page_clip_rect: self.page_clip_rect.get(),
-                };
+        let last_reflow_id = &self.last_reflow_id;
+        last_reflow_id.set(last_reflow_id.get() + 1);
+
+        let window_size = self.window_size.get();
 
-                let LayoutChan(ref chan) = self.layout_chan;
-                chan.send(ReflowMsg(reflow));
+        // Send new document and relevant styles to layout.
+        let reflow = box Reflow {
+            document_root: root.to_trusted_node_address(),
+            url: self.get_url(),
+            iframe: self.subpage_id.is_some(),
+            goal: goal,
+            window_size: window_size,
+            script_chan: script_chan,
+            script_join_chan: join_chan,
+            id: last_reflow_id.get(),
+            query_type: query_type,
+            page_clip_rect: self.page_clip_rect.get(),
+        };
 
-                debug!("script: layout forked")
-            }
-        }
-    }
+        let LayoutChan(ref chan) = self.layout_chan;
+        chan.send(ReflowMsg(reflow));
 
-    pub fn damage(&self) {
-        self.damaged.set(true);
+        debug!("script: layout forked");
+
+        self.join_layout();
     }
 
     /// Attempt to find a named element in this page's document.
     pub fn find_fragment_node(&self, fragid: DOMString) -> Option<Temporary<Element>> {
         let document = self.frame().as_ref().unwrap().document.root();
         document.find_fragment_node(fragid)
     }
 
--- a/servo/components/script/script_task.rs
+++ b/servo/components/script/script_task.rs
@@ -18,68 +18,65 @@ use dom::bindings::trace::JSTraceable;
 use dom::bindings::utils::{wrap_for_same_compartment, pre_wrap};
 use dom::document::{Document, HTMLDocument, DocumentHelpers, FromParser};
 use dom::element::{Element, HTMLButtonElementTypeId, HTMLInputElementTypeId};
 use dom::element::{HTMLSelectElementTypeId, HTMLTextAreaElementTypeId, HTMLOptionElementTypeId, ActivationElementHelpers};
 use dom::event::{Event, EventHelpers, Bubbles, DoesNotBubble, Cancelable, NotCancelable};
 use dom::uievent::UIEvent;
 use dom::eventtarget::{EventTarget, EventTargetHelpers};
 use dom::keyboardevent::KeyboardEvent;
-use dom::node;
-use dom::node::{ElementNodeTypeId, Node, NodeHelpers};
+use dom::node::{mod, ElementNodeTypeId, Node, NodeHelpers, OtherNodeDamage};
 use dom::window::{Window, WindowHelpers};
 use dom::worker::{Worker, TrustedWorkerAddress};
 use dom::xmlhttprequest::{TrustedXHRAddress, XMLHttpRequest, XHRProgress};
 use parse::html::{InputString, InputUrl, parse_html};
 use layout_interface::{ScriptLayoutChan, LayoutChan, NoQuery, ReflowForDisplay};
 use layout_interface;
 use page::{Page, IterablePage, Frame};
 use timers::TimerId;
 use devtools;
 
 use devtools_traits::{DevtoolsControlChan, DevtoolsControlPort, NewGlobal, GetRootNode, DevtoolsPageInfo};
 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::{GetTitleMsg};
+use script_traits::{ScriptControlChan, ReflowCompleteMsg, SendEventMsg};
 use servo_msg::compositor_msg::{FinishedLoading, LayerId, Loading, PerformingLayout};
 use servo_msg::compositor_msg::{ScriptListener};
 use servo_msg::constellation_msg::{ConstellationChan, LoadCompleteMsg};
 use servo_msg::constellation_msg::{LoadData, LoadUrlMsg, NavigationDirection, PipelineId};
 use servo_msg::constellation_msg::{Failure, FailureMsg, WindowSizeData, Key, KeyState};
 use servo_msg::constellation_msg::{KeyModifiers, SUPER, SHIFT, CONTROL, ALT, Repeated, Pressed};
 use servo_msg::constellation_msg::{Released};
 use servo_msg::constellation_msg;
 use servo_net::image_cache_task::ImageCacheTask;
 use servo_net::resource_task::{ResourceTask, Load};
 use servo_net::resource_task::LoadData as NetLoadData;
 use servo_net::storage_task::StorageTask;
 use servo_util::geometry::to_frac_px;
-use servo_util::smallvec::{SmallVec1, SmallVec};
+use servo_util::smallvec::SmallVec;
 use servo_util::task::spawn_named_with_send_on_failure;
 use servo_util::task_state;
 
 use geom::point::Point2D;
 use hyper::header::{Header, HeaderFormat};
 use hyper::header::common::util as header_util;
 use js::jsapi::{JS_SetWrapObjectCallbacks, JS_SetGCZeal, JS_DEFAULT_ZEAL_FREQ, JS_GC};
 use js::jsapi::{JSContext, JSRuntime, JSTracer};
 use js::jsapi::{JS_SetGCParameter, JSGC_MAX_BYTES};
 use js::jsapi::{JS_SetGCCallback, JSGCStatus, JSGC_BEGIN, JSGC_END};
 use js::rust::{Cx, RtUtils};
 use js;
 use url::Url;
 
 use libc::size_t;
 use std::any::{Any, AnyRefExt};
-use std::collections::HashSet;
 use std::comm::{channel, Sender, Receiver, Select};
 use std::fmt::{mod, Show};
 use std::mem::replace;
 use std::rc::Rc;
 use std::u32;
 use time::{Tm, strptime};
 
 local_data_key!(pub StackRoots: *const RootCollection)
@@ -475,44 +472,36 @@ impl ScriptTask {
                 FromConstellation(self.control_port.recv())
             } else if ret == port3.id() {
                 FromDevtools(self.devtools_port.recv())
             } else {
                 panic!("unexpected select result")
             }
         };
 
-        let mut needs_reflow = HashSet::new();
-
         // Squash any pending resize and reflow events in the queue.
         loop {
             match event {
                 // This has to be handled before the ResizeMsg below,
                 // otherwise the page may not have been added to the
                 // child list yet, causing the find() to fail.
                 FromConstellation(AttachLayoutMsg(new_layout_info)) => {
                     self.handle_new_layout(new_layout_info);
                 }
                 FromConstellation(ResizeMsg(id, size)) => {
                     let page = self.page.borrow_mut();
                     let page = page.find(id).expect("resize sent to nonexistent pipeline");
                     page.resize_event.set(Some(size));
                 }
-                FromConstellation(SendEventMsg(id, ReflowEvent(node_addresses))) => {
-                    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)) => {
                     let page = self.page.borrow_mut();
                     let inner_page = page.find(id).expect("Page rect message sent to nonexistent pipeline");
                     if inner_page.set_page_clip_rect_with_new_viewport(rect) {
-                        needs_reflow.insert(id);
+                        let page = get_page(&*self.page.borrow(), id);
+                        self.force_reflow(&*page);
                     }
                 }
                 _ => {
                     sequential.push(event);
                 }
             }
 
             // If any of our input sources has an event pending, we'll perform another iteration
@@ -536,21 +525,16 @@ impl ScriptTask {
                 FromConstellation(ExitPipelineMsg(id)) =>
                     if self.handle_exit_pipeline_msg(id) { return false },
                 FromConstellation(inner_msg) => self.handle_msg_from_constellation(inner_msg),
                 FromScript(inner_msg) => self.handle_msg_from_script(inner_msg),
                 FromDevtools(inner_msg) => self.handle_msg_from_devtools(inner_msg),
             }
         }
 
-        // Now process any pending reflows.
-        for id in needs_reflow.into_iter() {
-            self.handle_event(id, ReflowEvent(SmallVec1::new()));
-        }
-
         true
     }
 
     fn handle_msg_from_constellation(&self, msg: ConstellationControlMsg) {
         match msg {
             // TODO(tkuehn) need to handle auxiliary layouts for iframes
             AttachLayoutMsg(_) =>
                 panic!("should have handled AttachLayoutMsg already"),
@@ -661,21 +645,16 @@ impl ScriptTask {
              with this script task. This is a bug.");
         let last_reflow_id = page.last_reflow_id.get();
         if last_reflow_id == reflow_id {
             let mut layout_join_port = page.layout_join_port.borrow_mut();
             *layout_join_port = None;
         }
 
         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.
     /// TODO(tkuehn): is it ever possible to navigate only on a subframe?
     fn handle_navigate_msg(&self, direction: NavigationDirection) {
         let ConstellationChan(ref chan) = self.constellation_chan;
         chan.send(constellation_msg::NavigateMsg(direction));
     }
@@ -833,18 +812,22 @@ impl ScriptTask {
 
         parse_html(*document, parser_input, &final_url);
 
         document.set_ready_state(DocumentReadyStateValues::Interactive);
         self.compositor.borrow_mut().set_ready_state(pipeline_id, PerformingLayout);
 
         // 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
             let mut page_url = page.mut_url();
             *page_url = Some((final_url.clone(), false));
         }
 
         // https://html.spec.whatwg.org/multipage/#the-end step 4
@@ -890,46 +873,55 @@ impl ScriptTask {
                             to_frac_px(rect.origin.y).to_f32().unwrap());
         // FIXME(#2003, pcwalton): This is pretty bogus when multiple layers are involved.
         // Really what needs to happen is that this needs to go through layout to ask which
         // layer the element belongs to, and have it send the scroll message to the
         // compositor.
         self.compositor.borrow_mut().scroll_fragment_point(pipeline_id, LayerId::null(), point);
     }
 
+    /// Reflows non-incrementally.
     fn force_reflow(&self, page: &Page) {
-        {
-            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.dirty_all_nodes();
         page.reflow(ReflowForDisplay,
                     self.control_chan.clone(),
                     &mut **self.compositor.borrow_mut(),
                     NoQuery);
     }
 
     /// This is the main entry point for receiving and dispatching DOM events.
     ///
     /// TODO: Actually perform DOM event dispatch.
     fn handle_event(&self, pipeline_id: PipelineId, event: CompositorEvent) {
         match event {
             ResizeEvent(new_size) => {
               self.handle_resize_event(pipeline_id, new_size);
             }
 
-            // FIXME(pcwalton): This reflows the entire document and is not incremental-y.
-            ReflowEvent(to_dirty) => {
-              self.handle_reflow_event(pipeline_id, to_dirty);
+            ReflowEvent(nodes) => {
+                // FIXME(pcwalton): This event seems to only be used by the image cache task, and
+                // the interaction between it and the image holder is really racy. I think that, in
+                // order to fix this race, we need to rewrite the image cache task to make the
+                // image holder responsible for the lifecycle of image loading instead of having
+                // the image holder and layout task both be observers. Then we can have the DOM
+                // image element observe the state of the image holder and have it send reflows
+                // via the normal dirtying mechanism, and ultimately remove this event.
+                //
+                // See the implementation of `Width()` and `Height()` in `HTMLImageElement` for
+                // fallout of this problem.
+                for node in nodes.iter() {
+                    let node_to_dirty = node::from_untrusted_node_address(self.js_runtime.ptr,
+                                                                          *node).root();
+                    let page = get_page(&*self.page.borrow(), pipeline_id);
+                    let frame = page.frame();
+                    let document = frame.as_ref().unwrap().document.root();
+                    document.content_changed(*node_to_dirty, OtherNodeDamage);
+                }
+
+                self.handle_reflow_event(pipeline_id);
             }
 
             ClickEvent(_button, point) => {
               self.handle_click_event(pipeline_id, _button, point);
             }
 
             MouseDownEvent(..) => {}
             MouseUpEvent(..) => {}
@@ -1015,17 +1007,17 @@ impl ScriptTask {
             }
             Key::KeyEnter if !prevented && state == Released => {
                 let maybe_elem: Option<JSRef<Element>> = ElementCast::to_ref(target);
                 maybe_elem.map(|el| el.as_maybe_activatable().map(|a| a.implicit_submission(ctrl, alt, shift, meta)));
             }
             _ => ()
         }
 
-        window.flush_layout();
+        window.flush_layout(ReflowForDisplay, NoQuery);
     }
 
     /// The entry point for content to notify that a new load has been requested
     /// for the given pipeline.
     fn trigger_load(&self, pipeline_id: PipelineId, load_data: LoadData) {
         let ConstellationChan(ref const_chan) = self.constellation_chan;
         const_chan.send(LoadUrlMsg(pipeline_id, load_data));
     }
@@ -1079,28 +1071,22 @@ impl ScriptTask {
 
                 let wintarget: JSRef<EventTarget> = EventTargetCast::from_ref(*window);
                 let _ = wintarget.dispatch_event_with_target(None, event);
             }
             None => ()
         }
     }
 
-    fn handle_reflow_event(&self, pipeline_id: PipelineId, to_dirty: SmallVec1<UntrustedNodeAddress>) {
+    fn handle_reflow_event(&self, pipeline_id: PipelineId) {
         debug!("script got reflow event");
-        assert_eq!(to_dirty.len(), 0);
         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);
-            }
+            self.force_reflow(&*page);
         }
     }
 
     fn handle_click_event(&self, pipeline_id: PipelineId, _button: uint, point: Point2D<f32>) {
         debug!("ClickEvent: clicked at {}", point);
         let page = get_page(&*self.page.borrow(), pipeline_id);
         match page.hit_test(&point) {
             Some(node_address) => {
@@ -1133,17 +1119,17 @@ impl ScriptTask {
                                                Bubbles, Cancelable).root();
                                 // https://dvcs.w3.org/hg/dom3events/raw-file/tip/html/DOM3-Events.html#trusted-events
                                 event.set_trusted(true);
                                 // https://html.spec.whatwg.org/multipage/interaction.html#run-authentic-click-activation-steps
                                 let el = ElementCast::to_ref(node).unwrap(); // is_element() check already exists above
                                 el.authentic_click_activation(*event);
 
                                 doc.commit_focus_transaction();
-                                window.flush_layout();
+                                window.flush_layout(ReflowForDisplay, NoQuery);
                             }
                             None => {}
                         }
                     }
                     None => {}
                 }
             }
 
@@ -1215,18 +1201,16 @@ impl ScriptTask {
             None => {}
         }
     }
 }
 
 /// 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();
         let LayoutChan(ref chan) = page.layout_chan;
         chan.send(layout_interface::PrepareToExitMsg(response_chan));
         response_port.recv();
     }
 
--- a/servo/tests/content/test_getBoundingClientRect.html
+++ b/servo/tests/content/test_getBoundingClientRect.html
@@ -1,15 +1,16 @@
 <html>
 <head>
 <script src="harness.js"></script>
 <style>
 div {
-    margin-top:  100px;
-    margin-left: 100px;
+    position: relative;
+    top:  100px;
+    left: 100px;
     width: 100px;
     height: 100px;
 }
 </style>
 </head>
 <body>
     <div>my div</div>
     <script>