Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restructure content process shutdown ack with threaded sender #23972

Merged
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

@@ -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};
@@ -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,
@@ -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) =
@@ -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,
@@ -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,
@@ -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>,
@@ -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();

This comment has been minimized.

Copy link
@asajeffrey

asajeffrey Aug 16, 2019

Member

Should this be unbounded or a one-shot channel?

This comment has been minimized.

Copy link
@gterzian

gterzian Aug 16, 2019

Author Member

I'm not aware of crossbeam having a dedicated one-shot variant.

This comment has been minimized.

Copy link
@asajeffrey

asajeffrey Aug 16, 2019

Member

I meant bounded(1).

This comment has been minimized.

Copy link
@gterzian

gterzian Aug 16, 2019

Author Member

for what reason?

This comment has been minimized.

Copy link
@asajeffrey

asajeffrey Aug 17, 2019

Member

Possible space efficiency, but it probably doesn't matter that much.

This comment has been minimized.

Copy link
@gterzian

gterzian Aug 17, 2019

Author Member

I see what you mean, counterintuitively, the unbounded variant appears to require less bookkeeping than the bounded variant.

let layout_thread_busy_flag = Arc::new(AtomicBool::new(false));
let layout_pair = STF::create(
InitialScriptState {
@@ -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,
@@ -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,
@@ -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"),
}
}
}

@@ -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,
@@ -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");
}
@@ -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,
@@ -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,
@@ -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");
}
@@ -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,
@@ -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,
@@ -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();
@@ -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);
@@ -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>,
@@ -2276,7 +2276,6 @@ impl ScriptThread {
load_data,
window_size,
pipeline_port,
content_process_shutdown_chan,
} = new_layout_info;

let layout_pair = unbounded();
@@ -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(),
@@ -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>,
}
@@ -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?
@@ -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.
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.