servo: Merge #1500 - Fix crashes and deadlocks when running the content tests (from larsbergstrom:content_fixup); r=metajack

r? @metajack

Source-Repo: https://github.com/servo/servo
Source-Revision: 9b7425000b6b5df1e409699f8faead47eb9e41a8

servo/src/components/gfx/render_task.rs

@@ -187,6 +187,7 @@ impl<C: RenderListener + Send,T:Send+Freeze> RenderTask<C,T> {
                     |ctx| render_task.buffer_map.clear(ctx));
             }
 
+            debug!("render_task: shutdown_chan send");
             shutdown_chan.send(());
         });
     }
@@ -201,6 +202,7 @@ impl<C: RenderListener + Send,T:Send+Freeze> RenderTask<C,T> {
                         self.epoch.next();
                         self.compositor.set_layer_page_size_and_color(self.id, render_layer.size, self.epoch, render_layer.color);
                     } else {
+                        debug!("render_task: render ready msg");
                         self.constellation_chan.send(RendererReadyMsg(self.id));
                     }
                     self.render_layer = Some(render_layer);
@@ -232,6 +234,7 @@ impl<C: RenderListener + Send,T:Send+Freeze> RenderTask<C,T> {
                     self.paint_permission = false;
                 }
                 ExitMsg(response_ch) => {
+                    debug!("render_task: exitmsg response send");
                     response_ch.send(());
                     break;
                 }
@@ -383,6 +386,7 @@ impl<C: RenderListener + Send,T:Send+Freeze> RenderTask<C,T> {
             if self.paint_permission {
                 self.compositor.paint(self.id, layer_buffer_set, self.epoch);
             } else {
+                debug!("render_task: RendererReadyMsg send");
                 self.constellation_chan.send(RendererReadyMsg(self.id));
             }
             self.compositor.set_render_state(IdleRenderState);

servo/src/components/main/compositing/compositor.rs

@@ -32,7 +32,7 @@ use layers::scene::Scene;
 use opengles::gl2;
 use png;
 use servo_msg::compositor_msg::{Epoch, IdleRenderState, LayerBufferSet, RenderState};
-use servo_msg::constellation_msg::{ConstellationChan, NavigateMsg, ResizedWindowMsg, LoadUrlMsg, PipelineId};
+use servo_msg::constellation_msg::{ConstellationChan, ExitMsg, NavigateMsg, ResizedWindowMsg, LoadUrlMsg, PipelineId};
 use servo_msg::constellation_msg;
 use servo_util::time::{profile, ProfilerChan, Timer};
 use servo_util::{time, url};
@@ -129,7 +129,7 @@ impl IOCompositor {
             zoom_action: false,
             zoom_time: 0f64,
             compositor_layer: None,
-            constellation_chan: constellation_chan.clone(),
+            constellation_chan: constellation_chan,
             profiler_chan: profiler_chan,
             fragment_point: None
         }
@@ -202,10 +202,16 @@ impl IOCompositor {
                 None => break,
 
                 Some(Exit(chan)) => {
-                    self.done = true;
+                    debug!("shutting down the constellation");
+                    self.constellation_chan.send(ExitMsg);
                     chan.send(());
                 }
 
+                Some(ShutdownComplete) => {
+                    debug!("constellation completed shutdown");
+                    self.done = true;
+                }
+
                 Some(ChangeReadyState(ready_state)) => {
                     self.window.set_ready_state(ready_state);
                 }
@@ -496,12 +502,14 @@ impl IOCompositor {
             FinishedWindowEvent => {
                 let exit = self.opts.exit_after_load;
                 if exit {
-                    self.done = true;
+                    debug!("shutting down the constellation for FinishedWindowEvent");
+                    self.constellation_chan.send(ExitMsg);
                 }
             }
 
             QuitWindowEvent => {
-                self.done = true;
+                debug!("shutting down the constellation for QuitWindowEvent");
+                self.constellation_chan.send(ExitMsg);
             }
         }
     }
@@ -655,14 +663,16 @@ impl IOCompositor {
             let res = png::store_png(&img, &path);
             assert!(res.is_ok());
 
-            self.done = true;
+            debug!("shutting down the constellation after generating an output file");
+            self.constellation_chan.send(ExitMsg);
         }
 
         self.window.present();
 
         let exit = self.opts.exit_after_load;
         if exit {
-            self.done = true;
+            debug!("shutting down the constellation for exit_after_load");
+            self.constellation_chan.send(ExitMsg);
         }
     }
 

servo/src/components/main/compositing/compositor_task.rs

