Bug 173907 - REGRESSION(r218799): [GTK][WPE] Critical warning at exit
Summary: REGRESSION(r218799): [GTK][WPE] Critical warning at exit
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk, Regression
Depends on:
Blocks:
 
Reported: 2017-06-27 23:29 PDT by Carlos Garcia Campos
Modified: 2017-06-28 09:03 PDT (History)
7 users (show)

See Also:


Attachments
Patch (2.87 KB, patch)
2017-06-27 23:32 PDT, Carlos Garcia Campos
annulen: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2017-06-27 23:29:20 PDT
GLib-GObject-CRITICAL **: g_object_unref: assertion 'G_IS_OBJECT (object)' failed

This is now always happening when closing the MeiniBrowser and it's causing a lot of unit tests to fail. In r218799, GRefPtrGtk.h include was removed from PasteboardHelper.h that contains a GRefPtr<GtkTargetList>. The targets are destroyed at exit, but now trying to use g_object_unref instead of gtk_target_list_unref(). I've found two more cases like this in r218799, that removes GUniquePtrSoup.h from ResourceHandleInternal.h and ResourceRequest.h that have GUniquePtr<SoupBuffer> and GUniquePtr<SoupURI>. WE need to be very careful when removing GRefPtrGtk.h, RefPtrCairo.h, CairoUniquePtr.h, GRefPtrSoup.h, GUniquePtrSoup.h, GRefPtrGStreamer.h and GUniquePtrGStreamer.h. I'll check other commits removing headers, because this can lead to pretty weird bugs that are not that easy to find.
Comment 1 Carlos Garcia Campos 2017-06-27 23:32:04 PDT
Created attachment 313998 [details]
Patch
Comment 2 Konstantin Tokarev 2017-06-27 23:46:34 PDT
Oops, my bad.

Source/WebCore/platform/graphics/gstreamer/InbandMetadataTextTrackPrivateGStreamer.h should have #include "GRefPtrGStreamer.h"

Source/WebCore/platform/gtk/ScrollbarThemeGtk.h - #include <wtf/glib/GRefPtr.h>

I didn't find any other cases
Comment 3 Carlos Garcia Campos 2017-06-27 23:49:05 PDT
(In reply to Konstantin Tokarev from comment #2)
> Oops, my bad.
> 
> Source/WebCore/platform/graphics/gstreamer/
> InbandMetadataTextTrackPrivateGStreamer.h should have #include
> "GRefPtrGStreamer.h"
> 
> Source/WebCore/platform/gtk/ScrollbarThemeGtk.h - #include
> <wtf/glib/GRefPtr.h>
> 
> I didn't find any other cases

Those are not a problem because they don't use GRefPtr at all, I guess they did at some point and the header was never removed.
Comment 4 Carlos Garcia Campos 2017-06-28 00:20:35 PDT
Committed r218870: <http://trac.webkit.org/changeset/218870>
Comment 5 Michael Catanzaro 2017-06-28 06:59:37 PDT
Goodness gracious, it is terrifying that removing headers can cause runtime failures, because we all expect that if it builds it must be good. Did any tests catch this?
Comment 6 Carlos Garcia Campos 2017-06-28 09:03:38 PDT
"and it's causing a lot of unit tests to fail"