diff --git a/servo/components/layout/layout_task.rs b/servo/components/layout/layout_task.rs index 995742cd745c..deda7d481c7b 100644 --- a/servo/components/layout/layout_task.rs +++ b/servo/components/layout/layout_task.rs @@ -590,6 +590,8 @@ impl LayoutTask { 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); @@ -600,6 +602,8 @@ impl LayoutTask { 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); diff --git a/servo/components/net/image/holder.rs b/servo/components/net/image/holder.rs index 989021c3789e..4703f2cecd94 100644 --- a/servo/components/net/image/holder.rs +++ b/servo/components/net/image/holder.rs @@ -7,7 +7,6 @@ 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; @@ -87,24 +86,13 @@ impl ImageHolder { 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 { diff --git a/servo/components/script/dom/document.rs b/servo/components/script/dom/document.rs index 738ac6be90ef..c4dc787ac26c 100644 --- a/servo/components/script/dom/document.rs +++ b/servo/components/script/dom/document.rs @@ -49,7 +49,7 @@ use dom::mouseevent::MouseEvent; use dom::keyboardevent::KeyboardEvent; use dom::messageevent::MessageEvent; use dom::node::{Node, ElementNodeTypeId, DocumentNodeTypeId, NodeHelpers}; -use dom::node::{CloneChildren, DoNotCloneChildren}; +use dom::node::{CloneChildren, DoNotCloneChildren, NodeDamage, OtherNodeDamage}; use dom::nodelist::NodeList; use dom::text::Text; use dom::processinginstruction::ProcessingInstruction; @@ -176,10 +176,8 @@ pub trait DocumentHelpers<'a> { fn set_quirks_mode(self, mode: QuirksMode); fn set_last_modified(self, value: DOMString); fn set_encoding_name(self, name: DOMString); - fn content_changed(self, node: JSRef); - fn content_and_heritage_changed(self, node: JSRef); - fn reflow(self); - fn wait_until_safe_to_modify_dom(self); + fn content_changed(self, node: JSRef, damage: NodeDamage); + fn content_and_heritage_changed(self, node: JSRef, damage: NodeDamage); fn unregister_named_element(self, to_unregister: JSRef, id: Atom); fn register_named_element(self, element: JSRef, id: Atom); fn load_anchor_href(self, href: DOMString); @@ -190,6 +188,7 @@ pub trait DocumentHelpers<'a> { fn request_focus(self, elem: JSRef); fn commit_focus_transaction(self); fn send_title_to_compositor(self); + fn dirty_all_nodes(self); } impl<'a> DocumentHelpers<'a> for JSRef<'a, Document> { @@ -228,24 +227,14 @@ impl<'a> DocumentHelpers<'a> for JSRef<'a, Document> { *self.encoding_name.borrow_mut() = name; } - fn content_changed(self, node: JSRef) { - debug!("content_changed on {}", node.debug_str()); - node.dirty(); - self.reflow(); + fn content_changed(self, node: JSRef, damage: NodeDamage) { + node.dirty(damage); } - fn content_and_heritage_changed(self, node: JSRef) { + fn content_and_heritage_changed(self, node: JSRef, 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. @@ -376,6 +365,13 @@ impl<'a> DocumentHelpers<'a> for JSRef<'a, Document> { let window = self.window().root(); window.page().send_title_to_compositor(); } + + fn dirty_all_nodes(self) { + let root: JSRef = NodeCast::from_ref(self); + for node in root.traverse_preorder() { + node.dirty(OtherNodeDamage) + } + } } #[deriving(PartialEq)] diff --git a/servo/components/script/dom/element.rs b/servo/components/script/dom/element.rs index e24407d7957a..1e55f9f73c22 100644 --- a/servo/components/script/dom/element.rs +++ b/servo/components/script/dom/element.rs @@ -33,8 +33,9 @@ use dom::htmlcollection::HTMLCollection; use dom::htmlinputelement::{HTMLInputElement, RawLayoutHTMLInputElementHelpers}; use dom::htmlserializer::serialize; use dom::htmltablecellelement::{HTMLTableCellElement, HTMLTableCellElementHelpers}; -use dom::node::{ElementNodeTypeId, Node, NodeHelpers, NodeIterator, document_from_node, CLICK_IN_PROGRESS}; -use dom::node::{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; @@ -441,7 +442,6 @@ pub trait AttributeHandlers { value: DOMString) -> AttrValue; fn remove_attribute(self, namespace: Namespace, name: &str); - fn notify_content_changed(self); fn has_class(&self, name: &Atom) -> bool; fn set_atomic_attribute(self, name: &Atom, value: DOMString); @@ -500,9 +500,6 @@ impl<'a> AttributeHandlers for JSRef<'a, Element> { assert!(name.as_slice() == name.as_slice().to_ascii_lower().as_slice()); assert!(!name.as_slice().contains(":")); - let node: JSRef = NodeCast::from_ref(self); - node.wait_until_safe_to_modify_dom(); - self.do_set_attribute(name.clone(), value, name.clone(), ns!(""), None, |attr| *attr.local_name() == *name); } @@ -548,28 +545,26 @@ impl<'a> AttributeHandlers for JSRef<'a, Element> { match idx { None => (), Some(idx) => { - let node: JSRef = 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 = 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 = 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| { @@ -755,11 +750,6 @@ impl<'a> ElementMethods for JSRef<'a, Element> { fn SetAttribute(self, name: DOMString, value: DOMString) -> ErrorResult { - { - let node: JSRef = NodeCast::from_ref(self); - node.wait_until_safe_to_modify_dom(); - } - // Step 1. match xml_name_type(name.as_slice()) { InvalidXMLName => return Err(InvalidCharacter), @@ -787,11 +777,6 @@ impl<'a> ElementMethods for JSRef<'a, Element> { namespace_url: Option, name: DOMString, value: DOMString) -> ErrorResult { - { - let node: JSRef = NodeCast::from_ref(self); - node.wait_until_safe_to_modify_dom(); - } - // Step 1. let namespace = namespace::from_domstring(namespace_url); @@ -999,25 +984,48 @@ impl<'a> VirtualMethods for JSRef<'a, Element> { match attr.local_name() { &atom!("style") => { + // Modifying the `style` attribute might change style. + let node: JSRef = 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; - } - &atom!("id") => { - let node: JSRef = NodeCast::from_ref(*self); - let value = attr.value(); - if node.is_in_doc() && !value.as_slice().is_empty() { - let doc = document_from_node(*self).root(); - let value = Atom::from_slice(value.as_slice()); - doc.register_named_element(*self, value); + + if node.is_in_doc() { + doc.content_changed(node, NodeStyleDamaged); + } + } + &atom!("class") => { + // Modifying a class can change style. + let node: JSRef = 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 = NodeCast::from_ref(*self); + let value = attr.value(); + if node.is_in_doc() { + let doc = document_from_node(*self).root(); + if !value.as_slice().is_empty() { + let value = Atom::from_slice(value.as_slice()); + doc.register_named_element(*self, value); + } + doc.content_changed(node, NodeStyleDamaged); + } + } + _ => { + // Modifying any other attribute might change arbitrary things. + let node: JSRef = 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) { @@ -1028,21 +1036,45 @@ impl<'a> VirtualMethods for JSRef<'a, Element> { match attr.local_name() { &atom!("style") => { + // Modifying the `style` attribute might change style. *self.style_attribute.borrow_mut() = None; - } - &atom!("id") => { + let node: JSRef = 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); + doc.content_changed(node, NodeStyleDamaged); + } + } + &atom!("id") => { + // Modifying an ID can change style. + let node: JSRef = NodeCast::from_ref(*self); + let value = attr.value(); + if node.is_in_doc() { + let doc = document_from_node(*self).root(); + if !value.as_slice().is_empty() { + let value = Atom::from_slice(value.as_slice()); + doc.unregister_named_element(*self, value); + } + doc.content_changed(node, NodeStyleDamaged); + } + } + &atom!("class") => { + // Modifying a class can change style. + let node: JSRef = 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 = 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 { diff --git a/servo/components/script/dom/htmlimageelement.rs b/servo/components/script/dom/htmlimageelement.rs index 12175c9de9a3..dc1ef7d6e263 100644 --- a/servo/components/script/dom/htmlimageelement.rs +++ b/servo/components/script/dom/htmlimageelement.rs @@ -15,7 +15,7 @@ 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; @@ -112,7 +112,14 @@ impl<'a> HTMLImageElementMethods for JSRef<'a, HTMLImageElement> { } 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 = NodeCast::from_ref(self); + node.dirty(OtherNodeDamage); + let rect = node.get_bounding_content_box(); to_px(rect.size.width) as u32 } @@ -123,7 +130,14 @@ impl<'a> HTMLImageElementMethods for JSRef<'a, HTMLImageElement> { } 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 = NodeCast::from_ref(self); + node.dirty(OtherNodeDamage); + let rect = node.get_bounding_content_box(); to_px(rect.size.height) as u32 } diff --git a/servo/components/script/dom/htmlinputelement.rs b/servo/components/script/dom/htmlinputelement.rs index 4e211bde251d..5b0133395a72 100644 --- a/servo/components/script/dom/htmlinputelement.rs +++ b/servo/components/script/dom/htmlinputelement.rs @@ -26,8 +26,10 @@ use dom::event::{Event, Bubbles, NotCancelable, EventHelpers}; use dom::eventtarget::{EventTarget, NodeTargetTypeId}; use dom::htmlelement::HTMLElement; use dom::keyboardevent::KeyboardEvent; -use dom::htmlformelement::{InputElement, FormControl, HTMLFormElement, HTMLFormElementHelpers, NotFromFormSubmitMethod}; -use dom::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}; @@ -312,7 +314,7 @@ impl<'a> HTMLInputElementHelpers for JSRef<'a, HTMLInputElement> { fn force_relayout(self) { let doc = document_from_node(self).root(); let node: JSRef = NodeCast::from_ref(self); - doc.content_changed(node) + doc.content_changed(node, OtherNodeDamage) } fn radio_group_updated(self, group: Option<&str>) { diff --git a/servo/components/script/dom/htmltextareaelement.rs b/servo/components/script/dom/htmltextareaelement.rs index c169a169cf98..05d3a5d08684 100644 --- a/servo/components/script/dom/htmltextareaelement.rs +++ b/servo/components/script/dom/htmltextareaelement.rs @@ -20,7 +20,8 @@ use dom::event::Event; use dom::eventtarget::{EventTarget, NodeTargetTypeId}; use dom::htmlelement::HTMLElement; use dom::keyboardevent::KeyboardEvent; -use dom::node::{DisabledStateHelpers, Node, NodeHelpers, ElementNodeTypeId, document_from_node}; +use dom::node::{DisabledStateHelpers, Node, NodeHelpers, OtherNodeDamage, ElementNodeTypeId}; +use dom::node::{document_from_node}; use textinput::{Multiple, TextInput, TriggerDefaultAction, DispatchInput, Nothing}; use dom::virtualmethods::VirtualMethods; @@ -163,7 +164,7 @@ impl<'a> PrivateHTMLTextAreaElementHelpers for JSRef<'a, HTMLTextAreaElement> { fn force_relayout(self) { let doc = document_from_node(self).root(); let node: JSRef = NodeCast::from_ref(self); - doc.content_changed(node) + doc.content_changed(node, OtherNodeDamage) } } diff --git a/servo/components/script/dom/node.rs b/servo/components/script/dom/node.rs index 161c8fc918b2..ecfccf8aefc4 100644 --- a/servo/components/script/dom/node.rs +++ b/servo/components/script/dom/node.rs @@ -287,20 +287,15 @@ impl<'a> PrivateNodeHelpers for JSRef<'a, Node> { let parent = self.parent_node().root(); parent.map(|parent| vtable_for(&*parent).child_inserted(self)); - - document.content_and_heritage_changed(self); + 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); } // @@ -311,9 +306,6 @@ impl<'a> PrivateNodeHelpers for JSRef<'a, Node> { /// /// Fails unless `new_child` is disconnected from the tree. fn add_child(self, new_child: JSRef, before: Option>) { - let doc = self.owner_doc().root(); - doc.wait_until_safe_to_modify_dom(); - assert!(new_child.parent_node().is_none()); assert!(new_child.prev_sibling().is_none()); assert!(new_child.next_sibling().is_none()); @@ -354,9 +346,6 @@ impl<'a> PrivateNodeHelpers for JSRef<'a, Node> { /// /// Fails unless `child` is a child of this node. fn remove_child(self, child: JSRef) { - let doc = self.owner_doc().root(); - doc.wait_until_safe_to_modify_dom(); - assert!(child.parent_node().root().root_ref() == Some(self)); match child.prev_sibling.get().root() { @@ -428,8 +417,6 @@ pub trait NodeHelpers<'a> { fn set_owner_doc(self, document: JSRef); fn is_in_html_doc(self) -> bool; - fn wait_until_safe_to_modify_dom(self); - fn is_element(self) -> bool; fn is_document(self) -> bool; fn is_doctype(self) -> bool; @@ -460,10 +447,11 @@ pub trait NodeHelpers<'a> { fn get_has_dirty_descendants(self) -> bool; fn set_has_dirty_descendants(self, state: bool); - /// Marks the given node as `IS_DIRTY`, its siblings as `IS_DIRTY` (to deal - /// 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 @@ -471,9 +459,9 @@ pub trait NodeHelpers<'a> { /// still need to be marked as `HAS_DIRTY_DESCENDANTS`. /// /// See #4170 - fn force_dirty_ancestors(self); + fn force_dirty_ancestors(self, damage: NodeDamage); - fn dirty_impl(self, force_ancestors: bool); + fn dirty_impl(self, damage: NodeDamage, force_ancestors: bool); fn dump(self); fn dump_indent(self, indent: uint); @@ -656,17 +644,20 @@ impl<'a> NodeHelpers<'a> for JSRef<'a, Node> { self.set_flag(HAS_DIRTY_DESCENDANTS, state) } - fn force_dirty_ancestors(self) { - 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 @@ -830,11 +821,6 @@ impl<'a> NodeHelpers<'a> for JSRef<'a, Node> { .peekable() } - fn wait_until_safe_to_modify_dom(self) { - let document = self.owner_doc().root(); - document.wait_until_safe_to_modify_dom(); - } - fn remove_self(self) { match self.parent_node().root() { Some(parent) => parent.remove_child(self), @@ -1826,14 +1812,12 @@ impl<'a> NodeMethods for JSRef<'a, Node> { CommentNodeTypeId | TextNodeTypeId | ProcessingInstructionNodeTypeId => { - self.wait_until_safe_to_modify_dom(); - let characterdata: JSRef = CharacterDataCast::to_ref(self).unwrap(); characterdata.set_data(value); // Notify the document that the content of this node is different let document = self.owner_doc().root(); - document.content_changed(self); + document.content_changed(self, OtherNodeDamage); } DoctypeNodeTypeId | DocumentNodeTypeId => {} @@ -2366,3 +2350,13 @@ impl<'a> DisabledStateHelpers for JSRef<'a, Node> { self.set_enabled_state(!has_disabled_attrib); } } + +/// A summary of the changes that happened to a node. +#[deriving(Clone, PartialEq)] +pub enum NodeDamage { + /// The node's `style` attribute changed. + NodeStyleDamaged, + /// Other parts of a node changed; attributes, text content, etc. + OtherNodeDamage, +} + diff --git a/servo/components/script/dom/window.rs b/servo/components/script/dom/window.rs index ec12d9839188..f305bf1344cf 100644 --- a/servo/components/script/dom/window.rs +++ b/servo/components/script/dom/window.rs @@ -21,7 +21,7 @@ use dom::navigator::Navigator; use dom::performance::Performance; use dom::screen::Screen; use dom::storage::Storage; -use layout_interface::NoQuery; +use layout_interface::{NoQuery, ReflowForDisplay, ReflowGoal, ReflowQueryType}; use page::Page; use script_task::{ExitWindowMsg, ScriptChan, TriggerLoadMsg, TriggerFragmentMsg}; use script_task::FromWindow; @@ -299,9 +299,7 @@ impl Reflectable for Window { } pub trait WindowHelpers { - fn reflow(self); - fn flush_layout(self); - fn wait_until_safe_to_modify_dom(self); + fn flush_layout(self, goal: ReflowGoal, query: ReflowQueryType); fn init_browser_context(self, doc: JSRef); fn load_url(self, href: DOMString); fn handle_fire_timer(self, timer_id: TimerId); @@ -334,18 +332,8 @@ impl<'a> WindowHelpers for JSRef<'a, Window> { }) } - fn reflow(self) { - 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) { @@ -369,7 +357,7 @@ impl<'a> WindowHelpers for JSRef<'a, Window> { fn handle_fire_timer(self, timer_id: TimerId) { self.timers.fire_timer(timer_id, self.clone()); - self.flush_layout(); + self.flush_layout(ReflowForDisplay, NoQuery); } } diff --git a/servo/components/script/page.rs b/servo/components/script/page.rs index d634e86a84a1..c5b1f74c73ac 100644 --- a/servo/components/script/page.rs +++ b/servo/components/script/page.rs @@ -13,9 +13,9 @@ use dom::node::{Node, NodeHelpers}; use dom::window::Window; use layout_interface::{ ContentBoxQuery, ContentBoxResponse, ContentBoxesQuery, ContentBoxesResponse, - GetRPCMsg, HitTestResponse, LayoutChan, LayoutRPC, MouseOverResponse, NoQuery, - Reflow, ReflowForDisplay, ReflowForScriptQuery, ReflowGoal, ReflowMsg, - ReflowQueryType, TrustedNodeAddress + GetRPCMsg, HitTestResponse, LayoutChan, LayoutRPC, MouseOverResponse, Reflow, + ReflowForScriptQuery, ReflowGoal, ReflowMsg, ReflowQueryType, + TrustedNodeAddress }; use script_traits::{UntrustedNodeAddress, ScriptControlChan}; @@ -29,7 +29,7 @@ use servo_net::storage_task::StorageTask; use servo_util::geometry::{Au, MAX_RECT}; use servo_util::geometry; use servo_util::str::DOMString; -use servo_util::smallvec::{SmallVec1, SmallVec}; +use servo_util::smallvec::SmallVec; use std::cell::{Cell, Ref, RefMut}; use std::comm::{channel, Receiver, Empty, Disconnected}; use std::mem::replace; @@ -77,9 +77,6 @@ pub struct Page { /// Pending resize event, if any. pub resize_event: Cell>, - /// Any nodes that need to be dirtied before the next reflow. - pub pending_dirty_nodes: DOMRefCell>, - /// Pending scroll to fragment event, if any pub fragment_name: DOMRefCell>, @@ -95,15 +92,6 @@ pub struct Page { // Child Pages. pub children: DOMRefCell>>, - /// Whether layout needs to be run at all. - pub damaged: Cell, - - /// Number of pending reflows that were sent while layout was active. - pub pending_reflows: Cell, - - /// Number of unnecessary potential reflows that were skipped since the last reflow - pub avoided_reflows: Cell, - /// 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>, @@ -165,57 +153,35 @@ impl Page { url: DOMRefCell::new(None), next_subpage_id: Cell::new(SubpageId(0)), resize_event: Cell::new(None), - pending_dirty_nodes: DOMRefCell::new(SmallVec1::new()), fragment_name: DOMRefCell::new(None), last_reflow_id: Cell::new(0), resource_task: resource_task, storage_task: storage_task, constellation_chan: constellation_chan, children: DOMRefCell::new(vec!()), - damaged: Cell::new(false), - pending_reflows: Cell::new(0), - avoided_reflows: Cell::new(0), page_clip_rect: Cell::new(MAX_RECT), } } - pub fn flush_layout(&self, query: ReflowQueryType) { - // 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 { - 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> { - self.flush_layout(ContentBoxesQuery(content_boxes_request)); + self.flush_layout(ReflowForScriptQuery, ContentBoxesQuery(content_boxes_request)); self.join_layout(); //FIXME: is this necessary, or is layout_rpc's mutex good enough? let ContentBoxesResponse(rects) = self.layout_rpc.content_boxes(); rects @@ -276,6 +242,13 @@ impl Page { } } } + + pub fn dirty_all_nodes(&self) { + match *self.frame.borrow() { + None => {} + Some(ref frame) => frame.document.root().dirty_all_nodes(), + } + } } impl Iterator> for PageIterator { @@ -334,7 +307,7 @@ impl Page { /// Sends a ping to layout and waits for the response. The response will arrive when the /// layout task has finished any pending request messages. - pub fn join_layout(&self) { + fn join_layout(&self) { let mut layout_join_port = self.layout_join_port.borrow_mut(); if layout_join_port.is_some() { let join_port = replace(&mut *layout_join_port, None); @@ -358,11 +331,11 @@ impl Page { } } - /// Reflows the page if it's possible to do so. This method will wait until the layout task has - /// 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, @@ -375,54 +348,54 @@ impl Page { } }; - 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); + 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(); - - // Layout will let us know when it's done. - let (join_chan, join_port) = channel(); - let mut layout_join_port = self.layout_join_port.borrow_mut(); - *layout_join_port = Some(join_port); - - let last_reflow_id = &self.last_reflow_id; - last_reflow_id.set(last_reflow_id.get() + 1); - - let root: JSRef = NodeCast::from_ref(*root); - - let window_size = self.window_size.get(); - self.damaged.set(false); - - // Send new document and relevant styles to layout. - let reflow = box Reflow { - 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 LayoutChan(ref chan) = self.layout_chan; - chan.send(ReflowMsg(reflow)); - - debug!("script: layout forked") - } + let root: JSRef = NodeCast::from_ref(*root); + if !root.get_has_dirty_descendants() { + debug!("root has no dirty descendants; avoiding reflow"); + return } - } - pub fn damage(&self) { - self.damaged.set(true); + debug!("script: performing reflow for goal {}", goal); + + // Layout will let us know when it's done. + let (join_chan, join_port) = channel(); + + { + let mut layout_join_port = self.layout_join_port.borrow_mut(); + *layout_join_port = Some(join_port); + } + + let last_reflow_id = &self.last_reflow_id; + last_reflow_id.set(last_reflow_id.get() + 1); + + let window_size = self.window_size.get(); + + // 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 LayoutChan(ref chan) = self.layout_chan; + chan.send(ReflowMsg(reflow)); + + debug!("script: layout forked"); + + self.join_layout(); } /// Attempt to find a named element in this page's document. diff --git a/servo/components/script/script_task.rs b/servo/components/script/script_task.rs index d44bec8e2682..9a20afe4a7b0 100644 --- a/servo/components/script/script_task.rs +++ b/servo/components/script/script_task.rs @@ -23,8 +23,7 @@ use dom::event::{Event, EventHelpers, Bubbles, DoesNotBubble, Cancelable, NotCan use dom::uievent::UIEvent; use dom::eventtarget::{EventTarget, EventTargetHelpers}; use dom::keyboardevent::KeyboardEvent; -use dom::node; -use dom::node::{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}; @@ -40,10 +39,9 @@ use devtools_traits::{DevtoolScriptControlMsg, EvaluateJS, GetDocumentElement}; use devtools_traits::{GetChildren, GetLayout, ModifyAttribute}; use script_traits::{CompositorEvent, ResizeEvent, ReflowEvent, ClickEvent, MouseDownEvent}; use script_traits::{MouseMoveEvent, MouseUpEvent, ConstellationControlMsg, ScriptTaskFactory}; -use script_traits::{ResizeMsg, AttachLayoutMsg, LoadMsg, ViewportMsg, SendEventMsg}; +use script_traits::{ResizeMsg, AttachLayoutMsg, GetTitleMsg, KeyEvent, LoadMsg, ViewportMsg}; use script_traits::{ResizeInactiveMsg, ExitPipelineMsg, NewLayoutInfo, OpaqueScriptLayoutChannel}; -use script_traits::{ScriptControlChan, ReflowCompleteMsg, UntrustedNodeAddress, KeyEvent}; -use script_traits::{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}; @@ -57,7 +55,7 @@ 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; @@ -74,7 +72,6 @@ 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; @@ -480,8 +477,6 @@ impl ScriptTask { } }; - let mut needs_reflow = HashSet::new(); - // Squash any pending resize and reflow events in the queue. loop { match event { @@ -496,18 +491,12 @@ impl ScriptTask { let page = page.find(id).expect("resize sent to nonexistent pipeline"); page.resize_event.set(Some(size)); } - FromConstellation(SendEventMsg(id, ReflowEvent(node_addresses))) => { - 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); } } _ => { @@ -541,11 +530,6 @@ impl ScriptTask { } } - // Now process any pending reflows. - for id in needs_reflow.into_iter() { - self.handle_event(id, ReflowEvent(SmallVec1::new())); - } - true } @@ -666,11 +650,6 @@ impl ScriptTask { } self.compositor.borrow_mut().set_ready_state(pipeline_id, FinishedLoading); - - if page.pending_reflows.get() > 0 { - page.pending_reflows.set(0); - self.force_reflow(&*page); - } } /// Handles a navigate forward or backward message. @@ -838,8 +817,12 @@ impl ScriptTask { // Kick off the initial reflow of the page. debug!("kicking off initial reflow of {}", final_url); - document.content_changed(NodeCast::from_ref(*document)); - window.flush_layout(); + { + let document_js_ref = (&*document).clone(); + let document_as_node = NodeCast::from_ref(document_js_ref); + document.content_changed(document_as_node, OtherNodeDamage); + } + window.flush_layout(ReflowForDisplay, NoQuery); { // No more reflow required @@ -895,18 +878,9 @@ impl ScriptTask { 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(), @@ -922,9 +896,27 @@ impl ScriptTask { 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) => { @@ -1020,7 +1012,7 @@ impl ScriptTask { _ => () } - window.flush_layout(); + window.flush_layout(ReflowForDisplay, NoQuery); } /// The entry point for content to notify that a new load has been requested @@ -1084,18 +1076,12 @@ impl ScriptTask { } } - fn handle_reflow_event(&self, pipeline_id: PipelineId, to_dirty: SmallVec1) { + 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); } } @@ -1138,7 +1124,7 @@ impl ScriptTask { el.authentic_click_activation(*event); doc.commit_focus_transaction(); - window.flush_layout(); + window.flush_layout(ReflowForDisplay, NoQuery); } None => {} } @@ -1220,8 +1206,6 @@ impl ScriptTask { /// Shuts down layout for the given page tree. fn shut_down_layout(page_tree: &Rc, 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(); diff --git a/servo/tests/content/test_getBoundingClientRect.html b/servo/tests/content/test_getBoundingClientRect.html index e99bc1870dce..4e751db844f3 100644 --- a/servo/tests/content/test_getBoundingClientRect.html +++ b/servo/tests/content/test_getBoundingClientRect.html @@ -3,8 +3,9 @@