[Tab Capture] Simplify reuse of already captured content

Before this CL, class FrameSinkVideoCapturerImpl uses a somewhat
hard-to-follow protocol with InterprocessFramePool for resurrecting
video frames in case that the source content did not change.

This CL changes this protocol to a simpler-to-follow mechanism, which
also can reduce the cases where content is re-captured even though
it is already available in a previously captured buffer.

Change-Id: I635e3cf0dce61f704a6b613702791f03ff0c217a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1479154
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Reviewed-by: Yuri Wiitala <miu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#648688}

components/viz/service/frame_sinks/video_capture/frame_sink_video_capturer_impl.cc

@@ -141,6 +141,7 @@ void FrameSinkVideoCapturerImpl::SetFormat(media::VideoPixelFormat format,
   }
 
   if (format_changed) {
+    frame_pool_.ClearFrameMarking();
     RefreshEntireSourceSoon();
   }
 }
@@ -297,7 +298,7 @@ base::TimeDelta FrameSinkVideoCapturerImpl::GetDelayBeforeNextRefreshAttempt()
 void FrameSinkVideoCapturerImpl::RefreshEntireSourceSoon() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  dirty_rect_ = kMaxRect;
+  InvalidateEntireSource();
   RefreshSoon();
 }
 
@@ -327,11 +328,11 @@ void FrameSinkVideoCapturerImpl::RefreshSoon() {
   }
   if (source_size != oracle_->source_size()) {
     oracle_->SetSourceSize(source_size);
-    dirty_rect_ = kMaxRect;
+    InvalidateEntireSource();
   }
 
-  MaybeCaptureFrame(VideoCaptureOracle::kRefreshRequest,
-                    gfx::Rect(oracle_->source_size()), clock_->NowTicks(),
+  MaybeCaptureFrame(VideoCaptureOracle::kRefreshRequest, gfx::Rect(),
+                    clock_->NowTicks(),
                     *resolved_target_->GetLastActivatedFrameMetadata());
 }
 
