servo: Merge #10295 - Miscellaneous fixes to harden the constellation (from asajeffrey:remove-constellation-misc-panic); r=nox
authorAlan Jeffrey <ajeffrey@mozilla.com>
Fri, 01 Apr 2016 18:45:18 +0500
changeset 338378 8e11990e3ad1ccd414d3d74dc2e2ef6dc00b86f9
parent 338377 fd9cb158d8f0a27f40fd621eeb102a4c2f84fc17
child 338379 5f53e20c49e41e84742a1172754c79edd7279db4
push id31307
push usergszorc@mozilla.com
push dateSat, 04 Feb 2017 00:59:06 +0000
treeherdermozilla-central@94079d43835f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnox
servo: Merge #10295 - Miscellaneous fixes to harden the constellation (from asajeffrey:remove-constellation-misc-panic); r=nox Source-Repo: https://github.com/servo/servo Source-Revision: 7f06b467a4ed988154bae3bb58c235b969e750c8
servo/components/compositing/constellation.rs
--- a/servo/components/compositing/constellation.rs
+++ b/servo/components/compositing/constellation.rs
@@ -452,16 +452,19 @@ impl<LTF: LayoutThreadFactory, STF: Scri
         self.pipelines.insert(pipeline_id, pipeline);
     }
 
     #[cfg(not(target_os = "windows"))]
     fn spawn_multiprocess(&mut self,
                           pipeline_id: PipelineId,
                           unprivileged_pipeline_content: UnprivilegedPipelineContent)
     {
+        // Note that this function can panic, due to process creation,
+        // avoiding this panic would require a mechanism for dealing
+        // with low-resource scenarios.
         let (server, token) =
             IpcOneShotServer::<IpcSender<UnprivilegedPipelineContent>>::new().unwrap();
 
         // If there is a sandbox, use the `gaol` API to create the child process.
         let child_process = if opts::get().sandbox {
             let mut command = sandbox::Command::me().unwrap();
             command.arg("--content-process").arg(token);
             let profile = sandboxing::content_process_sandbox_profile();
@@ -719,17 +722,17 @@ impl<LTF: LayoutThreadFactory, STF: Scri
                         debug!("Error setting clipboard contents ({})", e);
                     }
                 }
             }
             Request::Script(FromScriptMsg::RemoveIFrame(pipeline_id, sender)) => {
                 debug!("constellation got remove iframe message");
                 self.handle_remove_iframe_msg(pipeline_id);
                 if let Some(sender) = sender {
-                    sender.send(()).unwrap();
+                    sender.send(()).unwrap_or_else(|e| debug!("Error replying to remove iframe ({})", e));
                 }
             }
             Request::Script(FromScriptMsg::NewFavicon(url)) => {
                 debug!("constellation got new favicon message");
                 self.compositor_proxy.send(ToCompositorMsg::NewFavicon(url));
             }
             Request::Script(FromScriptMsg::HeadParsed) => {
                 debug!("constellation got head parsed message");
@@ -1561,16 +1564,20 @@ impl<LTF: LayoutThreadFactory, STF: Scri
 
     /// Checks the state of all script and layout pipelines to see if they are idle
     /// and compares the current layout state to what the compositor has. This is used
     /// to check if the output image is "stable" and can be written as a screenshot
     /// for reftests.
     /// Since this function is only used in reftests, we do not harden it against panic.
     fn handle_is_ready_to_save_image(&mut self,
                                      pipeline_states: HashMap<PipelineId, Epoch>) -> ReadyToSave {
+        // Note that this function can panic, due to ipc-channel creation failure.
+        // avoiding this panic would require a mechanism for dealing
+        // with low-resource scenarios.
+        //
         // If there is no root frame yet, the initial page has
         // not loaded, so there is nothing to save yet.
         if self.root_frame_id.is_none() {
             return ReadyToSave::NoRootFrame;
         }
 
         // If there are pending loads, wait for those to complete.
         if self.pending_frames.len() > 0 {
@@ -1596,18 +1603,19 @@ impl<LTF: LayoutThreadFactory, STF: Scri
             // If GetWebFontLoadState returns false, either there are no
             // webfonts loading, or there's a WebFontLoaded message waiting in
             // script_chan's message queue. Therefore, we need to check this
             // before we check whether the document is ready; otherwise,
             // there's a race condition where a webfont has finished loading,
             // but hasn't yet notified the document.
             let (sender, receiver) = ipc::channel().unwrap();
             let msg = LayoutControlMsg::GetWebFontLoadState(sender);
-            pipeline.layout_chan.0.send(msg).unwrap();
-            if receiver.recv().unwrap() {
+            pipeline.layout_chan.0.send(msg)
+                .unwrap_or_else(|e| debug!("Get web font failed ({})", e));
+            if receiver.recv().unwrap_or(true) {
                 return ReadyToSave::WebFontNotLoaded;
             }
 
             // See if this pipeline has reached idle script state yet.
             match self.document_states.get(&frame.current) {
                 Some(&DocumentState::Idle) => {}
                 Some(&DocumentState::Pending) | None => {
                     return ReadyToSave::DocumentLoading;
@@ -1676,17 +1684,19 @@ impl<LTF: LayoutThreadFactory, STF: Scri
 
             pipelines_to_close
         };
 
         for pipeline_id in &pipelines_to_close {
             self.close_pipeline(*pipeline_id, exit_mode);
         }
 
-        self.frames.remove(&frame_id).unwrap();
+        if let None = self.frames.remove(&frame_id) {
+            debug!("Closing frame {:?} twice.", frame_id);
+        }
 
         if let Some((parent_pipeline_id, _)) = parent_info {
             let parent_pipeline = match self.pipelines.get_mut(&parent_pipeline_id) {
                 None => return debug!("Pipeline {:?} child closed after parent.", parent_pipeline_id),
                 Some(parent_pipeline) => parent_pipeline,
             };
             parent_pipeline.remove_child(frame_id);
         }
@@ -1708,17 +1718,20 @@ impl<LTF: LayoutThreadFactory, STF: Scri
             frames_to_close
         };
 
         // Remove any child frames
         for child_frame in &frames_to_close {
             self.close_frame(*child_frame, exit_mode);
         }
 
-        let pipeline = self.pipelines.remove(&pipeline_id).unwrap();
+        let pipeline = match self.pipelines.remove(&pipeline_id) {
+            Some(pipeline) => pipeline,
+            None => return debug!("Closing pipeline {:?} twice.", pipeline_id),
+        };
 
         // If a child pipeline, remove from subpage map
         if let Some(info) = pipeline.parent_info {
             self.subpage_map.remove(&info);
         }
 
         // Remove assocation between this pipeline and its holding frame
         self.pipeline_to_frame_map.remove(&pipeline_id);
@@ -1786,16 +1799,19 @@ impl<LTF: LayoutThreadFactory, STF: Scri
         for frame in self.current_frame_tree_iter(frame_id) {
             self.pipelines.get(&frame.current).map(|pipeline| pipeline.revoke_paint_permission());
         }
     }
 
     // Send the current frame tree to compositor, and grant paint
     // permission to each pipeline in the current frame tree.
     fn send_frame_tree_and_grant_paint_permission(&mut self) {
+        // Note that this function can panic, due to ipc-channel creation failure.
+        // avoiding this panic would require a mechanism for dealing
+        // with low-resource scenarios.
         if let Some(root_frame_id) = self.root_frame_id {
             if let Some(frame_tree) = self.frame_to_sendable(root_frame_id) {
 
                 let (chan, port) = ipc::channel().unwrap();
                 self.compositor_proxy.send(ToCompositorMsg::SetFrameTree(frame_tree,
                                                                          chan,
                                                                          self.compositor_sender.clone()));
                 if port.recv().is_err() {