servo: Merge #8098 - Track event state changes on Document and do the dirtying from layout (from bholley:dirty_from_layout); r=jdm

This is a first step in fixing #6942.

Source-Repo: https://github.com/servo/servo
Source-Revision: 285e29c06637f31a8b8a27c2e454468717924ebd

servo/components/layout/layout_task.rs

@@ -52,6 +52,7 @@ use query::{LayoutRPCImpl, process_content_box_request, process_content_boxes_re
 use query::{MarginPadding, MarginRetrievingFragmentBorderBoxIterator, PositionProperty};
 use query::{PositionRetrievingFragmentBorderBoxIterator, Side};
 use script::dom::bindings::js::LayoutJS;
+use script::dom::document::Document;
 use script::dom::node::{LayoutData, Node};
 use script::layout_interface::Animation;
 use script::layout_interface::{LayoutChan, LayoutRPC, OffsetParentResponse};
@@ -67,7 +68,7 @@ use std::cell::Cell;
 use std::collections::HashMap;
 use std::collections::hash_state::DefaultState;
 use std::mem::transmute;
-use std::ops::{Deref, DerefMut};
+use std::ops::{Deref, DerefMut, Drop};
 use std::sync::atomic::{AtomicUsize, Ordering};
 use std::sync::mpsc::{channel, Sender, Receiver};
 use std::sync::{Arc, Mutex, MutexGuard};
@@ -89,8 +90,7 @@ use util::opts;
 use util::task::spawn_named_with_send_on_failure;
 use util::task_state;
 use util::workqueue::WorkQueue;
-use wrapper::LayoutNode;
+use wrapper::{LayoutDocument, LayoutNode, ThreadSafeLayoutNode};
-use wrapper::ThreadSafeLayoutNode;
 
 /// The number of screens of data we're allowed to generate display lists for in each direction.
 pub const DISPLAY_PORT_SIZE_FACTOR: i32 = 8;
@@ -1125,14 +1125,28 @@ impl LayoutTask {
     fn handle_reflow<'a>(&'a self,
                          data: &ScriptReflow,
                          possibly_locked_rw_data: &mut Option<MutexGuard<'a, LayoutTaskData>>) {
-        // FIXME: Isolate this transmutation into a "bridge" module.
+
-        // FIXME(rust#16366): The following line had to be moved because of a
+        // Make sure that every return path from this method joins the script task,
-        // rustc bug. It should be in the next unsafe block.
+        // otherwise the script task will panic.
-        let mut node: LayoutJS<Node> = unsafe {
+        struct AutoJoinScriptTask<'a> { data: &'a ScriptReflow };
-            LayoutJS::from_trusted_node_address(data.document_root)
+        impl<'a> Drop for AutoJoinScriptTask<'a> {
+            fn drop(&mut self) {
+                self.data.script_join_chan.send(()).unwrap();
+            }
         };
-        let node: &mut LayoutNode = unsafe {
+        let _ajst = AutoJoinScriptTask { data: data };
-            transmute(&mut node)
+
+        // FIXME: Isolate this transmutation into a "bridge" module.
+        let mut doc: LayoutJS<Document> = unsafe {
+            LayoutJS::from_trusted_node_address(data.document).downcast::<Document>().unwrap()
+        };
+        let doc: &mut LayoutDocument = unsafe {
+            transmute(&mut doc)
+        };
+
+        let mut node: LayoutNode = match doc.root_node() {
+            None => return,
+            Some(x) => x,
         };
 
         debug!("layout: received layout request for: {}", self.url.serialize());
@@ -1176,15 +1190,23 @@ impl LayoutTask {
         let needs_reflow = screen_size_changed && !needs_dirtying;
         unsafe {
             if needs_dirtying {
-                LayoutTask::dirty_all_nodes(node);
+                LayoutTask::dirty_all_nodes(&mut node);
             }
         }
         if needs_reflow {
-            if let Some(mut flow) = self.try_get_layout_root(*node) {
+            if let Some(mut flow) = self.try_get_layout_root(node) {
                 LayoutTask::reflow_all_nodes(flow_ref::deref_mut(&mut flow));
             }
         }
 
+        let event_state_changes = doc.drain_event_state_changes();
+        if !needs_dirtying {
+            for &(el, state) in event_state_changes.iter() {
+                assert!(!state.is_empty());
+                el.note_event_state_change();
+            }
+        }
+
         // Create a layout context for use throughout the following passes.
         let mut shared_layout_context = self.build_shared_layout_context(&*rw_data,
                                                                          screen_size_changed,
@@ -1202,16 +1224,16 @@ impl LayoutTask {
                 let rw_data = &mut *rw_data;
                 match rw_data.parallel_traversal {
                     None => {
-                        sequential::traverse_dom_preorder(*node, &shared_layout_context);
+                        sequential::traverse_dom_preorder(node, &shared_layout_context);
                     }
                     Some(ref mut traversal) => {
-                        parallel::traverse_dom_preorder(*node, &shared_layout_context, traversal);
+                        parallel::traverse_dom_preorder(node, &shared_layout_context, traversal);
                     }
                 }
             });
 
             // Retrieve the (possibly rebuilt) root flow.
-            rw_data.root_flow = self.try_get_layout_root((*node).clone());
+            rw_data.root_flow = self.try_get_layout_root(node.clone());
         }
 
         // Send new canvas renderers to the paint task
@@ -1245,9 +1267,6 @@ impl LayoutTask {
                 ReflowQueryType::NoQuery => {}
             }
         }
-
-        // Tell script that we're done.
-        data.script_join_chan.send(()).unwrap();
     }
 
     fn set_visible_rects<'a>(&'a self,

servo/components/layout/wrapper.rs

@@ -43,8 +43,9 @@ use script::dom::bindings::codegen::InheritTypes::{HTMLElementTypeId, NodeTypeId
 use script::dom::bindings::conversions::Castable;
 use script::dom::bindings::js::LayoutJS;
 use script::dom::characterdata::LayoutCharacterDataHelpers;
+use script::dom::document::{Document, LayoutDocumentHelpers};
 use script::dom::element;
-use script::dom::element::{Element, LayoutElementHelpers, RawLayoutElementHelpers};
+use script::dom::element::{Element, EventState, LayoutElementHelpers, RawLayoutElementHelpers};
 use script::dom::htmlcanvaselement::{LayoutHTMLCanvasElementHelpers, HTMLCanvasData};
 use script::dom::htmliframeelement::HTMLIFrameElement;
 use script::dom::htmlimageelement::LayoutHTMLImageElementHelpers;
@@ -58,6 +59,7 @@ use selectors::parser::{AttrSelector, NamespaceConstraint};
 use smallvec::VecLike;
 use std::borrow::ToOwned;
 use std::cell::{Ref, RefMut};
+use std::iter::FromIterator;
 use std::marker::PhantomData;
 use std::mem;
 use std::sync::Arc;
@@ -105,6 +107,12 @@ impl<'ln> LayoutNode<'ln> {
         }
     }
 
+    pub fn is_element(&self) -> bool {
+        unsafe {
+            self.node.is_element_for_layout()
+        }
+    }
+
     pub fn dump(self) {
         self.dump_indent(0);
     }
@@ -336,6 +344,41 @@ impl<'a> Iterator for LayoutTreeIterator<'a> {
     }
 }
 
+// A wrapper around documents that ensures ayout can only ever access safe properties.
+#[derive(Copy, Clone)]
+pub struct LayoutDocument<'le> {
+    document: LayoutJS<Document>,
+    chain: PhantomData<&'le ()>,
+}
+
+impl<'le> LayoutDocument<'le> {
+    pub fn as_node(&self) -> LayoutNode<'le> {
+        LayoutNode {
+            node: self.document.upcast(),
+            chain: PhantomData,
+        }
+    }
+
+    pub fn root_node(&self) -> Option<LayoutNode<'le>> {
+        let mut node = self.as_node().first_child();
+        while node.is_some() && !node.unwrap().is_element() {
+            node = node.unwrap().next_sibling();
+        }
+        node
+    }
+
+    pub fn drain_event_state_changes(&self) -> Vec<(LayoutElement, EventState)> {
+        unsafe {
+            let changes = self.document.drain_event_state_changes();
+            Vec::from_iter(changes.iter().map(|&(el, state)|
+                (LayoutElement {
+                    element: el,
+                    chain: PhantomData,
+                }, state)))
+        }
+    }
+}
+
 /// A wrapper around elements that ensures layout can only ever access safe properties.
 #[derive(Copy, Clone)]
 pub struct LayoutElement<'le> {
