Bug 44354 – Cairo and EFL port shouldn't depend on glib.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Rafael Antognolli]

Since the commit http://trac.webkit.org/changeset/65530 these ports now depend on glib. That's because of the specialised reference pointers GRefPtr (that is used for glib stuff) that were added for cairo.

Is it possible to do the same kind of specialised pointer but using just RefPtr? I couldn't find any place in WebCore code that does this. If it's not, could we just avoid using this, or maybe just ifdef the code? I can send a patch for this if necessary.

[Martin Robinson]

In this case, it appears that WebKitGTK+ is loading a stale copy of the inspector files for the tests. I should have a patch up shortly. Sorry for the noise!

[Martin Robinson]

Whoops. Wrong bug.

[Martin Robinson]

Created attachment 65015 [details]
Patch

[Martin Robinson]

GRefPtr needs to be independent of GLib so that ports with other platform-specific reference-counted types can use it. I've attached a patch that effectively renames it PlatformRefPtr. I'm not stuck on the name and feel it's a bit long, but this should fix the build.

[Gustavo Noronha (kov)]

Comment on attachment 65015 [details]
Patch

hrrmm... I liked the original idea of doing a typedef PlatformRefPtr GRefPtr, what happened to it? How about PRefPtr? Naming this might be a good thing to take to the mailing list, I think. Also, clever trick with GVariant. I dislike that but I guess the hit on build performance is big enough that it outweights my dislike =(

[Martin Robinson]

(In reply to comment #5)
> (From update of attachment 65015 [details])
> hrrmm... I liked the original idea of doing a typedef PlatformRefPtr GRefPtr, what happened to it?

I found that I couldn't do the typedef, because C++ doesn't support making a typedef of a templated type (you can only typdef a specific specialization).

> How about PRefPtr? Naming this might be a good thing to take to the mailing list, I think. 

I'm fine with PRefPtr or even leaving it as GRefPtr, if others like it. The renaming is fairly easy. I'll make a post!

> Also, clever trick with GVariant. I dislike that but I guess the hit on build performance is big enough that it outweights my dislike =(

I was inspired to do this, because not using glib header in the include was one of the original conditions of the r+ of the original GRefPtr patch. I still think the best solution involves precopiled prefix headers thought! :)

[Martin Robinson]

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

[Xan Lopez]

I think this makes sense in general, and I don't have any problem with the name in the Patch (PlatRefPtr would also be OK). If it's still here tomorrow (my) morning I'll review it for real.

[Gustavo Noronha (kov)]

Comment on attachment 65015 [details]
Patch

OK, let's move forward! =)

[Martin Robinson]

Comment on attachment 65015 [details]
Patch

Clearing flags on attachment: 65015

Committed r66024: <http://trac.webkit.org/changeset/66024>

[Martin Robinson]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

I think it’s a strange choice to rename GRefPtr to PlatformRefPtr. I don’t understand what the “platform” is in this case.

[Gustavo Noronha (kov)]

(In reply to comment #12)
> I think it’s a strange choice to rename GRefPtr to PlatformRefPtr. I don’t understand what the “platform” is in this case.

The reasoning is we began using GRefPtr for non-glib-specific ref-counted types in backends used by several ports that ont necessarily use glib (in this case Cairo). The name 'Platform' comes from these types being platform-specific (or rather port specific). We could keep GRefPtr as it was for glib types, and add a CairoRefPtr that would be cairo-specific , but that seemed wasteful.

[Martin Robinson]

I should also say that I'm open to suggestions for renaming this template. It seems that it may one day combine with JSRetainPtr in which case PlatformRefPtr makes less sense.

[Darin Adler]

(In reply to comment #13)
> The reasoning is we began using GRefPtr for non-glib-specific ref-counted types in backends used by several ports that ont necessarily use glib (in this case Cairo). The name 'Platform' comes from these types being platform-specific (or rather port specific). We could keep GRefPtr as it was for glib types, and add a CairoRefPtr that would be cairo-specific , but that seemed wasteful.

So you’re saying there’s no chance that a single port might some day want to use GLib and Cairo as well?

[Gustavo Noronha (kov)]

(In reply to comment #15)
> So you’re saying there’s no chance that a single port might some day want to use GLib and Cairo as well?

Not really. GTK+ currently uses both glib and cairo, I meant some other ports (like the redistributable windows port and EFL) use only Cairo, so GRefPtr did not make sense for them as a name.

[Darin Adler]

Either we can make a version of RefPtr that automatically knows how to do the right thing, or we should have a pointer named CairoRefPtr. This PlatformRefPtr name concept makes no sense to me. I must be missing something.

[Martin Robinson]

Brent Fulgham had made another patch which just specialized RefPtr for cairo types. Does it make sense to simply remove PlatformRefPtr/GRefPtr and to just rely on template specializations of RefPtr?

[Martin Robinson]

Here's a link to the original series of comments about why this was implemented separately from RefPtr (in this case, it was for the GLib OwnPtr, which resulted in a separate GLib RefPtr later on): https://bugs.webkit.org/show_bug.cgi?id=20483#c2