@@ -350,7 +351,7 @@ void FrameSinkVideoCapturerImpl::OnFrameDamaged(
     InvalidateRect(damage_rect);
   } else {
     oracle_->SetSourceSize(frame_size);
-    dirty_rect_ = kMaxRect;
+    InvalidateEntireSource();
   }
 
   MaybeCaptureFrame(VideoCaptureOracle::kCompositorUpdate, damage_rect,
@@ -369,6 +370,14 @@ void FrameSinkVideoCapturerImpl::InvalidateRect(const gfx::Rect& rect) {
   gfx::Rect positive_rect = rect;
   positive_rect.Intersect(kMaxRect);
   dirty_rect_.Union(positive_rect);
+  content_version_++;
+}
+
+void FrameSinkVideoCapturerImpl::InvalidateEntireSource() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  dirty_rect_ = kMaxRect;
+  content_version_++;
 }
 
 void FrameSinkVideoCapturerImpl::OnOverlayConnectionLost(
@@ -431,27 +440,19 @@ void FrameSinkVideoCapturerImpl::MaybeCaptureFrame(
     return;
   }
 
-  // Reserve a buffer from the pool for the next frame. Optimization: If there
-  // are no changes in the source that need to be captured AND there are no
-  // captures currently in-flight, attempt to resurrect the last frame from the
-  // pool (and there is no need to capture anything new into the frame).
+  // Reserve a buffer from the pool for the next frame.
   const OracleFrameNumber oracle_frame_number = oracle_->next_frame_number();
   const gfx::Size capture_size =
       AdjustSizeForPixelFormat(oracle_->capture_size());
 
+  const bool can_resurrect_content =
+      content_version_in_marked_frame_ == content_version_ &&
+      frame_pool_.HasMarkedFrameWithSize(capture_size);
   scoped_refptr<VideoFrame> frame;
-  bool using_resurrected_frame =
-      dirty_rect_.IsEmpty() &&
-      next_capture_frame_number_ == next_delivery_frame_number_;
-  if (using_resurrected_frame) {
-    frame = frame_pool_.ResurrectLastVideoFrame(pixel_format_, capture_size);
-    // If the resurrection failed, promote to a full frame capture.
-    if (!frame) {
-      TRACE_EVENT_INSTANT0("gpu.capture", "ResurrectionFailed",
-                           TRACE_EVENT_SCOPE_THREAD);
-      using_resurrected_frame = false;
-      frame = frame_pool_.ReserveVideoFrame(pixel_format_, capture_size);
-    }
+  if (can_resurrect_content) {
+    TRACE_EVENT_INSTANT0("gpu.capture", "UsingResurrectedFrame",
+                         TRACE_EVENT_SCOPE_THREAD);
+    frame = frame_pool_.ResurrectOrDuplicateContentFromMarkedFrame();
   } else {
     frame = frame_pool_.ReserveVideoFrame(pixel_format_, capture_size);
   }
@@ -564,16 +565,16 @@ void FrameSinkVideoCapturerImpl::MaybeCaptureFrame(
       frame->set_color_space(gfx::ColorSpace::CreateSRGB());
     }
     dirty_rect_ = gfx::Rect();
-    DidCaptureFrame(capture_frame_number, oracle_frame_number, gfx::Rect(),
-                    std::move(frame));
+    OnFrameReadyForDelivery(capture_frame_number, oracle_frame_number,
+                            gfx::Rect(), std::move(frame));
     return;
   }
 
   // If the frame is a resurrected one, just deliver it since it already
   // contains the most up-to-date capture of the source content.
-  if (using_resurrected_frame) {
-    DidCaptureFrame(capture_frame_number, oracle_frame_number, content_rect,
-                    std::move(frame));
+  if (can_resurrect_content) {
+    OnFrameReadyForDelivery(capture_frame_number, oracle_frame_number,
+                            content_rect, std::move(frame));
     return;
   }
 
@@ -584,7 +585,7 @@ void FrameSinkVideoCapturerImpl::MaybeCaptureFrame(
           : CopyOutputRequest::ResultFormat::RGBA_BITMAP,
       base::BindOnce(&FrameSinkVideoCapturerImpl::DidCopyFrame,
                      capture_weak_factory_.GetWeakPtr(), capture_frame_number,
-                     oracle_frame_number, content_rect,
+                     oracle_frame_number, content_version_, content_rect,
                      VideoCaptureOverlay::MakeCombinedRenderer(
                          GetOverlaysInOrder(), content_rect, frame->format()),
                      std::move(frame))));
@@ -599,16 +600,7 @@ void FrameSinkVideoCapturerImpl::MaybeCaptureFrame(
   request->set_result_selection(gfx::Rect(content_rect.size()));
 
   // Clear the |dirty_rect_|, to indicate all changes at the source are now
-  // being captured. This will also enable the "frame resurrection" optimization
-  // in future calls to this method. In other words, while the source content
-  // remains unchanged, there is no need to make any more CopyOutputRequests.
-  //
-  // Note that some optimistic assumptions are being made here: 1) that this
-  // |request| will succeed and it's image data successfully transferred to the
-  // VideoFrame; and 2) that delivery of the VideoFrame to the consumer will
-  // succeed. If, later in the pipeline, either of these assumptions is
-  // violated, the |dirty_rect_| will be changed to indicate that there might
-  // still be source changes requiring capture. See MaybeDeliverFrame().
+  // being captured.
   dirty_rect_ = gfx::Rect();
 
   resolved_target_->RequestCopyOfOutput(LocalSurfaceId(), std::move(request));
@@ -617,6 +609,7 @@ void FrameSinkVideoCapturerImpl::MaybeCaptureFrame(
 void FrameSinkVideoCapturerImpl::DidCopyFrame(
     int64_t capture_frame_number,
     OracleFrameNumber oracle_frame_number,
+    int64_t content_version,
     const gfx::Rect& content_rect,
     VideoCaptureOverlay::OnceRenderer overlay_renderer,
     scoped_refptr<VideoFrame> frame,
@@ -680,13 +673,18 @@ void FrameSinkVideoCapturerImpl::DidCopyFrame(
     media::LetterboxVideoFrame(
         frame.get(), gfx::Rect(content_rect.origin(),
                                AdjustSizeForPixelFormat(result->size())));
+
+    if (content_version > content_version_in_marked_frame_) {
+      frame_pool_.MarkFrame(*frame);
+      content_version_in_marked_frame_ = content_version;
+    }
   }
 
-  DidCaptureFrame(capture_frame_number, oracle_frame_number, content_rect,
-                  std::move(frame));
+  OnFrameReadyForDelivery(capture_frame_number, oracle_frame_number,
+                          content_rect, std::move(frame));
 }
 
-void FrameSinkVideoCapturerImpl::DidCaptureFrame(
+void FrameSinkVideoCapturerImpl::OnFrameReadyForDelivery(
     int64_t capture_frame_number,
     OracleFrameNumber oracle_frame_number,
     const gfx::Rect& content_rect,
@@ -733,12 +731,6 @@ void FrameSinkVideoCapturerImpl::MaybeDeliverFrame(
     TRACE_EVENT_ASYNC_END1("gpu.capture", "Capture", oracle_frame_number,
                            "success", false);
 
-    // Mark the whole source as dirty, since this frame may have contained
-    // updated content that will not be delivered. See the comment at the end of
-    // MaybeCaptureFrame() regarding "optimistic assumptions" for further
-    // discussion.
-    dirty_rect_ = kMaxRect;
-
     ScheduleRefreshFrame();
     return;
   }

components/viz/service/frame_sinks/video_capture/frame_sink_video_capturer_impl.h

@@ -174,6 +174,8 @@ class VIZ_SERVICE_EXPORT FrameSinkVideoCapturerImpl final
   void InvalidateRect(const gfx::Rect& rect) final;
   void OnOverlayConnectionLost(VideoCaptureOverlay* overlay) final;
 
+  void InvalidateEntireSource();
+
   // Returns a list of the overlays in rendering order.
   std::vector<VideoCaptureOverlay*> GetOverlaysInOrder() const;
 
@@ -189,6 +191,7 @@ class VIZ_SERVICE_EXPORT FrameSinkVideoCapturerImpl final
   // |content_rect| region of a [possibly letterboxed] video |frame|.
   void DidCopyFrame(int64_t capture_frame_number,
                     OracleFrameNumber oracle_frame_number,
+                    int64_t content_version,
                     const gfx::Rect& content_rect,
                     VideoCaptureOverlay::OnceRenderer overlay_renderer,
                     scoped_refptr<media::VideoFrame> frame,
@@ -197,10 +200,10 @@ class VIZ_SERVICE_EXPORT FrameSinkVideoCapturerImpl final
   // Places the frame in the |delivery_queue_| and calls MaybeDeliverFrame(),
   // one frame at a time, in-order. |frame| may be null to indicate a
   // completed, but unsuccessful capture.
-  void DidCaptureFrame(int64_t capture_frame_number,
-                       OracleFrameNumber oracle_frame_number,
-                       const gfx::Rect& content_rect,
-                       scoped_refptr<media::VideoFrame> frame);
+  void OnFrameReadyForDelivery(int64_t capture_frame_number,
+                               OracleFrameNumber oracle_frame_number,
+                               const gfx::Rect& content_rect,
+                               scoped_refptr<media::VideoFrame> frame);
 
   // Delivers a |frame| to the consumer, if the VideoCaptureOracle allows
   // it. |frame| can be null to indicate a completed, but unsuccessful capture.
@@ -281,6 +284,12 @@ class VIZ_SERVICE_EXPORT FrameSinkVideoCapturerImpl final
   // frames in-flight at any one time.
   InterprocessFramePool frame_pool_;
 
+  // Increased every time the source content changes or a forced refresh is
+  // requested.
+  int64_t content_version_ = 0;
+
+  int64_t content_version_in_marked_frame_ = -1;
+
   // A queue of captured frames pending delivery. This queue is used to re-order
   // frames, if they should happen to be captured out-of-order.
   struct CapturedFrame {

components/viz/service/frame_sinks/video_capture/interprocess_frame_pool.cc

@@ -30,10 +30,6 @@ scoped_refptr<VideoFrame> InterprocessFramePool::ReserveVideoFrame(
     const gfx::Size& size) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  // Calling this method is a signal that there is no intention of resurrecting
-  // the last frame.
-  resurrectable_buffer_memory_ = nullptr;
-
   const size_t bytes_required = VideoFrame::AllocationSize(format, size);
 
   // Look for an available buffer that's large enough. If one is found, wrap it
@@ -56,6 +52,8 @@ scoped_refptr<VideoFrame> InterprocessFramePool::ReserveVideoFrame(
                          [](const PooledBuffer& a, const PooledBuffer& b) {
                            return a.mapping.size() < b.mapping.size();
                          });
+    if (it->mapping.memory() == marked_frame_buffer_)
+      marked_frame_buffer_ = nullptr;
     available_buffers_.erase(it.base() - 1);  // Release before allocating more.
     PooledBuffer reallocated =
         mojo::CreateReadOnlySharedMemoryRegion(bytes_required);
@@ -82,33 +80,78 @@ scoped_refptr<VideoFrame> InterprocessFramePool::ReserveVideoFrame(
   return WrapBuffer(std::move(additional), format, size);
 }
 
-scoped_refptr<VideoFrame> InterprocessFramePool::ResurrectLastVideoFrame(
-    VideoPixelFormat expected_format,
-    const gfx::Size& expected_size) {
+void InterprocessFramePool::MarkFrame(const media::VideoFrame& frame) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  marked_frame_buffer_ = frame.data(0);
+  marked_frame_size_ = frame.coded_size();
+  marked_frame_color_space_ = frame.ColorSpace();
+  marked_frame_pixel_format_ = frame.format();
+}
 
-  // Find the tracking entry for the resurrectable buffer. If it is still being
-  // used, or is not of the expected format and size, punt.
-  if (resurrectable_buffer_memory_ == nullptr ||
-      last_delivered_format_ != expected_format ||
-      last_delivered_size_ != expected_size) {
-    return nullptr;
-  }
-  const auto it = std::find_if(
-      available_buffers_.rbegin(), available_buffers_.rend(),
-      [this](const PooledBuffer& candidate) {
-        return candidate.mapping.memory() == resurrectable_buffer_memory_;
-      });
-  if (it == available_buffers_.rend()) {
+void InterprocessFramePool::ClearFrameMarking() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  marked_frame_buffer_ = nullptr;
+}
+
+bool InterprocessFramePool::HasMarkedFrameWithSize(
+    const gfx::Size& size) const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  return marked_frame_buffer_ != nullptr && marked_frame_size_ == size;
+}
+
+scoped_refptr<VideoFrame>
+InterprocessFramePool::ResurrectOrDuplicateContentFromMarkedFrame() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  if (!marked_frame_buffer_)
     return nullptr;
+
+  const auto it =
+      std::find_if(available_buffers_.rbegin(), available_buffers_.rend(),
+                   [this](const PooledBuffer& candidate) {
+                     return candidate.mapping.memory() == marked_frame_buffer_;
+                   });
+
+  // If the buffer is available, use it directly.
+  if (it != available_buffers_.rend()) {
+    // Wrap the buffer in a VideoFrame and return it.
+    PooledBuffer resurrected = std::move(*it);
+    available_buffers_.erase(it.base() - 1);
+    auto frame = WrapBuffer(std::move(resurrected), marked_frame_pixel_format_,
+                            marked_frame_size_);
+    frame->set_color_space(marked_frame_color_space_);
+    return frame;
   }
 
-  // Wrap the buffer in a VideoFrame and return it.
-  PooledBuffer resurrected = std::move(*it);
-  available_buffers_.erase(it.base() - 1);
+  // Buffer is currently in use. Reserve a new buffer and copy the contents
+  // over.
   auto frame =
-      WrapBuffer(std::move(resurrected), expected_format, expected_size);
-  frame->set_color_space(last_delivered_color_space_);
+      ReserveVideoFrame(marked_frame_pixel_format_, marked_frame_size_);
+  // The call to ReserverVideoFrame should not have cleared
+  // |marked_frame_buffer_|, because that buffer is currently in use.
+  DCHECK(marked_frame_buffer_);
+  if (!frame)
+    return nullptr;
+#if DCHECK_IS_ON()
+  // Sanity check that |marked_frame_buffer_| indeed corresponds to a buffer in
+  // |utilized_buffers_|. If MarkFrame() was erroneously called with a frame
+  // that did not belong to this pool or was otherwise tampered with, this might
+  // not be the case.
+  const auto source_it = std::find_if(
+      utilized_buffers_.rbegin(), utilized_buffers_.rend(),
+      [this](const std::pair<const media::VideoFrame*,
+                             base::ReadOnlySharedMemoryRegion>& candidate) {
+        return candidate.first->data(0) == marked_frame_buffer_;
+      });
+  DCHECK(source_it != utilized_buffers_.rend());
+#endif  // DCHECK_IS_ON()
+
+  // Copy the contents over.
+  const size_t num_bytes_to_copy = VideoFrame::AllocationSize(
+      marked_frame_pixel_format_, marked_frame_size_);
+  memcpy(frame->data(0), marked_frame_buffer_, num_bytes_to_copy);
+
+  frame->set_color_space(marked_frame_color_space_);
   return frame;
 }
 
@@ -116,16 +159,8 @@ base::ReadOnlySharedMemoryRegion InterprocessFramePool::CloneHandleForDelivery(
     const VideoFrame* frame) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  // Record that this frame is the last-delivered one, for possible future calls
-  // to ResurrectLastVideoFrame().
   const auto it = utilized_buffers_.find(frame);
   DCHECK(it != utilized_buffers_.end());
-  // Assumption: The first image plane's memory pointer should be the start of
-  // the writable mapped memory. WrapBuffer() sanity-checks this.
-  resurrectable_buffer_memory_ = frame->data(0);
-  last_delivered_format_ = frame->format();
-  last_delivered_size_ = frame->coded_size();
-  last_delivered_color_space_ = frame->ColorSpace();
 
   return it->second.Duplicate();
 }
@@ -161,7 +196,7 @@ scoped_refptr<VideoFrame> InterprocessFramePool::WrapBuffer(
       static_cast<uint8_t*>(pooled_buffer.mapping.memory()),
       pooled_buffer.mapping.size(), base::TimeDelta());
   DCHECK(frame);
-  // Sanity-check the assumption being made in CloneHandleForDelivery():
+  // Sanity-check the assumption being made for SetMarkedBuffer():
   DCHECK_EQ(frame->data(0), pooled_buffer.mapping.memory());
   utilized_buffers_.emplace(frame.get(), std::move(pooled_buffer.region));
   frame->AddDestructionObserver(

components/viz/service/frame_sinks/video_capture/interprocess_frame_pool.h

@@ -42,14 +42,20 @@ class VIZ_SERVICE_EXPORT InterprocessFramePool {
       media::VideoPixelFormat format,
       const gfx::Size& size);
 
-  // Finds the last VideoFrame delivered, and if it has been returned back to
-  // this pool already, re-materializes it. Otherwise, null is returned. This is
-  // used when the client knows the content of the video frame has not changed
-  // and is trying to avoid having to re-populate a new VideoFrame with the same
-  // content.
-  scoped_refptr<media::VideoFrame> ResurrectLastVideoFrame(
-      media::VideoPixelFormat expected_format,
-      const gfx::Size& expected_size);
+  // Clients may call this using a frame previously returned by
+  // ReserveVideoFrame() to mark it (or its contents) for later resurrection via
+  // ResurrectOrDuplicateContentFromMarkedFrame(). Note that only one frame can
+  // be marked at a time. MarkFrame() will overwrite any existing mark.
+  void MarkFrame(const media::VideoFrame& frame);
+  void ClearFrameMarking();
+  bool HasMarkedFrameWithSize(const gfx::Size& size) const;
+
+  // If no frame is marked, returns nullptr. Otherwise, if the marked frame is
+  // not currently in use, returns the marked frame. If the marked frame is in
+  // use, reserves a new frame and copies the contents of the marked frame to
+  // the newly reserved one. That last case may still return nullptr if the pool
+  // is fully utilized.
+  scoped_refptr<media::VideoFrame> ResurrectOrDuplicateContentFromMarkedFrame();
 
   // Returns a cloned handle to the shared memory backing |frame| and its size
   // in bytes. Note that the client should not allow the ref-count of the
@@ -99,13 +105,12 @@ class VIZ_SERVICE_EXPORT InterprocessFramePool {
   base::flat_map<const media::VideoFrame*, base::ReadOnlySharedMemoryRegion>
       utilized_buffers_;
 
-  // The pointer to the mapped memory of the buffer that was last delivered,
-  // along with its format and size. ResurrectLastVideoFrame() uses this
-  // information to locate and confirm that a prior frame can be resurrected.
-  const void* resurrectable_buffer_memory_ = nullptr;
-  media::VideoPixelFormat last_delivered_format_ = media::PIXEL_FORMAT_UNKNOWN;
-  gfx::Size last_delivered_size_;
-  gfx::ColorSpace last_delivered_color_space_;
+  // The pointer to the mapped memory of the buffer that was set as "marked"
+  // via a call to SetMarkedBuffer().
+  const void* marked_frame_buffer_ = nullptr;
+  gfx::Size marked_frame_size_;
+  gfx::ColorSpace marked_frame_color_space_;
+  media::VideoPixelFormat marked_frame_pixel_format_;
 
   // The time at which the last shared memory allocation or mapping failed.
   base::TimeTicks last_fail_log_time_;

components/viz/service/frame_sinks/video_capture/interprocess_frame_pool_unittest.cc

@@ -143,70 +143,154 @@ bool PlanesAreFilledWithValues(const VideoFrame& frame, const uint8_t* values) {
   return true;
 }
 
-TEST(InterprocessFramePoolTest, ResurrectsDeliveredFramesOnly) {
+TEST(InterprocessFramePoolTest, ResurrectFrameThatIsNotInUse) {
   InterprocessFramePool pool(2);
+  const gfx::ColorSpace kArbitraryColorSpace = gfx::ColorSpace::CreateREC709();
 
-  // Reserve a frame, populate it, but release it before delivery.
+  // Reserve a frame, populate it, mark it, and release it.
   scoped_refptr<media::VideoFrame> frame =
       pool.ReserveVideoFrame(kFormat, kSize);
   ASSERT_TRUE(frame);
-  media::FillYUV(frame.get(), 0x11, 0x22, 0x33);
-  frame = nullptr;  // Returns frame to pool.
-
-  // The pool should fail to resurrect the last frame because it was never
-  // delivered.
-  frame = pool.ResurrectLastVideoFrame(kFormat, kSize);
-  ASSERT_FALSE(frame);
-
-  // Reserve a frame and populate it with different color values; only this
-  // time, signal that it will be delivered before releasing it.
-  frame = pool.ReserveVideoFrame(kFormat, kSize);
-  ASSERT_TRUE(frame);
-  const uint8_t kValues[3] = {0x44, 0x55, 0x66};
+  const uint8_t kValues[3] = {0x11, 0x22, 0x33};
   media::FillYUV(frame.get(), kValues[0], kValues[1], kValues[2]);
-  {
-    auto handle = pool.CloneHandleForDelivery(frame.get());
-    ExpectValidHandleForDelivery(handle);
-  }
+  frame->set_color_space(kArbitraryColorSpace);
+  pool.MarkFrame(*frame);
   frame = nullptr;  // Returns frame to pool.
 
-  // Confirm that the last frame can be resurrected repeatedly.
+  ASSERT_TRUE(pool.HasMarkedFrameWithSize(kSize));
+  const gfx::Size kDifferentSize(kSize.width() - 2, kSize.height() + 2);
+  ASSERT_FALSE(pool.HasMarkedFrameWithSize(kDifferentSize));
+
+  // Resurrect the frame and expect it to still have the same content, size,
+  // format, and color space. Release and repeat that a few times.
   for (int i = 0; i < 3; ++i) {
-    frame = pool.ResurrectLastVideoFrame(kFormat, kSize);
+    frame = pool.ResurrectOrDuplicateContentFromMarkedFrame();
     ASSERT_TRUE(frame);
+    ASSERT_EQ(kFormat, frame->format());
+    ASSERT_EQ(kSize, frame->coded_size());
+    ASSERT_EQ(kSize, frame->visible_rect().size());
+    ASSERT_EQ(kSize, frame->natural_size());
+    ASSERT_EQ(kArbitraryColorSpace, frame->ColorSpace());
     ASSERT_TRUE(PlanesAreFilledWithValues(*frame, kValues));
-    frame = nullptr;  // Returns frame to pool.
+    frame = nullptr;
   }
+}
 
-  // A frame that is being delivered cannot be resurrected.
-  for (int i = 0; i < 2; ++i) {
-    if (i == 0) {  // Test this for a resurrected frame.
-      frame = pool.ResurrectLastVideoFrame(kFormat, kSize);
-      ASSERT_TRUE(frame);
-      ASSERT_TRUE(PlanesAreFilledWithValues(*frame, kValues));
-    } else {  // Test this for a normal frame.
-      frame = pool.ReserveVideoFrame(kFormat, kSize);
-      ASSERT_TRUE(frame);
-      media::FillYUV(frame.get(), 0x77, 0x88, 0x99);
-    }
-    {
-      auto handle = pool.CloneHandleForDelivery(frame.get());
-      ExpectValidHandleForDelivery(handle);
-    }
-    scoped_refptr<media::VideoFrame> should_be_null =
-        pool.ResurrectLastVideoFrame(kFormat, kSize);
-    ASSERT_FALSE(should_be_null);
-    frame = nullptr;  // Returns frame to pool.
-  }
+TEST(InterprocessFramePoolTest, ResurrectContentFromFrameThatIsStillInUse) {
+  InterprocessFramePool pool(2);
+  const gfx::ColorSpace kArbitraryColorSpace = gfx::ColorSpace::CreateREC709();
 
-  // Finally, reserve a frame, populate it, and don't deliver it. Expect that,
-  // still, undelivered frames cannot be resurrected.
-  frame = pool.ReserveVideoFrame(kFormat, kSize);
+  // Reserve a frame, populate it, mark it, and hold on to it.
+  scoped_refptr<media::VideoFrame> frame =
+      pool.ReserveVideoFrame(kFormat, kSize);
   ASSERT_TRUE(frame);
-  media::FillYUV(frame.get(), 0xaa, 0xbb, 0xcc);
-  frame = nullptr;  // Returns frame to pool.
-  frame = pool.ResurrectLastVideoFrame(kFormat, kSize);
+  const uint8_t kValues[3] = {0x11, 0x22, 0x33};
+  media::FillYUV(frame.get(), kValues[0], kValues[1], kValues[2]);
+  frame->set_color_space(kArbitraryColorSpace);
+  pool.MarkFrame(*frame);
+
+  ASSERT_TRUE(pool.HasMarkedFrameWithSize(kSize));
+  const gfx::Size kDifferentSize(kSize.width() - 2, kSize.height() + 2);
+  ASSERT_FALSE(pool.HasMarkedFrameWithSize(kDifferentSize));
+
+  scoped_refptr<media::VideoFrame> frame2 =
+      pool.ResurrectOrDuplicateContentFromMarkedFrame();
+  ASSERT_TRUE(frame2);
+  ASSERT_NE(frame, frame2);
+  ASSERT_NE(frame->data(0), frame2->data(0));
+  ASSERT_EQ(kFormat, frame2->format());
+  ASSERT_EQ(kSize, frame2->coded_size());
+  ASSERT_EQ(kSize, frame2->visible_rect().size());
+  ASSERT_EQ(kSize, frame2->natural_size());
+  ASSERT_EQ(kArbitraryColorSpace, frame2->ColorSpace());
+  ASSERT_TRUE(PlanesAreFilledWithValues(*frame2, kValues));
+}
+
+TEST(InterprocessFramePoolTest, ResurrectWhenAtCapacity) {
+  InterprocessFramePool pool(2);
+  const gfx::ColorSpace kArbitraryColorSpace = gfx::ColorSpace::CreateREC709();
+
+  // Reserve two frames and hold on to them
+  scoped_refptr<media::VideoFrame> frame1 =
+      pool.ReserveVideoFrame(kFormat, kSize);
+  scoped_refptr<media::VideoFrame> frame2 =
+      pool.ReserveVideoFrame(kFormat, kSize);
+  ASSERT_TRUE(frame1);
+  ASSERT_TRUE(frame2);
+  // Fill and mark one of them
+  const uint8_t kValues[3] = {0x11, 0x22, 0x33};
+  media::FillYUV(frame1.get(), kValues[0], kValues[1], kValues[2]);
+  frame1->set_color_space(kArbitraryColorSpace);
+  pool.MarkFrame(*frame1);
+
+  // Attempt to resurrect. This should fail, because the pool is already at
+  // capacity.
+  scoped_refptr<media::VideoFrame> frame3 =
+      pool.ResurrectOrDuplicateContentFromMarkedFrame();
+  ASSERT_FALSE(frame3);
+
+  // Release the first frame
+  frame1 = nullptr;
+
+  // Now, resurrecting should work again.
+  frame3 = pool.ResurrectOrDuplicateContentFromMarkedFrame();
+  ASSERT_TRUE(frame3);
+  ASSERT_EQ(kFormat, frame3->format());
+  ASSERT_EQ(kSize, frame3->coded_size());
+  ASSERT_EQ(kArbitraryColorSpace, frame3->ColorSpace());
+  ASSERT_TRUE(PlanesAreFilledWithValues(*frame3, kValues));
+}
+
+TEST(InterprocessFramePoolTest, ResurrectWhenNoFrameMarked) {
+  InterprocessFramePool pool(2);
+
+  // Attempt to resurrect before any frame was ever reserved.
+  scoped_refptr<media::VideoFrame> frame =
+      pool.ResurrectOrDuplicateContentFromMarkedFrame();
   ASSERT_FALSE(frame);
+
+  // Reserve a frame and release it without marking it.
+  scoped_refptr<media::VideoFrame> frame2 =
+      pool.ReserveVideoFrame(kFormat, kSize);
+  ASSERT_TRUE(frame2);
+  frame2 = nullptr;  // Returns frame to pool.
+
+  // Attempt to resurrect. This should fail, because no frame was marked.
+  scoped_refptr<media::VideoFrame> frame3 =
+      pool.ResurrectOrDuplicateContentFromMarkedFrame();
+  ASSERT_FALSE(frame3);
+}
+
+TEST(InterprocessFramePoolTest, FrameMarkingIsLostWhenBufferIsReallocated) {
+  InterprocessFramePool pool(2);
+
+  // Reserve enough frames to hit capacity.
+  scoped_refptr<media::VideoFrame> frame1 =
+      pool.ReserveVideoFrame(kFormat, kSize);
+  scoped_refptr<media::VideoFrame> frame2 =
+      pool.ReserveVideoFrame(kFormat, kSize);
+  ASSERT_TRUE(frame1);
+  ASSERT_TRUE(frame2);
+
+  // Mark one of them
+  pool.MarkFrame(*frame1);
+  ASSERT_TRUE(pool.HasMarkedFrameWithSize(kSize));
+
+  // Release all frames
+  frame1 = nullptr;
+  frame2 = nullptr;
+
+  // Reserve all frames again but this time request a bigger size.
+  // This should lead to all buffers being reallocated and the marking being
+  // lost.
+  gfx::Size kBiggerSize(kSize.width() + 2, kSize.height() + 2);
+  frame1 = pool.ReserveVideoFrame(kFormat, kBiggerSize);
+  frame2 = pool.ReserveVideoFrame(kFormat, kBiggerSize);
+  ASSERT_TRUE(frame1);
+  ASSERT_TRUE(frame2);
+
+  ASSERT_FALSE(pool.HasMarkedFrameWithSize(kSize));
+  ASSERT_FALSE(pool.HasMarkedFrameWithSize(kBiggerSize));
 }
 
 TEST(InterprocessFramePoolTest, ReportsCorrectUtilization) {
@@ -219,7 +303,7 @@ TEST(InterprocessFramePoolTest, ReportsCorrectUtilization) {
     // Reserve the frame and expect 1/2 the pool to be utilized.
     scoped_refptr<media::VideoFrame> frame =
         (i == 0) ? pool.ReserveVideoFrame(kFormat, kSize)
-                 : pool.ResurrectLastVideoFrame(kFormat, kSize);
+                 : pool.ResurrectOrDuplicateContentFromMarkedFrame();
     ASSERT_TRUE(frame);
     ASSERT_EQ(0.5f, pool.GetUtilization());
 
@@ -231,6 +315,9 @@ TEST(InterprocessFramePoolTest, ReportsCorrectUtilization) {
     }
     ASSERT_EQ(0.5f, pool.GetUtilization());
 
+    // Mark the frame for later resurrection.
+    pool.MarkFrame(*frame);
+
     // Finally, release the frame to indicate it has been delivered and is no
     // longer in-use by downstream consumers. This should cause the utilization
     // to go back down to zero.