Commit 19d2de0e authored by Maksim Sisov's avatar Maksim Sisov Committed by Commit Bot

Revert "ozone/wayland: Complete submission of a buffer submitted more than once"

This reverts commit a71959ba.

Reason for revert: causes crashes if buffers are submitted twice and Wayland compositor releases the buffer

Original change's description:
> ozone/wayland: Complete submission of a buffer submitted more than once
> 
> This CL fixes a corner use case that was caught by one of the users
> of Ozone/Wayland.
> 
> The problem appeared when the Chromium compositor sent a next frame into
> the same buffer that had just been submitted and a OnSubmission callback
> had been sent to the viz process located WaylandBufferManager. This
> happened only when the remote inspector was attached and the screencast
> was enabled.
> 
> In this case, Wayland compositor will not send an OnRelease callback
> and we should not wait for that. Instead, we must complete the
> submission immediately and set the |WaylandBuffer::released|
> member to be false. Once another buffer is committed, Wayland compositor
> will release the previous buffer.
> 
> Bug: 1029777
> Change-Id: Ib74c16f41f128298998bc9699f8d9ded3697cd43
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1946479
> Commit-Queue: Maksim Sisov <msisov@igalia.com>
> Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#720878}

TBR=rjkroege@chromium.org,msisov@igalia.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1029777
Change-Id: I14dc61f4dad186a17e5b657d87143cb61a24f48a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1950972Reviewed-by: default avatarMaksim Sisov <msisov@igalia.com>
Commit-Queue: Maksim Sisov <msisov@igalia.com>
Cr-Commit-Position: refs/heads/master@{#721586}
parent 81b320db
......@@ -219,20 +219,13 @@ class WaylandBufferManagerHost::Surface {
DCHECK(buffer && window_);
DCHECK(!pending_buffer_);
// Once the BufferRelease is called, the buffer will be released.
DCHECK(buffer->released);
buffer->released = false;
DCHECK(!submitted_buffer_);
submitted_buffer_ = buffer;
// if the same buffer has been submitted again right after the client
// received OnSubmission for that buffer, just verify that the buffer has
// not been released by Wayland compositor, which is correct.
if (prev_submitted_buffer_ != submitted_buffer_) {
// Once the BufferRelease is called, the buffer will be released.
DCHECK(buffer->released);
buffer->released = false;
} else {
DCHECK(!buffer->released);
}
AttachAndDamageBuffer(buffer);
SetupFrameCallback();
......@@ -253,12 +246,7 @@ class WaylandBufferManagerHost::Surface {
// If it was the very first frame, the surface has not had a back buffer
// before, and Wayland won't release the front buffer until next buffer is
// attached. Thus, notify about successful submission immediately.
//
// As said above, if the client submits the same buffer again, we must
// notify the client about the submission immediately as Wayland compositor
// is not going to send a release callback for a buffer committed more than
// once.
if (!prev_submitted_buffer_ || prev_submitted_buffer_ == submitted_buffer_)
if (!prev_submitted_buffer_)
CompleteSubmission();
return true;
......
......@@ -837,84 +837,6 @@ TEST_P(WaylandBufferManagerTest, DestroyedWindowNoSubmissionMultipleBuffers) {
DestroyBufferAndSetTerminateExpectation(widget, kBufferId2, false /*fail*/);
}
// This test verifies that submitting the buffer more than once results in
// OnSubmission callback as Wayland compositor is not supposed to release the
// buffer committed twice.
TEST_P(WaylandBufferManagerTest, SubmitSameBufferMultipleTimes) {
constexpr uint32_t kBufferId1 = 1;
constexpr uint32_t kBufferId2 = 2;
const gfx::AcceleratedWidget widget = window_->GetWidget();
const gfx::Rect bounds = window_->GetBounds();
MockSurfaceGpu mock_surface_gpu(buffer_manager_gpu_.get(), widget);
auto* linux_dmabuf = server_.zwp_linux_dmabuf_v1();
EXPECT_CALL(*linux_dmabuf, CreateParams(_, _, _)).Times(2);
CreateDmabufBasedBufferAndSetTerminateExpecation(false /*fail*/, kBufferId1);
CreateDmabufBasedBufferAndSetTerminateExpecation(false /*fail*/, kBufferId2);
Sync();
ProcessCreatedBufferResourcesWithExpectation(2u /* expected size */,
false /* fail */);
// All the other expectations must come in order.
::testing::InSequence sequence;
EXPECT_CALL(mock_surface_gpu,
OnSubmission(kBufferId1, gfx::SwapResult::SWAP_ACK))
.Times(1);
ASSERT_TRUE(!connection_->presentation());
EXPECT_CALL(mock_surface_gpu, OnPresentation(kBufferId1, _)).Times(1);
buffer_manager_gpu_->CommitBuffer(widget, kBufferId1, bounds);
Sync();
testing::Mock::VerifyAndClearExpectations(&mock_surface_gpu);
auto* mock_surface = server_.GetObject<wl::MockSurface>(widget);
mock_surface->SendFrameCallback();
Sync();
// Commit second buffer now.
buffer_manager_gpu_->CommitBuffer(widget, kBufferId2, bounds);
Sync();
EXPECT_CALL(mock_surface_gpu,
OnSubmission(kBufferId2, gfx::SwapResult::SWAP_ACK))
.Times(1);
EXPECT_CALL(mock_surface_gpu, OnPresentation(kBufferId2, _)).Times(1);
mock_surface->ReleasePrevAttachedBuffer();
mock_surface->SendFrameCallback();
Sync();
testing::Mock::VerifyAndClearExpectations(&mock_surface_gpu);
// Now, commit the buffer with the |kBufferId2| again and make sure the
// manager manually sends the submission callback as long as the compositor is
// not going to release a buffer as it was the same buffer submitted more than
// once.
EXPECT_CALL(mock_surface_gpu,
OnSubmission(kBufferId2, gfx::SwapResult::SWAP_ACK))
.Times(1);
EXPECT_CALL(mock_surface_gpu, OnPresentation(kBufferId2, _)).Times(1);
// Commit second buffer now.
buffer_manager_gpu_->CommitBuffer(widget, kBufferId2, bounds);
Sync();
testing::Mock::VerifyAndClearExpectations(&mock_surface_gpu);
DestroyBufferAndSetTerminateExpectation(widget, kBufferId1, false /*fail*/);
DestroyBufferAndSetTerminateExpectation(widget, kBufferId2, false /*fail*/);
}
INSTANTIATE_TEST_SUITE_P(XdgVersionV5Test,
WaylandBufferManagerTest,
::testing::Values(kXdgShellV5));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment