Skip to content

Commit

Permalink
Auto merge of #23972 - gterzian:cleanup_content_process_shutdown_ack,…
Browse files Browse the repository at this point in the history
… r=asajeffrey

Restructure content process shutdown ack with threaded sender

Recycling some work from #23909

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23972)
<!-- Reviewable:end -->
  • Loading branch information
bors-servo committed Aug 17, 2019
2 parents 8d51556 + 8482d2e commit 0ff5dc5
Show file tree
Hide file tree
Showing 9 changed files with 9 additions and 40 deletions.
27 changes: 7 additions & 20 deletions components/constellation/pipeline.rs
Expand Up @@ -9,7 +9,7 @@ use canvas_traits::webgl::WebGLPipeline;
use compositing::compositor_thread::Msg as CompositorMsg;
use compositing::CompositionPipeline;
use compositing::CompositorProxy;
use crossbeam_channel::Sender;
use crossbeam_channel::{unbounded, Sender};
use devtools_traits::{DevtoolsControlMsg, ScriptToDevtoolsControlMsg};
use embedder_traits::EventLoopWaker;
use euclid::{Scale, Size2D};
Expand Down Expand Up @@ -218,9 +218,6 @@ impl Pipeline {
// probably requires a general low-memory strategy.
let (pipeline_chan, pipeline_port) = ipc::channel().expect("Pipeline main chan");

let (layout_content_process_shutdown_chan, layout_content_process_shutdown_port) =
ipc::channel().expect("Pipeline layout content shutdown chan");

let window_size = WindowSizeData {
initial_viewport: state.window_size,
device_pixel_ratio: state.device_pixel_ratio,
Expand All @@ -237,7 +234,6 @@ impl Pipeline {
load_data: state.load_data.clone(),
window_size: window_size,
pipeline_port: pipeline_port,
content_process_shutdown_chan: Some(layout_content_process_shutdown_chan),
};

if let Err(e) =
Expand Down Expand Up @@ -275,9 +271,6 @@ impl Pipeline {
script_to_devtools_chan
});

let (script_content_process_shutdown_chan, script_content_process_shutdown_port) =
ipc::channel().expect("Pipeline script content process shutdown chan");

let mut unprivileged_pipeline_content = UnprivilegedPipelineContent {
id: state.id,
browsing_context_id: state.browsing_context_id,
Expand Down Expand Up @@ -306,10 +299,6 @@ impl Pipeline {
prefs: prefs::pref_map().iter().collect(),
pipeline_port: pipeline_port,
pipeline_namespace_id: state.pipeline_namespace_id,
layout_content_process_shutdown_chan: layout_content_process_shutdown_chan,
layout_content_process_shutdown_port: layout_content_process_shutdown_port,
script_content_process_shutdown_chan: script_content_process_shutdown_chan,
script_content_process_shutdown_port: script_content_process_shutdown_port,
webrender_api_sender: state.webrender_api_sender,
webrender_document: state.webrender_document,
webgl_chan: state.webgl_chan,
Expand Down Expand Up @@ -515,10 +504,6 @@ pub struct UnprivilegedPipelineContent {
prefs: HashMap<String, PrefValue>,
pipeline_port: IpcReceiver<LayoutControlMsg>,
pipeline_namespace_id: PipelineNamespaceId,
layout_content_process_shutdown_chan: IpcSender<()>,
layout_content_process_shutdown_port: IpcReceiver<()>,
script_content_process_shutdown_chan: IpcSender<()>,
script_content_process_shutdown_port: IpcReceiver<()>,
webrender_api_sender: webrender_api::RenderApiSender,
webrender_document: webrender_api::DocumentId,
webgl_chan: Option<WebGLPipeline>,
Expand All @@ -545,6 +530,7 @@ impl UnprivilegedPipelineContent {
self.script_chan.clone(),
self.load_data.url.clone(),
);
let (content_process_shutdown_chan, content_process_shutdown_port) = unbounded();
let layout_thread_busy_flag = Arc::new(AtomicBool::new(false));
let layout_pair = STF::create(
InitialScriptState {
Expand All @@ -567,7 +553,7 @@ impl UnprivilegedPipelineContent {
devtools_chan: self.devtools_chan,
window_size: self.window_size,
pipeline_namespace_id: self.pipeline_namespace_id,
content_process_shutdown_chan: self.script_content_process_shutdown_chan,
content_process_shutdown_chan: content_process_shutdown_chan,
webgl_chan: self.webgl_chan,
webvr_chan: self.webvr_chan,
webxr_registry: self.webxr_registry,
Expand Down Expand Up @@ -605,7 +591,6 @@ impl UnprivilegedPipelineContent {
self.font_cache_thread,
self.time_profiler_chan,
self.mem_profiler_chan,
Some(self.layout_content_process_shutdown_chan),
self.webrender_api_sender,
self.webrender_document,
paint_time_metrics,
Expand All @@ -624,8 +609,10 @@ impl UnprivilegedPipelineContent {
);

if wait_for_completion {
let _ = self.script_content_process_shutdown_port.recv();
let _ = self.layout_content_process_shutdown_port.recv();
match content_process_shutdown_port.recv() {
Ok(()) => {},
Err(_) => error!("Script-thread shut-down unexpectedly"),
}
}
}

Expand Down
5 changes: 0 additions & 5 deletions components/layout_thread/lib.rs
Expand Up @@ -306,7 +306,6 @@ impl LayoutThreadFactory for LayoutThread {
font_cache_thread: FontCacheThread,
time_profiler_chan: profile_time::ProfilerChan,
mem_profiler_chan: profile_mem::ProfilerChan,
content_process_shutdown_chan: Option<IpcSender<()>>,
webrender_api_sender: webrender_api::RenderApiSender,
webrender_document: webrender_api::DocumentId,
paint_time_metrics: PaintTimeMetrics,
Expand Down Expand Up @@ -383,9 +382,6 @@ impl LayoutThreadFactory for LayoutThread {
Msg::CollectReports,
);
}
if let Some(content_process_shutdown_chan) = content_process_shutdown_chan {
let _ = content_process_shutdown_chan.send(());
}
})
.expect("Thread spawning failed");
}
Expand Down Expand Up @@ -951,7 +947,6 @@ impl LayoutThread {
self.font_cache_thread.clone(),
self.time_profiler_chan.clone(),
self.mem_profiler_chan.clone(),
info.content_process_shutdown_chan,
self.webrender_api.clone_sender(),
self.webrender_document,
info.paint_time_metrics,
Expand Down
5 changes: 0 additions & 5 deletions components/layout_thread_2020/lib.rs
Expand Up @@ -243,7 +243,6 @@ impl LayoutThreadFactory for LayoutThread {
font_cache_thread: FontCacheThread,
time_profiler_chan: profile_time::ProfilerChan,
mem_profiler_chan: profile_mem::ProfilerChan,
content_process_shutdown_chan: Option<IpcSender<()>>,
webrender_api_sender: webrender_api::RenderApiSender,
webrender_document: webrender_api::DocumentId,
paint_time_metrics: PaintTimeMetrics,
Expand Down Expand Up @@ -312,9 +311,6 @@ impl LayoutThreadFactory for LayoutThread {
Msg::CollectReports,
);
}
if let Some(content_process_shutdown_chan) = content_process_shutdown_chan {
let _ = content_process_shutdown_chan.send(());
}
})
.expect("Thread spawning failed");
}
Expand Down Expand Up @@ -819,7 +815,6 @@ impl LayoutThread {
self.font_cache_thread.clone(),
self.time_profiler_chan.clone(),
self.mem_profiler_chan.clone(),
info.content_process_shutdown_chan,
self.webrender_api.clone_sender(),
self.webrender_document,
info.paint_time_metrics,
Expand Down
1 change: 0 additions & 1 deletion components/layout_traits/lib.rs
Expand Up @@ -43,7 +43,6 @@ pub trait LayoutThreadFactory {
font_cache_thread: FontCacheThread,
time_profiler_chan: time::ProfilerChan,
mem_profiler_chan: mem::ProfilerChan,
content_process_shutdown_chan: Option<IpcSender<()>>,
webrender_api_sender: webrender_api::RenderApiSender,
webrender_document: webrender_api::DocumentId,
paint_time_metrics: PaintTimeMetrics,
Expand Down
1 change: 0 additions & 1 deletion components/script/dom/htmliframeelement.rs
Expand Up @@ -198,7 +198,6 @@ impl HTMLIFrameElement {
opener: None,
load_data: load_data,
pipeline_port: pipeline_receiver,
content_process_shutdown_chan: None,
window_size: WindowSizeData {
initial_viewport: {
let rect = self.upcast::<Node>().bounding_content_box_or_zero();
Expand Down
1 change: 0 additions & 1 deletion components/script/dom/windowproxy.rs
Expand Up @@ -303,7 +303,6 @@ impl WindowProxy {
opener: Some(self.browsing_context_id),
load_data: load_data,
pipeline_port: pipeline_receiver,
content_process_shutdown_chan: None,
window_size: window.window_size(),
};
let constellation_msg = ScriptMsg::ScriptNewAuxiliary(load_info, pipeline_sender);
Expand Down
4 changes: 1 addition & 3 deletions components/script/script_thread.rs
Expand Up @@ -612,7 +612,7 @@ pub struct ScriptThread {
timer_event_chan: Sender<TimerEvent>,
timer_event_port: Receiver<TimerEvent>,

content_process_shutdown_chan: IpcSender<()>,
content_process_shutdown_chan: Sender<()>,

/// <https://html.spec.whatwg.org/multipage/#microtask-queue>
microtask_queue: Rc<MicrotaskQueue>,
Expand Down Expand Up @@ -2276,7 +2276,6 @@ impl ScriptThread {
load_data,
window_size,
pipeline_port,
content_process_shutdown_chan,
} = new_layout_info;

let layout_pair = unbounded();
Expand All @@ -2294,7 +2293,6 @@ impl ScriptThread {
constellation_chan: self.layout_to_constellation_chan.clone(),
script_chan: self.control_chan.clone(),
image_cache: self.image_cache.clone(),
content_process_shutdown_chan: content_process_shutdown_chan,
paint_time_metrics: PaintTimeMetrics::new(
new_pipeline_id,
self.time_profiler_chan.clone(),
Expand Down
1 change: 0 additions & 1 deletion components/script_layout_interface/message.rs
Expand Up @@ -225,7 +225,6 @@ pub struct LayoutThreadInit {
pub constellation_chan: IpcSender<ConstellationMsg>,
pub script_chan: IpcSender<ConstellationControlMsg>,
pub image_cache: Arc<dyn ImageCache>,
pub content_process_shutdown_chan: Option<IpcSender<()>>,
pub paint_time_metrics: PaintTimeMetrics,
pub layout_is_busy: Arc<AtomicBool>,
}
4 changes: 1 addition & 3 deletions components/script_traits/lib.rs
Expand Up @@ -219,8 +219,6 @@ pub struct NewLayoutInfo {
pub window_size: WindowSizeData,
/// A port on which layout can receive messages from the pipeline.
pub pipeline_port: IpcReceiver<LayoutControlMsg>,
/// A shutdown channel so that layout can tell the content process to shut down when it's done.
pub content_process_shutdown_chan: Option<IpcSender<()>>,
}

/// When a pipeline is closed, should its browsing context be discarded too?
Expand Down Expand Up @@ -651,7 +649,7 @@ pub struct InitialScriptState {
/// The ID of the pipeline namespace for this script thread.
pub pipeline_namespace_id: PipelineNamespaceId,
/// A ping will be sent on this channel once the script thread shuts down.
pub content_process_shutdown_chan: IpcSender<()>,
pub content_process_shutdown_chan: Sender<()>,
/// A channel to the WebGL thread used in this pipeline.
pub webgl_chan: Option<WebGLPipeline>,
/// A channel to the webvr thread, if available.
Expand Down

0 comments on commit 0ff5dc5

Please sign in to comment.