Reland "[macOS] PixelBufferPool: Test retaining IOSurface, not CVPixelBuffer."

This is a reland of be3eeb5b
On "Mac ASan 64 Tests (1)", the assertion that the second pixel buffer's
address will be identical to the first one does not hold. This assertion
was unnecessary anyway, so it is removed. With this re-land, the
IOSurface recycling uses EXPECT_EQ with IOSurfaceGetID() instead.

Original change's description:
> [macOS] PixelBufferPool: Test retaining IOSurface, not CVPixelBuffer.
>
> Pixel buffers originating from a CVPixelBufferPool are recycled when
> no longer in use, previously only tested by releasing the last pixel
> buffer reference and not touching the IOSurface.
>
> This CL tests what happens when we are retaining not references to the
> pixel buffer, but references to a pixel buffer's IOSurface or
> incrementing its use count.
>
> Outcome:
> - Retaining references (base::ScopedCFTypeRef<IOSurfaceRef>) to an
>   IOSurface does NOT prevent recycling.
> - Retaining use count (IOSurfaceIncrementUseCount) to an IOSurface DOES
>   prevent recycling.
>
> This behavior is now documented in pixel_buffer_pool_mac.h.
>
> Bug: chromium:1132299
> Change-Id: Ibdcd3752a4467bc495eba4b543ecd73da1b2f41f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2520829
> Commit-Queue: Markus Handell <handellm@google.com>
> Reviewed-by: Markus Handell <handellm@google.com>
> Cr-Commit-Position: refs/heads/master@{#824814}

TBR=handellm@google.com

Bug: chromium:1132299
Change-Id: Ibe924d3f3ffaaca69262e9cd436c60d05e3ea2b7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2521623
Commit-Queue: Henrik Boström <hbos@chromium.org>
Reviewed-by: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825304}

media/capture/video/mac/pixel_buffer_pool_mac.h

