Bug 1900225: Part 6 - Wait for Windows permission dialog to be dismissed before requesting geolocation r=emilio,win-reviewers,gstoll

When Windows presents the dialog asking the user to give geolocation permission
to Firefox, we need to wait for the user to make a choice before running the
geolocation request.    Previously, we were not waiting for the user's response
so most requests would timeout and fail, even if the user replied "Yes".

This dialog is only ever presented once -- the first time that Firefox asks
Windows for a wifi scan.  It does not reappear on restarts or reinstalls.   This
dedicated Yes/No system prompt is a bit more user-friendly than system settings.

This "system will prompt for permission" case could be completely avoided since
wifi scanning is not useful to us when it requires geolocation permissions as
geolocation would override it.  We would need the MLSFallback behavior to skip
scanning wifi when geolocation permission wasn't already granted, or else the
MLSFallback would present the system prompt in question, despite the user having
already denied permission.  On the other hand, we need the old scanning behavior
for this case when running versions of Windows 10 and 11 that don't have these
updates.  (The updates are set to appear in the 24H2 version of Windows 11.)
This approach avoids that kind of version special-casing.

Differential Revision: https://phabricator.services.mozilla.com/D218589

dom/geolocation/Geolocation.cpp

@@ -343,8 +343,8 @@ void Geolocation::ReallowWithSystemPermissionOrCancel(
   denyPermissionOnError.release();
 
   RefPtr<SystemGeolocationPermissionRequest> permissionRequest =
-      geolocation::PresentSystemSettings(aBrowsingContext,
-                                         std::move(aResolver));
+      geolocation::RequestLocationPermissionFromUser(aBrowsingContext,
+                                                     std::move(aResolver));
   NS_ENSURE_TRUE_VOID(permissionRequest);
 
   auto cancelRequestOnError = MakeScopeExit([&]() {
@@ -383,12 +383,13 @@ nsGeolocationRequest::Allow(JS::Handle<JS::Value> aChoices) {
     return NS_OK;
   }
 
-  if (mBehavior == SystemGeolocationPermissionBehavior::GeckoWillPromptUser) {
-    // Asynchronously present the system dialog and wait for the permission to
-    // change or the request to be canceled.  If the permission is (maybe)
-    // granted then it will call Allow again.  It actually will also re-call
-    // Allow if the permission is denied, in order to get the "denied
-    // permission" behavior.
+  if (mBehavior != SystemGeolocationPermissionBehavior::NoPrompt) {
+    // Asynchronously present the system dialog or open system preferences
+    // (RequestGeolocationPermissionFromUser will know which to do), and wait
+    // for the permission to change or the request to be canceled.  If the
+    // permission is (maybe) granted then it will call Allow again.  It actually
+    // will also re-call Allow if the permission is denied, in order to get the
+    // "denied permission" behavior.
     mBehavior = SystemGeolocationPermissionBehavior::NoPrompt;
     RefPtr<BrowsingContext> browsingContext = mWindow->GetBrowsingContext();
     if (ContentChild* cc = ContentChild::GetSingleton()) {

dom/geolocation/GeolocationSystem.cpp

@@ -12,8 +12,9 @@ SystemGeolocationPermissionBehavior GetGeolocationPermissionBehavior() {
   return SystemGeolocationPermissionBehavior::NoPrompt;
 }
 
-already_AddRefed<SystemGeolocationPermissionRequest> PresentSystemSettings(
-    BrowsingContext* aBrowsingContext, ParentRequestResolver&& aResolver) {
+already_AddRefed<SystemGeolocationPermissionRequest>
+RequestLocationPermissionFromUser(BrowsingContext* aBrowsingContext,
+                                  ParentRequestResolver&& aResolver) {
   MOZ_ASSERT_UNREACHABLE(
       "Should not warn user of need for system location permission "
       "since we cannot open system settings on this platform.");

dom/geolocation/GeolocationSystem.h

@@ -41,12 +41,13 @@ using ParentRequestResolver =
     PContentParent::RequestGeolocationPermissionFromUserResolver;
 
 /**
- * Opens the relevant (system or Gecko) window to request permission from the
- * user. Resolves aResolver when permission is granted, an error occurs, or Stop
- * has been called on the SystemGeolocationPermissionRequest.
+ * Opens the relevant system dialog to request permission from the user.
+ * Resolves aResolver when permission is granted, an error occurs, or Stop has
+ * been called on the SystemGeolocationPermissionRequest.
  */
-already_AddRefed<SystemGeolocationPermissionRequest> PresentSystemSettings(
-    BrowsingContext* aBrowsingContext, ParentRequestResolver&& aResolver);
+already_AddRefed<SystemGeolocationPermissionRequest>
+RequestLocationPermissionFromUser(BrowsingContext* aBrowsingContext,
+                                  ParentRequestResolver&& aResolver);
 
 }  // namespace geolocation
 }  // namespace mozilla::dom

dom/geolocation/GeolocationSystemWin.cpp

@@ -5,9 +5,11 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "GeolocationSystem.h"
+#include "mozilla/Components.h"
 #include "mozilla/dom/BrowsingContext.h"
 #include "mozilla/ScopeExit.h"
 #include "nsIGeolocationUIUtilsWin.h"
+#include "nsIWifiListener.h"
 #include "nsIWifiMonitor.h"
 
 #include <windows.system.h>
@@ -70,10 +72,7 @@ bool SystemWillPromptForPermissionHint() {
   // If wifi is not available (e.g. because there is no wifi device present)
   // then the API may report that Windows will request geolocation permission
   // but it can't without the wifi scanner.  Check for that case.
-  nsresult rv = NS_OK;
-  nsCOMPtr<nsIWifiMonitor> wifiMonitor =
-      do_GetService("@mozilla.org/wifi/monitor;1", &rv);
-  NS_ENSURE_SUCCESS(rv, false);
+  nsCOMPtr<nsIWifiMonitor> wifiMonitor = components::WifiMonitor::Service();
   NS_ENSURE_TRUE(wifiMonitor, false);
   return wifiMonitor->GetHasWifiAdapter();
 }
@@ -268,6 +267,46 @@ void OpenWindowsLocationSettings(
   cancelRequest.release();
 }
 
+class LocationPermissionWifiScanListener final : public nsIWifiListener {
+ public:
+  NS_DECL_ISUPPORTS
+
+  explicit LocationPermissionWifiScanListener(
+      SystemGeolocationPermissionRequest* aRequest)
+      : mRequest(aRequest) {}
+
+  NS_IMETHOD OnChange(const nsTArray<RefPtr<nsIWifiAccessPoint>>&) override {
+    // We will remove ourselves from the nsIWifiMonitor, which is our owner.
+    // Hold a strong reference to ourselves until we complete the callback.
+    RefPtr<LocationPermissionWifiScanListener> self = this;
+    return PermissionWasDecided();
+  }
+
+  NS_IMETHOD OnError(nsresult) override {
+    // We will remove ourselves from the nsIWifiMonitor, which is our owner.
+    // Hold a strong reference to ourselves until we complete the callback.
+    RefPtr<LocationPermissionWifiScanListener> self = this;
+    return PermissionWasDecided();
+  }
+
+ private:
+  virtual ~LocationPermissionWifiScanListener() = default;
+  RefPtr<SystemGeolocationPermissionRequest> mRequest;
+
+  // Any response to our wifi scan request means that the user has selected
+  // either to grant or deny permission in the Windows dialog.  Either way, we
+  // are done asking for permission, so Stop the permission request.
+  nsresult PermissionWasDecided() {
+    nsCOMPtr<nsIWifiMonitor> wifiMonitor = components::WifiMonitor::Service();
+    NS_ENSURE_TRUE(wifiMonitor, NS_ERROR_FAILURE);
+    wifiMonitor->StopWatching(this);
+    mRequest->Stop();
+    return NS_OK;
+  }
+};
+
+NS_IMPL_ISUPPORTS(LocationPermissionWifiScanListener, nsIWifiListener)
+
 }  // namespace
 
 //-----------------------------------------------------------------------------
@@ -282,8 +321,9 @@ SystemGeolocationPermissionBehavior GetGeolocationPermissionBehavior() {
   return SystemGeolocationPermissionBehavior::NoPrompt;
 }
 
-already_AddRefed<SystemGeolocationPermissionRequest> PresentSystemSettings(
-    BrowsingContext* aBrowsingContext, ParentRequestResolver&& aResolver) {
+already_AddRefed<SystemGeolocationPermissionRequest>
+RequestLocationPermissionFromUser(BrowsingContext* aBrowsingContext,
+                                  ParentRequestResolver&& aResolver) {
   RefPtr<WindowsGeolocationPermissionRequest> permissionRequest =
       new WindowsGeolocationPermissionRequest(aBrowsingContext,
                                               std::move(aResolver));
@@ -291,7 +331,20 @@ already_AddRefed<SystemGeolocationPermissionRequest> PresentSystemSettings(
   if (permissionRequest->IsStopped()) {
     return nullptr;
   }
-  OpenWindowsLocationSettings(permissionRequest);
+  if (SystemWillPromptForPermissionHint()) {
+    // To tell the system to prompt for permission, run one wifi scan (no need
+    // to poll). We won't use the result -- either the user will grant
+    // geolocation permission, meaning we will not need wifi scanning, or the
+    // user will deny permission, in which case no scan can be done.  We just
+    // want the prompt.
+    nsCOMPtr<nsIWifiMonitor> wifiMonitor = components::WifiMonitor::Service();
+    NS_ENSURE_TRUE(wifiMonitor, nullptr);
+    auto listener =
+        MakeRefPtr<LocationPermissionWifiScanListener>(permissionRequest);
+    wifiMonitor->StartWatching(listener, false);
+  } else {
+    OpenWindowsLocationSettings(permissionRequest);
+  }
   return permissionRequest.forget();
 }
 

netwerk/build/components.conf

@@ -764,6 +764,7 @@ Classes = [
 if defined('NECKO_WIFI'):
     Classes += [
         {
+            'name': 'WifiMonitor',
             'cid': '{3ff8fb9f-ee63-48df-89f0-dace0242fd82}',
             'contract_ids': ['@mozilla.org/wifi/monitor;1'],
             'singleton': True,