Implement wlr-foreign-toplevel-management-v1

[ddevault]

Chose to leave fullscreen unimplemented for now.

[RedSoxFan]

Segfaults whenever a toplevel is closed:

Backtrace

#0  0x00007fa155532672 in wl_event_loop_add_idle () at /lib64/libwayland-server.so.0
#1  0x00007fa155b8bc52 in toplevel_update_idle_source (toplevel=0x555d884bca20)
    at ../subprojects/wlroots/types/wlr_foreign_toplevel_management_v1.c:190
#2  0x00007fa155b8c328 in toplevel_send_state (toplevel=0x555d884bca20)
    at ../subprojects/wlroots/types/wlr_foreign_toplevel_management_v1.c:365
        states = {size = 0, alloc = 0, data = 0x4}
        r = true
        resource = 0x555d884bca08
#3  0x00007fa155b8c425 in wlr_foreign_toplevel_handle_v1_set_activated
    (toplevel=0x555d884bca20, activated=false)
    at ../subprojects/wlroots/types/wlr_foreign_toplevel_management_v1.c:395
#4  0x0000555d866740ab in view_set_activated (view=0x555d884bbc60, activated=false)
    at ../sway/tree/view.c:304
#5  0x0000555d86649011 in send_unfocus (con=0x555d884bcc60, data=0x555d87f10270)
    at ../sway/input/seat.c:849
#6  0x0000555d866787c2 in workspace_for_each_container (ws=0x555d87f2c1b0, f=
    0x555d86648fdf <send_unfocus>, data=0x555d87f10270) at ../sway/tree/workspace.c:599
        container = 0x555d884bcc60
        i = 0
#7  0x0000555d8664906e in seat_send_unfocus (node=0x555d87f2c1b0, seat=0x555d87f10270)
    at ../sway/input/seat.c:858
#8  0x0000555d8664931f in seat_set_focus (seat=0x555d87f10270, node=0x0)
    at ../sway/input/seat.c:927
        last_focus = 0x555d87f2c1b0
        last_workspace = 0x555d87f2c1b0
        new_workspace = 0x7fff3c3bcd80
        container = 0x555d87f2c1b0
        new_output = 0x7fff3c3bcd80
        new_output_last_ws = 0x555d86647405 <seat_get_focus_inactive_view+134>
#9  0x0000555d8664769b in handle_seat_node_destroy
    (listener=0x555d884bcb20, data=0x555d884bcc60) at ../sway/input/seat.c:246
        seat_node = 0x555d884bcb00
        seat = 0x555d87f10270
        node = 0x555d884bcc60
        parent = 0x555d87f4a7e0
        focus = 0x555d884bcc60
        needs_new_focus = true
        next_focus = 0x555d8845cc10
#10 0x0000555d8666cc27 in wl_signal_emit (signal=0x555d884bcc90, data=0x555d884bcc60)
    at /usr/include/wayland-server-core.h:472
        l = 0x555d884bcb20
        next = 0x555d884bcc90
#11 0x0000555d8666cff3 in container_begin_destroy (con=0x555d884bcc60)
    at ../sway/tree/container.c:100
#12 0x0000555d86675152 in view_unmap (view=0x555d884bbc60) at ../sway/tree/view.c:707
        parent = 0x0
        ws = 0x555d87f2c1b0
        seat = 0x7fff3c3bcf00
#13 0x0000555d8663f64b in handle_unmap (listener=0x555d884bbe60, data=0x555d884bb850)
    at ../sway/desktop/xdg_shell.c:397
        xdg_shell_view = 0x555d884bbc60
        view = 0x555d884bbc60
        __PRETTY_FUNCTION__ = "handle_unmap"
#14 0x00007fa155baa49a in wlr_signal_emit_safe (signal=0x555d884bb958, data=0x555d884bb850)
    at ../subprojects/wlroots/util/signal.c:29
        pos = 0x555d884bbe60
        l = 0x555d884bbe60
        cursor = 
          {link = {prev = 0x555d884bbe60, next = 0x7fff3c3bcf10}, notify = 0x7fa155baa3e4 <handle_noop>}
        end = 
          {link = {prev = 0x7fff3c3bcef0, next = 0x555d884bb958}, notify = 0x7fa155baa3e4 <handle_noop>}
