Auto merge of #12563 - emilio:stylo, r=bholley,jdm,pcwalton

stylo: Improve restyling performance

This commit adds hooks to the Servo style traversal to avoid traversing all the
DOM for every restyle. Additionally it changes the behavior of the dirty flag to
be propagated top down, to prevent extra overhead when an element is dirtied.

This commit doesn't aim to change the behavior on Servo just yet, since Servo does extra job when dirtying the node related with DOM revision counters that might be necessary.

CC @asajeffrey for the DOM revision counters stuff. When a node is dirty, do all its descendants really need to increment the revision counter, or is this an unintended effect? My intuition is that this is hurting performance quite a lot for servo.

r? @bholley

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors

<!-- Either: -->
- [x] These changes do not require tests because no geckolib tests yet.

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12563)
<!-- Reviewable:end -->

components/compositing/compositor.rs

@@ -2114,7 +2114,7 @@ impl<Window: WindowMethods> IOCompositor<Window> {
                 }
 
                 // Check if there are any pending frames. If so, the image is not stable yet.
-                if self.pending_subpages.len() > 0 {
+                if !self.pending_subpages.is_empty() {
                     return Err(NotReadyToPaint::PendingSubpages(self.pending_subpages.len()));
                 }
 

components/constellation/constellation.rs

@@ -828,11 +828,6 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
                 debug!("constellation got load complete message");
                 self.handle_load_complete_msg(pipeline_id)
             }
-            // The DOM load event fired on a document
-            FromScriptMsg::DOMLoad(pipeline_id) => {
-                debug!("constellation got dom load message");
-                self.handle_dom_load(pipeline_id)
-            }
             // Handle a forward or back request
             FromScriptMsg::TraverseHistory(pipeline_id, direction) => {
                 debug!("constellation got traverse history message from script");
@@ -1323,10 +1318,13 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
         }
     }
 
