Bug 194146

Summary: [WPE] MiniBrowser: use g_file_new_for_commandline_arg
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: WPE WebKitAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: aperez, bugs-noreply, calvaris, mcatanzaro, webkit-bug-importer
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch calvaris: review+

Description Carlos Garcia Campos 2019-02-01 01:38:55 PST
That way we can open relative paths.
Comment 1 Carlos Garcia Campos 2019-02-01 01:40:07 PST
Created attachment 360850 [details]
Patch
Comment 2 Adrian Perez 2019-02-01 01:58:07 PST
Comment on attachment 360850 [details]
Patch

Informal R+ \o/
Comment 3 Xabier Rodríguez Calvar 2019-02-01 01:59:41 PST
Comment on attachment 360850 [details]
Patch

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

> Tools/MiniBrowser/wpe/main.cpp:206
> +        GFile* file = g_file_new_for_commandline_arg(uriArguments[0]);
> +        char* url = g_file_get_uri(file);

I'd recommend using smart pointers for this
Comment 4 Carlos Garcia Campos 2019-02-01 02:02:25 PST
(In reply to Xabier Rodríguez Calvar from comment #3)
> Comment on attachment 360850 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=360850&action=review
> 
> > Tools/MiniBrowser/wpe/main.cpp:206
> > +        GFile* file = g_file_new_for_commandline_arg(uriArguments[0]);
> > +        char* url = g_file_get_uri(file);
> 
> I'd recommend using smart pointers for this

We can't. MiniBrowser uses only the public API.
Comment 5 Carlos Garcia Campos 2019-02-01 02:03:41 PST
Committed r240840: <https://trac.webkit.org/changeset/240840>
Comment 6 Michael Catanzaro 2019-02-01 08:25:13 PST
You should use g_autoptr though; then we'd have noticed that the WPE API doesn't define any autoptrs at all. That's still broken. :(
Comment 7 Adrian Perez 2019-02-01 08:55:16 PST
(In reply to Michael Catanzaro from comment #6)
> You should use g_autoptr though; then we'd have noticed that the WPE API
> doesn't define any autoptrs at all. That's still broken. :(

Somehow it does, I'm sure of it because I have been using g_auto* (of which
I am a fan) in a couple of applications which use WPE WebKit. It has happened
that some autoptr definitions have not been there at different moments in
time and we have been adding them whenever we notice.
Comment 8 Michael Catanzaro 2019-02-01 09:45:03 PST
There's no WebKitAutocleanups.h for WPE... where are the autocleanups defined?

Quick 'git grep' indicates G_DEFINE_AUTOPTR_CLEANUP_FUNC is only used in three GTK-specific headers.
Comment 9 Adrian Perez 2019-03-01 10:18:53 PST
(In reply to Michael Catanzaro from comment #8)
> There's no WebKitAutocleanups.h for WPE... where are the autocleanups
> defined?
> 
> Quick 'git grep' indicates G_DEFINE_AUTOPTR_CLEANUP_FUNC is only used in
> three GTK-specific headers.

Finally I got back to this — you are right and I just happened to
be only using g_autoptr() for types in GLib and libsoup in the
program I mentioned that uses WPE.

Let's handle adding the missing autocleanups in bug #195211 :)