Skip to content

Commit

Permalink
Auto merge of #8958 - jkachmar:separate-layout-msg, r=KiChjang
Browse files Browse the repository at this point in the history
Separate script and layout messages, issue #8843

Separated layout-specific messages to the constellation out from the `ScriptMsg` enum into a `LayoutMsg` enum within `script_traits/script_msg.rs`, addresses [#8843](#8843).

I initially tried to move `LayoutMsg` into `layout_traits/lib.rs`, but this introduced a cyclic dependency: `layout_traits` depends on `script_traits` for the `LayoutTaskFactory` implementation, and `script_traits/script_task.rs` now depends on `LayoutMsg` for new layout channels in `InitialScriptState` and `ScriptTask`.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8958)
<!-- Reviewable:end -->
  • Loading branch information
bors-servo committed Dec 26, 2015
2 parents d2e7fd8 + 655268d commit 941653d
Show file tree
Hide file tree
Showing 11 changed files with 80 additions and 27 deletions.
42 changes: 34 additions & 8 deletions components/compositing/constellation.rs
Expand Up @@ -45,7 +45,7 @@ use profile_traits::mem;
use profile_traits::time;
use sandboxing;
use script_traits::{CompositorEvent, ConstellationControlMsg, LayoutControlMsg};
use script_traits::{ScriptMsg as FromScriptMsg, ScriptTaskFactory};
use script_traits::{LayoutMsg as FromLayoutMsg, ScriptMsg as FromScriptMsg, ScriptTaskFactory};
use script_traits::{TimerEventRequest};
use std::borrow::ToOwned;
use std::collections::HashMap;
Expand Down Expand Up @@ -87,6 +87,9 @@ pub struct Constellation<LTF, STF> {
/// A channel through which compositor messages can be sent to this object.
pub compositor_sender: Sender<FromCompositorMsg>,

/// A channel through which layout task messages can be sent to this object.
pub layout_sender: ConstellationChan<FromLayoutMsg>,

/// A channel through which paint task messages can be sent to this object.
pub painter_sender: ConstellationChan<FromPaintMsg>,

Expand All @@ -96,6 +99,9 @@ pub struct Constellation<LTF, STF> {
/// Receives messages from the compositor
pub compositor_receiver: Receiver<FromCompositorMsg>,

/// Receives messages from the layout task
pub layout_receiver: Receiver<FromLayoutMsg>,

/// Receives messages from paint task.
pub painter_receiver: Receiver<FromPaintMsg>,

Expand Down Expand Up @@ -287,16 +293,20 @@ impl<LTF: LayoutTaskFactory, STF: ScriptTaskFactory> Constellation<LTF, STF> {
let (ipc_script_receiver, ipc_script_sender) = ConstellationChan::<FromScriptMsg>::new();
let script_receiver = ROUTER.route_ipc_receiver_to_new_mpsc_receiver(ipc_script_receiver);
let (compositor_sender, compositor_receiver) = channel();
let (ipc_layout_receiver, ipc_layout_sender) = ConstellationChan::<FromLayoutMsg>::new();
let layout_receiver = ROUTER.route_ipc_receiver_to_new_mpsc_receiver(ipc_layout_receiver);
let (ipc_painter_receiver, ipc_painter_sender) = ConstellationChan::<FromPaintMsg>::new();
let painter_receiver = ROUTER.route_ipc_receiver_to_new_mpsc_receiver(ipc_painter_receiver);
let compositor_sender_clone = compositor_sender.clone();
spawn_named("Constellation".to_owned(), move || {
let mut constellation: Constellation<LTF, STF> = Constellation {
script_sender: ipc_script_sender,
compositor_sender: compositor_sender_clone,
layout_sender: ipc_layout_sender,
painter_sender: ipc_painter_sender,
script_receiver: script_receiver,
compositor_receiver: compositor_receiver,
layout_receiver: layout_receiver,
painter_receiver: painter_receiver,
compositor_proxy: state.compositor_proxy,
devtools_chan: state.devtools_chan,
Expand Down Expand Up @@ -371,6 +381,7 @@ impl<LTF: LayoutTaskFactory, STF: ScriptTaskFactory> Constellation<LTF, STF> {
id: pipeline_id,
parent_info: parent_info,
constellation_chan: self.script_sender.clone(),
layout_to_constellation_chan: self.layout_sender.clone(),
painter_chan: self.painter_sender.clone(),
scheduler_chan: self.scheduler_chan.clone(),
compositor_proxy: self.compositor_proxy.clone_compositor_proxy(),
Expand Down Expand Up @@ -474,18 +485,22 @@ impl<LTF: LayoutTaskFactory, STF: ScriptTaskFactory> Constellation<LTF, STF> {
enum Request {
Script(FromScriptMsg),
Compositor(FromCompositorMsg),
Layout(FromLayoutMsg),
Paint(FromPaintMsg)
}

let request = {
let receiver_from_script = &self.script_receiver;
let receiver_from_compositor = &self.compositor_receiver;
let receiver_from_layout = &self.layout_receiver;
let receiver_from_paint = &self.painter_receiver;
select! {
msg = receiver_from_script.recv() =>
Request::Script(msg.unwrap()),
msg = receiver_from_compositor.recv() =>
Request::Compositor(msg.unwrap()),
msg = receiver_from_layout.recv() =>
Request::Layout(msg.unwrap()),
msg = receiver_from_paint.recv() =>
Request::Paint(msg.unwrap())
}
Expand Down Expand Up @@ -577,9 +592,6 @@ impl<LTF: LayoutTaskFactory, STF: ScriptTaskFactory> Constellation<LTF, STF> {
load_info.new_subpage_id);
self.handle_script_loaded_url_in_iframe_msg(load_info);
}
Request::Script(FromScriptMsg::SetCursor(cursor)) => {
self.handle_set_cursor_msg(cursor)
}
Request::Script(FromScriptMsg::ChangeRunningAnimationsState(pipeline_id, animation_state)) => {
self.handle_change_running_animations_state(pipeline_id, animation_state)
}
Expand Down Expand Up @@ -652,10 +664,6 @@ impl<LTF: LayoutTaskFactory, STF: ScriptTaskFactory> Constellation<LTF, STF> {
}
}
}
Request::Script(FromScriptMsg::ViewportConstrained(pipeline_id, constraints)) => {
debug!("constellation got viewport-constrained event message");
self.handle_viewport_constrained_msg(pipeline_id, constraints);
}
Request::Script(FromScriptMsg::RemoveIFrame(pipeline_id)) => {
debug!("constellation got remove iframe message");
self.handle_remove_iframe_msg(pipeline_id);
Expand Down Expand Up @@ -686,6 +694,24 @@ impl<LTF: LayoutTaskFactory, STF: ScriptTaskFactory> Constellation<LTF, STF> {
}


// Messages from layout task

Request::Layout(FromLayoutMsg::ChangeRunningAnimationsState(pipeline_id, animation_state)) => {
self.handle_change_running_animations_state(pipeline_id, animation_state)
}
Request::Layout(FromLayoutMsg::Failure(Failure { pipeline_id, parent_info })) => {
debug!("handling paint failure message from pipeline {:?}, {:?}", pipeline_id, parent_info);
self.handle_failure_msg(pipeline_id, parent_info);
}
Request::Layout(FromLayoutMsg::SetCursor(cursor)) => {
self.handle_set_cursor_msg(cursor)
}
Request::Layout(FromLayoutMsg::ViewportConstrained(pipeline_id, constraints)) => {
debug!("constellation got viewport-constrained event message");
self.handle_viewport_constrained_msg(pipeline_id, constraints);
}


// Messages from paint task


Expand Down
13 changes: 9 additions & 4 deletions components/compositing/pipeline.rs
Expand Up @@ -24,7 +24,7 @@ use net_traits::storage_task::StorageTask;
use profile_traits::mem as profile_mem;
use profile_traits::time;
use script_traits::{ConstellationControlMsg, InitialScriptState};
use script_traits::{LayoutControlMsg, NewLayoutInfo, ScriptMsg as ConstellationMsg};
use script_traits::{LayoutControlMsg, LayoutMsg, NewLayoutInfo, ScriptMsg};
use script_traits::{ScriptToCompositorMsg, ScriptTaskFactory, TimerEventRequest};
use std::mem;
use std::sync::mpsc::{Receiver, Sender, channel};
Expand Down Expand Up @@ -79,7 +79,9 @@ pub struct InitialPipelineState {
/// If `None`, this is the root.
pub parent_info: Option<(PipelineId, SubpageId)>,
/// A channel to the associated constellation.
pub constellation_chan: ConstellationChan<ConstellationMsg>,
pub constellation_chan: ConstellationChan<ScriptMsg>,
/// A channel for the layout task to send messages to the constellation.
pub layout_to_constellation_chan: ConstellationChan<LayoutMsg>,
/// A channel to the associated paint task.
pub painter_chan: ConstellationChan<PaintMsg>,
/// A channel to schedule timer events.
Expand Down Expand Up @@ -207,6 +209,7 @@ impl Pipeline {
time_profiler_chan: state.time_profiler_chan.clone(),
mem_profiler_chan: state.mem_profiler_chan.clone(),
window_size: window_size,
layout_to_constellation_chan: state.layout_to_constellation_chan,
script_chan: script_chan,
load_data: state.load_data.clone(),
failure: failure,
Expand Down Expand Up @@ -347,7 +350,8 @@ impl Pipeline {
pub struct UnprivilegedPipelineContent {
id: PipelineId,
parent_info: Option<(PipelineId, SubpageId)>,
constellation_chan: ConstellationChan<ConstellationMsg>,
constellation_chan: ConstellationChan<ScriptMsg>,
layout_to_constellation_chan: ConstellationChan<LayoutMsg>,
scheduler_chan: IpcSender<TimerEventRequest>,
devtools_chan: Option<IpcSender<ScriptToDevtoolsControlMsg>>,
script_to_compositor_chan: IpcSender<ScriptToCompositorMsg>,
Expand Down Expand Up @@ -386,6 +390,7 @@ impl UnprivilegedPipelineContent {
control_chan: self.script_chan.clone(),
control_port: mem::replace(&mut self.script_port, None).unwrap(),
constellation_chan: self.constellation_chan.clone(),
layout_to_constellation_chan: self.layout_to_constellation_chan.clone(),
scheduler_chan: self.scheduler_chan.clone(),
failure_info: self.failure.clone(),
resource_task: self.resource_task,
Expand All @@ -405,7 +410,7 @@ impl UnprivilegedPipelineContent {
self.parent_info.is_some(),
layout_pair,
self.pipeline_port.unwrap(),
self.constellation_chan,
self.layout_to_constellation_chan,
self.failure,
self.script_chan.clone(),
self.layout_to_paint_chan.clone(),
Expand Down
2 changes: 1 addition & 1 deletion components/layout/animation.rs
Expand Up @@ -9,7 +9,7 @@ use gfx::display_list::OpaqueNode;
use incremental::{self, RestyleDamage};
use msg::constellation_msg::{AnimationState, ConstellationChan, PipelineId};
use script::layout_interface::Animation;
use script_traits::ScriptMsg as ConstellationMsg;
use script_traits::LayoutMsg as ConstellationMsg;
use std::collections::HashMap;
use std::collections::hash_map::Entry;
use std::sync::mpsc::{Sender, Receiver};
Expand Down
4 changes: 2 additions & 2 deletions components/layout/layout_task.rs
Expand Up @@ -51,8 +51,8 @@ use script::layout_interface::{LayoutRPC, OffsetParentResponse};
use script::layout_interface::{Msg, NewLayoutTaskInfo, Reflow, ReflowGoal, ReflowQueryType};
use script::layout_interface::{ScriptLayoutChan, ScriptReflow};
use script::reporter::CSSErrorReporter;
use script_traits::ScriptMsg as ConstellationMsg;
use script_traits::{ConstellationControlMsg, LayoutControlMsg, OpaqueScriptLayoutChannel};
use script_traits::ConstellationControlMsg;
use script_traits::{LayoutControlMsg, LayoutMsg as ConstellationMsg, OpaqueScriptLayoutChannel};
use sequential;
use serde_json;
use std::borrow::ToOwned;
Expand Down
2 changes: 1 addition & 1 deletion components/layout/query.rs
Expand Up @@ -18,7 +18,7 @@ use opaque_node::OpaqueNodeMethods;
use script::layout_interface::{ContentBoxResponse, ContentBoxesResponse, NodeGeometryResponse};
use script::layout_interface::{HitTestResponse, LayoutRPC, MouseOverResponse, OffsetParentResponse};
use script::layout_interface::{ResolvedStyleResponse, ScriptLayoutChan};
use script_traits::ScriptMsg as ConstellationMsg;
use script_traits::LayoutMsg as ConstellationMsg;
use selectors::parser::PseudoElement;
use sequential;
use std::ops::Deref;
Expand Down
2 changes: 1 addition & 1 deletion components/layout_traits/lib.rs
Expand Up @@ -27,7 +27,7 @@ use ipc_channel::ipc::{IpcReceiver, IpcSender};
use msg::constellation_msg::{ConstellationChan, Failure, PipelineId};
use net_traits::image_cache_task::ImageCacheTask;
use profile_traits::{mem, time};
use script_traits::ScriptMsg as ConstellationMsg;
use script_traits::LayoutMsg as ConstellationMsg;
use script_traits::{LayoutControlMsg, ConstellationControlMsg, OpaqueScriptLayoutChannel};
use url::Url;
use util::ipc::OptionalIpcSender;
Expand Down
9 changes: 8 additions & 1 deletion components/script/dom/bindings/trace.rs
Expand Up @@ -63,7 +63,7 @@ use net_traits::storage_task::StorageType;
use profile_traits::mem::ProfilerChan as MemProfilerChan;
use profile_traits::time::ProfilerChan as TimeProfilerChan;
use script_task::ScriptChan;
use script_traits::{ScriptMsg, TimerEventId, TimerSource, UntrustedNodeAddress};
use script_traits::{LayoutMsg, ScriptMsg, TimerEventId, TimerSource, UntrustedNodeAddress};
use selectors::parser::PseudoElement;
use selectors::states::*;
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -306,6 +306,13 @@ impl JSTraceable for ConstellationChan<ScriptMsg> {
}
}

impl JSTraceable for ConstellationChan<LayoutMsg> {
#[inline]
fn trace(&self, _trc: *mut JSTracer) {
// Do nothing
}
}

impl JSTraceable for Box<ScriptChan + Send> {
#[inline]
fn trace(&self, _trc: *mut JSTracer) {
Expand Down
4 changes: 2 additions & 2 deletions components/script/layout_interface.rs
Expand Up @@ -18,8 +18,8 @@ use msg::constellation_msg::{ConstellationChan, Failure, PipelineId};
use msg::constellation_msg::{WindowSizeData};
use net_traits::image_cache_task::ImageCacheTask;
use profile_traits::mem::ReportsChan;
use script_traits::{ConstellationControlMsg, LayoutControlMsg, OpaqueScriptLayoutChannel};
use script_traits::{ScriptMsg as ConstellationMsg, UntrustedNodeAddress};
use script_traits::{ConstellationControlMsg, LayoutControlMsg, LayoutMsg as ConstellationMsg};
use script_traits::{OpaqueScriptLayoutChannel, UntrustedNodeAddress};
use selectors::parser::PseudoElement;
use std::any::Any;
use std::sync::Arc;
Expand Down
8 changes: 6 additions & 2 deletions components/script/script_task.rs
Expand Up @@ -80,7 +80,7 @@ use profile_traits::time::{self, ProfilerCategory, profile};
use script_traits::CompositorEvent::{KeyEvent, MouseButtonEvent, MouseMoveEvent, ResizeEvent};
use script_traits::CompositorEvent::{TouchEvent};
use script_traits::{CompositorEvent, ConstellationControlMsg, EventResult, InitialScriptState, NewLayoutInfo};
use script_traits::{OpaqueScriptLayoutChannel, ScriptMsg as ConstellationMsg};
use script_traits::{LayoutMsg, OpaqueScriptLayoutChannel, ScriptMsg as ConstellationMsg};
use script_traits::{ScriptTaskFactory, ScriptToCompositorMsg, TimerEvent, TimerEventRequest, TimerSource};
use script_traits::{TouchEventType, TouchId};
use std::any::Any;
Expand Down Expand Up @@ -406,6 +406,9 @@ pub struct ScriptTask {
/// For communicating load url messages to the constellation
constellation_chan: ConstellationChan<ConstellationMsg>,

/// For communicating layout messages to the constellation
layout_to_constellation_chan: ConstellationChan<LayoutMsg>,

/// A handle to the compositor for communicating ready state messages.
compositor: DOMRefCell<IpcSender<ScriptToCompositorMsg>>,

Expand Down Expand Up @@ -669,6 +672,7 @@ impl ScriptTask {
control_chan: state.control_chan,
control_port: control_port,
constellation_chan: state.constellation_chan,
layout_to_constellation_chan: state.layout_to_constellation_chan,
compositor: DOMRefCell::new(state.compositor),
time_profiler_chan: state.time_profiler_chan,
mem_profiler_chan: state.mem_profiler_chan,
Expand Down Expand Up @@ -1195,7 +1199,7 @@ impl ScriptTask {
is_parent: false,
layout_pair: layout_pair,
pipeline_port: pipeline_port,
constellation_chan: self.constellation_chan.clone(),
constellation_chan: self.layout_to_constellation_chan.clone(),
failure: failure,
paint_chan: paint_chan,
script_chan: self.control_chan.clone(),
Expand Down
4 changes: 3 additions & 1 deletion components/script_traits/lib.rs
Expand Up @@ -52,7 +52,7 @@ use std::any::Any;
use util::ipc::OptionalOpaqueIpcSender;
use util::mem::HeapSizeOf;

pub use script_msg::ScriptMsg;
pub use script_msg::{LayoutMsg, ScriptMsg};

/// The address of a node. Layout sends these back. They must be validated via
/// `from_untrusted_node_address` before they can be used, because we do not trust layout.
Expand Down Expand Up @@ -250,6 +250,8 @@ pub struct InitialScriptState {
pub control_port: IpcReceiver<ConstellationControlMsg>,
/// A channel on which messages can be sent to the constellation from script.
pub constellation_chan: ConstellationChan<ScriptMsg>,
/// A channel for the layout task to send messages to the constellation.
pub layout_to_constellation_chan: ConstellationChan<LayoutMsg>,
/// A channel to schedule timer events.
pub scheduler_chan: IpcSender<TimerEventRequest>,
/// Information that script sends out when it panics.
Expand Down
17 changes: 13 additions & 4 deletions components/script_traits/script_msg.rs
Expand Up @@ -15,6 +15,19 @@ use style_traits::viewport::ViewportConstraints;
use url::Url;
use util::cursor::Cursor;

/// Messages from the layout to the constellation.
#[derive(Deserialize, Serialize)]
pub enum LayoutMsg {
/// Indicates whether this pipeline is currently running animations.
ChangeRunningAnimationsState(PipelineId, AnimationState),
/// Layout task failure.
Failure(Failure),
/// Requests that the constellation inform the compositor of the a cursor change.
SetCursor(Cursor),
/// Notifies the constellation that the viewport has been constrained in some manner
ViewportConstrained(PipelineId, ViewportConstraints),
}

/// Messages from the script to the constellation.
#[derive(Deserialize, Serialize)]
pub enum ScriptMsg {
Expand Down Expand Up @@ -62,10 +75,6 @@ pub enum ScriptMsg {
ScriptLoadedURLInIFrame(IframeLoadInfo),
/// Requests that the constellation set the contents of the clipboard
SetClipboardContents(String),
/// Requests that the constellation inform the compositor of the a cursor change.
SetCursor(Cursor),
/// Notifies the constellation that the viewport has been constrained in some manner
ViewportConstrained(PipelineId, ViewportConstraints),
/// Mark a new document as active
ActivateDocument(PipelineId),
/// Set the document state for a pipeline (used by screenshot / reftests)
Expand Down

0 comments on commit 941653d

Please sign in to comment.