compositor: Handle synchronous messages while shutting down (#31733)

During the shutdown process, various threads (such as the
font cache thread) may be finishing up their work. If those threads make
synchronous requests to the compositor, answer them -- even if the
results will be unused. This is at least enough processing for them to
finish their work and exit cleanly.

This addresses crashes that are sometimes seen at exit, particuarly when
the font cache thread tries to register a font during shutdown.

In addition, this change also removes an unused compositor message.

components/compositing/compositor.rs

@@ -524,38 +524,38 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
     }
 
     fn handle_browser_message(&mut self, msg: CompositorMsg) -> bool {
-        if matches!(msg, CompositorMsg::NewWebRenderFrameReady(..)) {
+        match self.shutdown_state {
-            self.pending_frames -= 1;
+            ShutdownState::NotShuttingDown => {},
-        }
+            ShutdownState::ShuttingDown => {
-
+                return self.handle_browser_message_while_shutting_down(msg)
-        match (msg, self.shutdown_state) {
+            },
-            (_, ShutdownState::FinishedShuttingDown) => {
+            ShutdownState::FinishedShuttingDown => {
                 error!("compositor shouldn't be handling messages after shutting down");
                 return false;
             },
+        }
 
-            (CompositorMsg::ShutdownComplete, _) => {
+        match msg {
+            CompositorMsg::ShutdownComplete => {
+                warn!("Received `ShutdownComplete` while not shutting down.");
                 self.finish_shutting_down();
                 return false;
             },
 
-            (
+            CompositorMsg::ChangeRunningAnimationsState(pipeline_id, animation_state) => {
-                CompositorMsg::ChangeRunningAnimationsState(pipeline_id, animation_state),
-                ShutdownState::NotShuttingDown,
-            ) => {
                 self.change_running_animations_state(pipeline_id, animation_state);
             },
 
-            (CompositorMsg::SetFrameTree(frame_tree), ShutdownState::NotShuttingDown) => {
+            CompositorMsg::SetFrameTree(frame_tree) => {
                 self.set_frame_tree(&frame_tree);
                 self.send_scroll_positions_to_layout_for_pipeline(&frame_tree.pipeline.id);
             },
 
-            (CompositorMsg::TouchEventProcessed(result), ShutdownState::NotShuttingDown) => {
+            CompositorMsg::TouchEventProcessed(result) => {
                 self.touch_handler.on_event_processed(result);
             },
 
-            (CompositorMsg::CreatePng(page_rect, reply), ShutdownState::NotShuttingDown) => {
+            CompositorMsg::CreatePng(page_rect, reply) => {
                 let res = self.composite_specific_target(CompositeTarget::SharedMemory, page_rect);
                 if let Err(ref e) = res {
                     info!("Error retrieving PNG: {:?}", e);
@@ -566,7 +566,7 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
                 }
             },
 
-            (CompositorMsg::IsReadyToSaveImageReply(is_ready), ShutdownState::NotShuttingDown) => {
+            CompositorMsg::IsReadyToSaveImageReply(is_ready) => {
                 assert_eq!(
                     self.ready_to_save_state,
                     ReadyState::WaitingForConstellationReply
@@ -579,24 +579,20 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
                 self.composite_if_necessary(CompositingReason::Headless);
             },
 
-            (
+            CompositorMsg::PipelineVisibilityChanged(pipeline_id, visible) => {
-                CompositorMsg::PipelineVisibilityChanged(pipeline_id, visible),
-                ShutdownState::NotShuttingDown,
-            ) => {
                 self.pipeline_details(pipeline_id).visible = visible;
                 self.process_animations(true);
             },
 
-            (CompositorMsg::PipelineExited(pipeline_id, sender), _) => {
+            CompositorMsg::PipelineExited(pipeline_id, sender) => {
                 debug!("Compositor got pipeline exited: {:?}", pipeline_id);
                 self.remove_pipeline_root_layer(pipeline_id);
                 let _ = sender.send(());
             },
 
-            (
+            CompositorMsg::NewWebRenderFrameReady(recomposite_needed) => {
-                CompositorMsg::NewWebRenderFrameReady(recomposite_needed),
+                self.pending_frames -= 1;
-                ShutdownState::NotShuttingDown,
+
-            ) => {
                 if recomposite_needed {
                     if let Some(result) = self.hit_test_at_point(self.cursor_pos) {
                         self.update_cursor(result);
@@ -608,13 +604,7 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
                 }
             },
 
-            (CompositorMsg::Dispatch(func), ShutdownState::NotShuttingDown) => {
+            CompositorMsg::LoadComplete(_) => {
-                // The functions sent here right now are really dumb, so they can't panic.
-                // But if we start running more complex code here, we should really catch panic here.
-                func();
-            },
-
-            (CompositorMsg::LoadComplete(_), ShutdownState::NotShuttingDown) => {
                 // If we're painting in headless mode, schedule a recomposite.
                 if matches!(self.composite_target, CompositeTarget::PngFile(_)) ||
                     self.exit_after_load
@@ -623,10 +613,7 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
                 }
             },
 
-            (
+            CompositorMsg::WebDriverMouseButtonEvent(mouse_event_type, mouse_button, x, y) => {
-                CompositorMsg::WebDriverMouseButtonEvent(mouse_event_type, mouse_button, x, y),
-                ShutdownState::NotShuttingDown,
-            ) => {
                 let dppx = self.device_pixels_per_page_pixel();
                 let point = dppx.transform_point(Point2D::new(x, y));
                 self.on_mouse_window_event_class(match mouse_event_type {
@@ -636,29 +623,29 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
                 });
             },
 
-            (CompositorMsg::WebDriverMouseMoveEvent(x, y), ShutdownState::NotShuttingDown) => {
+            CompositorMsg::WebDriverMouseMoveEvent(x, y) => {
                 let dppx = self.device_pixels_per_page_pixel();
                 let point = dppx.transform_point(Point2D::new(x, y));
                 self.on_mouse_window_move_event_class(DevicePoint::new(point.x, point.y));
             },
 
-            (CompositorMsg::PendingPaintMetric(pipeline_id, epoch), _) => {
+            CompositorMsg::PendingPaintMetric(pipeline_id, epoch) => {
                 self.pending_paint_metrics.insert(pipeline_id, epoch);
             },
 
-            (CompositorMsg::GetClientWindow(req), ShutdownState::NotShuttingDown) => {
+            CompositorMsg::GetClientWindow(req) => {
                 if let Err(e) = req.send(self.embedder_coordinates.window) {
                     warn!("Sending response to get client window failed ({:?}).", e);
                 }
             },
 
-            (CompositorMsg::GetScreenSize(req), ShutdownState::NotShuttingDown) => {
+            CompositorMsg::GetScreenSize(req) => {
                 if let Err(e) = req.send(self.embedder_coordinates.screen) {
                     warn!("Sending response to get screen size failed ({:?}).", e);
                 }
             },
 
-            (CompositorMsg::GetScreenAvailSize(req), ShutdownState::NotShuttingDown) => {
+            CompositorMsg::GetScreenAvailSize(req) => {
                 if let Err(e) = req.send(self.embedder_coordinates.screen_avail) {
                     warn!(
                         "Sending response to get screen avail size failed ({:?}).",
@@ -667,14 +654,9 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
                 }
             },
 
-            (CompositorMsg::Forwarded(msg), ShutdownState::NotShuttingDown) => {
+            CompositorMsg::Forwarded(msg) => {
                 self.handle_webrender_message(msg);
             },
-
-            // When we are shutting_down, we need to avoid performing operations
-            // such as Paint that may crash because we have begun tearing down
-            // the rest of our resources.
-            (_, ShutdownState::ShuttingDown) => {},
         }
 
         true
@@ -917,6 +899,74 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
         }
     }
 
+    /// Handle messages sent to the compositor during the shutdown process. In general,
+    /// the things the compositor can do in this state are limited. It's very important to
+    /// answer any synchronous messages though as other threads might be waiting on the
+    /// results to finish their own shut down process. We try to do as little as possible
+    /// during this time.
+    ///
+    /// When that involves generating WebRender ids, our approach here is to simply
+    /// generate them, but assume they will never be used, since once shutting down the
+    /// compositor no longer does any WebRender frame generation.
+    fn handle_browser_message_while_shutting_down(&mut self, msg: CompositorMsg) -> bool {
+        match msg {
+            CompositorMsg::ShutdownComplete => {
+                self.finish_shutting_down();
+                return false;
+            },
+            CompositorMsg::PipelineExited(pipeline_id, sender) => {
+                debug!("Compositor got pipeline exited: {:?}", pipeline_id);
+                self.remove_pipeline_root_layer(pipeline_id);
+                let _ = sender.send(());
+            },
+            CompositorMsg::Forwarded(ForwardedToCompositorMsg::Font(
+                FontToCompositorMsg::AddFontInstance(_, _, sender),
+            )) => {
+                let _ = sender.send(self.webrender_api.generate_font_instance_key());
+            },
+            CompositorMsg::Forwarded(ForwardedToCompositorMsg::Font(
+                FontToCompositorMsg::AddFont(_, sender),
+            )) => {
+                let _ = sender.send(self.webrender_api.generate_font_key());
+            },
+            CompositorMsg::Forwarded(ForwardedToCompositorMsg::Canvas(
+                CanvasToCompositorMsg::GenerateKey(sender),
+            )) => {
+                let _ = sender.send(self.webrender_api.generate_image_key());
+            },
+            CompositorMsg::GetClientWindow(sender) => {
+                if let Err(e) = sender.send(self.embedder_coordinates.window) {
+                    warn!("Sending response to get client window failed ({:?}).", e);
+                }
+            },
+            CompositorMsg::GetScreenSize(sender) => {
+                if let Err(e) = sender.send(self.embedder_coordinates.screen) {
+                    warn!("Sending response to get screen size failed ({:?}).", e);
+                }
+            },
+            CompositorMsg::GetScreenAvailSize(sender) => {
+                if let Err(e) = sender.send(self.embedder_coordinates.screen_avail) {
+                    warn!(
+                        "Sending response to get screen avail size failed ({:?}).",
+                        e
+                    );
+                }
+            },
+            CompositorMsg::NewWebRenderFrameReady(_) => {
+                // Subtract from the number of pending frames, but do not do any compositing.
+                self.pending_frames -= 1;
+            },
+            CompositorMsg::PendingPaintMetric(pipeline_id, epoch) => {
+                self.pending_paint_metrics.insert(pipeline_id, epoch);
+            },
+
+            _ => {
+                debug!("Ignoring message ({:?} while shutting down", msg);
+            },
+        }
+        true
+    }
+
     /// Queue a new frame in the transaction and increase the pending frames count.
     fn generate_frame(&mut self, transaction: &mut Transaction, reason: RenderReasons) {
         self.pending_frames += 1;

components/shared/compositing/lib.rs

@@ -92,10 +92,6 @@ pub enum CompositorMsg {
     // sends a reply on the IpcSender, the constellation knows it's safe to
     // tear down the other threads associated with this pipeline.
     PipelineExited(PipelineId, IpcSender<()>),
-    /// Runs a closure in the compositor thread.
-    /// It's used to dispatch functions from webrender to the main thread's event loop.
-    /// Required to allow WGL GLContext sharing in Windows.
-    Dispatch(Box<dyn Fn() + Send>),
     /// Indicates to the compositor that it needs to record the time when the frame with
     /// the given ID (epoch) is painted and report it to the layout of the given
     /// pipeline ID.
@@ -164,7 +160,6 @@ impl Debug for CompositorMsg {
             CompositorMsg::PipelineVisibilityChanged(..) => write!(f, "PipelineVisibilityChanged"),
             CompositorMsg::PipelineExited(..) => write!(f, "PipelineExited"),
             CompositorMsg::NewWebRenderFrameReady(..) => write!(f, "NewWebRenderFrameReady"),
-            CompositorMsg::Dispatch(..) => write!(f, "Dispatch"),
             CompositorMsg::PendingPaintMetric(..) => write!(f, "PendingPaintMetric"),
             CompositorMsg::LoadComplete(..) => write!(f, "LoadComplete"),
             CompositorMsg::WebDriverMouseButtonEvent(..) => write!(f, "WebDriverMouseButtonEvent"),