#15 0x00007fa155b826fb in unmap_xdg_surface (surface=0x555d884bb850)
    at ../subprojects/wlroots/types/xdg_shell/wlr_xdg_surface.c:39
        __PRETTY_FUNCTION__ = "unmap_xdg_surface"
        popup = 0x555d884bb880
        popup_tmp = 0x555d884bb880
        configure = 0x7fff3c3bcfd0
        tmp = 0x0
#16 0x00007fa155b83599 in reset_xdg_surface (xdg_surface=0x555d884bb850)
    at ../subprojects/wlroots/types/xdg_shell/wlr_xdg_surface.c:453
#17 0x00007fa155b836e8 in destroy_xdg_surface (surface=0x555d884bb850)
    at ../subprojects/wlroots/types/xdg_shell/wlr_xdg_surface.c:492
#18 0x00007fa155b832c3 in xdg_surface_handle_surface_destroy
    (listener=0x555d884bb8e8, data=0x555d884bb4a0)
    at ../subprojects/wlroots/types/xdg_shell/wlr_xdg_surface.c:388
        xdg_surface = 0x555d884bb850
#19 0x00007fa155baa49a in wlr_signal_emit_safe (signal=0x555d884bb760, data=0x555d884bb4a0)
    at ../subprojects/wlroots/util/signal.c:29
        pos = 0x555d884bb8e8
        l = 0x555d884bb8e8
        cursor = 
          {link = {prev = 0x555d884bb8e8, next = 0x555d884bbee8}, notify = 0x7fa155baa3e4 <handle_noop>}
        end = 
          {link = {prev = 0x555d87f104d8, next = 0x555d884bb760}, notify = 0x7fa155baa3e4 <handle_noop>}
#20 0x00007fa155ba3929 in surface_handle_resource_destroy (resource=0x555d884bb7c0)
    at ../subprojects/wlroots/types/wlr_surface.c:567
        surface = 0x555d884bb4a0

[ddevault]

Good catch, thanks. Use-after-free issue.

[ddevault]

Note: this can be tested with @wmww's mate-panel branch here:

https://github.com/wmww/mate-panel/tree/wayland-snap

[wmww]

wayland-snap is indeed the correct MATE Panel branch. Sway disconnects clients with toplevels when mate panel from this branch is running and there are multiple toplevels. The branch in question switches focus between clients as fast as it can.

[johan-bjareholt]

Is there something blocking this PR to get merged?

[ddevault]

According to @wmww it fails under a stress test. I'm not necessarily opposed to merging it anyway if no one else is, but reproducing @wmww's issue and finding a fix is the next logical step.

[johan-bjareholt]

I've been testing both this branch and comparing my app using wlr-foreign-toplevel-management with how phosh uses it.

When I focus URxvt in phosh the appid and title is properly set, when I focus URxvt in sway with this branch the title is properly set but the appid is not. Evince works fine however. I have not investigated more than that yet though.

[progandy]

When I focus URxvt in phosh the appid and title is properly set, when I focus URxvt in sway with this branch the title is properly set but the appid is not.

So far sway strictly separates wayland appids and xorg window classes. That results in an empty appid for xwayland clients. I don't know if both of those should be thrown together in this protocol.

[johan-bjareholt]

@progandy Huh, that sucks I guess.
At least for my use-case it seems kind of pointless to have an appid if it's not set for most applications anyway (as most applications are run under XWayland nowadays unfortunately).
It's also an issue if phosh and sway would interpret differently what an appid is.

[johan-bjareholt]

This is how I'm using the protocol at least if anyone wants something to try out the protocol with

https://github.com/ActivityWatch/aw-watcher-window-wayland

[AGCaesar]

Any idea when this might be merged? I am waiting to try the new waybar taskbar module on sway :)

[ddevault]

It would be helpful if someone adopted this and got it up to date.

[johan-bjareholt]

Here it is a patch file of this PR rebased on master because I don't have write permissions to this branch.
There were no risky merge conflicts. Compiled and shortly tested as well.

0001-Implement-wlr-foreign-toplevel-management-v1.txt

[ddevault]

Open a new pull request, please.

[johan-bjareholt]

@ddevault Done #5477

[rpigott]

Chose to leave fullscreen unimplemented for now.

@ddevault Is fullscreen intentionally unsupported, or would you accept a PR to implement it?

[emersion]

I think so, please do!