Bug 645325 - Part 1: Use NaN to indicate unset optional coordinate values returned from the location providers. r=garvank r?jdm draft
authorChris Peterson <cpeterson@mozilla.com>
Sun, 25 Feb 2018 23:35:16 -0800
changeset 791838 2d9fa5d2cc86c3c0774bee476163a3a9791c982b
parent 779997 0528a414c2a86dad0623779abde5301d37337934
child 791839 c49aae50f4ae12883e8b7392239a444ce2c45869
push id108905
push usercpeterson@mozilla.com
push dateSun, 06 May 2018 06:27:00 +0000
reviewersgarvank, jdm
bugs645325
milestone61.0a1
Bug 645325 - Part 1: Use NaN to indicate unset optional coordinate values returned from the location providers. r=garvank r?jdm nsGeoPositionCoords will convert NaNs returned from the location providers to null properties of the JavaScript Coordinates object. MozReview-Commit-ID: Cmuf2aO0ClD
dom/geolocation/nsGeoPosition.cpp
dom/system/NetworkGeolocationProvider.js
dom/system/linux/GpsdLocationProvider.cpp
dom/system/mac/CoreLocationLocationProvider.mm
dom/system/windows/WindowsLocationProvider.cpp
mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java
widget/android/nsAppShell.cpp
--- a/dom/geolocation/nsGeoPosition.cpp
+++ b/dom/geolocation/nsGeoPosition.cpp
@@ -1,34 +1,69 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsGeoPosition.h"
 
+#include "mozilla/FloatingPoint.h"
 #include "mozilla/dom/PositionBinding.h"
 #include "mozilla/dom/CoordinatesBinding.h"
 
+using mozilla::IsNaN;
+
+// NaN() is a more convenient function name.
+inline
+double NaN()
+{
+  return mozilla::UnspecifiedNaN<double>();
+}
+
+#ifdef DEBUG
+static
+bool EqualOrNaN(double a, double b)
+{
+  return (a == b) || (IsNaN(a) && IsNaN(b));
+}
+#endif
+
 ////////////////////////////////////////////////////
 // nsGeoPositionCoords
 ////////////////////////////////////////////////////
 nsGeoPositionCoords::nsGeoPositionCoords(double aLat, double aLong,
                                          double aAlt, double aHError,
                                          double aVError, double aHeading,
                                          double aSpeed)
   : mLat(aLat)
   , mLong(aLong)
   , mAlt(aAlt)
-  , mHError(aHError)
-  , mVError(aVError)
-  , mHeading(aHeading)
-  , mSpeed(aSpeed)
+  , mHError((aHError >= 0) ? aHError : 0)
+    // altitudeAccuracy without an altitude doesn't make any sense.
+  , mVError((aVError >= 0 && !IsNaN(aAlt)) ? aVError : NaN())
+    // If the hosting device is stationary (i.e. the value of the speed attribute
+    // is 0), then the value of the heading attribute must be NaN (or null).
+  , mHeading((aHeading >= 0 && aHeading < 360 && aSpeed > 0)
+             ? aHeading : NaN())
+  , mSpeed(aSpeed >= 0 ? aSpeed : NaN())
 {
+  // Sanity check the location provider's results in debug builds. If the
+  // location provider is returning bogus results, we'd like to know, but
+  // we prefer to return some position data to JavaScript over a
+  // POSITION_UNAVAILABLE error.
+  MOZ_ASSERT(aLat >= -90 && aLat <= 90);
+  MOZ_ASSERT(aLong >= -180 && aLong <= 180);
+  MOZ_ASSERT(!(aLat == 0 && aLong == 0)); // valid but probably a bug
+
+  MOZ_ASSERT(EqualOrNaN(mAlt, aAlt));
+  MOZ_ASSERT(mHError == aHError);
+  MOZ_ASSERT(EqualOrNaN(mVError, aVError));
+  MOZ_ASSERT(EqualOrNaN(mHeading, aHeading));
+  MOZ_ASSERT(EqualOrNaN(mSpeed, aSpeed));
 }
 
 nsGeoPositionCoords::~nsGeoPositionCoords()
 {
 }
 
 NS_INTERFACE_MAP_BEGIN(nsGeoPositionCoords)
 NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIDOMGeoPositionCoords)
