layout: Stop going to the DOM for iframe sizes

src/components/main/layout/block.rs

@@ -13,9 +13,9 @@ use layout::model::{MaybeAuto, Specified, Auto, specified_or_none, specified};
 use layout::float_context::{FloatContext, PlacementInfo, Invalid, FloatType};
 
 use std::cell::Cell;
-use geom::{Point2D, Rect, SideOffsets2D, Size2D};
+use geom::{Point2D, Rect, SideOffsets2D};
 use gfx::display_list::DisplayList;
-use servo_util::geometry::{Au, to_frac_px};
+use servo_util::geometry::Au;
 use servo_util::geometry;
 
 /// Information specific to floated blocks.
@@ -58,7 +58,7 @@ pub struct BlockFlow {
     /// Whether this block flow is the root flow.
     is_root: bool,
 
-    // Additional floating flow members.
+    /// Additional floating flow members.
     float: Option<~FloatedBlockInfo>
 }
 
@@ -471,27 +471,6 @@ impl BlockFlow {
             return self.build_display_list_float(builder, dirty, list);
         }
 
-        if self.base.node.is_iframe_element() {
-            let x = self.base.abs_position.x + do self.box.as_ref().map_default(Au::new(0)) |box| {
-                box.margin.get().left + box.border.get().left + box.padding.get().left
-            };
-            let y = self.base.abs_position.y + do self.box.as_ref().map_default(Au::new(0)) |box| {
-                box.margin.get().top + box.border.get().top + box.padding.get().top
-            };
-            let w = self.base.position.size.width - do self.box.as_ref().map_default(Au::new(0)) |box| {
-                box.noncontent_width()
-            };
-            let h = self.base.position.size.height - do self.box.as_ref().map_default(Au::new(0)) |box| {
-                box.noncontent_height()
-            };
-            do self.base.node.with_mut_iframe_element |iframe_element| {
-                iframe_element.size.get_mut_ref().set_rect(Rect(Point2D(to_frac_px(x) as f32,
-                                                                        to_frac_px(y) as f32),
-                                                                Size2D(to_frac_px(w) as f32,
-                                                                       to_frac_px(h) as f32)));
-            }
-        }
-
         let abs_rect = Rect(self.base.abs_position, self.base.position.size);
         if !abs_rect.intersects(dirty) {
             return true;
@@ -514,21 +493,17 @@ impl BlockFlow {
         false
     }
 
-    pub fn build_display_list_float<E:ExtraDisplayListData>(&mut self,
+    pub fn build_display_list_float<E:ExtraDisplayListData>(
-                                                            builder: &DisplayListBuilder,
+                                    &mut self,
-                                                            dirty: &Rect<Au>,
+                                    builder: &DisplayListBuilder,
-                                                            list: &Cell<DisplayList<E>>)
+                                    dirty: &Rect<Au>,
-                                                            -> bool {
+                                    list: &Cell<DisplayList<E>>)
-        //TODO: implement iframe size messaging
+                                    -> bool {
-        if self.base.node.is_iframe_element() {
-            error!("float iframe size messaging not implemented yet");
-        }
         let abs_rect = Rect(self.base.abs_position, self.base.position.size);
         if !abs_rect.intersects(dirty) {
-            return true;
+            return true
         }
 
-
         let offset = self.base.abs_position + self.float.get_ref().rel_pos;
         // add box that starts block context
         for box in self.box.iter() {

src/components/main/layout/box.rs

@@ -16,9 +16,11 @@ use gfx::display_list::{ClipDisplayItemClass};
 use gfx::font::{FontStyle, FontWeight300};
 use gfx::text::text_run::TextRun;
 use script::dom::node::{AbstractNode, LayoutView};
+use servo_msg::constellation_msg::{FrameRectMsg, PipelineId, SubpageId};
 use servo_net::image::holder::ImageHolder;
 use servo_net::local_image_cache::LocalImageCache;
 use servo_util::geometry::Au;
+use servo_util::geometry;
 use servo_util::range::*;
 use servo_util::slot::Slot;
 use servo_util::tree::{TreeNodeRef, ElementLike};
@@ -32,6 +34,7 @@ use style::computed_values::{border_style, clear, font_family, font_style, line_
 use style::computed_values::{text_align, text_decoration, vertical_align, visibility};
 
 use css::node_style::StyledNode;
+use layout::context::LayoutContext;
 use layout::display_list_builder::{DisplayListBuilder, ExtraDisplayListData, ToGfxColor};
 use layout::float_context::{ClearType, ClearLeft, ClearRight, ClearBoth};
 use layout::flow::Flow;
@@ -89,6 +92,7 @@ pub struct Box {
 pub enum SpecificBoxInfo {
     GenericBox,
     ImageBox(ImageBoxInfo),
+    IframeBox(IframeBoxInfo),
     ScannedTextBox(ScannedTextBoxInfo),
     UnscannedTextBox(UnscannedTextBoxInfo),
 }
@@ -146,6 +150,29 @@ impl ImageBoxInfo {
     }
 }
 
+/// A box that represents an inline frame (iframe). This stores the pipeline ID so that the size
+/// of this iframe can be communicated via the constellation to the iframe's own layout task.
+#[deriving(Clone)]
+pub struct IframeBoxInfo {
+    /// The pipeline ID of this iframe.
+    pipeline_id: PipelineId,
+    /// The subpage ID of this iframe.
+    subpage_id: SubpageId,
+}
+
+impl IframeBoxInfo {
+    /// Creates the information specific to an iframe box.
+    pub fn new(node: &AbstractNode<LayoutView>) -> IframeBoxInfo {
+        node.with_imm_iframe_element(|iframe_element| {
+            let size = iframe_element.size.unwrap();
+            IframeBoxInfo {
+                pipeline_id: size.pipeline_id,
+                subpage_id: size.subpage_id,
+            }
+        })
+    }
+}
+
 /// A scanned text box represents a single run of text with a distinct style. A `TextBox` may be
 /// split into two or more boxes across line breaks. Several `TextBox`es may correspond to a single
 /// DOM text node. Split text boxes are implemented by referring to subsets of a single `TextRun`
@@ -540,7 +567,7 @@ impl Box {
     /// Appendix E. Finally, the builder flattens the list.
     pub fn build_display_list<E:ExtraDisplayListData>(
                               &self,
-                              _: &DisplayListBuilder,
+                              builder: &DisplayListBuilder,
                               dirty: &Rect<Au>,
                               offset: Point2D<Au>,
                               flow: &Flow,
@@ -652,7 +679,7 @@ impl Box {
                     ()
                 });
             },
-            GenericBox => {
+            GenericBox | IframeBox(_) => {
                 do list.with_mut_ref |list| {
                     let item = ~ClipDisplayItem {
                         base: BaseDisplayItem {
@@ -726,6 +753,23 @@ impl Box {
             }
         }
 
+        // If this is an iframe, then send its position and size up to the constellation.
+        //
+        // FIXME(pcwalton): Doing this during display list construction seems potentially
+        // problematic if iframes are outside the area we're computing the display list for, since
+        // they won't be able to reflow at all until the user scrolls to them. Perhaps we should
+        // separate this into two parts: first we should send the size only to the constellation
+        // once that's computed during assign-heights, and second we should should send the origin
+        // to the constellation here during display list construction. This should work because
+        // layout for the iframe only needs to know size, and origin is only relevant if the
+        // iframe is actually going to be displayed.
+        match self.specific {
+            IframeBox(ref iframe_box) => {
+                self.finalize_position_and_size_of_iframe(iframe_box, offset, builder.ctx)
+            }
+            GenericBox | ImageBox(_) | ScannedTextBox(_) | UnscannedTextBox(_) => {}
+        }
+
         // Add a border, if applicable.
         //
         // TODO: Outlines.
@@ -736,7 +780,7 @@ impl Box {
     pub fn minimum_and_preferred_widths(&self) -> (Au, Au) {
         let guessed_width = self.guess_width();
         let (additional_minimum, additional_preferred) = match self.specific {
-            GenericBox => (Au(0), Au(0)),
+            GenericBox | IframeBox(_) => (Au(0), Au(0)),
             ImageBox(ref image_box_info) => {
                 let image_width = image_box_info.image_width();
                 (image_width, image_width)
@@ -764,7 +808,7 @@ impl Box {
     /// FIXME(pcwalton): This function *mutates* the height? Gross! Refactor please.
     pub fn box_height(&self) -> Au {
         match self.specific {
-            GenericBox => Au(0),
+            GenericBox | IframeBox(_) => Au(0),
             ImageBox(ref image_box_info) => {
                 let size = image_box_info.image.mutate().ptr.get_size();
                 let height = Au::from_px(size.unwrap_or(Size2D(0, 0)).height);
@@ -789,7 +833,7 @@ impl Box {
     /// Attempts to split this box so that its width is no more than `max_width`.
     pub fn split_to_width(&self, max_width: Au, starts_line: bool) -> SplitBoxResult {
         match self.specific {
-            GenericBox | ImageBox(_) => CannotSplit,
+            GenericBox | IframeBox(_) | ImageBox(_) => CannotSplit,
             UnscannedTextBox(_) => fail!("Unscanned text boxes should have been scanned by now!"),
             ScannedTextBox(ref text_box_info) => {
                 let mut pieces_processed_count: uint = 0;
@@ -899,7 +943,7 @@ impl Box {
     /// Assigns the appropriate width to this box.
     pub fn assign_width(&self) {
         match self.specific {
-            GenericBox => {
+            GenericBox | IframeBox(_) => {
                 // FIXME(pcwalton): This seems clownshoes; can we remove?
                 self.position.mutate().ptr.size.width = Au::from_px(45)
             }
@@ -942,6 +986,7 @@ impl Box {
     pub fn debug_str(&self) -> ~str {
         let class_name = match self.specific {
             GenericBox => "GenericBox",
+            IframeBox(_) => "IframeBox",
             ImageBox(_) => "ImageBox",
             ScannedTextBox(_) => "ScannedTextBox",
             UnscannedTextBox(_) => "UnscannedTextBox",
@@ -968,5 +1013,29 @@ impl Box {
                 *value.bottom,
                 *value.left)
     } 
+
+    /// Sends the size and position of this iframe box to the constellation. This is out of line to
+    /// guide inlining.
+    #[inline(never)]
+    fn finalize_position_and_size_of_iframe(&self,
+                                            iframe_box: &IframeBoxInfo,
+                                            offset: Point2D<Au>,
+                                            layout_context: &LayoutContext) {
+        let left = offset.x + self.margin.get().left + self.border.get().left +
+            self.padding.get().left;
+        let top = offset.y + self.margin.get().top + self.border.get().top +
+            self.padding.get().top;
+        let width = self.position.get().size.width - self.noncontent_width();
+        let height = self.position.get().size.height - self.noncontent_height();
+        let origin = Point2D(geometry::to_frac_px(left) as f32, geometry::to_frac_px(top) as f32);
+        let size = Size2D(geometry::to_frac_px(width) as f32, geometry::to_frac_px(height) as f32);
+        let rect = Rect(origin, size);
+
+        debug!("finalizing position and size of iframe for {:?},{:?}",
+               iframe_box.pipeline_id,
+               iframe_box.subpage_id);
+        let msg = FrameRectMsg(iframe_box.pipeline_id, iframe_box.subpage_id, rect);
+        layout_context.constellation_chan.send(msg)
+    }
 }
 

src/components/main/layout/construct.rs

@@ -22,7 +22,8 @@
 
 use css::node_style::StyledNode;
 use layout::block::BlockFlow;
-use layout::box::{Box, GenericBox, ImageBox, ImageBoxInfo, UnscannedTextBox, UnscannedTextBoxInfo};
+use layout::box::{Box, GenericBox, IframeBox, IframeBoxInfo, ImageBox, ImageBoxInfo};
+use layout::box::{UnscannedTextBox, UnscannedTextBoxInfo};
 use layout::context::LayoutContext;
 use layout::float_context::FloatType;
 use layout::flow::{Flow, FlowData, MutableFlowUtils};
@@ -30,7 +31,7 @@ use layout::inline::InlineFlow;
 use layout::text::TextRunScanner;
 use layout::util::LayoutDataAccess;
 
-use script::dom::element::HTMLImageElementTypeId;
+use script::dom::element::{HTMLIframeElementTypeId, HTMLImageElementTypeId};
 use script::dom::node::{AbstractNode, CommentNodeTypeId, DoctypeNodeTypeId};
 use script::dom::node::{DocumentFragmentNodeTypeId, DocumentNodeTypeId, ElementNodeTypeId};
 use script::dom::node::{LayoutView, PostorderNodeMutTraversal, TextNodeTypeId};
@@ -223,6 +224,7 @@ impl<'self> FlowConstructor<'self> {
                     Some(image_box_info) => ImageBox(image_box_info),
                 }
             }
+            ElementNodeTypeId(HTMLIframeElementTypeId) => IframeBox(IframeBoxInfo::new(&node)),
             TextNodeTypeId => UnscannedTextBox(UnscannedTextBoxInfo::new(&node)),
             _ => GenericBox,
         };

src/components/main/layout/context.rs

@@ -4,16 +4,19 @@
 
 //! Data needed by the layout task.
 
+use extra::arc::MutexArc;
 use geom::rect::Rect;
 use gfx::font_context::FontContext;
-use servo_util::geometry::Au;
+use servo_msg::constellation_msg::ConstellationChan;
 use servo_net::local_image_cache::LocalImageCache;
-
+use servo_util::geometry::Au;
-use extra::arc::MutexArc;
 
 /// Data needed by the layout task.
 pub struct LayoutContext {
     font_ctx: ~FontContext,
     image_cache: MutexArc<LocalImageCache>,
-    screen_size: Rect<Au>
+    screen_size: Rect<Au>,
+
+    /// A channel up to the constellation.
+    constellation_chan: ConstellationChan,
 }

src/components/main/layout/inline.rs

@@ -3,7 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use css::node_style::StyledNode;
-use layout::box::{Box, CannotSplit, GenericBox, ImageBox, ScannedTextBox, SplitDidFit};
+use layout::box::{Box, CannotSplit, GenericBox, IframeBox, ImageBox, ScannedTextBox, SplitDidFit};
 use layout::box::{SplitDidNotFit, UnscannedTextBox};
 use layout::context::LayoutContext;
 use layout::display_list_builder::{DisplayListBuilder, ExtraDisplayListData};
@@ -474,17 +474,12 @@ impl InlineFlow {
         self.boxes = ~[];
     }
 
-    pub fn build_display_list_inline<E:ExtraDisplayListData>(&self,
+    pub fn build_display_list_inline<E:ExtraDisplayListData>(
-                                                             builder: &DisplayListBuilder,
+                                     &self,
-                                                             dirty: &Rect<Au>,
+                                     builder: &DisplayListBuilder,
-                                                             list: &Cell<DisplayList<E>>)
+                                     dirty: &Rect<Au>,
-                                                             -> bool {
+                                     list: &Cell<DisplayList<E>>)
-
+                                     -> bool {
-        //TODO: implement inline iframe size messaging
-        if self.base.node.is_iframe_element() {
-            error!("inline iframe size messaging not implemented yet");
-        }
-
         let abs_rect = Rect(self.base.abs_position, self.base.position.size);
         if !abs_rect.intersects(dirty) {
             return true;
@@ -766,7 +761,7 @@ impl Flow for InlineFlow {
 
                         (text_offset, line_height - text_offset, text_ascent)
                     },
-                    GenericBox => {
+                    GenericBox | IframeBox(_) => {
                         let height = cur_box.position.get().size.height;
                         (height, Au::new(0), height)
                     },

src/components/main/layout/layout_task.rs

@@ -269,6 +269,7 @@ impl LayoutTask {
             image_cache: image_cache,
             font_ctx: font_ctx,
             screen_size: Rect(Point2D(Au(0), Au(0)), screen_size),
+            constellation_chan: self.constellation_chan.clone(),
         }
     }
 

src/components/script/dom/htmliframeelement.rs

@@ -11,8 +11,7 @@ use dom::node::{AbstractNode, Node, ScriptView};
 use dom::windowproxy::WindowProxy;
 
 use extra::url::Url;
-use geom::rect::Rect;
+use servo_msg::constellation_msg::{PipelineId, SubpageId};
-use servo_msg::constellation_msg::{ConstellationChan, FrameRectMsg, PipelineId, SubpageId};
 use std::ascii::StrAsciiExt;
 
 enum SandboxAllowance {
@@ -32,16 +31,9 @@ pub struct HTMLIFrameElement {
     sandbox: Option<u8>
 }
 
-struct IFrameSize {
+pub struct IFrameSize {
     pipeline_id: PipelineId,
     subpage_id: SubpageId,
-    constellation_chan: ConstellationChan,
-}
-
-impl IFrameSize {
-    pub fn set_rect(&mut self, rect: Rect<f32>) {
-        self.constellation_chan.send(FrameRectMsg(self.pipeline_id, self.subpage_id, rect));
-    }
 }
 
 impl HTMLIFrameElement {

src/components/script/html/hubbub_html_parser.rs

@@ -17,7 +17,7 @@ use script_task::page_from_context;
 use extra::url::Url;
 use hubbub::hubbub;
 use js::jsapi::JSContext;
-use servo_msg::constellation_msg::{ConstellationChan, SubpageId};
+use servo_msg::constellation_msg::SubpageId;
 use servo_net::image_cache_task::ImageCacheTask;
 use servo_net::resource_task::{Load, Payload, Done, ResourceTask, load_whole_resource};
 use servo_util::tree::{TreeNodeRef, ElementLike};
@@ -248,8 +248,8 @@ pub fn parse_html(cx: *JSContext,
                   url: Url,
                   resource_task: ResourceTask,
                   image_cache_task: ImageCacheTask,
-                  next_subpage_id: SubpageId,
+                  next_subpage_id: SubpageId)
-                  constellation_chan: ConstellationChan) -> HtmlParserResult {
+                  -> HtmlParserResult {
     debug!("Hubbub: parsing {:?}", url);
     // Spawn a CSS parser to receive links to CSS style sheets.
     let resource_task2 = resource_task.clone();
@@ -385,7 +385,6 @@ pub fn parse_html(cx: *JSContext,
                             iframe_element.size = Some(IFrameSize {
                                 pipeline_id: pipeline_id,
                                 subpage_id: subpage_id,
-                                constellation_chan: constellation_chan.clone(),
                             });
                             iframe_chan.send(HtmlDiscoveredIFrame((iframe_url,
                                                                    subpage_id,

src/components/script/script_task.rs

@@ -723,11 +723,11 @@ impl ScriptTask {
                                                                  url.clone(),
                                                                  self.resource_task.clone(),
                                                                  self.image_cache_task.clone(),
-                                                                 page.next_subpage_id.clone(),
+                                                                 page.next_subpage_id.clone());
-                                                                 self.constellation_chan.clone());
 
-
+        let HtmlParserResult {
-        let HtmlParserResult {discovery_port} = html_parsing_result;
+            discovery_port
+        } = html_parsing_result;
 
         // Create the root frame.
         page.frame = Some(Frame {