Code review comment for lp:~mir-team/mir/fix-1339700

I'm not really familiar with this. It's all new stuff that appeared while I was on vacation. But simple logic still applies:

(1) Accessing a member variable after having given up the lock that was protecting it, I think is a bad idea:
36 + give_buffer_to_client(buffer, std::move(lock));
37 framedrop_policy->swap_unblocked();
38 - give_buffer_to_client(buffer, std::move(lock));

(2) This unlock also results in a member variable no longer being safely locked. It looks like it should stay locked. Certainly it doesn't appear to do any callbacks...
28 + lock.unlock();
29 framedrop_policy->swap_now_blocking();

(3) No regression test!
Maybe I can help with that as I remember seeing the offending assertion fail for me a couple of times this week. I just need to find where...