[webkit-reviews] review granted: [Bug 239682] [GTK] Bubblewrap sandbox lacks support for pipewire : [Attachment 458223] [GTK] BubblewrapLauncher.cpp: Add paths required for pipewire

[bugzilla-daemon at webkit.org]

Michael Catanzaro <mcatanzaro at gnome.org> has granted Adrian Perez
<aperez at igalia.com>'s request for review:
Bug 239682: [GTK] Bubblewrap sandbox lacks support for pipewire
https://bugs.webkit.org/show_bug.cgi?id=239682

Attachment 458223: [GTK] BubblewrapLauncher.cpp: Add paths required for
pipewire

https://bugs.webkit.org/attachment.cgi?id=458223&action=review




--- Comment #8 from Michael Catanzaro <mcatanzaro at gnome.org> ---
Comment on attachment 458223
  --> https://bugs.webkit.org/attachment.cgi?id=458223
[GTK] BubblewrapLauncher.cpp: Add paths required for pipewire

View in context: https://bugs.webkit.org/attachment.cgi?id=458223&action=review

OK, the code changes look fine to me. We really do need to disable Pulse when
enabling Pipewire, but that can be a future improvement.

You want to set r? and cq?, not r+ and cq+. Adrian already fixed it for you
this time, but that's why the commit bot is angry.

> Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp:310
> +    if (!pwRemote) {
> +	   pwRemote = "pipewire-0";
> +    }

Style nit: remove the braces. That's why style bot is angry.

> Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp:780
>	   // FIXME: We should move to Pipewire as soon as viable, Pulse
doesn't restrict clients atm.
>	   bindPulse(sandboxArgs);

Can you update this comment please?

// FIXME: Disable PulseAudio access when Pipewire is available.

Bonus points available if you fix it. Should be just checking for sockets.