@@ -356,6 +399,41 @@ impl<'le> LayoutElement<'le> {
             chain: PhantomData,
         }
     }
+
+    /// Properly marks nodes as dirty in response to event state changes.
+    ///
+    /// Currently this implementation is very conservative, and basically mirrors node::dirty_impl.
+    /// With restyle hints, we can do less work here.
+    pub fn note_event_state_change(&self) {
+        let node = self.as_node();
+
+        // Bail out if we're already dirty. This won't be valid when we start doing more targeted
+        // dirtying with restyle hints.
+        if node.is_dirty() { return }
+
+        // Dirty descendants.
+        fn dirty_subtree(node: LayoutNode) {
+            // Stop if this subtree is already dirty. This won't be valid with restyle hints, see above.
+            if node.is_dirty() { return }
+
+            unsafe {
+                node.set_dirty(true);
+                node.set_dirty_descendants(true);
+            }
+
+            for kid in node.children() {
+                dirty_subtree(kid);
+            }
+        }
+        dirty_subtree(node);
+
+        let mut curr = node;
+        while let Some(parent) = curr.parent_node() {
+            if parent.has_dirty_descendants() { break }
+            unsafe { parent.set_dirty_descendants(true); }
+            curr = parent;
+        }
+    }
 }
 
 fn as_element<'le>(node: LayoutJS<Node>) -> Option<LayoutElement<'le>> {

servo/components/script/dom/bindings/cell.rs

@@ -94,6 +94,13 @@ impl<T> DOMRefCell<T> {
             _ => None,
         }
     }
