[Webkit-unassigned] [Bug 253858] [GTK] Crash in webkit_web_view_session_state_new()

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=253858

Carlos Garcia Campos <cgarcia at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |cgarcia at igalia.com

--- Comment #9 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to Michael Catanzaro from comment #7)
> (In reply to Christian Hergert from comment #6)
> > Is there an init function I can force-call?
> 
> No. We need to call webkitInitialize() but it's not public.
> 
> I really want to add this to class init in WTFGType.h to ensure every public
> type calls the init function, but can't do that because WTF cannot depend on
> WebKit stuff. Could have WebKit register some delegate function, but to do
> that, it would have to first be initialized, chicken/egg, so that's no good.
> Probably not a good idea to use a library constructor. So I think best we
> can do is manually call it at the top of class init for all API types, or at
> least all types that really need it. Currently that's done only for a few
> types:
> 
> $ git grep webkitInitialize
> WebKit/UIProcess/API/glib/WebKitInitialize.cpp:void webkitInitialize()
> WebKit/UIProcess/API/glib/WebKitInitialize.h:void webkitInitialize();
> WebKit/UIProcess/API/glib/WebKitInputMethodContext.cpp:   
> WebKit::webkitInitialize();
> WebKit/UIProcess/API/glib/WebKitNetworkSession.cpp:    webkitInitialize();
> WebKit/UIProcess/API/glib/WebKitSettings.cpp:    webkitInitialize();
> WebKit/UIProcess/API/glib/WebKitUserContentFilterStore.cpp:   
> webkitInitialize();
> WebKit/UIProcess/API/glib/WebKitUserContentManager.cpp:   
> webkitInitialize();
> WebKit/UIProcess/API/glib/WebKitWebContext.cpp:    webkitInitialize();
> WebKit/UIProcess/API/glib/WebKitWebsiteDataManager.cpp:   
> webkitInitialize();
> WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp:    // Usually starting a
> context triggers webkitInitialize, but in case
> WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp:   
> WebKit::webkitInitialize();
> 
> I first tried this:
> 
> -G_DEFINE_BOXED_TYPE(WebKitWebViewSessionState,
> webkit_web_view_session_state, webkit_web_view_session_state_ref,
> webkit_web_view_session_state_unref)
> +G_DEFINE_BOXED_TYPE_WITH_CODE(WebKitWebViewSessionState,
> webkit_web_view_session_state, webkit_web_view_session_state_ref,
> webkit_web_view_session_state_unref, webkitInitialize())
> 
> But it's not enough because the crash occurs in
> webkit_web_view_session_state_new() before the WebKitWebViewSessionState
> object is actually created. That's an awkward location for a
> webkitInitialize() call, but it seems unavoidable. So fix is:
> 
> diff --git a/Source/WebKit/UIProcess/API/glib/WebKitWebViewSessionState.cpp
> b/Source/WebKit/UIProcess/API/glib/WebKitWebViewSessionState.cpp
> index c34347b08ab3..b58bc19b44c0 100644
> --- a/Source/WebKit/UIProcess/API/glib/WebKitWebViewSessionState.cpp
> +++ b/Source/WebKit/UIProcess/API/glib/WebKitWebViewSessionState.cpp
> @@ -20,6 +20,7 @@
>  #include "config.h"
>  #include "WebKitWebViewSessionState.h"
>  
> +#include "WebKitInitialize.h"
>  #include "WebKitWebViewSessionStatePrivate.h"
>  #include <WebCore/BackForwardItemIdentifier.h>
>  #include <wtf/glib/GRefPtr.h>
> @@ -459,6 +460,8 @@ WebKitWebViewSessionState*
> webkit_web_view_session_state_new(GBytes* data)
>  {
>      g_return_val_if_fail(data, nullptr);
>  
> +    webkitInitialize();
> +
>      SessionState sessionState;
>      if (!decodeSessionState(data, sessionState))
>          return nullptr;

This is indeed the right fix.

> Normally this is where I would prepare a pull request, but we have to fix
> bug #253758 first.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20230315/8eafca33/attachment.htm>