[GTK][WK2] Error loading libMiniBrowserWebBundle.so

[Ravi Phaneendra Kasibhatla]

We encounter following message on MiniBrowser launch:
Error loading the injected bundle (Libraries/.libs/libMiniBrowserWebBundle.so): Libraries/.libs/libMiniBrowserWebBundle.so: cannot open shared object file: No such file or directory

Path of the library file to be corrected.

[Ravi Phaneendra Kasibhatla]

Created attachment 108200 [details]
Corrected libMiniBrowserWebBundle.so loading path

Remove this unrelated change from https://bugs.webkit.org/show_bug.cgi?id=68235 patch and adding it separately.

[Martin Robinson]

Comment on attachment 108200 [details]
Corrected libMiniBrowserWebBundle.so loading path

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

I'm going to add a helper function soon that discovers the binary location. Could this patch be changed to simply find the shared object relative to the binary location after that patch lands?

> Tools/MiniBrowser/gtk/main.c:48
> +    g_free((gpointer)webBundlePath);

It's totally unecessary to cast to gpointer here.

[Ravi Phaneendra Kasibhatla]

(In reply to comment #2)
> (From update of attachment 108200 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=108200&action=review
> 
> I'm going to add a helper function soon that discovers the binary location. Could this patch be changed to simply find the shared object relative to the binary location after that patch lands?
> 
> > Tools/MiniBrowser/gtk/main.c:48
> > +    g_free((gpointer)webBundlePath);
> 
> It's totally unecessary to cast to gpointer here.

Actually I was getting a compiler warning for passing const gchar to g_free though it expects a gpointer. So I casted it.

[Martin Robinson]

Comment on attachment 108200 [details]
Corrected libMiniBrowserWebBundle.so loading path

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

> Tools/MiniBrowser/gtk/main.c:37
> +    const gchar* webBundlePath = g_build_filename(bundleEnvPath ? bundleEnvPath : LIBEXECDIR, "libMiniBrowserWebBundle.so", NULL);

This should not be a const pointer, since g_build_filename allocaetes heap memory to hold the new string.

[Ravi Phaneendra Kasibhatla]

Created attachment 108206 [details]
Corrected libMiniBrowserWebBundle.so loading path

Addressing comments.

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/MiniBrowser/gtk/..." exit_code: 1

Tools/MiniBrowser/gtk/main.c:37:  Declaration has space between * and variable name in gchar* webBundlePath  [whitespace/declaration] [3]
Total errors found: 1 in 3 files


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

[Carlos Garcia Campos]

Comment on attachment 108206 [details]
Corrected libMiniBrowserWebBundle.so loading path

LIBEXECDIR is where executables for the library are installed. In this case, injected bundle is a library, not an executable. MiniBrowser is not installed, and the injected bundle library either.

[Philippe Normand]

Comment on attachment 108206 [details]
Corrected libMiniBrowserWebBundle.so loading path

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

As Martin says in comment 2 he has another plan for this bug as well. Maybe discuss a bit more about the approach to take to fix this bug? r- because of wrong use of libexecdir.

> Tools/MiniBrowser/gtk/GNUmakefile.am:8
> +	-DLIBEXECDIR=\""$(libexecdir)"\" \

This should point to the directory in $(top_builddir) storing the libMiniBrowserWebBundle.so. As Carlos points out it's not installed, so not available outside the build directory.

[Carlos Garcia Campos]

MiniBrowser doesn't use injected bundle anymore, so this problem is now fixed.