+
+    /// Version of the above that we use during restyle while the script task
+    /// is blocked.
+    pub fn borrow_mut_for_layout(&self) -> RefMut<T> {
+        debug_assert!(task_state::get().is_layout());
+        self.value.borrow_mut()
+    }
 }
 
 impl<T: JSTraceable> JSTraceable for DOMRefCell<T> {

servo/components/script/dom/bindings/js.rs

@@ -35,6 +35,7 @@ use layout_interface::TrustedNodeAddress;
 use script_task::STACK_ROOTS;
 use std::cell::UnsafeCell;
 use std::default::Default;
+use std::hash::{Hash, Hasher};
 use std::mem;
 use std::ops::Deref;
 use std::ptr;
@@ -147,12 +148,24 @@ impl<T> PartialEq for JS<T> {
     }
 }
 
+impl<T> Eq for JS<T> {}
+
 impl<T> PartialEq for LayoutJS<T> {
     fn eq(&self, other: &LayoutJS<T>) -> bool {
         self.ptr == other.ptr
     }
 }
 
+impl<T> Eq for LayoutJS<T> {}
+
+impl<T> Hash for JS<T> {
+    fn hash<H: Hasher>(&self, state: &mut H) { self.ptr.hash(state) }
+}
+
+impl<T> Hash for LayoutJS<T> {
+    fn hash<H: Hasher>(&self, state: &mut H) { self.ptr.hash(state) }
+}
+
 impl <T> Clone for JS<T> {
     #[inline]
     #[allow(unrooted_must_root)]

servo/components/script/dom/document.rs

@@ -32,7 +32,7 @@ use dom::customevent::CustomEvent;
 use dom::documentfragment::DocumentFragment;
 use dom::documenttype::DocumentType;
 use dom::domimplementation::DOMImplementation;
-use dom::element::{Element, ElementCreator};
+use dom::element::{Element, ElementCreator, EventState};
 use dom::event::{Event, EventBubbles, EventCancelable};
 use dom::eventtarget::{EventTarget};
 use dom::htmlanchorelement::HTMLAnchorElement;
@@ -174,6 +174,8 @@ pub struct Document {
     /// This field is set to the document itself for inert documents.
     /// https://html.spec.whatwg.org/multipage/#appropriate-template-contents-owner-document
     appropriate_template_contents_owner_document: MutNullableHeap<JS<Document>>,
+    // The collection of EventStates that have been changed since the last restyle.
+    event_state_changes: DOMRefCell<HashMap<JS<Element>, EventState>>,
 }
 
 impl PartialEq for Document {
@@ -301,6 +303,11 @@ impl Document {
         }
     }
 
+    pub fn needs_reflow(&self) -> bool {
+        self.GetDocumentElement().is_some() &&
+        (self.upcast::<Node>().get_has_dirty_descendants() || !self.event_state_changes.borrow().is_empty())
+    }
+
     /// Returns the first `base` element in the DOM that has an `href` attribute.
     pub fn base_element(&self) -> Option<Root<HTMLBaseElement>> {
         self.base_element.get_rooted()
@@ -1175,18 +1182,27 @@ pub enum DocumentSource {
     NotFromParser,
 }
 
+#[allow(unsafe_code)]
 pub trait LayoutDocumentHelpers {
-    #[allow(unsafe_code)]
     unsafe fn is_html_document_for_layout(&self) -> bool;
+    unsafe fn drain_event_state_changes(&self) -> Vec<(LayoutJS<Element>, EventState)>;
 }
 
+#[allow(unsafe_code)]
 impl LayoutDocumentHelpers for LayoutJS<Document> {
-    #[allow(unrooted_must_root)]
     #[inline]
-    #[allow(unsafe_code)]
     unsafe fn is_html_document_for_layout(&self) -> bool {
         (*self.unsafe_get()).is_html_document
     }
+
+    #[inline]
+    #[allow(unrooted_must_root)]
+    unsafe fn drain_event_state_changes(&self) -> Vec<(LayoutJS<Element>, EventState)> {
+        let mut changes = (*self.unsafe_get()).event_state_changes.borrow_mut_for_layout();
+        let drain = changes.drain();
+        let layout_drain = drain.map(|(k, v)| (k.to_layout(), v));
+        Vec::from_iter(layout_drain)
+    }
 }
 
 impl Document {
@@ -1252,6 +1268,7 @@ impl Document {
             reflow_timeout: Cell::new(None),
             base_element: Default::default(),
             appropriate_template_contents_owner_document: Default::default(),
+            event_state_changes: DOMRefCell::new(HashMap::new()),
         }
     }
 
@@ -1316,6 +1333,20 @@ impl Document {
     pub fn get_element_by_id(&self, id: &Atom) -> Option<Root<Element>> {
         self.idmap.borrow().get(&id).map(|ref elements| (*elements)[0].root())
     }
+
+    pub fn record_event_state_change(&self, el: &Element, which: EventState) {
+        let mut map = self.event_state_changes.borrow_mut();
+        let empty;
+        {
+            let states = map.entry(JS::from_ref(el))
+                            .or_insert(EventState::empty());
+            states.toggle(which);
+            empty = states.is_empty();
+        }
+        if empty {
+            map.remove(&JS::from_ref(el));
+        }
+    }
 }
 
 

servo/components/script/dom/element.rs

@@ -1836,6 +1836,9 @@ impl Element {
 
     fn set_state(&self, which: EventState, value: bool) {
         let mut state = self.event_state.get();
+        if state.contains(which) == value {
+            return
+        }
         match value {
             true => state.insert(which),
             false => state.remove(which),
@@ -1843,7 +1846,7 @@ impl Element {
         self.event_state.set(state);
 
         let node = self.upcast::<Node>();
-        node.dirty(NodeDamage::NodeStyleDamaged);
+        node.owner_doc().record_event_state_change(self, which);
     }
 
     pub fn get_active_state(&self) -> bool {

servo/components/script/dom/window.rs

@@ -885,14 +885,6 @@ impl Window {
     ///
     /// TODO(pcwalton): Only wait for style recalc, since we have off-main-thread layout.
     pub fn force_reflow(&self, goal: ReflowGoal, query_type: ReflowQueryType, reason: ReflowReason) {
-        let document = self.Document();
-        let root = document.r().GetDocumentElement();
-        let root = match root.r() {
-            Some(root) => root,
-            None => return,
-        };
-        let root = root.upcast::<Node>();
-
         let window_size = match self.window_size.get() {
             Some(window_size) => window_size,
             None => return,
@@ -923,7 +915,7 @@ impl Window {
                 goal: goal,
                 page_clip_rect: self.page_clip_rect.get(),
             },
-            document_root: root.to_trusted_node_address(),
+            document: self.Document().r().upcast::<Node>().to_trusted_node_address(),
             window_size: window_size,
             script_chan: self.control_chan.clone(),
             script_join_chan: join_chan,
@@ -965,16 +957,8 @@ impl Window {
     ///
     /// TODO(pcwalton): Only wait for style recalc, since we have off-main-thread layout.
     pub fn reflow(&self, goal: ReflowGoal, query_type: ReflowQueryType, reason: ReflowReason) {
-        let document = self.Document();
+        if query_type == ReflowQueryType::NoQuery && !self.Document().needs_reflow() {
-        let root = document.r().GetDocumentElement();
+            debug!("Document doesn't need reflow - skipping it (reason {:?})", reason);
-        let root = match root.r() {
-            Some(root) => root,
-            None => return,
-        };
-
-        let root = root.upcast::<Node>();
-        if query_type == ReflowQueryType::NoQuery && !root.get_has_dirty_descendants() {
-            debug!("root has no dirty descendants; avoiding reflow (reason {:?})", reason);
             return
         }
 

servo/components/script/layout_interface.rs

@@ -174,7 +174,7 @@ pub struct ScriptReflow {
     /// General reflow data.
     pub reflow_info: Reflow,
     /// The document node.
-    pub document_root: TrustedNodeAddress,
+    pub document: TrustedNodeAddress,
     /// The channel through which messages can be sent back to the script task.
     pub script_chan: Sender<ConstellationControlMsg>,
     /// The current window size.