Merge ~gunnarhj/chromium-browser/+git/snap-from-source:flash-fix into ~chromium-team/chromium-browser/+git/snap-from-source:stable

[Gunnar Hjalmarsson]

This change is desired irrespective of what happens with my confinement idea. Without it Chromium unnecessarily complains about the plugin being too old.

[Olivier Tilloy (osomon)]

Good catch Gunnar, thanks!

This looks mostly fine to me, but in practice it doesn't work because: "strings: command not found" inside confinement. We would need to add binutils-$ARCH and libbinutils to the stage packages (and we would probably want to keep only the strings binary, filtering out all the other utilities).

Also you will need to remove the double quotes around $FLASH_OPTIONS in the exec line, otherwise chromium will interpret the contents of the variable as one single parameter, and will fail to load the shared library.

review: Needs Fixing

[Gunnar Hjalmarsson (gunnarhj)]

Thanks for looking! Didn't realize that also strings() requires a confinement adjustment. (Why is the confinement sooo restrictive?)

If you talk to someone about modifying the confinement, please take the opportunity to ask them to fix it for adobe-flashplugin and pepperflashplugin-nonfree too. :)

As regards the quotes, are you sure? The "$@" is quoted, and it seems to allow for multiple options to be passed manually. But to be honest I was in doubt; submitted a commit which removes them.

[Olivier Tilloy (osomon)]

I tested with quotes, and got an error message that the shared lib wasn't recognized, the name contained the rest of the arguments, and I tested again without quotes and it worked.

Regarding the strings requirement, I think we can just drop it, grep can deal with binary data to some extent:

    grep -a -z LNX libpepflashplayer.so | cut -d ' ' -f 2 | sed -e "s/,/./g"

Can you do that change?

Thanks!

[Gunnar Hjalmarsson (gunnarhj)]

Ah, that was a nice trick. :) (I had simply stolen the use of strings() from the pepperflashplugin-nonfree package.)

Committed the change to this MP.

diff --git a/launcher/chromium.launcher b/launcher/chromium.launcher
index 03ba33d..2a35b7e 100755
--- a/launcher/chromium.launcher
+++ b/launcher/chromium.launcher
@@ -52,11 +52,12 @@ fi
 # distribution (https://get.adobe.com/flashplayer/) and copied there.
 # Ref: https://launchpad.net/bugs/1771162
 FLASHSO=$SNAP_USER_DATA/.local/lib/libpepflashplayer.so
-if [ ! -e $FLASHSO ]; then
-  FLASHSO=
+if [ -e $FLASHSO ]; then
+  FLASHVERSION=$(grep -a -z LNX $FLASHSO | cut -d ' ' -f 2 | sed -e "s/,/./g")
+  FLASH_OPTIONS="--ppapi-flash-path=$FLASHSO --ppapi-flash-version=$FLASHVERSION"
 fi
 
 # Custom version string for chrome://version
 export CHROME_VERSION_EXTRA=snap
 
-exec "$SNAP/usr/lib/chromium-browser/chrome" --no-default-browser-check --no-first-run --password-store=$PASSWORD_STORE --ppapi-flash-path=$FLASHSO "$@"
+exec "$SNAP/usr/lib/chromium-browser/chrome" --no-default-browser-check --no-first-run --password-store=$PASSWORD_STORE $FLASH_OPTIONS "$@"