From fbb1e0c6b848303be79d2f30ce6257d70a8e38da Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Mon, 10 Nov 2014 11:34:44 -0800 Subject: [PATCH] Send incremental frame tree updates to the compositor This allows the compositor to add frames after the call to SetIds, where the initial frame tree is created. There are still some issues preventing proper late frame creation, but this prevents crashes when it happens. Fixes #3738. --- components/compositing/compositor.rs | 46 +++++++++-- components/compositing/compositor_task.rs | 5 +- components/compositing/constellation.rs | 89 +++++++++++++++++++--- components/compositing/headless.rs | 6 +- components/script/dom/htmliframeelement.rs | 12 +-- 5 files changed, 130 insertions(+), 28 deletions(-) diff --git a/components/compositing/compositor.rs b/components/compositing/compositor.rs index eb0301e9208..50a47af45a4 100644 --- a/components/compositing/compositor.rs +++ b/components/compositing/compositor.rs @@ -4,12 +4,13 @@ use compositor_layer::{CompositorData, CompositorLayer, DoesntWantScrollEvents}; use compositor_layer::{ScrollPositionChanged, WantsScrollEvents}; -use compositor_task::{Msg, CompositorTask, Exit, ChangeReadyState, SetIds, LayerProperties}; -use compositor_task::{GetGraphicsMetadata, CreateOrUpdateRootLayer, CreateOrUpdateDescendantLayer}; -use compositor_task::{SetLayerOrigin, Paint, ScrollFragmentPoint, LoadComplete}; -use compositor_task::{ShutdownComplete, ChangeRenderState, RenderMsgDiscarded, ScrollTimeout}; -use compositor_task::{CompositorEventListener, CompositorProxy, CompositorReceiver}; -use constellation::SendableFrameTree; +use compositor_task::{ChangeReadyState, ChangeRenderState, CompositorEventListener}; +use compositor_task::{CompositorProxy, CompositorReceiver, CompositorTask}; +use compositor_task::{CreateOrUpdateDescendantLayer, CreateOrUpdateRootLayer, Exit}; +use compositor_task::{FrameTreeUpdateMsg, GetGraphicsMetadata, LayerProperties}; +use compositor_task::{LoadComplete, Msg, Paint, RenderMsgDiscarded, ScrollFragmentPoint}; +use compositor_task::{ScrollTimeout, SetIds, SetLayerOrigin, ShutdownComplete}; +use constellation::{SendableFrameTree, FrameTreeDiff}; use pipeline::CompositionPipeline; use scrolling::ScrollingTimerProxy; use windowing; @@ -267,6 +268,11 @@ impl IOCompositor { new_constellation_chan); } + (FrameTreeUpdateMsg(frame_tree_diff, response_channel), NotShuttingDown) => { + self.update_frame_tree(&frame_tree_diff); + response_channel.send(()); + } + (CreateOrUpdateRootLayer(layer_properties), NotShuttingDown) => { self.create_or_update_root_layer(layer_properties); } @@ -446,6 +452,34 @@ impl IOCompositor { return root_layer; } + fn update_frame_tree(&mut self, frame_tree_diff: &FrameTreeDiff) { + let layer_properties = LayerProperties { + pipeline_id: frame_tree_diff.pipeline.id, + epoch: Epoch(0), + id: LayerId::null(), + rect: Rect::zero(), + background_color: azure_hl::Color::new(0., 0., 0., 0.), + scroll_policy: Scrollable, + }; + let root_layer = CompositorData::new_layer(frame_tree_diff.pipeline.clone(), + layer_properties, + WantsScrollEvents, + opts::get().tile_size); + + match frame_tree_diff.rect { + Some(ref frame_rect) => { + *root_layer.masks_to_bounds.borrow_mut() = true; + + let frame_rect = frame_rect.to_untyped(); + *root_layer.bounds.borrow_mut() = Rect::from_untyped(&frame_rect); + } + None => {} + } + + let parent_layer = self.find_pipeline_root_layer(frame_tree_diff.parent_pipeline.id); + parent_layer.add_child(root_layer); + } + fn find_pipeline_root_layer(&self, pipeline_id: PipelineId) -> Rc> { match self.find_layer_with_pipeline_and_layer_id(pipeline_id, LayerId::null()) { Some(ref layer) => layer.clone(), diff --git a/components/compositing/compositor_task.rs b/components/compositing/compositor_task.rs index 5fe2c4bb57a..cfbb5acb2a6 100644 --- a/components/compositing/compositor_task.rs +++ b/components/compositing/compositor_task.rs @@ -5,7 +5,7 @@ //! Communication with the compositor task. pub use windowing; -pub use constellation::SendableFrameTree; +pub use constellation::{SendableFrameTree, FrameTreeDiff}; use compositor; use headless; @@ -189,6 +189,8 @@ pub enum Msg { RenderMsgDiscarded, /// Sets the channel to the current layout and render tasks, along with their id SetIds(SendableFrameTree, Sender<()>, ConstellationChan), + /// Sends an updated version of the frame tree. + FrameTreeUpdateMsg(FrameTreeDiff, Sender<()>), /// The load of a page for a given URL has completed. LoadComplete(PipelineId, Url), /// Indicates that the scrolling timeout with the given starting timestamp has happened and a @@ -211,6 +213,7 @@ impl Show for Msg { ChangeRenderState(..) => write!(f, "ChangeRenderState"), RenderMsgDiscarded(..) => write!(f, "RenderMsgDiscarded"), SetIds(..) => write!(f, "SetIds"), + FrameTreeUpdateMsg(..) => write!(f, "FrameTreeUpdateMsg"), LoadComplete(..) => write!(f, "LoadComplete"), ScrollTimeout(..) => write!(f, "ScrollTimeout"), } diff --git a/components/compositing/constellation.rs b/components/compositing/constellation.rs index 2360315f6a3..7bbf1ebb5fb 100644 --- a/components/compositing/constellation.rs +++ b/components/compositing/constellation.rs @@ -4,7 +4,7 @@ use pipeline::{Pipeline, CompositionPipeline}; -use compositor_task::{CompositorProxy, LoadComplete, ShutdownComplete, SetLayerOrigin, SetIds}; +use compositor_task::{CompositorProxy, FrameTreeUpdateMsg, LoadComplete, ShutdownComplete, SetLayerOrigin, SetIds}; use devtools_traits::DevtoolsControlChan; use geom::rect::{Rect, TypedRect}; use geom::scale_factor::ScaleFactor; @@ -29,7 +29,7 @@ use servo_util::geometry::{PagePx, ViewportPx}; use servo_util::opts; use servo_util::task::spawn_named; use servo_util::time::TimeProfilerChan; -use std::cell::RefCell; +use std::cell::{Cell, RefCell}; use std::collections::hashmap::{HashMap, HashSet}; use std::io; use std::mem::replace; @@ -83,6 +83,7 @@ struct FrameTree { pub pipeline: Rc, pub parent: RefCell>>, pub children: RefCell>, + pub has_compositor_layer: Cell, } impl FrameTree { @@ -94,6 +95,7 @@ impl FrameTree { None => RefCell::new(None), }, children: RefCell::new(vec!()), + has_compositor_layer: Cell::new(false), } } } @@ -130,6 +132,16 @@ enum ReplaceResult { OriginalNode(Rc), } +/// A struct that triggers the addition of a new frame to a previously existing frame tree. +pub struct FrameTreeDiff { + /// The parent pipeline of the new frame. + pub parent_pipeline: CompositionPipeline, + /// The pipeline of the new frame itself. + pub pipeline: CompositionPipeline, + /// The frame rect of the new frame used for positioning its compositor layer. + pub rect: Option>, +} + impl FrameTree { fn to_sendable(&self) -> SendableFrameTree { let sendable_frame_tree = SendableFrameTree { @@ -744,21 +756,22 @@ impl Constellation { } + fn pipeline_is_in_current_frame(&self, pipeline_id: PipelineId) -> bool { + self.current_frame().iter() + .any(|current_frame| current_frame.contains(pipeline_id)) + } + fn handle_renderer_ready_msg(&mut self, pipeline_id: PipelineId) { debug!("Renderer {:?} ready to send paint msg", pipeline_id); // This message could originate from a pipeline in the navigation context or // from a pending frame. The only time that we will grant paint permission is // when the message originates from a pending frame or the current frame. - for current_frame in self.current_frame().iter() { - // Messages originating in the current frame are not navigations; - // they may come from a page load in a subframe. - if current_frame.contains(pipeline_id) { - for frame in current_frame.iter() { - frame.pipeline.grant_paint_permission(); - } - return; - } + // Messages originating in the current frame are not navigations; + // they may come from a page load in a subframe. + if self.pipeline_is_in_current_frame(pipeline_id) { + self.create_compositor_layer_for_iframe_if_necessary(pipeline_id); + return; } // Find the pending frame change whose new pipeline id is pipeline_id. @@ -916,11 +929,65 @@ impl Constellation { Ok(()) => { let mut iter = frame_tree.iter(); for frame in iter { + frame.has_compositor_layer.set(true); frame.pipeline.grant_paint_permission(); } } Err(()) => {} // message has been discarded, probably shutting down } } + + fn find_child_parent_pair_in_frame_tree(&self, + frame_tree: Rc, + child_pipeline_id: PipelineId) + -> Option<(ChildFrameTree, Rc)> { + for child in frame_tree.children.borrow().iter() { + let child_frame_tree = child.frame_tree.clone(); + if child.frame_tree.pipeline.id == child_pipeline_id { + return Some((ChildFrameTree::new(child_frame_tree, child.rect), + frame_tree.clone())); + } + let result = self.find_child_parent_pair_in_frame_tree(child_frame_tree, + child_pipeline_id); + if result.is_some() { + return result; + } + } + None + } + + fn create_compositor_layer_for_iframe_if_necessary(&mut self, pipeline_id: PipelineId) { + let current_frame_tree = match self.current_frame() { + &Some(ref tree) => tree.clone(), + &None => return, + }; + + let (child, parent) = + match self.find_child_parent_pair_in_frame_tree(current_frame_tree, pipeline_id) { + Some(pair) => pair, + None => return, + }; + + if child.frame_tree.has_compositor_layer.get() { + child.frame_tree.pipeline.grant_paint_permission(); + return; + } + + let sendable_frame_tree_diff = FrameTreeDiff { + parent_pipeline: parent.pipeline.to_sendable(), + pipeline: child.frame_tree.pipeline.to_sendable(), + rect: child.rect, + }; + + let (chan, port) = channel(); + self.compositor_proxy.send(FrameTreeUpdateMsg(sendable_frame_tree_diff, chan)); + match port.recv_opt() { + Ok(()) => { + child.frame_tree.has_compositor_layer.set(true); + child.frame_tree.pipeline.grant_paint_permission(); + } + Err(()) => {} // The message has been discarded, we are probably shutting down. + } + } } diff --git a/components/compositing/headless.rs b/components/compositing/headless.rs index 648b71034e0..eaeb09eb7da 100644 --- a/components/compositing/headless.rs +++ b/components/compositing/headless.rs @@ -5,7 +5,7 @@ use compositor_task::{GetGraphicsMetadata, CreateOrUpdateRootLayer, CreateOrUpdateDescendantLayer}; use compositor_task::{Exit, ChangeReadyState, LoadComplete, Paint, ScrollFragmentPoint, SetIds}; use compositor_task::{SetLayerOrigin, ShutdownComplete, ChangeRenderState, RenderMsgDiscarded}; -use compositor_task::{CompositorEventListener, CompositorReceiver, ScrollTimeout}; +use compositor_task::{CompositorEventListener, CompositorReceiver, ScrollTimeout, FrameTreeUpdateMsg}; use windowing::WindowEvent; use geom::scale_factor::ScaleFactor; @@ -92,6 +92,10 @@ impl CompositorEventListener for NullCompositor { response_chan.send(()); } + FrameTreeUpdateMsg(_, response_channel) => { + response_channel.send(()); + } + // Explicitly list ignored messages so that when we add a new one, // we'll notice and think about whether it needs a response, like // SetIds. diff --git a/components/script/dom/htmliframeelement.rs b/components/script/dom/htmliframeelement.rs index 902ee4ee6ea..3312925e2ff 100644 --- a/components/script/dom/htmliframeelement.rs +++ b/components/script/dom/htmliframeelement.rs @@ -4,7 +4,6 @@ use dom::attr::Attr; use dom::attr::AttrHelpers; -use dom::bindings::codegen::Bindings::DocumentBinding::{DocumentMethods, DocumentReadyStateValues}; use dom::bindings::codegen::Bindings::HTMLIFrameElementBinding; use dom::bindings::codegen::Bindings::HTMLIFrameElementBinding::HTMLIFrameElementMethods; use dom::bindings::codegen::InheritTypes::{NodeCast, ElementCast}; @@ -16,7 +15,7 @@ use dom::element::{HTMLIFrameElementTypeId, Element}; use dom::element::AttributeHandlers; use dom::eventtarget::{EventTarget, NodeTargetTypeId}; use dom::htmlelement::HTMLElement; -use dom::node::{Node, NodeHelpers, ElementNodeTypeId, window_from_node, document_from_node}; +use dom::node::{Node, NodeHelpers, ElementNodeTypeId, window_from_node}; use dom::virtualmethods::VirtualMethods; use dom::window::Window; use page::IterablePage; @@ -120,13 +119,8 @@ impl<'a> HTMLIFrameElementHelpers for JSRef<'a, HTMLIFrameElement> { subpage_id: subpage_id, })); - let doc = document_from_node(self).root(); - if doc.ReadyState() == DocumentReadyStateValues::Loading { - // https://github.com/servo/servo/issues/3738 - // We can't handle dynamic frame tree changes in the compositor right now. - let ConstellationChan(ref chan) = page.constellation_chan; - chan.send(ScriptLoadedURLInIFrameMsg(url, page.id, subpage_id, sandboxed)); - } + let ConstellationChan(ref chan) = page.constellation_chan; + chan.send(ScriptLoadedURLInIFrameMsg(url, page.id, subpage_id, sandboxed)); } }