@@ -16,7 +16,7 @@ use gfx::opts::Opts;
 use layers::platform::surface::{NativeCompositingGraphicsContext, NativeGraphicsMetadata};
 use servo_msg::compositor_msg::{Epoch, RenderListener, LayerBufferSet, RenderState, ReadyState};
 use servo_msg::compositor_msg::{ScriptListener, Tile};
-use servo_msg::constellation_msg::{ConstellationChan, PipelineId, ExitMsg};
+use servo_msg::constellation_msg::{ConstellationChan, PipelineId};
 use servo_util::time::ProfilerChan;
 use std::comm::{Chan, SharedChan, Port};
 use std::num::Orderable;
@@ -112,11 +112,16 @@ impl CompositorChan {
         self.chan.send(msg);
     }
 }
-
 /// Messages from the painting task and the constellation task to the compositor task.
 pub enum Msg {
     /// Requests that the compositor shut down.
     Exit(Chan<()>),
+
+    /// Informs the compositor that the constellation has completed shutdown.
+    /// Required because the constellation can have pending calls to make (e.g. SetIds)
+    /// at the time that we send it an ExitMsg.
+    ShutdownComplete,
+
     /// Requests the compositor's graphics metadata. Graphics metadata is what the renderer needs
     /// to create surfaces that the compositor can see. On Linux this is the X display; on Mac this
     /// is the pixel format.
@@ -185,9 +190,7 @@ impl CompositorTask {
     pub fn create(opts: Opts,
                   port: Port<Msg>,
                   constellation_chan: ConstellationChan,
-                  profiler_chan: ProfilerChan,
-                  exit_chan: Chan<()>,
-                  exit_response_from_constellation: Port<()>) {
+                  profiler_chan: ProfilerChan) {
 
         let compositor = CompositorTask::new(opts.headless);
 
@@ -197,18 +200,12 @@ impl CompositorTask {
                                                  opts,
                                                  port,
                                                  constellation_chan.clone(),
-                                                 profiler_chan);
+                                                 profiler_chan)
             }
             Headless => {
                 headless::NullCompositor::create(port,
-                                                 constellation_chan.clone());
+                                                 constellation_chan.clone())
             }
-        }
-
-        // Constellation has to be shut down before the compositor goes out of
-        // scope, as the compositor manages setup/teardown of global subsystems
-        debug!("shutting down the constellation");
-        constellation_chan.send(ExitMsg(exit_chan));
-        exit_response_from_constellation.recv();
+        };
     }
 }

servo/src/components/main/compositing/headless.rs

@@ -5,7 +5,7 @@
 use compositing::*;
 
 use geom::size::Size2D;
-use servo_msg::constellation_msg::{ConstellationChan, ResizedWindowMsg};
+use servo_msg::constellation_msg::{ConstellationChan, ExitMsg, ResizedWindowMsg};
 use std::comm::Port;
 
 
@@ -32,14 +32,20 @@ impl NullCompositor {
 
         // Tell the constellation about the initial fake size.
         constellation_chan.send(ResizedWindowMsg(Size2D(640u, 480u)));
-        compositor.handle_message();
+        compositor.handle_message(constellation_chan);
     }
 
