Remove dangling pointers in mock_surface [chromium/src : main]

0 views
Skip to first unread message

AbdAlRahman Gad (Gerrit)

unread,
Aug 26, 2025, 12:50:11 AMAug 26
to chromium...@chromium.org, max+watc...@igalia.com, nickdiego+wa...@igalia.com, ozone-...@chromium.org
Attention needed from Max Ihlenfeldt

AbdAlRahman Gad added 1 comment

Patchset-level comments
File-level comment, Patchset 1 (Latest):
AbdAlRahman Gad . resolved

I was trying to do something like this in the [Attach function](https://source.chromium.org/chromium/chromium/src/+/main:ui/ozone/platform/wayland/test/mock_surface.cc;l=15) but it didn't work well:

```
void Attach(wl_client* client,
wl_resource* resource,
wl_resource* buffer_resource,
int32_t x,
int32_t y) {
auto* surface = GetUserDataAs<MockSurface>(resource);
  std::vector<base::ScopedFD> empty_fds;
wl_resource* b_resource =
CreateResourceWithImpl<::testing::NiceMock<TestBuffer>>(
client, &wl_buffer_interface, 1, &kTestWlBufferImpl, 0,
base::BindOnce(&MockSurface::ResetBuffer, surface->GetWeakPtr(),
buffer_resource),
std::move(empty_fds));
  surface->AttachNewBuffer(b_resource, x, y);
}
```

I think I need help on how to fix these dangling pointers.

Open in Gerrit

Related details

Attention is currently required from:
  • Max Ihlenfeldt
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I6c7e56ab88af09b8a04997e10c1b7764a923c68d
Gerrit-Change-Number: 6883470
Gerrit-PatchSet: 1
Gerrit-Owner: AbdAlRahman Gad <ag...@igalia.com>
Gerrit-Reviewer: Max Ihlenfeldt <m...@igalia.com>
Gerrit-Attention: Max Ihlenfeldt <m...@igalia.com>
Gerrit-Comment-Date: Tue, 26 Aug 2025 04:49:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Max Ihlenfeldt (Gerrit)

unread,
Aug 26, 2025, 6:59:53 AMAug 26
to AbdAlRahman Gad, Chromium LUCI CQ, chromium...@chromium.org, max+watc...@igalia.com, nickdiego+wa...@igalia.com, ozone-...@chromium.org
Attention needed from AbdAlRahman Gad

Max Ihlenfeldt added 1 comment

Patchset-level comments
AbdAlRahman Gad . resolved

I was trying to do something like this in the [Attach function](https://source.chromium.org/chromium/chromium/src/+/main:ui/ozone/platform/wayland/test/mock_surface.cc;l=15) but it didn't work well:

```
void Attach(wl_client* client,
wl_resource* resource,
wl_resource* buffer_resource,
int32_t x,
int32_t y) {
auto* surface = GetUserDataAs<MockSurface>(resource);
  std::vector<base::ScopedFD> empty_fds;
wl_resource* b_resource =
CreateResourceWithImpl<::testing::NiceMock<TestBuffer>>(
client, &wl_buffer_interface, 1, &kTestWlBufferImpl, 0,
base::BindOnce(&MockSurface::ResetBuffer, surface->GetWeakPtr(),
buffer_resource),
std::move(empty_fds));
  surface->AttachNewBuffer(b_resource, x, y);
}
```

I think I need help on how to fix these dangling pointers.

Max Ihlenfeldt

I think a log of running a test that fails due to the dangling pointer with `WAYLAND_DEBUG=client` might help here, could you please gather one and post it (or a link to it, depending on how long it is) here?

Open in Gerrit

Related details

Attention is currently required from:
  • AbdAlRahman Gad
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I6c7e56ab88af09b8a04997e10c1b7764a923c68d
Gerrit-Change-Number: 6883470
Gerrit-PatchSet: 1
Gerrit-Owner: AbdAlRahman Gad <ag...@igalia.com>
Gerrit-Reviewer: Max Ihlenfeldt <m...@igalia.com>
Gerrit-Attention: AbdAlRahman Gad <ag...@igalia.com>
Gerrit-Comment-Date: Tue, 26 Aug 2025 10:59:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: AbdAlRahman Gad <ag...@igalia.com>
satisfied_requirement
unsatisfied_requirement
open
diffy

AbdAlRahman Gad (Gerrit)

unread,
Aug 27, 2025, 1:25:04 AMAug 27
to Chromium LUCI CQ, chromium...@chromium.org, max+watc...@igalia.com, nickdiego+wa...@igalia.com, ozone-...@chromium.org
Attention needed from Max Ihlenfeldt

AbdAlRahman Gad added 1 comment

Patchset-level comments
AbdAlRahman Gad . unresolved

I was trying to do something like this in the [Attach function](https://source.chromium.org/chromium/chromium/src/+/main:ui/ozone/platform/wayland/test/mock_surface.cc;l=15) but it didn't work well:

```
void Attach(wl_client* client,
wl_resource* resource,
wl_resource* buffer_resource,
int32_t x,
int32_t y) {
auto* surface = GetUserDataAs<MockSurface>(resource);
  std::vector<base::ScopedFD> empty_fds;
wl_resource* b_resource =
CreateResourceWithImpl<::testing::NiceMock<TestBuffer>>(
client, &wl_buffer_interface, 1, &kTestWlBufferImpl, 0,
base::BindOnce(&MockSurface::ResetBuffer, surface->GetWeakPtr(),
buffer_resource),
std::move(empty_fds));
  surface->AttachNewBuffer(b_resource, x, y);
}
```

I think I need help on how to fix these dangling pointers.

Max Ihlenfeldt

I think a log of running a test that fails due to the dangling pointer with `WAYLAND_DEBUG=client` might help here, could you please gather one and post it (or a link to it, depending on how long it is) here?

Attention is currently required from:
  • Max Ihlenfeldt
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I6c7e56ab88af09b8a04997e10c1b7764a923c68d
    Gerrit-Change-Number: 6883470
    Gerrit-PatchSet: 1
    Gerrit-Owner: AbdAlRahman Gad <ag...@igalia.com>
    Gerrit-Reviewer: Max Ihlenfeldt <m...@igalia.com>
    Gerrit-Attention: Max Ihlenfeldt <m...@igalia.com>
    Gerrit-Comment-Date: Wed, 27 Aug 2025 05:24:47 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Max Ihlenfeldt <m...@igalia.com>
    Comment-In-Reply-To: AbdAlRahman Gad <ag...@igalia.com>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Max Ihlenfeldt (Gerrit)

    unread,
    Aug 28, 2025, 4:45:54 AMAug 28
    to AbdAlRahman Gad, Kramer Ge, Chromium LUCI CQ, chromium...@chromium.org, max+watc...@igalia.com, nickdiego+wa...@igalia.com, ozone-...@chromium.org
    Attention needed from AbdAlRahman Gad and Kramer Ge

    Max Ihlenfeldt added 1 comment

    Patchset-level comments
    AbdAlRahman Gad . unresolved

    I was trying to do something like this in the [Attach function](https://source.chromium.org/chromium/chromium/src/+/main:ui/ozone/platform/wayland/test/mock_surface.cc;l=15) but it didn't work well:

    ```
    void Attach(wl_client* client,
    wl_resource* resource,
    wl_resource* buffer_resource,
    int32_t x,
    int32_t y) {
    auto* surface = GetUserDataAs<MockSurface>(resource);
      std::vector<base::ScopedFD> empty_fds;
    wl_resource* b_resource =
    CreateResourceWithImpl<::testing::NiceMock<TestBuffer>>(
    client, &wl_buffer_interface, 1, &kTestWlBufferImpl, 0,
    base::BindOnce(&MockSurface::ResetBuffer, surface->GetWeakPtr(),
    buffer_resource),
    std::move(empty_fds));
      surface->AttachNewBuffer(b_resource, x, y);
    }
    ```

    I think I need help on how to fix these dangling pointers.

    Max Ihlenfeldt

    I think a log of running a test that fails due to the dangling pointer with `WAYLAND_DEBUG=client` might help here, could you please gather one and post it (or a link to it, depending on how long it is) here?

    AbdAlRahman Gad

    Log: https://notes.igalia.com/6wAOuWy4RhWO8TmbgzAAgg

    Max Ihlenfeldt

    Hm, I think this might be non-trivial to do correctly without dangling pointers. Maybe we can use weak pointers, similar to https://crrev.com/c/6787208. Kramer, wdyt?

    Open in Gerrit

    Related details

    Attention is currently required from:
    • AbdAlRahman Gad
    • Kramer Ge
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I6c7e56ab88af09b8a04997e10c1b7764a923c68d
    Gerrit-Change-Number: 6883470
    Gerrit-PatchSet: 1
    Gerrit-Owner: AbdAlRahman Gad <ag...@igalia.com>
    Gerrit-Reviewer: Kramer Ge <fang...@chromium.org>
    Gerrit-Reviewer: Max Ihlenfeldt <m...@igalia.com>
    Gerrit-Attention: Kramer Ge <fang...@chromium.org>
    Gerrit-Attention: AbdAlRahman Gad <ag...@igalia.com>
    Gerrit-Comment-Date: Thu, 28 Aug 2025 08:45:38 +0000
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Kramer Ge (Gerrit)

    unread,
    Aug 28, 2025, 11:17:20 AMAug 28
    to AbdAlRahman Gad, Chromium LUCI CQ, chromium...@chromium.org, max+watc...@igalia.com, nickdiego+wa...@igalia.com, ozone-...@chromium.org
    Attention needed from AbdAlRahman Gad

    Kramer Ge added 1 comment

    Patchset-level comments
    Kramer Ge

    Yes you need to store the weakptr to test_buffers in `AttachNewBuffer` and get `wl_resource` from it indirectly when the these getters are called.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • AbdAlRahman Gad
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I6c7e56ab88af09b8a04997e10c1b7764a923c68d
    Gerrit-Change-Number: 6883470
    Gerrit-PatchSet: 1
    Gerrit-Owner: AbdAlRahman Gad <ag...@igalia.com>
    Gerrit-Reviewer: Kramer Ge <fang...@chromium.org>
    Gerrit-Reviewer: Max Ihlenfeldt <m...@igalia.com>
    Gerrit-Attention: AbdAlRahman Gad <ag...@igalia.com>
    Gerrit-Comment-Date: Thu, 28 Aug 2025 15:17:15 +0000
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    AbdAlRahman Gad (Gerrit)

    unread,
    Sep 1, 2025, 1:10:29 AMSep 1
    to Kramer Ge, Chromium LUCI CQ, chromium...@chromium.org, max+watc...@igalia.com, nickdiego+wa...@igalia.com, ozone-...@chromium.org
    Attention needed from Kramer Ge and Max Ihlenfeldt

    AbdAlRahman Gad added 1 comment

    File ui/ozone/platform/wayland/test/mock_surface.cc
    Line 154, Patchset 2 (Latest): attached_buffer_ = buffer_resource;
    AbdAlRahman Gad . unresolved

    Could you please me with how to assign `wl_resource* buffer_resource` to `base::WeakPtr<wl_resource> attached_buffer_`?

    Also, is my approach correct? I was going to use `base::WeakPtr<WaylandBufferHandle>` but it's using `wl_buffer` instead of `wl_resource` so I thought it's not the right thing to do.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Kramer Ge
    • Max Ihlenfeldt
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I6c7e56ab88af09b8a04997e10c1b7764a923c68d
    Gerrit-Change-Number: 6883470
    Gerrit-PatchSet: 2
    Gerrit-Owner: AbdAlRahman Gad <ag...@igalia.com>
    Gerrit-Reviewer: Kramer Ge <fang...@chromium.org>
    Gerrit-Reviewer: Max Ihlenfeldt <m...@igalia.com>
    Gerrit-Attention: Kramer Ge <fang...@chromium.org>
    Gerrit-Attention: Max Ihlenfeldt <m...@igalia.com>
    Gerrit-Comment-Date: Mon, 01 Sep 2025 05:10:14 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Kramer Ge (Gerrit)

    unread,
    Sep 2, 2025, 11:39:08 AMSep 2
    to AbdAlRahman Gad, Chromium LUCI CQ, chromium...@chromium.org, max+watc...@igalia.com, nickdiego+wa...@igalia.com, ozone-...@chromium.org
    Attention needed from AbdAlRahman Gad and Max Ihlenfeldt

    Kramer Ge added 1 comment

    File ui/ozone/platform/wayland/test/mock_surface.cc
    Line 154, Patchset 2 (Latest): attached_buffer_ = buffer_resource;
    AbdAlRahman Gad . unresolved

    Could you please me with how to assign `wl_resource* buffer_resource` to `base::WeakPtr<wl_resource> attached_buffer_`?

    Also, is my approach correct? I was going to use `base::WeakPtr<WaylandBufferHandle>` but it's using `wl_buffer` instead of `wl_resource` so I thought it's not the right thing to do.

    Kramer Ge

    You can use [TestBuffer](https://source.chromium.org/chromium/chromium/src/+/main:ui/ozone/platform/wayland/test/test_buffer.h;l=21;bpv=0;bpt=1), it's a `ServerObject`, whose lifecycle is controlled by the `wl_resource` in wayland if it's created from `CreateResourceWithImpl` [here](https://source.chromium.org/chromium/chromium/src/+/main:ui/ozone/platform/wayland/test/test_zwp_linux_buffer_params.cc;l=40;drc=391e98eeaee80326c3a9d9e9dd731570dba0981a;bpv=0;bpt=1). You can add a `WeakPtrFactory` to the `TestBuffer`, and the `MockSurface` would store a `weak_ptr` of the test_buffer and invoke `resource()` to get the `wl_resource`. To get `TestBuffer` from `resource`, you call `GetUserDataAs<TestBuffer>()`

    Open in Gerrit

    Related details

    Attention is currently required from:
    • AbdAlRahman Gad
    • Max Ihlenfeldt
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I6c7e56ab88af09b8a04997e10c1b7764a923c68d
    Gerrit-Change-Number: 6883470
    Gerrit-PatchSet: 2
    Gerrit-Owner: AbdAlRahman Gad <ag...@igalia.com>
    Gerrit-Reviewer: Kramer Ge <fang...@chromium.org>
    Gerrit-Reviewer: Max Ihlenfeldt <m...@igalia.com>
    Gerrit-Attention: Max Ihlenfeldt <m...@igalia.com>
    Gerrit-Attention: AbdAlRahman Gad <ag...@igalia.com>
    Gerrit-Comment-Date: Tue, 02 Sep 2025 15:39:03 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: AbdAlRahman Gad <ag...@igalia.com>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    AbdAlRahman Gad (Gerrit)

    unread,
    Sep 4, 2025, 9:35:48 AMSep 4
    to Kramer Ge, Chromium LUCI CQ, chromium...@chromium.org, max+watc...@igalia.com, nickdiego+wa...@igalia.com, ozone-...@chromium.org
    Attention needed from Kramer Ge and Max Ihlenfeldt

    AbdAlRahman Gad added 1 comment

    File ui/ozone/platform/wayland/test/mock_surface.cc
    Line 157, Patchset 3 (Latest): attached_buffer_ = GetUserDataAs<TestBuffer>(buffer_resource)->GetWeakPtr();
    AbdAlRahman Gad . unresolved

    Calling `GetWeakPtr()` fails here.


    A failed test example:
    ```
    [ RUN ] WaylandFrameManagerTest.UnblockFrames_OnTransitionToSwapWithoutCommit
    [2296185:2296185:WARNING:ui/ozone/platform/wayland/gpu/wayland_buffer_manager_gpu.cc:444] Failed to initialize drm render node handle.
    [2296185:2296185:WARNING:ui/ozone/platform/wayland/common/wayland_object.cc:119] Binding to wl_shm version 1 but version 2 is available.
    [2296185:2296185:WARNING:ui/ozone/platform/wayland/common/wayland_object.cc:119] Binding to zwp_pointer_gestures_v1 version 1 but version 3 is available.
    [2296185:2296185:WARNING:ui/ozone/platform/wayland/host/wayland_surface.cc:200] Server doesn't support wp_content_type_v1
    [2296185:2296185:WARNING:ui/ozone/platform/wayland/host/wayland_surface.cc:214] Server doesn't support wp_color_management_surface_v1.
    Received signal 11 SI_KERNEL000000000000
    Possibly a General Protection Fault, can be due to a non-canonical address dereference. See "Intel 64 and IA-32 Architectures Software Developer’s Manual", Volume 1, Section 3.3.7.1.
    #0 0x639ed07af869 base::debug::CollectStackTrace() [../../base/debug/stack_trace_posix.cc:1052:7]
    #1 0x639ed077e19a base::debug::StackTrace::StackTrace() [../../base/debug/stack_trace.cc:255:20]
    #2 0x639ed077e105 base::debug::StackTrace::StackTrace() [../../base/debug/stack_trace.cc:250:28]
    #3 0x639ed07af0d9 base::debug::(anonymous namespace)::StackDumpSignalHandler() [../../base/debug/stack_trace_posix.cc:487:3]
    #4 0x73d36fa45330 (/usr/lib/x86_64-linux-gnu/libc.so.6+0x4532f)
    #5 0x639ecdc7bd1b std::__Cr::__cxx_atomic_load<>() [gen/third_party/libc++/src/include/__atomic/support/c11.h:81:10]
    #6 0x639ecdc7bccb std::__Cr::__atomic_base<>::load() [gen/third_party/libc++/src/include/__atomic/atomic.h:70:12]
    #7 0x639ed051bbca base::AtomicRefCount::IsOne() [../../base/atomic_ref_count.h:52:42]
    #8 0x639ed051b8f5 base::subtle::RefCountedThreadSafeBase::HasOneRef() [../../base/memory/ref_counted.cc:25:21]
    #9 0x639ed05207fd base::internal::WeakReferenceOwner::HasRefs() [../../base/memory/weak_ptr.h:163:41]
    #10 0x639ed05200c0 base::internal::WeakReferenceOwner::GetRef() [../../base/memory/weak_ptr.cc:95:8]
    #11 0x639ecf5ce3b9 _ZN4base14WeakPtrFactoryIN2wl10TestBufferEE10GetWeakPtrEvQntsr3stdE10is_const_vIT_E [../../base/memory/weak_ptr.h:382:45]
    #12 0x639ecf5ce269 wl::TestBuffer::GetWeakPtr() [../../ui/ozone/platform/wayland/test/test_buffer.cc:17:28]
    #13 0x639ecf592952 wl::MockSurface::AttachNewBuffer() [../../ui/ozone/platform/wayland/test/mock_surface.cc:157:68]
    #14 0x639ecf591fbb wl::(anonymous namespace)::Attach() [../../ui/ozone/platform/wayland/test/mock_surface.cc:22:12]
    #15 0x639ed38f0405 ffi_call_unix64
    #16 0x639ed38ef959 ffi_call_int
    #17 0x639ecf530497 wl_closure_invoke [../../third_party/wayland/src/src/connection.c:1228:2]
    #18 0x639ecf5fa656 wl_client_connection_data [../../third_party/wayland/src/src/wayland-server.c:444:4]
    #19 0x639ecf5f7e1e wl_event_source_fd_dispatch [../../third_party/wayland/src/src/event-loop.c:113:9]
    #20 0x639ecf5f94be wl_event_loop_dispatch [../../third_party/wayland/src/src/event-loop.c:1105:4]
    #21 0x639ecf5e957a wl::TestWaylandServerThread::OnFileCanReadWithoutBlocking() [../../ui/ozone/platform/wayland/test/test_wayland_server_thread.cc:299:3]
    #22 0x639ed07d1124 base::MessagePumpEpoll::FdWatchController::OnFdReadable() [../../base/message_loop/message_pump_epoll.cc:761:13]
    #23 0x639ed07d0e8e base::MessagePumpEpoll::HandleEvent() [../../base/message_loop/message_pump_epoll.cc:669:17]
    #24 0x639ed07d09f7 base::MessagePumpEpoll::OnEpollEvent() [../../base/message_loop/message_pump_epoll.cc:615:7]
    #25 0x639ed07cf883 base::MessagePumpEpoll::WaitForEpollEvents() [../../base/message_loop/message_pump_epoll.cc:506:7]
    #26 0x639ed07cf172 base::MessagePumpEpoll::Run() [../../base/message_loop/message_pump_epoll.cc:285:5]
    #27 0x639ed0668837 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run() [../../base/task/sequence_manager/thread_controller_with_message_pump_impl.cc:647:12]
    #28 0x639ed058a1fb base::RunLoop::Run() [../../base/run_loop.cc:134:14]
    #29 0x639ed0713049 base::Thread::Run() [../../base/threading/thread.cc:361:13]
    #30 0x639ed0713482 base::Thread::ThreadMain() [../../base/threading/thread.cc:436:3]
    #31 0x639ed0769419 base::(anonymous namespace)::ThreadFunc() [../../base/threading/platform_thread_posix.cc:101:13]
    #32 0x73d36fa9caa4 (/usr/lib/x86_64-linux-gnu/libc.so.6+0x9caa3)
    #33 0x73d36fb29c3c (/usr/lib/x86_64-linux-gnu/libc.so.6+0x129c3b)
    r8: 0000000000000000 r9: 000013b400051f00 r10: 0000000000000000 r11: 0000639ecf591f80
    r12: 0000000000000000 r13: 000073d36c9f8b20 r14: 000073d36c9f8c68 r15: 000073d36c9f8b1c
    di: 0dd0fecaefbeadde si: 0000000000000002 bp: 000073d36c9f88b0 bx: 000073d36c9f8cf0
    dx: 0000000000000000 ax: 0dd0fecaefbeadde cx: 0000000000000000 sp: 000073d36c9f88b0
    ip: 0000639ecdc7bd1b efl: 0000000000010297 cgf: 002b000000000033 erf: 0000000000000000
    trp: 000000000000000d msk: 0000000000000000 cr2: 0000000000000000
    [end of stack trace]
    ```

    Is there something I should do in order to have a valid `WeakPtr` here?

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Kramer Ge
    • Max Ihlenfeldt
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I6c7e56ab88af09b8a04997e10c1b7764a923c68d
    Gerrit-Change-Number: 6883470
    Gerrit-PatchSet: 3
    Gerrit-Owner: AbdAlRahman Gad <ag...@igalia.com>
    Gerrit-Reviewer: Kramer Ge <fang...@chromium.org>
    Gerrit-Reviewer: Max Ihlenfeldt <m...@igalia.com>
    Gerrit-Attention: Kramer Ge <fang...@chromium.org>
    Gerrit-Attention: Max Ihlenfeldt <m...@igalia.com>
    Gerrit-Comment-Date: Thu, 04 Sep 2025 13:35:31 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Kramer Ge (Gerrit)

    unread,
    Sep 4, 2025, 12:04:31 PMSep 4
    to AbdAlRahman Gad, Kramer Ge, Chromium LUCI CQ, chromium...@chromium.org, max+watc...@igalia.com, nickdiego+wa...@igalia.com, ozone-...@chromium.org
    Attention needed from AbdAlRahman Gad, Kramer Ge and Max Ihlenfeldt

    Kramer Ge added 1 comment

    File ui/ozone/platform/wayland/test/mock_surface.cc
    Kramer Ge

    I guess it's because `ui/ozone/platform/wayland/test/test_wayland_server_thread.h` doesn't implement `wl_shm_pool`, implement it and have it do similar things as `MockZwpLinuxDmabufV1`. Although I'm not fully certain how we ended up with a non-null `buffer_resource` without implementing `wl_shm_pool`.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • AbdAlRahman Gad
    • Kramer Ge
    • Max Ihlenfeldt
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I6c7e56ab88af09b8a04997e10c1b7764a923c68d
    Gerrit-Change-Number: 6883470
    Gerrit-PatchSet: 3
    Gerrit-Owner: AbdAlRahman Gad <ag...@igalia.com>
    Gerrit-Reviewer: Kramer Ge <fang...@chromium.org>
    Gerrit-Reviewer: Max Ihlenfeldt <m...@igalia.com>
    Gerrit-CC: Kramer Ge <fang...@google.com>
    Gerrit-Attention: Kramer Ge <fang...@chromium.org>
    Gerrit-Attention: Max Ihlenfeldt <m...@igalia.com>
    Gerrit-Attention: AbdAlRahman Gad <ag...@igalia.com>
    Gerrit-Comment-Date: Thu, 04 Sep 2025 16:04:24 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: AbdAlRahman Gad <ag...@igalia.com>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    AbdAlRahman Gad (Gerrit)

    unread,
    Sep 27, 2025, 12:27:43 PM (10 days ago) Sep 27
    to Kramer Ge, Kramer Ge, Chromium LUCI CQ, chromium...@chromium.org, max+watc...@igalia.com, nickdiego+wa...@igalia.com, ozone-...@chromium.org
    Attention needed from Kramer Ge, Kramer Ge and Max Ihlenfeldt

    AbdAlRahman Gad added 1 comment

    File ui/ozone/platform/wayland/test/mock_surface.cc
    Line 157, Patchset 3: attached_buffer_ = GetUserDataAs<TestBuffer>(buffer_resource)->GetWeakPtr();
    AbdAlRahman Gad

    Could you please take a look at the latest patchset? I'm not sure what the next step should be.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Kramer Ge
    • Kramer Ge
    • Max Ihlenfeldt
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I6c7e56ab88af09b8a04997e10c1b7764a923c68d
    Gerrit-Change-Number: 6883470
    Gerrit-PatchSet: 4
    Gerrit-Owner: AbdAlRahman Gad <ag...@igalia.com>
    Gerrit-Reviewer: Kramer Ge <fang...@chromium.org>
    Gerrit-Reviewer: Max Ihlenfeldt <m...@igalia.com>
    Gerrit-CC: Kramer Ge <fang...@google.com>
    Gerrit-Attention: Kramer Ge <fang...@google.com>
    Gerrit-Attention: Kramer Ge <fang...@chromium.org>
    Gerrit-Attention: Max Ihlenfeldt <m...@igalia.com>
    Gerrit-Comment-Date: Sat, 27 Sep 2025 16:27:22 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: AbdAlRahman Gad <ag...@igalia.com>
    Comment-In-Reply-To: Kramer Ge <fang...@google.com>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Kramer Ge (Gerrit)

    unread,
    Sep 29, 2025, 3:34:11 PM (8 days ago) Sep 29
    to AbdAlRahman Gad, Kramer Ge, Chromium LUCI CQ, chromium...@chromium.org, max+watc...@igalia.com, nickdiego+wa...@igalia.com, ozone-...@chromium.org
    Attention needed from AbdAlRahman Gad, Kramer Ge and Max Ihlenfeldt

    Kramer Ge added 1 comment

    File ui/ozone/platform/wayland/test/mock_surface.cc
    Kramer Ge

    Instead of `MOCK` the `CreateBuffer()` method for `shm_pool`, it should actually produce a `wl_resource` for the buffer, like `test_zwp_linux_buffer_params.cc` `CreateCommon()`.

    The way `mock_zwp_linux_dmabuf.cc` uses `MOCK` is it has real implementation in the static wayland impl, that impl calls the `MOCK`ed create buffer function.

    Similarly, we need `shm_pool` to actually produce and store a buffer resource, instead of just mocking.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • AbdAlRahman Gad
    • Kramer Ge
    • Max Ihlenfeldt
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I6c7e56ab88af09b8a04997e10c1b7764a923c68d
    Gerrit-Change-Number: 6883470
    Gerrit-PatchSet: 4
    Gerrit-Owner: AbdAlRahman Gad <ag...@igalia.com>
    Gerrit-Reviewer: Kramer Ge <fang...@chromium.org>
    Gerrit-Reviewer: Max Ihlenfeldt <m...@igalia.com>
    Gerrit-CC: Kramer Ge <fang...@google.com>
    Gerrit-Attention: AbdAlRahman Gad <ag...@igalia.com>
    Gerrit-Attention: Kramer Ge <fang...@google.com>
    Gerrit-Attention: Max Ihlenfeldt <m...@igalia.com>
    Gerrit-Comment-Date: Mon, 29 Sep 2025 19:34:07 +0000
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    AbdAlRahman Gad (Gerrit)

    unread,
    Oct 1, 2025, 11:00:10 AM (6 days ago) Oct 1
    to Kramer Ge, Kramer Ge, Chromium LUCI CQ, chromium...@chromium.org, max+watc...@igalia.com, nickdiego+wa...@igalia.com, ozone-...@chromium.org
    Attention needed from Kramer Ge and Kramer Ge

    AbdAlRahman Gad added 1 comment

    File ui/ozone/platform/wayland/test/mock_surface.cc

    Could you please take a look at the latest change? Is that the right approach?

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Kramer Ge
    • Kramer Ge
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I6c7e56ab88af09b8a04997e10c1b7764a923c68d
    Gerrit-Change-Number: 6883470
    Gerrit-PatchSet: 5
    Gerrit-Owner: AbdAlRahman Gad <ag...@igalia.com>
    Gerrit-Reviewer: Kramer Ge <fang...@chromium.org>
    Gerrit-Reviewer: Max Ihlenfeldt <m...@igalia.com>
    Gerrit-CC: Kramer Ge <fang...@google.com>
    Gerrit-Attention: Kramer Ge <fang...@google.com>
    Gerrit-Attention: Kramer Ge <fang...@chromium.org>
    Gerrit-Comment-Date: Wed, 01 Oct 2025 14:59:57 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: AbdAlRahman Gad <ag...@igalia.com>
    Comment-In-Reply-To: Kramer Ge <fang...@google.com>
    Comment-In-Reply-To: Kramer Ge <fang...@chromium.org>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Kramer Ge (Gerrit)

    unread,
    Oct 1, 2025, 11:30:08 AM (6 days ago) Oct 1
    to AbdAlRahman Gad, Kramer Ge, Chromium LUCI CQ, chromium...@chromium.org, max+watc...@igalia.com, nickdiego+wa...@igalia.com, ozone-...@chromium.org
    Attention needed from AbdAlRahman Gad and Kramer Ge

    Kramer Ge added 2 comments

    File ui/ozone/platform/wayland/test/mock_shm_pool.cc
    Line 31, Patchset 5 (Latest): mock_shm_pool->SetBufferResource(buffer_resource);
    Kramer Ge . unresolved

    This is a shm pool that can have produce multiple different buffer_resources. So `SetBufferResource()` here does not reflect that, you also `std::move`d the `fds_` field, this caused `mock_shm_pool` to only be able to create at most 1 buffer.

    My suggestion would be to remove this call altogether, and initialize TestBuffer with an empty `fd` vector above.

    File ui/ozone/platform/wayland/test/mock_surface.cc
    Kramer Ge

    It's towards the right direction although there are some nuances

    Open in Gerrit

    Related details

    Attention is currently required from:
    • AbdAlRahman Gad
    • Kramer Ge
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I6c7e56ab88af09b8a04997e10c1b7764a923c68d
    Gerrit-Change-Number: 6883470
    Gerrit-PatchSet: 5
    Gerrit-Owner: AbdAlRahman Gad <ag...@igalia.com>
    Gerrit-Reviewer: Kramer Ge <fang...@chromium.org>
    Gerrit-Reviewer: Max Ihlenfeldt <m...@igalia.com>
    Gerrit-CC: Kramer Ge <fang...@google.com>
    Gerrit-Attention: AbdAlRahman Gad <ag...@igalia.com>
    Gerrit-Attention: Kramer Ge <fang...@google.com>
    Gerrit-Comment-Date: Wed, 01 Oct 2025 15:30:03 +0000
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    AbdAlRahman Gad (Gerrit)

    unread,
    Oct 1, 2025, 12:52:23 PM (6 days ago) Oct 1
    to Kramer Ge, Kramer Ge, Chromium LUCI CQ, chromium...@chromium.org, max+watc...@igalia.com, nickdiego+wa...@igalia.com, ozone-...@chromium.org
    Attention needed from Kramer Ge and Kramer Ge

    AbdAlRahman Gad added 1 comment

    File ui/ozone/platform/wayland/test/mock_shm_pool.cc
    Line 31, Patchset 5: mock_shm_pool->SetBufferResource(buffer_resource);
    Kramer Ge . unresolved

    This is a shm pool that can have produce multiple different buffer_resources. So `SetBufferResource()` here does not reflect that, you also `std::move`d the `fds_` field, this caused `mock_shm_pool` to only be able to create at most 1 buffer.

    My suggestion would be to remove this call altogether, and initialize TestBuffer with an empty `fd` vector above.

    AbdAlRahman Gad

    Done. Could you please take a look?

    What would you suggest we do next?

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Kramer Ge
    • Kramer Ge
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I6c7e56ab88af09b8a04997e10c1b7764a923c68d
    Gerrit-Change-Number: 6883470
    Gerrit-PatchSet: 6
    Gerrit-Owner: AbdAlRahman Gad <ag...@igalia.com>
    Gerrit-Reviewer: Kramer Ge <fang...@chromium.org>
    Gerrit-Reviewer: Max Ihlenfeldt <m...@igalia.com>
    Gerrit-CC: Kramer Ge <fang...@google.com>
    Gerrit-Attention: Kramer Ge <fang...@google.com>
    Gerrit-Attention: Kramer Ge <fang...@chromium.org>
    Gerrit-Comment-Date: Wed, 01 Oct 2025 16:52:07 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Kramer Ge <fang...@chromium.org>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Kramer Ge (Gerrit)

    unread,
    Oct 1, 2025, 2:15:01 PM (6 days ago) Oct 1
    to AbdAlRahman Gad, Kramer Ge, Chromium LUCI CQ, chromium...@chromium.org, max+watc...@igalia.com, nickdiego+wa...@igalia.com, ozone-...@chromium.org
    Attention needed from AbdAlRahman Gad and Kramer Ge

    Kramer Ge added 1 comment

    File ui/ozone/platform/wayland/test/mock_shm_pool.cc
    Line 31, Patchset 5: mock_shm_pool->SetBufferResource(buffer_resource);
    Kramer Ge . unresolved

    This is a shm pool that can have produce multiple different buffer_resources. So `SetBufferResource()` here does not reflect that, you also `std::move`d the `fds_` field, this caused `mock_shm_pool` to only be able to create at most 1 buffer.

    My suggestion would be to remove this call altogether, and initialize TestBuffer with an empty `fd` vector above.

    AbdAlRahman Gad

    Done. Could you please take a look?

    What would you suggest we do next?

    Kramer Ge

    Now your `std::vector<raw_ptr<wl_resource>> buffer_resources_` is dangling. It should be a list of weakptr to `TestBuffer`s.

    You may want to clean them up in some fashion when `TestBuffer` destructs, but I don't think it's immediately needed for tests to be passing.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • AbdAlRahman Gad
    • Kramer Ge
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I6c7e56ab88af09b8a04997e10c1b7764a923c68d
    Gerrit-Change-Number: 6883470
    Gerrit-PatchSet: 6
    Gerrit-Owner: AbdAlRahman Gad <ag...@igalia.com>
    Gerrit-Reviewer: Kramer Ge <fang...@chromium.org>
    Gerrit-Reviewer: Max Ihlenfeldt <m...@igalia.com>
    Gerrit-CC: Kramer Ge <fang...@google.com>
    Gerrit-Attention: AbdAlRahman Gad <ag...@igalia.com>
    Gerrit-Attention: Kramer Ge <fang...@google.com>
    Gerrit-Comment-Date: Wed, 01 Oct 2025 18:14:50 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: AbdAlRahman Gad <ag...@igalia.com>
    Comment-In-Reply-To: Kramer Ge <fang...@chromium.org>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    AbdAlRahman Gad (Gerrit)

    unread,
    Oct 1, 2025, 7:44:56 PM (6 days ago) Oct 1
    to Kramer Ge, Chromium LUCI CQ, chromium...@chromium.org, max+watc...@igalia.com, nickdiego+wa...@igalia.com, ozone-...@chromium.org
    Attention needed from Kramer Ge

    AbdAlRahman Gad added 1 comment

    File ui/ozone/platform/wayland/test/mock_shm_pool.cc
    Line 31, Patchset 5: mock_shm_pool->SetBufferResource(buffer_resource);
    Kramer Ge . unresolved

    This is a shm pool that can have produce multiple different buffer_resources. So `SetBufferResource()` here does not reflect that, you also `std::move`d the `fds_` field, this caused `mock_shm_pool` to only be able to create at most 1 buffer.

    My suggestion would be to remove this call altogether, and initialize TestBuffer with an empty `fd` vector above.

    AbdAlRahman Gad

    Done. Could you please take a look?

    What would you suggest we do next?

    Kramer Ge

    Now your `std::vector<raw_ptr<wl_resource>> buffer_resources_` is dangling. It should be a list of weakptr to `TestBuffer`s.

    You may want to clean them up in some fashion when `TestBuffer` destructs, but I don't think it's immediately needed for tests to be passing.

    AbdAlRahman Gad

    Done but the tests doesn't pass yet. Am I missing something?

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Kramer Ge
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I6c7e56ab88af09b8a04997e10c1b7764a923c68d
    Gerrit-Change-Number: 6883470
    Gerrit-PatchSet: 7
    Gerrit-Owner: AbdAlRahman Gad <ag...@igalia.com>
    Gerrit-Reviewer: Kramer Ge <fang...@chromium.org>
    Gerrit-Reviewer: Max Ihlenfeldt <m...@igalia.com>
    Gerrit-Attention: Kramer Ge <fang...@chromium.org>
    Gerrit-Comment-Date: Wed, 01 Oct 2025 23:44:36 +0000
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Kramer Ge (Gerrit)

    unread,
    Oct 2, 2025, 5:04:18 PM (5 days ago) Oct 2
    to AbdAlRahman Gad, Chromium LUCI CQ, chromium...@chromium.org, max+watc...@igalia.com, nickdiego+wa...@igalia.com, ozone-...@chromium.org
    Attention needed from AbdAlRahman Gad

    Kramer Ge added 1 comment

    File ui/ozone/platform/wayland/test/mock_shm_pool.cc
    Line 31, Patchset 5: mock_shm_pool->SetBufferResource(buffer_resource);
    Kramer Ge . unresolved

    This is a shm pool that can have produce multiple different buffer_resources. So `SetBufferResource()` here does not reflect that, you also `std::move`d the `fds_` field, this caused `mock_shm_pool` to only be able to create at most 1 buffer.

    My suggestion would be to remove this call altogether, and initialize TestBuffer with an empty `fd` vector above.

    AbdAlRahman Gad

    Done. Could you please take a look?

    What would you suggest we do next?

    Kramer Ge

    Now your `std::vector<raw_ptr<wl_resource>> buffer_resources_` is dangling. It should be a list of weakptr to `TestBuffer`s.

    You may want to clean them up in some fashion when `TestBuffer` destructs, but I don't think it's immediately needed for tests to be passing.

    AbdAlRahman Gad

    Done but the tests doesn't pass yet. Am I missing something?

    Kramer Ge

    I'm not sure. What is the failure?

    Open in Gerrit

    Related details

    Attention is currently required from:
    • AbdAlRahman Gad
    Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement is not satisfiedCode-Owners
      • requirement is not satisfiedCode-Review
      • requirement is not satisfiedNo-Unresolved-Comments
      • requirement is not satisfiedReview-Enforcement
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I6c7e56ab88af09b8a04997e10c1b7764a923c68d
      Gerrit-Change-Number: 6883470
      Gerrit-PatchSet: 7
      Gerrit-Owner: AbdAlRahman Gad <ag...@igalia.com>
      Gerrit-Reviewer: Kramer Ge <fang...@chromium.org>
      Gerrit-Reviewer: Max Ihlenfeldt <m...@igalia.com>
      Gerrit-Attention: AbdAlRahman Gad <ag...@igalia.com>
      Gerrit-Comment-Date: Thu, 02 Oct 2025 21:04:15 +0000
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      AbdAlRahman Gad (Gerrit)

      unread,
      Oct 3, 2025, 12:15:39 PM (4 days ago) Oct 3
      to Kramer Ge, Chromium LUCI CQ, chromium...@chromium.org, max+watc...@igalia.com, nickdiego+wa...@igalia.com, ozone-...@chromium.org
      Attention needed from Kramer Ge

      AbdAlRahman Gad added 1 comment

      File ui/ozone/platform/wayland/test/mock_shm_pool.cc
      Line 31, Patchset 5: mock_shm_pool->SetBufferResource(buffer_resource);
      Kramer Ge . unresolved

      This is a shm pool that can have produce multiple different buffer_resources. So `SetBufferResource()` here does not reflect that, you also `std::move`d the `fds_` field, this caused `mock_shm_pool` to only be able to create at most 1 buffer.

      My suggestion would be to remove this call altogether, and initialize TestBuffer with an empty `fd` vector above.

      AbdAlRahman Gad

      Done. Could you please take a look?

      What would you suggest we do next?

      Kramer Ge

      Now your `std::vector<raw_ptr<wl_resource>> buffer_resources_` is dangling. It should be a list of weakptr to `TestBuffer`s.

      You may want to clean them up in some fashion when `TestBuffer` destructs, but I don't think it's immediately needed for tests to be passing.

      AbdAlRahman Gad

      Done but the tests doesn't pass yet. Am I missing something?

      Kramer Ge

      I'm not sure. What is the failure?

      AbdAlRahman Gad

      It's actually the same as before implementing `mock_shm_pool`. I ran the CQ so you can see them in: https://chromium-review.googlesource.com/c/chromium/src/+/6883470?tab=checks

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Kramer Ge
      Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement is not satisfiedCode-Owners
      • requirement is not satisfiedCode-Review
      • requirement is not satisfiedNo-Unresolved-Comments
      • requirement is not satisfiedReview-Enforcement
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I6c7e56ab88af09b8a04997e10c1b7764a923c68d
      Gerrit-Change-Number: 6883470
      Gerrit-PatchSet: 7
      Gerrit-Owner: AbdAlRahman Gad <ag...@igalia.com>
      Gerrit-Reviewer: AbdAlRahman Gad <ag...@igalia.com>
      Gerrit-Reviewer: Kramer Ge <fang...@chromium.org>
      Gerrit-Reviewer: Max Ihlenfeldt <m...@igalia.com>
      Gerrit-Attention: Kramer Ge <fang...@chromium.org>
      Gerrit-Comment-Date: Fri, 03 Oct 2025 16:15:21 +0000
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Kramer Ge (Gerrit)

      unread,
      Oct 3, 2025, 1:35:20 PM (4 days ago) Oct 3
      to AbdAlRahman Gad, Chromium LUCI CQ, chromium...@chromium.org, max+watc...@igalia.com, nickdiego+wa...@igalia.com, ozone-...@chromium.org
      Attention needed from AbdAlRahman Gad

      Kramer Ge added 1 comment

      File ui/ozone/platform/wayland/test/mock_shm_pool.cc
      Line 31, Patchset 5: mock_shm_pool->SetBufferResource(buffer_resource);
      Kramer Ge . unresolved

      This is a shm pool that can have produce multiple different buffer_resources. So `SetBufferResource()` here does not reflect that, you also `std::move`d the `fds_` field, this caused `mock_shm_pool` to only be able to create at most 1 buffer.

      My suggestion would be to remove this call altogether, and initialize TestBuffer with an empty `fd` vector above.

      AbdAlRahman Gad

      Done. Could you please take a look?

      What would you suggest we do next?

      Kramer Ge

      Now your `std::vector<raw_ptr<wl_resource>> buffer_resources_` is dangling. It should be a list of weakptr to `TestBuffer`s.

      You may want to clean them up in some fashion when `TestBuffer` destructs, but I don't think it's immediately needed for tests to be passing.

      AbdAlRahman Gad

      Done but the tests doesn't pass yet. Am I missing something?

      Kramer Ge

      I'm not sure. What is the failure?

      AbdAlRahman Gad

      It's actually the same as before implementing `mock_shm_pool`. I ran the CQ so you can see them in: https://chromium-review.googlesource.com/c/chromium/src/+/6883470?tab=checks

      Kramer Ge

      `GetUserDataAs<TestBuffer>(buffer_resource)` is null, but I'm not sure why (probably because wl_buffers are destructed before attach), you need some printfs to figure them out.

      Open in Gerrit

      Related details

      Attention is currently required from:
      • AbdAlRahman Gad
      Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement is not satisfiedCode-Owners
      • requirement is not satisfiedCode-Review
      • requirement is not satisfiedNo-Unresolved-Comments
      • requirement is not satisfiedReview-Enforcement
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I6c7e56ab88af09b8a04997e10c1b7764a923c68d
      Gerrit-Change-Number: 6883470
      Gerrit-PatchSet: 7
      Gerrit-Owner: AbdAlRahman Gad <ag...@igalia.com>
      Gerrit-Reviewer: AbdAlRahman Gad <ag...@igalia.com>
      Gerrit-Reviewer: Kramer Ge <fang...@chromium.org>
      Gerrit-Reviewer: Max Ihlenfeldt <m...@igalia.com>
      Gerrit-Attention: AbdAlRahman Gad <ag...@igalia.com>
      Gerrit-Comment-Date: Fri, 03 Oct 2025 17:35:14 +0000
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy
      Reply all
      Reply to author
      Forward
      0 new messages