While I was working on the new tab capture impl, and integrating it with the video capture stack, I came across a very subtle race. The problem manifests as a DCHECK() failure in VideoCaptureImpl::OnBufferDestroyed() because the ref-count on the ClientBuffer is not exactly 1, as expected. This leads the DCHECK() to believe a buffer is being destroyed while in-use.

Digging deeper, I believe the root-cause is that VideoCaptureImpl::DidFinishConsumingFrame(), which runs on a different thread, is implicitly holding a reference to the ClientBuffer via the callback it is running. The following very unlikely, but obviously possible sequence of steps occurs:

1. DidFinishConsumingFrame() is called and runs the "callback_to_io_thread". It has NOT YET returned from the Run() call. The thread sleeps throughout the remaining steps, and so there is a reference being held on the ClientBuffer from here the entire time.

2. The callback itself trampolines to the IO thread and it calls OnClientBufferFinished().

3. OnClientBufferFinished() calls VideoCaptureHost::ReleaseBuffer(), which sends a mojo message to the browser process.

4. The browser process then sends a mojo message back, causing OnBufferDestroyed() to be called. Since the ref-count on the ClientBuffer is one too high, the DCHECK() fails.

5. Finally, the thread from step 1 resumes and exits, auto-releasing its reference to the ClientBuffer. Unfortunately, it's too late.

So, there's actually nothing catastrophic going on here other than the DCHECK() thinking the buffer is still in use. Nevertheless, we can correct the problem so that the DCHECK() is not "false alarming" by making sure the scoped_refptr<ClientBuffer> is always moved and never copied, and that the thread in step 1 can't possibly hold a reference. When I make this modification locally, my "frequently crashes" use case stops crashing, showing the problem is solved.