ozone/wayland: discard presentation feedbacks after the threshold.

The frame management mechanism expects that there are no more than
25 submitted frames at the same time. However, this number can
grow bigger if Wayland compositor doesn't send feedbacks (although
the protocol is supported). One of such examples is Sway that
runs on top of gnome/mutter, which doesn't expose wp_presentation
if the underlying system doesn't support that.

See bug reported to wlroots -
https://gitlab.freedesktop.org/wlroots/wlroots/-/issues/3340

PS also the number of the threshold has been lowered to avoid
FATAL:compositor_frame_sink_support.cc(734)] Check failed: pending_received_frame_times_.size() <= 25u (26 vs. 25)
errors.

(cherry picked from commit c0d0144)

Bug: 1253566, 1187077, 1243133, 1254273
Change-Id: Ib2b222f40588212342dd88b2cb880647024788f4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3322619
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Reviewed-by: Kramer Ge <fangzhoug@chromium.org>
Commit-Queue: Maksim Sisov <msisov@igalia.com>
Cr-Original-Commit-Position: refs/heads/main@{#950548}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3338605
Auto-Submit: Kramer Ge <fangzhoug@chromium.org>
Commit-Queue: Robert Kroeger <rjkroege@chromium.org>
Cr-Commit-Position: refs/branch-heads/4758@{#44}
Cr-Branched-From: 4a2cf4b-refs/heads/main@{#950365}

Author: msisov

ui/ozone/platform/wayland/host/wayland_frame_manager.cc

@@ -18,6 +18,8 @@ namespace ui {
 
 namespace {
 
+constexpr uint32_t kMaxNumberOfFrames = 20u;
+
 uint32_t GetPresentationKindFlags(uint32_t flags) {
   // Wayland spec has different meaning of VSync. In Chromium, VSync means to
   // update the begin frame vsync timing based on presentation feedback.
@@ -208,8 +210,8 @@ void WaylandFrameManager::PlayBackFrame(std::unique_ptr<WaylandFrame> frame) {
   if (!empty_frame)
     submitted_frames_.push_back(std::move(frame));
 
-  // This queue should be small - if not it's likely a bug.
-  DCHECK_LE(submitted_frames_.size(), 25u);
+  VerifyNumberOfSubmittedFrames();
+
   MaybeProcessSubmittedFrames();
 }
 
@@ -382,6 +384,31 @@ void WaylandFrameManager::OnPresentation(
   MaybeProcessSubmittedFrames();
 }
 
+void WaylandFrameManager::VerifyNumberOfSubmittedFrames() {
+  static constexpr uint32_t kLastThreeFrames = 3u;
+
+  // This queue should be small - if not it's likely a bug.
+  //
+  // Ideally this should be a DCHECK, but if the feedbacks are never sent (a bug
+  // in the server, everything crashes).
+  if (submitted_frames_.size() >= kMaxNumberOfFrames) {
+    LOG(ERROR)
+        << "The server has buggy presentation feedback. Discarding all "
+           "presentation feedback requests in all frames except the last "
+        << kLastThreeFrames << ".";
+    // Leave three last frames in case if the server restores its behavior
+    // (unlikely).
+    for (auto it = submitted_frames_.begin();
+         it < (submitted_frames_.end() - kLastThreeFrames); it++) {
+      if (!(*it)->submission_acked || !(*it)->pending_feedback)
+        break;
+      DCHECK(!(*it)->feedback.has_value());
+      (*it)->feedback = gfx::PresentationFeedback::Failure();
+      (*it)->pending_feedback.reset();
+    }
+  }
+}
+
 void WaylandFrameManager::OnExplicitBufferRelease(
     WaylandSurface* surface,
     struct wl_buffer* wl_buffer,
@@ -455,8 +482,6 @@ void WaylandFrameManager::MaybeProcessSubmittedFrames() {
                              gfx::GpuFenceHandle());
   }
 
-  CHECK_LT(submitted_frames_.size(), 25u);
-
   // Buffers may be released out of order, but we need to provide the
   // guarantee that OnSubmission will be called in order of buffer submission.
   for (auto iter = submitted_frames_.begin();
@@ -504,6 +529,8 @@ void WaylandFrameManager::MaybeProcessSubmittedFrames() {
     DCHECK(submitted_frames_.front()->submission_acked);
     submitted_frames_.pop_front();
   }
+
+  DCHECK_LE(submitted_frames_.size(), kMaxNumberOfFrames);
 }
 
 void WaylandFrameManager::ProcessOldSubmittedFrame(

ui/ozone/platform/wayland/host/wayland_frame_manager.h

@@ -156,6 +156,10 @@ class WaylandFrameManager {
                       const gfx::PresentationFeedback& feedback,
                       bool discarded = false);
 
+  // Verifies the number of submitted frames and discards pending presentation
+  // feedbacks if the number is too big.
+  void VerifyNumberOfSubmittedFrames();
+
   WaylandWindow* const window_;
 
   // When RecordFrame() is called, a Frame is pushed to |pending_frames_|. See

ui/ozone/platform/wayland/wayland_buffer_manager_unittest.cc

@@ -19,6 +19,7 @@
 #include "ui/gfx/gpu_fence_handle.h"
 #include "ui/gfx/linux/drm_util_linux.h"
 #include "ui/gfx/overlay_priority_hint.h"
+#include "ui/gfx/presentation_feedback.h"
 #include "ui/ozone/platform/wayland/gpu/wayland_buffer_manager_gpu.h"
 #include "ui/ozone/platform/wayland/gpu/wayland_surface_gpu.h"
 #include "ui/ozone/platform/wayland/host/wayland_buffer_manager_host.h"
@@ -2113,6 +2114,101 @@ TEST_P(WaylandBufferManagerTest, CanSetRoundedCorners) {
   }
 }
 
+// Verifies that there are no more than certain number of submitted frames that
+// wait presentation feedbacks. If the number of pending frames hit the
+// threshold, the feedbacks are marked as failed and discarded. See the comments
+// below in the test.
+TEST_P(WaylandBufferManagerTest, FeedbacksAreDiscardedIfClientMisbehaves) {
+  auto* mock_wp_presentation = server_.EnsureWpPresentation();
+  ASSERT_TRUE(mock_wp_presentation);
+
+  // 2 buffers are enough.
+  constexpr uint32_t kBufferId1 = 1;
+  constexpr uint32_t kBufferId2 = 2;
+
+  const gfx::AcceleratedWidget widget = window_->GetWidget();
+  const gfx::Rect bounds = gfx::Rect({0, 0}, kDefaultSize);
+  window_->SetBounds(bounds);
+
+  MockSurfaceGpu mock_surface_gpu(buffer_manager_gpu_.get(), widget_);
+
+  auto* linux_dmabuf = server_.zwp_linux_dmabuf_v1();
+  EXPECT_CALL(*linux_dmabuf, CreateParams(_, _, _)).Times(2);
+  CreateDmabufBasedBufferAndSetTerminateExpectation(false /*fail*/, kBufferId1);
+  CreateDmabufBasedBufferAndSetTerminateExpectation(false /*fail*/, kBufferId2);
+
+  Sync();
+
+  ProcessCreatedBufferResourcesWithExpectation(2u /* expected size */,
+                                               false /* fail */);
+
+  auto* mock_surface = server_.GetObject<wl::MockSurface>(
+      window_->root_surface()->GetSurfaceId());
+
+  // There will be 235 frames/commits.
+  constexpr uint32_t kNumberOfCommits = 235u;
+  EXPECT_CALL(*mock_surface, Attach(_, _, _)).Times(kNumberOfCommits);
+  EXPECT_CALL(*mock_surface, Frame(_)).Times(kNumberOfCommits);
+  EXPECT_CALL(*mock_wp_presentation, Feedback(_, _, _, _))
+      .Times(kNumberOfCommits);
+  EXPECT_CALL(*mock_surface, Commit()).Times(kNumberOfCommits);
+
+  // The presentation feedbacks should fail after first 20 commits (that's the
+  // threshold that WaylandFrameManager maintains). Next, the presentation
+  // feedbacks will fail every consequent 17 commits as 3 frames out of 20
+  // previous frames that WaylandFrameManager stores are always preserved in
+  // case if the client restores the behavior (which is very unlikely).
+  uint32_t expect_presentation_failure_on_commit_seq = 20u;
+  // Chooses the next buffer id that should be committed.
+  uint32_t next_buffer_id_commit = 0;
+  // Specifies the expected number of failing feedbacks if the client
+  // misbehaves.
+  constexpr uint32_t kExpectedFailedFeedbacks = 17u;
+  for (auto commit_seq = 1u; commit_seq <= kNumberOfCommits; commit_seq++) {
+    // All the other expectations must come in order.
+    if (next_buffer_id_commit == kBufferId1)
+      next_buffer_id_commit = kBufferId2;
+    else
+      next_buffer_id_commit = kBufferId1;
+
+    EXPECT_CALL(mock_surface_gpu, OnSubmission(next_buffer_id_commit,
+                                               gfx::SwapResult::SWAP_ACK, _))
+        .Times(1);
+
+    if (commit_seq % expect_presentation_failure_on_commit_seq == 0) {
+      EXPECT_CALL(mock_surface_gpu,
+                  OnPresentation(_, gfx::PresentationFeedback::Failure()))
+          .Times(kExpectedFailedFeedbacks);
+      // See comment near |expect_presentation_failure_on_commit_seq|.
+      expect_presentation_failure_on_commit_seq += kExpectedFailedFeedbacks;
+    } else {
+      // The client misbehaves and doesn't send presentation feedbacks. The
+      // frame manager doesn't mark the feedbacks as failed until the threshold
+      // is hit.
+      EXPECT_CALL(mock_surface_gpu, OnPresentation(_, _)).Times(0);
+    }
+
+    buffer_manager_gpu_->CommitBuffer(widget, next_buffer_id_commit, bounds,
+                                      kDefaultScale, bounds);
+
+    Sync();
+
+    if (auto* buffer = mock_surface->prev_attached_buffer())
+      mock_surface->ReleaseBuffer(buffer);
+
+    wl_resource_destroy(mock_wp_presentation->ReleasePresentationCallback());
+
+    mock_surface->SendFrameCallback();
+
+    Sync();
+
+    testing::Mock::VerifyAndClearExpectations(&mock_surface_gpu);
+  }
+
+  DestroyBufferAndSetTerminateExpectation(widget, kBufferId1, false /*fail*/);
+  DestroyBufferAndSetTerminateExpectation(widget, kBufferId2, false /*fail*/);
+}
+
 INSTANTIATE_TEST_SUITE_P(XdgVersionStableTest,
                          WaylandBufferManagerTest,
                          Values(wl::ServerConfig{