Bug 199416 - [WPE][GTK] Remove flatpak sandbox
Summary: [WPE][GTK] Remove flatpak sandbox
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-07-02 12:12 PDT by Patrick Griffis
Modified: 2020-01-09 00:57 PST (History)
4 users (show)

See Also:

Attachments
Patch (9.86 KB, patch)
2019-07-02 12:13 PDT, Patrick Griffis 		no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews116 for mac-highsierra (3.07 MB, application/zip)
2019-07-02 14:01 PDT, EWS Watchlist 		no flags Details
Patch (9.87 KB, patch)
2019-07-03 11:09 PDT, Patrick Griffis 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick Griffis 2019-07-02 12:12:41 PDT 
[WPE][GTK] Remove flatpak sandbox
Comment 1 Patrick Griffis 2019-07-02 12:13:58 PDT 
Created attachment 373351 [details]
Patch
Comment 2 EWS Watchlist 2019-07-02 14:01:25 PDT Comment hidden (spam)
Comment 3 EWS Watchlist 2019-07-02 14:01:27 PDT Comment hidden (spam)
Comment 4 Michael Catanzaro 2019-07-02 15:27:09 PDT 
Comment on attachment 373351 [details]
Patch

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

> Source/WebKit/UIProcess/Launcher/glib/ProcessLauncherGLib.cpp:130
> -    if (sandboxEnabled && isInsideFlatpak())
> -        process = flatpakSpawn(launcher.get(), m_launchOptions, argv, socketPair.client, &error.outPtr());
> -#if ENABLE(BUBBLEWRAP_SANDBOX)
> -    else if (sandboxEnabled)
> +    if (sandboxEnabled)
>          process = bubblewrapSpawn(launcher.get(), m_launchOptions, argv, &error.outPtr());
> -#endif
>      else
>  #endif

Don't we still need the isInsideFlatpak() check? Surely bubblewrap isn't going to work inside flatpak?
Comment 5 Patrick Griffis 2019-07-02 16:24:17 PDT 
(In reply to Michael Catanzaro from comment #4)
> Don't we still need the isInsideFlatpak() check? Surely bubblewrap isn't
> going to work inside flatpak?

ENABLE_BUBBLEWRAP_SANDBOX defaults to false when built inside Flatpak. If somebody forcefully enables it then thats on them.
Comment 6 Michael Catanzaro 2019-07-03 07:42:40 PDT 
(In reply to Patrick Griffis from comment #5)
> ENABLE_BUBBLEWRAP_SANDBOX defaults to false when built inside Flatpak. If
> somebody forcefully enables it then thats on them.

Well that doesn't seem like it's enough, because our flatpak runtimes are not built inside flatpak.
Comment 7 Patrick Griffis 2019-07-03 10:58:13 PDT 
(In reply to Michael Catanzaro from comment #6)
> (In reply to Patrick Griffis from comment #5)
> > ENABLE_BUBBLEWRAP_SANDBOX defaults to false when built inside Flatpak. If
> > somebody forcefully enables it then thats on them.
> 
> Well that doesn't seem like it's enough, because our flatpak runtimes are
> not built inside flatpak.

Alright, I'll re-add it.
Comment 8 Patrick Griffis 2019-07-03 11:09:22 PDT 
Created attachment 373399 [details]
Patch
Comment 9 WebKit Commit Bot 2019-07-03 12:50:31 PDT 
Comment on attachment 373399 [details]
Patch

Clearing flags on attachment: 373399

Committed r247096: <https://trac.webkit.org/changeset/247096>
Comment 10 WebKit Commit Bot 2019-07-03 12:50:32 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 11 Michael Catanzaro 2020-01-08 08:45:21 PST 
o/ didn't you mention you had this working again? Looks like that never made it upstream...?
Comment 12 Patrick Griffis 2020-01-09 00:57:16 PST 
(In reply to Michael Catanzaro from comment #11)
> o/ didn't you mention you had this working again? Looks like that never made
> it upstream...?

bug 204732 has a patch, will get it reviewed after a patch lands in flatpak-xdg-utils soon.