The SurfaceCache is intended to only consider a surface used if some caller is actually, well, using it. However, that's not what's happening in practice, because: (1) SurfaceCacheImpl::Insert calls SurfaceCacheImpl::Lookup, which marks the looked-up surface used. This is incorrect because Insert() is primarily called by decoders, and if we happen to create multiple decoders for the same SurfaceKey, the decoders which lose the race to create the surface should not mark that surface as used. (2) SurfaceCacheImpl::LookupBestMatch marks surfaces as used whether it's returning an exact match or not. This is a bad idea, because it can greatly increase the lifetime of a surface that we no longer need, just because we happened to call Draw() one time before the new surface finished decoding. Let's fix these issues.

Here's the patch. SurfaceCacheImpl::Lookup() gets a new boolean aMarkUsed parameter that determines whether it should mark the resulting surface used, and we pass false for this parameter in SurfaceCacheImpl::Insert(). SurfaceCacheImpl::LookupBestMatch() only marks the resulting surface used if it's an exact match. To avoid some ugly nested |if| constructs, I've gone ahead and moved the code that marks a surface used into its own private method, SurfaceCacheImpl::MarkUsed(). This seemed too trivial for its own patch.

Ack, comment 3 is actually a try job for bug 1151359, posted in this bug by mistake.

Comment on attachment 8633698 [details] [diff] [review] Only mark surfaces as used in the SurfaceCache if a caller requested exactly that surface r=me

Thanks for the review!