I think we explicitly decided against this a while back because it would involve deep copying CompositorFrames when we need to store them. Currently, we std::move CompositorFrames around and so this is a shallow copy because we pass the containers over across CompositorFrame instances.

It's a move-only type like a unique_ptr. It's a bit heavier because of the metadata to move but majority of it is behind pointers (render pass/resource list vectors). Did you have some reason to change it?

Reasoning: seemed like it could be a small optimization. Maybe a low-priority starter bug.

Some background: I was the one who changed CompositorFrames to be movable everywhere to match expectations of mojo serialization where we want to map to the same thing on either side of the IPC boundary. After a bunch of discussion with piman@ and danakj@, we decided that passing CompositorFrames by unique_ptr would incur a bunch of unnecessary heap allocations and it would probably be the same or faster to move everywhere (note that I didn't benchmark this).

Ultimately, what I wanted to achieve was code that was mojo serialization friendly.

My first CL: https://codereview.chromium.org/2096493002/
My second CL: https://codereview.chromium.org/2096493002/

We don't pass by const ref because more often than not we actually want to stash the CompositorFrame locally: in Surface say. Passing by const ref in that case would actually be slower because we would need to do a full copy as opposed to a shallow copy incurred by std::move.

Has this been profiled? The size of CompositorFrame on my machine is 256 bytes, and it seems to go through at least 5 or 6 shalow copies:

Someone calls
CompositorFrameSink::SubmitCompositorFrame which calls
CompositorFrameSinkSupport::SubmitCompositorFrame which calls
SurfaceFactory::SubmitCompositorFrame which calls
Surface::QueueFrame which then either stores this or calls
Surface::ActivateFrame

I'm just curious how a single deep copy compare to that or a unique_ptr for that matter.

(Maybe if we want to really optimize this, we can have CompositorFrameSink::SubmitCompositorFrame take a value, but then pass through a raw pointer to everywhere with expectation that the end point will move from the object to store a member).

I did some profiling here, and at least on my machine, passive CompositorFrame by pointer and moving out of it is <2% faster which is within the noise of the perftest. I don't think this is worth fixing.