Remove compositor layers when iframes are removed from doc or display:none.

10 files changed, 145 insertions, 37 deletions

diff --git a/components/layout/block.rs b/components/layout/block.rs
index e26fb5f8b8e..95ae010be07 100644
--- a/components/layout/block.rs
+++ b/components/layout/block.rs
@@ -52,6 +52,7 @@ use geom::{Point2D, Rect, Size2D};
 use gfx::display_list::{ClippingRegion, DisplayList};
 use rustc_serialize::{Encoder, Encodable};
 use msg::compositor_msg::LayerId;
+use msg::constellation_msg::ConstellationChan;
 use servo_util::geometry::{Au, MAX_AU};
 use servo_util::logical_geometry::{LogicalPoint, LogicalRect, LogicalSize};
 use servo_util::opts;
@@ -1915,6 +1916,10 @@ impl Flow for BlockFlow {
                                                             CoordinateSystem::Parent)
                               .translate(stacking_context_position));
     }
+
+    fn remove_compositor_layers(&self, constellation_chan: ConstellationChan) {
+        self.fragment.remove_compositor_layers(constellation_chan);
+    }
 }
 
 impl fmt::Debug for BlockFlow {
diff --git a/components/layout/construct.rs b/components/layout/construct.rs
index b2ea8090ea6..c7e56426adc 100644
--- a/components/layout/construct.rs
+++ b/components/layout/construct.rs
@@ -232,6 +232,20 @@ impl<'a> FlowConstructor<'a> {
         }
     }
 