-    fn handle_alert(&mut self, pipeline_id: PipelineId, message: String, sender: IpcSender<bool>) {
+    fn handle_alert(&mut self,
+                    pipeline_id: PipelineId,
+                    message: String,
+                    sender: IpcSender<bool>) {
         let display_alert_dialog = if PREFS.is_mozbrowser_enabled() {
             let parent_pipeline_info = self.pipelines.get(&pipeline_id).and_then(|source| source.parent_info);
-            if let Some(_) = parent_pipeline_info {
+            if parent_pipeline_info.is_some() {
                 let root_pipeline_id = self.root_frame_id
                     .and_then(|root_frame_id| self.frames.get(&root_frame_id))
                     .map(|root_frame| root_frame.current.0);
@@ -1435,15 +1433,6 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
     }
 
     fn handle_load_complete_msg(&mut self, pipeline_id: PipelineId) {
-        if let Some(frame_id) = self.get_top_level_frame_for_pipeline(Some(pipeline_id)) {
-            let forward = !self.joint_session_future(frame_id).is_empty();
-            let back = !self.joint_session_past(frame_id).is_empty();
-            let root = self.root_frame_id.is_none() || self.root_frame_id == Some(frame_id);
-            self.compositor_proxy.send(ToCompositorMsg::LoadComplete(back, forward, root));
-        }
-    }
-
-    fn handle_dom_load(&mut self, pipeline_id: PipelineId) {
         let mut webdriver_reset = false;
         if let Some((expected_pipeline_id, ref reply_chan)) = self.webdriver.load_channel {
             debug!("Sending load to WebDriver");
@@ -1455,7 +1444,12 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
         if webdriver_reset {
             self.webdriver.load_channel = None;
         }
-
+        if let Some(frame_id) = self.get_top_level_frame_for_pipeline(Some(pipeline_id)) {
+            let forward = !self.joint_session_future(frame_id).is_empty();
+            let back = !self.joint_session_past(frame_id).is_empty();
+            let root = self.root_frame_id.is_none() || self.root_frame_id == Some(frame_id);
+            self.compositor_proxy.send(ToCompositorMsg::LoadComplete(back, forward, root));
+        }
         self.handle_subframe_loaded(pipeline_id);
     }
 
@@ -2073,7 +2067,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
         }
 
         // If there are pending loads, wait for those to complete.
-        if self.pending_frames.len() > 0 {
+        if !self.pending_frames.is_empty() {
             return ReadyToSave::PendingFrames;
         }
 
@@ -2089,7 +2083,10 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
             let pipeline_id = frame.current.0;
 
             let pipeline = match self.pipelines.get(&pipeline_id) {
-                None => { warn!("Pipeline {:?} screenshot while closing.", pipeline_id); continue; },
+                None => {
+                    warn!("Pipeline {:?} screenshot while closing.", pipeline_id);
+                    continue;
+                },
                 Some(pipeline) => pipeline,
             };
 

components/constellation/pipeline.rs

@@ -503,6 +503,10 @@ impl UnprivilegedPipelineContent {
                 command.env("RUST_BACKTRACE", value);
             }
 
+            if let Ok(value) = env::var("RUST_LOG") {
+                command.env("RUST_LOG", value);
+            }
+
             let profile = content_process_sandbox_profile();
             ChildProcess::Sandboxed(Sandbox::new(profile).start(&mut command)
                                     .expect("Failed to start sandboxed child process!"))
@@ -517,6 +521,10 @@ impl UnprivilegedPipelineContent {
                 child_process.env("RUST_BACKTRACE", value);
             }
 
+            if let Ok(value) = env::var("RUST_LOG") {
+                child_process.env("RUST_LOG", value);
+            }
+
             ChildProcess::Unsandboxed(child_process.spawn()
                                       .expect("Failed to start unsandboxed child process!"))
         };

components/gfx/paint_thread.rs

@@ -379,8 +379,7 @@ impl<C> PaintThread<C> where C: PaintListener + Send + 'static {
                   font_cache_thread: FontCacheThread,
                   time_profiler_chan: time::ProfilerChan,
                   mem_profiler_chan: mem::ProfilerChan) {
-        thread::spawn_named(format!("PaintThread {:?}", id),
-                            move || {
+        thread::spawn_named(format!("PaintThread {:?}", id), move || {
             thread_state::initialize(thread_state::PAINT);
             PipelineId::install(id);
 
@@ -425,9 +424,9 @@ impl<C> PaintThread<C> where C: PaintListener + Send + 'static {
                 let chrome_to_paint = &self.chrome_to_paint_port;
                 select! {
                     msg = layout_to_paint.recv() =>
-                        Msg::FromLayout(msg.unwrap()),
+                        Msg::FromLayout(msg.expect("expected message from layout")),
                     msg = chrome_to_paint.recv() =>
-                        Msg::FromChrome(msg.unwrap())
+                        Msg::FromChrome(msg.expect("expected message from chrome"))
                 }
             };
 

components/layout/traversal.rs

@@ -77,7 +77,9 @@ impl<'lc, N> DomTraversalContext<N> for RecalcStyleAndConstructFlows<'lc>
         recalc_style_at(&self.context, self.root, node);
     }
 
-    fn process_postorder(&self, node: N) { construct_flows_at(&self.context, self.root, node); }
+    fn process_postorder(&self, node: N) {
+        construct_flows_at(&self.context, self.root, node);
+    }
 }
 
 /// A bottom-up, parallelizable traversal.
@@ -96,7 +98,7 @@ fn construct_flows_at<'a, N: LayoutNode>(context: &'a LayoutContext<'a>, root: O
 
         // Always reconstruct if incremental layout is turned off.
         let nonincremental_layout = opts::get().nonincremental_layout;
-        if nonincremental_layout || node.has_dirty_descendants() {
+        if nonincremental_layout || node.is_dirty() || node.has_dirty_descendants() {
             let mut flow_constructor = FlowConstructor::new(context);
             if nonincremental_layout || !flow_constructor.repair_if_possible(&tnode) {
                 flow_constructor.process(&tnode);

components/layout_thread/lib.rs

@@ -1105,12 +1105,16 @@ impl LayoutThread {
                        .unwrap();
             }
             if data.document_stylesheets.iter().any(|sheet| sheet.dirty_on_viewport_size_change) {
-                for node in node.traverse_preorder() {
+                let mut iter = node.traverse_preorder();
+
+                let mut next = iter.next();
+                while let Some(node) = next {
                     if node.needs_dirty_on_viewport_size_changed() {
-                        node.dirty_self();
-                        node.dirty_descendants();
-                        // TODO(shinglyu): We can skip the traversal if the descendants were already
-                        // dirtied
+                        // NB: The dirty bit is propagated down the tree.
+                        unsafe { node.set_dirty(true); }
+                        next = iter.next_skipping_children();
+                    } else {
+                        next = iter.next();
                     }
                 }
             }
@@ -1122,7 +1126,9 @@ impl LayoutThread {
         let needs_reflow = viewport_size_changed && !needs_dirtying;
         unsafe {
             if needs_dirtying {
-                LayoutThread::dirty_all_nodes(node);
+                // NB: The dirty flag is propagated down during the restyle
+                // process.
+                node.set_dirty(true);
             }
         }
         if needs_reflow {
@@ -1476,16 +1482,6 @@ impl LayoutThread {
         }
     }
 
-    unsafe fn dirty_all_nodes<N: LayoutNode>(node: N) {
-        for node in node.traverse_preorder() {
-            // TODO(cgaebel): mark nodes which are sensitive to media queries as
-            // "changed":
-            // > node.set_changed(true);
-            node.set_dirty(true);
-            node.set_dirty_descendants(true);
-        }
-    }
-
     fn reflow_all_nodes(flow: &mut Flow) {
         debug!("reflowing all nodes!");
         flow::mut_base(flow).restyle_damage.insert(REPAINT | STORE_OVERFLOW | REFLOW);

components/script/dom/document.rs

@@ -376,6 +376,7 @@ impl Document {
         // that workable.
         match self.GetDocumentElement() {
             Some(root) => {
+                root.upcast::<Node>().is_dirty() ||
                 root.upcast::<Node>().has_dirty_descendants() ||
                 !self.modified_elements.borrow().is_empty()
             }
@@ -1371,6 +1372,7 @@ impl Document {
     }
 
     pub fn finish_load(&self, load: LoadType) {
+        debug!("Document got finish_load: {:?}", load);
         // The parser might need the loader, so restrict the lifetime of the borrow.
         {
             let mut loader = self.loader.borrow_mut();
@@ -1396,9 +1398,9 @@ impl Document {
             // If we don't have a parser, and the reflow timer has been reset, explicitly
             // trigger a reflow.
             if let LoadType::Stylesheet(_) = load {
-                self.window().reflow(ReflowGoal::ForDisplay,
-                                     ReflowQueryType::NoQuery,
-                                     ReflowReason::StylesheetLoaded);
+                self.window.reflow(ReflowGoal::ForDisplay,
+                                   ReflowQueryType::NoQuery,
+                                   ReflowReason::StylesheetLoaded);
             }
         }
 
@@ -1487,24 +1489,25 @@ impl Document {
             return;
         }
         self.domcontentloaded_dispatched.set(true);
+        assert!(self.ReadyState() != DocumentReadyState::Complete,
+                "Complete before DOMContentLoaded?");
 
         update_with_current_time_ms(&self.dom_content_loaded_event_start);
 
         let window = self.window();
         window.dom_manipulation_task_source().queue_event(self.upcast(), atom!("DOMContentLoaded"),
             EventBubbles::Bubbles, EventCancelable::NotCancelable, window);
+
         window.reflow(ReflowGoal::ForDisplay,
                       ReflowQueryType::NoQuery,
                       ReflowReason::DOMContentLoaded);
-
         update_with_current_time_ms(&self.dom_content_loaded_event_end);
     }
 
     pub fn notify_constellation_load(&self) {
         let pipeline_id = self.window.pipeline();
-        let event = ConstellationMsg::DOMLoad(pipeline_id);
-        self.window.constellation_chan().send(event).unwrap();
-
+        let load_event = ConstellationMsg::LoadComplete(pipeline_id);
+        self.window.constellation_chan().send(load_event).unwrap();
     }
 
     pub fn set_current_parser(&self, script: Option<ParserRef>) {
@@ -2908,16 +2911,18 @@ impl DocumentProgressHandler {
         // http://w3c.github.io/navigation-timing/#widl-PerformanceNavigationTiming-loadEventStart
         update_with_current_time_ms(&document.load_event_start);
 
+        debug!("About to dispatch load for {:?}", document.url());
         let _ = wintarget.dispatch_event_with_target(document.upcast(), &event);
 
         // http://w3c.github.io/navigation-timing/#widl-PerformanceNavigationTiming-loadEventEnd
         update_with_current_time_ms(&document.load_event_end);
 
-        document.notify_constellation_load();
 
         window.reflow(ReflowGoal::ForDisplay,
                       ReflowQueryType::NoQuery,
                       ReflowReason::DocumentLoaded);
+
+        document.notify_constellation_load();
     }
 }
 

components/script/dom/node.rs

@@ -479,19 +479,7 @@ impl Node {
             return
         }
 
-        // 2. Dirty descendants.
-        fn dirty_subtree(node: &Node) {
-            // Stop if this subtree is already dirty.
-            if node.is_dirty() { return }
-
-            node.set_flag(IS_DIRTY | HAS_DIRTY_DESCENDANTS, true);
-
-            for kid in node.children() {
-                dirty_subtree(kid.r());
-            }
-        }
-
-        dirty_subtree(self);
+        self.set_flag(IS_DIRTY, true);
 
         // 4. Dirty ancestors.
         for ancestor in self.ancestors() {
@@ -1299,6 +1287,31 @@ impl TreeIterator {
             depth: 0,
         }
     }
+
+    pub fn next_skipping_children(&mut self) -> Option<Root<Node>> {
+        let current = match self.current.take() {
+            None => return None,
+            Some(current) => current,
+        };
+
+        self.next_skipping_children_impl(current)
+    }
+
+    fn next_skipping_children_impl(&mut self, current: Root<Node>) -> Option<Root<Node>> {
+        for ancestor in current.inclusive_ancestors() {
+            if self.depth == 0 {
+                break;
+            }
+            if let Some(next_sibling) = ancestor.GetNextSibling() {
+                self.current = Some(next_sibling);
+                return Some(current);
+            }
+            self.depth -= 1;
+        }
+        debug_assert!(self.depth == 0);
+        self.current = None;
+        Some(current)
+    }
 }
 
 impl Iterator for TreeIterator {
@@ -1315,19 +1328,8 @@ impl Iterator for TreeIterator {
             self.depth += 1;
             return Some(current);
         };
-        for ancestor in current.inclusive_ancestors() {
-            if self.depth == 0 {
-                break;
-            }
-            if let Some(next_sibling) = ancestor.GetNextSibling() {
-                self.current = Some(next_sibling);
-                return Some(current);
-            }
-            self.depth -= 1;
-        }
-        debug_assert!(self.depth == 0);
-        self.current = None;
-        Some(current)
+
+        self.next_skipping_children_impl(current)
     }
 }
 

components/script/dom/window.rs

@@ -450,13 +450,15 @@ impl WindowMethods for Window {
         // Right now, just print to the console
         // Ensure that stderr doesn't trample through the alert() we use to
         // communicate test results (see executorservo.py in wptrunner).
-        let stderr = stderr();
-        let mut stderr = stderr.lock();
-        let stdout = stdout();
-        let mut stdout = stdout.lock();
-        writeln!(&mut stdout, "ALERT: {}", s).unwrap();
-        stdout.flush().unwrap();
-        stderr.flush().unwrap();
+        {
+            let stderr = stderr();
+            let mut stderr = stderr.lock();
+            let stdout = stdout();
+            let mut stdout = stdout.lock();
+            writeln!(&mut stdout, "ALERT: {}", s).unwrap();
+            stdout.flush().unwrap();
+            stderr.flush().unwrap();
+        }
 
         let (sender, receiver) = ipc::channel().unwrap();
         self.constellation_chan().send(ConstellationMsg::Alert(self.pipeline(), s.to_string(), sender)).unwrap();

components/script/layout_wrapper.rs

@@ -199,15 +199,6 @@ impl<'ln> TNode for ServoLayoutNode<'ln> {
         self.node.set_flag(DIRTY_ON_VIEWPORT_SIZE_CHANGE, true);
     }
 
-    fn set_descendants_dirty_on_viewport_size_changed(&self) {
-        for ref child in self.children() {
-            unsafe {
-                child.set_dirty_on_viewport_size_changed();
-            }
-            child.set_descendants_dirty_on_viewport_size_changed();
-        }
-    }
-
     fn can_be_fragmented(&self) -> bool {
         unsafe { self.node.get_flag(CAN_BE_FRAGMENTED) }
     }

components/script/script_thread.rs

@@ -1198,8 +1198,6 @@ impl ScriptThread {
         // https://html.spec.whatwg.org/multipage/#the-end step 7
         let handler = box DocumentProgressHandler::new(Trusted::new(doc));
         self.dom_manipulation_task_source.queue(handler, GlobalRef::Window(doc.window())).unwrap();
-
-        self.constellation_chan.send(ConstellationMsg::LoadComplete(pipeline)).unwrap();
     }
 
     fn collect_reports(&self, reports_chan: ReportsChan) {

components/script_traits/script_msg.rs

@@ -71,10 +71,6 @@ pub enum ScriptMsg {
     CreateWebGLPaintThread(Size2D<i32>,
                            GLContextAttributes,
                            IpcSender<Result<(IpcSender<CanvasMsg>, GLLimits), String>>),
-    /// Dispatched after the DOM load event has fired on a document
-    /// Causes a `load` event to be dispatched to any enclosing frame context element
-    /// for the given pipeline.
-    DOMLoad(PipelineId),
     /// Notifies the constellation that this frame has received focus.
     Focus(PipelineId),
     /// Re-send a mouse button event that was sent to the parent window.
@@ -85,7 +81,8 @@ pub enum ScriptMsg {
     GetClipboardContents(IpcSender<String>),
     /// <head> tag finished parsing
     HeadParsed,
-    /// All pending loads are complete.
+    /// All pending loads are complete, and the `load` event for this pipeline
+    /// has been dispatched.
     LoadComplete(PipelineId),
     /// A new load has been requested.
     LoadUrl(PipelineId, LoadData),

components/style/data.rs

@@ -37,13 +37,13 @@ impl PrivateStyleData {
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub struct DomParallelInfo {
     /// The number of children that still need work done.
-    pub children_count: AtomicIsize,
+    pub children_to_process: AtomicIsize,
 }
 
 impl DomParallelInfo {
     pub fn new() -> DomParallelInfo {
         DomParallelInfo {
-            children_count: AtomicIsize::new(0),
+            children_to_process: AtomicIsize::new(0),
         }
     }
 }

components/style/dom.rs

@@ -111,33 +111,10 @@ pub trait TNode : Sized + Copy + Clone {
 
     unsafe fn set_dirty_descendants(&self, value: bool);
 
-    fn dirty_self(&self) {
-        unsafe {
-            self.set_dirty(true);
-            self.set_dirty_descendants(true);
-        }
-    }
-
-    fn dirty_descendants(&self) {
-        for ref child in self.children() {
-            child.dirty_self();
-            child.dirty_descendants();
-        }
-    }
-
     fn needs_dirty_on_viewport_size_changed(&self) -> bool;
 
     unsafe fn set_dirty_on_viewport_size_changed(&self);
 
-    fn set_descendants_dirty_on_viewport_size_changed(&self) {
-        for ref child in self.children() {
-            unsafe {
-                child.set_dirty_on_viewport_size_changed();
-            }
-            child.set_descendants_dirty_on_viewport_size_changed();
-        }
-    }
-
     fn can_be_fragmented(&self) -> bool;
 
     unsafe fn set_can_be_fragmented(&self, value: bool);
@@ -215,7 +192,7 @@ pub trait TElement : Sized + Copy + Clone + ElementExt + PresentationalHintsSynt
     fn attr_equals(&self, namespace: &Namespace, attr: &Atom, value: &Atom) -> bool;
 
     /// Properly marks nodes as dirty in response to restyle hints.
-    fn note_restyle_hint(&self, mut hint: RestyleHint) {
+    fn note_restyle_hint(&self, hint: RestyleHint) {
         // Bail early if there's no restyling to do.
         if hint.is_empty() {
             return;
@@ -233,23 +210,21 @@ pub trait TElement : Sized + Copy + Clone + ElementExt + PresentationalHintsSynt
 
         // Process hints.
         if hint.contains(RESTYLE_SELF) {
-            node.dirty_self();
+            unsafe { node.set_dirty(true); }
+        // XXX(emilio): For now, dirty implies dirty descendants if found.
+        } else if hint.contains(RESTYLE_DESCENDANTS) {
+            let mut current = node.first_child();
+            while let Some(node) = current {
+                unsafe { node.set_dirty(true); }
+                current = node.next_sibling();
+            }
+        }
 
-            // FIXME(bholley, #8438): We currently need to RESTYLE_DESCENDANTS in the
-            // RESTYLE_SELF case in order to make sure "inherit" style structs propagate
-            // properly. See the explanation in the github issue.
-            hint.insert(RESTYLE_DESCENDANTS);
-        }
-        if hint.contains(RESTYLE_DESCENDANTS) {
-            unsafe { node.set_dirty_descendants(true); }
-            node.dirty_descendants();
-        }
         if hint.contains(RESTYLE_LATER_SIBLINGS) {
             let mut next = ::selectors::Element::next_sibling_element(self);
             while let Some(sib) = next {
                 let sib_node = sib.as_node();
-                sib_node.dirty_self();
-                sib_node.dirty_descendants();
+                unsafe { sib_node.set_dirty(true) };
                 next = ::selectors::Element::next_sibling_element(&sib);
             }
         }
@@ -262,12 +237,16 @@ pub struct TreeIterator<ConcreteNode> where ConcreteNode: TNode {
 
 impl<ConcreteNode> TreeIterator<ConcreteNode> where ConcreteNode: TNode {
     fn new(root: ConcreteNode) -> TreeIterator<ConcreteNode> {
-        let mut stack = vec!();
+        let mut stack = vec![];
         stack.push(root);
         TreeIterator {
             stack: stack,
         }
     }
+
+    pub fn next_skipping_children(&mut self) -> Option<ConcreteNode> {
+        self.stack.pop()
+    }
 }
 
 impl<ConcreteNode> Iterator for TreeIterator<ConcreteNode>

components/style/parallel.rs

@@ -63,25 +63,40 @@ fn top_down_dom<N, C>(unsafe_nodes: UnsafeNodeList,
         // Get a real layout node.
         let node = unsafe { N::from_unsafe(&unsafe_node) };
 
+        if !context.should_process(node) {
+            continue;
+        }
+
         // Perform the appropriate traversal.
         context.process_preorder(node);
 
-        let child_count = node.children_count();
+        // Possibly enqueue the children.
+        let mut children_to_process = 0isize;
+        for kid in node.children() {
+            // Trigger the hook pre-adding the kid to the list. This can (and in
+            // fact uses to) change the result of the should_process operation.
+            //
+            // As of right now, this hook takes care of propagating the restyle
+            // flag down the tree. In the future, more accurate behavior is
+            // probably going to be needed.
+            context.pre_process_child_hook(node, kid);
+            if context.should_process(kid) {
+                children_to_process += 1;
+                discovered_child_nodes.push(kid.to_unsafe())
+            }
+        }
 
         // Reset the count of children.
         {
             let data = node.mutate_data().unwrap();
-            data.parallel.children_count.store(child_count as isize,
-                                               Ordering::Relaxed);
+            data.parallel.children_to_process
+                         .store(children_to_process,
+                                Ordering::Relaxed);
         }
 
-        // Possibly enqueue the children.
-        if child_count != 0 {
-            for kid in node.children() {
-                discovered_child_nodes.push(kid.to_unsafe())
-            }
-        } else {
-            // If there were no more children, start walking back up.
+
+        // If there were no more children, start walking back up.
+        if children_to_process == 0 {
             bottom_up_dom::<N, C>(unsafe_nodes.1, unsafe_node, proxy)
         }
     }
@@ -128,7 +143,7 @@ fn bottom_up_dom<N, C>(root: OpaqueNode,
 
         if parent_data
             .parallel
-            .children_count
+            .children_to_process
             .fetch_sub(1, Ordering::Relaxed) != 1 {
             // Get out of here and find another node to work on.
             break
@@ -138,4 +153,3 @@ fn bottom_up_dom<N, C>(root: OpaqueNode,
         node = parent;
     }
 }
-

components/style/sequential.rs

@@ -9,20 +9,29 @@ use traversal::DomTraversalContext;
 
 pub fn traverse_dom<N, C>(root: N,
                           shared: &C::SharedContext)
-                          where N: TNode,
-                                C: DomTraversalContext<N> {
+    where N: TNode,
+          C: DomTraversalContext<N>
+{
     fn doit<'a, N, C>(context: &'a C, node: N)
-                      where N: TNode, C: DomTraversalContext<N> {
+        where N: TNode,
+              C: DomTraversalContext<N>
+    {
+        debug_assert!(context.should_process(node));
         context.process_preorder(node);
 
         for kid in node.children() {
-            doit::<N, C>(context, kid);
+            context.pre_process_child_hook(node, kid);
+            if context.should_process(node) {
+                doit::<N, C>(context, kid);
+            }
         }
 
         context.process_postorder(node);
     }
 
     let context = C::new(shared, root.opaque());
-    doit::<N, C>(&context, root);
+    if context.should_process(root) {
+        doit::<N, C>(&context, root);
+    }
 }
 

components/style/traversal.rs

@@ -147,6 +147,30 @@ pub trait DomTraversalContext<N: TNode>  {
     fn process_preorder(&self, node: N);
     /// Process `node` on the way up, after its children have been processed.
     fn process_postorder(&self, node: N);
+
+    /// Returns if the node should be processed by the preorder traversal (and
+    /// then by the post-order one).
+    ///
+    /// Note that this is true unconditionally for servo, since it requires to
+    /// bubble the widths bottom-up for all the DOM.
+    fn should_process(&self, node: N) -> bool {
+        node.is_dirty() || node.has_dirty_descendants()
+    }
+
+    /// Do an action over the child before pushing him to the work queue.
+    ///
+    /// By default, propagate the IS_DIRTY flag down the tree.
+    #[allow(unsafe_code)]
+    fn pre_process_child_hook(&self, parent: N, kid: N) {
+        // NOTE: At this point is completely safe to modify either the parent or
+        // the child, since we have exclusive access to both of them.
+        if parent.is_dirty() {
+            unsafe {
+                kid.set_dirty(true);
+                parent.set_dirty_descendants(true);
+            }
+        }
+    }
 }
 
 /// Calculates the style for a single node.
@@ -244,22 +268,17 @@ pub fn recalc_style_at<'a, N, C>(context: &'a C,
     // NB: flow construction updates the bloom filter on the way up.
     put_thread_local_bloom_filter(bf, &unsafe_layout_node, context.shared_context());
 
-    // Mark the node as DIRTY_ON_VIEWPORT_SIZE_CHANGE is it uses viewport percentage units.
-    match node.as_element() {
-        Some(element) => {
-            match *element.style_attribute() {
-                Some(ref property_declaration_block) => {
-                    if property_declaration_block.declarations().any(|d| d.0.has_viewport_percentage()) {
-                        unsafe {
-                            node.set_dirty_on_viewport_size_changed();
-                        }
-                        node.set_descendants_dirty_on_viewport_size_changed();
+    // Mark the node as DIRTY_ON_VIEWPORT_SIZE_CHANGE is it uses viewport
+    // percentage units.
+    if !node.needs_dirty_on_viewport_size_changed() {
+        if let Some(element) = node.as_element() {
+            if let Some(ref property_declaration_block) = *element.style_attribute() {
+                if property_declaration_block.declarations().any(|d| d.0.has_viewport_percentage()) {
+                    unsafe {
+                        node.set_dirty_on_viewport_size_changed();
                     }
-                },
-                None => {}
+                }
             }
-        },
-        None => {}
+        }
     }
 }
-

ports/geckolib/glue.rs

@@ -113,13 +113,13 @@ fn restyle_subtree(node: GeckoNode, raw_data: *mut RawServoStyleSet) {
         timer: Timer::new(),
     };
 
-    if node.is_dirty() || node.has_dirty_descendants() {
-        if per_doc_data.num_threads == 1 {
-            sequential::traverse_dom::<GeckoNode, RecalcStyleOnly>(node, &shared_style_context);
-        } else {
-            parallel::traverse_dom::<GeckoNode, RecalcStyleOnly>(node, &shared_style_context,
-                                                                 &mut per_doc_data.work_queue);
-        }
+    // We ensure this is true before calling Servo_RestyleSubtree()
+    debug_assert!(node.is_dirty() || node.has_dirty_descendants());
+    if per_doc_data.num_threads == 1 {
+        sequential::traverse_dom::<GeckoNode, RecalcStyleOnly>(node, &shared_style_context);
+    } else {
+        parallel::traverse_dom::<GeckoNode, RecalcStyleOnly>(node, &shared_style_context,
+                                                             &mut per_doc_data.work_queue);
     }
 }