diff --git a/components/layout/layout_task.rs b/components/layout/layout_task.rs index 995742cd745..deda7d481c7 100644 --- a/components/layout/layout_task.rs +++ b/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/components/net/image/holder.rs b/components/net/image/holder.rs index 989021c3789..4703f2cecd9 100644 --- a/components/net/image/holder.rs +++ b/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/components/script/dom/htmlimageelement.rs b/components/script/dom/htmlimageelement.rs index 12175c9de9a..dc1ef7d6e26 100644 --- a/components/script/dom/htmlimageelement.rs +++ b/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/components/script/script_task.rs b/components/script/script_task.rs index be034ed1888..9a20afe4a7b 100644 --- a/components/script/script_task.rs +++ b/components/script/script_task.rs @@ -55,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; @@ -72,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; @@ -478,8 +477,6 @@ impl ScriptTask { } }; - let mut needs_reflow = HashSet::new(); - // Squash any pending resize and reflow events in the queue. loop { match event { @@ -494,14 +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(_))) => { - 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); } } _ => { @@ -535,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 } @@ -906,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) => { @@ -1068,9 +1076,8 @@ 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() { diff --git a/tests/content/test_getBoundingClientRect.html b/tests/content/test_getBoundingClientRect.html index e99bc1870dc..4e751db844f 100644 --- a/tests/content/test_getBoundingClientRect.html +++ b/tests/content/test_getBoundingClientRect.html @@ -3,8 +3,9 @@