Linux Snap: Prevent GDK_PIXBUF env from leaking out

[joaomoreno]

This is a nasty one:

• In order to fix Snap: User fonts are missing #63194 and Linux: dialogs are looking broken in the snap package #57019, we needed to set some GFK_PIXBUF env for our snap executable (PR from Snap team: Snapcraft update. Fixes #63194 Fixes #57019 #70884):

  vscode/resources/linux/snap/electron-launch

  Lines 24 to 29 in d4da283

  	# Gdk-pixbuf loaders
  	export GDK_PIXBUF_MODULE_FILE="$GDK_CACHE_DIR/gdk-pixbuf-loaders.cache"
  	export GDK_PIXBUF_MODULEDIR="$SNAP/usr/lib/$ARCH/gdk-pixbuf-2.0/2.10.0/loaders"
  	if [ -f "$SNAP/usr/lib/$ARCH/gdk-pixbuf-2.0/gdk-pixbuf-query-loaders" ]; then
  	"$SNAP/usr/lib/$ARCH/gdk-pixbuf-2.0/gdk-pixbuf-query-loaders" > "$GDK_PIXBUF_MODULE_FILE"
  	fi

• That caused a crash when opening URIs in Firefox (Firefox crashes when I click on a link in VSCode.  #85344), so in order to fix that, the Snap team recommended we delete that env right after startup: 7d62a5b
• That now causes a crash when opening any native dialog in VS Code in pretty much every Linux distribution other than Ubuntu (Snap crashes when native dialogs open #100940)

This PR takes the debacle even further by reverting 7d62a5b and making sure every single shell.openExternal call is wrapped with a unset-set env operation,when running as a Linux Snap. This hopefully fixes both the issue with Firefox as well as the issue with the native dialogs.

• @bpasero: Does the wrapping of shell.openExternal make sense and is it in the right place?
• @Tyriar: Does the approach generally make sense to you?

I've triggered a build and I have VMs ready to verify whether it works, will let you know asap.

cc @flexiondotorg @sergiusens

[bpasero]

Overall this looks OK to me, but ideally we could have this logic inside INativeHostMainService and everyone else depends on this service. I think it should be possible to use it everywhere.

[joaomoreno]

@bpasero I actually started there, but stumbled over that AddFirstParameterToFunctions thing and didn't know if passing undefined would be OK. If everything works well, I'll adapt the PR to do that.

[bpasero]

@joaomoreno yeah that is fine, I use it in electron-main too, e.g. here:

vscode/src/vs/code/electron-main/auth2.ts

Line 156 in 0ae821e

const encryptedSerializedProxyCredentials = await this.nativeHostMainService.getPassword(undefined, ProxyAuthHandler2.PROXY_CREDENTIALS_SERVICE_KEY, authInfoHash);

[joaomoreno]

Fix works great 🎆 Tested Ubuntu, Manjaro and CentOS.

Will refactor tomorrow.

[Tyriar]

Approach seems fine, it's a shame we need to jump through these hoops though.

[joaomoreno]

Approach seems fine, it's a shame we need to jump through these hoops though.

Tell me about it. 🤦‍♂️

---

@deepak1556 Says:

Hey Joao! I was following your work on https://github.com/microsoft/vscode/compare/joao/snap , isn't the core problem related to the incorrect path to gdk-pixbuf-2.0 on these platforms ? for 64 bit we always default to x86_64-linux-gnu https://github.com/microsoft/vscode/blob/master/resources/linux/snap/electron-launch#L8 but the library is under /usr/lib/. Am i understanding the problem incorrectly ?

The actual problem is that Code should always use the gdk-pixbuf inside the snap. But by removing the env very early on (7d62a5b) we hit the situation in which the About dialog reads the wrong env later on and looks up a path which doesn't exist in the snap, so ends up falling back to the OS. In Ubuntu this isn't a problem, but in other Linuxes the path doesn't exist.

7d62a5b must be reverted, but we still need to address spawning other processes (eg Firefox), that's what this PR does.