Bug 194979 – [GTK][WPE] Enable PSON

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Michael Catanzaro]

Enable PSON. Seems to work. YOLO, etc.

[Michael Catanzaro]

Created attachment 362832 [details]
Patch

[Zan Dobersek]

WPE output locks up on a backwards navigation.

It'd be nice to enable this by default, but there has to be a way to override it at runtime.

[Michael Catanzaro]

Hmm, back/forward works in GTK MiniBrowser.

We don't want to provide a way to disable it because it's required for Spectre mitigation. If it doesn't work yet, we should just not enable it.

[Chris Dumez]

Looks like you may need some fixes specific to your ports. PSON has been enabled on macOS and iOS by default for a while now and works as expected although we are still ironing out a few (minor) quirks.

[Michael Catanzaro]

(In reply to Chris Dumez from comment #4)
> Looks like you may need some fixes specific to your ports. PSON has been
> enabled on macOS and iOS by default for a while now and works as expected
> although we are still ironing out a few (minor) quirks.

Yeah, I don't have time to look into it soon though, so we can either enable it only for GTK and not for WPE, or just not do it yet.

Let's try the former, actually, and see if anyone tries problems.

[Michael Catanzaro]

(Note: it's expected that something will be wrong, but at least back/forward does work for me.)

[Michael Catanzaro]

Created attachment 362902 [details]
Patch

[Zan Dobersek]

If you can, please open a WPE-specific bug for this. Otherwise I will when I get to it.

[WebKit Commit Bot]

Comment on attachment 362902 [details]
Patch

Clearing flags on attachment: 362902

Committed r242045: <https://trac.webkit.org/changeset/242045>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Michael Catanzaro]

Before:

Failed 2 jsc tests failed 25 failures 2 new passes 4 flakes api tests: 7 failures, 3 crashes, 13 timeouts webdriver-test 48 failures, 7 new passes

After:

Failed 1 jsc test failed 26 failures 3 new passes 7 flakes api tests: 7 failures, 3 crashes, 12 timeouts webdriver-test 49 failures, 7 new passes

👍

[Carlos Garcia Campos]

(In reply to Michael Catanzaro from comment #11)
> Before:
> 
> Failed 2 jsc tests failed 25 failures 2 new passes 4 flakes api tests: 7
> failures, 3 crashes, 13 timeouts webdriver-test 48 failures, 7 new passes
> 
> After:
> 
> Failed 1 jsc test failed 26 failures 3 new passes 7 flakes api tests: 7
> failures, 3 crashes, 12 timeouts webdriver-test 49 failures, 7 new passes
> 
> 👍

Layout tests are not a good way to test PSON, since there aren't many cross-site navigations. We are not yet ready to enable PSON.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 195273

[Michael Catanzaro]

Hm, I indeed never tested compositing mode. I only tested webkitgtk.org in MiniBrowser. I tested several other sites, but in Epiphany, without compositing mode. :(

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #14)
> I only tested webkitgtk.org in MiniBrowser.

And yeah, in retrospect that sounds pretty dumb, since there's no cross-site navigations involved there.

[Carlos Garcia Campos]

Created attachment 371478 [details]
Patch

Let's try again now.

[Carlos Garcia Campos]

Chris, do you plan to keep the WebProcessPool configuration option to enable/disable and/or the web preference? I wonder if we should add API to allow disabling PSON and how. What about the other configurations? alwaysKeepAndReuseSwappedProcesses is only for testing/debugging, right? and processSwapsOnWindowOpenWithOpener? I think we always use related views for window.open().

[Carlos Garcia Campos]

*** Bug 195005 has been marked as a duplicate of this bug. ***

[Carlos Garcia Campos]

Committed r246148: <https://trac.webkit.org/changeset/246148>

[Michael Catanzaro]

Great work.

(In reply to Carlos Garcia Campos from comment #17)
> Chris, do you plan to keep the WebProcessPool configuration option to
> enable/disable and/or the web preference? I wonder if we should add API to
> allow disabling PSON and how. What about the other configurations?
> alwaysKeepAndReuseSwappedProcesses is only for testing/debugging, right? and
> processSwapsOnWindowOpenWithOpener? I think we always use related views for
> window.open().

I don't think we should expose any of these.

[Chris Dumez]

(In reply to Carlos Garcia Campos from comment #17)
> Chris, do you plan to keep the WebProcessPool configuration option to
> enable/disable and/or the web preference? I wonder if we should add API to
> allow disabling PSON and how. 

Ideally, PSON becomes de default everywhere and there is no reason to disable it. I do not think we should expose API for this. Apple currently has private API (SPI) but hopefully it will get dropped.

> What about the other configurations?
> alwaysKeepAndReuseSwappedProcesses is only for testing/debugging, right?

Right, solely for debugging and I suspect we'll drop support for this soon because we're not using it and it has some maintenance cost.

> and
> processSwapsOnWindowOpenWithOpener? I think we always use related views for
> window.open().

This is an experiment at this point and does not work yet. No need to expose this.

[Carlos Garcia Campos]

Perfect, thanks Chris!