Bug 645325 - Part 1: Use NaN to indicate unset optional coordinate va…

…lues 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

--HG--
extra : rebase_source : cbccead609ff53a3e5f1bcf7698eba7382220ce5
extra : source : b4ced6e2bab2d17cf642f5850bda5998f635fccb

dom/geolocation/nsGeoPosition.cpp

@@ -6,9 +6,27 @@
 
 #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
 ////////////////////////////////////////////////////
@@ -19,11 +37,28 @@ nsGeoPositionCoords::nsGeoPositionCoords(double aLat, double aLong,
   : 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()
@@ -146,7 +181,6 @@ nsGeoPosition::GetCoords(nsIDOMGeoPositionCoords * *aCoords)
 namespace mozilla {
 namespace dom {
 
-
 NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE(Position, mParent, mCoordinates)
 NS_IMPL_CYCLE_COLLECTING_ADDREF(Position)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(Position)
@@ -243,9 +277,13 @@ Coordinates::name() const                    \
 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)

dom/system/NetworkGeolocationProvider.js

@@ -203,20 +203,26 @@ function isCachedRequestMoreAccurateThanServerRequest(newCell, newWifiList)
   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:  ChromeUtils.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();
 }

dom/system/linux/GpsdLocationProvider.cpp

@@ -217,13 +217,15 @@ class GpsdLocationProvider::PollRunnable final : public Runnable
 
     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()) {
 

dom/system/mac/CoreLocationLocationProvider.mm

@@ -12,6 +12,7 @@
 #include "CoreLocationLocationProvider.h"
 #include "nsCocoaFeatures.h"
 #include "prtime.h"
+#include "mozilla/FloatingPoint.h"
 #include "mozilla/Telemetry.h"
 #include "mozilla/dom/PositionErrorBinding.h"
 #include "MLSFallback.h"
@@ -92,14 +93,36 @@ - (void)locationManager:(CLLocationManager*)aManager didUpdateLocations:(NSArray
 
   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);

dom/system/windows/WindowsLocationProvider.cpp

@@ -9,6 +9,7 @@
 #include "nsComponentManagerUtils.h"
 #include "prtime.h"
 #include "MLSFallback.h"
+#include "mozilla/FloatingPoint.h"
 #include "mozilla/Telemetry.h"
 #include "mozilla/dom/PositionErrorBinding.h"
 
@@ -162,17 +163,22 @@ LocationEvent::OnLocationChanged(REFIID aReportType,
   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);
 

mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java

@@ -401,7 +401,8 @@ private static void enableLocationHighAccuracy(final boolean enable) {
     @WrapForJNI(calledFrom = "any", dispatchTo = "gecko")
     /* package */ static native void onLocationChanged(double latitude, double longitude,
                                                        double altitude, float accuracy,
-                                                       float bearing, float speed, long time);
+                                                       float altitudeAccuracy,
+                                                       float heading, float speed, long time);
 
     private static class DefaultListeners implements SensorEventListener,
                                                      LocationListener,
@@ -498,10 +499,34 @@ public void onSensorChanged(SensorEvent s) {
         @Override
         public void onLocationChanged(final Location location) {
             // No logging here: user-identifying information.
+
+            double altitude = location.hasAltitude()
+                            ? location.getAltitude()
+                            : Double.NaN;
+
+            float accuracy = location.hasAccuracy()
+                           ? location.getAccuracy()
+                           : Float.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

widget/android/nsAppShell.cpp

@@ -343,15 +343,16 @@ class GeckoAppShellSupport final
 
     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);
     }