From 2bdc604c667f539b18900a17afb0ad24a8240db3 Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Wed, 7 Oct 2015 21:28:39 -0600 Subject: [PATCH] servo: Merge #7913 - Simplify and unify compositor shutdown code paths (from mrobinson:compositor-shutdown); r=pcwalton Unify all compositor shutdown code paths into two methods, one which starts the shutdown and the other that finishes it. This simplifies the way the compositor shuts down and prevents "leaking" pixmaps when exiting in uncommon ways. Source-Repo: https://github.com/servo/servo Source-Revision: 26dd1233103eb75c2e94fcc2ba34c18fa4432afc --- servo/components/compositing/compositor.rs | 84 +++++++++---------- .../components/compositing/compositor_task.rs | 1 - servo/components/compositing/headless.rs | 17 ++-- servo/components/servo/lib.rs | 4 - servo/components/servo/main.rs | 5 -- servo/ports/gonk/src/main.rs | 5 -- 6 files changed, 47 insertions(+), 69 deletions(-) diff --git a/servo/components/compositing/compositor.rs b/servo/components/compositing/compositor.rs index dcacb440e667..919f68b0bde8 100644 --- a/servo/components/compositing/compositor.rs +++ b/servo/components/compositing/compositor.rs @@ -155,9 +155,6 @@ pub struct IOCompositor { /// Pending scroll events. pending_scroll_events: Vec, - /// Has a Quit event been seen? - has_seen_quit_event: bool, - /// Used by the logic that determines when it is safe to output an /// image for the reftest framework. ready_to_save_state: ReadyState, @@ -309,7 +306,6 @@ impl IOCompositor { mem_profiler_chan: state.mem_profiler_chan, fragment_point: None, last_composite_time: 0, - has_seen_quit_event: false, ready_to_save_state: ReadyState::Unknown, surface_map: SurfaceMap::new(BUFFER_MAP_SIZE), pending_subpages: HashSet::new(), @@ -329,25 +325,48 @@ impl IOCompositor { compositor } + pub fn start_shutting_down(&mut self) { + debug!("Compositor sending Exit message to Constellation"); + let ConstellationChan(ref constellation_channel) = self.constellation_chan; + constellation_channel.send(ConstellationMsg::Exit).unwrap(); + + self.mem_profiler_chan.send(mem::ProfilerMsg::UnregisterReporter(reporter_name())); + + self.shutdown_state = ShutdownState::ShuttingDown; + } + + pub fn finish_shutting_down(&mut self) { + debug!("Compositor received message that constellation shutdown is complete"); + + // Clear out the compositor layers so that painting tasks can destroy the buffers. + if let Some(ref root_layer) = self.scene.root { + root_layer.forget_all_tiles(); + } + + // Drain compositor port, sometimes messages contain channels that are blocking + // another task from finishing (i.e. SetFrameTree). + while self.port.try_recv_compositor_msg().is_some() {} + + // Tell the profiler, memory profiler, and scrolling timer to shut down. + self.time_profiler_chan.send(time::ProfilerMsg::Exit); + self.mem_profiler_chan.send(mem::ProfilerMsg::Exit); + self.scrolling_timer.shutdown(); + + self.shutdown_state = ShutdownState::FinishedShuttingDown; + } + fn handle_browser_message(&mut self, msg: Msg) -> bool { match (msg, self.shutdown_state) { (_, ShutdownState::FinishedShuttingDown) => panic!("compositor shouldn't be handling messages after shutting down"), - (Msg::Exit(chan), _) => { - debug!("shutting down the constellation"); - let ConstellationChan(ref con_chan) = self.constellation_chan; - con_chan.send(ConstellationMsg::Exit).unwrap(); - chan.send(()).unwrap(); - - self.mem_profiler_chan.send(mem::ProfilerMsg::UnregisterReporter(reporter_name())); - - self.shutdown_state = ShutdownState::ShuttingDown; + (Msg::Exit(channel), _) => { + self.start_shutting_down(); + channel.send(()).unwrap(); } (Msg::ShutdownComplete, _) => { - debug!("constellation completed shutdown"); - self.shutdown_state = ShutdownState::FinishedShuttingDown; + self.finish_shutting_down(); return false; } @@ -1010,12 +1029,9 @@ impl IOCompositor { } WindowEvent::Quit => { - if !self.has_seen_quit_event { - self.has_seen_quit_event = true; - debug!("shutting down the constellation for WindowEvent::Quit"); - let ConstellationChan(ref chan) = self.constellation_chan; - chan.send(ConstellationMsg::Exit).unwrap(); - self.shutdown_state = ShutdownState::ShuttingDown; + if self.shutdown_state == ShutdownState::NotShuttingDown { + debug!("Shutting down the constellation for WindowEvent::Quit"); + self.start_shutting_down(); } } } @@ -1495,10 +1511,8 @@ impl IOCompositor { let composited = self.composite_specific_target(target); if composited.is_ok() && (opts::get().output_file.is_some() || opts::get().exit_after_load) { - debug!("shutting down the constellation (after generating an output file or exit flag specified)"); - let ConstellationChan(ref chan) = self.constellation_chan; - chan.send(ConstellationMsg::Exit).unwrap(); - self.shutdown_state = ShutdownState::ShuttingDown; + debug!("Shutting down the Constellation after generating an output file or exit flag specified"); + self.start_shutting_down(); } } @@ -1807,9 +1821,6 @@ impl CompositorEventListener for IOCompositor where Window: Wind } if self.shutdown_state == ShutdownState::FinishedShuttingDown { - // We have exited the compositor and passing window - // messages to script may crash. - debug!("Exiting the compositor due to a request from script."); return false; } @@ -1858,23 +1869,6 @@ impl CompositorEventListener for IOCompositor where Window: Wind } } - fn shutdown(&mut self) { - // Clear out the compositor layers so that painting tasks can destroy the buffers. - match self.scene.root { - None => {} - Some(ref layer) => layer.forget_all_tiles(), - } - - // Drain compositor port, sometimes messages contain channels that are blocking - // another task from finishing (i.e. SetFrameTree). - while self.port.try_recv_compositor_msg().is_some() {} - - // Tell the profiler, memory profiler, and scrolling timer to shut down. - self.time_profiler_chan.send(time::ProfilerMsg::Exit); - self.mem_profiler_chan.send(mem::ProfilerMsg::Exit); - self.scrolling_timer.shutdown(); - } - fn pinch_zoom_level(&self) -> f32 { self.viewport_zoom.get() as f32 } diff --git a/servo/components/compositing/compositor_task.rs b/servo/components/compositing/compositor_task.rs index 2b3b572a0118..b4388ed51323 100644 --- a/servo/components/compositing/compositor_task.rs +++ b/servo/components/compositing/compositor_task.rs @@ -280,7 +280,6 @@ impl CompositorTask { pub trait CompositorEventListener { fn handle_events(&mut self, events: Vec) -> bool; fn repaint_synchronously(&mut self); - fn shutdown(&mut self); fn pinch_zoom_level(&self) -> f32; /// Requests that the compositor send the title for the main frame as soon as possible. fn title_for_main_frame(&self); diff --git a/servo/components/compositing/headless.rs b/servo/components/compositing/headless.rs index 41bcc3811d2e..d322ef774458 100644 --- a/servo/components/compositing/headless.rs +++ b/servo/components/compositing/headless.rs @@ -69,6 +69,14 @@ impl CompositorEventListener for NullCompositor { Msg::ShutdownComplete => { debug!("constellation completed shutdown"); + + // Drain compositor port, sometimes messages contain channels that are blocking + // another task from finishing (i.e. SetIds) + while self.port.try_recv_compositor_msg().is_some() {} + + self.time_profiler_chan.send(time::ProfilerMsg::Exit); + self.mem_profiler_chan.send(mem::ProfilerMsg::Exit); + return false } @@ -130,15 +138,6 @@ impl CompositorEventListener for NullCompositor { fn repaint_synchronously(&mut self) {} - fn shutdown(&mut self) { - // Drain compositor port, sometimes messages contain channels that are blocking - // another task from finishing (i.e. SetIds) - while self.port.try_recv_compositor_msg().is_some() {} - - self.time_profiler_chan.send(time::ProfilerMsg::Exit); - self.mem_profiler_chan.send(mem::ProfilerMsg::Exit); - } - fn pinch_zoom_level(&self) -> f32 { 1.0 } diff --git a/servo/components/servo/lib.rs b/servo/components/servo/lib.rs index eb144007dab1..359fc6a82ec6 100644 --- a/servo/components/servo/lib.rs +++ b/servo/components/servo/lib.rs @@ -186,10 +186,6 @@ impl Browser { pub fn request_title_for_main_frame(&self) { self.compositor.title_for_main_frame() } - - pub fn shutdown(mut self) { - self.compositor.shutdown(); - } } fn create_constellation(opts: opts::Opts, diff --git a/servo/components/servo/main.rs b/servo/components/servo/main.rs index 8112b7fa4cc4..3e81da46f1f5 100644 --- a/servo/components/servo/main.rs +++ b/servo/components/servo/main.rs @@ -75,11 +75,6 @@ fn main() { }; maybe_unregister_glutin_resize_handler(&window); - - let BrowserWrapper { - browser - } = browser; - browser.shutdown(); } fn maybe_register_glutin_resize_handler(window: &Option>, diff --git a/servo/ports/gonk/src/main.rs b/servo/ports/gonk/src/main.rs index 777cf999f6f6..80f5a4624e53 100644 --- a/servo/ports/gonk/src/main.rs +++ b/servo/ports/gonk/src/main.rs @@ -98,9 +98,4 @@ fn main() { break } } - - let BrowserWrapper { - browser - } = browser; - browser.shutdown(); }