User machines seem to sometimes get in a state where stdout isn't writeable, and println! will panic if it can't actually print, crashing the browser. Since we only use println! for debug logging, it should be fine to just continue the program if println fails. It should be fairly easy to make a debugln! macro which is a copy of println! that simply suppresses its result.

Ah actually webrender already has log/env_logger all setup. We should just use it everywhere: https://github.com/servo/webrender/blob/master/webrender/src/device.rs#L769

Some upstream work for this: https://github.com/servo/webrender/pull/2648

Not 100% confident in the log levels I chose, nor if this actually just works without me doing anything else. Non-trivial review appreciated :D

Comment on attachment 8967454 [details] [diff] [review] Bug 1434630 - use proper logging framework instead of println. Review of attachment 8967454 [details] [diff] [review]: ----------------------------------------------------------------- Looks ok. The default level is controlled from https://searchfox.org/mozilla-central/rev/4114ad2cfcbc511705c7865a4a34741812f9a2a9/gfx/webrender_bindings/WebRenderAPI.cpp#253 - I think we might want to change that to info or warn by default. I'm not too sure how much spew it will generate but until we ship webrender getting more output by default is probably useful. We could even increase the level on debug builds by #ifdef DEBUG ::: gfx/webrender_bindings/src/bindings.rs @@ +753,2 @@ > let version = gl.get_string(gl::VERSION); > + debug!("WebRender - OpenGL version new {}", version); I think this one should be info instead of debug

changed debug! to info!

Comment on attachment 8967576 [details] [diff] [review] Bug 1434630 - use proper logging framework instead of println. Review of attachment 8967576 [details] [diff] [review]: ----------------------------------------------------------------- A commit message would be good too

commit message added

The patch had the bug number as 1434630. Which is not this bug, but basically the same thing. Since mc-merge will close that bug I'm going to close this one now.