@@ -141,17 +176,16 @@ nsGeoPosition::GetCoords(nsIDOMGeoPositi
 {
   NS_IF_ADDREF(*aCoords = mCoords);
   return NS_OK;
 }
 
 namespace mozilla {
 namespace dom {
 
-
 NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE(Position, mParent, mCoordinates)
 NS_IMPL_CYCLE_COLLECTING_ADDREF(Position)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(Position)
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(Position)
   NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY
   NS_INTERFACE_MAP_ENTRY(nsISupports)
 NS_INTERFACE_MAP_END
 
@@ -238,19 +272,23 @@ Coordinates::name() const               
   mCoords->Get##name(&rv);                   \
   return rv;                                 \
 }
 
 #define GENERATE_COORDS_WRAPPED_GETTER_NULLABLE(name) \
 Nullable<double>                                      \
 Coordinates::Get##name() const                        \
 {                                                     \
-  double rv;                                          \
-  mCoords->Get##name(&rv);                            \
-  return Nullable<double>(rv);                        \
+  double value;                                       \
+  mCoords->Get##name(&value);                         \
+  Nullable<double> rv;                                \
+  if (!IsNaN(value)) {                                \
+    rv.SetValue(value);                               \
+  }                                                   \
+  return rv;                                          \
 }
 
 GENERATE_COORDS_WRAPPED_GETTER(Latitude)
 GENERATE_COORDS_WRAPPED_GETTER(Longitude)
 GENERATE_COORDS_WRAPPED_GETTER_NULLABLE(Altitude)
 GENERATE_COORDS_WRAPPED_GETTER(Accuracy)
 GENERATE_COORDS_WRAPPED_GETTER_NULLABLE(AltitudeAccuracy)
 GENERATE_COORDS_WRAPPED_GETTER_NULLABLE(Heading)
--- a/dom/system/NetworkGeolocationProvider.js
+++ b/dom/system/NetworkGeolocationProvider.js
@@ -197,30 +197,36 @@ function isCachedRequestMoreAccurateThan
     {
      return true;
     }
   }
 
   return false;
 }
 
