Merge ~dilyn-corner/chromium-browser/+git/snap-from-source:daemon into ~chromium-team/chromium-browser/+git/snap-from-source:daemon

[Nathan Teodosio]

This MR does one key thing: add a configure and install hook to enable the daemon just in case a specific value is set (daemon=true)
Because the daemon is installed disabled by default, only people who intentionally enable it have the daemon run. Allowing a config option to be set with `snap set` means that this can be set as a default value for Ubuntu Core systems.

Other changes include adding a wrapper to guarantee the Wayland socket is in the right place, and avoiding space splitting in the url set by `snap set chromium url`.

[Nathan Teodosio (nteodosio)]

I deleted the 037c879 and a8740c8 (corresponding to the patch sent via e-mail) from the target branch so as we could review the whole changes.

Launchpad picked it up in the "Unmerged commits" below but the it nonetheless didn't list them in the "Preview diff".

I am superseding this proposal so that we can have a whole diff for review. Sorry for the inconvenience.

[Nathan Teodosio (nteodosio)]

Thanks for the merge request, Dilyn. Some points I'd like to better understand:

1. I don't see any statement to set daemon to true, only to false. Why is that? And why do we need to set it to false if (as far as I know) nothing checks whether it is false?

2. Do we need both an install and a configure hook? Judging by the documentation[1], if the install hook is run, so is the configure hook, and *install + configure* seems to do essentially the same a lone *configure*, namely "if snapctl get daemon, then start and enable the service, else stop and disable it".

[1] https://snapcraft.io/docs/supported-snap-hooks#heading--the-configure-hook

review: Needs Information

[Dilyn (dilyn-corner)]

> 1. I don't see any statement to set daemon to true, only to false. Why is that?

The ultimate intention here is to provide a way of running chromium as a proper kiosk. This is probably not the preferred way of running chromium, so we want to safeguard this as an opt-in choice for users of the snap. In addition, people who want to run chromium as a kiosk will probably want to ensure it launches automatically without intervention. Thus, the onus should be on users to explicitly say they want the kiosk feature, and only then should the daemon be enabled.

One use-case is for Ubuntu Core users: they can add a config value to their gadget snap to set daemon=true so that when they install the image on their device, chromium will launch in the right way "out of the box".

> And why do we need to set it to false if (as far as I know) nothing checks whether it is false?

We set it to false at the very least as a sanity check for users - it offers a transparent way to tell if the daemon is actually enabled or not.

> 2. Do we need both an install and a configure hook?

That's a fair question. Examples are provided by us here:
https://github.com/MirServer/iot-example-graphical-snap

Their install hook explicitly checks for whether or not the snap is installed on an Ubuntu Core system. I see no real need for this, as the question can be side-stepped in a simpler way (namely, setting the daemon as enabled via the gadget snap as mentioned above). I think we could safely eliminate the install hook for the reason you point out; I included it here for a more "complete" handling of the snap during its life cycle.

[Nathan Teodosio (nteodosio)]

>> 1.
> so we want to safeguard this as an opt-in choice for users of the snap.

Ah alright, we never set daemon to true because the user is supposed to run

  snap set chromium daemon=true

, correct?

[Dilyn (dilyn-corner)]

Exactly, yes!

[Nathan Teodosio (nteodosio)]

OK, then please remove the install hook for the simplification.

As far as I'm concerned this is good to merge for a latest/stable/daemon
channel.

[Dilyn (dilyn-corner)]

Done!

[Nathan Teodosio (nteodosio)]

review: Approve

diff --git a/launcher/chromium.launcher b/launcher/chromium.launcher
index 1c57644..e0f40db 100755
--- a/launcher/chromium.launcher
+++ b/launcher/chromium.launcher
@@ -151,6 +151,17 @@ esac
 # Refer to https://bugs.archlinux.org/task/76268.
 CHROMIUM_FLAGS="$CHROMIUM_FLAGS --disable-features=TFLiteLanguageDetectionEnabled"
 
