diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index 80269dda626..3f8ee084065 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -2005,14 +2005,20 @@ ContentParent::RecvAddGeolocationListener(const IPC::Principal& aPrincipal) } #endif - if (mGeolocationWatchID == -1) { - nsCOMPtr geo = do_GetService("@mozilla.org/geolocation;1"); - if (!geo) { - return true; - } - jsval dummy = JSVAL_VOID; - geo->WatchPosition(this, nullptr, dummy, nullptr, &mGeolocationWatchID); + // To ensure no geolocation updates are skipped, we always force the + // creation of a new listener. + RecvRemoveGeolocationListener(); + + nsCOMPtr geo = do_GetService("@mozilla.org/geolocation;1"); + if (!geo) { + return true; } + + nsRefPtr geosvc = static_cast(geo.get()); + nsAutoPtr options(new mozilla::dom::GeoPositionOptions()); + jsval null = JS::NullValue(); + options->Init(nullptr, &null); + geosvc->WatchPosition(this, nullptr, options.forget(), &mGeolocationWatchID); return true; } diff --git a/dom/src/geolocation/nsGeoPositionIPCSerialiser.h b/dom/src/geolocation/nsGeoPositionIPCSerialiser.h index 27a388c8754..7fbbabc229c 100644 --- a/dom/src/geolocation/nsGeoPositionIPCSerialiser.h +++ b/dom/src/geolocation/nsGeoPositionIPCSerialiser.h @@ -9,15 +9,14 @@ #include "nsGeoPosition.h" #include "nsIDOMGeoPosition.h" -typedef nsGeoPositionCoords *GeoPositionCoords; -typedef nsIDOMGeoPosition *GeoPosition; +typedef nsIDOMGeoPosition* GeoPosition; namespace IPC { template <> -struct ParamTraits +struct ParamTraits { - typedef GeoPositionCoords paramType; + typedef nsIDOMGeoPositionCoords* paramType; // Function to serialize a geoposition static void Write(Message *aMsg, const paramType& aParam) @@ -96,9 +95,9 @@ struct ParamTraits }; template <> -struct ParamTraits +struct ParamTraits { - typedef GeoPosition paramType; + typedef nsIDOMGeoPosition* paramType; // Function to serialize a geoposition static void Write(Message *aMsg, const paramType& aParam) @@ -114,8 +113,7 @@ struct ParamTraits nsCOMPtr coords; aParam->GetCoords(getter_AddRefs(coords)); - GeoPositionCoords simpleCoords = static_cast(coords.get()); - WriteParam(aMsg, simpleCoords); + WriteParam(aMsg, coords.get()); } // Function to de-serialize a geoposition @@ -131,16 +129,14 @@ struct ParamTraits } DOMTimeStamp timeStamp; - GeoPositionCoords coords = nullptr; + nsIDOMGeoPositionCoords* coords = nullptr; // It's not important to us where it fails, but rather if it fails - if (!( ReadParam(aMsg, aIter, &timeStamp) - && ReadParam(aMsg, aIter, &coords ))) { - // note it is fine to do "delete nullptr" in case coords hasn't - // been allocated - delete coords; - return false; - } + if (!ReadParam(aMsg, aIter, &timeStamp) || + !ReadParam(aMsg, aIter, &coords)) { + nsCOMPtr tmpcoords = coords; + return false; + } *aResult = new nsGeoPosition(coords, timeStamp); diff --git a/dom/src/geolocation/nsGeolocation.cpp b/dom/src/geolocation/nsGeolocation.cpp index a96b04932e5..0fc664b614d 100644 --- a/dom/src/geolocation/nsGeolocation.cpp +++ b/dom/src/geolocation/nsGeolocation.cpp @@ -285,6 +285,7 @@ nsDOMGeoPositionError::NotifyCallback(nsIDOMGeoPositionErrorCallback* aCallback) nsGeolocationRequest::nsGeolocationRequest(nsGeolocation* aLocator, nsIDOMGeoPositionCallback* aCallback, nsIDOMGeoPositionErrorCallback* aErrorCallback, + mozilla::dom::GeoPositionOptions* aOptions, bool aWatchPositionRequest, int32_t aWatchId) : mAllowed(false), @@ -293,6 +294,7 @@ nsGeolocationRequest::nsGeolocationRequest(nsGeolocation* aLocator, mIsWatchPositionRequest(aWatchPositionRequest), mCallback(aCallback), mErrorCallback(aErrorCallback), + mOptions(aOptions), mLocator(aLocator), mWatchId(aWatchId) { @@ -302,15 +304,21 @@ nsGeolocationRequest::~nsGeolocationRequest() { } -nsresult -nsGeolocationRequest::Init(JSContext* aCx, const jsval& aOptions) + +static mozilla::dom::GeoPositionOptions* +OptionsFromJSOptions(JSContext* aCx, const jsval& aOptions, nsresult* aRv) { + *aRv = NS_OK; + nsAutoPtr options(nullptr); if (aCx && !JSVAL_IS_VOID(aOptions) && !JSVAL_IS_NULL(aOptions)) { - mOptions = new mozilla::dom::GeoPositionOptions(); - nsresult rv = mOptions->Init(aCx, &aOptions); - NS_ENSURE_SUCCESS(rv, rv); + options = new mozilla::dom::GeoPositionOptions(); + nsresult rv = options->Init(aCx, &aOptions); + if (NS_FAILED(rv)) { + *aRv = rv; + return nullptr; + } } - return NS_OK; + return options.forget(); } NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsGeolocationRequest) @@ -865,10 +873,6 @@ nsGeolocationService::IsBetterPosition(nsIDOMGeoPosition *aSomewhere) return false; } - if (XRE_GetProcessType() == GeckoProcessType_Content) { - return true; - } - if (mProviders.Count() == 1 || !mLastPosition) { return true; } @@ -1242,8 +1246,20 @@ nsGeolocation::Update(nsIDOMGeoPosition *aSomewhere, bool aIsBetter) NS_IMETHODIMP nsGeolocation::GetCurrentPosition(nsIDOMGeoPositionCallback *callback, nsIDOMGeoPositionErrorCallback *errorCallback, - const jsval& options, + const jsval& jsoptions, JSContext* cx) +{ + nsresult rv; + nsAutoPtr options( + OptionsFromJSOptions(cx, jsoptions, &rv)); + NS_ENSURE_SUCCESS(rv, rv); + return GetCurrentPosition(callback, errorCallback, options.forget()); +} + +nsresult +nsGeolocation::GetCurrentPosition(nsIDOMGeoPositionCallback *callback, + nsIDOMGeoPositionErrorCallback *errorCallback, + mozilla::dom::GeoPositionOptions *options) { NS_ENSURE_ARG_POINTER(callback); @@ -1254,13 +1270,8 @@ nsGeolocation::GetCurrentPosition(nsIDOMGeoPositionCallback *callback, nsRefPtr request = new nsGeolocationRequest(this, callback, errorCallback, + options, false); - if (!request) { - return NS_ERROR_OUT_OF_MEMORY; - } - - nsresult rv = request->Init(cx, options); - NS_ENSURE_SUCCESS(rv, rv); if (!sGeoEnabled) { nsCOMPtr ev = new RequestAllowEvent(false, request); @@ -1307,9 +1318,22 @@ nsGeolocation::GetCurrentPositionReady(nsGeolocationRequest* aRequest) NS_IMETHODIMP nsGeolocation::WatchPosition(nsIDOMGeoPositionCallback *callback, nsIDOMGeoPositionErrorCallback *errorCallback, - const jsval& options, + const jsval& jsoptions, JSContext* cx, int32_t *_retval) +{ + nsresult rv; + nsAutoPtr options( + OptionsFromJSOptions(cx, jsoptions, &rv)); + NS_ENSURE_SUCCESS(rv, rv); + return WatchPosition(callback, errorCallback, options.forget(), _retval); +} + +nsresult +nsGeolocation::WatchPosition(nsIDOMGeoPositionCallback *callback, + nsIDOMGeoPositionErrorCallback *errorCallback, + mozilla::dom::GeoPositionOptions *options, + int32_t *_retval) { NS_ENSURE_ARG_POINTER(callback); @@ -1323,14 +1347,9 @@ nsGeolocation::WatchPosition(nsIDOMGeoPositionCallback *callback, nsRefPtr request = new nsGeolocationRequest(this, callback, errorCallback, + options, true, *_retval); - if (!request) { - return NS_ERROR_OUT_OF_MEMORY; - } - - nsresult rv = request->Init(cx, options); - NS_ENSURE_SUCCESS(rv, rv); if (!sGeoEnabled) { nsCOMPtr ev = new RequestAllowEvent(false, request); diff --git a/dom/src/geolocation/nsGeolocation.h b/dom/src/geolocation/nsGeolocation.h index 6fe207915ba..8f59322dc6f 100644 --- a/dom/src/geolocation/nsGeolocation.h +++ b/dom/src/geolocation/nsGeolocation.h @@ -53,9 +53,9 @@ class nsGeolocationRequest nsGeolocationRequest(nsGeolocation* locator, nsIDOMGeoPositionCallback* callback, nsIDOMGeoPositionErrorCallback* errorCallback, + mozilla::dom::GeoPositionOptions* aOptions, bool watchPositionRequest = false, int32_t watchId = 0); - nsresult Init(JSContext* aCx, const jsval& aOptions); void Shutdown(); // Called by the geolocation device to notify that a location has changed. @@ -75,7 +75,6 @@ class nsGeolocationRequest void IPDLRelease() { Release(); } int32_t WatchId() { return mWatchId; } - private: void NotifyError(int16_t errorCode); @@ -202,6 +201,14 @@ public: // Notification from the service: void ServiceReady(); + // Versions of the DOM APIs that don't require JS option values + nsresult WatchPosition(nsIDOMGeoPositionCallback *callback, + nsIDOMGeoPositionErrorCallback *errorCallback, + mozilla::dom::GeoPositionOptions *options, + int32_t *_retval); + nsresult GetCurrentPosition(nsIDOMGeoPositionCallback *callback, + nsIDOMGeoPositionErrorCallback *errorCallback, + mozilla::dom::GeoPositionOptions *options); private: ~nsGeolocation(); diff --git a/dom/system/NetworkGeolocationProvider.js b/dom/system/NetworkGeolocationProvider.js index 05d5955f7b0..d9ff63579cd 100755 --- a/dom/system/NetworkGeolocationProvider.js +++ b/dom/system/NetworkGeolocationProvider.js @@ -15,7 +15,7 @@ const Cc = Components.classes; let gLoggingEnabled = false; let gTestingEnabled = false; -let gDesist = false; +let gUseScanning = true; let gPrivateAccessToken = ''; let gPrivateAccessTime = 0; @@ -76,6 +76,10 @@ function WifiGeoPositionProvider() { gTestingEnabled = Services.prefs.getBoolPref("geo.wifi.testing"); } catch (e) {} + try { + gUseScanning = Services.prefs.getBoolPref("geo.wifi.scan"); + } catch (e) {} + this.wifiService = null; this.timer = null; this.hasSeenWiFi = false; @@ -111,20 +115,20 @@ WifiGeoPositionProvider.prototype = { watch: function(c, requestPrivate) { LOG("watch called"); - let useScanning = true; - try { - useScanning = Services.prefs.getBoolPref("geo.wifi.scan"); - } catch (e) {} - - if (!this.wifiService && useScanning) { + if (!this.wifiService && gUseScanning) { this.wifiService = Cc["@mozilla.org/wifi/monitor;1"].getService(Components.interfaces.nsIWifiMonitor); this.wifiService.startWatching(this); this.lastRequestPrivate = requestPrivate; } if (this.hasSeenWiFi) { this.hasSeenWiFi = false; - this.wifiService.stopWatching(this); - this.wifiService.startWatching(this); + if (gUseScanning) { + this.wifiService.stopWatching(this); + this.wifiService.startWatching(this); + } else { + // For testing situations, ensure that we always trigger an update. + this.timer.initWithCallback(this, 5000, this.timer.TYPE_ONE_SHOT); + } this.lastRequestPrivate = requestPrivate; } }, @@ -314,7 +318,7 @@ WifiGeoPositionProvider.prototype = { }, notify: function (timer) { - if (gTestingEnabled) { + if (gTestingEnabled || !gUseScanning) { // if we are testing, timer is repeating this.onChange(null); } diff --git a/dom/tests/unit/test_multiple_geo_listeners.js b/dom/tests/unit/test_multiple_geo_listeners.js new file mode 100644 index 00000000000..6318f8fa240 --- /dev/null +++ b/dom/tests/unit/test_multiple_geo_listeners.js @@ -0,0 +1,69 @@ +const Cc = Components.classes; +const Ci = Components.interfaces; +const Cu = Components.utils; +const Cr = Components.results; + +Cu.import("resource://testing-common/httpd.js"); + +var httpserver = null; +var geolocation = null; +var success = false; +var watchId = -1; + +let gAccuracy = 42; +function geoHandler(metadata, response) +{ + var georesponse = { + status: "OK", + location: { + lat: 0.45, + lng: 0.45, + }, + accuracy: gAccuracy, + }; + var position = JSON.stringify(georesponse); + response.setStatusLine("1.0", 200, "OK"); + response.setHeader("Cache-Control", "no-cache", false); + response.setHeader("Content-Type", "aplication/x-javascript", false); + response.write(position); +} + +function errorCallback() { + do_check_true(false); + do_test_finished(); +} + +function run_test() +{ + do_test_pending(); + + httpserver = new HttpServer(); + httpserver.registerPathHandler("/geo", geoHandler); + httpserver.start(4444); + + if (Cc["@mozilla.org/xre/app-info;1"].getService(Ci.nsIXULRuntime) + .processType == Ci.nsIXULRuntime.PROCESS_TYPE_DEFAULT) { + var prefs = Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefBranch); + prefs.setBoolPref("geo.wifi.scan", false); + prefs.setCharPref("geo.wifi.uri", "http://localhost:4444/geo"); + } + + let timesCalled = 0; + geolocation = Cc["@mozilla.org/geolocation;1"].createInstance(Ci.nsIDOMGeoGeolocation); + geolocation.watchPosition(function(pos) { + do_check_eq(++timesCalled, 1); + do_check_eq(pos.coords.accuracy, gAccuracy); + + gAccuracy = 420; + geolocation2 = Cc["@mozilla.org/geolocation;1"].createInstance(Ci.nsIDOMGeoGeolocation); + geolocation2.getCurrentPosition(function(pos) { + do_check_eq(pos.coords.accuracy, gAccuracy); + + gAccuracy = 400; + geolocation2.getCurrentPosition(function(pos) { + do_check_eq(pos.coords.accuracy, 42); + do_test_finished(); + }, errorCallback); + }, errorCallback, {maximumAge: 0}); + }, errorCallback, {maximumAge: 0}); +} diff --git a/dom/tests/unit/test_multiple_geo_listeners_wrap.js b/dom/tests/unit/test_multiple_geo_listeners_wrap.js new file mode 100644 index 00000000000..a2538318465 --- /dev/null +++ b/dom/tests/unit/test_multiple_geo_listeners_wrap.js @@ -0,0 +1,9 @@ +const Cc = Components.classes; +const Ci = Components.interfaces; + +function run_test() { + var prefs = Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefBranch); + prefs.setBoolPref("geo.wifi.scan", false); + prefs.setCharPref("geo.wifi.uri", "http://localhost:4444/geo"); + run_test_in_child("test_multiple_geo_listeners.js"); +} \ No newline at end of file diff --git a/dom/tests/unit/xpcshell.ini b/dom/tests/unit/xpcshell.ini index 737b117db18..8103289d877 100644 --- a/dom/tests/unit/xpcshell.ini +++ b/dom/tests/unit/xpcshell.ini @@ -5,8 +5,11 @@ tail = [test_bug319968.js] [test_bug465752.js] [test_geolocation_provider.js] +# Bug 684962: test hangs consistently on Android +skip-if = os == "android" [test_geolocation_timeout.js] [test_geolocation_timeout_wrap.js] skip-if = os == "mac" -# Bug 684962: test hangs consistently on Android -skip-if = os == "android" +[test_multiple_geo_listeners.js] +[test_multiple_geo_listeners_wrap.js] +skip-if = os == "mac"