Bug 489813 - fire geolocation timeout error if the provider fails to response after initial response. r=olli
authorDoug Turner <dougt@dougt.org>
Tue, 23 Feb 2010 20:27:27 -0800
changeset 38451 0542680c6f745032675cd997c9e9969002f480d2
parent 38450 f59d3c5a7ac730aeecf41031e297a7188830969b
child 38458 1fb0da119b03dc68692a73920574ac9002403d6f
child 38459 f2eab8c52567721a0e660361101d883fe100d519
push id11774
push userdougt@mozilla.com
push dateWed, 24 Feb 2010 04:46:06 +0000
treeherdermozilla-central@0542680c6f74 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersolli
bugs489813
milestone1.9.3a2pre
Bug 489813 - fire geolocation timeout error if the provider fails to response after initial response. r=olli
build/automation.py.in
dom/src/geolocation/NetworkGeolocationProvider.js
dom/src/geolocation/nsGeolocation.cpp
dom/src/geolocation/nsGeolocation.h
dom/tests/mochitest/geolocation/Makefile.in
dom/tests/mochitest/geolocation/test_timerRestartWatch.html
--- a/build/automation.py.in
+++ b/build/automation.py.in
@@ -271,16 +271,17 @@ user_pref("network.manage-offline-status
 user_pref("test.mousescroll", true);
 user_pref("security.default_personal_cert", "Select Automatically"); // Need to client auth test be w/o any dialogs
 user_pref("network.http.prompt-temp-redirect", false);
 user_pref("media.cache_size", 100);
 user_pref("security.warn_viewing_mixed", false);
 
 user_pref("geo.wifi.uri", "http://localhost:8888/tests/dom/tests/mochitest/geolocation/network_geolocation.sjs");
 user_pref("geo.wifi.testing", true);
+user_pref("geo.ignore.location_filter", true);
 
 user_pref("camino.warn_when_closing", false); // Camino-only, harmless to others
 
 // Make url-classifier updates so rare that they won't affect tests
 user_pref("urlclassifier.updateinterval", 172800);
 // Point the url-classifier to the local testing server for fast failures
 user_pref("browser.safebrowsing.provider.0.gethashURL", "http://localhost:8888/safebrowsing-dummy/gethash");
 user_pref("browser.safebrowsing.provider.0.keyURL", "http://localhost:8888/safebrowsing-dummy/newkey");
--- a/dom/src/geolocation/NetworkGeolocationProvider.js
+++ b/dom/src/geolocation/NetworkGeolocationProvider.js
@@ -232,43 +232,34 @@ WifiGeoPositionProvider.prototype = {
     classID:          Components.ID("{77DA64D3-7458-4920-9491-86CC9914F904}"),
     contractID:       "@mozilla.org/geolocation/provider;1",
     QueryInterface:   XPCOMUtils.generateQI([Ci.nsIGeolocationProvider,
                                              Ci.nsIWifiListener,
                                              Ci.nsITimerCallback]),
 
     prefService:     null,
 
-    provider_url:    null,
     wifi_service:    null,
     timer:           null,
     protocol:        null,
     hasSeenWiFi:     false,
 
     startup:         function() {
-        LOG("startup called");
-
-        this.provider_url = this.prefService.getCharPref("geo.wifi.uri");
-        LOG("provider url = " + this.provider_url);
 
-        try {
-            this.protocol = this.prefService.getIntPref("geo.wifi.protocol");
-            LOG("protocol = " + this.protocol);
-        } catch (e) {
-            this.protocol = 0;
-        }
+        LOG("startup called.  testing mode is" + gTestingEnabled);
+
         // if we don't see anything in 5 seconds, kick of one IP geo lookup.
         // if we are testing, just hammer this callback so that we are more or less
         // always sending data.  It doesn't matter if we have an access point or not.
         this.hasSeenWiFi = false;
         this.timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
         if (gTestingEnabled == false)
             this.timer.initWithCallback(this, 5000, this.timer.TYPE_ONE_SHOT);
         else
-            this.timer.initWithCallback(this, 200, this.timer.TYPE_REPEATING_SLACK);
+            this.timer.initWithCallback(this, 750, this.timer.TYPE_REPEATING_SLACK);
     },
 
     watch: function(c) {
         LOG("watch called");
         if (!this.wifi_service) {
             this.wifi_service = Cc["@mozilla.org/wifi/monitor;1"].getService(Components.interfaces.nsIWifiMonitor);
             this.wifi_service.startWatching(this);
         }
@@ -322,41 +313,47 @@ WifiGeoPositionProvider.prototype = {
         return accessToken;
     },
 
     onChange: function(accessPoints) {
 
         LOG("onChange called");
         this.hasSeenWiFi = true;
 
-        // Cache the preferred protocol for use inside the XHR callback
-        var protocol = this.protocol;
-
         // send our request to a wifi geolocation network provider:
         var xhr = Components.classes["@mozilla.org/xmlextras/xmlhttprequest;1"].createInstance(Ci.nsIXMLHttpRequest);
 
         // This is a background load
         xhr.mozBackgroundRequest = true;
 
-        xhr.open("POST", this.provider_url, false);
+        var provider_url = this.prefService.getCharPref("geo.wifi.uri");
+        LOG("provider url = " + provider_url);
+
+        var protocol = 0;
+        try {
+            protocol = this.prefService.getIntPref("geo.wifi.protocol");
+        } catch (e) {}
+        LOG("protocol = " + protocol);
+
+        xhr.open("POST", provider_url, false);
         
         // set something so that we can strip cookies
         xhr.channel.loadFlags = Ci.nsIChannel.LOAD_ANONYMOUS;
             
         xhr.onerror = function(req) {
             LOG("onerror: " + req);
         };
 
         xhr.onload = function (req) {  
 
             LOG("xhr onload...");
 
             // if we get a bad response, we will throw and never report a location
             var response;
-            switch (this.protocol) {
+            switch (protocol) {
                 case 1:
                     LOG("service returned: " + req.target.responseXML);
                     response = HELD.decode(req.target.responseXML);
                     break;
                 case 0:
                 default:
                     LOG("service returned: " + req.target.responseText);
                     response = JSON.parse(req.target.responseText);
@@ -393,17 +390,17 @@ WifiGeoPositionProvider.prototype = {
             LOG("sending update to geolocation.");
 
             var newLocation = new WifiGeoPositionObject(response.location, address);
 
             var update = Cc["@mozilla.org/geolocation/service;1"].getService(Ci.nsIGeolocationUpdate);
             update.update(newLocation);
         };
 
-        var accessToken = this.getAccessTokenForURL(this.provider_url);
+        var accessToken = this.getAccessTokenForURL(provider_url);
 
         var request = {
             version: "1.1.0",
             request_address: true,
         };
 
         if (accessToken != "")
             request.access_token = accessToken;
@@ -434,19 +431,24 @@ WifiGeoPositionProvider.prototype = {
         } catch (e) {}
     },
 
     onError: function (code) {
         LOG("wifi error: " + code);
     },
 
     notify: function (timer) {
-        if (this.hasSeenWiFi == false)
-            this.onChange(null);
-        this.timer = null;
+        if (!gTestingEnabled) {
+            if (this.hasSeenWiFi == false)
+                this.onChange(null);
+            this.timer = null;
+            return;
+        }
+        // if we are testing, we need to hammer this.
+        this.onChange(null);
     },
 
 };
 
 var components = [WifiGeoPositionProvider];
 function NSGetModule(compMgr, fileSpec) {
   return XPCOMUtils.generateModule(components);
 }
--- a/dom/src/geolocation/nsGeolocation.cpp
+++ b/dom/src/geolocation/nsGeolocation.cpp
@@ -129,17 +129,16 @@ nsDOMGeoPositionError::NotifyCallback(ns
 ////////////////////////////////////////////////////
 
 nsGeolocationRequest::nsGeolocationRequest(nsGeolocation* aLocator,
                                            nsIDOMGeoPositionCallback* aCallback,
                                            nsIDOMGeoPositionErrorCallback* aErrorCallback,
                                            nsIDOMGeoPositionOptions* aOptions)
   : mAllowed(PR_FALSE),
     mCleared(PR_FALSE),
-    mHasSentData(PR_FALSE),
     mCallback(aCallback),
     mErrorCallback(aErrorCallback),
     mOptions(aOptions),
     mLocator(aLocator)
 {
 }
 
 nsGeolocationRequest::~nsGeolocationRequest()
@@ -186,21 +185,19 @@ nsGeolocationRequest::NotifyError(PRInt1
 
 NS_IMETHODIMP
 nsGeolocationRequest::Notify(nsITimer* aTimer)
 {
   // If we haven't gotten an answer from the geolocation
   // provider yet, cancel the request.  Same logic as
   // ::Cancel, just a different error
   
-  if (!mHasSentData) {
-    NotifyError(nsIDOMGeoPositionError::TIMEOUT);
-    // remove ourselves from the locator's callback lists.
-    mLocator->RemoveRequest(this);
-  }
+  NotifyError(nsIDOMGeoPositionError::TIMEOUT);
+  // remove ourselves from the locator's callback lists.
+  mLocator->RemoveRequest(this);
 
   mTimeoutTimer = nsnull;
   return NS_OK;
 }
  
 NS_IMETHODIMP
 nsGeolocationRequest::GetRequestingURI(nsIURI * *aRequestingURI)
 {
@@ -272,42 +269,57 @@ nsGeolocationRequest::Allow()
   if (lastPosition && maximumAge > 0 && ( (PR_Now() / PR_USEC_PER_MSEC ) - maximumAge <= cachedPositionTime) ) {
     // okay, we can return a cached position
     mAllowed = PR_TRUE;
     
     // send the cached location
     SendLocation(lastPosition);
   }
 
+  SetTimeoutTimer();
+
+  mAllowed = PR_TRUE;
+  return NS_OK;
+}
+
+void
+nsGeolocationRequest::SetTimeoutTimer()
+{
+  if (mTimeoutTimer) {
+    mTimeoutTimer->Cancel();
+    mTimeoutTimer = nsnull;
+  }
   PRInt32 timeout;
   if (mOptions && NS_SUCCEEDED(mOptions->GetTimeout(&timeout)) && timeout > 0) {
     
     if (timeout < 10)
       timeout = 10;
 
     mTimeoutTimer = do_CreateInstance("@mozilla.org/timer;1");
     mTimeoutTimer->InitWithCallback(this, timeout, nsITimer::TYPE_ONE_SHOT);
   }
-
-  mAllowed = PR_TRUE;
-  return NS_OK;
 }
 
 void
 nsGeolocationRequest::MarkCleared()
 {
   mCleared = PR_TRUE;
 }
 
 void
 nsGeolocationRequest::SendLocation(nsIDOMGeoPosition* aPosition)
 {
   if (mCleared || !mAllowed)
     return;
 
+  if (mTimeoutTimer) {
+    mTimeoutTimer->Cancel();
+    mTimeoutTimer = nsnull;
+  }
+
   // we should not pass null back to the DOM.
   if (!aPosition) {
     NotifyError(nsIDOMGeoPositionError::POSITION_UNAVAILABLE);
     return;
   }
 
   // Ensure that the proper context is on the stack (bug 452762)
   nsCOMPtr<nsIJSContextStack> stack(do_GetService("@mozilla.org/js/xpc/ContextStack;1"));
@@ -315,17 +327,17 @@ nsGeolocationRequest::SendLocation(nsIDO
     return; // silently fail
   
   mCallback->HandleEvent(aPosition);
 
   // remove the stack
   JSContext* cx;
   stack->Pop(&cx);
 
-  mHasSentData = PR_TRUE;
+  SetTimeoutTimer();
 }
 
 void
 nsGeolocationRequest::Shutdown()
 {
   mCleared = PR_TRUE;
   mCallback = nsnull;
   mErrorCallback = nsnull;
@@ -340,16 +352,18 @@ NS_INTERFACE_MAP_BEGIN(nsGeolocationServ
   NS_INTERFACE_MAP_ENTRY(nsIObserver)
 NS_INTERFACE_MAP_END
 
 NS_IMPL_THREADSAFE_ADDREF(nsGeolocationService)
 NS_IMPL_THREADSAFE_RELEASE(nsGeolocationService)
 
 
 static PRBool sGeoEnabled = PR_TRUE;
+static PRBool sGeoIgnoreLocationFilter = PR_FALSE;
+
 static int
 GeoEnabledChangedCallback(const char *aPrefName, void *aClosure)
 {
   sGeoEnabled = nsContentUtils::GetBoolPref("geo.enabled", PR_TRUE);
   return 0;
 }
 
 nsresult nsGeolocationService::Init()
@@ -471,24 +485,27 @@ nsGeolocationService::Update(nsIDOMGeoPo
 
   for (PRUint32 i = 0; i< mGeolocators.Length(); i++)
     mGeolocators[i]->Update(aSomewhere);
   return NS_OK;
 }
 
 PRBool
 nsGeolocationService::IsBetterPosition(nsIDOMGeoPosition *aSomewhere)
-{
+{  
   if (!aSomewhere)
     return PR_FALSE;
 
   nsRefPtr<nsGeolocationService> geoService = nsGeolocationService::GetInstance();
   if (!geoService)
     return PR_FALSE;
 
+  if (sGeoIgnoreLocationFilter)
+    return PR_TRUE;
+
   nsCOMPtr<nsIDOMGeoPosition> lastPosition = geoService->GetCachedPosition();
   if (!lastPosition)
     return PR_TRUE;
   
   nsresult rv;
   DOMTimeStamp oldTime;
   rv = lastPosition->GetTimestamp(&oldTime);
   NS_ENSURE_SUCCESS(rv, PR_FALSE);
--- a/dom/src/geolocation/nsGeolocation.h
+++ b/dom/src/geolocation/nsGeolocation.h
@@ -75,25 +75,25 @@ class nsGeolocationRequest : public nsIG
                        nsIDOMGeoPositionErrorCallback* errorCallback,
                        nsIDOMGeoPositionOptions* options);
   nsresult Init();
   void Shutdown();
 
   void SendLocation(nsIDOMGeoPosition* location);
   void MarkCleared();
   PRBool Allowed() {return mAllowed;}
+  void SetTimeoutTimer();
 
   ~nsGeolocationRequest();
 
  private:
 
   void NotifyError(PRInt16 errorCode);
   PRPackedBool mAllowed;
   PRPackedBool mCleared;
-  PRPackedBool mHasSentData;
 
   nsCOMPtr<nsITimer> mTimeoutTimer;
   nsCOMPtr<nsIDOMGeoPositionCallback> mCallback;
   nsCOMPtr<nsIDOMGeoPositionErrorCallback> mErrorCallback;
   nsCOMPtr<nsIDOMGeoPositionOptions> mOptions;
 
   nsRefPtr<nsGeolocation> mLocator;
 };
--- a/dom/tests/mochitest/geolocation/Makefile.in
+++ b/dom/tests/mochitest/geolocation/Makefile.in
@@ -40,16 +40,17 @@ srcdir		= @srcdir@
 VPATH		= @srcdir@
 relativesrcdir	= dom/tests/mochitest/geolocation
 
 include $(DEPTH)/config/autoconf.mk
 
 include $(topsrcdir)/config/rules.mk
 
 _TEST_FILES	= \
+        test_timerRestartWatch.html \
         test_manyCurrentSerial.html \
         test_manyCurrentConcurrent.html \
 		test_garbageWatch.html \
 		test_manyWatchConcurrent.html \
 		test_manyWatchSerial.html \
 		test_manyWindows.html \
 		test_allowCurrent.html \
 		test_allowWatch.html \
new file mode 100644
--- /dev/null
+++ b/dom/tests/mochitest/geolocation/test_timerRestartWatch.html
@@ -0,0 +1,64 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=526326
+-->
+<head>
+  <title>Test for watchPosition </title>
+  <script type="text/javascript" src="/MochiKit/packed.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="geolocation_common.js"></script>
+
+<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=526326">Mozilla Bug 526326</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+
+var watchID;
+var hasAccepted = false;
+
+function errorCallback(err) {
+  ok(err.code == err.TIMEOUT, "ensure error is a timeout.");
+  resume_geolocationProvider();
+  SimpleTest.finish();
+}
+
+function successCallback(position) {
+  ok(hasAccepted, "Ensure that accept was pressed");
+
+  // Now that we got a success callback, lets try to ensure
+  // that we get a timeout error.
+
+  stop_geolocationProvider();
+}
+
+function accept() {
+  hasAccepted = true;
+  clickNotificationButton(kAcceptButton);
+}
+
+/** Test for Bug  **/
+
+SimpleTest.waitForExplicitFinish();
+
+
+var options = {
+    maximumAge: 0,
+    timeout: 1000
+};
+
+watchID = navigator.geolocation.watchPosition(successCallback, errorCallback, options);
+
+setTimeout(accept, 50);
+
+</script>
+</pre>
+</body>
+</html>
+