-function WifiGeoCoordsObject(lat, lon, acc, alt, altacc) {
+function WifiGeoCoordsObject(lat, lon, acc) {
   this.latitude = lat;
   this.longitude = lon;
   this.accuracy = acc;
-  this.altitude = alt;
-  this.altitudeAccuracy = altacc;
+
+  // Neither GLS nor MLS return the following properties, so set them to NaN
+  // here. nsGeoPositionCoords will convert NaNs to null for optional properties
+  // of the JavaScript Coordinates object.
+  this.altitude = NaN;
+  this.altitudeAccuracy = NaN;
+  this.heading = NaN;
+  this.speed = NaN;
 }
 
 WifiGeoCoordsObject.prototype = {
   QueryInterface:  XPCOMUtils.generateQI([Ci.nsIDOMGeoPositionCoords])
 };
 
 function WifiGeoPositionObject(lat, lng, acc) {
-  this.coords = new WifiGeoCoordsObject(lat, lng, acc, 0, 0);
+  this.coords = new WifiGeoCoordsObject(lat, lng, acc);
   this.address = null;
   this.timestamp = Date.now();
 }
 
 WifiGeoPositionObject.prototype = {
   QueryInterface:   XPCOMUtils.generateQI([Ci.nsIDOMGeoPosition])
 };
 
--- a/dom/system/linux/GpsdLocationProvider.cpp
+++ b/dom/system/linux/GpsdLocationProvider.cpp
@@ -212,23 +212,25 @@ protected:
     if (res < 0) {
       return ErrnoToError(errno);
     }
 
     gps_stream(&gpsData, WATCH_ENABLE | WATCH_JSON, NULL);
 
     int err = 0;
 
-    double lat = -1;
-    double lon = -1;
-    double alt = -1;
-    double hError = -1;
-    double vError = -1;
-    double heading = -1;
-    double speed = -1;
+    // nsGeoPositionCoords will convert NaNs to null for optional properties of
+    // the JavaScript Coordinates object.
+    double lat = 0;
+    double lon = 0;
+    double alt = UnspecifiedNaN<double>();
+    double hError = 0;
+    double vError = UnspecifiedNaN<double>();
+    double heading = UnspecifiedNaN<double>();
+    double speed = UnspecifiedNaN<double>();
 
     while (IsRunning()) {
 
       errno = 0;
       auto hasGpsData = gps_waiting(&gpsData, GPSD_WAIT_TIMEOUT_US);
 
       if (errno) {
         err = ErrnoToError(errno);
--- a/dom/system/mac/CoreLocationLocationProvider.mm
+++ b/dom/system/mac/CoreLocationLocationProvider.mm
@@ -8,16 +8,17 @@
 #include "nsCOMPtr.h"
 #include "nsGeoPosition.h"
 #include "nsIConsoleService.h"
 #include "nsServiceManagerUtils.h"
 #include "nsIDOMGeoPositionError.h"
 #include "CoreLocationLocationProvider.h"
 #include "nsCocoaFeatures.h"
 #include "prtime.h"
+#include "mozilla/FloatingPoint.h"
 #include "mozilla/Telemetry.h"
 #include "MLSFallback.h"
 
 #include <CoreLocation/CLError.h>
 #include <CoreLocation/CLLocation.h>
 #include <CoreLocation/CLLocationManager.h>
 #include <CoreLocation/CLLocationManagerDelegate.h>
 
@@ -87,24 +88,46 @@ static const CLLocationAccuracy kDEFAULT
   if (aLocations.count < 1) {
     return;
   }
 
   mProvider->CancelMLSFallbackProvider();
 
   CLLocation* location = [aLocations objectAtIndex:0];
 
+  double altitude;
+  double altitudeAccuracy;
+
+  // A negative verticalAccuracy indicates that the altitude value is invalid.
+  if (location.verticalAccuracy >= 0) {
+    altitude = location.altitude;
+    altitudeAccuracy = location.verticalAccuracy;
+  } else {
+    altitude = UnspecifiedNaN<double>();
+    altitudeAccuracy = UnspecifiedNaN<double>();
+  }
+
+  double speed = location.speed >= 0
+               ? location.speed
+               : UnspecifiedNaN<double>();
+
+  double heading = location.course >= 0
+                 ? location.course
+                 : UnspecifiedNaN<double>();
+
+  // nsGeoPositionCoords will convert NaNs to null for optional properties of
+  // the JavaScript Coordinates object.
   nsCOMPtr<nsIDOMGeoPosition> geoPosition =
     new nsGeoPosition(location.coordinate.latitude,
                       location.coordinate.longitude,
-                      location.altitude,
+                      altitude,
                       location.horizontalAccuracy,
-                      location.verticalAccuracy,
-                      location.course,
-                      location.speed,
+                      altitudeAccuracy,
+                      heading,
+                      speed,
                       PR_Now() / PR_USEC_PER_MSEC);
 
   mProvider->Update(geoPosition);
   Telemetry::Accumulate(Telemetry::GEOLOCATION_OSX_SOURCE_IS_MLS, false);
 }
 @end
 
 NS_IMPL_ISUPPORTS(CoreLocationLocationProvider::MLSUpdate, nsIGeolocationUpdate);
--- a/dom/system/windows/WindowsLocationProvider.cpp
+++ b/dom/system/windows/WindowsLocationProvider.cpp
@@ -5,16 +5,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "WindowsLocationProvider.h"
 #include "nsGeoPosition.h"
 #include "nsIDOMGeoPositionError.h"
 #include "nsComponentManagerUtils.h"
 #include "prtime.h"
 #include "MLSFallback.h"
+#include "mozilla/FloatingPoint.h"
 #include "mozilla/Telemetry.h"
 
 namespace mozilla {
 namespace dom {
 
 NS_IMPL_ISUPPORTS(WindowsLocationProvider::MLSUpdate, nsIGeolocationUpdate);
 
 WindowsLocationProvider::MLSUpdate::MLSUpdate(nsIGeolocationUpdate* aCallback)
@@ -157,27 +158,32 @@ LocationEvent::OnLocationChanged(REFIID 
   }
 
   DOUBLE latitude = 0.0;
   latLongReport->GetLatitude(&latitude);
 
   DOUBLE longitude = 0.0;
   latLongReport->GetLongitude(&longitude);
 
-  DOUBLE alt = 0.0;
+  DOUBLE alt = UnspecifiedNaN<double>();
   latLongReport->GetAltitude(&alt);
 
   DOUBLE herror = 0.0;
   latLongReport->GetErrorRadius(&herror);
 
-  DOUBLE verror = 0.0;
+  DOUBLE verror = UnspecifiedNaN<double>();
   latLongReport->GetAltitudeError(&verror);
 
+  double heading = UnspecifiedNaN<double>();
+  double speed = UnspecifiedNaN<double>();
+
+  // nsGeoPositionCoords will convert NaNs to null for optional properties of
+  // the JavaScript Coordinates object.
   RefPtr<nsGeoPosition> position =
-    new nsGeoPosition(latitude, longitude, alt, herror, verror, 0.0, 0.0,
+    new nsGeoPosition(latitude, longitude, alt, herror, verror, heading, speed,
                       PR_Now() / PR_USEC_PER_MSEC);
   mCallback->Update(position);
 
   Telemetry::Accumulate(Telemetry::GEOLOCATION_WIN8_SOURCE_IS_MLS, false);
 
   return S_OK;
 }
 
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java
@@ -482,20 +482,44 @@ public class GeckoAppShell
 
             GeckoAppShell.onSensorChanged(hal_type, x, y, z, w, accuracy, time);
         }
 
         // Geolocation.
         @Override
         public void onLocationChanged(final Location location) {
             // No logging here: user-identifying information.
+
+            double altitude = location.hasAltitude()
+                            ? location.getAltitude()
+                            : Double.NaN;
+
+            double accuracy = location.hasAccuracy()
+                            ? location.getAccuracy()
+                            : Double.NaN;
+
+            float altitudeAccuracy = Build.VERSION.SDK_INT >= 26 &&
+                                     location.hasVerticalAccuracy()
+                                   ? location.getVerticalAccuracyMeters()
+                                   : Float.NaN;
+
+            float speed = location.hasSpeed()
+                        ? location.getSpeed()
+                        : Float.NaN;
+
+            float heading = location.hasBearing()
+                          ? location.getBearing()
+                          : Float.NaN;
+
+            // nsGeoPositionCoords will convert NaNs to null for optional
+            // properties of the JavaScript Coordinates object.
             GeckoAppShell.onLocationChanged(
                 location.getLatitude(), location.getLongitude(),
-                location.getAltitude(), location.getAccuracy(),
-                location.getBearing(), location.getSpeed(), location.getTime());
+                altitude, accuracy, altitudeAccuracy,
+                heading, speed, location.getTime());
         }
 
         @Override
         public void onProviderDisabled(String provider)
         {
         }
 
         @Override
--- a/widget/android/nsAppShell.cpp
+++ b/widget/android/nsAppShell.cpp
@@ -332,25 +332,26 @@ public:
 
         hal::SensorData sdata(hal::SensorType(aType), aTime, values,
                               hal::SensorAccuracyType(aAccuracy));
         hal::NotifySensorChange(sdata);
     }
 
     static void OnLocationChanged(double aLatitude, double aLongitude,
                                   double aAltitude, float aAccuracy,
-                                  float aBearing, float aSpeed, int64_t aTime)
+                                  float aAltitudeAccuracy,
+                                  float aHeading, float aSpeed, int64_t aTime)
     {
         if (!gLocationCallback) {
             return;
         }
 
         RefPtr<nsIDOMGeoPosition> geoPosition(
                 new nsGeoPosition(aLatitude, aLongitude, aAltitude, aAccuracy,
-                                  aAccuracy, aBearing, aSpeed, aTime));
+                                  aAltitudeAccuracy, aHeading, aSpeed, aTime));
         gLocationCallback->Update(geoPosition);
     }
 
     static void NotifyUriVisited(jni::String::Param aUri)
     {
 #ifdef MOZ_ANDROID_HISTORY
         nsCOMPtr<IHistory> history = services::GetHistoryService();
         nsCOMPtr<nsIURI> visitedURI;