Bug 732923 - Make watchPosition timeouts adhere to spec. r=jdm

This also disables a couple of tests that depend on the network
provider on Android. These tests were passing by accident before
this patch (and started failing afterwards), and were never meant
to run on Android anyway.

dom/src/geolocation/nsGeolocation.cpp

@@ -80,6 +80,7 @@ class nsGeolocationRequest
   void SendLocation(nsIDOMGeoPosition* location);
   bool WantsHighAccuracy() {return !mShutdown && mOptions && mOptions->mEnableHighAccuracy;}
   void SetTimeoutTimer();
+  void StopTimeoutTimer();
   void NotifyErrorAndShutdown(uint16_t);
   nsIPrincipal* GetPrincipal();
 
@@ -348,6 +349,7 @@ NS_IMPL_CYCLE_COLLECTION_3(nsGeolocationRequest, mCallback, mErrorCallback, mLoc
 NS_IMETHODIMP
 nsGeolocationRequest::Notify(nsITimer* aTimer)
 {
+  StopTimeoutTimer();
   NotifyErrorAndShutdown(nsIDOMGeoPositionError::TIMEOUT);
   return NS_OK;
 }
@@ -363,10 +365,6 @@ nsGeolocationRequest::NotifyErrorAndShutdown(uint16_t aErrorCode)
   }
 
   NotifyError(aErrorCode);
-
-  if (!mShutdown) {
-    SetTimeoutTimer();
-  }
 }
 
 NS_IMETHODIMP
@@ -475,10 +473,7 @@ nsGeolocationRequest::Allow()
 void
 nsGeolocationRequest::SetTimeoutTimer()
 {
-  if (mTimeoutTimer) {
-    mTimeoutTimer->Cancel();
-    mTimeoutTimer = nullptr;
-  }
+  StopTimeoutTimer();
 
   int32_t timeout;
   if (mOptions && (timeout = mOptions->mTimeout) != 0) {
@@ -494,6 +489,15 @@ nsGeolocationRequest::SetTimeoutTimer()
   }
 }
 
+void
+nsGeolocationRequest::StopTimeoutTimer()
+{
+  if (mTimeoutTimer) {
+    mTimeoutTimer->Cancel();
+    mTimeoutTimer = nullptr;
+  }
+}
+
 void
 nsGeolocationRequest::SendLocation(nsIDOMGeoPosition* aPosition)
 {
@@ -539,12 +543,9 @@ nsGeolocationRequest::SendLocation(nsIDOMGeoPosition* aPosition)
     callback->HandleEvent(aPosition);
   }
 
-  if (!mShutdown) {
-    // For watch requests, the handler may have called clearWatch
-    MOZ_ASSERT(mIsWatchPositionRequest,
-               "non-shutdown getCurrentPosition request after callback!");
-    SetTimeoutTimer();
-  }
+  StopTimeoutTimer();
+  MOZ_ASSERT(mShutdown || mIsWatchPositionRequest,
+             "non-shutdown getCurrentPosition request after callback!");
 }
 
 nsIPrincipal*
@@ -564,6 +565,16 @@ nsGeolocationRequest::Update(nsIDOMGeoPosition* aPosition)
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsGeolocationRequest::LocationUpdatePending()
+{
+  if (!mTimeoutTimer) {
+    SetTimeoutTimer();
+  }
+
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 nsGeolocationRequest::NotifyError(uint16_t aErrorCode)
 {
@@ -809,6 +820,16 @@ nsGeolocationService::Update(nsIDOMGeoPosition *aSomewhere)
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsGeolocationService::LocationUpdatePending()
+{
+  for (uint32_t i = 0; i< mGeolocators.Length(); i++) {
+    mGeolocators[i]->LocationUpdatePending();
+  }
+
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 nsGeolocationService::NotifyError(uint16_t aErrorCode)
 {
@@ -1130,6 +1151,17 @@ Geolocation::Update(nsIDOMGeoPosition *aSomewhere)
   return NS_OK;
 }
 
+NS_IMETHODIMP
+Geolocation::LocationUpdatePending()
+{
+  // this event is only really interesting for watch callbacks
+  for (uint32_t i = 0; i < mWatchingCallbacks.Length(); i++) {
+    mWatchingCallbacks[i]->LocationUpdatePending();
+  }
+
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 Geolocation::NotifyError(uint16_t aErrorCode)
 {

dom/system/NetworkGeolocationProvider.js

@@ -143,6 +143,9 @@ WifiGeoPositionProvider.prototype = {
     LOG("onChange called, highAccuracy = " + (this.highAccuracy?"TRUE":"FALSE"));
     this.hasSeenWiFi = true;
 
+    Cc["@mozilla.org/geolocation/service;1"].getService(Ci.nsIGeolocationUpdate)
+        .locationUpdatePending();
+
     let url = Services.urlFormatter.formatURLPref("geo.wifi.uri");
 
     function isPublic(ap) {

testing/mochitest/android.json

@@ -187,6 +187,8 @@
  "dom/tests/mochitest/geolocation/test_mozsettings.html": "mozSettings is undefined",
  "dom/tests/mochitest/geolocation/test_mozsettingsWatch.html": "mozSettings is undefined",
  "dom/tests/mochitest/geolocation/test_shutdown.html": "TIMED_OUT",
+ "dom/tests/mochitest/geolocation/test_timeoutWatch.html": "TIMED_OUT",
+ "dom/tests/mochitest/geolocation/test_timerRestartWatch.html": "TIMED_OUT",
  "dom/tests/mochitest/geolocation/test_windowClose.html": "TIMED_OUT",
  "dom/tests/mochitest/geolocation/test_worseAccuracyDoesNotBlockCallback.html": "TIMED_OUT",
  "dom/tests/mochitest/localstorage/test_bug624047.html": "TIMED_OUT",

xpcom/system/nsIGeolocationProvider.idl

@@ -17,7 +17,7 @@ interface nsIGeolocationPrompt;
  * Interface provides a way for a geolocation provider to
  * notify the system that a new location is available.
  */
-[scriptable, uuid(f00ff730-acff-4e8c-9991-0d4c84ba0e10)]
+[scriptable, uuid(643dc5e9-b911-4b2c-8d44-603162696baf)]
 interface nsIGeolocationUpdate : nsISupports {
 
   /**
@@ -27,6 +27,13 @@ interface nsIGeolocationUpdate : nsISupports {
    */
   void update(in nsIDOMGeoPosition position);
 
+  /**
+   * Notify the geolocation service that the location has
+   * potentially changed, and thus a new position is in the
+   * process of being acquired.
+   */
+  void locationUpdatePending();
+
   /**
    * Notify the geolocation service of an error.
    * This must be called on the main thread.