Bug 194146 - [WPE] MiniBrowser: use g_file_new_for_commandline_arg
Summary: [WPE] MiniBrowser: use g_file_new_for_commandline_arg
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WPE WebKit (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-02-01 01:38 PST by Carlos Garcia Campos
Modified: 2019-03-01 10:18 PST (History)
5 users (show)

See Also:


Attachments
Patch (1.41 KB, patch)
2019-02-01 01:40 PST, Carlos Garcia Campos
calvaris: 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 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 :)