Bug 732923 - Make watchPosition timeouts adhere to spec. r=jdm
authorGuilherme Gonçalves <guilherme.p.gonc@gmail.com>
Wed, 08 Jan 2014 16:04:37 -0500
changeset 178650 44ad28c2159eff4c00262ab236f56631d7dcfde4
parent 178649 1a13c7459626faffb7b805b2189b5720c19bac65
child 178651 57e1604ca1b279efa029a51fec73b025d67c2485
push id3343
push userffxbld
push dateMon, 17 Mar 2014 21:55:32 +0000
treeherdermozilla-beta@2f7d3415f79f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdm
bugs732923
milestone29.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 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
dom/system/NetworkGeolocationProvider.js
testing/mochitest/android.json
xpcom/system/nsIGeolocationProvider.idl
--- a/dom/src/geolocation/nsGeolocation.cpp
+++ b/dom/src/geolocation/nsGeolocation.cpp
@@ -75,16 +75,17 @@ class nsGeolocationRequest
                        PositionOptions* aOptions,
                        bool aWatchPositionRequest = false,
                        int32_t aWatchId = 0);
   void Shutdown();
 
   void SendLocation(nsIDOMGeoPosition* location);
   bool WantsHighAccuracy() {return !mShutdown && mOptions && mOptions->mEnableHighAccuracy;}
   void SetTimeoutTimer();
+  void StopTimeoutTimer();
   void NotifyErrorAndShutdown(uint16_t);
   nsIPrincipal* GetPrincipal();
 
   ~nsGeolocationRequest();
 
   virtual bool Recv__delete__(const bool& allow) MOZ_OVERRIDE;
   virtual void IPDLRelease() MOZ_OVERRIDE { Release(); }
 
@@ -343,35 +344,32 @@ NS_INTERFACE_MAP_END
 NS_IMPL_CYCLE_COLLECTING_ADDREF(nsGeolocationRequest)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(nsGeolocationRequest)
 
 NS_IMPL_CYCLE_COLLECTION_3(nsGeolocationRequest, mCallback, mErrorCallback, mLocator)
 
 NS_IMETHODIMP
 nsGeolocationRequest::Notify(nsITimer* aTimer)
 {
+  StopTimeoutTimer();
   NotifyErrorAndShutdown(nsIDOMGeoPositionError::TIMEOUT);
   return NS_OK;
 }
 
 void
 nsGeolocationRequest::NotifyErrorAndShutdown(uint16_t aErrorCode)
 {
   MOZ_ASSERT(!mShutdown, "timeout after shutdown");
 
   if (!mIsWatchPositionRequest) {
     Shutdown();
     mLocator->RemoveRequest(this);
   }
 
   NotifyError(aErrorCode);
-
-  if (!mShutdown) {
-    SetTimeoutTimer();
-  }
 }
 
 NS_IMETHODIMP
 nsGeolocationRequest::GetPrincipal(nsIPrincipal * *aRequestingPrincipal)
 {
   NS_ENSURE_ARG_POINTER(aRequestingPrincipal);
 
   nsCOMPtr<nsIPrincipal> principal = mLocator->GetPrincipal();
@@ -470,36 +468,42 @@ nsGeolocationRequest::Allow()
   SetTimeoutTimer();
 
   return NS_OK;
 }
 
 void
 nsGeolocationRequest::SetTimeoutTimer()
 {
-  if (mTimeoutTimer) {
-    mTimeoutTimer->Cancel();
-    mTimeoutTimer = nullptr;
-  }
+  StopTimeoutTimer();
 
   int32_t timeout;
   if (mOptions && (timeout = mOptions->mTimeout) != 0) {
 
     if (timeout < 0) {
       timeout = 0;
     } else if (timeout < 10) {
       timeout = 10;
     }
 
     mTimeoutTimer = do_CreateInstance("@mozilla.org/timer;1");
     mTimeoutTimer->InitWithCallback(this, timeout, nsITimer::TYPE_ONE_SHOT);
   }
 }
 
 void
