[webkit-reviews] review requested: [Bug 35191] [Gtk] use geoclue providers with don't provide update : [Attachment 49357] patch v1.3

[bugzilla-daemon at webkit.org]

arno. <arno at renevier.net> has asked  for review:
Bug 35191: [Gtk] use geoclue providers with don't provide update
https://bugs.webkit.org/show_bug.cgi?id=35191

Attachment 49357: patch v1.3
https://bugs.webkit.org/attachment.cgi?id=49357&action=review

------- Additional Comments from arno. <arno at renevier.net>
(In reply to comment #7)
> (From update of attachment 49322 [details])
>  8	     No new tests. (OOPS!)
> 
> You should state here that there's no testable behaviour change, (I assume
> there isn't because this depends on the system).

ok, here is an updated patch

> Also, you're not doing only
> what you claim in the changelog, in this patch. You're also making updating
the
> location information async, which does sound like a good idea, but should be
> made into another patch. r- for that reason.

asynchronous reply is needed here. As explained in comment 1, if we return a
position synchronously from startUpdating, callback fonctions will not be
called. Currently (providers providing update only), position return is
asynchronous. If we use providers which don't provide updates, we need to get
position asynchronously (otherwise, geolocation simply won't work).

> Also, we had many crashes happen because the WebCore object goes away between

> an async call, and its callback returning, so passin 'this' here worries me:
> 
>  118	   geoclue_position_get_position_async(m_geocluePosition,
> (GeocluePositionCallback)get_position, this);
> 
> Have you checked what the life cycle of the geolocation service object is,
and
> if it will react well if its client goes away?

I've tested what happens when page is unloaded before position is available.
GeolocationService is destroyed and get_position is not called, and no crash
append.