Bug 237084 - [GStreamer] De-initialize GStreamer before terminating WebProcess
Summary: [GStreamer] De-initialize GStreamer before terminating WebProcess
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Philippe Normand
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-02-23 06:51 PST by Philippe Normand
Modified: 2022-02-23 08:37 PST (History)
5 users (show)

See Also:


Attachments
Patch (3.09 KB, patch)
2022-02-23 06:57 PST, Philippe Normand
no flags Details | Formatted Diff | Diff
Patch (3.60 KB, patch)
2022-02-23 07:13 PST, Philippe Normand
no flags Details | Formatted Diff | Diff
Patch (3.96 KB, patch)
2022-02-23 07:45 PST, Philippe Normand
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Normand 2022-02-23 06:51:34 PST
Allowing the leak tracer to report/log leaks. From https://bugs.webkit.org/show_bug.cgi?id=217655#c42
Comment 1 Philippe Normand 2022-02-23 06:57:40 PST
Created attachment 452976 [details]
Patch
Comment 2 Carlos Garcia Campos 2022-02-23 07:00:50 PST
Comment on attachment 452976 [details]
Patch

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

> Source/WebKit/WebProcess/glib/WebProcessGLib.cpp:88
> +#if USE(GSTREAMER)
> +    gst_deinit();
> +#endif

I think the right place for this is WebProcessMainGtk.cpp and WebProcessMainWPE.cpp. You need to override platformFinalize(). Closing the pages is done here because m_pageMap is private.
Comment 3 Philippe Normand 2022-02-23 07:13:28 PST
Created attachment 452977 [details]
Patch
Comment 4 Carlos Garcia Campos 2022-02-23 07:20:45 PST
Comment on attachment 452977 [details]
Patch

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

> Source/WebKit/WebProcess/gtk/WebProcessMainGtk.cpp:75
> +#if USE(GSTREAMER)
> +        gst_deinit();
> +#endif

Don't we need to include a gst header for this?

> Source/WebKit/WebProcess/wpe/WebProcessMainWPE.cpp:107
> +#if USE(GSTREAMER)
> +        gst_deinit();
> +#endif

Ditto.
Comment 5 Philippe Normand 2022-02-23 07:26:28 PST
Somehow they were pulled in, the magics of unified builds I suppose. Anyway yes, I'll add explicit includes, it won't hurt.
Comment 6 Philippe Normand 2022-02-23 07:45:08 PST
Created attachment 452979 [details]
Patch
Comment 7 Michael Catanzaro 2022-02-23 07:53:40 PST
(In reply to Philippe Normand from comment #5)
> Somehow they were pulled in, the magics of unified builds I suppose. Anyway
> yes, I'll add explicit includes, it won't hurt.

You'll save Adrian from fixing it after you ;)
Comment 8 EWS 2022-02-23 08:34:50 PST
Committed r290375 (247691@main): <https://commits.webkit.org/247691@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 452979 [details].
Comment 9 Radar WebKit Bug Importer 2022-02-23 08:35:29 PST
<rdar://problem/89357593>
Comment 10 Adrian Perez 2022-02-23 08:37:43 PST
(In reply to Michael Catanzaro from comment #7)
> (In reply to Philippe Normand from comment #5)
> > Somehow they were pulled in, the magics of unified builds I suppose. Anyway
> > yes, I'll add explicit includes, it won't hurt.
> 
> You'll save Adrian from fixing it after you ;)

Indeed! Thanks for adding the explicit includes 😁️