Comment 98 for bug 1100326

(In reply to comment #59)
> (From update of attachment 226084 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=226084&action=review
>
> > Source/WebCore/platform/geoclue/GeolocationProviderGeoclue.h:28
> > +#include <wtf/text/WTFString.h>
>

I'd swear I removed those, but must forgot to add the change to the commit.

> Please, read my previous review, or am I wrong and String is actually used by this header?
>
> > Source/WebCore/platform/geoclue/GeolocationProviderGeoclue.h:55
> > + void setGeoclueManagerProxyAndGetClient(GeoclueManager*);
> > + void setGeoclueClientProxyAndStart(GeoclueClient*);
>
> Are these methods implemented somewhere?
>
Nope, probably the same mistake creating the patch.

> > Source/WebCore/platform/geoclue/GeolocationProviderGeoclue2.cpp:142
> > + // Geoclue2 requires the client to provide a desktop ID for security
> > + // reasons, which should identify the application requesting the location.
> > + geoclue_client_set_desktop_id(provider->m_clientProxy.get(), g_get_prgname());
>
> I still don't know whether using the program name is the right thing in the end or a workaround.
> If it's a workaround, please file a bug report and add a FIXME here pointing to the bug report.
> Because at the moment your comment is confusing here, it says geoclue requires a desktop ID,
> but we are providing an application name

In theory we should provide the "application name" as the desktop ID for geoclue to be able to handle, where "application name" is typically the name of the .desktop file (if any), or what you get from calling g_application_get_application_id().

The problem is that knowing that information from this point seems to be tricky. We couldgo the route of finding the top level GtkWindow and extracting the application ID from there, but then you would be introducing a dependency on GTK here, which I'm not sure that's what we want as, for instance, this provider might be used from other platforms too (e.g EFL).

Other option could be to provide specific API (not now) for the application to specify an "application ID" that webkit might use for different purposes, if present, such as to pass it through geoclue, or even to let the application willing to use WebKit with geolocation capabilities to talk directly to Geoclue to let it know the application ID.

Unfortunately, it is not clear to me which one would be the best option, but introducing the dependency on GTK seems to be the weirdest one, so in the end I preferred to go with g_get_prgname() for now because (1) in many cases it will provide the same string than the GNOME application name and (2) because it should be enough, at least for now, for geoclue to decide whether to whitelist some trusted apps known to use WebKitGTK+, such as epiphany.

So, in a nutshell, whether we should file a bug or not is still not clear to me, but sure I can at list put a FIXME in the code to reflect this situation, so we don't forget about it and file a bug once we agree on whether it's needed or not.

I'll update the patch now and upload a new version soon. Thanks for the review