Bug 39944 – [GTK] GtkLauncher should support relative file paths

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Martin Robinson]

This is supported for WinLauncher. It makes GtkLauncher much easier to use for debugging failing tests.

[Martin Robinson]

Created attachment 57444 [details]
Patch for this issue

[WebKit Review Bot]

Attachment 57444 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebKitTools/GtkLauncher/main.c:191:  Extra space before ( in function call  [whitespace/parens] [4]
WebKitTools/GtkLauncher/main.c:193:  Declaration has space between * and variable name in GFile* gfile  [whitespace/declaration] [3]
WebKitTools/GtkLauncher/main.c:193:  Extra space before ( in function call  [whitespace/parens] [4]
WebKitTools/GtkLauncher/main.c:197:  Declaration has space between * and variable name in gchar* full_path  [whitespace/declaration] [3]
WebKitTools/GtkLauncher/main.c:197:  Extra space before ( in function call  [whitespace/parens] [4]
WebKitTools/GtkLauncher/main.c:197:  full_path is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebKitTools/GtkLauncher/main.c:201:  Declaration has space between * and variable name in gchar* file_url  [whitespace/declaration] [3]
WebKitTools/GtkLauncher/main.c:201:  Extra space before ( in function call  [whitespace/parens] [4]
WebKitTools/GtkLauncher/main.c:201:  file_url is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebKitTools/GtkLauncher/main.c:223:  Declaration has space between * and variable name in gchar* file_url  [whitespace/declaration] [3]
WebKitTools/GtkLauncher/main.c:223:  file_url is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebKitTools/GtkLauncher/main.c:226:  Extra space before ( in function call  [whitespace/parens] [4]
WebKitTools/GtkLauncher/main.c:229:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 13 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Martin Robinson]

> Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
> WebKitTools/GtkLauncher/main.c:191:  Extra space before ( in function call  [whitespace/parens] 

Bug for check-webkit-style false positives filed here: https://bugs.webkit.org/show_bug.cgi?id=39947

[Martin Robinson]

Created attachment 57449 [details]
Patch with style and memory leak fix

Xan is saying that GtkLauncher should gradually move to WebKit style, so I've redone my patch using WebKit style and with a small memory leak fix.

[Xan Lopez]

Comment on attachment 57449 [details]
Patch with style and memory leak fix

>+static gchar* filenameToURL(const char* filename)
>+{
>+    GFile *gFile = g_file_new_for_path(filename);
>+    if (!g_file_query_exists(gFile, 0)) {
>+        g_object_unref(gFile);
>+        return 0;
>+    }
>+    g_object_unref(gFile);

Maybe use g_file_test if you only create the object to check that the path is valid?

>+
>+    gchar *fullPath = realpath(filename, 0);
>+    if (!fullPath)
>+        return 0;
>+
>+    gchar *fileURL = g_filename_to_uri(fullPath, 0, 0);
>+    free(fullPath);

GOwnPtr for fullPath.

>+
>+    return fileURL;
>+}
>+
> int
> main (int argc, char* argv[])
> {
>@@ -201,8 +221,15 @@ main (int argc, char* argv[])
>     main_window = create_window ();
>     gtk_container_add (GTK_CONTAINER (main_window), vbox);
> 
>-    gchar* uri = (gchar*) (argc > 1 ? argv[1] : "http://www.google.com/");
>-    webkit_web_view_load_uri (web_view, uri);
>+    gchar *uri = (gchar*) (argc > 1 ? argv[1] : "http://www.google.com/");
>+    gchar *fileURL = filenameToURL(uri);
>+
>+    if (fileURL) {
>+        webkit_web_view_load_uri(web_view, fileURL);
>+        g_free(fileURL);
>+    } else {
>+        webkit_web_view_load_uri(web_view, uri);
>+    }

You can use GOwnPtr for fileURL, and have only one branch with fileURI ? fileURI : uri

> 
>     gtk_widget_grab_focus (GTK_WIDGET (web_view));
>     gtk_widget_show_all (main_window);

[Xan Lopez]

Eh, ok, you can't use GOwnPtr ok, so disregard that part.

[Xan Lopez]

I meant GOwnPtr here... you can still keep a single branch since it's ok to call g_free with a NULL pointer though.

[Martin Robinson]

Created attachment 57836 [details]
Patch with fixes

[Martin Robinson]

Thanks for the review! As we discussed recently, GOwnPtr cannot be used in GtkLauncher. I've implemented your other suggestions though.

[Xan Lopez]

Comment on attachment 57836 [details]
Patch with fixes

r=me

[Martin Robinson]

Comment on attachment 57836 [details]
Patch with fixes

Clearing flags on attachment: 57836

Committed r60713: <http://trac.webkit.org/changeset/60713>

[Martin Robinson]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

http://trac.webkit.org/changeset/60713 might have broken GTK Linux 32-bit Release
The following changes are on the blame list:
http://trac.webkit.org/changeset/60712
http://trac.webkit.org/changeset/60713
http://trac.webkit.org/changeset/60709
http://trac.webkit.org/changeset/60710
http://trac.webkit.org/changeset/60711