1430607 - WindowsLocationProvider is using deprecated Windows APIs

Status: RESOLVED DUPLICATE of bug 1980653

(Core :: DOM: Geolocation, enhancement, P5)

Description

[Jonathan Kew [:jfkthame]]

Building with an up-to-date Windows SDK, I notice that we're hitting deprecated-symbol warnings in the WindowsLocationProvider code (see https://treeherder.mozilla.org/#/jobs?repo=try&revision=31d8a205f3720101678b306591271735dfdb3374&selectedJob=156164668).

According to comments in the locationapi.h header, we should avoid ILocation, ILocationEvents, etc., and "use Geolocation WinRT APIs instead".

Comment 1

[Drew Willcoxon :adw]

Is there still interest in doing this? I ended up writing a WinRT patch as part of an exploration for a front-end feature (geolocal search suggestions) and I'd be happy to clean it up. It's pretty simple but I haven't looked into tests.

Comment 2

[Jonathan Kew [:jfkthame]]

I'm not aware of any specific problem this is causing for now, but in general it seems good to move away from explicitly deprecated APIs where possible.

And maybe using the new APIs would be more reliable, and avoid the issue reported in bug 1652303? Worth trying, IMO.

Comment 3

[Drew Willcoxon :adw]

Attachment: Bug 1430607 - Add a WinRT geolocation API implementation.

This adds WinRT Geolocation [1] support to WindowsLocationProvider. The
provider will try to use the WinRT API first and then fall back to the Win32
API. MinGW builds use only the old Win32 API. I tried to generalize the
implementation where I could to support both APIs and to otherwise keep the
existing Win32 code intact.

Similar to the Win32 API, the WinRT API has a function to request location
permission from the user. It also has a function to access a less accurate
"consentless" location that doesn't require user permission. This patch requests
both types of access so that we'll fall back to the consentless location if
necessary. It doesn't seem to be any less accurate than MLS.

There are a couple of other changes worth calling out.

For the WinRT path only, I cache the last known position so we can return it to
the consumer when we're already watching the provider. I added a comment in
Watch() about it. I could do this for the Win32 path too, I'd just need to add
a public method to WindowsLocationProvider or more drastically alter the
architecture (e.g., make LocationEvent a friend class and move it to the
header), but it doesn't seem worth it.

I cache the high-accuracy-enabled boolean for a couple reasons:

(1) IMO the provider should remember its accuracy level in between shutdowns and
restarts so that when it does restart, it uses the requested level. The consumer
actually sets the accuracy before (re)starting the provider, so the current
behavior where we set the level only if mLocation is non-null seems incorrect.

(2) For the WinRT API at least, setting the accuracy causes the geolocator to go
through a reinitialization. Since we create the MLS provider each time that
happens, that seems like a lot of wasted effort. Caching the accuracy level lets
us bail if the new level is the same as the current one.

[1] https://docs.microsoft.com/en-us/uwp/api/windows.devices.geolocation?view=winrt-22621

Comment 4

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Given we now have widget_windows package, maybe worth retrying this on Rust (mostly because the code becomes cleaner when we skip all the legacy WRL thing).

Comment 5

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Fixed in bug 1980653. Unfortunately still C++ with legacy WRL though.