auto merge of #1544 : larsbergstrom/servo/improve-reftesting, r=metajack

r? @metajack

mk/check.mk

@@ -40,7 +40,7 @@ servo-test: $(DEPS_servo)
 
 reftest: $(S)src/test/harness/reftest/reftest.rs servo
 	@$(call E, compile: $@)
-	$(Q)$(RUSTC) -o $@ $<
+	$(Q)$(RUSTC) -L$(B)/src/support/png/rust-png/ -o $@ $<
 
 contenttest: $(S)src/test/harness/contenttest/contenttest.rs servo
 	@$(call E, compile: $@)

src/components/main/compositing/compositor.rs

@@ -66,6 +66,9 @@ pub struct IOCompositor {
     /// Tracks whether the renderer has finished its first rendering
     composite_ready: bool,
 
+    /// Tracks whether we are in the process of shutting down.
+    shutting_down: bool,
+
     /// Tracks whether we should close compositor.
     done: bool,
 
@@ -123,6 +126,7 @@ impl IOCompositor {
             window_size: Size2D(window_size.width as uint, window_size.height as uint),
             graphics_context: CompositorTask::create_graphics_context(),
             composite_ready: false,
+            shutting_down: false,
             done: false,
             recomposite: false,
             world_zoom: 1f32,
@@ -198,68 +202,74 @@ impl IOCompositor {
 
     fn handle_message(&mut self) {
         loop {
-            match self.port.try_recv() {
-                None => break,
+            match (self.port.try_recv(), self.shutting_down) {
+                (None, _) => break,
 
-                Some(Exit(chan)) => {
+                (Some(Exit(chan)), _) => {
                     debug!("shutting down the constellation");
                     self.constellation_chan.send(ExitMsg);
                     chan.send(());
+                    self.shutting_down = true;
                 }
 
-                Some(ShutdownComplete) => {
+                (Some(ShutdownComplete), _) => {
                     debug!("constellation completed shutdown");
                     self.done = true;
                 }
 
-                Some(ChangeReadyState(ready_state)) => {
+                (Some(ChangeReadyState(ready_state)), false) => {
                     self.window.set_ready_state(ready_state);
                 }
 
-                Some(ChangeRenderState(render_state)) => {
+                (Some(ChangeRenderState(render_state)), false) => {
                     self.change_render_state(render_state);
                 }
 
-                Some(SetUnRenderedColor(_id, color)) => {
+                (Some(SetUnRenderedColor(_id, color)), false) => {
                     self.set_unrendered_color(_id, color);
                 }
 
 
-                Some(SetIds(frame_tree, response_chan, new_constellation_chan)) => {
+                (Some(SetIds(frame_tree, response_chan, new_constellation_chan)), _) => {
                     self.set_ids(frame_tree, response_chan, new_constellation_chan);
                 }
 
-                Some(GetGraphicsMetadata(chan)) => {
+                (Some(GetGraphicsMetadata(chan)), false) => {
                     chan.send(Some(azure_hl::current_graphics_metadata()));
                 }
 
-                Some(NewLayer(_id, new_size)) => {
+                (Some(NewLayer(_id, new_size)), false) => {
                     self.create_new_layer(_id, new_size);
                 }
 
-                Some(SetLayerPageSize(id, new_size, epoch)) => {
+                (Some(SetLayerPageSize(id, new_size, epoch)), false) => {
                     self.set_layer_page_size(id, new_size, epoch);
                 }
 
-                Some(SetLayerClipRect(id, new_rect)) => {
+                (Some(SetLayerClipRect(id, new_rect)), false) => {
                     self.set_layer_clip_rect(id, new_rect);
                 }
 
-                Some(DeleteLayer(id)) => {
+                (Some(DeleteLayer(id)), _) => {
                     self.delete_layer(id);
                 }
 
-                Some(Paint(id, new_layer_buffer_set, epoch)) => {
+                (Some(Paint(id, new_layer_buffer_set, epoch)), false) => {
                     self.paint(id, new_layer_buffer_set, epoch);
                 }
 
-                Some(InvalidateRect(id, rect)) => {
+                (Some(InvalidateRect(id, rect)), false) => {
                     self.invalidate_rect(id, rect);
                 }
 
-                Some(ScrollFragmentPoint(id, point)) => {
+                (Some(ScrollFragmentPoint(id, point)), false) => {
                     self.scroll_fragment_to_point(id, point);
                 }
+
+                // 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.
+                (_, true) => { }
             }
         }
     }
@@ -504,12 +514,14 @@ impl IOCompositor {
                 if exit {
                     debug!("shutting down the constellation for FinishedWindowEvent");
                     self.constellation_chan.send(ExitMsg);
+                    self.shutting_down = true;
                 }
             }
 
             QuitWindowEvent => {
                 debug!("shutting down the constellation for QuitWindowEvent");
                 self.constellation_chan.send(ExitMsg);
+                self.shutting_down = true;
             }
         }
     }
@@ -665,6 +677,7 @@ impl IOCompositor {
 
             debug!("shutting down the constellation after generating an output file");
             self.constellation_chan.send(ExitMsg);
+            self.shutting_down = true;
         }
 
         self.window.present();

src/test/harness/reftest/reftest.rs

@@ -7,12 +7,14 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-extern mod std;
 extern mod extra;
+extern mod png;
+extern mod std;
 
 use std::io;
 use std::io::{File, Reader};
 use std::io::process::ExitStatus;
+use std::num::abs;
 use std::os;
 use std::run::{Process, ProcessOptions};
 use std::str;
@@ -45,6 +47,7 @@ fn main() {
     }
 }
 
+#[deriving(Eq)]
 enum ReftestKind {
     Same,
     Different,
@@ -131,12 +134,29 @@ fn check_reftest(reftest: Reftest) {
     assert!(retval == ExitStatus(0));
 
     // check the pngs are bit equal
-    let args = ~[left_filename.clone(), right_filename.clone()];
-    let mut process = Process::new("cmp", args, ProcessOptions::new()).unwrap();
-    let retval = process.finish();
+    let left = png::load_png(&from_str::<Path>(left_filename).unwrap()).unwrap();
+    let right = png::load_png(&from_str::<Path>(right_filename).unwrap()).unwrap();
 
-    match reftest.kind {
-        Same => assert!(retval == ExitStatus(0)),
-        Different => assert!(retval != ExitStatus(0)),
+    let pixels: ~[u8] = left.pixels.iter().zip(right.pixels.iter()).map(|(&a, &b)| {
+            let a_signed = a as i8;
+            let b_signed = b as i8;
+            255-abs(a_signed - b_signed) as u8
+        }).collect();
+
+    if pixels.iter().any(|&a| a < 255) {
+        let output = from_str::<Path>(format!("/tmp/servo-reftest-{:06u}-diff.png", reftest.id)).unwrap();
+
+        let img = png::Image {
+            width: left.width,
+            height: left.height,
+            color_type: png::RGBA8,
+            pixels: pixels,
+        };
+        let res = png::store_png(&img, &output);
+        assert!(res.is_ok());
+
+        assert!(reftest.kind == Different);
+    } else {
+        assert!(reftest.kind == Same);
     }
 }