+nsGeolocationRequest::StopTimeoutTimer()
+{
+  if (mTimeoutTimer) {
+    mTimeoutTimer->Cancel();
+    mTimeoutTimer = nullptr;
+  }
+}
+
+void
 nsGeolocationRequest::SendLocation(nsIDOMGeoPosition* aPosition)
 {
   if (mShutdown) {
     // Ignore SendLocationEvents issued before we were cleared.
     return;
   }
 
   nsRefPtr<Position> wrapped, cachedWrapper = mLocator->GetCachedPosition();
@@ -534,22 +538,19 @@ nsGeolocationRequest::SendLocation(nsIDO
     callback->Call(*wrapped, err);
   } else {
     nsIDOMGeoPositionCallback* callback = mCallback.GetXPCOMCallback();
 
     MOZ_ASSERT(callback);
     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*
 nsGeolocationRequest::GetPrincipal()
 {
   if (!mLocator) {
     return nullptr;
   }
@@ -560,16 +561,26 @@ NS_IMETHODIMP
 nsGeolocationRequest::Update(nsIDOMGeoPosition* aPosition)
 {
   nsCOMPtr<nsIRunnable> ev = new RequestSendLocationEvent(aPosition, this);
   NS_DispatchToMainThread(ev);
   return NS_OK;
 }
 
 NS_IMETHODIMP
+nsGeolocationRequest::LocationUpdatePending()
+{
+  if (!mTimeoutTimer) {
+    SetTimeoutTimer();
+  }
+
+  return NS_OK;
+}
+
+NS_IMETHODIMP
 nsGeolocationRequest::NotifyError(uint16_t aErrorCode)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   nsRefPtr<PositionError> positionError = new PositionError(mLocator, aErrorCode);
   positionError->NotifyCallback(mErrorCallback);
   return NS_OK;
 }
@@ -805,16 +816,26 @@ nsGeolocationService::Update(nsIDOMGeoPo
 
   for (uint32_t i = 0; i< mGeolocators.Length(); i++) {
     mGeolocators[i]->Update(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)
 {
   for (uint32_t i = 0; i < mGeolocators.Length(); i++) {
     mGeolocators[i]->NotifyError(aErrorCode);
   }
 
   return NS_OK;
 }
@@ -1126,16 +1147,27 @@ Geolocation::Update(nsIDOMGeoPosition *a
   for (uint32_t i = 0; i < mWatchingCallbacks.Length(); i++) {
     mWatchingCallbacks[i]->Update(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)
 {
   if (!WindowOwnerStillExists()) {
     Shutdown();
     return NS_OK;
   }
 
   for (uint32_t i = mPendingCallbacks.Length(); i > 0; i--) {
--- a/dom/system/NetworkGeolocationProvider.js
+++ b/dom/system/NetworkGeolocationProvider.js
@@ -138,16 +138,19 @@ WifiGeoPositionProvider.prototype = {
     this.highAccuracy = enable;
     LOG("setting highAccuracy to " + (this.highAccuracy?"TRUE":"FALSE"));
   },
 
   onChange: function(accessPoints) {
     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) {
         let mask = "_nomap"
         let result = ap.ssid.indexOf(mask, ap.ssid.length - mask.length) == -1;
         if (result != -1) {
             LOG("Filtering out " + ap.ssid);
         }
--- a/testing/mochitest/android.json
+++ b/testing/mochitest/android.json
@@ -182,16 +182,18 @@
  "dom/tests/mochitest/geolocation/test_handlerSpinsEventLoop.html": "Don't run modal tests on Android",
  "dom/tests/mochitest/geolocation/test_manyCurrentConcurrent.html": "TIMED_OUT",
  "dom/tests/mochitest/geolocation/test_manyCurrentSerial.html": "TIMED_OUT",
  "dom/tests/mochitest/geolocation/test_manyWatchConcurrent.html": "TIMED_OUT",
  "dom/tests/mochitest/geolocation/test_manyWatchSerial.html": "TIMED_OUT",
  "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",
  "dom/tests/mochitest/localstorage/test_localStorageOriginsSchemaDiffs.html": "TIMED_OUT",
  "dom/tests/mochitest/localstorage/test_localStorageOriginsEquals.html": "",
  "dom/tests/mochitest/localstorage/test_localStorageQuota.html": "TIMED_OUT",
  "dom/tests/mochitest/localstorage/test_localStorageQuotaSessionOnly.html": "TIMED_OUT",
  "dom/tests/mochitest/localstorage/test_localStorageQuotaSessionOnly2.html": "TIMED_OUT",
--- a/xpcom/system/nsIGeolocationProvider.idl
+++ b/xpcom/system/nsIGeolocationProvider.idl
@@ -12,27 +12,34 @@ interface nsIDOMElement;
 interface nsIDOMGeoPosition;
 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 {
 
   /**
    * Notify the geolocation service that a new geolocation
    * has been discovered.
    * This must be called on the main thread
    */
   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.
    * The parameter refers to one of the constants in the
    * nsIDOMGeoPositionError interface.
    * Use this to report spurious errors coming from the
    * provider; for errors occurring inside the methods in
    * the nsIGeolocationProvider interface, just use the return
    * value.