Bug 210287 - [GTK] Disable PSON unless WPE_RENDERER is enabled
Summary: [GTK] Disable PSON unless WPE_RENDERER is enabled
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: PC Linux
: P2 Normal
Assignee: Michael Catanzaro
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-04-09 10:01 PDT by Michael Catanzaro
Modified: 2020-04-10 14:07 PDT (History)
7 users (show)

See Also:


Attachments
Patch (2.02 KB, patch)
2020-04-09 10:15 PDT, Michael Catanzaro
no flags Details | Formatted Diff | Diff
Patch (2.02 KB, patch)
2020-04-09 10:15 PDT, Michael Catanzaro
cgarcia: review-
cgarcia: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 2020-04-09 10:01:41 PDT
I've failed to fix WPE_RENDERER in F32 (bug #209118) and failed to fix WaylandCompositor to work with PSON (bug #209345). We should disable PSON when using WaylandCompositor until bug #209345 is fixed.
Comment 1 Michael Catanzaro 2020-04-09 10:15:12 PDT
Created attachment 395970 [details]
Patch
Comment 2 Michael Catanzaro 2020-04-09 10:15:30 PDT
Created attachment 395971 [details]
Patch
Comment 3 EWS Watchlist 2020-04-09 10:16:12 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 4 Дилян Палаузов 2020-04-09 10:52:46 PDT
https://webkitgtk.org/reference/webkit2gtk/stable/WebKitWebContext.html#WebKitWebContext--process-swap-on-cross-site-navigation-enabled

why does it say that by default PSON defaults to FALSE?
Comment 5 Дилян Палаузов 2020-04-09 10:54:19 PDT
Why do I need WPE (renderer)?
Comment 6 Michael Catanzaro 2020-04-09 11:41:38 PDT
(In reply to Дилян Палаузов from comment #4)
> https://webkitgtk.org/reference/webkit2gtk/stable/WebKitWebContext.
> html#WebKitWebContext--process-swap-on-cross-site-navigation-enabled
> 
> why does it say that by default PSON defaults to FALSE?

Because it breaks apps that aren't prepared for it. Epiphany enables it because it's an important security feature.
Comment 7 Michael Catanzaro 2020-04-09 13:39:35 PDT
(In reply to Дилян Палаузов from comment #5)
> Why do I need WPE (renderer)?

You don't. It's just faster.
Comment 8 Michael Catanzaro 2020-04-09 14:14:23 PDT
Backport for 2.28 branch:

diff --git a/Source/WebKit/UIProcess/API/glib/WebKitWebContext.cpp b/Source/WebKit/UIProcess/API/glib/WebKitWebContext.cpp
index 33a9b7d5ad060f275dcf7156a8cff3f37644e736..70b485da05a92caa91bf91b9653740b8e93cf96c 100644
--- a/Source/WebKit/UIProcess/API/glib/WebKitWebContext.cpp
+++ b/Source/WebKit/UIProcess/API/glib/WebKitWebContext.cpp
@@ -376,6 +376,9 @@ static void webkitWebContextConstructed(GObject* object)
     API::ProcessPoolConfiguration configuration;
     configuration.setInjectedBundlePath(FileSystem::stringFromFileSystemRepresentation(bundleFilename.get()));
 #if PLATFORM(GTK)
+// FIXME: PSON is currently broken unless WPE_RENDERER is enabled.
+// https://bugs.webkit.org/show_bug.cgi?id=209345
+#if USE(WPE_RENDERER)
     configuration.setProcessSwapsOnNavigation(priv->psonEnabled);
     if (!priv->psonEnabled) {
         const char* useSingleWebProcess = getenv("WEBKIT_USE_SINGLE_WEB_PROCESS");
@@ -387,6 +390,8 @@ static void webkitWebContextConstructed(GObject* object)
                 configuration.setUsesSingleWebProcess(true);
         }
     }
+#else
+    configuration.setProcessSwapsOnNavigation(false);
 #endif
 
     if (!priv->websiteDataManager)
Comment 9 Michael Catanzaro 2020-04-09 14:17:43 PDT
Backport is busted, sorry....
Comment 10 Michael Catanzaro 2020-04-09 14:24:54 PDT
Better backport:

diff --git a/Source/WebKit/UIProcess/API/glib/WebKitWebContext.cpp b/Source/WebKit/UIProcess/API/glib/WebKitWebContext.cpp
index 33a9b7d5ad060f275dcf7156a8cff3f37644e736..70b485da05a92caa91bf91b9653740b8e93cf96c 100644
--- a/Source/WebKit/UIProcess/API/glib/WebKitWebContext.cpp
+++ b/Source/WebKit/UIProcess/API/glib/WebKitWebContext.cpp
@@ -376,7 +376,11 @@ static void webkitWebContextConstructed(GObject* object)
     API::ProcessPoolConfiguration configuration;
     configuration.setInjectedBundlePath(FileSystem::stringFromFileSystemRepresentation(bundleFilename.get()));
 #if PLATFORM(GTK)
+#if USE(WPE_RENDERER)
+    configuration.setProcessSwapsOnNavigation(priv->psonEnabled);
+#else
     configuration.setProcessSwapsOnNavigation(false);
+#endif
     if (!priv->psonEnabled) {
         const char* useSingleWebProcess = getenv("WEBKIT_USE_SINGLE_WEB_PROCESS");
         if (useSingleWebProcess && strcmp(useSingleWebProcess, "0"))
Comment 11 Carlos Garcia Campos 2020-04-10 06:28:29 PDT
Comment on attachment 395971 [details]
Patch

You know I prefer to fix bugs instead of disabling features to work around them.
Comment 12 Michael Catanzaro 2020-04-10 14:07:27 PDT
:)