auto merge of #1394 : kmcallister/servo/style-damage, r=pcwalton

This fixes the computation of restyle damage on `color-change-text.html`, which can be seen with `RUST_LOG=servo::layout::layout_task`.

However we can't prune the layout traversals yet, because we don't reuse `Flow` objects between reflows, so we have no old values to fall back to.

I think this used to work because `FlowContexts` (as they were called then) were stored in a DOM node's `LayoutData` and reused.  But it's possible that it never really worked, and my testing when I landed the restyle damage code was insufficient (I didn't understand the layout code nearly as well back then).

r? @pcwalton

src/components/main/layout/layout_task.rs

@@ -15,7 +15,7 @@ use layout::extra::LayoutAuxMethods;
 use layout::flow::{Flow, ImmutableFlowUtils, MutableFlowUtils, PreorderFlowTraversal};
 use layout::flow::{PostorderFlowTraversal};
 use layout::flow;
-use layout::incremental::{RestyleDamage, BubbleWidths};
+use layout::incremental::{RestyleDamage};
 use layout::util::{LayoutData, LayoutDataAccess};
 
 use extra::arc::{Arc, RWArc, MutexArc};
@@ -100,7 +100,7 @@ impl PostorderFlowTraversal for ComputeDamageTraversal {
     fn process(&mut self, flow: &mut Flow) -> bool {
         let mut damage = flow::base(flow).restyle_damage;
         for child in flow::child_iter(flow) {
-            damage.union_in_place(flow::base(*child).restyle_damage)
+            damage.union_in_place(flow::base(*child).restyle_damage.propagate_up())
         }
         flow::mut_base(flow).restyle_damage = damage;
         true
@@ -120,6 +120,7 @@ impl PreorderFlowTraversal for PropagateDamageTraversal {
         if self.all_style_damage {
             flow::mut_base(flow).restyle_damage.union_in_place(RestyleDamage::all())
         }
+        debug!("restyle damage = {:?}", flow::base(flow).restyle_damage);
 
         let prop = flow::base(flow).restyle_damage.propagate_down();
         if prop.is_nonempty() {
@@ -142,10 +143,13 @@ impl<'self> PostorderFlowTraversal for BubbleWidthsTraversal<'self> {
         true
     }
 
+    // FIXME: We can't prune until we start reusing flows
+    /*
     #[inline]
     fn should_prune(&mut self, flow: &mut Flow) -> bool {
         flow::mut_base(flow).restyle_damage.lacks(BubbleWidths)
     }
+    */
 }
 
 /// The assign-widths traversal. In Gecko this corresponds to `Reflow`.
@@ -371,10 +375,9 @@ impl LayoutTask {
                          layout_context: &mut LayoutContext) {
         let _ = layout_root.traverse_postorder(&mut BubbleWidthsTraversal(layout_context));
 
-        // FIXME(kmc): We want to do
+        // FIXME(kmc): We want to prune nodes without the Reflow restyle damage
-        //     for flow in layout_root.traverse_preorder_prune(|f|
+        // bit, but FloatContext values can't be reused, so we need to
-        //          f.restyle_damage().lacks(Reflow)) 
+        // recompute them every time.
-        // but FloatContext values can't be reused, so we need to recompute them every time.
         // NOTE: this currently computes borders, so any pruning should separate that operation out.
         let _ = layout_root.traverse_preorder(&mut AssignWidthsTraversal(layout_context));
 

src/components/script/dom/document.rs

@@ -23,6 +23,7 @@ use dom::htmltitleelement::HTMLTitleElement;
 use html::hubbub_html_parser::build_element_from_tag;
 use js::jsapi::{JSObject, JSContext, JSTracer};
 use servo_util::tree::{TreeNodeRef, ElementLike};
+use layout_interface::{DocumentDamageLevel, ContentChangedDocumentDamage};
 
 use std::hashmap::HashMap;
 
@@ -319,7 +320,11 @@ impl Document {
     }
 
     pub fn content_changed(&self) {
-        self.window.content_changed();
+        self.damage_and_reflow(ContentChangedDocumentDamage);
+    }
+
+    pub fn damage_and_reflow(&self, damage: DocumentDamageLevel) {
+        self.window.damage_and_reflow(damage);
     }
 
     pub fn wait_until_safe_to_modify_dom(&self) {

src/components/script/dom/element.rs

@@ -18,7 +18,8 @@ use dom::document;
 use dom::namespace;
 use dom::namespace::Namespace;
 use layout_interface::{ContentBoxQuery, ContentBoxResponse, ContentBoxesQuery};
-use layout_interface::{ContentBoxesResponse};
+use layout_interface::{ContentBoxesResponse, ContentChangedDocumentDamage};
+use layout_interface::{MatchSelectorsDocumentDamage};
 use style;
 use servo_util::tree::{TreeNodeRef, ElementLike};
 
@@ -252,19 +253,20 @@ impl<'self> Element {
                               }
                           });
 
-        self.after_set_attr(abstract_self, &namespace, local_name, value, old_raw_value);
+        if namespace == namespace::Null {
+            self.after_set_attr(abstract_self, local_name, value, old_raw_value);
+        }
         Ok(())
     }
 
     fn after_set_attr(&mut self,
                       abstract_self: AbstractNode<ScriptView>,
-                      namespace: &Namespace,
                       local_name: DOMString,
                       value: DOMString,
                       old_value: Option<DOMString>) {
 
         match local_name.as_slice() {
-            "style" if *namespace == namespace::Null => {
+            "style" => {
                 self.style_attribute = Some(style::parse_style_attribute(value))
             }
             "id" => {
@@ -292,8 +294,12 @@ impl<'self> Element {
         }
 
         if abstract_self.is_in_doc() {
+            let damage = match local_name.as_slice() {
+                "style" | "id" | "class" => MatchSelectorsDocumentDamage,
+                _ => ContentChangedDocumentDamage
+            };
             let document = self.node.owner_doc();
-            document.document().content_changed();
+            document.document().damage_and_reflow(damage);
         }
     }
 }

src/components/script/dom/window.rs

@@ -12,7 +12,7 @@ use dom::node::{AbstractNode, ScriptView};
 use dom::location::Location;
 use dom::navigator::Navigator;
 
-use layout_interface::ReflowForDisplay;
+use layout_interface::{ReflowForDisplay, DocumentDamageLevel};
 use script_task::{ExitWindowMsg, FireTimerMsg, Page, ScriptChan};
 use servo_msg::compositor_msg::ScriptListener;
 use servo_net::image_cache_task::ImageCacheTask;
@@ -185,11 +185,12 @@ impl Window {
         self.active_timers.remove(&handle);
     }
 
-    pub fn content_changed(&self) {
+    pub fn damage_and_reflow(&self, damage: DocumentDamageLevel) {
         // FIXME This should probably be ReflowForQuery, not Display. All queries currently
         // currently rely on the display list, which means we can't destroy it by
         // doing a query reflow.
-        self.page.reflow_all(ReflowForDisplay, self.script_chan.clone(), self.compositor);
+        self.page.damage(damage);
+        self.page.reflow(ReflowForDisplay, self.script_chan.clone(), self.compositor);
     }
 
     pub fn wait_until_safe_to_modify_dom(&self) {

src/components/script/script_task.rs

@@ -224,8 +224,11 @@ impl<'self> Iterator<@mut Page> for PageTreeIterator<'self> {
 
 impl Page {
     /// Adds the given damage.
-    fn damage(&mut self, level: DocumentDamageLevel) {
+    pub fn damage(&mut self, level: DocumentDamageLevel) {
-        let root = self.frame.get_ref().document.document().GetDocumentElement();
+        let root = match self.frame {
+            None => return,
+            Some(ref frame) => frame.document.document().GetDocumentElement()
+        };
         match root {
             None => {},
             Some(root) => {
@@ -282,7 +285,7 @@ impl Page {
     /// computation to finish.
     ///
     /// This function fails if there is no root frame.
-    fn reflow(&mut self, goal: ReflowGoal, script_chan: ScriptChan, compositor: @ScriptListener) {
+    pub fn reflow(&mut self, goal: ReflowGoal, script_chan: ScriptChan, compositor: @ScriptListener) {
         let root = match self.frame {
             None => return,
             Some(ref frame) => {
@@ -325,19 +328,6 @@ impl Page {
         }
     }
 
-    /// Reflows the entire document.
-    ///
-    /// FIXME: This should basically never be used.
-    pub fn reflow_all(&mut self, goal: ReflowGoal, script_chan: ScriptChan, compositor: @ScriptListener) {
-        if self.frame.is_some() {
-            self.damage(ContentChangedDocumentDamage);
-        }
-
-        //FIXME: In the case where an initial reflow is required, we should always
-        //       ReflowForDisplay, regardless of the original goal.
-        self.reflow(goal, script_chan, compositor)
-    }
-
     pub fn initialize_js_info(&mut self, js_context: @Cx, global: *JSObject) {
         // Note that the order that these variables are initialized is _not_ arbitrary. Switching them around
         // can -- and likely will -- lead to things breaking.
@@ -598,8 +588,6 @@ impl ScriptTask {
                                  &rval);
 
         }
-        // We don't know what the script changed, so for now we will do a total redisplay.
-        page.reflow_all(ReflowForDisplay, self.chan.clone(), self.compositor);
     }
 
     /// Handles a notification that reflow completed.
@@ -683,7 +671,8 @@ impl ScriptTask {
             if *loaded == url {
                 page.url = Some((loaded.clone(), false));
                 if needs_reflow {
-                    page.reflow_all(ReflowForDisplay, self.chan.clone(), self.compositor);
+                    page.damage(ContentChangedDocumentDamage);
+                    page.reflow(ReflowForDisplay, self.chan.clone(), self.compositor);
                 }
                 return;
             }