Bug 264879 – [GTK][WPE] Limit memory used by target strings serialized in WebKitWebViewSessionState

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Guilaume Ayoub]

More information about this bug can be found here: https://gitlab.gnome.org/GNOME/epiphany/-/issues/1610

[Michael Catanzaro]

I'm not sure we're even gaining much anything from all this memory use. The point of serializing the state of the web view is to be able to restore it instantly without having to load everything again. And for Apple ports this actually happens, but for GTK/WPE we do a new load anyway to ensure we don't display stale content. (E.g. it would be really confusing to open your browser to this Bugzilla page and not see new comments posted since the last time you manually reloaded the page, which could be days or weeks ago.)

[Kdwk]

I think the ideal experience would be to restore the old page first, then do a fresh load in the background, and switch to that when the new one has been fully loaded.

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #1)
> I'm not sure we're even gaining much anything from all this memory use. The
> point of serializing the state of the web view is to be able to restore it
> instantly without having to load everything again. And for Apple ports this
> actually happens, but for GTK/WPE we do a new load anyway to ensure we don't
> display stale content. (E.g. it would be really confusing to open your
> browser to this Bugzilla page and not see new comments posted since the last
> time you manually reloaded the page, which could be days or weeks ago.)

I did some archaeology and found bug #115600 was where we agreed to disable this. I wonder if we should revisit this choice. Anyway, that won't fix this bug; it would only make the excessive memory use worth something. Right now I suspect it's all wasted.

[Michael Catanzaro]

Sorry, bug #115600 was where this API was introduced. I meant to link to bug #153230.

[Michael Catanzaro]

Apple recommends we limit the size of the session state in our platform-specific code, bug #268410. I think this is acceptable; it should be OK to drop the session state if it gets too large.

That said, ideally we should not attempt to save the entire page cache into session state. It should be sufficient to save only the current page. Serializing everything in the back/forward list just to make the back button work faster after the browser is closed and reopened is overkill.

[Guilaume Ayoub]

(In reply to Michael Catanzaro from comment #5)
> Apple recommends we limit the size of the session state in our
> platform-specific code, bug #268410. I think this is acceptable; it should
> be OK to drop the session state if it gets too large.

It would definitely work for me, as my current "solution" is a sed command removing the history attribute from session_state.xml when it gets too large.

[Michael Catanzaro]

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

[EWS]

Committed 292282@main (99f7a1b31bae): <https://commits.webkit.org/292282@main>

Reviewed commits have been landed. Closing PR #42568 and removing active labels.

[Guilaume Ayoub]

Thanks a lot! 💘

[Adrian Perez]

@Guillaume: No problem at all!

We may be able to make the session state somewhat smaller by limiting the total size stored, see bug #289945 for that.