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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Mar 22 07:57:32 PDT 2019


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

Michael Catanzaro <mcatanzaro at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mcatanzaro at igalia.com
 Attachment #365531|review?                     |review+
              Flags|                            |

--- Comment #5 from Michael Catanzaro <mcatanzaro at igalia.com> ---
Comment on attachment 365531
  --> https://bugs.webkit.org/attachment.cgi?id=365531
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).

Good job getting rid of the build dependency, btw.

> 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?

-- 
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/20190322/d4d694e4/attachment.html>


More information about the webkit-unassigned mailing list