Code review comment for lp:~vanvugt/mir/client-side-vsync

LGTM overall (still haven't fully understood the logic in FrameClock, so more comments on that later, perhaps)

discussion:
135 + * TODO: Remove these functions in future, after
136 + * mir_buffer_stream_swap_buffers has been converted to use
137 + * client-side vsync, and the server has been told to always use
138 + * dropping for BufferStream. Then there will be no need to ever
139 + * change the server swap interval from zero.

Would prefer if this read "and the server has other ways to express buffer queuing other than swapinterval". Would leave room for some of the vulkan submission modes to work via server-side queuing (jury's still out on whether this is a good idea, but I don't know if we want to limit ourselves to client-side queuing only)

needs info:
1)
+void mcl::BufferStream::adopted_by(MirSurface* surface)

Other than BufferStream's currently-terrible constructor, why couldn't the frame clock be given as a build dependency via the constructor? One more parameter to the constructor seems better than a too-long constructor list + a 2step initialization

2)
Where the code currently is might be only useful to android, post-MirRenderSurface/MirSurface/mir-egl-platform. (which still uses MirBufferStream as part of its egl stack for now). From what I've heard from Cemil, mesa is using MirPresentationChains + MirBuffers, so might need some way via an extension or a public-api to get the timing info. (android might also might be better suited for that as well, for now its just easier to re-use MirBufferStream for that platform)

nit:
348 +typedef std::unique_lock<std::mutex> Lock;
IMHO, doesn't improve readability.

I guess that's mostly a 'discussion' vote.