+    #[inline]
+    fn set_flow_construction_result(&self, node: &ThreadSafeLayoutNode, result: ConstructionResult) {
+        match result {
+            ConstructionResult::None => {
+                let mut layout_data_ref = node.mutate_layout_data();
+                let layout_data = layout_data_ref.as_mut().expect("no layout data");
+                layout_data.remove_compositor_layers(self.layout_context.shared.constellation_chan.clone());
+            }
+            _ => {}
+        }
+
+        node.set_flow_construction_result(result);
+    }
+
     /// Builds the `ImageFragmentInfo` for the given image. This is out of line to guide inlining.
     fn build_fragment_info_for_image(&mut self, node: &ThreadSafeLayoutNode, url: Option<Url>)
                                 -> SpecificFragmentInfo {
@@ -380,7 +394,7 @@ impl<'a> FlowConstructor<'a> {
                 // If kid_flow is TableCaptionFlow, kid_flow should be added under
                 // TableWrapperFlow.
                 if flow.is_table() && kid_flow.is_table_caption() {
-                    kid.set_flow_construction_result(ConstructionResult::Flow(kid_flow,
+                    self.set_flow_construction_result(&kid, ConstructionResult::Flow(kid_flow,
                                                                             Descendants::new()))
                 } else if flow.need_anonymous_flow(&*kid_flow) {
                     consecutive_siblings.push(kid_flow)
@@ -561,7 +575,7 @@ impl<'a> FlowConstructor<'a> {
             // box, so don't construct them.
             if node.type_id() == Some(NodeTypeId::Element(ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLTextAreaElement))) {
                 for kid in node.children() {
-                    kid.set_flow_construction_result(ConstructionResult::None)
+                    self.set_flow_construction_result(&kid, ConstructionResult::None)
                 }
             }
             Some(Fragment::new_from_specific_info(
@@ -691,7 +705,7 @@ impl<'a> FlowConstructor<'a> {
     fn build_fragments_for_replaced_inline_content(&mut self, node: &ThreadSafeLayoutNode)
                                                    -> ConstructionResult {
         for kid in node.children() {
-            kid.set_flow_construction_result(ConstructionResult::None)
+            self.set_flow_construction_result(&kid, ConstructionResult::None)
         }
 
         // If this node is ignorable whitespace, bail out now.
@@ -1045,7 +1059,7 @@ impl<'a> FlowConstructor<'a> {
                                         -> ConstructionResult {
         // CSS 2.1 § 17.2.1. Treat all child fragments of a `table-column` as `display: none`.
         for kid in node.children() {
-            kid.set_flow_construction_result(ConstructionResult::None)
+            self.set_flow_construction_result(&kid, ConstructionResult::None)
         }
 
         let specific = SpecificFragmentInfo::TableColumn(TableColumnFragmentInfo::new(node));
@@ -1173,15 +1187,15 @@ impl<'a> PostorderNodeMutTraversal for FlowConstructor<'a> {
             // results of children.
             (display::T::none, _, _) => {
                 for child in node.children() {
-                    child.set_flow_construction_result(ConstructionResult::None);
+                    self.set_flow_construction_result(&child, ConstructionResult::None);
                 }
-                node.set_flow_construction_result(ConstructionResult::None);
+                self.set_flow_construction_result(node, ConstructionResult::None);
             }
 
             // Table items contribute table flow construction results.
             (display::T::table, float_value, _) => {
                 let construction_result = self.build_flow_for_table_wrapper(node, float_value);
-                node.set_flow_construction_result(construction_result)
+                self.set_flow_construction_result(node, construction_result)
             }
 
             // Absolutely positioned elements will have computed value of
@@ -1192,13 +1206,14 @@ impl<'a> PostorderNodeMutTraversal for FlowConstructor<'a> {
             // below.
             (display::T::block, _, position::T::absolute) |
             (_, _, position::T::fixed) => {
-                node.set_flow_construction_result(self.build_flow_for_nonfloated_block(node))
+                let construction_result = self.build_flow_for_nonfloated_block(node);
+                self.set_flow_construction_result(node, construction_result)
             }
 
             // List items contribute their own special flows.
             (display::T::list_item, float_value, _) => {
-                node.set_flow_construction_result(self.build_flow_for_list_item(node,
-                                                                                float_value))
+                let construction_result = self.build_flow_for_list_item(node, float_value);
+                self.set_flow_construction_result(node, construction_result)
             }
 
             // Inline items that are absolutely-positioned contribute inline fragment construction
@@ -1206,7 +1221,7 @@ impl<'a> PostorderNodeMutTraversal for FlowConstructor<'a> {
             (display::T::inline, _, position::T::absolute) => {
                 let construction_result =
                     self.build_fragment_for_absolutely_positioned_inline(node);
-                node.set_flow_construction_result(construction_result)
+                self.set_flow_construction_result(node, construction_result)
             }
 
             // Inline items contribute inline fragment construction results.
@@ -1214,31 +1229,31 @@ impl<'a> PostorderNodeMutTraversal for FlowConstructor<'a> {
             // FIXME(pcwalton, #3307): This is not sufficient to handle floated generated content.
             (display::T::inline, float::T::none, _) => {
                 let construction_result = self.build_fragments_for_inline(node);
-                node.set_flow_construction_result(construction_result)
+                self.set_flow_construction_result(node, construction_result)
             }
 
             // Inline-block items contribute inline fragment construction results.
             (display::T::inline_block, float::T::none, _) => {
                 let construction_result = self.build_fragment_for_inline_block(node);
-                node.set_flow_construction_result(construction_result)
+                self.set_flow_construction_result(node, construction_result)
             }
 
             // Table items contribute table flow construction results.
             (display::T::table_caption, _, _) => {
                 let construction_result = self.build_flow_for_table_caption(node);
-                node.set_flow_construction_result(construction_result)
+                self.set_flow_construction_result(node, construction_result)
             }
 
             // Table items contribute table flow construction results.
             (display::T::table_column_group, _, _) => {
                 let construction_result = self.build_flow_for_table_colgroup(node);
-                node.set_flow_construction_result(construction_result)
+                self.set_flow_construction_result(node, construction_result)
             }
 
             // Table items contribute table flow construction results.
             (display::T::table_column, _, _) => {
                 let construction_result = self.build_fragments_for_table_column(node);
-                node.set_flow_construction_result(construction_result)
+                self.set_flow_construction_result(node, construction_result)
             }
 
             // Table items contribute table flow construction results.
@@ -1246,19 +1261,19 @@ impl<'a> PostorderNodeMutTraversal for FlowConstructor<'a> {
             (display::T::table_header_group, _, _) |
             (display::T::table_footer_group, _, _) => {
                 let construction_result = self.build_flow_for_table_rowgroup(node);
-                node.set_flow_construction_result(construction_result)
+                self.set_flow_construction_result(node, construction_result)
             }
 
             // Table items contribute table flow construction results.
             (display::T::table_row, _, _) => {
                 let construction_result = self.build_flow_for_table_row(node);
-                node.set_flow_construction_result(construction_result)
+                self.set_flow_construction_result(node, construction_result)
             }
 
             // Table items contribute table flow construction results.
             (display::T::table_cell, _, _) => {
                 let construction_result = self.build_flow_for_table_cell(node);
-                node.set_flow_construction_result(construction_result)
+                self.set_flow_construction_result(node, construction_result)
             }
 
             // Block flows that are not floated contribute block flow construction results.
@@ -1267,14 +1282,15 @@ impl<'a> PostorderNodeMutTraversal for FlowConstructor<'a> {
             // properties separately.
 
             (_, float::T::none, _) => {
-                node.set_flow_construction_result(self.build_flow_for_nonfloated_block(node))
+                let construction_result = self.build_flow_for_nonfloated_block(node);
+                self.set_flow_construction_result(node, construction_result)
             }
 
             // Floated flows contribute float flow construction results.
             (_, float_value, _) => {
                 let float_kind = FloatKind::from_property(float_value);
-                node.set_flow_construction_result(
-                    self.build_flow_for_floated_block(node, float_kind))
+                let construction_result = self.build_flow_for_floated_block(node, float_kind);
+                self.set_flow_construction_result(node, construction_result)
             }
         }
 
@@ -1329,13 +1345,6 @@ impl<'ln> NodeUtils for ThreadSafeLayoutNode<'ln> {
         let mut layout_data_ref = self.mutate_layout_data();
         let layout_data = layout_data_ref.as_mut().expect("no layout data");
 
-        match result {
-            ConstructionResult::None => {
-                layout_data.clear();
-            }
-            _ => {}
-        }
-
         let dst = self.get_construction_result(layout_data);
 
         *dst = result;
diff --git a/components/layout/flow.rs b/components/layout/flow.rs
index 19f1df86c3a..ec618e03847 100644
--- a/components/layout/flow.rs
+++ b/components/layout/flow.rs
@@ -49,6 +49,7 @@ use wrapper::ThreadSafeLayoutNode;
 use geom::{Point2D, Rect, Size2D};
 use gfx::display_list::ClippingRegion;
 use rustc_serialize::{Encoder, Encodable};
+use msg::constellation_msg::ConstellationChan;
 use msg::compositor_msg::LayerId;
 use servo_util::geometry::{Au, ZERO_RECT};
 use servo_util::logical_geometry::{LogicalRect, LogicalSize, WritingMode};
@@ -312,6 +313,9 @@ pub trait Flow: fmt::Debug + Sync {
     /// Attempts to perform incremental fixup of this flow by replacing its fragment's style with
     /// the new style. This can only succeed if the flow has exactly one fragment.
     fn repair_style(&mut self, new_style: &Arc<ComputedValues>);
+
+    /// Remove any compositor layers associated with this flow
+    fn remove_compositor_layers(&self, _: ConstellationChan) {}
 }
 
 // Base access
diff --git a/components/layout/fragment.rs b/components/layout/fragment.rs
index 26a91139b31..31642bb4cd9 100644
--- a/components/layout/fragment.rs
+++ b/components/layout/fragment.rs
@@ -30,7 +30,7 @@ use gfx::text::glyph::CharIndex;
 use gfx::text::text_run::{TextRun, TextRunSlice};
 use script_traits::UntrustedNodeAddress;
 use rustc_serialize::{Encodable, Encoder};
-use msg::constellation_msg::{PipelineId, SubpageId};
+use msg::constellation_msg::{ConstellationChan, Msg, PipelineId, SubpageId};
 use net::image::holder::ImageHolder;
 use net::local_image_cache::LocalImageCache;
 use servo_util::geometry::{self, Au, ZERO_POINT};
@@ -2058,6 +2058,23 @@ impl Fragment {
         // box too.
         overflow
     }
+
+    /// Remove any compositor layers associated with this fragment - it is being
+    /// removed from the tree or had its display property set to none.
+    /// TODO(gw): This just hides the compositor layer for now. In the future
+    /// it probably makes sense to provide a hint to the compositor whether
+    /// the layers should be destroyed to free memory.
+    pub fn remove_compositor_layers(&self, constellation_chan: ConstellationChan) {
+        match self.specific {
+            SpecificFragmentInfo::Iframe(ref iframe_info) => {
+                let ConstellationChan(ref chan) = constellation_chan;
+                chan.send(Msg::FrameRect(iframe_info.pipeline_id,
+                                         iframe_info.subpage_id,
+                                         Rect::zero())).unwrap();
+            }
+            _ => {}
+        }
+    }
 }
 
 impl fmt::Debug for Fragment {
diff --git a/components/layout/layout_task.rs b/components/layout/layout_task.rs
index ca321f23526..dedc682af4c 100644
--- a/components/layout/layout_task.rs
+++ b/components/layout/layout_task.rs
@@ -124,6 +124,9 @@ pub struct LayoutTask {
     //// The channel to send messages to ourself.
     pub chan: LayoutChan,
 
+    /// The channel on which messages can be sent to the constellation.
+    pub constellation_chan: ConstellationChan,
+
     /// The channel on which messages can be sent to the script task.
     pub script_chan: ScriptControlChan,
 
@@ -272,6 +275,7 @@ impl LayoutTask {
             pipeline_port: pipeline_port,
             chan: chan,
             script_chan: script_chan,
+            constellation_chan: constellation_chan.clone(),
             paint_chan: paint_chan,
             time_profiler_chan: time_profiler_chan,
             resource_task: resource_task,
@@ -412,7 +416,7 @@ impl LayoutTask {
             },
             Msg::ReapLayoutData(dead_layout_data) => {
                 unsafe {
-                    LayoutTask::handle_reap_layout_data(dead_layout_data)
+                    self.handle_reap_layout_data(dead_layout_data)
                 }
             },
             Msg::PrepareToExit(response_chan) => {
@@ -441,7 +445,7 @@ impl LayoutTask {
             match self.port.recv().unwrap() {
                 Msg::ReapLayoutData(dead_layout_data) => {
                     unsafe {
-                        LayoutTask::handle_reap_layout_data(dead_layout_data)
+                        self.handle_reap_layout_data(dead_layout_data)
                     }
                 }
                 Msg::ExitNow(exit_type) => {
@@ -938,9 +942,9 @@ impl LayoutTask {
 
     /// Handles a message to destroy layout data. Layout data must be destroyed on *this* task
     /// because the struct type is transmuted to a different type on the script side.
-    unsafe fn handle_reap_layout_data(layout_data: LayoutData) {
+    unsafe fn handle_reap_layout_data(&self, layout_data: LayoutData) {
         let layout_data_wrapper: LayoutDataWrapper = mem::transmute(layout_data);
-        layout_data_wrapper.clear();
+        layout_data_wrapper.remove_compositor_layers(self.constellation_chan.clone());
     }
 
     /// Returns profiling information which is passed to the time profiler.
diff --git a/components/layout/util.rs b/components/layout/util.rs
index 8d4d72e7466..5b3bccd5b07 100644
--- a/components/layout/util.rs
+++ b/components/layout/util.rs
@@ -4,7 +4,7 @@
 
 #![allow(unsafe_blocks)]
 
-use construct::ConstructionResult;
+use construct::{ConstructionItem, ConstructionResult};
 use incremental::RestyleDamage;
 use parallel::DomParallelInfo;
 use wrapper::{LayoutNode, TLayoutNode, ThreadSafeLayoutNode};
@@ -13,6 +13,7 @@ use azure::azure_hl::Color;
 use gfx::display_list::OpaqueNode;
 use gfx;
 use libc::{c_void, uintptr_t};
+use msg::constellation_msg::ConstellationChan;
 use script::dom::bindings::js::LayoutJS;
 use script::dom::node::{Node, SharedLayoutData};
 use script::layout_interface::{LayoutChan, TrustedNodeAddress};
@@ -79,8 +80,26 @@ pub struct LayoutDataWrapper {
 }
 
 impl LayoutDataWrapper {
-    pub fn clear(&self) {
-        // TODO: Clear items related to this node, e.g. compositor layers
+    pub fn remove_compositor_layers(&self, constellation_chan: ConstellationChan) {
+        match self.data.flow_construction_result {
+            ConstructionResult::None => {}
+            ConstructionResult::Flow(ref flow_ref, _) => {
+                flow_ref.remove_compositor_layers(constellation_chan);
+            }
+            ConstructionResult::ConstructionItem(ref construction_item) => {
+                match construction_item {
+                    &ConstructionItem::InlineFragments(ref inline_fragments) => {
+                        for fragment in inline_fragments.fragments.iter() {
+                            fragment.remove_compositor_layers(constellation_chan.clone());
+                        }
+                    }
+                    &ConstructionItem::Whitespace(..) => {}
+                    &ConstructionItem::TableColumnFragment(ref fragment) => {
+                        fragment.remove_compositor_layers(constellation_chan.clone());
+                    }
+                }
+            }
+        }
     }
 }
 
diff --git a/tests/ref/basic.list b/tests/ref/basic.list
index 0d83cc16d1f..b4bcd5d5e81 100644
--- a/tests/ref/basic.list
+++ b/tests/ref/basic.list
@@ -139,6 +139,8 @@ fragment=top != ../html/acid2.html acid2_ref.html
 == iframe/overflow.html iframe/overflow_ref.html
 == iframe/positioning_margin.html iframe/positioning_margin_ref.html
 == iframe/hide_and_show.html iframe/hide_and_show_ref.html
+== iframe/hide_layers1.html iframe/hide_layers_ref.html
+== iframe/hide_layers2.html iframe/hide_layers_ref.html
 
 == floated_generated_content_a.html floated_generated_content_b.html
 == inline_block_margin_a.html inline_block_margin_ref.html
diff --git a/tests/ref/iframe/hide_layers1.html b/tests/ref/iframe/hide_layers1.html
new file mode 100644
index 00000000000..159723211de
--- /dev/null
+++ b/tests/ref/iframe/hide_layers1.html
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+    <style type="text/css">
+        .hidden {
+            display: none;
+        }
+        body {
+            background-color: green;
+        }
+    </style>
+    <body>
+        <iframe id="iframe" src="data:text/html,%3Cspan%3EJust%20a%20simple%20little%20iframe.%3C%2Fspan%3E"></iframe>
+    </body>
+    <script type="text/javascript">
+        window.onload = function() {
+            document.getElementById("iframe").classList.add("hidden");
+            document.documentElement.classList.remove("reftest-wait");
+        }
+    </script>
+</html>
diff --git a/tests/ref/iframe/hide_layers2.html b/tests/ref/iframe/hide_layers2.html
new file mode 100644
index 00000000000..1bc79c654da
--- /dev/null
+++ b/tests/ref/iframe/hide_layers2.html
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+    <style type="text/css">
+        body {
+            background-color: green;
+        }
+    </style>
+    <body>
+        <iframe id="iframe" src="data:text/html,%3Cspan%3EJust%20a%20simple%20little%20iframe.%3C%2Fspan%3E"></iframe>
+    </body>
+    <script type="text/javascript">
+        window.onload = function() {
+            var iframe = document.getElementById("iframe");
+            iframe.parentNode.removeChild(iframe);
+            document.documentElement.classList.remove("reftest-wait");
+        }
+    </script>
+</html>
diff --git a/tests/ref/iframe/hide_layers_ref.html b/tests/ref/iframe/hide_layers_ref.html
new file mode 100644
index 00000000000..cb61f63d447
--- /dev/null
+++ b/tests/ref/iframe/hide_layers_ref.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+    <style type="text/css">
+        body {
+            background-color: green;
+        }
+    </style>
+    <body>
+    </body>
+</html>