-    fn handle_message(&self) {
+    fn handle_message(&self, constellation_chan: ConstellationChan) {
         loop {
             match self.port.recv() {
                 Exit(chan) => {
+                    debug!("shutting down the constellation");
+                    constellation_chan.send(ExitMsg);
                     chan.send(());
+                }
+
+                ShutdownComplete => {
+                    debug!("constellation completed shutdown");
                     break
                 }
 

servo/src/components/main/constellation.rs

@@ -2,7 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-use compositing::{CompositorChan, SetIds, SetLayerClipRect};
+use compositing::{CompositorChan, SetIds, SetLayerClipRect, ShutdownComplete};
 
 use extra::url::Url;
 use geom::rect::Rect;
@@ -315,9 +315,9 @@ impl Constellation {
     /// Handles loading pages, navigation, and granting access to the compositor
     fn handle_request(&mut self, request: Msg) -> bool {
         match request {
-            ExitMsg(sender) => {
+            ExitMsg => {
                 debug!("constellation exiting");
-                self.handle_exit(sender);
+                self.handle_exit();
                 return false;
             }
             FailureMsg(pipeline_id, subpage_id) => {
@@ -363,14 +363,13 @@ impl Constellation {
         true
     }
 
-    fn handle_exit(&self, sender: Chan<()>) {
+    fn handle_exit(&self) {
         for (_id, ref pipeline) in self.pipelines.iter() {
             pipeline.exit();
         }
         self.image_cache_task.exit();
         self.resource_task.send(resource_task::Exit);
-
-        sender.send(());
+        self.compositor_chan.send(ShutdownComplete);
     }
 
     fn handle_failure_msg(&mut self, pipeline_id: PipelineId, subpage_id: Option<SubpageId>) {
@@ -804,6 +803,7 @@ impl Constellation {
 
     fn set_ids(&self, frame_tree: @mut FrameTree) {
         let (port, chan) = Chan::new();
+        debug!("Constellation sending SetIds");
         self.compositor_chan.send(SetIds(frame_tree.to_sendable(), chan, self.chan.clone()));
         match port.recv_opt() {
             Some(()) => {

servo/src/components/main/pipeline.rs

@@ -181,6 +181,7 @@ impl Pipeline {
     }
 
     pub fn revoke_paint_permission(&self) {
+        debug!("pipeline revoking render channel paint permission");
         self.render_chan.send(PaintPermissionRevoked);
     }
 

servo/src/components/main/servo.rc

@@ -135,7 +135,6 @@ pub extern "C" fn android_start(argc: int, argv: **u8) -> int {
 fn run(opts: Opts) {
     let mut pool = green::SchedPool::new(green::PoolConfig::new());
 
-    let (exit_response_from_constellation, exit_chan) = Chan::new();
     let (compositor_port, compositor_chan) = CompositorChan::new();
     let profiler_chan = Profiler::create(opts.profiler_period);
 
@@ -154,13 +153,13 @@ fn run(opts: Opts) {
                                                       image_cache_task,
                                                       profiler_chan_clone);
 
-        // Send the constallation Chan as the result
-        result_chan.send(constellation_chan.clone());
-
         // Send the URL command to the constellation.
         for filename in opts.urls.iter() {
             constellation_chan.send(InitLoadUrlMsg(make_url(filename.clone(), None)))
         }
+
+        // Send the constallation Chan as the result
+        result_chan.send(constellation_chan);
     });
 
     let constellation_chan = result_port.recv();
@@ -169,9 +168,7 @@ fn run(opts: Opts) {
     CompositorTask::create(opts,
                            compositor_port,
                            constellation_chan,
-                           profiler_chan,
-                           exit_chan,
-                           exit_response_from_constellation);
+                           profiler_chan);
 
     pool.shutdown();
 }

servo/src/components/msg/constellation_msg.rs

@@ -8,7 +8,7 @@
 use extra::url::Url;
 use geom::rect::Rect;
 use geom::size::Size2D;
-use std::comm::{Chan, SharedChan};
+use std::comm::SharedChan;
 
 #[deriving(Clone)]
 pub struct ConstellationChan(SharedChan<Msg>);
@@ -28,7 +28,7 @@ pub enum IFrameSandboxState {
 
 /// Messages from the compositor to the constellation.
 pub enum Msg {
-    ExitMsg(Chan<()>),
+    ExitMsg,
     FailureMsg(PipelineId, Option<SubpageId>),
     InitLoadUrlMsg(Url),
     FrameRectMsg(PipelineId, SubpageId, Rect<f32>),

servo/src/components/script/script_task.rs

@@ -531,10 +531,7 @@ impl ScriptTask {
                 ReflowCompleteMsg(id, reflow_id) => self.handle_reflow_complete_msg(id, reflow_id),
                 ResizeInactiveMsg(id, new_size) => self.handle_resize_inactive_msg(id, new_size),
                 ExitPipelineMsg(id) => if self.handle_exit_pipeline_msg(id) { return false },
-                ExitWindowMsg(id) => {
-                    self.handle_exit_window_msg(id);
-                    return false
-                },
+                ExitWindowMsg(id) => self.handle_exit_window_msg(id),
                 ResizeMsg(..) => fail!("should have handled ResizeMsg already"),
             }
         }
@@ -614,9 +611,13 @@ impl ScriptTask {
         }
     }
 
-    fn handle_exit_window_msg(&mut self, id: PipelineId) {
+    /// We have gotten a window.close from script, which we pass on to the compositor.
+    /// We do not shut down the script task now, because the compositor will ask the
+    /// constellation to shut down the pipeline, which will clean everything up
+    /// normally. If we do exit, we will tear down the DOM nodes, possibly at a point
+    /// where layout is still accessing them.
+    fn handle_exit_window_msg(&mut self, _: PipelineId) {
         debug!("script task handling exit window msg");
-        self.handle_exit_pipeline_msg(id);
 
         // TODO(tkuehn): currently there is only one window,
         // so this can afford to be naive and just shut down the