+# Check if the user wants to actually run chromium as a daemon
+daemon="$(snapctl get daemon)"
+if [ "$daemon" = "true" ]; then
+    # If chromium is running as a daemon, assume we're running as a kiosk.
+    CHROMIUM_FLAGS="$CHROMIUM_FLAGS --kiosk --no-sandbox --disable-dev-shm-usage"
+    # If we're running as a kiosk, we are using Frame, so use Wayland.
+    CHROMIUM_FLAGS="$CHROMIUM_FLAGS --enable-features=UseOzonePlatform --ozone-platform=wayland"
+    # If we're running as a kiosk, we are loading a specific URL.
+    set -- "$@" "$(snapctl get url)"
+fi
+
 if [ $WANT_TEMP_PROFILE -eq 0 ] ; then
   exec "$SNAP/usr/lib/chromium-browser/chrome" --password-store=$PASSWORD_STORE $CHROMIUM_FLAGS "$@"
 else
diff --git a/launcher/daemon.wrapper b/launcher/daemon.wrapper
new file mode 100755
index 0000000..1724efd
--- /dev/null
+++ b/launcher/daemon.wrapper
@@ -0,0 +1,17 @@
+#!/bin/sh
+
+set -e
+
+real_xdg_runtime_dir=$(dirname "${XDG_RUNTIME_DIR}")
+real_wayland=${real_xdg_runtime_dir}/${WAYLAND_DISPLAY:-wayland-0}
+
+# Permissions applying to the deepest directory is fine
+#shellcheck disable=2174
+mkdir -p "$XDG_RUNTIME_DIR" -m 700
+
+ln -sf "${real_wayland}"      "$XDG_RUNTIME_DIR"
+ln -sf "${real_wayland}.lock" "$XDG_RUNTIME_DIR"
+
+unset DISPLAY
+
+exec "$@"
diff --git a/snap/hooks/configure b/snap/hooks/configure
new file mode 100755
index 0000000..1255869
--- /dev/null
+++ b/snap/hooks/configure
@@ -0,0 +1,13 @@
+#!/bin/sh
+
+set -ex
+
+daemon="$(snapctl get daemon)"
+
+if [ "$daemon" = "true" ]; then
+    if snapctl services "$SNAP_INSTANCE_NAME.daemon" | grep -q inactive; then
+       snapctl start --enable "$SNAP_INSTANCE_NAME.daemon" 2>&1 || true
+    fi
+else
+    snapctl stop --disable "$SNAP_INSTANCE_NAME.daemon" 2>&1 || true
+fi
diff --git a/snapcraft.yaml b/snapcraft.yaml
index 3d4ec43..1fccc46 100644
--- a/snapcraft.yaml
+++ b/snapcraft.yaml
@@ -73,6 +73,40 @@ apps:
       - upower-observe
     slots:
       - mpris
+  daemon:
+    extensions: [gnome-3-38]
+    daemon: simple
+    command: bin/chromium.launcher
+    command-chain:
+      - bin/daemon.wrapper
+      - snap/command-chain/desktop-launch
+    restart-delay: 3s
+    install-mode: disable
+    environment:
+      CHROME_CONFIG_HOME: $SNAP_USER_COMMON
+      PIPEWIRE_CONFIG_NAME: $SNAP/usr/share/pipewire/pipewire.conf
+      PIPEWIRE_MODULE_DIR: $SNAP/usr/lib/$SNAPCRAFT_ARCH_TRIPLET/pipewire-0.3
+      SPA_PLUGIN_DIR: $SNAP/usr/lib/$SNAPCRAFT_ARCH_TRIPLET/spa-0.2
+    plugs:
+      - audio-playback
+      - audio-record
+      - bluez # for Web Bluetooth (https://launchpad.net/bugs/1887201)
+      - camera
+      - cups
+      - home
+      - joystick
+      - mount-observe
+      - network
+      - network-manager
+      - password-manager-service
+      - pulseaudio # remove once snapd 2.41 is available everywhere
+      - raw-usb # for WebUSB (https://launchpad.net/bugs/1780678)
+      - removable-media
+      - screen-inhibit-control
+      - system-packages-doc
+      - u2f-devices
+      - unity7 # required for xdg-open to work
+      - upower-observe
 
 plugs:
   # This is not used or needed for anything other than to trigger automatic