[Webkit-unassigned] [Bug 195994] [GTK] Remove build time dependency on Geoclue2

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=195994

--- Comment #6 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to Michael Catanzaro from comment #5)
> Comment on attachment 365531 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=365531&action=review
> 
> Can we have WebKitGeolocationManager own the GeoclueGeolocationProvider,
> instead of making it a singleton? We know from experience that singleton
> classes are dangerous/problematic due to order-of-initialization and
> order-of-destruction issues, so it's good to avoid singletons whenever
> possible. Consider also that the WebKitGeolocationManager is owned by
> WebKitWebContext, so we can have multiple unrelated
> WebKitGeolocationManagers in the same process that will be fighting with
> each other to control the singleton GeoclueGeolocationProvider. E.g. if one
> WebKitGeolocationManager executes
> GeoclueGeolocationProvider::singleton().start(), the other might execute
> GeoclueGeolocationProvider::singleton().stop() and defeat it, and that's no
> good. So I think using the singleton is just not right here. r=me assumes
> you change this (or add some sort of refcounting to the start/stop requests
> to ensure one WebKitGeolocationManager cannot stomp on another by turning
> off the geolocation, but making it non-singleton seems better; it's OK to
> have multiple clients of the same D-Bus service in the same process IMO).

I don't think there's any problem with this singleton from the point of view of construction and destruction order (it's never destroyed indeed), but you are right we should have one per process pool.

> Good job getting rid of the build dependency, btw.

Thanks

> > Source/WebKit/UIProcess/geoclue/GeoclueGeolocationProvider.cpp:35
> > +#if USE(GLIB_EVENT_LOOP)
> > +#include <wtf/glib/RunLoopSourcePriority.h>
> > +#endif
> 
> It's already a GLib-specific file, do we really need #if
> USE(GLIB_EVENT_LOOP) guards?

It's consistent with the definition of setProperty in RunLoop, it's harmless in any case

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20190323/041b3dcf/attachment.html>