@@ -35,6 +35,15 @@ class CAPTURE_EXPORT PixelBufferPool {
   // Freeing all buffer references returns the underlying buffer to the pool. In
   // order to free memory, you must both release all buffers and call Flush() or
   // delete the pool. It is safe for a buffer to outlive its pool.
+  //
+  // Retaining a pixel buffer and preventing it from returning to the pool can
+  // be done either by keeping a reference directly to the CVPixelBuffer, e.g.
+  // with a base::ScopedCFTypeRef<CVPixelBufferRef>, or by incrementing the use
+  // count of the IOSurface, i.e. with IOSurfaceIncrementUseCount().
+  //
+  // WARNING: Retaining references to the pixel buffer's IOSurface (e.g. with
+  // base::ScopedCFTypeRef<IOSurfaceRef>) without incrementing its use count
+  // does NOT prevent it from being recycled!
   base::ScopedCFTypeRef<CVPixelBufferRef> CreateBuffer();
 
   // Frees the memory of any released buffers returned to the pool.

media/capture/video/mac/pixel_buffer_pool_mac_unittest.mm

@@ -43,9 +43,10 @@ TEST(PixelBufferPoolTest, CreatedBufferHasIOSurface) {
   EXPECT_TRUE(CVPixelBufferGetIOSurface(buffer));
 }
 
-TEST(PixelBufferPoolTest, CannotExceedMaxBuffers) {
-  std::unique_ptr<PixelBufferPool> pool =
-      PixelBufferPool::Create(kPixelFormatNv12, kVgaWidth, kVgaHeight, 2);
+TEST(PixelBufferPoolTest, CannotExceedMaxBuffersWhenHoldingOnToPixelBuffer) {
+  constexpr size_t kPoolMaxBuffers = 2;
+  std::unique_ptr<PixelBufferPool> pool = PixelBufferPool::Create(
+      kPixelFormatNv12, kVgaWidth, kVgaHeight, kPoolMaxBuffers);
   base::ScopedCFTypeRef<CVPixelBufferRef> first_buffer = pool->CreateBuffer();
   EXPECT_TRUE(first_buffer);
   base::ScopedCFTypeRef<CVPixelBufferRef> second_buffer = pool->CreateBuffer();
@@ -54,6 +55,25 @@ TEST(PixelBufferPoolTest, CannotExceedMaxBuffers) {
   EXPECT_FALSE(third_buffer);
 }
 
+TEST(PixelBufferPoolTest, CannotExceedMaxBuffersWhenIOSurfaceIsInUse) {
+  constexpr size_t kPoolMaxBuffers = 1;
+  std::unique_ptr<PixelBufferPool> pool = PixelBufferPool::Create(
+      kPixelFormatNv12, kVgaWidth, kVgaHeight, kPoolMaxBuffers);
+  base::ScopedCFTypeRef<CVPixelBufferRef> first_buffer = pool->CreateBuffer();
+  EXPECT_TRUE(first_buffer);
+  IOSurfaceRef io_surface = CVPixelBufferGetIOSurface(first_buffer);
+  EXPECT_TRUE(io_surface);
+  // Incremet use count of raw ptr IOSurface reference while releasing the pixel
+  // buffer's only reference.
+  IOSurfaceIncrementUseCount(io_surface);
+  first_buffer.reset();
+  // The pixel buffer has not been returned to the pool.
+  base::ScopedCFTypeRef<CVPixelBufferRef> second_buffer = pool->CreateBuffer();
+  EXPECT_FALSE(second_buffer);
+  // Cleanup.
+  IOSurfaceDecrementUseCount(io_surface);
+}
+
 TEST(PixelBufferPoolTest, CanCreateBuffersIfMaxIsNull) {
   std::unique_ptr<PixelBufferPool> pool = PixelBufferPool::Create(
       kPixelFormatNv12, kVgaWidth, kVgaHeight, base::nullopt);
@@ -66,14 +86,57 @@ TEST(PixelBufferPoolTest, CanCreateBuffersIfMaxIsNull) {
 }
 
 TEST(PixelBufferPoolTest, CanCreateBufferAfterPreviousBufferIsReleased) {
-  std::unique_ptr<PixelBufferPool> pool =
-      PixelBufferPool::Create(kPixelFormatNv12, kVgaWidth, kVgaHeight, 1);
+  constexpr size_t kPoolMaxBuffers = 1;
+  std::unique_ptr<PixelBufferPool> pool = PixelBufferPool::Create(
+      kPixelFormatNv12, kVgaWidth, kVgaHeight, kPoolMaxBuffers);
   base::ScopedCFTypeRef<CVPixelBufferRef> buffer = pool->CreateBuffer();
   buffer.reset();
   buffer = pool->CreateBuffer();
   EXPECT_TRUE(buffer);
 }
 
+TEST(PixelBufferPoolTest, CanCreateBufferAfterPreviousIOSurfaceIsNoLongerUsed) {
+  constexpr size_t kPoolMaxBuffers = 1;
+  std::unique_ptr<PixelBufferPool> pool = PixelBufferPool::Create(
+      kPixelFormatNv12, kVgaWidth, kVgaHeight, kPoolMaxBuffers);
+  base::ScopedCFTypeRef<CVPixelBufferRef> first_buffer = pool->CreateBuffer();
+  EXPECT_TRUE(first_buffer);
+  IOSurfaceRef io_surface = CVPixelBufferGetIOSurface(first_buffer);
+  EXPECT_TRUE(io_surface);
+  IOSurfaceIncrementUseCount(io_surface);
+  first_buffer.reset();
+  // Decrementing the use count when there are no pixel buffer references
+  // returns it to the pool.
+  IOSurfaceDecrementUseCount(io_surface);
+  base::ScopedCFTypeRef<CVPixelBufferRef> second_buffer = pool->CreateBuffer();
+  EXPECT_TRUE(second_buffer);
+}
+
+TEST(PixelBufferPoolTest,
+     SimplyReferencingAnIOSurfaceDoesNotPreventItReturningToThePool) {
+  constexpr size_t kPoolMaxBuffers = 1;
+  std::unique_ptr<PixelBufferPool> pool = PixelBufferPool::Create(
+      kPixelFormatNv12, kVgaWidth, kVgaHeight, kPoolMaxBuffers);
+  base::ScopedCFTypeRef<CVPixelBufferRef> first_buffer = pool->CreateBuffer();
+  EXPECT_TRUE(first_buffer);
+  base::ScopedCFTypeRef<IOSurfaceRef> first_buffer_io_surface(
+      CVPixelBufferGetIOSurface(first_buffer), base::scoped_policy::RETAIN);
+  EXPECT_TRUE(first_buffer_io_surface);
+  // Releasing the first buffer returns it to the pool, despite the IOSurface
+  // still being referenced by |first_buffer_io_surface|.
+  first_buffer.reset();
+
+  base::ScopedCFTypeRef<CVPixelBufferRef> second_buffer = pool->CreateBuffer();
+  EXPECT_TRUE(second_buffer);
+  base::ScopedCFTypeRef<IOSurfaceRef> second_buffer_io_surface(
+      CVPixelBufferGetIOSurface(second_buffer), base::scoped_policy::RETAIN);
+  EXPECT_TRUE(second_buffer_io_surface);
+
+  // Because this is a recycled pixel buffer, the IOSurface is also recycled.
+  EXPECT_EQ(IOSurfaceGetID(first_buffer_io_surface),
+            IOSurfaceGetID(second_buffer_io_surface));
+}
+
 TEST(PixelBufferPoolTest, BuffersCanOutliveThePool) {
   std::unique_ptr<PixelBufferPool> pool =
       PixelBufferPool::Create(kPixelFormatNv12, kVgaWidth, kVgaHeight, 1);