Bug 645325 - Part 1: Use NaN to indicate unset optional coordinate values returned from the location providers. r=garvank r=jdm
authorChris Peterson <cpeterson@mozilla.com>
Sun, 25 Feb 2018 23:35:16 -0800
changeset 422060 de5472e604faa2f69196f131af198366c2b38e8f
parent 422059 5e0625eb0387a71b26de6a26ef31971d0d28cf3e
child 422061 814c7e1c16e505a87ee1c27b814a00839dba08c8
push id34114
push userbtara@mozilla.com
push dateSat, 09 Jun 2018 15:31:58 +0000
treeherdermozilla-central@e02a5155d815 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgarvank, jdm
bugs645325
milestone62.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
@@ -198,30 +198,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:  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();
 }
 
 WifiGeoPositionObject.prototype = {
   QueryInterface:   ChromeUtils.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
@@ -7,16 +7,17 @@
 #include "nsAutoPtr.h"
 #include "nsCOMPtr.h"
 #include "nsGeoPosition.h"
 #include "nsIConsoleService.h"
 #include "nsServiceManagerUtils.h"
 #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"
 
 #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
@@ -4,16 +4,17 @@
  * 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 "WindowsLocationProvider.h"
 #include "nsGeoPosition.h"
 #include "nsComponentManagerUtils.h"
 #include "prtime.h"
 #include "MLSFallback.h"
+#include "mozilla/FloatingPoint.h"
 #include "mozilla/Telemetry.h"
 #include "mozilla/dom/PositionErrorBinding.h"
 
 namespace mozilla {
 namespace dom {
 
 NS_IMPL_ISUPPORTS(WindowsLocationProvider::MLSUpdate, nsIGeolocationUpdate);
 
@@ -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
@@ -396,17 +396,18 @@ public class GeckoAppShell
 
     @WrapForJNI(calledFrom = "ui", dispatchTo = "gecko")
     /* package */ static native void onSensorChanged(int hal_type, float x, float y, float z,
                                                      float w, int accuracy, long time);
 
     @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,
                                                      NotificationListener,
                                                      ScreenOrientationDelegate,
                                                      WakeLockDelegate,
                                                      HapticFeedbackDelegate {
         @Override
@@ -493,20 +494,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;
+
+            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
         public void onProviderDisabled(String provider)
         {
         }
 
         @Override
--- a/widget/android/nsAppShell.cpp
+++ b/widget/android/nsAppShell.cpp
@@ -338,25 +338,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;